[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 02/13] qcrypto-luks: implement encryption key management
From: |
Markus Armbruster |
Subject: |
Re: [PATCH 02/13] qcrypto-luks: implement encryption key management |
Date: |
Tue, 21 Jan 2020 08:54:51 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) |
Reviewing just the QAPI schema.
Maxim Levitsky <address@hidden> writes:
> Next few patches will expose that functionality
> to the user.
>
> Signed-off-by: Maxim Levitsky <address@hidden>
> ---
> crypto/block-luks.c | 374 +++++++++++++++++++++++++++++++++++++++++++-
> qapi/crypto.json | 50 +++++-
> 2 files changed, 421 insertions(+), 3 deletions(-)
>
> diff --git a/crypto/block-luks.c b/crypto/block-luks.c
> index 4861db810c..349e95fed3 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,112 @@ 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 +1212,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 +1278,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 +1319,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 +1359,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 +1588,260 @@ 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, return @slots_bitmap with all slots
> + * that will be updated with new password (or erased)
> + * returns number of affected slots
> + */
> +static int qcrypto_block_luks_get_slots_bitmap(QCryptoBlock *block,
> + QCryptoBlockReadFunc readfunc,
> + void *opaque,
> + const LUKSKeyslotUpdate
> *command,
> + unsigned long *slots_bitmap,
> + Error **errp)
> +{
> + const QCryptoBlockLUKS *luks = block->opaque;
> + size_t i;
> + int ret = 0;
> +
> + if (command->has_keyslot) {
> + /* keyslot set, select only this keyslot */
> + int keyslot = command->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);
> + goto error;
> + }
> + bitmap_set(slots_bitmap, keyslot, 1);
> + ret++;
> +
> + } else if (command->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);
> + ret++;
> + }
> + }
> + } 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");
> + goto error;
> + }
> + bitmap_set(slots_bitmap, slot, 1);
> + ret++;
> + }
> +
> + if (command->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(command->old_secret,
> + errp);
> + if (!old_password) {
> + goto error;
> + }
> +
> + rv = qcrypto_block_luks_load_key(block,
> + i,
> + old_password,
> + tmpkey,
> + readfunc,
> + opaque,
> + errp);
> + if (rv == -1)
> + goto error;
> + else if (rv == 0) {
> + bitmap_clear(slots_bitmap, i, 1);
> + ret--;
> + }
> + }
> + }
> + return ret;
> +error:
> + return -1;
> +}
> +
> +/*
> + * Apply a single keyslot update command as described in @command
> + * Optionally use @unlock_secret to retrieve the master key
> + */
> +static int
> +qcrypto_block_luks_apply_keyslot_update(QCryptoBlock *block,
> + QCryptoBlockReadFunc readfunc,
> + QCryptoBlockWriteFunc writefunc,
> + void *opaque,
> + LUKSKeyslotUpdate *command,
> + const char *unlock_secret,
> + uint8_t **master_key,
> + bool force,
> + Error **errp)
> +{
> + QCryptoBlockLUKS *luks = block->opaque;
> + g_autofree unsigned long *slots_bitmap = NULL;
> + int64_t iter_time = QCRYPTO_BLOCK_LUKS_DEFAULT_ITER_TIME_MS;
> + int slot_count;
> + size_t i;
> + char *new_password;
> + bool erasing;
> +
> + slots_bitmap = bitmap_new(QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS);
> + slot_count = qcrypto_block_luks_get_slots_bitmap(block, readfunc, opaque,
> + command, slots_bitmap,
> + errp);
> + if (slot_count == -1) {
> + goto error;
> + }
> + /* no matching slots, so nothing to do */
> + if (slot_count == 0) {
> + error_setg(errp, "Requested operation didn't match any slots");
> + goto error;
> + }
> + /*
> + * slot is erased when the password is set to null, or empty string
> + * (for compatibility with command line)
> + */
> + erasing = command->new_secret->type == QTYPE_QNULL ||
> + strlen(command->new_secret->u.s) == 0;
> +
> + /* safety checks */
> + if (!force) {
> + if (erasing) {
> + if (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");
> + goto error;
> + }
> + } else {
> + 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);
> + goto error;
> + }
> + }
> + }
> + }
> +
> + /* setup the data needed for storing the new keyslot */
> + if (!erasing) {
> + /* Load the master key if it wasn't already loaded */
> + if (!*master_key) {
> + g_autofree char *old_password;
> + old_password = qcrypto_secret_lookup_as_utf8(unlock_secret,
> errp);
> + if (!old_password) {
> + goto error;
> + }
> + *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");
> + goto error;
> + }
> + }
> + new_password =
> qcrypto_secret_lookup_as_utf8(command->new_secret->u.s,
> + errp);
> + if (!new_password) {
> + goto error;
> + }
> + if (command->has_iter_time) {
> + iter_time = command->iter_time;
> + }
> + }
> +
> + /* new apply the update */
> + for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
> + if (!test_bit(i, slots_bitmap)) {
> + continue;
> + }
> + if (erasing) {
> + if (qcrypto_block_luks_erase_key(block, i,
> + writefunc,
> + opaque,
> + errp)) {
> + error_append_hint(errp, "Failed to erase keyslot %zu", i);
> + goto error;
> + }
> + } else {
> + 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);
> + goto error;
> + }
> + }
> + }
> + return 0;
> +error:
> + return -EINVAL;
> +}
> +
> +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 *options_luks = &options->u.luks;
> + LUKSKeyslotUpdateList *ptr;
> + g_autofree uint8_t *master_key = NULL;
> + int ret;
> +
> + char *unlock_secret = options_luks->has_unlock_secret ?
> + options_luks->unlock_secret :
> + luks->secret;
> +
> + for (ptr = options_luks->keys; ptr; ptr = ptr->next) {
> + ret = qcrypto_block_luks_apply_keyslot_update(block, readfunc,
> + writefunc, opaque,
> + ptr->value,
> + unlock_secret,
> + &master_key,
> + force, errp);
> +
> + if (ret != 0) {
> + goto error;
> + }
> + }
> + return 0;
> +error:
> + return -1;
> +}
>
> static int qcrypto_block_luks_get_info(QCryptoBlock *block,
> QCryptoBlockInfo *info,
> @@ -1523,7 +1890,9 @@ 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;
> + g_free(luks->secret);
> + g_free(luks);
> }
>
>
> @@ -1560,6 +1929,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 9faebd03d4..e83847c71e 100644
> --- a/qapi/crypto.json
> +++ b/qapi/crypto.json
> @@ -1,6 +1,8 @@
> # -*- Mode: Python -*-
> #
>
> +{ 'include': 'common.json' }
> +
> ##
> # = Cryptography
> ##
> @@ -310,6 +312,52 @@
> 'discriminator': 'format',
> 'data': { 'luks': 'QCryptoBlockInfoLUKS' } }
>
> +##
> +# @LUKSKeyslotUpdate:
> +#
> +# @keyslot: If specified, will update only keyslot with this index
> +#
> +# @old-secret: If specified, will only update keyslots that
> +# can be opened with password which is contained in
> +# QCryptoSecret with @old-secret ID
> +#
> +# If neither @keyslot nor @old-secret is specified,
> +# first empty keyslot is selected for the update
> +#
> +# @new-secret: The ID of a QCryptoSecret object providing a new
> decryption
> +# key to place in all matching keyslots.
> +# null/empty string erases all matching keyslots
I hate making the empty string do something completely different than a
non-empty string.
What about making @new-secret optional, and have absent @new-secret
erase?
> +# unless
> +# last valid keyslot is erased.
Leaves me to wonder what happens when I try. If I read your code
correctly, it's an error. Suggest "You cannot erase the last valid
keyslot".
Not documented here: "Refusing to overwrite active slot".
> +#
> +# @iter-time: number of milliseconds to spend in
> +# PBKDF passphrase processing
Default?
> +# Since: 5.0
> +##
> +{ 'struct': 'LUKSKeyslotUpdate',
> + 'data': {
> + '*keyslot': 'int',
> + '*old-secret': 'str',
> + 'new-secret' : 'StrOrNull',
> + '*iter-time' : 'int' } }
> +
> +
> +##
> +# @QCryptoBlockAmendOptionsLUKS:
> +#
> +# The options that can be changed on existing luks encrypted device
> +# @keys: list of keyslot updates to perform
> +# (updates are performed in order)
> +# @unlock-secret: use this secret to retrieve the current master key
> +# if not given will use the same secret as one
"as the one"?
> +# that was used to open the image
> +#
> +# Since: 5.0
> +##
> +{ 'struct': 'QCryptoBlockAmendOptionsLUKS',
> + 'data' : {
> + 'keys': ['LUKSKeyslotUpdate'],
> + '*unlock-secret' : 'str' } }
> +
>
>
> ##
> @@ -324,4 +372,4 @@
> 'base': 'QCryptoBlockOptionsBase',
> 'discriminator': 'format',
> 'data': {
> - } }
> + 'luks': 'QCryptoBlockAmendOptionsLUKS' } }
- [PATCH 00/13] LUKS: encryption slot management using amend interface, Maxim Levitsky, 2020/01/14
- [PATCH 01/13] qcrypto: add generic infrastructure for crypto options amendment, Maxim Levitsky, 2020/01/14
- [PATCH 02/13] qcrypto-luks: implement encryption key management, Maxim Levitsky, 2020/01/14
- Re: [PATCH 02/13] qcrypto-luks: implement encryption key management,
Markus Armbruster <=
- Re: [PATCH 02/13] qcrypto-luks: implement encryption key management, Maxim Levitsky, 2020/01/21
- Re: [PATCH 02/13] qcrypto-luks: implement encryption key management, Daniel P . Berrangé, 2020/01/28
- Re: [PATCH 02/13] qcrypto-luks: implement encryption key management, Daniel P . Berrangé, 2020/01/28
- Re: [PATCH 02/13] qcrypto-luks: implement encryption key management, Maxim Levitsky, 2020/01/29
- Re: [PATCH 02/13] qcrypto-luks: implement encryption key management, Kevin Wolf, 2020/01/30
- Re: [PATCH 02/13] qcrypto-luks: implement encryption key management, Daniel P . Berrangé, 2020/01/30
- Re: [PATCH 02/13] qcrypto-luks: implement encryption key management, Kevin Wolf, 2020/01/30
- Re: [PATCH 02/13] qcrypto-luks: implement encryption key management, Daniel P . Berrangé, 2020/01/30
- Re: [PATCH 02/13] qcrypto-luks: implement encryption key management, Markus Armbruster, 2020/01/30
- Re: [PATCH 02/13] qcrypto-luks: implement encryption key management, Markus Armbruster, 2020/01/30