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

[Maxim Levitsky <mlevitsk@redhat.com>]

On Thu, 2020-02-06 at 16:19 +0100, Markus Armbruster wrote:
> Kevin Wolf <kwolf@redhat.com> writes:
> 
> > Am 06.02.2020 um 14:36 hat Daniel P. Berrangé geschrieben:
> > > On Thu, Feb 06, 2020 at 02:20:11PM +0100, Markus Armbruster wrote:
> > > > One more question regarding the array in
> > > > 
> > > >     { 'struct': 'QCryptoBlockAmendOptionsLUKS',
> > > >       'data' : {
> > > >                 'keys': ['LUKSKeyslotUpdate'],
> > > >                  '*unlock-secret' : 'str' } }
> > > > 
> > > > Why an array?  Do we really need multiple keyslot updates in one amend
> > > > operation?
> > > 
> > > I think it it is unlikely we'd use this in libvirt. In the case of wanting
> > > to *change* a key, it is safer to do a sequence of "add key" and then
> > > "remove key". If you combine them into the same operation, and you get
> > > an error back, it is hard to know /where/ it failed ? was the new key
> > > added or not ?
> > 
> > I think the array came in because of the "describe the new state"
> > approach. The state has eight keyslots, so in order to fully describe
> > the new state, you would have to be able to pass multiple slots at once.
> 
> I see.
> 
> Of course, it can also describe multiple new states for the same slot.
> 
> Example:
> 
>     [{'state': 'active', 'keyslot': 0, 'secret': 'sec0'},
>      {'state': 'active', 'keyslot': 0, 'secret': 'sec1'}]
> 
>     where slot 0's old state is 'inactive'.
> 
> Which one is the new state?
> 
> If we execute the array elements one by one, this first makes slot 0
> active with secret 'sec0', then tries to make it active with secret
> 'sec1', which fails.  Simple enough, but it's not really "describe the
> new state", it's still "specify a series of state transitions".
> 
> If we merge the array elements into a description of the new state of
> all eight slots, where a slot's description can be "same as old state",
> then this makes slot 0 active with either secret 'sec0' or 'sec1',
> depending on how we resolve the conflict.  We could even make conflicts
> an error, and then this would fail without changing anything.
> 
> What do we want?
> 
> Is this worth the trouble?

Yes, that is my thoughts on this as well.

Best regards,
	Maxim Levitsky