| Message ID | 20180509165530.29561-5-mreitz@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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.
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 >
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:
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(-)