| Message ID | 20200308151903.25941-3-mlevitsk@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | LUKS: encryption slot management using amend interface | expand |
On 08.03.20 16:18, Maxim Levitsky wrote: > Next few patches will expose that functionality > to the user. > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > --- > 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? Max
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 <mlevitsk@redhat.com> > > --- > > 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 >
Am 10.03.2020 um 12:05 hat Maxim Levitsky geschrieben: > 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 <mlevitsk@redhat.com> > > > --- > > > 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. It is very much documented (just a few lines above this one), and even if it weren't documented, that wouldn't make it an unstable ABI. If you don't want to make it to become stable ABI, you either need to drop it or it needs an x- prefix, and its documentation should specify what prevents it from being a stable ABI. > 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. How will this requirement disappear one day? Kevin
On Tue, 2020-03-10 at 12:59 +0100, Kevin Wolf wrote: > Am 10.03.2020 um 12:05 hat Maxim Levitsky geschrieben: > > 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 <mlevitsk@redhat.com> > > > > --- > > > > 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. > > It is very much documented (just a few lines above this one), and even > if it weren't documented, that wouldn't make it an unstable ABI. > > If you don't want to make it to become stable ABI, you either need to > drop it or it needs an x- prefix, and its documentation should specify > what prevents it from being a stable ABI. > > > 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. > > How will this requirement disappear one day? If I cave in and keep a copy of the master key in the memory :-) Best regards, Maxim Levitsky > > Kevin
On Tue, 2020-03-10 at 14:02 +0200, Maxim Levitsky wrote: > On Tue, 2020-03-10 at 12:59 +0100, Kevin Wolf wrote: > > Am 10.03.2020 um 12:05 hat Maxim Levitsky geschrieben: > > > 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 <mlevitsk@redhat.com> > > > > > --- > > > > > 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. > > > > It is very much documented (just a few lines above this one), and even > > if it weren't documented, that wouldn't make it an unstable ABI. > > > > If you don't want to make it to become stable ABI, you either need to > > drop it or it needs an x- prefix, and its documentation should specify > > what prevents it from being a stable ABI. > > > > > 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. > > > > How will this requirement disappear one day? > > If I cave in and keep a copy of the master key in the memory :-) > > Best regards, > Maxim Levitsky > > > > > Kevin > > OK folks, besides this hack (which I can remove if you insist, although I don't think it matters), what else should I do to move forward to get this accepted? Best regards, Maxim Levitsky
On Sun, Mar 08, 2020 at 05:18:51PM +0200, Maxim Levitsky wrote: > Next few patches will expose that functionality > to the user. > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > --- > crypto/block-luks.c | 398 +++++++++++++++++++++++++++++++++++++++++++- > qapi/crypto.json | 61 ++++++- > 2 files changed, 455 insertions(+), 4 deletions(-) > > diff --git a/crypto/block-luks.c b/crypto/block-luks.c > index 4861db810c..b11ee08c6d 100644 > --- a/crypto/block-luks.c > +++ b/crypto/block-luks.c > +/* > + * Erases an keyslot given its index > + * Returns: > + * 0 if the keyslot was erased successfully > + * -1 if a error occurred while erasing the keyslot > + * > + */ > +static int > +qcrypto_block_luks_erase_key(QCryptoBlock *block, > + unsigned int slot_idx, > + QCryptoBlockWriteFunc writefunc, > + void *opaque, > + Error **errp) > +{ > + QCryptoBlockLUKS *luks = block->opaque; > + QCryptoBlockLUKSKeySlot *slot = &luks->header.key_slots[slot_idx]; > + g_autofree uint8_t *garbagesplitkey = NULL; > + size_t splitkeylen = luks->header.master_key_len * slot->stripes; > + size_t i; > + > + assert(slot_idx < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS); > + assert(splitkeylen > 0); > + garbagesplitkey = g_new0(uint8_t, splitkeylen); > + > + /* Reset the key slot header */ > + memset(slot->salt, 0, QCRYPTO_BLOCK_LUKS_SALT_LEN); > + slot->iterations = 0; > + slot->active = QCRYPTO_BLOCK_LUKS_KEY_SLOT_DISABLED; > + > + qcrypto_block_luks_store_header(block, writefunc, opaque, errp); This may set errp and we don't return immediately, so.... > + /* > + * Now try to erase the key material, even if the header > + * update failed > + */ > + for (i = 0; i < QCRYPTO_BLOCK_LUKS_ERASE_ITERATIONS; i++) { > + if (qcrypto_random_bytes(garbagesplitkey, splitkeylen, errp) < 0) { ...this may then set errp a second time, which is not permitted. This call needs to use a "local_err", and error_propagate(errp, local_err). The latter is a no-op if errp is already set. > + /* > + * If we failed to get the random data, still write > + * at least zeros to the key slot at least once > + */ > + if (i > 0) { > + return -1; > + } > + } > + if (writefunc(block, > + slot->key_offset_sector * QCRYPTO_BLOCK_LUKS_SECTOR_SIZE, > + garbagesplitkey, > + splitkeylen, > + opaque, > + errp) != splitkeylen) { same issue with errp here too. > + return -1; > + } > + } > + return 0; > +} > +/* > + * Given LUKSKeyslotUpdate command, set @slots_bitmap with all slots > + * that will be updated with new password (or erased) > + * returns 0 on success, and -1 on failure > + */ > +static int > +qcrypto_block_luks_get_update_bitmap(QCryptoBlock *block, > + QCryptoBlockReadFunc readfunc, > + void *opaque, > + const QCryptoBlockAmendOptionsLUKS *opts, > + unsigned long *slots_bitmap, > + Error **errp) > +{ > + const QCryptoBlockLUKS *luks = block->opaque; > + size_t i; > + > + bitmap_zero(slots_bitmap, QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS); > + > + if (opts->has_keyslot) { > + /* keyslot set, select only this keyslot */ > + int keyslot = opts->keyslot; > + > + if (keyslot < 0 || keyslot >= QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS) { > + error_setg(errp, > + "Invalid slot %u specified, must be between 0 and %u", > + keyslot, QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS - 1); > + return -1; > + } > + bitmap_set(slots_bitmap, keyslot, 1); > + > + } else if (opts->has_old_secret) { > + /* initially select all active keyslots */ > + for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) { > + if (qcrypto_block_luks_slot_active(luks, i)) { > + bitmap_set(slots_bitmap, i, 1); > + } > + } > + } else { > + /* find a free keyslot */ > + int slot = qcrypto_block_luks_find_free_keyslot(luks); > + > + if (slot == -1) { > + error_setg(errp, > + "Can't add a keyslot - all key slots are in use"); > + return -1; > + } > + bitmap_set(slots_bitmap, slot, 1); > + } > + > + if (opts->has_old_secret) { > + /* now deselect all keyslots that don't contain the password */ > + g_autofree uint8_t *tmpkey = g_new0(uint8_t, > + luks->header.master_key_len); > + > + for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) { > + g_autofree char *old_password = NULL; > + int rv; > + > + if (!test_bit(i, slots_bitmap)) { > + continue; > + } > + > + old_password = qcrypto_secret_lookup_as_utf8(opts->old_secret, > + errp); > + if (!old_password) { > + return -1; > + } > + > + rv = qcrypto_block_luks_load_key(block, > + i, > + old_password, > + tmpkey, > + readfunc, > + opaque, > + errp); > + if (rv == -1) { > + return -1; > + } else if (rv == 0) { > + bitmap_clear(slots_bitmap, i, 1); > + } > + } > + } > + return 0; > +} I'm not really liking this function as a concept. Some of the code only applies to the "add key" code path, while some of it only applies to the "erase key" code path. I'd prefer it if qcrypto_block_luks_erase_keys directly had the required logic, likewise qcrypto_block_luks_set_keys, and thus get rid of the bitmap concept entirely. I thin kit'd make the logic easier to understand. > + > +/* > + * Erase a set of keyslots given in @slots_bitmap > + */ > +static int qcrypto_block_luks_erase_keys(QCryptoBlock *block, > + QCryptoBlockReadFunc readfunc, > + QCryptoBlockWriteFunc writefunc, > + void *opaque, > + unsigned long *slots_bitmap, > + bool force, > + Error **errp) > +{ > + QCryptoBlockLUKS *luks = block->opaque; > + long slot_count = bitmap_count_one(slots_bitmap, > + QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS); > + size_t i; > + > + /* safety checks */ > + if (!force && slot_count == qcrypto_block_luks_count_active_slots(luks)) { > + error_setg(errp, > + "Requested operation will erase all active keyslots" > + " which will erase all the data in the image" > + " irreversibly - refusing operation"); > + return -EINVAL; > + } > + > + /* new apply the update */ > + for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) { > + if (!test_bit(i, slots_bitmap)) { > + continue; > + } > + if (qcrypto_block_luks_erase_key(block, i, writefunc, opaque, errp)) { > + error_append_hint(errp, "Failed to erase keyslot %zu", i); > + return -EINVAL; > + } > + } > + return 0; > +} > + > +/* > + * Set a set of keyslots to @master_key encrypted by @new_secret > + */ > +static int qcrypto_block_luks_set_keys(QCryptoBlock *block, > + QCryptoBlockReadFunc readfunc, > + QCryptoBlockWriteFunc writefunc, > + void *opaque, > + unsigned long *slots_bitmap, > + uint8_t *master_key, > + uint64_t iter_time, > + char *new_secret, > + bool force, > + Error **errp) I'd call this "add_key" instead of "set_keys". I'm also unclear why we need to support setting a range of keyslots. AFAIK, adding a key should only ever affect a single keyslot. > +{ > + QCryptoBlockLUKS *luks = block->opaque; > + g_autofree char *new_password = NULL; > + size_t i; > + > + /* safety checks */ > + if (!force) { > + for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) { > + if (!test_bit(i, slots_bitmap)) { > + continue; > + } > + if (qcrypto_block_luks_slot_active(luks, i)) { > + error_setg(errp, > + "Refusing to overwrite active slot %zu - " > + "please erase it first", i); > + return -EINVAL; > + } > + } > + } > + > + /* Load the new password */ > + new_password = qcrypto_secret_lookup_as_utf8(new_secret, errp); > + if (!new_password) { > + return -EINVAL; > + } > + > + /* Apply the update */ > + for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) { > + if (!test_bit(i, slots_bitmap)) { > + continue; > + } > + if (qcrypto_block_luks_store_key(block, i, new_password, master_key, > + iter_time, writefunc, opaque, errp)) { > + error_append_hint(errp, "Failed to write to keyslot %zu", i); > + return -EINVAL; > + } > + } > + return 0; > +} > + > +static int > +qcrypto_block_luks_amend_options(QCryptoBlock *block, > + QCryptoBlockReadFunc readfunc, > + QCryptoBlockWriteFunc writefunc, > + void *opaque, > + QCryptoBlockAmendOptions *options, > + bool force, > + Error **errp) > +{ > + QCryptoBlockLUKS *luks = block->opaque; > + QCryptoBlockAmendOptionsLUKS *opts_luks = &options->u.luks; > + g_autofree uint8_t *master_key = NULL; > + g_autofree unsigned long *update_bitmap = NULL; > + char *unlock_secret = NULL; > + long slot_count; > + > + unlock_secret = opts_luks->has_unlock_secret ? opts_luks->unlock_secret : > + luks->secret; > + > + /* Retrieve set of slots that we need to update */ > + update_bitmap = bitmap_new(QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS); > + if (qcrypto_block_luks_get_update_bitmap(block, readfunc, opaque, opts_luks, > + update_bitmap, errp) != 0) { > + return -1; > + } > + > + slot_count = bitmap_count_one(update_bitmap, > + QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS); > + > + /* no matching slots, so nothing to do */ > + if (slot_count == 0) { > + error_setg(errp, "Requested operation didn't match any slots"); > + return -1; > + } > + > + if (opts_luks->state == LUKS_KEYSLOT_STATE_ACTIVE) { > + > + uint64_t iter_time = opts_luks->has_iter_time ? > + opts_luks->iter_time : > + QCRYPTO_BLOCK_LUKS_DEFAULT_ITER_TIME_MS; > + > + if (!opts_luks->has_new_secret) { > + error_setg(errp, "'new-secret' is required to activate a keyslot"); > + return -EINVAL; return -1, we shouldn't return errno values in luks code in general as we use Error **errp. > + } > + if (opts_luks->has_old_secret) { > + error_setg(errp, > + "'old-secret' must not be given when activating keyslots"); > + return -EINVAL; > + } > + > + /* Locate the password that will be used to retrieve the master key */ > + g_autofree char *old_password; > + old_password = qcrypto_secret_lookup_as_utf8(unlock_secret, errp); > + if (!old_password) { > + return -EINVAL; > + } > + > + /* Try to retrieve the master key */ > + master_key = g_new0(uint8_t, luks->header.master_key_len); > + if (qcrypto_block_luks_find_key(block, old_password, master_key, > + readfunc, opaque, errp) < 0) { > + error_append_hint(errp, "Failed to retrieve the master key"); > + return -EINVAL; > + } > + > + /* Now set the new keyslots */ > + if (qcrypto_block_luks_set_keys(block, readfunc, writefunc, > + opaque, update_bitmap, master_key, > + iter_time, > + opts_luks->new_secret, > + force, errp) != 0) { > + return -1; > + } > + } else { > + if (opts_luks->has_new_secret) { > + error_setg(errp, > + "'new-secret' must not be given when erasing keyslots"); > + return -EINVAL; > + } > + if (opts_luks->has_iter_time) { > + error_setg(errp, > + "'iter-time' must not be given when erasing keyslots"); > + return -EINVAL; > + } > + if (opts_luks->has_unlock_secret) { > + error_setg(errp, > + "'unlock_secret' must not be given when erasing keyslots"); > + return -EINVAL; > + } > + > + if (qcrypto_block_luks_erase_keys(block, readfunc, writefunc, > + opaque, update_bitmap, force, > + errp) != 0) { > + return -1; > + } > + } > + return 0; > +} > > static int qcrypto_block_luks_get_info(QCryptoBlock *block, > QCryptoBlockInfo *info, > @@ -1523,7 +1912,11 @@ static int qcrypto_block_luks_get_info(QCryptoBlock *block, > > static void qcrypto_block_luks_cleanup(QCryptoBlock *block) > { > - g_free(block->opaque); > + QCryptoBlockLUKS *luks = block->opaque; > + if (luks) { > + g_free(luks->secret); > + g_free(luks); > + } > } > > > @@ -1560,6 +1953,7 @@ qcrypto_block_luks_encrypt(QCryptoBlock *block, > const QCryptoBlockDriver qcrypto_block_driver_luks = { > .open = qcrypto_block_luks_open, > .create = qcrypto_block_luks_create, > + .amend = qcrypto_block_luks_amend_options, > .get_info = qcrypto_block_luks_get_info, > .cleanup = qcrypto_block_luks_cleanup, > .decrypt = qcrypto_block_luks_decrypt, > diff --git a/qapi/crypto.json b/qapi/crypto.json > index 3fd0ce177e..fe600fc608 100644 > --- a/qapi/crypto.json > +++ b/qapi/crypto.json > @@ -1,6 +1,8 @@ > # -*- Mode: Python -*- > # > > +{ 'include': 'common.json' } > + > ## > # = Cryptography > ## > @@ -297,7 +299,6 @@ > 'uuid': 'str', > 'slots': [ 'QCryptoBlockInfoLUKSSlot' ] }} > > - > ## > # @QCryptoBlockInfo: > # > @@ -310,7 +311,63 @@ > 'discriminator': 'format', > 'data': { 'luks': 'QCryptoBlockInfoLUKS' } } > > +## > +# @LUKSKeyslotState: > +# > +# Defines state of keyslots that are affected by the update > +# > +# @active: The slots contain the given password and marked as active > +# @inactive: The slots are erased (contain garbage) and marked as inactive > +# > +# Since: 5.0 > +## > +{ 'enum': 'LUKSKeyslotState', > + 'data': [ 'active', 'inactive' ] } This should be called QCryptoBLockLUKSKeyslotState > +## > +# @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 My inclination would be to just call this "@secret", as it serves the same purpose as the "@secret" parameter used when opening the image. > +{ 'struct': 'QCryptoBlockAmendOptionsLUKS', > + 'data': { 'state': 'LUKSKeyslotState', > + '*new-secret': 'str', > + '*old-secret': 'str', > + '*keyslot': 'int', > + '*iter-time': 'int', > + '*unlock-secret': 'str' } } > > ## > # @QCryptoBlockAmendOptions: > @@ -324,4 +381,4 @@ > 'base': 'QCryptoBlockOptionsBase', > 'discriminator': 'format', > 'data': { > - } } > + 'luks': 'QCryptoBlockAmendOptionsLUKS' } } Regards, Daniel
On Tue, 2020-04-28 at 14:16 +0100, Daniel P. Berrangé wrote: > On Sun, Mar 08, 2020 at 05:18:51PM +0200, Maxim Levitsky wrote: > > Next few patches will expose that functionality > > to the user. > > > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > > --- > > crypto/block-luks.c | 398 +++++++++++++++++++++++++++++++++++++++++++- > > qapi/crypto.json | 61 ++++++- > > 2 files changed, 455 insertions(+), 4 deletions(-) > > > > diff --git a/crypto/block-luks.c b/crypto/block-luks.c > > index 4861db810c..b11ee08c6d 100644 > > --- a/crypto/block-luks.c > > +++ b/crypto/block-luks.c > > +/* > > + * Erases an keyslot given its index > > + * Returns: > > + * 0 if the keyslot was erased successfully > > + * -1 if a error occurred while erasing the keyslot > > + * > > + */ > > +static int > > +qcrypto_block_luks_erase_key(QCryptoBlock *block, > > + unsigned int slot_idx, > > + QCryptoBlockWriteFunc writefunc, > > + void *opaque, > > + Error **errp) > > +{ > > + QCryptoBlockLUKS *luks = block->opaque; > > + QCryptoBlockLUKSKeySlot *slot = &luks->header.key_slots[slot_idx]; > > + g_autofree uint8_t *garbagesplitkey = NULL; > > + size_t splitkeylen = luks->header.master_key_len * slot->stripes; > > + size_t i; > > + > > + assert(slot_idx < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS); > > + assert(splitkeylen > 0); > > + garbagesplitkey = g_new0(uint8_t, splitkeylen); > > + > > + /* Reset the key slot header */ > > + memset(slot->salt, 0, QCRYPTO_BLOCK_LUKS_SALT_LEN); > > + slot->iterations = 0; > > + slot->active = QCRYPTO_BLOCK_LUKS_KEY_SLOT_DISABLED; > > + > > + qcrypto_block_luks_store_header(block, writefunc, opaque, errp); > > This may set errp and we don't return immediately, so.... > > > + /* > > + * Now try to erase the key material, even if the header > > + * update failed > > + */ > > + for (i = 0; i < QCRYPTO_BLOCK_LUKS_ERASE_ITERATIONS; i++) { > > + if (qcrypto_random_bytes(garbagesplitkey, splitkeylen, errp) < 0) { > > ...this may then set errp a second time, which is not permitted. > > This call needs to use a "local_err", and error_propagate(errp, local_err). > The latter is a no-op if errp is already set. Fixed! Thanks for pointing this out! > > > + /* > > + * If we failed to get the random data, still write > > + * at least zeros to the key slot at least once > > + */ > > + if (i > 0) { > > + return -1; > > + } > > + } > > + if (writefunc(block, > > + slot->key_offset_sector * QCRYPTO_BLOCK_LUKS_SECTOR_SIZE, > > + garbagesplitkey, > > + splitkeylen, > > + opaque, > > + errp) != splitkeylen) { > > same issue with errp here too. Fixed too of course > > > + return -1; > > + } > > + } > > + return 0; > > +} > > > > +/* > > + * Given LUKSKeyslotUpdate command, set @slots_bitmap with all slots > > + * that will be updated with new password (or erased) > > + * returns 0 on success, and -1 on failure > > + */ > > +static int > > +qcrypto_block_luks_get_update_bitmap(QCryptoBlock *block, > > + QCryptoBlockReadFunc readfunc, > > + void *opaque, > > + const QCryptoBlockAmendOptionsLUKS *opts, > > + unsigned long *slots_bitmap, > > + Error **errp) > > +{ > > + const QCryptoBlockLUKS *luks = block->opaque; > > + size_t i; > > + > > + bitmap_zero(slots_bitmap, QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS); > > + > > + if (opts->has_keyslot) { > > + /* keyslot set, select only this keyslot */ > > + int keyslot = opts->keyslot; > > + > > + if (keyslot < 0 || keyslot >= QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS) { > > + error_setg(errp, > > + "Invalid slot %u specified, must be between 0 and %u", > > + keyslot, QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS - 1); > > + return -1; > > + } > > + bitmap_set(slots_bitmap, keyslot, 1); > > + > > + } else if (opts->has_old_secret) { > > + /* initially select all active keyslots */ > > + for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) { > > + if (qcrypto_block_luks_slot_active(luks, i)) { > > + bitmap_set(slots_bitmap, i, 1); > > + } > > + } > > + } else { > > + /* find a free keyslot */ > > + int slot = qcrypto_block_luks_find_free_keyslot(luks); > > + > > + if (slot == -1) { > > + error_setg(errp, > > + "Can't add a keyslot - all key slots are in use"); > > + return -1; > > + } > > + bitmap_set(slots_bitmap, slot, 1); > > + } > > + > > + if (opts->has_old_secret) { > > + /* now deselect all keyslots that don't contain the password */ > > + g_autofree uint8_t *tmpkey = g_new0(uint8_t, > > + luks->header.master_key_len); > > + > > + for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) { > > + g_autofree char *old_password = NULL; > > + int rv; > > + > > + if (!test_bit(i, slots_bitmap)) { > > + continue; > > + } > > + > > + old_password = qcrypto_secret_lookup_as_utf8(opts->old_secret, > > + errp); > > + if (!old_password) { > > + return -1; > > + } > > + > > + rv = qcrypto_block_luks_load_key(block, > > + i, > > + old_password, > > + tmpkey, > > + readfunc, > > + opaque, > > + errp); > > + if (rv == -1) { > > + return -1; > > + } else if (rv == 0) { > > + bitmap_clear(slots_bitmap, i, 1); > > + } > > + } > > + } > > + return 0; > > +} > > I'm not really liking this function as a concept. Some of the code > only applies to the "add key" code path, while some of it only > applies to the "erase key" code path. > > I'd prefer it if qcrypto_block_luks_erase_keys directly had the > required logic, likewise qcrypto_block_luks_set_keys, and thus > get rid of the bitmap concept entirely. I thin kit'd make the > logic easier to understand. It used to be like that in former versions that I did send, I added the concept of the bitmap very recently to reflect the way we defined this in the spec. I don't mind that much coming back to older version of doing this, but beware that it won't be that clear either. > > > + > > +/* > > + * Erase a set of keyslots given in @slots_bitmap > > + */ > > +static int qcrypto_block_luks_erase_keys(QCryptoBlock *block, > > + QCryptoBlockReadFunc readfunc, > > + QCryptoBlockWriteFunc writefunc, > > + void *opaque, > > + unsigned long *slots_bitmap, > > + bool force, > > + Error **errp) > > +{ > > + QCryptoBlockLUKS *luks = block->opaque; > > + long slot_count = bitmap_count_one(slots_bitmap, > > + QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS); > > + size_t i; > > + > > + /* safety checks */ > > + if (!force && slot_count == qcrypto_block_luks_count_active_slots(luks)) { > > + error_setg(errp, > > + "Requested operation will erase all active keyslots" > > + " which will erase all the data in the image" > > + " irreversibly - refusing operation"); > > + return -EINVAL; > > + } > > + > > + /* new apply the update */ > > + for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) { > > + if (!test_bit(i, slots_bitmap)) { > > + continue; > > + } > > + if (qcrypto_block_luks_erase_key(block, i, writefunc, opaque, errp)) { > > + error_append_hint(errp, "Failed to erase keyslot %zu", i); > > + return -EINVAL; > > + } > > + } > > + return 0; > > +} > > + > > +/* > > + * Set a set of keyslots to @master_key encrypted by @new_secret > > + */ > > +static int qcrypto_block_luks_set_keys(QCryptoBlock *block, > > + QCryptoBlockReadFunc readfunc, > > + QCryptoBlockWriteFunc writefunc, > > + void *opaque, > > + unsigned long *slots_bitmap, > > + uint8_t *master_key, > > + uint64_t iter_time, > > + char *new_secret, > > + bool force, > > + Error **errp) > > I'd call this "add_key" instead of "set_keys". I'm also unclear why > we need to support setting a range of keyslots. AFAIK, adding a key > should only ever affect a single keyslot. Mostly for consistency. There is a very corner case of inline replacing all keys that match one password with another. If possible I would like to keep it this way though. > > > +{ > > + QCryptoBlockLUKS *luks = block->opaque; > > + g_autofree char *new_password = NULL; > > + size_t i; > > + > > + /* safety checks */ > > + if (!force) { > > + for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) { > > + if (!test_bit(i, slots_bitmap)) { > > + continue; > > + } > > + if (qcrypto_block_luks_slot_active(luks, i)) { > > + error_setg(errp, > > + "Refusing to overwrite active slot %zu - " > > + "please erase it first", i); > > + return -EINVAL; > > + } > > + } > > + } > > + > > + /* Load the new password */ > > + new_password = qcrypto_secret_lookup_as_utf8(new_secret, errp); > > + if (!new_password) { > > + return -EINVAL; > > + } > > + > > + /* Apply the update */ > > + for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) { > > + if (!test_bit(i, slots_bitmap)) { > > + continue; > > + } > > + if (qcrypto_block_luks_store_key(block, i, new_password, master_key, > > + iter_time, writefunc, opaque, errp)) { > > + error_append_hint(errp, "Failed to write to keyslot %zu", i); > > + return -EINVAL; > > + } > > + } > > + return 0; > > +} > > + > > +static int > > +qcrypto_block_luks_amend_options(QCryptoBlock *block, > > + QCryptoBlockReadFunc readfunc, > > + QCryptoBlockWriteFunc writefunc, > > + void *opaque, > > + QCryptoBlockAmendOptions *options, > > + bool force, > > + Error **errp) > > +{ > > + QCryptoBlockLUKS *luks = block->opaque; > > + QCryptoBlockAmendOptionsLUKS *opts_luks = &options->u.luks; > > + g_autofree uint8_t *master_key = NULL; > > + g_autofree unsigned long *update_bitmap = NULL; > > + char *unlock_secret = NULL; > > + long slot_count; > > + > > + unlock_secret = opts_luks->has_unlock_secret ? opts_luks->unlock_secret : > > + luks->secret; > > + > > + /* Retrieve set of slots that we need to update */ > > + update_bitmap = bitmap_new(QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS); > > + if (qcrypto_block_luks_get_update_bitmap(block, readfunc, opaque, opts_luks, > > + update_bitmap, errp) != 0) { > > + return -1; > > + } > > + > > + slot_count = bitmap_count_one(update_bitmap, > > + QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS); > > + > > + /* no matching slots, so nothing to do */ > > + if (slot_count == 0) { > > + error_setg(errp, "Requested operation didn't match any slots"); > > + return -1; > > + } > > + > > + if (opts_luks->state == LUKS_KEYSLOT_STATE_ACTIVE) { > > + > > + uint64_t iter_time = opts_luks->has_iter_time ? > > + opts_luks->iter_time : > > + QCRYPTO_BLOCK_LUKS_DEFAULT_ITER_TIME_MS; > > + > > + if (!opts_luks->has_new_secret) { > > + error_setg(errp, "'new-secret' is required to activate a keyslot"); > > + return -EINVAL; > > return -1, we shouldn't return errno values in luks code in general > as we use Error **errp. Yep, fixed. > > > + } > > + if (opts_luks->has_old_secret) { > > + error_setg(errp, > > + "'old-secret' must not be given when activating keyslots"); > > + return -EINVAL; > > + } > > + > > + /* Locate the password that will be used to retrieve the master key */ > > + g_autofree char *old_password; > > + old_password = qcrypto_secret_lookup_as_utf8(unlock_secret, errp); > > + if (!old_password) { > > + return -EINVAL; > > + } > > + > > + /* Try to retrieve the master key */ > > + master_key = g_new0(uint8_t, luks->header.master_key_len); > > + if (qcrypto_block_luks_find_key(block, old_password, master_key, > > + readfunc, opaque, errp) < 0) { > > + error_append_hint(errp, "Failed to retrieve the master key"); > > + return -EINVAL; > > + } > > + > > + /* Now set the new keyslots */ > > + if (qcrypto_block_luks_set_keys(block, readfunc, writefunc, > > + opaque, update_bitmap, master_key, > > + iter_time, > > + opts_luks->new_secret, > > + force, errp) != 0) { > > + return -1; > > + } > > + } else { > > + if (opts_luks->has_new_secret) { > > + error_setg(errp, > > + "'new-secret' must not be given when erasing keyslots"); > > + return -EINVAL; > > + } > > + if (opts_luks->has_iter_time) { > > + error_setg(errp, > > + "'iter-time' must not be given when erasing keyslots"); > > + return -EINVAL; > > + } > > + if (opts_luks->has_unlock_secret) { > > + error_setg(errp, > > + "'unlock_secret' must not be given when erasing keyslots"); > > + return -EINVAL; > > + } > > + > > + if (qcrypto_block_luks_erase_keys(block, readfunc, writefunc, > > + opaque, update_bitmap, force, > > + errp) != 0) { > > + return -1; > > + } > > + } > > + return 0; > > +} > > > > static int qcrypto_block_luks_get_info(QCryptoBlock *block, > > QCryptoBlockInfo *info, > > @@ -1523,7 +1912,11 @@ static int qcrypto_block_luks_get_info(QCryptoBlock *block, > > > > static void qcrypto_block_luks_cleanup(QCryptoBlock *block) > > { > > - g_free(block->opaque); > > + QCryptoBlockLUKS *luks = block->opaque; > > + if (luks) { > > + g_free(luks->secret); > > + g_free(luks); > > + } > > } > > > > > > @@ -1560,6 +1953,7 @@ qcrypto_block_luks_encrypt(QCryptoBlock *block, > > const QCryptoBlockDriver qcrypto_block_driver_luks = { > > .open = qcrypto_block_luks_open, > > .create = qcrypto_block_luks_create, > > + .amend = qcrypto_block_luks_amend_options, > > .get_info = qcrypto_block_luks_get_info, > > .cleanup = qcrypto_block_luks_cleanup, > > .decrypt = qcrypto_block_luks_decrypt, > > diff --git a/qapi/crypto.json b/qapi/crypto.json > > index 3fd0ce177e..fe600fc608 100644 > > --- a/qapi/crypto.json > > +++ b/qapi/crypto.json > > @@ -1,6 +1,8 @@ > > # -*- Mode: Python -*- > > # > > > > +{ 'include': 'common.json' } > > + > > ## > > # = Cryptography > > ## > > @@ -297,7 +299,6 @@ > > 'uuid': 'str', > > 'slots': [ 'QCryptoBlockInfoLUKSSlot' ] }} > > > > - > > ## > > # @QCryptoBlockInfo: > > # > > @@ -310,7 +311,63 @@ > > 'discriminator': 'format', > > 'data': { 'luks': 'QCryptoBlockInfoLUKS' } } > > > > +## > > +# @LUKSKeyslotState: > > +# > > +# Defines state of keyslots that are affected by the update > > +# > > +# @active: The slots contain the given password and marked as active > > +# @inactive: The slots are erased (contain garbage) and marked as inactive > > +# > > +# Since: 5.0 > > +## > > +{ 'enum': 'LUKSKeyslotState', > > + 'data': [ 'active', 'inactive' ] } > > This should be called QCryptoBLockLUKSKeyslotState Roger that! > > > +## > > +# @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 > > My inclination would be to just call this "@secret", as it serves the > same purpose as the "@secret" parameter used when opening the image. Let it be 'secret' I don't mind at all. > > > +{ 'struct': 'QCryptoBlockAmendOptionsLUKS', > > + 'data': { 'state': 'LUKSKeyslotState', > > + '*new-secret': 'str', > > + '*old-secret': 'str', > > + '*keyslot': 'int', > > + '*iter-time': 'int', > > + '*unlock-secret': 'str' } } > > > > ## > > # @QCryptoBlockAmendOptions: > > @@ -324,4 +381,4 @@ > > 'base': 'QCryptoBlockOptionsBase', > > 'discriminator': 'format', > > 'data': { > > - } } > > + 'luks': 'QCryptoBlockAmendOptionsLUKS' } } > > Regards, > Daniel Best regards and thanks for the review, Maxim Levitsky
On Sun, May 03, 2020 at 11:55:35AM +0300, Maxim Levitsky wrote: > On Tue, 2020-04-28 at 14:16 +0100, Daniel P. Berrangé wrote: > > On Sun, Mar 08, 2020 at 05:18:51PM +0200, Maxim Levitsky wrote: > > > Next few patches will expose that functionality > > > to the user. > > > > > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > > > --- > > > crypto/block-luks.c | 398 +++++++++++++++++++++++++++++++++++++++++++- > > > qapi/crypto.json | 61 ++++++- > > > 2 files changed, 455 insertions(+), 4 deletions(-) > > > +/* > > > + * Given LUKSKeyslotUpdate command, set @slots_bitmap with all slots > > > + * that will be updated with new password (or erased) > > > + * returns 0 on success, and -1 on failure > > > + */ > > > +static int > > > +qcrypto_block_luks_get_update_bitmap(QCryptoBlock *block, > > > + QCryptoBlockReadFunc readfunc, > > > + void *opaque, > > > + const QCryptoBlockAmendOptionsLUKS *opts, > > > + unsigned long *slots_bitmap, > > > + Error **errp) > > > +{ > > > + const QCryptoBlockLUKS *luks = block->opaque; > > > + size_t i; > > > + > > > + bitmap_zero(slots_bitmap, QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS); > > > + > > > + if (opts->has_keyslot) { > > > + /* keyslot set, select only this keyslot */ > > > + int keyslot = opts->keyslot; > > > + > > > + if (keyslot < 0 || keyslot >= QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS) { > > > + error_setg(errp, > > > + "Invalid slot %u specified, must be between 0 and %u", > > > + keyslot, QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS - 1); > > > + return -1; > > > + } > > > + bitmap_set(slots_bitmap, keyslot, 1); > > > + > > > + } else if (opts->has_old_secret) { > > > + /* initially select all active keyslots */ > > > + for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) { > > > + if (qcrypto_block_luks_slot_active(luks, i)) { > > > + bitmap_set(slots_bitmap, i, 1); > > > + } > > > + } > > > + } else { > > > + /* find a free keyslot */ > > > + int slot = qcrypto_block_luks_find_free_keyslot(luks); > > > + > > > + if (slot == -1) { > > > + error_setg(errp, > > > + "Can't add a keyslot - all key slots are in use"); > > > + return -1; > > > + } > > > + bitmap_set(slots_bitmap, slot, 1); > > > + } > > > + > > > + if (opts->has_old_secret) { > > > + /* now deselect all keyslots that don't contain the password */ > > > + g_autofree uint8_t *tmpkey = g_new0(uint8_t, > > > + luks->header.master_key_len); > > > + > > > + for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) { > > > + g_autofree char *old_password = NULL; > > > + int rv; > > > + > > > + if (!test_bit(i, slots_bitmap)) { > > > + continue; > > > + } > > > + > > > + old_password = qcrypto_secret_lookup_as_utf8(opts->old_secret, > > > + errp); > > > + if (!old_password) { > > > + return -1; > > > + } > > > + > > > + rv = qcrypto_block_luks_load_key(block, > > > + i, > > > + old_password, > > > + tmpkey, > > > + readfunc, > > > + opaque, > > > + errp); > > > + if (rv == -1) { > > > + return -1; > > > + } else if (rv == 0) { > > > + bitmap_clear(slots_bitmap, i, 1); > > > + } > > > + } > > > + } > > > + return 0; > > > +} > > > > I'm not really liking this function as a concept. Some of the code > > only applies to the "add key" code path, while some of it only > > applies to the "erase key" code path. > > > > I'd prefer it if qcrypto_block_luks_erase_keys directly had the > > required logic, likewise qcrypto_block_luks_set_keys, and thus > > get rid of the bitmap concept entirely. I thin kit'd make the > > logic easier to understand. > > It used to be like that in former versions that I did send, I added the concept > of the bitmap very recently to reflect the way we defined this in the spec. > I don't mind that much coming back to older version of doing this, > but beware that it won't be that clear either. My view is that removing and adding keys are fundamentally different operations, so although there's some parts that are in common, overall it is better to keep them clearly separate. > > > +/* > > > + * Erase a set of keyslots given in @slots_bitmap > > > + */ > > > +static int qcrypto_block_luks_erase_keys(QCryptoBlock *block, > > > + QCryptoBlockReadFunc readfunc, > > > + QCryptoBlockWriteFunc writefunc, > > > + void *opaque, > > > + unsigned long *slots_bitmap, > > > + bool force, > > > + Error **errp) > > > +{ > > > + QCryptoBlockLUKS *luks = block->opaque; > > > + long slot_count = bitmap_count_one(slots_bitmap, > > > + QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS); > > > + size_t i; > > > + > > > + /* safety checks */ > > > + if (!force && slot_count == qcrypto_block_luks_count_active_slots(luks)) { > > > + error_setg(errp, > > > + "Requested operation will erase all active keyslots" > > > + " which will erase all the data in the image" > > > + " irreversibly - refusing operation"); > > > + return -EINVAL; > > > + } > > > + > > > + /* new apply the update */ > > > + for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) { > > > + if (!test_bit(i, slots_bitmap)) { > > > + continue; > > > + } > > > + if (qcrypto_block_luks_erase_key(block, i, writefunc, opaque, errp)) { > > > + error_append_hint(errp, "Failed to erase keyslot %zu", i); > > > + return -EINVAL; > > > + } > > > + } > > > + return 0; > > > +} > > > + > > > +/* > > > + * Set a set of keyslots to @master_key encrypted by @new_secret > > > + */ > > > +static int qcrypto_block_luks_set_keys(QCryptoBlock *block, > > > + QCryptoBlockReadFunc readfunc, > > > + QCryptoBlockWriteFunc writefunc, > > > + void *opaque, > > > + unsigned long *slots_bitmap, > > > + uint8_t *master_key, > > > + uint64_t iter_time, > > > + char *new_secret, > > > + bool force, > > > + Error **errp) > > > > I'd call this "add_key" instead of "set_keys". I'm also unclear why > > we need to support setting a range of keyslots. AFAIK, adding a key > > should only ever affect a single keyslot. > Mostly for consistency. There is a very corner case of inline replacing > all keys that match one password with another. I don't see that as a use case we care about. There's no benefit to having the same password repeated in multiple slots. > If possible I would like to keep it this way though. IMHO the the bitmap just needlessly complicates the code for a feature that is irrelevant to us. Regards, Daniel
diff --git a/crypto/block-luks.c b/crypto/block-luks.c index 4861db810c..b11ee08c6d 100644 --- a/crypto/block-luks.c +++ b/crypto/block-luks.c @@ -32,6 +32,7 @@ #include "qemu/uuid.h" #include "qemu/coroutine.h" +#include "qemu/bitmap.h" /* * Reference for the LUKS format implemented here is @@ -70,6 +71,9 @@ typedef struct QCryptoBlockLUKSKeySlot QCryptoBlockLUKSKeySlot; #define QCRYPTO_BLOCK_LUKS_SECTOR_SIZE 512LL +#define QCRYPTO_BLOCK_LUKS_DEFAULT_ITER_TIME_MS 2000 +#define QCRYPTO_BLOCK_LUKS_ERASE_ITERATIONS 40 + static const char qcrypto_block_luks_magic[QCRYPTO_BLOCK_LUKS_MAGIC_LEN] = { 'L', 'U', 'K', 'S', 0xBA, 0xBE }; @@ -219,6 +223,9 @@ struct QCryptoBlockLUKS { /* Hash algorithm used in pbkdf2 function */ QCryptoHashAlgorithm hash_alg; + + /* Name of the secret that was used to open the image */ + char *secret; }; @@ -1069,6 +1076,108 @@ qcrypto_block_luks_find_key(QCryptoBlock *block, return -1; } +/* + * Returns true if a slot i is marked as active + * (contains encrypted copy of the master key) + */ +static bool +qcrypto_block_luks_slot_active(const QCryptoBlockLUKS *luks, + unsigned int slot_idx) +{ + uint32_t val = luks->header.key_slots[slot_idx].active; + return val == QCRYPTO_BLOCK_LUKS_KEY_SLOT_ENABLED; +} + +/* + * Returns the number of slots that are marked as active + * (slots that contain encrypted copy of the master key) + */ +static unsigned int +qcrypto_block_luks_count_active_slots(const QCryptoBlockLUKS *luks) +{ + size_t i = 0; + unsigned int ret = 0; + + for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) { + if (qcrypto_block_luks_slot_active(luks, i)) { + ret++; + } + } + return ret; +} + +/* + * Finds first key slot which is not active + * Returns the key slot index, or -1 if it doesn't exist + */ +static int +qcrypto_block_luks_find_free_keyslot(const QCryptoBlockLUKS *luks) +{ + size_t i; + + for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) { + if (!qcrypto_block_luks_slot_active(luks, i)) { + return i; + } + } + return -1; +} + +/* + * Erases an keyslot given its index + * Returns: + * 0 if the keyslot was erased successfully + * -1 if a error occurred while erasing the keyslot + * + */ +static int +qcrypto_block_luks_erase_key(QCryptoBlock *block, + unsigned int slot_idx, + QCryptoBlockWriteFunc writefunc, + void *opaque, + Error **errp) +{ + QCryptoBlockLUKS *luks = block->opaque; + QCryptoBlockLUKSKeySlot *slot = &luks->header.key_slots[slot_idx]; + g_autofree uint8_t *garbagesplitkey = NULL; + size_t splitkeylen = luks->header.master_key_len * slot->stripes; + size_t i; + + assert(slot_idx < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS); + assert(splitkeylen > 0); + garbagesplitkey = g_new0(uint8_t, splitkeylen); + + /* Reset the key slot header */ + memset(slot->salt, 0, QCRYPTO_BLOCK_LUKS_SALT_LEN); + slot->iterations = 0; + slot->active = QCRYPTO_BLOCK_LUKS_KEY_SLOT_DISABLED; + + qcrypto_block_luks_store_header(block, writefunc, opaque, errp); + /* + * Now try to erase the key material, even if the header + * update failed + */ + for (i = 0; i < QCRYPTO_BLOCK_LUKS_ERASE_ITERATIONS; i++) { + if (qcrypto_random_bytes(garbagesplitkey, splitkeylen, errp) < 0) { + /* + * If we failed to get the random data, still write + * at least zeros to the key slot at least once + */ + if (i > 0) { + return -1; + } + } + if (writefunc(block, + slot->key_offset_sector * QCRYPTO_BLOCK_LUKS_SECTOR_SIZE, + garbagesplitkey, + splitkeylen, + opaque, + errp) != splitkeylen) { + return -1; + } + } + return 0; +} static int qcrypto_block_luks_open(QCryptoBlock *block, @@ -1099,6 +1208,7 @@ qcrypto_block_luks_open(QCryptoBlock *block, luks = g_new0(QCryptoBlockLUKS, 1); block->opaque = luks; + luks->secret = g_strdup(options->u.luks.key_secret); if (qcrypto_block_luks_load_header(block, readfunc, opaque, errp) < 0) { goto fail; @@ -1164,6 +1274,7 @@ qcrypto_block_luks_open(QCryptoBlock *block, fail: qcrypto_block_free_cipher(block); qcrypto_ivgen_free(block->ivgen); + g_free(luks->secret); g_free(luks); return -1; } @@ -1204,7 +1315,7 @@ qcrypto_block_luks_create(QCryptoBlock *block, memcpy(&luks_opts, &options->u.luks, sizeof(luks_opts)); if (!luks_opts.has_iter_time) { - luks_opts.iter_time = 2000; + luks_opts.iter_time = QCRYPTO_BLOCK_LUKS_DEFAULT_ITER_TIME_MS; } if (!luks_opts.has_cipher_alg) { luks_opts.cipher_alg = QCRYPTO_CIPHER_ALG_AES_256; @@ -1244,6 +1355,8 @@ qcrypto_block_luks_create(QCryptoBlock *block, optprefix ? optprefix : ""); goto error; } + luks->secret = g_strdup(options->u.luks.key_secret); + password = qcrypto_secret_lookup_as_utf8(luks_opts.key_secret, errp); if (!password) { goto error; @@ -1471,10 +1584,286 @@ qcrypto_block_luks_create(QCryptoBlock *block, qcrypto_block_free_cipher(block); qcrypto_ivgen_free(block->ivgen); + g_free(luks->secret); g_free(luks); return -1; } +/* + * Given LUKSKeyslotUpdate command, set @slots_bitmap with all slots + * that will be updated with new password (or erased) + * returns 0 on success, and -1 on failure + */ +static int +qcrypto_block_luks_get_update_bitmap(QCryptoBlock *block, + QCryptoBlockReadFunc readfunc, + void *opaque, + const QCryptoBlockAmendOptionsLUKS *opts, + unsigned long *slots_bitmap, + Error **errp) +{ + const QCryptoBlockLUKS *luks = block->opaque; + size_t i; + + bitmap_zero(slots_bitmap, QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS); + + if (opts->has_keyslot) { + /* keyslot set, select only this keyslot */ + int keyslot = opts->keyslot; + + if (keyslot < 0 || keyslot >= QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS) { + error_setg(errp, + "Invalid slot %u specified, must be between 0 and %u", + keyslot, QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS - 1); + return -1; + } + bitmap_set(slots_bitmap, keyslot, 1); + + } else if (opts->has_old_secret) { + /* initially select all active keyslots */ + for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) { + if (qcrypto_block_luks_slot_active(luks, i)) { + bitmap_set(slots_bitmap, i, 1); + } + } + } else { + /* find a free keyslot */ + int slot = qcrypto_block_luks_find_free_keyslot(luks); + + if (slot == -1) { + error_setg(errp, + "Can't add a keyslot - all key slots are in use"); + return -1; + } + bitmap_set(slots_bitmap, slot, 1); + } + + if (opts->has_old_secret) { + /* now deselect all keyslots that don't contain the password */ + g_autofree uint8_t *tmpkey = g_new0(uint8_t, + luks->header.master_key_len); + + for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) { + g_autofree char *old_password = NULL; + int rv; + + if (!test_bit(i, slots_bitmap)) { + continue; + } + + old_password = qcrypto_secret_lookup_as_utf8(opts->old_secret, + errp); + if (!old_password) { + return -1; + } + + rv = qcrypto_block_luks_load_key(block, + i, + old_password, + tmpkey, + readfunc, + opaque, + errp); + if (rv == -1) { + return -1; + } else if (rv == 0) { + bitmap_clear(slots_bitmap, i, 1); + } + } + } + return 0; +} + +/* + * Erase a set of keyslots given in @slots_bitmap + */ +static int qcrypto_block_luks_erase_keys(QCryptoBlock *block, + QCryptoBlockReadFunc readfunc, + QCryptoBlockWriteFunc writefunc, + void *opaque, + unsigned long *slots_bitmap, + bool force, + Error **errp) +{ + QCryptoBlockLUKS *luks = block->opaque; + long slot_count = bitmap_count_one(slots_bitmap, + QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS); + size_t i; + + /* safety checks */ + if (!force && slot_count == qcrypto_block_luks_count_active_slots(luks)) { + error_setg(errp, + "Requested operation will erase all active keyslots" + " which will erase all the data in the image" + " irreversibly - refusing operation"); + return -EINVAL; + } + + /* new apply the update */ + for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) { + if (!test_bit(i, slots_bitmap)) { + continue; + } + if (qcrypto_block_luks_erase_key(block, i, writefunc, opaque, errp)) { + error_append_hint(errp, "Failed to erase keyslot %zu", i); + return -EINVAL; + } + } + return 0; +} + +/* + * Set a set of keyslots to @master_key encrypted by @new_secret + */ +static int qcrypto_block_luks_set_keys(QCryptoBlock *block, + QCryptoBlockReadFunc readfunc, + QCryptoBlockWriteFunc writefunc, + void *opaque, + unsigned long *slots_bitmap, + uint8_t *master_key, + uint64_t iter_time, + char *new_secret, + bool force, + Error **errp) +{ + QCryptoBlockLUKS *luks = block->opaque; + g_autofree char *new_password = NULL; + size_t i; + + /* safety checks */ + if (!force) { + for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) { + if (!test_bit(i, slots_bitmap)) { + continue; + } + if (qcrypto_block_luks_slot_active(luks, i)) { + error_setg(errp, + "Refusing to overwrite active slot %zu - " + "please erase it first", i); + return -EINVAL; + } + } + } + + /* Load the new password */ + new_password = qcrypto_secret_lookup_as_utf8(new_secret, errp); + if (!new_password) { + return -EINVAL; + } + + /* Apply the update */ + for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) { + if (!test_bit(i, slots_bitmap)) { + continue; + } + if (qcrypto_block_luks_store_key(block, i, new_password, master_key, + iter_time, writefunc, opaque, errp)) { + error_append_hint(errp, "Failed to write to keyslot %zu", i); + return -EINVAL; + } + } + return 0; +} + +static int +qcrypto_block_luks_amend_options(QCryptoBlock *block, + QCryptoBlockReadFunc readfunc, + QCryptoBlockWriteFunc writefunc, + void *opaque, + QCryptoBlockAmendOptions *options, + bool force, + Error **errp) +{ + QCryptoBlockLUKS *luks = block->opaque; + QCryptoBlockAmendOptionsLUKS *opts_luks = &options->u.luks; + g_autofree uint8_t *master_key = NULL; + g_autofree unsigned long *update_bitmap = NULL; + char *unlock_secret = NULL; + long slot_count; + + unlock_secret = opts_luks->has_unlock_secret ? opts_luks->unlock_secret : + luks->secret; + + /* Retrieve set of slots that we need to update */ + update_bitmap = bitmap_new(QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS); + if (qcrypto_block_luks_get_update_bitmap(block, readfunc, opaque, opts_luks, + update_bitmap, errp) != 0) { + return -1; + } + + slot_count = bitmap_count_one(update_bitmap, + QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS); + + /* no matching slots, so nothing to do */ + if (slot_count == 0) { + error_setg(errp, "Requested operation didn't match any slots"); + return -1; + } + + if (opts_luks->state == LUKS_KEYSLOT_STATE_ACTIVE) { + + uint64_t iter_time = opts_luks->has_iter_time ? + opts_luks->iter_time : + QCRYPTO_BLOCK_LUKS_DEFAULT_ITER_TIME_MS; + + if (!opts_luks->has_new_secret) { + error_setg(errp, "'new-secret' is required to activate a keyslot"); + return -EINVAL; + } + if (opts_luks->has_old_secret) { + error_setg(errp, + "'old-secret' must not be given when activating keyslots"); + return -EINVAL; + } + + /* Locate the password that will be used to retrieve the master key */ + g_autofree char *old_password; + old_password = qcrypto_secret_lookup_as_utf8(unlock_secret, errp); + if (!old_password) { + return -EINVAL; + } + + /* Try to retrieve the master key */ + master_key = g_new0(uint8_t, luks->header.master_key_len); + if (qcrypto_block_luks_find_key(block, old_password, master_key, + readfunc, opaque, errp) < 0) { + error_append_hint(errp, "Failed to retrieve the master key"); + return -EINVAL; + } + + /* Now set the new keyslots */ + if (qcrypto_block_luks_set_keys(block, readfunc, writefunc, + opaque, update_bitmap, master_key, + iter_time, + opts_luks->new_secret, + force, errp) != 0) { + return -1; + } + } else { + if (opts_luks->has_new_secret) { + error_setg(errp, + "'new-secret' must not be given when erasing keyslots"); + return -EINVAL; + } + if (opts_luks->has_iter_time) { + error_setg(errp, + "'iter-time' must not be given when erasing keyslots"); + return -EINVAL; + } + if (opts_luks->has_unlock_secret) { + error_setg(errp, + "'unlock_secret' must not be given when erasing keyslots"); + return -EINVAL; + } + + if (qcrypto_block_luks_erase_keys(block, readfunc, writefunc, + opaque, update_bitmap, force, + errp) != 0) { + return -1; + } + } + return 0; +} static int qcrypto_block_luks_get_info(QCryptoBlock *block, QCryptoBlockInfo *info, @@ -1523,7 +1912,11 @@ static int qcrypto_block_luks_get_info(QCryptoBlock *block, static void qcrypto_block_luks_cleanup(QCryptoBlock *block) { - g_free(block->opaque); + QCryptoBlockLUKS *luks = block->opaque; + if (luks) { + g_free(luks->secret); + g_free(luks); + } } @@ -1560,6 +1953,7 @@ qcrypto_block_luks_encrypt(QCryptoBlock *block, const QCryptoBlockDriver qcrypto_block_driver_luks = { .open = qcrypto_block_luks_open, .create = qcrypto_block_luks_create, + .amend = qcrypto_block_luks_amend_options, .get_info = qcrypto_block_luks_get_info, .cleanup = qcrypto_block_luks_cleanup, .decrypt = qcrypto_block_luks_decrypt, diff --git a/qapi/crypto.json b/qapi/crypto.json index 3fd0ce177e..fe600fc608 100644 --- a/qapi/crypto.json +++ b/qapi/crypto.json @@ -1,6 +1,8 @@ # -*- Mode: Python -*- # +{ 'include': 'common.json' } + ## # = Cryptography ## @@ -297,7 +299,6 @@ 'uuid': 'str', 'slots': [ 'QCryptoBlockInfoLUKSSlot' ] }} - ## # @QCryptoBlockInfo: # @@ -310,7 +311,63 @@ 'discriminator': 'format', 'data': { 'luks': 'QCryptoBlockInfoLUKS' } } +## +# @LUKSKeyslotState: +# +# Defines state of keyslots that are affected by the update +# +# @active: The slots contain the given password and marked as active +# @inactive: The slots are erased (contain garbage) and marked as inactive +# +# Since: 5.0 +## +{ 'enum': 'LUKSKeyslotState', + 'data': [ 'active', 'inactive' ] } + +## +# @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 +# +# +# Since 5.0 +## +{ 'struct': 'QCryptoBlockAmendOptionsLUKS', + 'data': { 'state': 'LUKSKeyslotState', + '*new-secret': 'str', + '*old-secret': 'str', + '*keyslot': 'int', + '*iter-time': 'int', + '*unlock-secret': 'str' } } ## # @QCryptoBlockAmendOptions: @@ -324,4 +381,4 @@ 'base': 'QCryptoBlockOptionsBase', 'discriminator': 'format', 'data': { - } } + 'luks': 'QCryptoBlockAmendOptionsLUKS' } }
Next few patches will expose that functionality to the user. Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> --- crypto/block-luks.c | 398 +++++++++++++++++++++++++++++++++++++++++++- qapi/crypto.json | 61 ++++++- 2 files changed, 455 insertions(+), 4 deletions(-)