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

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 4554 bytes --]

On 2018-05-10 09:58, Daniel P. Berrangé wrote:
> 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 <mreitz@redhat.com>
>> ---
>>  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" ?

Sure.  I like "probe" a bit better than "auto", although "auto" is what
we usually have...  But I think "probe" is still a bit better.

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

Ah, right, I forgot that.  Will fix.

Thanks for reviewing!

Max

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



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]