Re: QAPI schema for desired state of LUKS keyslots

[Markus Armbruster <armbru@redhat.com>]

Maxim Levitsky <mlevitsk@redhat.com> writes:

> On Sat, 2020-02-15 at 15:51 +0100, Markus Armbruster wrote:
>> Review of this patch led to a lengthy QAPI schema design discussion.
>> Let me try to condense it into a concrete proposal.
>> 
>> This is about the QAPI schema, and therefore about QMP.  The
>> human-friendly interface is out of scope.  Not because it's not
>> important (it clearly is!), only because we need to *focus* to have a
>> chance at success.
> 100% agree.
>> 
>> I'm going to include a few design options.  I'll mark them "Option:".
>> 
>> The proposed "amend" interface takes a specification of desired state,
>> and figures out how to get from here to there by itself.  LUKS keyslots
>> are one part of desired state.
>> 
>> We commonly have eight LUKS keyslots.  Each keyslot is either active or
>> inactive.  An active keyslot holds a secret.
>> 
>> Goal: a QAPI type for specifying desired state of LUKS keyslots.
>> 
>> Proposal:
>> 
>>     { 'enum': 'LUKSKeyslotState',
>>       'data': [ 'active', 'inactive' ] }
>> 
>>     { 'struct': 'LUKSKeyslotActive',
>>       'data': { 'secret': 'str',
>>                 '*iter-time': 'int } }
>> 
>>     { 'struct': 'LUKSKeyslotInactive',
>>       'data': { '*old-secret': 'str' } }
>> 
>>     { 'union': 'LUKSKeyslotAmend',
>>       'base': { '*keyslot': 'int',
>>                 'state': 'LUKSKeyslotState' }
>>       'discriminator': 'state',
>>       'data': { 'active': 'LUKSKeyslotActive',
>>                 'inactive': 'LUKSKeyslotInactive' } }
>> 
>> LUKSKeyslotAmend specifies desired state for a set of keyslots.
>> 
>> Four cases:
>> 
>> * @state is "active"
>> 
>>   Desired state is active holding the secret given by @secret.  Optional
>>   @iter-time tweaks key stretching.
>> 
>>   The keyslot is chosen either by the user or by the system, as follows:
>> 
>>   - @keyslot absent
>> 
>>     One inactive keyslot chosen by the system.  If none exists, error.
>> 
>>   - @keyslot present
>> 
>>     The keyslot given by @keyslot.
>> 
>>     If it's already active holding @secret, no-op.  Rationale: the
>>     current state is the desired state.
>> 
>>     If it's already active holding another secret, error.  Rationale:
>>     update in place is unsafe.
>> 
>>     Option: delete the "already active holding @secret" case.  Feels
>>     inelegant to me.  Okay if it makes things substantially simpler.
> I didn't really understand this, since in state=active we shouldn't
> delete anything. Looks OK otherwise.

Let me try to clarify.

Option: make the "already active holding @secret" case an error like the
"already active holding another secret" case.  In longhand:

     - @keyslot present

       The keyslot given by @keyslot.

       If it's already active, error.

It feels inelegant to me, because it deviates from "specify desired
state" paradigm: the specified desired state is fine, the way to get
there from current state is obvious (do nothing), yet it's still an
error.

>> * @state is "inactive"
>> 
>>   Desired state is inactive.
>> 
>>   Error if the current state has active keyslots, but the desired state
>>   has none.
>> 
>>   The user choses the keyslot by number and/or by the secret it holds,
>>   as follows:
>> 
>>   - @keyslot absent, @old-secret present
>> 
>>     All active keyslots holding @old-secret.  If none exists, error.
>> 
>>   - @keyslot present, @old-secret absent
>> 
>>     The keyslot given by @keyslot.
>> 
>>     If it's already inactive, no-op.  Rationale: the current state is
>>     the desired state.
>> 
>>   - both @keyslot and @old-secret present
>> 
>>     The keyslot given by keyslot.
>> 
>>     If it's inactive or holds a secret other than @old-secret, error.
> Yea, that would be very nice to have.
>> 
>>     Option: error regardless of @old-secret, if that makes things
>>     simpler.
>> 
>>   - neither @keyslot not @old-secret present
>> 
>>     All keyslots.  Note that this will error out due to "desired state
>>     has no active keyslots" unless the current state has none, either.
>> 
>>     Option: error out unconditionally.
> Yep, that the best IMHO.

It's a matter of taste.

If we interpret "both absent" as "all keyslots", then all cases flow out
of the following simple spec:

    0. Start with the set of all keyslots

    1. If @old-secret is present, interset it with the set of slots
       holding that secret.

    2. If @keyslots is present, intersect it with the set of slots with
       that slot number.

The order of steps 1 and 2 doesn't matter.

To error out unconditionally, we have to make "both absent" a special
case.

A good way to resolve such matters of taste is to try writing doc
comments for all proposals.  If you find you hate one of them much less,
you have a winner :)

[...]