Re: [Qemu-block] [Qemu-devel] [PATCH 04/13] qapi: Formalize qcow2 encryption probing

[Daniel P . Berrangé]

On Wed, May 09, 2018 at 06:55:21PM +0200, Max Reitz wrote:
> Currently, you can give no encryption format for a qcow2 file while
> still passing a key-secret.  That does not conform to the schema, so
> this patch changes the schema to allow it.
> 
> Signed-off-by: Max Reitz <address@hidden>
> ---
>  qapi/block-core.json | 44 ++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 40 insertions(+), 4 deletions(-)
> 
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 71c9ab8538..092a1aba2d 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -43,6 +43,19 @@
>  { 'struct': 'ImageInfoSpecificQCow2EncryptionBase',
>    'data': { 'format': 'BlockdevQcow2EncryptionFormat'}}
>  
> +##
> +# @ImageInfoSpecificQCow2EncryptionNoInfo:
> +#
> +# Only used for the qcow2 encryption format "from-image" in which the
> +# actual encryption format is determined from the image header.
> +# Therefore, this encryption format will never be reported in
> +# ImageInfoSpecificQCow2Encryption.
> +#
> +# Since: 2.13
> +##
> +{ 'struct': 'ImageInfoSpecificQCow2EncryptionNoInfo',
> +  'data': { } }
> +
>  ##
>  # @ImageInfoSpecificQCow2Encryption:
>  #
> @@ -52,7 +65,8 @@
>    'base': 'ImageInfoSpecificQCow2EncryptionBase',
>    'discriminator': 'format',
>    'data': { 'aes': 'QCryptoBlockInfoQCow',
> -            'luks': 'QCryptoBlockInfoLUKS' } }
> +            'luks': 'QCryptoBlockInfoLUKS',
> +            'from-image': 'ImageInfoSpecificQCow2EncryptionNoInfo' } }
>  
>  ##
>  # @ImageInfoSpecificQCow2:
> @@ -2739,10 +2753,30 @@
>  # @BlockdevQcow2EncryptionFormat:
>  # @aes: AES-CBC with plain64 initialization venctors
>  #
> +# @from-image:      Determine the encryption format from the image
> +#                   header.  This only allows the use of the
> +#                   key-secret option.  (Since: 2.13)
> +#
>  # Since: 2.10
>  ##
>  { 'enum': 'BlockdevQcow2EncryptionFormat',
> -  'data': [ 'aes', 'luks' ] }
> +  'data': [ 'aes', 'luks', 'from-image' ] }
> +
> +##
> +# @BlockdevQcow2EncryptionSecret:
> +#
> +# Allows specifying a key-secret without specifying the exact
> +# encryption format, which is determined automatically from the image
> +# header.
> +#
> +# @key-secret:      The ID of a QCryptoSecret object providing the
> +#                   decryption key.  Mandatory except when probing
> +#                   image for metadata only.
> +#
> +# Since: 2.13
> +##
> +{ 'struct': 'BlockdevQcow2EncryptionSecret',
> +  'data': { '*key-secret': 'str' } }
>  
>  ##
>  # @BlockdevQcow2Encryption:
> @@ -2750,10 +2784,12 @@
>  # Since: 2.10
>  ##
>  { 'union': 'BlockdevQcow2Encryption',
> -  'base': { 'format': 'BlockdevQcow2EncryptionFormat' },
> +  'base': { '*format': 'BlockdevQcow2EncryptionFormat' },
>    'discriminator': 'format',
> +  'default-variant': 'from-image',
>    'data': { 'aes': 'QCryptoBlockOptionsQCow',
> -            'luks': 'QCryptoBlockOptionsLUKS'} }
> +            'luks': 'QCryptoBlockOptionsLUKS',
> +            'from-image': 'BlockdevQcow2EncryptionSecret' } }

Bike-shedding on name, how about "auto" or "probe" ?

IIUC, this schema addition means the QAPI parser now allows

   encrypt.format=from-image,encrypt.key-secret=sec0,...other opts...

but the code will not accept "from-image" as a valid string.

eg qcow2_update_options_prepare() will do

    case QCOW_CRYPT_AES:
        if (encryptfmt && !g_str_equal(encryptfmt, "aes")) {
            error_setg(errp,
                       "Header reported 'aes' encryption format but "
                       "options specify '%s'", encryptfmt);
            ret = -EINVAL;
            goto fail;
        }

       ...snip....

    case QCOW_CRYPT_LUKS:
        if (encryptfmt && !g_str_equal(encryptfmt, "luks")) {
            error_setg(errp,
                       "Header reported 'luks' encryption format but "
                       "options specify '%s'", encryptfmt);
            ret = -EINVAL;
            goto fail;
        }


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|