[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 02/14] qcrypto/luks: implement encryption key management
From: |
Daniel P . Berrangé |
Subject: |
Re: [PATCH v4 02/14] qcrypto/luks: implement encryption key management |
Date: |
Thu, 7 May 2020 12:02:10 +0100 |
User-agent: |
Mutt/1.13.4 (2020-02-15) |
On Tue, May 05, 2020 at 11:08:07PM +0300, Maxim Levitsky wrote:
> Next few patches will expose that functionality
> to the user.
>
> Signed-off-by: Maxim Levitsky <address@hidden>
> ---
> crypto/block-luks.c | 395 +++++++++++++++++++++++++++++++++++++++++++-
> qapi/crypto.json | 61 ++++++-
> 2 files changed, 452 insertions(+), 4 deletions(-)
>
> diff --git a/crypto/block-luks.c b/crypto/block-luks.c
> index 4861db810c..c108518df1 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;
> + Error *local_err = NULL;
> + int ret;
> +
> + 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;
> +
> + ret = qcrypto_block_luks_store_header(block, writefunc,
> + opaque, &local_err);
> +
> + if (ret) {
ret < 0
> + error_propagate(errp, local_err);
> + }
> + /*
> + * 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, &local_err) < 0) {
> + /*
> + * If we failed to get the random data, still write
> + * at least zeros to the key slot at least once
> + */
> + error_propagate(errp, local_err);
> +
> + if (i > 0) {
> + return -1;
> + }
> + }
> + if (writefunc(block,
> + slot->key_offset_sector *
> QCRYPTO_BLOCK_LUKS_SECTOR_SIZE,
> + garbagesplitkey,
> + splitkeylen,
> + opaque,
> + &local_err) != splitkeylen) {
> + error_propagate(errp, local_err);
> + return -1;
> + }
> + }
> + return 0;
We need to "return ret" here, in case the earlier store_header() failed
> +}
>
> +static int
> +qcrypto_block_luks_amend_add_keyslot(QCryptoBlock *block,
> + QCryptoBlockReadFunc readfunc,
> + QCryptoBlockWriteFunc writefunc,
> + void *opaque,
> + QCryptoBlockAmendOptionsLUKS *opts_luks,
> + bool force,
> + Error **errp)
> +{
> + QCryptoBlockLUKS *luks = block->opaque;
> + uint64_t iter_time = opts_luks->has_iter_time ?
> + opts_luks->iter_time :
> + QCRYPTO_BLOCK_LUKS_DEFAULT_ITER_TIME_MS;
> + int keyslot;
> + g_autofree char *old_password = NULL;
> + g_autofree char *new_password = NULL;
> + g_autofree uint8_t *master_key = NULL;
> +
> + char *secret = opts_luks->has_secret ? opts_luks->secret : luks->secret;
> +
> + if (!opts_luks->has_new_secret) {
> + error_setg(errp, "'new-secret' is required to activate a keyslot");
> + return -1;
> + }
> + if (opts_luks->has_old_secret) {
> + error_setg(errp,
> + "'old-secret' must not be given when activating
> keyslots");
> + return -1;
> + }
> +
> + if (opts_luks->has_keyslot) {
> + keyslot = opts_luks->keyslot;
> + if (keyslot < 0 || keyslot >= QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS) {
> + error_setg(errp,
> + "Invalid keyslot %u specified, must be between 0 and
> %u",
> + keyslot, QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS - 1);
> + return -1;
> + }
> + } else {
> + keyslot = qcrypto_block_luks_find_free_keyslot(luks);
> + if (keyslot == -1) {
> + error_setg(errp,
> + "Can't add a keyslot - all keyslots are in use");
> + return -1;
> + }
> + }
> +
> + if (!force && qcrypto_block_luks_slot_active(luks, keyslot)) {
> + error_setg(errp,
> + "Refusing to overwrite active keyslot %i - "
> + "please erase it first",
> + keyslot);
> + return -EINVAL;
s/-EINVAL/-1/
> + }
> +
> + /* Locate the password that will be used to retrieve the master key */
> + old_password = qcrypto_secret_lookup_as_utf8(secret, errp);
> + if (!old_password) {
> + return -1;
> + }
> +
> + /* 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 -1;
> + }
> +
> + /* Locate the new password*/
> + new_password = qcrypto_secret_lookup_as_utf8(opts_luks->new_secret,
> errp);
> + if (!new_password) {
> + return -EINVAL;
s/-EINVAL/-1/
> + }
> +
> + /* Now set the new keyslots */
> + if (qcrypto_block_luks_store_key(block, keyslot, new_password,
> master_key,
> + iter_time, writefunc, opaque, errp)) {
> + error_append_hint(errp, "Failed to write to keyslot %i", keyslot);
> + return -EINVAL;
s/-EINVAL/-1/
> + }
> + return 0;
> +}
> +
> +static int
> +qcrypto_block_luks_amend_erase_keyslots(QCryptoBlock *block,
> + QCryptoBlockReadFunc readfunc,
> + QCryptoBlockWriteFunc writefunc,
> + void *opaque,
> + QCryptoBlockAmendOptionsLUKS
> *opts_luks,
> + bool force,
> + Error **errp)
> +{
> + QCryptoBlockLUKS *luks = block->opaque;
> + g_autofree uint8_t *tmpkey = NULL;
> + g_autofree char *old_password = NULL;
> +
> + if (opts_luks->has_new_secret) {
> + error_setg(errp,
> + "'new-secret' must not be given when erasing keyslots");
> + return -1;
> + }
> + if (opts_luks->has_iter_time) {
> + error_setg(errp,
> + "'iter-time' must not be given when erasing keyslots");
> + return -1;
> + }
> + if (opts_luks->has_secret) {
> + error_setg(errp,
> + "'secret' must not be given when erasing keyslots");
> + return -1;
> + }
> +
> + /* Load the old password if given */
> + if (opts_luks->has_old_secret) {
> + old_password = qcrypto_secret_lookup_as_utf8(opts_luks->old_secret,
> + errp);
> + if (!old_password) {
> + return -1;
> + }
> +
> + /*
> + * Allocate a temporary key buffer that we will need when
> + * checking if slot matches the given old password
> + */
> + tmpkey = g_new0(uint8_t, luks->header.master_key_len);
> +}
Indent missing
With the minor points fixed
Reviewed-by: Daniel P. Berrangé <address@hidden>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
- [PATCH v4 00/14] LUKS: encryption slot management using amend interface, Maxim Levitsky, 2020/05/05
- [PATCH v4 01/14] qcrypto/core: add generic infrastructure for crypto options amendment, Maxim Levitsky, 2020/05/05
- [PATCH v4 02/14] qcrypto/luks: implement encryption key management, Maxim Levitsky, 2020/05/05
- Re: [PATCH v4 02/14] qcrypto/luks: implement encryption key management,
Daniel P . Berrangé <=
- [PATCH v4 03/14] block/amend: add 'force' option, Maxim Levitsky, 2020/05/05
- [PATCH v4 04/14] block/amend: separate amend and create options for qemu-img, Maxim Levitsky, 2020/05/05
- [PATCH v4 06/14] block/crypto: rename two functions, Maxim Levitsky, 2020/05/05
- [PATCH v4 05/14] block/amend: refactor qcow2 amend options, Maxim Levitsky, 2020/05/05
- [PATCH v4 07/14] block/crypto: implement the encryption key management, Maxim Levitsky, 2020/05/05
- [PATCH v4 09/14] iotests: filter few more luks specific create options, Maxim Levitsky, 2020/05/05
- [PATCH v4 10/14] iotests: qemu-img tests for luks key management, Maxim Levitsky, 2020/05/05
- [PATCH v4 08/14] block/qcow2: extend qemu-img amend interface with crypto options, Maxim Levitsky, 2020/05/05