Am 05.02.2020 um 11:03 hat Markus Armbruster geschrieben:
> Kevin Wolf <kw...@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".
"new secret inactive" can't select a slot by secret because 'secret'
doesn't even exist for 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.
Right. I wonder if qemu-img wants support for that specifically
(possibly with allowing to enter the key interactively) rather than
requiring the user to call qemu-img amend twice.
> > 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.
Yes. The usual case is that you just want to add a new key somwhere.
> > 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.
Not with your definition above, but with the appropriate changes, this
makes sense.
> > 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 :)
Not really. I mean, I really know to appreciate the advantages of
-blockdev where needed, but usually I don't want to type all that stuff
for the most common tasks. qemu-img amend is similar.
For deleting, I might actually agree that explicitness is an
improvement, but for creating it's just unnecessary verbosity.
> 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.
I guess this would improve the usability in this case.
Kevin
