On Mon, 2019-11-11 at 18:34 +0000, Daniel P. Berrangé wrote: > On Mon, Nov 11, 2019 at 05:58:20PM +0200, Maxim Levitsky wrote: > > Hi! > > > > I would like to discuss the API for LUKS key management. > > > > First of all very brief overview of LUKS v1 format: > > > > Each sector of the image is encrypted with same master key, which > > is not stored directly on the disk. > > > > Instead in the LUKS header we have 8 slots. Each slot optionally stores > > an encrypted version of the master key, encrypted by the user password. > > Knowing the password, you can retrieve the master key from the keyslot. > > Slot can be marked as active or inactive, inactive slots are not considered > > when opening the image. > > > > In addition to that LUKS header has a hash of the master key, so that > > you can check if the password 'opens' a keyslot by decrypting it > > with given the password, and then checking if > > the hash of the decrypted master key candidate obtained matches the stored > > hash. > > > > That basically means that you can have up to 8 different passwords that will > > open a luks volume and you can change them as you wish without re-encrypting > > everything. > > > > Now for raw luks volumes you have cryptsetup which allows to manage these > > keyslots, but we also have so called encrypted qcow2 format which > > basically has the luks header, together with keyslots embedded, plus each > > cluster is encrypted with the master key as in raw luks. > > Cryptsetup doesn't support this, thus I implemented this in qemu block > > layer. > > Even for raw luks volumes, the traditional "cryptsetup" tool is > undesirable. eg consider LUKS on an RBD or ISCSI volume where > you are using the in-QEMU RBD/ISCSI client. You don't want to > have to configure the host kernel client just to change the > keyslot info. You don't want to use the in-QEMU clients for > qemu-img.
I didn't thought about it. This is a very good point! > > > > > Link to bugzilla here: https://bugzilla.redhat.com/show_bug.cgi?id=1662412 > > > > > > Relevant to the API, > > first of all qemu has the notion of amend (qemu-img amend), which allows > > currently to change format specific extensions of qcow2. > > > > Since luks, especially luks inside qcow2 is a format on its own, it fits to > > use that interface to change the 'format' options, in this case, > > the encryption key slots. > > > > > > There are the following requirements (they are 100% hardcoded, we might > > discuss > > to drop some of these): > > > > > > 1. ability to add a new password to a free keyslot > > (best is to let api to pick a free keyslot) > > Also user should not need to know all the passwords in existing keyslots. > > > > > > 2. ability to erase a keyslot, usually by giving the password that should > > be erased, and erasing all > > the keyslots that match the password, or by giving a keyslot index. > > This will usually be done after adding a new password. > > > > > > 3. Allow to do so online, that is while qemu is running, but also support > > offline management. > > Note that online management is even useful for raw luks volumes, since its > > not safe > > to run cryptsetup on them while qemu is using the images. > > > > > > I implemented those requirements using the following interface. > > (I have sent the patches already) > > > > I will try to explain the interface with bunch of examples: > > > > > > # adds a new password, defined by qemu secret 'sec0' to first unused slot > > # give user a error if all keyslots are occupied > > qemu-img amend --secret ... -o key-secret=sec1 image.luks > > I think you meant "--object secret,...." instead of "--secret ..." > True, sorry about that! > Also, this example needs to have 2 secrets provided. The first > secret to unlock the image using the existing password, and the > second secret is the one being added. > > > # erases all keyslots that can be opened by password that is contained in a > > qemu secret 'sec0' > > # active=off means that the given password/keyslot won't be active after > > the operation > > qemu-img amend --secret ... -o key-secret=sec0,active=off image.luks > > > > > > # erase the slot 5 (this is more low level command, less expected to be > > used) > > qemu-img amend --secret ... -o slot=5,active=off image.luks > > > > # add new secret to slot 5 (will fail if the slot is already marked as > > active) > > qemu-img amend --secret ... -o slot=5,key-secret=sec1 image.luks > > This also needs two secrets provideed. > > > > > > > This is basically it. > > > > The full option syntax is as following: > > > > active=on/off (optional, default to on) - toggles if we enabling a keyslot > > or are erasing it. > > > > slot=number (optional, advanced option) - specifies which exactly slot to > > erase or which > > slot to put the new key on > > > > key-secret = id of the secret object - specifies the secret. when slot is > > enabled, > > it will be put into the new slot. when disabling (erasing a keyslot), all > > keyslots > > matching that secret will be erased. > > Specifying both key-secret and slot index is treated as error I think > > > > > > As as very advanced option, --force is added to qemu-img to allow to do > > unsafe operation, > > which in this case is removing last keyslot which will render the encrypted > > image useless. > > > > > > In addition to that, QMP interface was added for online version of the > > above. > > It is very similiar, but since we don't have blockdev-amend, > > I added one and it has the following interface: > > > > > > > > ## > > # @x-blockdev-amend: > > # > > # Starts a job to amend format specific options of an existing open block > > device. > > # The job is automatically finalized, but a manual job-dismiss is required. > > # > > # @job-id: Identifier for the newly created job. > > # > > # @node-name: Name of the block node to work on > > # > > # @options: Options (same as for image creation) > > # > > # @force: Allow unsafe operations, format specific > > # For luks that allows erase of the last active keyslot > > # (permanent loss of data), > > # and replacement of an active keyslot > > # (possible loss of data if IO error happens) > > # > > # Since: 4.2 > > ## > > { 'command': 'x-blockdev-amend', > > 'data': { 'job-id': 'str', > > 'node-name': 'str', > > 'options': 'BlockdevCreateOptions', > > '*force': 'bool' } } > > > > > > > > It takes the same BlockdevCreateOptions as blockdev-create (this is open to > > debate if to leave this as is) > > > > > > BlockdevCreateOptionsLUKS (its parent QCryptoBlockCreateOptionsLUKS > > technically is extended in this way): > > > > > > --- a/qapi/crypto.json > > +++ b/qapi/crypto.json > > @@ -190,6 +190,21 @@ > > # Currently defaults to 'sha256' > > # @hash-alg: the master key hash algorithm > > # Currently defaults to 'sha256' > > +# > > +# @active: Should the new secret be added (true) or erased (false) > > +# (amend only, since 4.2) > > +# > > +# @slot: The slot in which to put/erase the secret > > +# if not given, will select first free slot for secret addtion > > +# and erase all keyslots that match the given @key-secret for erase. > > +# except the last one > > +# (optional, since 4.2) > > +# > > +# @unlock-secret: The secret to use to unlock the image > > +# If not given, will use the secret that was used > > +# when opening the image. > > +# (optional, for amend only, since 4.2) > > +# > > # @iter-time: number of milliseconds to spend in > > # PBKDF passphrase processing. Currently defaults > > # to 2000. (since 2.8) > > @@ -201,7 +216,12 @@ > > '*cipher-mode': 'QCryptoCipherMode', > > '*ivgen-alg': 'QCryptoIVGenAlgorithm', > > '*ivgen-hash-alg': 'QCryptoHashAlgorithm', > > + > > '*hash-alg': 'QCryptoHashAlgorithm', > > + '*active' : 'bool', > > + '*slot': 'int', > > + '*unlock-secret': 'str', > > + > > '*iter-time': 'int'}} > > > > > > Here note that key-secret is already present in the in api, and I am adding > > the 'slot','active' and 'unlock-secret' > > > > 'slot' can be also used for new created image to specify where to place the > > the secret. > > 'active' not allowed to be false for blockdev-create of an image and can be > > true/false for 'blockdev-amend' > > > > 'unlock-secret' (might be removed later) covers an corner case that is > > specific for online key management. > > The case is that if the keyslot used to open the image in first place is > > removed, it can be used to specify > > the password to retrieve the master key from one of existing keyslots, > > since the driver doesn't officially > > keep the master key all the time (it can be in theory only loaded in > > hardware crypto device) > > > > That is why for adding a new keyslot, the secret that was used to open the > > image is tried first, and if it > > doesn't open a keyslot, the 'unlock-secret' can be used instead. This can > > be thought of as the 'current password' > > that is need to update the password on many web forums. > > > > > > One of the concerns that was raised during the review was that amend > > interface for luks that I propose is > > different from the amend inteface used currently for qcow2. > > > > qcow2 amend interface specifies all the format options, thus overwrites the > > existing options. > > Thus it seems natural to make the luks amend interface work the same way, > > that it receive an array > > of 8 slots, and for each slot specify if it is active, and if true what > > password to put in it. > > This does allow to add and erase the keyslots, but it doesn't allow: > > > > * add a password without knowing all other passwords that exist in > > existing keyslots > > this can be mitigated by specifying which keyslots to modify for > > example by omitting the > > keyslots that shouldn't be touched from the array (passing null > > placeholder instead) > > but then it already doesn't follow the 'specify all the options each > > time' principle. > > I think this is highly undesirable, as we must not assume that the > mgmt app has access to all the passwords currently set. > > The two key use cases for having multiple key slots are > > - To enable a two-phase change of passwords to ensure new password > is safely written out before erasing the old password > > - To allow for multiple access passwords with different controls > or access to when each password is made available. > > eg each VM may have a separate "backup password" securely > stored off host that is only made available for use when > doing disaster recovery. > > the second use case is doomed if you need to always provide all > current passwords when changing any key slots. Fully agree, and thanks for these examples! > > > > * erase all keyslots matching a password - this is really hard to do > > using this approach, > > unless we give user some kind of api to try each keyslot with given > > password, > > which is kind of ugly and might be racy as well. > > So what do you think? > > The point of using "amend" is that we already have some of the boilerplate > supporting framework around that, so it saves effort for both QEMU and > our users. If the semantics of "amend" don't fit nicely though, then the > benefit of re-using "amend" is cancelled out and we should go back to > considering a separate "key-manage" command. I guess we should anyway use amend interface, while updating its definition a bit to suit the broader requirements of the drivers, e.g luks. I see the amend interface being like generic 'edit the device options' thing, which is maybe better that adding device specific commands like add-key,remove-key, etc. No strong opinion on this though. Best regards, Maxim Levitsky
