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

[Fam Zheng]

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?

> +    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.

> +    qcrypto_ivgen_essiv_calculate,
> +    qcrypto_ivgen_essiv_cleanup,
> +};
> +