[Qemu-devel] [PATCH v2 04/10] qapi: Formalize qcow2 encryption probing

Max Reitz posted 10 patches 7 years, 8 months ago
[Qemu-devel] [PATCH v2 04/10] qapi: Formalize qcow2 encryption probing
Posted by Max Reitz 7 years, 8 months ago
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 ++++++++++++++++++++++++++++++++++++++++----
 block/qcow2.c        |  3 +++
 2 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 4bd85dd1bb..295ace42ae 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -44,6 +44,19 @@
 { 'struct': 'ImageInfoSpecificQCow2EncryptionBase',
   'data': { 'format': 'BlockdevQcow2EncryptionFormat'}}
 
+##
+# @ImageInfoSpecificQCow2EncryptionNoInfo:
+#
+# Only used for the qcow2 encryption format "auto" in which the actual
+# encryption format is determined from the image header.  Therefore,
+# this encryption format will never be reported in
+# ImageInfoSpecificQCow2Encryption.
+#
+# Since: 3.0
+##
+{ 'struct': 'ImageInfoSpecificQCow2EncryptionNoInfo',
+  'data': { } }
+
 ##
 # @ImageInfoSpecificQCow2Encryption:
 #
@@ -53,7 +66,8 @@
   'base': 'ImageInfoSpecificQCow2EncryptionBase',
   'discriminator': 'format',
   'data': { 'aes': 'QCryptoBlockInfoQCow',
-            'luks': 'QCryptoBlockInfoLUKS' } }
+            'luks': 'QCryptoBlockInfoLUKS',
+            'auto': 'ImageInfoSpecificQCow2EncryptionNoInfo' } }
 
 ##
 # @ImageInfoSpecificQCow2:
@@ -2658,10 +2672,30 @@
 # @BlockdevQcow2EncryptionFormat:
 # @aes: AES-CBC with plain64 initialization venctors
 #
+# @auto:            Determine the encryption format from the image
+#                   header.  This only allows the use of the
+#                   key-secret option.  (Since: 3.0)
+#
 # Since: 2.10
 ##
 { 'enum': 'BlockdevQcow2EncryptionFormat',
-  'data': [ 'aes', 'luks' ] }
+  'data': [ 'aes', 'luks', 'auto' ] }
+
+##
+# @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: 3.0
+##
+{ 'struct': 'BlockdevQcow2EncryptionSecret',
+  'data': { '*key-secret': 'str' } }
 
 ##
 # @BlockdevQcow2Encryption:
@@ -2669,10 +2703,12 @@
 # Since: 2.10
 ##
 { 'union': 'BlockdevQcow2Encryption',
-  'base': { 'format': 'BlockdevQcow2EncryptionFormat' },
+  'base': { '*format': 'BlockdevQcow2EncryptionFormat' },
   'discriminator': 'format',
+  'default-variant': 'auto',
   'data': { 'aes': 'QCryptoBlockOptionsQCow',
-            'luks': 'QCryptoBlockOptionsLUKS'} }
+            'luks': 'QCryptoBlockOptionsLUKS',
+            'auto': 'BlockdevQcow2EncryptionSecret' } }
 
 ##
 # @BlockdevOptionsQcow2:
diff --git a/block/qcow2.c b/block/qcow2.c
index 945132f692..6bfcba4ee1 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -868,6 +868,9 @@ static int qcow2_update_options_prepare(BlockDriverState *bs,
 
     qdict_extract_subqdict(options, &encryptopts, "encrypt.");
     encryptfmt = qdict_get_try_str(encryptopts, "format");
+    if (!g_strcmp0(encryptfmt, "auto")) {
+        encryptfmt = NULL;
+    }
 
     opts = qemu_opts_create(&qcow2_runtime_opts, NULL, 0, &error_abort);
     qemu_opts_absorb_qdict(opts, options, &local_err);
-- 
2.17.1


Re: [Qemu-devel] [PATCH v2 04/10] qapi: Formalize qcow2 encryption probing
Posted by Eric Blake 7 years, 8 months ago
On 06/11/2018 03:51 PM, 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 ++++++++++++++++++++++++++++++++++++++++----
>   block/qcow2.c        |  3 +++
>   2 files changed, 43 insertions(+), 4 deletions(-)

> +##
> +# @ImageInfoSpecificQCow2EncryptionNoInfo:
> +#
> +# Only used for the qcow2 encryption format "auto" in which the actual
> +# encryption format is determined from the image header.  Therefore,
> +# this encryption format will never be reported in
> +# ImageInfoSpecificQCow2Encryption.
> +#
> +# Since: 3.0
> +##
> +{ 'struct': 'ImageInfoSpecificQCow2EncryptionNoInfo',
> +  'data': { } }

Do we actually need this type, given Anton's work on making omitted 
branches automatically use an empty struct?

https://lists.gnu.org/archive/html/qemu-devel/2018-05/msg06836.html

> +
>   ##
>   # @ImageInfoSpecificQCow2Encryption:
>   #
> @@ -53,7 +66,8 @@
>     'base': 'ImageInfoSpecificQCow2EncryptionBase',
>     'discriminator': 'format',
>     'data': { 'aes': 'QCryptoBlockInfoQCow',
> -            'luks': 'QCryptoBlockInfoLUKS' } }
> +            'luks': 'QCryptoBlockInfoLUKS',
> +            'auto': 'ImageInfoSpecificQCow2EncryptionNoInfo' } }

If Anton's patches go in first, you don't even have to change this type;

>   
>   ##
>   # @ImageInfoSpecificQCow2:
> @@ -2658,10 +2672,30 @@
>   # @BlockdevQcow2EncryptionFormat:
>   # @aes: AES-CBC with plain64 initialization venctors
>   #
> +# @auto:            Determine the encryption format from the image
> +#                   header.  This only allows the use of the
> +#                   key-secret option.  (Since: 3.0)
> +#
>   # Since: 2.10
>   ##
>   { 'enum': 'BlockdevQcow2EncryptionFormat',
> -  'data': [ 'aes', 'luks' ] }
> +  'data': [ 'aes', 'luks', 'auto' ] }

the changed enum would be sufficient.

> +
> +##
> +# @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: 3.0
> +##
> +{ 'struct': 'BlockdevQcow2EncryptionSecret',
> +  'data': { '*key-secret': 'str' } }
>   
>   ##
>   # @BlockdevQcow2Encryption:
> @@ -2669,10 +2703,12 @@
>   # Since: 2.10
>   ##
>   { 'union': 'BlockdevQcow2Encryption',
> -  'base': { 'format': 'BlockdevQcow2EncryptionFormat' },
> +  'base': { '*format': 'BlockdevQcow2EncryptionFormat' },
>     'discriminator': 'format',
> +  'default-variant': 'auto',
>     'data': { 'aes': 'QCryptoBlockOptionsQCow',
> -            'luks': 'QCryptoBlockOptionsLUKS'} }
> +            'luks': 'QCryptoBlockOptionsLUKS',
> +            'auto': 'BlockdevQcow2EncryptionSecret' } }

This part is necessary, though, and looks correct.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH v2 04/10] qapi: Formalize qcow2 encryption probing
Posted by Max Reitz 7 years, 8 months ago
On 2018-06-11 23:02, Eric Blake wrote:
> On 06/11/2018 03:51 PM, 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 ++++++++++++++++++++++++++++++++++++++++----
>>   block/qcow2.c        |  3 +++
>>   2 files changed, 43 insertions(+), 4 deletions(-)
> 
>> +##
>> +# @ImageInfoSpecificQCow2EncryptionNoInfo:
>> +#
>> +# Only used for the qcow2 encryption format "auto" in which the actual
>> +# encryption format is determined from the image header.  Therefore,
>> +# this encryption format will never be reported in
>> +# ImageInfoSpecificQCow2Encryption.
>> +#
>> +# Since: 3.0
>> +##
>> +{ 'struct': 'ImageInfoSpecificQCow2EncryptionNoInfo',
>> +  'data': { } }
> 
> Do we actually need this type, given Anton's work on making omitted
> branches automatically use an empty struct?
> 
> https://lists.gnu.org/archive/html/qemu-devel/2018-05/msg06836.html

Looks like no, we don't.  Great! :-)

I think I'll still keep part of the comment and move it down into the
description of ImageInfoSpecificQCow2Encryption so that anyone who's
wondering knows that this value won't appear.

Thanks for reviewing and pointing me at it,

Max

Re: [Qemu-devel] [PATCH v2 04/10] qapi: Formalize qcow2 encryption probing
Posted by Daniel P. Berrangé 7 years, 8 months ago
On Mon, Jun 11, 2018 at 10:51:57PM +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 ++++++++++++++++++++++++++++++++++++++++----
>  block/qcow2.c        |  3 +++
>  2 files changed, 43 insertions(+), 4 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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