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. 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 Deleting a key: qemu-img amend -o encrypt.keys.0.state=inactive,encrypt.keys.0.keyslot=2 test.qcow2 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. Deleting stays more or less the same, you just change the state instead of clearing the secret. Kevin > Markus Armbruster <arm...@redhat.com> writes: > > [...] > > A keyslot can be either inactive or active. > > > > Let's start low-level, i.e. we specify the slot by slot#: > > > > state new state action > > inactive inactive nop > > inactive active put secret, iter-time, mark active > > active inactive mark inactive (effectively deletes secret) > > active active in general, error (unsafe update in place) > > we can make it a nop when secret, iter-time > > remain unchanged > > we can allow unsafe update with force: true > > > > As struct: > > > > { 'struct': 'LUKSKeyslotUpdate', > > 'data': { 'active': 'bool', # could do enum instead > > 'keyslot': 'int', > > '*secret': 'str', # present if @active is true > > '*iter-time': 'int' } } # absent if @active is false > > > > As union: > > > > { 'enum': 'LUKSKeyslotState', > > 'data': [ 'active', 'inactive' ] } > > { 'struct': 'LUKSKeyslotActive', > > 'data': { 'secret': 'str', > > '*iter-time': 'int } } > > { 'union': 'LUKSKeyslotAmend', > > 'base': { 'state': 'LUKSKeyslotState' } # must do enum > > 'discriminator': 'state', > > 'data': { 'active': 'LUKSKeyslotActive' } } > > > > 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). > > > > New state active: > > > > state new state action > > inactive active put secret, iter-time, mark active > > active active N/A (system choses inactive slot) > > > > New state inactive, for each slot holding the specified secret: > > > > state new state action > > inactive inactive N/A (inactive slot holds no secret) > > active inactive mark inactive (effectively deletes secret) > > > > As struct: > > > > { 'struct': 'LUKSKeyslotUpdate', > > 'data': { 'active': 'bool', # could do enum instead > > '*keyslot': 'int', > > '*secret': 'str', # present if @active is true > > '*iter-time': 'int' } } # absent if @active is false > > > > As union: > > > > { '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' } } > > > > Union looks more complicated because our union notation sucks[*]. I > > like it anyway, because you don't have to explain when which optional > > members aren't actually optional. > > > > Regardless of struct vs. union, this supports an active -> active > > transition only with an explicit keyslot. Feels fine to me. If we want > > to support it without keyslot as well, we need a way to specify both old > > and new secret. Do we? > > > > > > [*] I hope to fix that one day. It's not even hard.