Re: [Qemu-block] [PATCH v5 11/18] qcow2: convert QCow2 to use QCryptoBlock for encryption

[Alberto Garcia]

On Tue 21 Feb 2017 12:55:05 PM CET, Daniel P. Berrange wrote:
> +    switch (s->crypt_method_header) {
> +    case QCOW_CRYPT_NONE:
> +        break;
> +
> +    case QCOW_CRYPT_AES:
> +        r->crypto_opts = block_crypto_open_opts_init(
> +            Q_CRYPTO_BLOCK_FORMAT_QCOW, opts, "aes-", errp);
> +        break;
> +
> +    default:
> +        error_setg(errp, "Unsupported encryption method %d",
> +                   s->crypt_method_header);
> +        break;
> +    }
> +    if (s->crypt_method_header && !r->crypto_opts) {
> +        ret = -EINVAL;
> +        goto fail;
> +    }

This last condition relies on the assumption that QCOW_CRYPT_NONE == 0.

I think it's safe to assume that its value is never going to change and
therefore this isn't too important, but I'm just pointing it out in case
you want to make it explicit.

> @@ -1122,6 +1145,24 @@ static int qcow2_open(BlockDriverState *bs, QDict 
> *options, int flags,
>          goto fail;
>      }
>  
> +    if (s->crypt_method_header == QCOW_CRYPT_AES) {
> +        unsigned int cflags = 0;
> +        if (flags & BDRV_O_NO_IO) {
> +            cflags |= QCRYPTO_BLOCK_OPEN_NO_IO;
> +        }
> +        /* TODO how do we pass the same crypto opts down to the
> +         * backing file by default, so we don't have to manually
> +         * provide the same key-secret property against the full
> +         * backing chain
> +         */
> +        s->crypto = qcrypto_block_open(s->crypto_opts, NULL, NULL,
> +                                       cflags, errp);
> +        if (!s->crypto) {
> +            ret = -EINVAL;
> +            goto fail;
> +        }
> +    }

Actually this has the same problem that I mentioned for patch 9: if
qcow2_open() fails then s->crypto is leaked.

Berto