Re: [Qemu-devel] [PATCH 03/10] qcrypto-luks: implement the encryption key management
On Fri, 2019-09-06 at 14:55 +0100, Daniel P. Berrangé wrote: > On Fri, Aug 30, 2019 at 11:56:01PM +0300, Maxim Levitsky wrote: > > Signed-off-by: Maxim Levitsky > > --- > > crypto/block-luks.c | 366 +++- > > 1 file changed, 364 insertions(+), 2 deletions(-) > > > > diff --git a/crypto/block-luks.c b/crypto/block-luks.c > > index ba20d55246..21325fbc79 100644 > > --- a/crypto/block-luks.c > > +++ b/crypto/block-luks.c > > @@ -70,6 +70,9 @@ typedef struct QCryptoBlockLUKSKeySlot > > QCryptoBlockLUKSKeySlot; > > > > #define QCRYPTO_BLOCK_LUKS_SECTOR_SIZE 512LL > > > > +#define QCRYPTO_BLOCK_LUKS_DEFAULT_ITER_TIME 2000 > > Perhaps use ITER_TIME_MS to make it clear it is millisecs Why not... done. > > > +#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 +222,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; > > + > > +/* > > + * Returns true if a slot i is marked as active > > + * (contains encrypted copy of the master key) > > + */ > > + > > +static bool > > No blank line is wanted between the comment & function. > Likewise for the rest of this patch series No problem! > > > +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; > > +} > > > > > +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_malloc0(splitkeylen); > > I'd prefer g_new0(uint8_t, splitkeylen) Done. > > > + > > +/* 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; > > + > > > > @@ -1522,6 +1700,187 @@ qcrypto_block_luks_create(QCryptoBlock *block, > > } > > > > > > +#define CHECK_NON_AMEND_OPTION(luks, luks_opts, name) \ > > +if (luks_opts.has_##name && luks_opts.name != luks->name) { \ > > +error_setg(errp, "Option \"" #name "\" can't be amended"); \ > > +goto cleanup; \ > > +} > > + > > +static int > > +qcrypto_block_luks_amend_options(QCryptoBlock *block, > > + QCryptoBlockReadFunc readfunc, > > + QCryptoBlockWriteFunc writefunc, > > + void *opaque, > > + QCryptoBlockCreateOptions *options, > > + bool force, > > + Error **errp) > > +{ > > +QCryptoBlockLUKS *luks = block->opaque; > > +QCryptoBlockCreateOptionsLUKS luks_opts; > > +g_autofree char *old_password = NULL; > > +g_autofree char *password = NULL; > > +const char *unlock_secret = luks->secret; > > +g_autofree uint8_t *masterkey = NULL; > > +int slot = -1; > > +int ret = -1; > > +bool active = true; > > +int64_t iter_time = QCRYPTO_BLOCK_LUKS_DEFAULT_ITER_TIME; > > + > > +memcpy(&luks_opts, &options->u.luks, sizeof(luks_opts)); > > + > > +CHECK_NON_AMEND_OPTION(luks, luks_opts, cipher_alg); > > +CHECK_NON_AMEND_OPTION(luks, luks_opts, cipher_mode); > > +CHECK_NON_AMEND_OPTION(luks, luks_opts, ivgen_alg); > > +CHECK_NON_AMEND_OPTION(luks, luks_opts, ivgen_hash_alg); > > +CHECK_NON_AMEND_OPTION(luks, luks_opts, hash_alg); > > + > > +/* Read given slot and check it */ > > +if (luks_opts.has_slot) { > > +slot = luks_opts.slot; > > +if (slot < 0 || slot >= QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS) { > > +error_setg(errp, > > + "Given key slot %i is not supported by LUKS", slot); > > + goto cleanup; > > Off by 1 one indent Sorry about that. > > > +} > > +} > > + > > +if (luks_opts.has_iter_time) { > > +iter_time = luks_opts.iter_time; > > +} > > + > > +if (luks_opts.has_active && luks_opts.active == false) { > > +active = false; > > +} > >
Re: [Qemu-devel] [PATCH 03/10] qcrypto-luks: implement the encryption key management
On Fri, Aug 30, 2019 at 11:56:01PM +0300, Maxim Levitsky wrote: > Signed-off-by: Maxim Levitsky > --- > crypto/block-luks.c | 366 +++- > 1 file changed, 364 insertions(+), 2 deletions(-) > > diff --git a/crypto/block-luks.c b/crypto/block-luks.c > index ba20d55246..21325fbc79 100644 > --- a/crypto/block-luks.c > +++ b/crypto/block-luks.c > @@ -70,6 +70,9 @@ typedef struct QCryptoBlockLUKSKeySlot > QCryptoBlockLUKSKeySlot; > > #define QCRYPTO_BLOCK_LUKS_SECTOR_SIZE 512LL > > +#define QCRYPTO_BLOCK_LUKS_DEFAULT_ITER_TIME 2000 Perhaps use ITER_TIME_MS to make it clear it is millisecs > +#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 +222,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; > + > +/* > + * Returns true if a slot i is marked as active > + * (contains encrypted copy of the master key) > + */ > + > +static bool No blank line is wanted between the comment & function. Likewise for the rest of this patch series > +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; > +} > +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_malloc0(splitkeylen); I'd prefer 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; > + > @@ -1522,6 +1700,187 @@ qcrypto_block_luks_create(QCryptoBlock *block, > } > > > +#define CHECK_NON_AMEND_OPTION(luks, luks_opts, name) \ > +if (luks_opts.has_##name && luks_opts.name != luks->name) { \ > +error_setg(errp, "Option \"" #name "\" can't be amended"); \ > +goto cleanup; \ > +} > + > +static int > +qcrypto_block_luks_amend_options(QCryptoBlock *block, > + QCryptoBlockReadFunc readfunc, > + QCryptoBlockWriteFunc writefunc, > + void *opaque, > + QCryptoBlockCreateOptions *options, > + bool force, > + Error **errp) > +{ > +QCryptoBlockLUKS *luks = block->opaque; > +QCryptoBlockCreateOptionsLUKS luks_opts; > +g_autofree char *old_password = NULL; > +g_autofree char *password = NULL; > +const char *unlock_secret = luks->secret; > +g_autofree uint8_t *masterkey = NULL; > +int slot = -1; > +int ret = -1; > +bool active = true; > +int64_t iter_time = QCRYPTO_BLOCK_LUKS_DEFAULT_ITER_TIME; > + > +memcpy(&luks_opts, &options->u.luks, sizeof(luks_opts)); > + > +CHECK_NON_AMEND_OPTION(luks, luks_opts, cipher_alg); > +CHECK_NON_AMEND_OPTION(luks, luks_opts, cipher_mode); > +CHECK_NON_AMEND_OPTION(luks, luks_opts, ivgen_alg); > +CHECK_NON_AMEND_OPTION(luks, luks_opts, ivgen_hash_alg); > +CHECK_NON_AMEND_OPTION(luks, luks_opts, hash_alg); > + > +/* Read given slot and check it */ > +if (luks_opts.has_slot) { > +slot = luks_opts.slot; > +if (slot < 0 || slot >= QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS) { > +error_setg(errp, > + "Given key slot %i is not supported by LUKS", slot); > + goto cleanup; Off by 1 one indent > +} > +} > + > +if (luks_opts.has_iter_time) { > +iter_time = luks_opts.iter_time; > +} > + > +if (luks_opts.has_active && luks_opts.active == false) { > +active = false; > +} > + > +if (active) { > + > +/* Check that we are not overwriting an active slot */ > +if (!force && slot != -1 && > +qcrypto_block_luks_slot_active(luks, slot)) { > + > +error_setg(errp, "Can't update an active key slot %i", > + slot); > +goto cleanup; > +} > + > +/* check that we have the passwords*/ > +
[Qemu-devel] [PATCH 03/10] qcrypto-luks: implement the encryption key management
Signed-off-by: Maxim Levitsky --- crypto/block-luks.c | 366 +++- 1 file changed, 364 insertions(+), 2 deletions(-) diff --git a/crypto/block-luks.c b/crypto/block-luks.c index ba20d55246..21325fbc79 100644 --- a/crypto/block-luks.c +++ b/crypto/block-luks.c @@ -70,6 +70,9 @@ typedef struct QCryptoBlockLUKSKeySlot QCryptoBlockLUKSKeySlot; #define QCRYPTO_BLOCK_LUKS_SECTOR_SIZE 512LL +#define QCRYPTO_BLOCK_LUKS_DEFAULT_ITER_TIME 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 +222,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; }; @@ -1089,6 +1095,175 @@ qcrypto_block_luks_find_key(QCryptoBlock *block, } + +/* + * 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 + * (contains 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 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_malloc0(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; +} + + +/* + * Erase all the keys that match the given password + * Will stop when only one keyslot is remaining + * Returns number of slots that were erased or -1 on failure + */ + +static int +qcrypto_block_luks_erase_matching_keys(QCryptoBlock *block, + const char *password, + QCryptoBlockReadFunc readfunc, + QCryptoBlockWriteFunc writefunc, + void *opaque, + bool force, + Error **errp) +{ +QCryptoBlockLUKS *luks = block->opaque; +size_t i; +int rv; +g_autofree uint8_t *masterkey = NULL; +unsigned int erased_cnt = 0; +unsigned int active_slot_cnt = qcrypto_block_luks_count_active_slots(luks); + +masterkey = g_new0(uint8_t, luks->header.master_key_le