Re: [PATCH 02/13] qcrypto-luks: implement encryption key management

[Markus Armbruster <armbru@redhat.com>]

Kevin Wolf <kwolf@redhat.com> writes:

> Am 05.02.2020 um 09:24 hat Markus Armbruster geschrieben:
>> Daniel, Kevin, any comments or objections to the QAPI schema design
>> sketch developed below?
>> 
>> For your convenience, here's the result again:
>> 
>>     { 'enum': 'LUKSKeyslotState',
>>       'data': [ 'active', 'inactive' ] }
>>     { 'struct': 'LUKSKeyslotActive',
>>       'data': { 'secret': 'str',
>>                 '*iter-time': 'int } }
>>     { 'union': 'LUKSKeyslotAmend',
>>       'base': { '*keyslot': 'int',
>>                 'state': 'LUKSKeyslotState' }
>>       'discriminator': 'state',
>>       'data': { 'active': 'LUKSKeyslotActive' } }
>
> I think one of the requirements was that you can specify the keyslot not
> only by using its number, but also by specifying the old secret.

Quoting myself:

  When we don't specify the slot#, then "new state active" selects an
  inactive slot (chosen by the system, and "new state inactive selects
  slots by secret (commonly just one slot).

This takes care of selecting (active) slots by old secret with "new
state inactive".

I intentionally did not provide for selecting (active) slots by old
secret with "new state active", because that's unsafe update in place.

We want to update secrets, of course.  But the safe way to do that is to
put the new secret into a free slot, and if that succeeds, deactivate
the old secret.  If deactivation fails, you're left with both old and
new secret, which beats being left with no secret when update in place
fails.

>                                                                  Trivial
> extension, you just get another optional field that can be specified
> instead of 'keyslot'.
>
> Resulting commands:
>
>     Adding a key:
>     qemu-img amend -o encrypt.keys.0.state=active,encrypt.keys.0.secret=sec0 test.qcow2

This activates an inactive slot chosen by the sysem.

You can activate a specific keyslot N by throwing in
encrypt.keys.0.keyslot=N.

>     Deleting a key:
>     qemu-img amend -o encrypt.keys.0.state=inactive,encrypt.keys.0.keyslot=2 test.qcow2

This deactivates keyslot#2.

You can deactivate slots holding a specific secret S by replacing
encrypt.keys.0.keyslot=2 by encrypt.keys.0.secret=S.

> Previous version (if this series is applied unchanged):
>
>     Adding a key:
>     qemu-img amend -o encrypt.keys.0.new-secret=sec0 test.qcow2
>
>     Deleting a key:
>     qemu-img amend -o encrypt.keys.0.new-secret=,encrypt.keys.0.keyslot=2 test.qcow2
>
> Adding a key gets more complicated with your proposed interface because
> state must be set explicitly now whereas before it was derived
> automatically from the fact that if you give a key, only active makes
> sense.

The explicitness could be viewed as an improvement :)

If you'd prefer implicit here: Max has patches for making union tags
optional with a default.  They'd let you default active to true.

> Deleting stays more or less the same, you just change the state instead
> of clearing the secret.

Thanks for your input!