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' } }
##
# @BlockdevOptionsQcow2:
--
2.14.3
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" ?
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 :|
On 05/10/2018 02:58 AM, 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(-)
>> { '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" ?
Either of those sounds nicer to me; 'auto' might be better in the
context of creation (that way, we can state that creating a NEW image
with x-blockdev-create maps 'auto' to 'luks'; while connecting to an
EXISTING image maps 'auto' to either 'aes' or 'luks' as appropriate).
>
> IIUC, this schema addition means the QAPI parser now allows
>
> encrypt.format=from-image,encrypt.key-secret=sec0,...other opts...
Yes. You could, perhaps, add a special case on the command line parsing
code to reject an explicit use of format=from-image, but the QMP should
not reject an explicit discriminator.
Hmm, it plays in with my comment on 1/13 - should the QMP parser
automatically set has_discriminator to true when it supplies the
default? If it does, you lose the ability to see whether the user
supplied an explicit encrypt.format=from-image (or the equivalent when
using QMP instead of the command line), if you wanted to enforce that
the user MUST omit format when relying on the from-image variant.
I don't see a problem in allowing the user to explicitly specify the
name of the default branch, but I _do_ think the patch is incomplete for
not handling the new QCOW_CRYPT_FROM_IMAGE case and converting it as
soon as possible back into one of the other two preferred enum values.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
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
>
© 2016 - 2025 Red Hat, Inc.