Re: [PATCH v2 02/14] qcrypto/luks: implement encryption key management

[Maxim Levitsky]

On Tue, 2020-03-10 at 11:58 +0100, Max Reitz wrote:
> On 08.03.20 16:18, Maxim Levitsky wrote:
> > Next few patches will expose that functionality
> > to the user.
> > 
> > Signed-off-by: Maxim Levitsky <address@hidden>
> > ---
> >  crypto/block-luks.c | 398 +++++++++++++++++++++++++++++++++++++++++++-
> >  qapi/crypto.json    |  61 ++++++-
> >  2 files changed, 455 insertions(+), 4 deletions(-)
> 
> [...]
> 
> > +##
> > +# @QCryptoBlockAmendOptionsLUKS:
> > +#
> > +# This struct defines the update parameters that activate/de-activate set
> > +# of keyslots
> > +#
> > +# @state: the desired state of the keyslots
> > +#
> > +# @new-secret:    The ID of a QCryptoSecret object providing the password 
> > to be
> > +#                 written into added active keyslots
> > +#
> > +# @old-secret:    Optional (for deactivation only)
> > +#                 If given will deactive all keyslots that
> > +#                 match password located in QCryptoSecret with this ID
> > +#
> > +# @iter-time:     Optional (for activation only)
> > +#                 Number of milliseconds to spend in
> > +#                 PBKDF passphrase processing for the newly activated 
> > keyslot.
> > +#                 Currently defaults to 2000.
> > +#
> > +# @keyslot:       Optional. ID of the keyslot to activate/deactivate.
> > +#                 For keyslot activation, keyslot should not be active 
> > already
> > +#                 (this is unsafe to update an active keyslot),
> > +#                 but possible if 'force' parameter is given.
> > +#                 If keyslot is not given, first free keyslot will be 
> > written.
> > +#
> > +#                 For keyslot deactivation, this parameter specifies the 
> > exact
> > +#                 keyslot to deactivate
> > +#
> > +# @unlock-secret: Optional. The ID of a QCryptoSecret object providing the
> > +#                 password to use to retrive current master key.
> > +#                 Defaults to the same secret that was used to open the 
> > image
> 
> So this matches Markus’ proposal except everything is flattened (because
> we don’t support nested unions, AFAIU).  Sounds OK to me.  The only
> difference is @unlock-secret, which did not appear in his proposal.  Why
> do we need it again?

That a little undocumented hack that will disappear one day.
Its because the driver currently doesn't keep a copy of the master key,
and instead only keeps ciper objects, often from outside libraries,
and in theory these objects might even be implemented in hardware so that
master key might be not in memory at all, so I kind of don't want yet
to keep it in memory.
Thus when doing the key management, I need to retrieve the master key again,
similar to how it is done on image opening. I use the same secret as was used 
for opening,
but in case the keys were changed already, that secret might not work anymore.
Thus I added this parameter to specify basically the old password, which is 
reasonable
when updating passwords.
I usually omit this hack in the discussions as it is orthogonal to the rest of 
the API.

Best regards,
        Maxim Levitsky


> 
> Max
>