Re: [Qemu-devel] [PATCH v2 02/11] qcrypto-luks: extend the create options for upcoming encryption key management

From: Markus Armbruster <armbru@redhat.com>
To: Maxim Levitsky <mlevitsk@redhat.com>
Cc: "Kevin Wolf" <kwolf@redhat.com>,
	"Daniel P. Berrangé" <berrange@redhat.com>,
	qemu-block@nongnu.org, qemu-devel@nongnu.org,
	"Max Reitz" <mreitz@redhat.com>, "John Snow" <jsnow@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v2 02/11] qcrypto-luks: extend the create options for upcoming encryption key management
Date: Mon, 07 Oct 2019 09:49:19 +0200	[thread overview]
Message-ID: <87imp1j8qo.fsf@dusky.pond.sub.org> (raw)
In-Reply-To: <20190912223028.18496-3-mlevitsk@redhat.com> (Maxim Levitsky's message of "Fri, 13 Sep 2019 01:30:19 +0300")

Quick QAPI schema review only.

Maxim Levitsky <mlevitsk@redhat.com> writes:

> Now you can specify which slot to put the encryption key to
> Plus add 'active' option which will let  user erase the key secret
> instead of adding it.
> Check that active=true it when creating.
>
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
[...]
> diff --git a/qapi/crypto.json b/qapi/crypto.json
> index b2a4cff683..9b83a70634 100644
> --- a/qapi/crypto.json
> +++ b/qapi/crypto.json
> @@ -190,6 +190,20 @@
   ##
   # @QCryptoBlockCreateOptionsLUKS:
   #
   # The options that apply to LUKS encryption format initialization
   #
   # @cipher-alg: the cipher algorithm for data encryption
   #              Currently defaults to 'aes-256'.
   # @cipher-mode: the cipher mode for data encryption
   #               Currently defaults to 'xts'
   # @ivgen-alg: the initialization vector generator
   #             Currently defaults to 'plain64'
   # @ivgen-hash-alg: the initialization vector generator hash
>  #                  Currently defaults to 'sha256'
>  # @hash-alg: the master key hash algorithm
>  #            Currently defaults to 'sha256'
> +#
> +# @active: Should the new secret be added (true) or erased (false)
> +#          (amend only, since 4.2)

Is "active" established terminology?  I wouldn't have guessed its
meaning from its name...

As far as I can see, QCryptoBlockCreateOptionsLUKS is used just for
blockdev-create with options.driver \in { luks, qcow, qcow2 }:

   { 'command': 'blockdev-create',
     'data': { ...
               'options': 'BlockdevCreateOptions' } }

   { 'union': 'BlockdevCreateOptions',
     ...
     'data': {
         ...
         'luks':           'BlockdevCreateOptionsLUKS',
         ...
         'qcow':           'BlockdevCreateOptionsQcow',
         'qcow2':          'BlockdevCreateOptionsQcow2',
         ... } }

With luks:

   { 'struct': 'BlockdevCreateOptionsLUKS',
     'base': 'QCryptoBlockCreateOptionsLUKS',
     ... }

With qcow and qcow2:

    { 'struct': 'BlockdevCreateOptionsQcow',
      'data': { ...
                '*encrypt':         'QCryptoBlockCreateOptions' } }
    { 'struct': 'BlockdevCreateOptionsQcow2',
      'data': { ...
                '*encrypt':         'QCryptoBlockCreateOptions',
                ... } }

    { 'union': 'QCryptoBlockCreateOptions',
      'base': 'QCryptoBlockOptionsBase',
      'discriminator': 'format',
      'data': { ...
                'luks': 'QCryptoBlockCreateOptionsLUKS' } }

I think I understand why we want blockdev-create to be able to specify a
new secret.

Why do we want it to be able to delete an existing secret?  How would
that even work?  Color me confused...

> +#
> +# @slot: The slot in which to put/erase the secret
> +#        if not given, will select first free slot for secret addtion
> +#        and erase all matching keyslots for erase. except last one
> +#        (optional, since 4.2)

Excuse my possibly ignorant question: what exactly is a "matching
keyslot"?

> +#
> +# @unlock-secret: The secret to use to unlock the image
> +#        If not given, will use the secret that was used
> +#        when opening the image.
> +#        (optional, for amend only, since 4.2)

More ignorance: what is "amend"?  No mention of it in qapi/*json...

> +#
>  # @iter-time: number of milliseconds to spend in
>  #             PBKDF passphrase processing. Currently defaults
>  #             to 2000. (since 2.8)
> @@ -201,7 +215,12 @@
>              '*cipher-mode': 'QCryptoCipherMode',
>              '*ivgen-alg': 'QCryptoIVGenAlgorithm',
>              '*ivgen-hash-alg': 'QCryptoHashAlgorithm',
> +
>              '*hash-alg': 'QCryptoHashAlgorithm',
> +            '*active' : 'bool',
> +            '*slot': 'int',
> +            '*unlock-secret': 'str',
> +
>              '*iter-time': 'int'}}
>  
>  
[...]