Kevin Wolf <kw...@redhat.com> writes: > Am 15.02.2020 um 15:51 hat Markus Armbruster geschrieben: >> 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. >> >> 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. > > Though not arbitrary sets of keyslots, it's only a single keyslot or > multiple keyslots containing the same secret. Might be good enough in > practice, though it means that you may have to issue multiple amend > commands to get to the final state that you really want (even if doing > everything at once would be safe).
True. I traded expressiveness for simplicity. Here's the only practical case I can think of where the lack of expressiveness may hurt: replace secrets. With this interface, you need two operations: activate a free slot with the new secret, deactivate the slot(s) with the old secret. There is an intermediate state with both secrets active. A more expressive interface could let you do both in one step. Relevant only if the implementation actually provides atomicity. Can it? >> 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. >> >> * @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. >> >> 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. >> >> Note that LUKSKeyslotAmend can specify only one desired state for >> commonly just one keyslot. Rationale: this satisfies practical needs. >> An array of LUKSKeyslotAmend could specify desired state for all >> keyslots. However, multiple array elements could then apply to the same >> slot. We'd have to specify how to resolve such conflicts, and we'd have >> to code up conflict detection. Not worth it. >> >> Examples: >> >> * Add a secret to some free keyslot: >> >> { "state": "active", "secret": "CIA/GRU/MI6" } >> >> * Deactivate all keyslots holding a secret: >> >> { "state": "inactive", "old-secret": "CIA/GRU/MI6" } >> >> * Add a secret to a specific keyslot: >> >> { "state": "active", "secret": "CIA/GRU/MI6", "keyslot": 0 } >> >> * Deactivate a specific keyslot: >> >> { "state": "inactive", "keyslot": 0 } >> >> Possibly less dangerous: >> >> { "state": "inactive", "keyslot": 0, "old-secret": "CIA/GRU/MI6" } >> >> Option: Make use of Max's patches to support optional union tag with >> default value to let us default @state to "active". I doubt this makes >> much of a difference in QMP. A human-friendly interface should probably >> be higher level anyway (Daniel pointed to cryptsetup). >> >> Option: LUKSKeyslotInactive member @old-secret could also be named >> @secret. I don't care. >> >> Option: delete @keyslot. It provides low-level slot access. >> Complicates the interface. Fine if we need lov-level slot access. Do >> we? >> >> I apologize for the time it has taken me to write this. >> >> Comments? > > Works for me (without taking any of the options). > > The unclear part is what the human-friendly interface should look like > and where it should live. I'm afraid doing only the QMP part and calling > the feature completed like we do so often won't work in this case. No argument. Perhaps Daniel can help with designing a human-friendly high-level interface.