Re: [Qemu-block] [PATCH v2 04/17] crypto: add support for generating initialization vectors

[Daniel P. Berrange]

On Thu, Jan 21, 2016 at 03:51:37PM +0800, Fam Zheng wrote:
> On Wed, 01/20 17:38, Daniel P. Berrange wrote:
> > +static int qcrypto_ivgen_essiv_init(QCryptoIVGen *ivgen,
> > +                                    const uint8_t *key, size_t nkey,
> > +                                    Error **errp)
> > +{
> > +    uint8_t *salt;
> > +    size_t nhash;
> > +    QCryptoIVGenESSIV *essiv = g_new0(QCryptoIVGenESSIV, 1);
> > +
> > +    nhash = qcrypto_hash_digest_len(ivgen->hash);
> > +    /* Salt must be larger of hash size or key size */
> > +    salt = g_new0(uint8_t, nhash > nkey ? nhash : nkey);
> > +
> > +    if (qcrypto_hash_bytes(ivgen->hash, (const gchar *)key, nkey,
> > +                           &salt, &nhash,
> > +                           errp) < 0) {
> > +        g_free(essiv);
> > +        return -1;
> > +    }
> > +
> > +    essiv->cipher = qcrypto_cipher_new(ivgen->cipher,
> > +                                       QCRYPTO_CIPHER_MODE_ECB,
> > +                                       salt, nkey,
> > +                                       errp);
> > +    if (!essiv->cipher) {
> > +        g_free(essiv);
> > +        g_free(salt);
> > +        return -1;
> > +    }
> > +
> > +    g_free(salt);
> > +    ivgen->private = essiv;
> > +
> > +    return 0;
> > +}
> > +
> > +static int qcrypto_ivgen_essiv_calculate(QCryptoIVGen *ivgen,
> > +                                         uint64_t sector,
> > +                                         uint8_t *iv, size_t niv,
> > +                                         Error **errp)
> > +{
> > +    QCryptoIVGenESSIV *essiv = ivgen->private;
> > +    size_t ndata = qcrypto_cipher_get_block_len(ivgen->cipher);
> > +    uint8_t *data = g_new(uint8_t, ndata);
> > +
> > +    sector = cpu_to_le64((uint32_t)sector);
> 
> Why casting to 32 bit?

Bugtastic.

> > +    memcpy(data, (uint8_t *)&sector, ndata);
> > +    if (sizeof(sector) < ndata) {
> > +        memset(data + sizeof(sector), 0, ndata - sizeof(sector));
> > +    }
> > +
> > +    if (qcrypto_cipher_encrypt(essiv->cipher,
> > +                               data,
> > +                               data,
> > +                               ndata,
> > +                               errp) < 0) {
> > +        g_free(data);
> > +        return -1;
> > +    }
> > +
> > +    if (ndata > niv) {
> > +        ndata = niv;
> > +    }
> > +    memcpy(iv, data, ndata);
> > +    if (ndata < niv) {
> > +        memset(iv + ndata, 0, niv - ndata);
> > +    }
> > +    g_free(data);
> > +    return 0;
> > +}
> > +
> > +static void qcrypto_ivgen_essiv_cleanup(QCryptoIVGen *ivgen)
> > +{
> > +    QCryptoIVGenESSIV *essiv = ivgen->private;
> > +
> > +    qcrypto_cipher_free(essiv->cipher);
> > +    g_free(essiv);
> > +}
> > +
> > +
> > +struct QCryptoIVGenDriver qcrypto_ivgen_essiv = {
> > +    qcrypto_ivgen_essiv_init,
> 
> It'd be more readable to explicitly write
> 
>     .init = qcrypto_ivgen_essiv_init,
>     .calculate = ...,
>     .cleanup = ...
> 
> but it is fine since there are only three operations now.

Yeah, I normally do it that way anyway, so will change it.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|