* [Qemu-devel] [PATCH 00/12] crypto/luks: preparation for encryption key managment
@ 2019-09-12 9:16 Maxim Levitsky
2019-09-12 9:16 ` [Qemu-devel] [PATCH 01/12] block-crypto: misc refactoring Maxim Levitsky
` (11 more replies)
0 siblings, 12 replies; 18+ messages in thread
From: Maxim Levitsky @ 2019-09-12 9:16 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Daniel P. Berrangé,
qemu-block, John Snow, Markus Armbruster, Max Reitz,
Maxim Levitsky
Hi!
This patch series is the refactoring/preparation part of the
former patch series I had sent which adds support for luks
key management.
This series includes all the feedback from the last review iteration
and one new patch that removes errno values from .open
callback of luks crypto driver since these values are not
used anyway.
Best regards,
Maxim Levitsky
Maxim Levitsky (12):
block-crypto: misc refactoring
qcrypto-luks: rename some fields in QCryptoBlockLUKSHeader
qcrypto-luks: don't overwrite cipher_mode in header
qcrypto-luks: simplify masterkey and masterkey length
qcrypto-luks: pass keyslot index rather that pointer to the keyslot
qcrypto-luks: use the parsed encryption settings in QCryptoBlockLUKS
qcrypto-luks: purge unused error codes from open callback
qcrypto-luks: extract store and load header
qcrypto-luks: extract check and parse header
qcrypto-luks: extract store key function
qcrypto-luks: simplify the math used for keyslot locations
qcrypto-luks: more rigorous header checking
block/crypto.c | 12 +-
crypto/block-luks.c | 1023 +++++++++++++++++++++++++------------------
2 files changed, 602 insertions(+), 433 deletions(-)
--
2.17.2
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH 01/12] block-crypto: misc refactoring
2019-09-12 9:16 [Qemu-devel] [PATCH 00/12] crypto/luks: preparation for encryption key managment Maxim Levitsky
@ 2019-09-12 9:16 ` Maxim Levitsky
2019-09-12 9:17 ` [Qemu-devel] [PATCH 02/12] qcrypto-luks: rename some fields in QCryptoBlockLUKSHeader Maxim Levitsky
` (10 subsequent siblings)
11 siblings, 0 replies; 18+ messages in thread
From: Maxim Levitsky @ 2019-09-12 9:16 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Daniel P. Berrangé,
qemu-block, John Snow, Markus Armbruster, Max Reitz,
Maxim Levitsky
* rename the write_func to create_write_func,
and init_func to create_init_func
this is preparation for other write_func that will
be used to update the encryption keys.
No functional changes
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
block/crypto.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/block/crypto.c b/block/crypto.c
index 7eb698774e..6e822c6e50 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -78,7 +78,7 @@ struct BlockCryptoCreateData {
};
-static ssize_t block_crypto_write_func(QCryptoBlock *block,
+static ssize_t block_crypto_create_write_func(QCryptoBlock *block,
size_t offset,
const uint8_t *buf,
size_t buflen,
@@ -96,8 +96,7 @@ static ssize_t block_crypto_write_func(QCryptoBlock *block,
return ret;
}
-
-static ssize_t block_crypto_init_func(QCryptoBlock *block,
+static ssize_t block_crypto_create_init_func(QCryptoBlock *block,
size_t headerlen,
void *opaque,
Error **errp)
@@ -109,7 +108,8 @@ static ssize_t block_crypto_init_func(QCryptoBlock *block,
return -EFBIG;
}
- /* User provided size should reflect amount of space made
+ /*
+ * User provided size should reflect amount of space made
* available to the guest, so we must take account of that
* which will be used by the crypto header
*/
@@ -279,8 +279,8 @@ static int block_crypto_co_create_generic(BlockDriverState *bs,
};
crypto = qcrypto_block_create(opts, NULL,
- block_crypto_init_func,
- block_crypto_write_func,
+ block_crypto_create_init_func,
+ block_crypto_create_write_func,
&data,
errp);
--
2.17.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH 02/12] qcrypto-luks: rename some fields in QCryptoBlockLUKSHeader
2019-09-12 9:16 [Qemu-devel] [PATCH 00/12] crypto/luks: preparation for encryption key managment Maxim Levitsky
2019-09-12 9:16 ` [Qemu-devel] [PATCH 01/12] block-crypto: misc refactoring Maxim Levitsky
@ 2019-09-12 9:17 ` Maxim Levitsky
2019-09-12 9:17 ` [Qemu-devel] [PATCH 03/12] qcrypto-luks: don't overwrite cipher_mode in header Maxim Levitsky
` (9 subsequent siblings)
11 siblings, 0 replies; 18+ messages in thread
From: Maxim Levitsky @ 2019-09-12 9:17 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Daniel P. Berrangé,
qemu-block, John Snow, Markus Armbruster, Max Reitz,
Maxim Levitsky
* key_bytes -> master_key_len
* payload_offset = payload_offset_sector (to emphasise that this isn't byte offset)
* key_offset -> key_offset_sector - same as above for luks slots
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
crypto/block-luks.c | 91 +++++++++++++++++++++++----------------------
1 file changed, 47 insertions(+), 44 deletions(-)
diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index 743949adbf..f12fa2d270 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -143,7 +143,7 @@ struct QCryptoBlockLUKSKeySlot {
/* salt for PBKDF2 */
uint8_t salt[QCRYPTO_BLOCK_LUKS_SALT_LEN];
/* start sector of key material */
- uint32_t key_offset;
+ uint32_t key_offset_sector;
/* number of anti-forensic stripes */
uint32_t stripes;
};
@@ -172,10 +172,10 @@ struct QCryptoBlockLUKSHeader {
char hash_spec[QCRYPTO_BLOCK_LUKS_HASH_SPEC_LEN];
/* start offset of the volume data (in 512 byte sectors) */
- uint32_t payload_offset;
+ uint32_t payload_offset_sector;
/* Number of key bytes */
- uint32_t key_bytes;
+ uint32_t master_key_len;
/* master key checksum after PBKDF2 */
uint8_t master_key_digest[QCRYPTO_BLOCK_LUKS_DIGEST_LEN];
@@ -466,7 +466,7 @@ qcrypto_block_luks_load_key(QCryptoBlock *block,
* then encrypted.
*/
rv = readfunc(block,
- slot->key_offset * QCRYPTO_BLOCK_LUKS_SECTOR_SIZE,
+ slot->key_offset_sector * QCRYPTO_BLOCK_LUKS_SECTOR_SIZE,
splitkey, splitkeylen,
opaque,
errp);
@@ -584,8 +584,8 @@ qcrypto_block_luks_find_key(QCryptoBlock *block,
size_t i;
int rv;
- *masterkey = g_new0(uint8_t, luks->header.key_bytes);
- *masterkeylen = luks->header.key_bytes;
+ *masterkey = g_new0(uint8_t, luks->header.master_key_len);
+ *masterkeylen = luks->header.master_key_len;
for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
rv = qcrypto_block_luks_load_key(block,
@@ -677,14 +677,14 @@ qcrypto_block_luks_open(QCryptoBlock *block,
/* The header is always stored in big-endian format, so
* convert everything to native */
be16_to_cpus(&luks->header.version);
- be32_to_cpus(&luks->header.payload_offset);
- be32_to_cpus(&luks->header.key_bytes);
+ be32_to_cpus(&luks->header.payload_offset_sector);
+ be32_to_cpus(&luks->header.master_key_len);
be32_to_cpus(&luks->header.master_key_iterations);
for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
be32_to_cpus(&luks->header.key_slots[i].active);
be32_to_cpus(&luks->header.key_slots[i].iterations);
- be32_to_cpus(&luks->header.key_slots[i].key_offset);
+ be32_to_cpus(&luks->header.key_slots[i].key_offset_sector);
be32_to_cpus(&luks->header.key_slots[i].stripes);
}
@@ -743,10 +743,11 @@ qcrypto_block_luks_open(QCryptoBlock *block,
goto fail;
}
- cipheralg = qcrypto_block_luks_cipher_name_lookup(luks->header.cipher_name,
- ciphermode,
- luks->header.key_bytes,
- &local_err);
+ cipheralg =
+ qcrypto_block_luks_cipher_name_lookup(luks->header.cipher_name,
+ ciphermode,
+ luks->header.master_key_len,
+ &local_err);
if (local_err) {
ret = -ENOTSUP;
error_propagate(errp, local_err);
@@ -838,7 +839,7 @@ qcrypto_block_luks_open(QCryptoBlock *block,
}
block->sector_size = QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
- block->payload_offset = luks->header.payload_offset *
+ block->payload_offset = luks->header.payload_offset_sector *
block->sector_size;
luks->cipher_alg = cipheralg;
@@ -993,9 +994,11 @@ qcrypto_block_luks_create(QCryptoBlock *block,
strcpy(luks->header.cipher_mode, cipher_mode_spec);
strcpy(luks->header.hash_spec, hash_alg);
- luks->header.key_bytes = qcrypto_cipher_get_key_len(luks_opts.cipher_alg);
+ luks->header.master_key_len =
+ qcrypto_cipher_get_key_len(luks_opts.cipher_alg);
+
if (luks_opts.cipher_mode == QCRYPTO_CIPHER_MODE_XTS) {
- luks->header.key_bytes *= 2;
+ luks->header.master_key_len *= 2;
}
/* Generate the salt used for hashing the master key
@@ -1008,9 +1011,9 @@ qcrypto_block_luks_create(QCryptoBlock *block,
}
/* Generate random master key */
- masterkey = g_new0(uint8_t, luks->header.key_bytes);
+ masterkey = g_new0(uint8_t, luks->header.master_key_len);
if (qcrypto_random_bytes(masterkey,
- luks->header.key_bytes, errp) < 0) {
+ luks->header.master_key_len, errp) < 0) {
goto error;
}
@@ -1018,7 +1021,7 @@ qcrypto_block_luks_create(QCryptoBlock *block,
/* Setup the block device payload encryption objects */
if (qcrypto_block_init_cipher(block, luks_opts.cipher_alg,
luks_opts.cipher_mode, masterkey,
- luks->header.key_bytes, 1, errp) < 0) {
+ luks->header.master_key_len, 1, errp) < 0) {
goto error;
}
@@ -1028,7 +1031,7 @@ qcrypto_block_luks_create(QCryptoBlock *block,
block->ivgen = qcrypto_ivgen_new(luks_opts.ivgen_alg,
ivcipheralg,
luks_opts.ivgen_hash_alg,
- masterkey, luks->header.key_bytes,
+ masterkey, luks->header.master_key_len,
errp);
if (!block->ivgen) {
@@ -1040,7 +1043,7 @@ qcrypto_block_luks_create(QCryptoBlock *block,
* key, in order to have 1 second of compute time used
*/
iters = qcrypto_pbkdf2_count_iters(luks_opts.hash_alg,
- masterkey, luks->header.key_bytes,
+ masterkey, luks->header.master_key_len,
luks->header.master_key_salt,
QCRYPTO_BLOCK_LUKS_SALT_LEN,
QCRYPTO_BLOCK_LUKS_DIGEST_LEN,
@@ -1080,7 +1083,7 @@ qcrypto_block_luks_create(QCryptoBlock *block,
* valid master key
*/
if (qcrypto_pbkdf2(luks_opts.hash_alg,
- masterkey, luks->header.key_bytes,
+ masterkey, luks->header.master_key_len,
luks->header.master_key_salt,
QCRYPTO_BLOCK_LUKS_SALT_LEN,
luks->header.master_key_iterations,
@@ -1093,7 +1096,7 @@ qcrypto_block_luks_create(QCryptoBlock *block,
/* Although LUKS has multiple key slots, we're just going
* to use the first key slot */
- splitkeylen = luks->header.key_bytes * QCRYPTO_BLOCK_LUKS_STRIPES;
+ splitkeylen = luks->header.master_key_len * QCRYPTO_BLOCK_LUKS_STRIPES;
for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
luks->header.key_slots[i].active = i == 0 ?
QCRYPTO_BLOCK_LUKS_KEY_SLOT_ENABLED :
@@ -1103,7 +1106,7 @@ qcrypto_block_luks_create(QCryptoBlock *block,
/* This calculation doesn't match that shown in the spec,
* but instead follows the cryptsetup implementation.
*/
- luks->header.key_slots[i].key_offset =
+ luks->header.key_slots[i].key_offset_sector =
(QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET /
QCRYPTO_BLOCK_LUKS_SECTOR_SIZE) +
(ROUND_UP(DIV_ROUND_UP(splitkeylen, QCRYPTO_BLOCK_LUKS_SECTOR_SIZE),
@@ -1124,7 +1127,7 @@ qcrypto_block_luks_create(QCryptoBlock *block,
(uint8_t *)password, strlen(password),
luks->header.key_slots[0].salt,
QCRYPTO_BLOCK_LUKS_SALT_LEN,
- luks->header.key_bytes,
+ luks->header.master_key_len,
&local_err);
if (local_err) {
error_propagate(errp, local_err);
@@ -1155,13 +1158,13 @@ qcrypto_block_luks_create(QCryptoBlock *block,
/* Generate a key that we'll use to encrypt the master
* key, from the user's password
*/
- slotkey = g_new0(uint8_t, luks->header.key_bytes);
+ slotkey = g_new0(uint8_t, luks->header.master_key_len);
if (qcrypto_pbkdf2(luks_opts.hash_alg,
(uint8_t *)password, strlen(password),
luks->header.key_slots[0].salt,
QCRYPTO_BLOCK_LUKS_SALT_LEN,
luks->header.key_slots[0].iterations,
- slotkey, luks->header.key_bytes,
+ slotkey, luks->header.master_key_len,
errp) < 0) {
goto error;
}
@@ -1172,7 +1175,7 @@ qcrypto_block_luks_create(QCryptoBlock *block,
*/
cipher = qcrypto_cipher_new(luks_opts.cipher_alg,
luks_opts.cipher_mode,
- slotkey, luks->header.key_bytes,
+ slotkey, luks->header.master_key_len,
errp);
if (!cipher) {
goto error;
@@ -1181,7 +1184,7 @@ qcrypto_block_luks_create(QCryptoBlock *block,
ivgen = qcrypto_ivgen_new(luks_opts.ivgen_alg,
ivcipheralg,
luks_opts.ivgen_hash_alg,
- slotkey, luks->header.key_bytes,
+ slotkey, luks->header.master_key_len,
errp);
if (!ivgen) {
goto error;
@@ -1193,7 +1196,7 @@ qcrypto_block_luks_create(QCryptoBlock *block,
splitkey = g_new0(uint8_t, splitkeylen);
if (qcrypto_afsplit_encode(luks_opts.hash_alg,
- luks->header.key_bytes,
+ luks->header.master_key_len,
luks->header.key_slots[0].stripes,
masterkey,
splitkey,
@@ -1217,7 +1220,7 @@ qcrypto_block_luks_create(QCryptoBlock *block,
* slot headers, rounded up to the nearest sector, combined with
* the size of each master key material region, also rounded up
* to the nearest sector */
- luks->header.payload_offset =
+ luks->header.payload_offset_sector =
(QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET /
QCRYPTO_BLOCK_LUKS_SECTOR_SIZE) +
(ROUND_UP(DIV_ROUND_UP(splitkeylen, QCRYPTO_BLOCK_LUKS_SECTOR_SIZE),
@@ -1226,7 +1229,7 @@ qcrypto_block_luks_create(QCryptoBlock *block,
QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS);
block->sector_size = QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
- block->payload_offset = luks->header.payload_offset *
+ block->payload_offset = luks->header.payload_offset_sector *
block->sector_size;
/* Reserve header space to match payload offset */
@@ -1239,14 +1242,14 @@ qcrypto_block_luks_create(QCryptoBlock *block,
/* Everything on disk uses Big Endian, so flip header fields
* before writing them */
cpu_to_be16s(&luks->header.version);
- cpu_to_be32s(&luks->header.payload_offset);
- cpu_to_be32s(&luks->header.key_bytes);
+ cpu_to_be32s(&luks->header.payload_offset_sector);
+ cpu_to_be32s(&luks->header.master_key_len);
cpu_to_be32s(&luks->header.master_key_iterations);
for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
cpu_to_be32s(&luks->header.key_slots[i].active);
cpu_to_be32s(&luks->header.key_slots[i].iterations);
- cpu_to_be32s(&luks->header.key_slots[i].key_offset);
+ cpu_to_be32s(&luks->header.key_slots[i].key_offset_sector);
cpu_to_be32s(&luks->header.key_slots[i].stripes);
}
@@ -1263,14 +1266,14 @@ qcrypto_block_luks_create(QCryptoBlock *block,
/* Byte swap the header back to native, in case we need
* to read it again later */
be16_to_cpus(&luks->header.version);
- be32_to_cpus(&luks->header.payload_offset);
- be32_to_cpus(&luks->header.key_bytes);
+ be32_to_cpus(&luks->header.payload_offset_sector);
+ be32_to_cpus(&luks->header.master_key_len);
be32_to_cpus(&luks->header.master_key_iterations);
for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
be32_to_cpus(&luks->header.key_slots[i].active);
be32_to_cpus(&luks->header.key_slots[i].iterations);
- be32_to_cpus(&luks->header.key_slots[i].key_offset);
+ be32_to_cpus(&luks->header.key_slots[i].key_offset_sector);
be32_to_cpus(&luks->header.key_slots[i].stripes);
}
@@ -1282,7 +1285,7 @@ qcrypto_block_luks_create(QCryptoBlock *block,
/* Write out the master key material, starting at the
* sector immediately following the partition header. */
if (writefunc(block,
- luks->header.key_slots[0].key_offset *
+ luks->header.key_slots[0].key_offset_sector *
QCRYPTO_BLOCK_LUKS_SECTOR_SIZE,
splitkey, splitkeylen,
opaque,
@@ -1296,17 +1299,17 @@ qcrypto_block_luks_create(QCryptoBlock *block,
luks->ivgen_hash_alg = luks_opts.ivgen_hash_alg;
luks->hash_alg = luks_opts.hash_alg;
- memset(masterkey, 0, luks->header.key_bytes);
- memset(slotkey, 0, luks->header.key_bytes);
+ memset(masterkey, 0, luks->header.master_key_len);
+ memset(slotkey, 0, luks->header.master_key_len);
return 0;
error:
if (masterkey) {
- memset(masterkey, 0, luks->header.key_bytes);
+ memset(masterkey, 0, luks->header.master_key_len);
}
if (slotkey) {
- memset(slotkey, 0, luks->header.key_bytes);
+ memset(slotkey, 0, luks->header.master_key_len);
}
qcrypto_block_free_cipher(block);
@@ -1346,7 +1349,7 @@ static int qcrypto_block_luks_get_info(QCryptoBlock *block,
slots->value = slot = g_new0(QCryptoBlockInfoLUKSSlot, 1);
slot->active = luks->header.key_slots[i].active ==
QCRYPTO_BLOCK_LUKS_KEY_SLOT_ENABLED;
- slot->key_offset = luks->header.key_slots[i].key_offset
+ slot->key_offset = luks->header.key_slots[i].key_offset_sector
* QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
if (slot->active) {
slot->has_iters = true;
--
2.17.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH 03/12] qcrypto-luks: don't overwrite cipher_mode in header
2019-09-12 9:16 [Qemu-devel] [PATCH 00/12] crypto/luks: preparation for encryption key managment Maxim Levitsky
2019-09-12 9:16 ` [Qemu-devel] [PATCH 01/12] block-crypto: misc refactoring Maxim Levitsky
2019-09-12 9:17 ` [Qemu-devel] [PATCH 02/12] qcrypto-luks: rename some fields in QCryptoBlockLUKSHeader Maxim Levitsky
@ 2019-09-12 9:17 ` Maxim Levitsky
2019-09-12 9:17 ` [Qemu-devel] [PATCH 04/12] qcrypto-luks: simplify masterkey and masterkey length Maxim Levitsky
` (8 subsequent siblings)
11 siblings, 0 replies; 18+ messages in thread
From: Maxim Levitsky @ 2019-09-12 9:17 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Daniel P. Berrangé,
qemu-block, John Snow, Markus Armbruster, Max Reitz,
Maxim Levitsky
This way we can store the header we loaded, which
will be used in key management code
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
crypto/block-luks.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index f12fa2d270..25f8a9f1c4 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -645,6 +645,7 @@ qcrypto_block_luks_open(QCryptoBlock *block,
QCryptoHashAlgorithm hash;
QCryptoHashAlgorithm ivhash;
g_autofree char *password = NULL;
+ g_autofree char *cipher_mode = NULL;
if (!(flags & QCRYPTO_BLOCK_OPEN_NO_IO)) {
if (!options->u.luks.key_secret) {
@@ -701,6 +702,8 @@ qcrypto_block_luks_open(QCryptoBlock *block,
goto fail;
}
+ cipher_mode = g_strdup(luks->header.cipher_mode);
+
/*
* The cipher_mode header contains a string that we have
* to further parse, of the format
@@ -709,11 +712,11 @@ qcrypto_block_luks_open(QCryptoBlock *block,
*
* eg cbc-essiv:sha256, cbc-plain64
*/
- ivgen_name = strchr(luks->header.cipher_mode, '-');
+ ivgen_name = strchr(cipher_mode, '-');
if (!ivgen_name) {
ret = -EINVAL;
error_setg(errp, "Unexpected cipher mode string format %s",
- luks->header.cipher_mode);
+ cipher_mode);
goto fail;
}
*ivgen_name = '\0';
@@ -735,7 +738,7 @@ qcrypto_block_luks_open(QCryptoBlock *block,
}
}
- ciphermode = qcrypto_block_luks_cipher_mode_lookup(luks->header.cipher_mode,
+ ciphermode = qcrypto_block_luks_cipher_mode_lookup(cipher_mode,
&local_err);
if (local_err) {
ret = -ENOTSUP;
--
2.17.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH 04/12] qcrypto-luks: simplify masterkey and masterkey length
2019-09-12 9:16 [Qemu-devel] [PATCH 00/12] crypto/luks: preparation for encryption key managment Maxim Levitsky
` (2 preceding siblings ...)
2019-09-12 9:17 ` [Qemu-devel] [PATCH 03/12] qcrypto-luks: don't overwrite cipher_mode in header Maxim Levitsky
@ 2019-09-12 9:17 ` Maxim Levitsky
2019-09-12 9:17 ` [Qemu-devel] [PATCH 05/12] qcrypto-luks: pass keyslot index rather that pointer to the keyslot Maxim Levitsky
` (7 subsequent siblings)
11 siblings, 0 replies; 18+ messages in thread
From: Maxim Levitsky @ 2019-09-12 9:17 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Daniel P. Berrangé,
qemu-block, John Snow, Markus Armbruster, Max Reitz,
Maxim Levitsky
Let the caller allocate masterkey
Always use master key len from the header
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
crypto/block-luks.c | 44 +++++++++++++++++++++-----------------------
1 file changed, 21 insertions(+), 23 deletions(-)
diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index 25f8a9f1c4..9e59a791a6 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -419,7 +419,6 @@ qcrypto_block_luks_load_key(QCryptoBlock *block,
QCryptoCipherAlgorithm ivcipheralg,
QCryptoHashAlgorithm ivhash,
uint8_t *masterkey,
- size_t masterkeylen,
QCryptoBlockReadFunc readfunc,
void *opaque,
Error **errp)
@@ -438,9 +437,9 @@ qcrypto_block_luks_load_key(QCryptoBlock *block,
return 0;
}
- splitkeylen = masterkeylen * slot->stripes;
+ splitkeylen = luks->header.master_key_len * slot->stripes;
splitkey = g_new0(uint8_t, splitkeylen);
- possiblekey = g_new0(uint8_t, masterkeylen);
+ possiblekey = g_new0(uint8_t, luks->header.master_key_len);
/*
* The user password is used to generate a (possible)
@@ -453,7 +452,7 @@ qcrypto_block_luks_load_key(QCryptoBlock *block,
(const uint8_t *)password, strlen(password),
slot->salt, QCRYPTO_BLOCK_LUKS_SALT_LEN,
slot->iterations,
- possiblekey, masterkeylen,
+ possiblekey, luks->header.master_key_len,
errp) < 0) {
return -1;
}
@@ -478,7 +477,7 @@ qcrypto_block_luks_load_key(QCryptoBlock *block,
/* Setup the cipher/ivgen that we'll use to try to decrypt
* the split master key material */
cipher = qcrypto_cipher_new(cipheralg, ciphermode,
- possiblekey, masterkeylen,
+ possiblekey, luks->header.master_key_len,
errp);
if (!cipher) {
return -1;
@@ -489,7 +488,7 @@ qcrypto_block_luks_load_key(QCryptoBlock *block,
ivgen = qcrypto_ivgen_new(ivalg,
ivcipheralg,
ivhash,
- possiblekey, masterkeylen,
+ possiblekey, luks->header.master_key_len,
errp);
if (!ivgen) {
return -1;
@@ -519,7 +518,7 @@ qcrypto_block_luks_load_key(QCryptoBlock *block,
* it back together to get the actual master key.
*/
if (qcrypto_afsplit_decode(hash,
- masterkeylen,
+ luks->header.master_key_len,
slot->stripes,
splitkey,
masterkey,
@@ -537,11 +536,13 @@ qcrypto_block_luks_load_key(QCryptoBlock *block,
* header
*/
if (qcrypto_pbkdf2(hash,
- masterkey, masterkeylen,
+ masterkey,
+ luks->header.master_key_len,
luks->header.master_key_salt,
QCRYPTO_BLOCK_LUKS_SALT_LEN,
luks->header.master_key_iterations,
- keydigest, G_N_ELEMENTS(keydigest),
+ keydigest,
+ G_N_ELEMENTS(keydigest),
errp) < 0) {
return -1;
}
@@ -574,8 +575,7 @@ qcrypto_block_luks_find_key(QCryptoBlock *block,
QCryptoIVGenAlgorithm ivalg,
QCryptoCipherAlgorithm ivcipheralg,
QCryptoHashAlgorithm ivhash,
- uint8_t **masterkey,
- size_t *masterkeylen,
+ uint8_t *masterkey,
QCryptoBlockReadFunc readfunc,
void *opaque,
Error **errp)
@@ -584,9 +584,6 @@ qcrypto_block_luks_find_key(QCryptoBlock *block,
size_t i;
int rv;
- *masterkey = g_new0(uint8_t, luks->header.master_key_len);
- *masterkeylen = luks->header.master_key_len;
-
for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
rv = qcrypto_block_luks_load_key(block,
&luks->header.key_slots[i],
@@ -597,8 +594,7 @@ qcrypto_block_luks_find_key(QCryptoBlock *block,
ivalg,
ivcipheralg,
ivhash,
- *masterkey,
- *masterkeylen,
+ masterkey,
readfunc,
opaque,
errp);
@@ -613,9 +609,6 @@ qcrypto_block_luks_find_key(QCryptoBlock *block,
error_setg(errp, "Invalid password, cannot unlock any keyslot");
error:
- g_free(*masterkey);
- *masterkey = NULL;
- *masterkeylen = 0;
return -1;
}
@@ -636,7 +629,6 @@ qcrypto_block_luks_open(QCryptoBlock *block,
size_t i;
ssize_t rv;
g_autofree uint8_t *masterkey = NULL;
- size_t masterkeylen;
char *ivgen_name, *ivhash_name;
QCryptoCipherMode ciphermode;
QCryptoCipherAlgorithm cipheralg;
@@ -802,6 +794,9 @@ qcrypto_block_luks_open(QCryptoBlock *block,
/* Try to find which key slot our password is valid for
* and unlock the master key from that slot.
*/
+
+ masterkey = g_new0(uint8_t, luks->header.master_key_len);
+
if (qcrypto_block_luks_find_key(block,
password,
cipheralg, ciphermode,
@@ -809,7 +804,7 @@ qcrypto_block_luks_open(QCryptoBlock *block,
ivalg,
ivcipheralg,
ivhash,
- &masterkey, &masterkeylen,
+ masterkey,
readfunc, opaque,
errp) < 0) {
ret = -EACCES;
@@ -825,7 +820,8 @@ qcrypto_block_luks_open(QCryptoBlock *block,
block->ivgen = qcrypto_ivgen_new(ivalg,
ivcipheralg,
ivhash,
- masterkey, masterkeylen,
+ masterkey,
+ luks->header.master_key_len,
errp);
if (!block->ivgen) {
ret = -ENOTSUP;
@@ -833,7 +829,9 @@ qcrypto_block_luks_open(QCryptoBlock *block,
}
ret = qcrypto_block_init_cipher(block, cipheralg, ciphermode,
- masterkey, masterkeylen, n_threads,
+ masterkey,
+ luks->header.master_key_len,
+ n_threads,
errp);
if (ret < 0) {
ret = -ENOTSUP;
--
2.17.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH 05/12] qcrypto-luks: pass keyslot index rather that pointer to the keyslot
2019-09-12 9:16 [Qemu-devel] [PATCH 00/12] crypto/luks: preparation for encryption key managment Maxim Levitsky
` (3 preceding siblings ...)
2019-09-12 9:17 ` [Qemu-devel] [PATCH 04/12] qcrypto-luks: simplify masterkey and masterkey length Maxim Levitsky
@ 2019-09-12 9:17 ` Maxim Levitsky
2019-09-12 9:17 ` [Qemu-devel] [PATCH 06/12] qcrypto-luks: use the parsed encryption settings in QCryptoBlockLUKS Maxim Levitsky
` (6 subsequent siblings)
11 siblings, 0 replies; 18+ messages in thread
From: Maxim Levitsky @ 2019-09-12 9:17 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Daniel P. Berrangé,
qemu-block, John Snow, Markus Armbruster, Max Reitz,
Maxim Levitsky
Another minor refactoring
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
crypto/block-luks.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index 9e59a791a6..b759cc8d19 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -410,7 +410,7 @@ qcrypto_block_luks_essiv_cipher(QCryptoCipherAlgorithm cipher,
*/
static int
qcrypto_block_luks_load_key(QCryptoBlock *block,
- QCryptoBlockLUKSKeySlot *slot,
+ size_t slot_idx,
const char *password,
QCryptoCipherAlgorithm cipheralg,
QCryptoCipherMode ciphermode,
@@ -424,6 +424,7 @@ qcrypto_block_luks_load_key(QCryptoBlock *block,
Error **errp)
{
QCryptoBlockLUKS *luks = block->opaque;
+ const QCryptoBlockLUKSKeySlot *slot = &luks->header.key_slots[slot_idx];
g_autofree uint8_t *splitkey = NULL;
size_t splitkeylen;
g_autofree uint8_t *possiblekey = NULL;
@@ -580,13 +581,12 @@ qcrypto_block_luks_find_key(QCryptoBlock *block,
void *opaque,
Error **errp)
{
- QCryptoBlockLUKS *luks = block->opaque;
size_t i;
int rv;
for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
rv = qcrypto_block_luks_load_key(block,
- &luks->header.key_slots[i],
+ i,
password,
cipheralg,
ciphermode,
--
2.17.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH 06/12] qcrypto-luks: use the parsed encryption settings in QCryptoBlockLUKS
2019-09-12 9:16 [Qemu-devel] [PATCH 00/12] crypto/luks: preparation for encryption key managment Maxim Levitsky
` (4 preceding siblings ...)
2019-09-12 9:17 ` [Qemu-devel] [PATCH 05/12] qcrypto-luks: pass keyslot index rather that pointer to the keyslot Maxim Levitsky
@ 2019-09-12 9:17 ` Maxim Levitsky
2019-09-12 9:17 ` [Qemu-devel] [PATCH 07/12] qcrypto-luks: purge unused error codes from open callback Maxim Levitsky
` (5 subsequent siblings)
11 siblings, 0 replies; 18+ messages in thread
From: Maxim Levitsky @ 2019-09-12 9:17 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Daniel P. Berrangé,
qemu-block, John Snow, Markus Armbruster, Max Reitz,
Maxim Levitsky
Prior to that patch, the parsed encryption settings
were already stored into the QCryptoBlockLUKS but not
used anywhere but in qcrypto_block_luks_get_info
Using them simplifies the code
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
crypto/block-luks.c | 169 +++++++++++++++++++++-----------------------
1 file changed, 79 insertions(+), 90 deletions(-)
diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index b759cc8d19..f3bfc921b2 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -199,13 +199,25 @@ QEMU_BUILD_BUG_ON(sizeof(struct QCryptoBlockLUKSHeader) != 592);
struct QCryptoBlockLUKS {
QCryptoBlockLUKSHeader header;
- /* Cache parsed versions of what's in header fields,
- * as we can't rely on QCryptoBlock.cipher being
- * non-NULL */
+ /* Main encryption algorithm used for encryption*/
QCryptoCipherAlgorithm cipher_alg;
+
+ /* Mode of encryption for the selected encryption algorithm */
QCryptoCipherMode cipher_mode;
+
+ /* Initialization vector generation algorithm */
QCryptoIVGenAlgorithm ivgen_alg;
+
+ /* Hash algorithm used for IV generation*/
QCryptoHashAlgorithm ivgen_hash_alg;
+
+ /*
+ * Encryption algorithm used for IV generation.
+ * Usually the same as main encryption algorithm
+ */
+ QCryptoCipherAlgorithm ivgen_cipher_alg;
+
+ /* Hash algorithm used in pbkdf2 function */
QCryptoHashAlgorithm hash_alg;
};
@@ -412,12 +424,6 @@ static int
qcrypto_block_luks_load_key(QCryptoBlock *block,
size_t slot_idx,
const char *password,
- QCryptoCipherAlgorithm cipheralg,
- QCryptoCipherMode ciphermode,
- QCryptoHashAlgorithm hash,
- QCryptoIVGenAlgorithm ivalg,
- QCryptoCipherAlgorithm ivcipheralg,
- QCryptoHashAlgorithm ivhash,
uint8_t *masterkey,
QCryptoBlockReadFunc readfunc,
void *opaque,
@@ -449,7 +455,7 @@ qcrypto_block_luks_load_key(QCryptoBlock *block,
* the key is correct and validate the results of
* decryption later.
*/
- if (qcrypto_pbkdf2(hash,
+ if (qcrypto_pbkdf2(luks->hash_alg,
(const uint8_t *)password, strlen(password),
slot->salt, QCRYPTO_BLOCK_LUKS_SALT_LEN,
slot->iterations,
@@ -477,19 +483,23 @@ qcrypto_block_luks_load_key(QCryptoBlock *block,
/* Setup the cipher/ivgen that we'll use to try to decrypt
* the split master key material */
- cipher = qcrypto_cipher_new(cipheralg, ciphermode,
- possiblekey, luks->header.master_key_len,
+ cipher = qcrypto_cipher_new(luks->cipher_alg,
+ luks->cipher_mode,
+ possiblekey,
+ luks->header.master_key_len,
errp);
if (!cipher) {
return -1;
}
- niv = qcrypto_cipher_get_iv_len(cipheralg,
- ciphermode);
- ivgen = qcrypto_ivgen_new(ivalg,
- ivcipheralg,
- ivhash,
- possiblekey, luks->header.master_key_len,
+ niv = qcrypto_cipher_get_iv_len(luks->cipher_alg,
+ luks->cipher_mode);
+
+ ivgen = qcrypto_ivgen_new(luks->ivgen_alg,
+ luks->ivgen_cipher_alg,
+ luks->ivgen_hash_alg,
+ possiblekey,
+ luks->header.master_key_len,
errp);
if (!ivgen) {
return -1;
@@ -518,7 +528,7 @@ qcrypto_block_luks_load_key(QCryptoBlock *block,
* Now we've decrypted the split master key, join
* it back together to get the actual master key.
*/
- if (qcrypto_afsplit_decode(hash,
+ if (qcrypto_afsplit_decode(luks->hash_alg,
luks->header.master_key_len,
slot->stripes,
splitkey,
@@ -536,7 +546,7 @@ qcrypto_block_luks_load_key(QCryptoBlock *block,
* then comparing that to the hash stored in the key slot
* header
*/
- if (qcrypto_pbkdf2(hash,
+ if (qcrypto_pbkdf2(luks->hash_alg,
masterkey,
luks->header.master_key_len,
luks->header.master_key_salt,
@@ -570,12 +580,6 @@ qcrypto_block_luks_load_key(QCryptoBlock *block,
static int
qcrypto_block_luks_find_key(QCryptoBlock *block,
const char *password,
- QCryptoCipherAlgorithm cipheralg,
- QCryptoCipherMode ciphermode,
- QCryptoHashAlgorithm hash,
- QCryptoIVGenAlgorithm ivalg,
- QCryptoCipherAlgorithm ivcipheralg,
- QCryptoHashAlgorithm ivhash,
uint8_t *masterkey,
QCryptoBlockReadFunc readfunc,
void *opaque,
@@ -588,12 +592,6 @@ qcrypto_block_luks_find_key(QCryptoBlock *block,
rv = qcrypto_block_luks_load_key(block,
i,
password,
- cipheralg,
- ciphermode,
- hash,
- ivalg,
- ivcipheralg,
- ivhash,
masterkey,
readfunc,
opaque,
@@ -607,7 +605,6 @@ qcrypto_block_luks_find_key(QCryptoBlock *block,
}
error_setg(errp, "Invalid password, cannot unlock any keyslot");
-
error:
return -1;
}
@@ -623,19 +620,13 @@ qcrypto_block_luks_open(QCryptoBlock *block,
size_t n_threads,
Error **errp)
{
- QCryptoBlockLUKS *luks;
+ QCryptoBlockLUKS *luks = NULL;
Error *local_err = NULL;
int ret = 0;
size_t i;
ssize_t rv;
g_autofree uint8_t *masterkey = NULL;
char *ivgen_name, *ivhash_name;
- QCryptoCipherMode ciphermode;
- QCryptoCipherAlgorithm cipheralg;
- QCryptoIVGenAlgorithm ivalg;
- QCryptoCipherAlgorithm ivcipheralg;
- QCryptoHashAlgorithm hash;
- QCryptoHashAlgorithm ivhash;
g_autofree char *password = NULL;
g_autofree char *cipher_mode = NULL;
@@ -716,13 +707,13 @@ qcrypto_block_luks_open(QCryptoBlock *block,
ivhash_name = strchr(ivgen_name, ':');
if (!ivhash_name) {
- ivhash = 0;
+ luks->ivgen_hash_alg = 0;
} else {
*ivhash_name = '\0';
ivhash_name++;
- ivhash = qcrypto_block_luks_hash_name_lookup(ivhash_name,
- &local_err);
+ luks->ivgen_hash_alg = qcrypto_block_luks_hash_name_lookup(ivhash_name,
+ &local_err);
if (local_err) {
ret = -ENOTSUP;
error_propagate(errp, local_err);
@@ -730,17 +721,17 @@ qcrypto_block_luks_open(QCryptoBlock *block,
}
}
- ciphermode = qcrypto_block_luks_cipher_mode_lookup(cipher_mode,
- &local_err);
+ luks->cipher_mode = qcrypto_block_luks_cipher_mode_lookup(cipher_mode,
+ &local_err);
if (local_err) {
ret = -ENOTSUP;
error_propagate(errp, local_err);
goto fail;
}
- cipheralg =
+ luks->cipher_alg =
qcrypto_block_luks_cipher_name_lookup(luks->header.cipher_name,
- ciphermode,
+ luks->cipher_mode,
luks->header.master_key_len,
&local_err);
if (local_err) {
@@ -749,31 +740,33 @@ qcrypto_block_luks_open(QCryptoBlock *block,
goto fail;
}
- hash = qcrypto_block_luks_hash_name_lookup(luks->header.hash_spec,
- &local_err);
+ luks->hash_alg =
+ qcrypto_block_luks_hash_name_lookup(luks->header.hash_spec,
+ &local_err);
if (local_err) {
ret = -ENOTSUP;
error_propagate(errp, local_err);
goto fail;
}
- ivalg = qcrypto_block_luks_ivgen_name_lookup(ivgen_name,
- &local_err);
+ luks->ivgen_alg = qcrypto_block_luks_ivgen_name_lookup(ivgen_name,
+ &local_err);
if (local_err) {
ret = -ENOTSUP;
error_propagate(errp, local_err);
goto fail;
}
- if (ivalg == QCRYPTO_IVGEN_ALG_ESSIV) {
+ if (luks->ivgen_alg == QCRYPTO_IVGEN_ALG_ESSIV) {
if (!ivhash_name) {
ret = -EINVAL;
error_setg(errp, "Missing IV generator hash specification");
goto fail;
}
- ivcipheralg = qcrypto_block_luks_essiv_cipher(cipheralg,
- ivhash,
- &local_err);
+ luks->ivgen_cipher_alg =
+ qcrypto_block_luks_essiv_cipher(luks->cipher_alg,
+ luks->ivgen_hash_alg,
+ &local_err);
if (local_err) {
ret = -ENOTSUP;
error_propagate(errp, local_err);
@@ -787,7 +780,7 @@ qcrypto_block_luks_open(QCryptoBlock *block,
* ignore hash names with these ivgens rather than report
* an error about the invalid usage
*/
- ivcipheralg = cipheralg;
+ luks->ivgen_cipher_alg = luks->cipher_alg;
}
if (!(flags & QCRYPTO_BLOCK_OPEN_NO_IO)) {
@@ -799,11 +792,6 @@ qcrypto_block_luks_open(QCryptoBlock *block,
if (qcrypto_block_luks_find_key(block,
password,
- cipheralg, ciphermode,
- hash,
- ivalg,
- ivcipheralg,
- ivhash,
masterkey,
readfunc, opaque,
errp) < 0) {
@@ -814,12 +802,13 @@ qcrypto_block_luks_open(QCryptoBlock *block,
/* We have a valid master key now, so can setup the
* block device payload decryption objects
*/
- block->kdfhash = hash;
- block->niv = qcrypto_cipher_get_iv_len(cipheralg,
- ciphermode);
- block->ivgen = qcrypto_ivgen_new(ivalg,
- ivcipheralg,
- ivhash,
+ block->kdfhash = luks->hash_alg;
+ block->niv = qcrypto_cipher_get_iv_len(luks->cipher_alg,
+ luks->cipher_mode);
+
+ block->ivgen = qcrypto_ivgen_new(luks->ivgen_alg,
+ luks->ivgen_cipher_alg,
+ luks->ivgen_hash_alg,
masterkey,
luks->header.master_key_len,
errp);
@@ -828,7 +817,9 @@ qcrypto_block_luks_open(QCryptoBlock *block,
goto fail;
}
- ret = qcrypto_block_init_cipher(block, cipheralg, ciphermode,
+ ret = qcrypto_block_init_cipher(block,
+ luks->cipher_alg,
+ luks->cipher_mode,
masterkey,
luks->header.master_key_len,
n_threads,
@@ -843,11 +834,6 @@ qcrypto_block_luks_open(QCryptoBlock *block,
block->payload_offset = luks->header.payload_offset_sector *
block->sector_size;
- luks->cipher_alg = cipheralg;
- luks->cipher_mode = ciphermode;
- luks->ivgen_alg = ivalg;
- luks->ivgen_hash_alg = ivhash;
- luks->hash_alg = hash;
return 0;
@@ -893,7 +879,6 @@ qcrypto_block_luks_create(QCryptoBlock *block,
const char *ivgen_hash_alg = NULL;
const char *hash_alg;
g_autofree char *cipher_mode_spec = NULL;
- QCryptoCipherAlgorithm ivcipheralg = 0;
uint64_t iters;
memcpy(&luks_opts, &options->u.luks, sizeof(luks_opts));
@@ -918,6 +903,17 @@ qcrypto_block_luks_create(QCryptoBlock *block,
luks_opts.has_ivgen_hash_alg = true;
}
}
+
+ luks = g_new0(QCryptoBlockLUKS, 1);
+ block->opaque = luks;
+
+ luks->cipher_alg = luks_opts.cipher_alg;
+ luks->cipher_mode = luks_opts.cipher_mode;
+ luks->ivgen_alg = luks_opts.ivgen_alg;
+ luks->ivgen_hash_alg = luks_opts.ivgen_hash_alg;
+ luks->hash_alg = luks_opts.hash_alg;
+
+
/* Note we're allowing ivgen_hash_alg to be set even for
* non-essiv iv generators that don't need a hash. It will
* be silently ignored, for compatibility with dm-crypt */
@@ -925,15 +921,13 @@ qcrypto_block_luks_create(QCryptoBlock *block,
if (!options->u.luks.key_secret) {
error_setg(errp, "Parameter '%skey-secret' is required for cipher",
optprefix ? optprefix : "");
- return -1;
+ goto error;
}
password = qcrypto_secret_lookup_as_utf8(luks_opts.key_secret, errp);
if (!password) {
- return -1;
+ goto error;
}
- luks = g_new0(QCryptoBlockLUKS, 1);
- block->opaque = luks;
memcpy(luks->header.magic, qcrypto_block_luks_magic,
QCRYPTO_BLOCK_LUKS_MAGIC_LEN);
@@ -980,15 +974,16 @@ qcrypto_block_luks_create(QCryptoBlock *block,
}
if (luks_opts.ivgen_alg == QCRYPTO_IVGEN_ALG_ESSIV) {
- ivcipheralg = qcrypto_block_luks_essiv_cipher(luks_opts.cipher_alg,
- luks_opts.ivgen_hash_alg,
- &local_err);
+ luks->ivgen_cipher_alg =
+ qcrypto_block_luks_essiv_cipher(luks_opts.cipher_alg,
+ luks_opts.ivgen_hash_alg,
+ &local_err);
if (local_err) {
error_propagate(errp, local_err);
goto error;
}
} else {
- ivcipheralg = luks_opts.cipher_alg;
+ luks->ivgen_cipher_alg = luks_opts.cipher_alg;
}
strcpy(luks->header.cipher_name, cipher_alg);
@@ -1030,7 +1025,7 @@ qcrypto_block_luks_create(QCryptoBlock *block,
block->niv = qcrypto_cipher_get_iv_len(luks_opts.cipher_alg,
luks_opts.cipher_mode);
block->ivgen = qcrypto_ivgen_new(luks_opts.ivgen_alg,
- ivcipheralg,
+ luks->ivgen_cipher_alg,
luks_opts.ivgen_hash_alg,
masterkey, luks->header.master_key_len,
errp);
@@ -1183,7 +1178,7 @@ qcrypto_block_luks_create(QCryptoBlock *block,
}
ivgen = qcrypto_ivgen_new(luks_opts.ivgen_alg,
- ivcipheralg,
+ luks->ivgen_cipher_alg,
luks_opts.ivgen_hash_alg,
slotkey, luks->header.master_key_len,
errp);
@@ -1294,12 +1289,6 @@ qcrypto_block_luks_create(QCryptoBlock *block,
goto error;
}
- luks->cipher_alg = luks_opts.cipher_alg;
- luks->cipher_mode = luks_opts.cipher_mode;
- luks->ivgen_alg = luks_opts.ivgen_alg;
- luks->ivgen_hash_alg = luks_opts.ivgen_hash_alg;
- luks->hash_alg = luks_opts.hash_alg;
-
memset(masterkey, 0, luks->header.master_key_len);
memset(slotkey, 0, luks->header.master_key_len);
--
2.17.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH 07/12] qcrypto-luks: purge unused error codes from open callback
2019-09-12 9:16 [Qemu-devel] [PATCH 00/12] crypto/luks: preparation for encryption key managment Maxim Levitsky
` (5 preceding siblings ...)
2019-09-12 9:17 ` [Qemu-devel] [PATCH 06/12] qcrypto-luks: use the parsed encryption settings in QCryptoBlockLUKS Maxim Levitsky
@ 2019-09-12 9:17 ` Maxim Levitsky
2019-09-17 10:01 ` Daniel P. Berrangé
2019-09-12 9:17 ` [Qemu-devel] [PATCH 08/12] qcrypto-luks: extract store and load header Maxim Levitsky
` (4 subsequent siblings)
11 siblings, 1 reply; 18+ messages in thread
From: Maxim Levitsky @ 2019-09-12 9:17 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Daniel P. Berrangé,
qemu-block, John Snow, Markus Armbruster, Max Reitz,
Maxim Levitsky
These values are not used by generic crypto code anyway
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
crypto/block-luks.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index f3bfc921b2..ba63e9b442 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -675,13 +675,13 @@ qcrypto_block_luks_open(QCryptoBlock *block,
if (memcmp(luks->header.magic, qcrypto_block_luks_magic,
QCRYPTO_BLOCK_LUKS_MAGIC_LEN) != 0) {
error_setg(errp, "Volume is not in LUKS format");
- ret = -EINVAL;
+ ret = -1;
goto fail;
}
if (luks->header.version != QCRYPTO_BLOCK_LUKS_VERSION) {
error_setg(errp, "LUKS version %" PRIu32 " is not supported",
luks->header.version);
- ret = -ENOTSUP;
+ ret = -1;
goto fail;
}
@@ -697,7 +697,7 @@ qcrypto_block_luks_open(QCryptoBlock *block,
*/
ivgen_name = strchr(cipher_mode, '-');
if (!ivgen_name) {
- ret = -EINVAL;
+ ret = -1;
error_setg(errp, "Unexpected cipher mode string format %s",
cipher_mode);
goto fail;
@@ -715,7 +715,7 @@ qcrypto_block_luks_open(QCryptoBlock *block,
luks->ivgen_hash_alg = qcrypto_block_luks_hash_name_lookup(ivhash_name,
&local_err);
if (local_err) {
- ret = -ENOTSUP;
+ ret = -1;
error_propagate(errp, local_err);
goto fail;
}
@@ -724,7 +724,7 @@ qcrypto_block_luks_open(QCryptoBlock *block,
luks->cipher_mode = qcrypto_block_luks_cipher_mode_lookup(cipher_mode,
&local_err);
if (local_err) {
- ret = -ENOTSUP;
+ ret = -1;
error_propagate(errp, local_err);
goto fail;
}
@@ -735,7 +735,7 @@ qcrypto_block_luks_open(QCryptoBlock *block,
luks->header.master_key_len,
&local_err);
if (local_err) {
- ret = -ENOTSUP;
+ ret = -1;
error_propagate(errp, local_err);
goto fail;
}
@@ -744,7 +744,7 @@ qcrypto_block_luks_open(QCryptoBlock *block,
qcrypto_block_luks_hash_name_lookup(luks->header.hash_spec,
&local_err);
if (local_err) {
- ret = -ENOTSUP;
+ ret = -1;
error_propagate(errp, local_err);
goto fail;
}
@@ -752,14 +752,14 @@ qcrypto_block_luks_open(QCryptoBlock *block,
luks->ivgen_alg = qcrypto_block_luks_ivgen_name_lookup(ivgen_name,
&local_err);
if (local_err) {
- ret = -ENOTSUP;
+ ret = -1;
error_propagate(errp, local_err);
goto fail;
}
if (luks->ivgen_alg == QCRYPTO_IVGEN_ALG_ESSIV) {
if (!ivhash_name) {
- ret = -EINVAL;
+ ret = -1;
error_setg(errp, "Missing IV generator hash specification");
goto fail;
}
@@ -768,7 +768,7 @@ qcrypto_block_luks_open(QCryptoBlock *block,
luks->ivgen_hash_alg,
&local_err);
if (local_err) {
- ret = -ENOTSUP;
+ ret = -1;
error_propagate(errp, local_err);
goto fail;
}
@@ -795,7 +795,7 @@ qcrypto_block_luks_open(QCryptoBlock *block,
masterkey,
readfunc, opaque,
errp) < 0) {
- ret = -EACCES;
+ ret = -1;
goto fail;
}
@@ -813,7 +813,7 @@ qcrypto_block_luks_open(QCryptoBlock *block,
luks->header.master_key_len,
errp);
if (!block->ivgen) {
- ret = -ENOTSUP;
+ ret = -1;
goto fail;
}
@@ -825,7 +825,7 @@ qcrypto_block_luks_open(QCryptoBlock *block,
n_threads,
errp);
if (ret < 0) {
- ret = -ENOTSUP;
+ ret = -1;
goto fail;
}
}
--
2.17.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH 08/12] qcrypto-luks: extract store and load header
2019-09-12 9:16 [Qemu-devel] [PATCH 00/12] crypto/luks: preparation for encryption key managment Maxim Levitsky
` (6 preceding siblings ...)
2019-09-12 9:17 ` [Qemu-devel] [PATCH 07/12] qcrypto-luks: purge unused error codes from open callback Maxim Levitsky
@ 2019-09-12 9:17 ` Maxim Levitsky
2019-09-17 10:02 ` Daniel P. Berrangé
2019-09-12 9:17 ` [Qemu-devel] [PATCH 09/12] qcrypto-luks: extract check and parse header Maxim Levitsky
` (3 subsequent siblings)
11 siblings, 1 reply; 18+ messages in thread
From: Maxim Levitsky @ 2019-09-12 9:17 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Daniel P. Berrangé,
qemu-block, John Snow, Markus Armbruster, Max Reitz,
Maxim Levitsky
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
crypto/block-luks.c | 158 ++++++++++++++++++++++++++------------------
1 file changed, 94 insertions(+), 64 deletions(-)
diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index ba63e9b442..c3f3488222 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -409,6 +409,97 @@ qcrypto_block_luks_essiv_cipher(QCryptoCipherAlgorithm cipher,
}
}
+/*
+ * Stores the main LUKS header, taking care of endianess
+ */
+static int
+qcrypto_block_luks_store_header(QCryptoBlock *block,
+ QCryptoBlockWriteFunc writefunc,
+ void *opaque,
+ Error **errp)
+{
+ const QCryptoBlockLUKS *luks = block->opaque;
+ Error *local_err = NULL;
+ size_t i;
+ g_autofree QCryptoBlockLUKSHeader *hdr_copy = NULL;
+
+ /* Create a copy of the header */
+ hdr_copy = g_new0(QCryptoBlockLUKSHeader, 1);
+ memcpy(hdr_copy, &luks->header, sizeof(QCryptoBlockLUKSHeader));
+
+ /*
+ * Everything on disk uses Big Endian (tm), so flip header fields
+ * before writing them
+ */
+ cpu_to_be16s(&hdr_copy->version);
+ cpu_to_be32s(&hdr_copy->payload_offset_sector);
+ cpu_to_be32s(&hdr_copy->master_key_len);
+ cpu_to_be32s(&hdr_copy->master_key_iterations);
+
+ for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
+ cpu_to_be32s(&hdr_copy->key_slots[i].active);
+ cpu_to_be32s(&hdr_copy->key_slots[i].iterations);
+ cpu_to_be32s(&hdr_copy->key_slots[i].key_offset_sector);
+ cpu_to_be32s(&hdr_copy->key_slots[i].stripes);
+ }
+
+ /* Write out the partition header and key slot headers */
+ writefunc(block, 0, (const uint8_t *)hdr_copy, sizeof(*hdr_copy),
+ opaque, &local_err);
+
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return -1;
+ }
+ return 0;
+}
+
+/*
+ * Loads the main LUKS header,and byteswaps it to native endianess
+ * And run basic sanity checks on it
+ */
+static int
+qcrypto_block_luks_load_header(QCryptoBlock *block,
+ QCryptoBlockReadFunc readfunc,
+ void *opaque,
+ Error **errp)
+{
+ ssize_t rv;
+ size_t i;
+ QCryptoBlockLUKS *luks = block->opaque;
+
+ /*
+ * Read the entire LUKS header, minus the key material from
+ * the underlying device
+ */
+ rv = readfunc(block, 0,
+ (uint8_t *)&luks->header,
+ sizeof(luks->header),
+ opaque,
+ errp);
+ if (rv < 0) {
+ return rv;
+ }
+
+ /*
+ * The header is always stored in big-endian format, so
+ * convert everything to native
+ */
+ be16_to_cpus(&luks->header.version);
+ be32_to_cpus(&luks->header.payload_offset_sector);
+ be32_to_cpus(&luks->header.master_key_len);
+ be32_to_cpus(&luks->header.master_key_iterations);
+
+ for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
+ be32_to_cpus(&luks->header.key_slots[i].active);
+ be32_to_cpus(&luks->header.key_slots[i].iterations);
+ be32_to_cpus(&luks->header.key_slots[i].key_offset_sector);
+ be32_to_cpus(&luks->header.key_slots[i].stripes);
+ }
+
+ return 0;
+}
+
/*
* Given a key slot, and user password, this will attempt to unlock
* the master encryption key from the key slot.
@@ -623,8 +714,6 @@ qcrypto_block_luks_open(QCryptoBlock *block,
QCryptoBlockLUKS *luks = NULL;
Error *local_err = NULL;
int ret = 0;
- size_t i;
- ssize_t rv;
g_autofree uint8_t *masterkey = NULL;
char *ivgen_name, *ivhash_name;
g_autofree char *password = NULL;
@@ -646,31 +735,11 @@ qcrypto_block_luks_open(QCryptoBlock *block,
luks = g_new0(QCryptoBlockLUKS, 1);
block->opaque = luks;
- /* Read the entire LUKS header, minus the key material from
- * the underlying device */
- rv = readfunc(block, 0,
- (uint8_t *)&luks->header,
- sizeof(luks->header),
- opaque,
- errp);
- if (rv < 0) {
- ret = rv;
+ ret = qcrypto_block_luks_load_header(block, readfunc, opaque, errp);
+ if (ret < 0) {
goto fail;
}
- /* The header is always stored in big-endian format, so
- * convert everything to native */
- be16_to_cpus(&luks->header.version);
- be32_to_cpus(&luks->header.payload_offset_sector);
- be32_to_cpus(&luks->header.master_key_len);
- be32_to_cpus(&luks->header.master_key_iterations);
-
- for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
- be32_to_cpus(&luks->header.key_slots[i].active);
- be32_to_cpus(&luks->header.key_slots[i].iterations);
- be32_to_cpus(&luks->header.key_slots[i].key_offset_sector);
- be32_to_cpus(&luks->header.key_slots[i].stripes);
- }
if (memcmp(luks->header.magic, qcrypto_block_luks_magic,
QCRYPTO_BLOCK_LUKS_MAGIC_LEN) != 0) {
@@ -1235,46 +1304,7 @@ qcrypto_block_luks_create(QCryptoBlock *block,
goto error;
}
- /* Everything on disk uses Big Endian, so flip header fields
- * before writing them */
- cpu_to_be16s(&luks->header.version);
- cpu_to_be32s(&luks->header.payload_offset_sector);
- cpu_to_be32s(&luks->header.master_key_len);
- cpu_to_be32s(&luks->header.master_key_iterations);
-
- for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
- cpu_to_be32s(&luks->header.key_slots[i].active);
- cpu_to_be32s(&luks->header.key_slots[i].iterations);
- cpu_to_be32s(&luks->header.key_slots[i].key_offset_sector);
- cpu_to_be32s(&luks->header.key_slots[i].stripes);
- }
-
-
- /* Write out the partition header and key slot headers */
- writefunc(block, 0,
- (const uint8_t *)&luks->header,
- sizeof(luks->header),
- opaque,
- &local_err);
-
- /* Delay checking local_err until we've byte-swapped */
-
- /* Byte swap the header back to native, in case we need
- * to read it again later */
- be16_to_cpus(&luks->header.version);
- be32_to_cpus(&luks->header.payload_offset_sector);
- be32_to_cpus(&luks->header.master_key_len);
- be32_to_cpus(&luks->header.master_key_iterations);
-
- for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
- be32_to_cpus(&luks->header.key_slots[i].active);
- be32_to_cpus(&luks->header.key_slots[i].iterations);
- be32_to_cpus(&luks->header.key_slots[i].key_offset_sector);
- be32_to_cpus(&luks->header.key_slots[i].stripes);
- }
-
- if (local_err) {
- error_propagate(errp, local_err);
+ if (qcrypto_block_luks_store_header(block, writefunc, opaque, errp) < 0) {
goto error;
}
--
2.17.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH 09/12] qcrypto-luks: extract check and parse header
2019-09-12 9:16 [Qemu-devel] [PATCH 00/12] crypto/luks: preparation for encryption key managment Maxim Levitsky
` (7 preceding siblings ...)
2019-09-12 9:17 ` [Qemu-devel] [PATCH 08/12] qcrypto-luks: extract store and load header Maxim Levitsky
@ 2019-09-12 9:17 ` Maxim Levitsky
2019-09-17 10:04 ` Daniel P. Berrangé
2019-09-12 9:17 ` [Qemu-devel] [PATCH 10/12] qcrypto-luks: extract store key function Maxim Levitsky
` (2 subsequent siblings)
11 siblings, 1 reply; 18+ messages in thread
From: Maxim Levitsky @ 2019-09-12 9:17 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Daniel P. Berrangé,
qemu-block, John Snow, Markus Armbruster, Max Reitz,
Maxim Levitsky
This is just to make qcrypto_block_luks_open more
reasonable in size.
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
crypto/block-luks.c | 235 ++++++++++++++++++++++++--------------------
1 file changed, 127 insertions(+), 108 deletions(-)
diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index c3f3488222..24c1da3739 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -500,6 +500,129 @@ qcrypto_block_luks_load_header(QCryptoBlock *block,
return 0;
}
+/*
+ * Does basic sanity checks on the LUKS header
+ */
+static int
+qcrypto_block_luks_check_header(const QCryptoBlockLUKS *luks, Error **errp)
+{
+ if (memcmp(luks->header.magic, qcrypto_block_luks_magic,
+ QCRYPTO_BLOCK_LUKS_MAGIC_LEN) != 0) {
+ error_setg(errp, "Volume is not in LUKS format");
+ return -1;
+ }
+
+ if (luks->header.version != QCRYPTO_BLOCK_LUKS_VERSION) {
+ error_setg(errp, "LUKS version %" PRIu32 " is not supported",
+ luks->header.version);
+ return -1;
+ }
+ return 0;
+}
+
+/*
+ * Parses the crypto parameters that are stored in the LUKS header
+ */
+
+static int
+qcrypto_block_luks_parse_header(QCryptoBlockLUKS *luks, Error **errp)
+{
+ g_autofree char *cipher_mode = g_strdup(luks->header.cipher_mode);
+ char *ivgen_name, *ivhash_name;
+ Error *local_err = NULL;
+
+ /*
+ * The cipher_mode header contains a string that we have
+ * to further parse, of the format
+ *
+ * <cipher-mode>-<iv-generator>[:<iv-hash>]
+ *
+ * eg cbc-essiv:sha256, cbc-plain64
+ */
+ ivgen_name = strchr(cipher_mode, '-');
+ if (!ivgen_name) {
+ error_setg(errp, "Unexpected cipher mode string format %s",
+ luks->header.cipher_mode);
+ return -1;
+ }
+ *ivgen_name = '\0';
+ ivgen_name++;
+
+ ivhash_name = strchr(ivgen_name, ':');
+ if (!ivhash_name) {
+ luks->ivgen_hash_alg = 0;
+ } else {
+ *ivhash_name = '\0';
+ ivhash_name++;
+
+ luks->ivgen_hash_alg = qcrypto_block_luks_hash_name_lookup(ivhash_name,
+ &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return -1;
+ }
+ }
+
+ luks->cipher_mode = qcrypto_block_luks_cipher_mode_lookup(cipher_mode,
+ &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return -1;
+ }
+
+ luks->cipher_alg =
+ qcrypto_block_luks_cipher_name_lookup(luks->header.cipher_name,
+ luks->cipher_mode,
+ luks->header.master_key_len,
+ &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return -1;
+ }
+
+ luks->hash_alg =
+ qcrypto_block_luks_hash_name_lookup(luks->header.hash_spec,
+ &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return -1;
+ }
+
+ luks->ivgen_alg = qcrypto_block_luks_ivgen_name_lookup(ivgen_name,
+ &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return -1;
+ }
+
+ if (luks->ivgen_alg == QCRYPTO_IVGEN_ALG_ESSIV) {
+ if (!ivhash_name) {
+ error_setg(errp, "Missing IV generator hash specification");
+ return -1;
+ }
+ luks->ivgen_cipher_alg =
+ qcrypto_block_luks_essiv_cipher(luks->cipher_alg,
+ luks->ivgen_hash_alg,
+ &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return -1;
+ }
+ } else {
+
+ /*
+ * Note we parsed the ivhash_name earlier in the cipher_mode
+ * spec string even with plain/plain64 ivgens, but we
+ * will ignore it, since it is irrelevant for these ivgens.
+ * This is for compat with dm-crypt which will silently
+ * ignore hash names with these ivgens rather than report
+ * an error about the invalid usage
+ */
+ luks->ivgen_cipher_alg = luks->cipher_alg;
+ }
+ return 0;
+}
+
/*
* Given a key slot, and user password, this will attempt to unlock
* the master encryption key from the key slot.
@@ -712,12 +835,9 @@ qcrypto_block_luks_open(QCryptoBlock *block,
Error **errp)
{
QCryptoBlockLUKS *luks = NULL;
- Error *local_err = NULL;
int ret = 0;
g_autofree uint8_t *masterkey = NULL;
- char *ivgen_name, *ivhash_name;
g_autofree char *password = NULL;
- g_autofree char *cipher_mode = NULL;
if (!(flags & QCRYPTO_BLOCK_OPEN_NO_IO)) {
if (!options->u.luks.key_secret) {
@@ -741,117 +861,16 @@ qcrypto_block_luks_open(QCryptoBlock *block,
}
- if (memcmp(luks->header.magic, qcrypto_block_luks_magic,
- QCRYPTO_BLOCK_LUKS_MAGIC_LEN) != 0) {
- error_setg(errp, "Volume is not in LUKS format");
- ret = -1;
- goto fail;
- }
- if (luks->header.version != QCRYPTO_BLOCK_LUKS_VERSION) {
- error_setg(errp, "LUKS version %" PRIu32 " is not supported",
- luks->header.version);
- ret = -1;
- goto fail;
- }
-
- cipher_mode = g_strdup(luks->header.cipher_mode);
-
- /*
- * The cipher_mode header contains a string that we have
- * to further parse, of the format
- *
- * <cipher-mode>-<iv-generator>[:<iv-hash>]
- *
- * eg cbc-essiv:sha256, cbc-plain64
- */
- ivgen_name = strchr(cipher_mode, '-');
- if (!ivgen_name) {
- ret = -1;
- error_setg(errp, "Unexpected cipher mode string format %s",
- cipher_mode);
- goto fail;
- }
- *ivgen_name = '\0';
- ivgen_name++;
-
- ivhash_name = strchr(ivgen_name, ':');
- if (!ivhash_name) {
- luks->ivgen_hash_alg = 0;
- } else {
- *ivhash_name = '\0';
- ivhash_name++;
-
- luks->ivgen_hash_alg = qcrypto_block_luks_hash_name_lookup(ivhash_name,
- &local_err);
- if (local_err) {
- ret = -1;
- error_propagate(errp, local_err);
- goto fail;
- }
- }
-
- luks->cipher_mode = qcrypto_block_luks_cipher_mode_lookup(cipher_mode,
- &local_err);
- if (local_err) {
- ret = -1;
- error_propagate(errp, local_err);
- goto fail;
- }
-
- luks->cipher_alg =
- qcrypto_block_luks_cipher_name_lookup(luks->header.cipher_name,
- luks->cipher_mode,
- luks->header.master_key_len,
- &local_err);
- if (local_err) {
- ret = -1;
- error_propagate(errp, local_err);
- goto fail;
- }
-
- luks->hash_alg =
- qcrypto_block_luks_hash_name_lookup(luks->header.hash_spec,
- &local_err);
- if (local_err) {
- ret = -1;
- error_propagate(errp, local_err);
+ ret = qcrypto_block_luks_check_header(luks, errp);
+ if (ret < 0) {
goto fail;
}
- luks->ivgen_alg = qcrypto_block_luks_ivgen_name_lookup(ivgen_name,
- &local_err);
- if (local_err) {
- ret = -1;
- error_propagate(errp, local_err);
+ ret = qcrypto_block_luks_parse_header(luks, errp);
+ if (ret < 0) {
goto fail;
}
- if (luks->ivgen_alg == QCRYPTO_IVGEN_ALG_ESSIV) {
- if (!ivhash_name) {
- ret = -1;
- error_setg(errp, "Missing IV generator hash specification");
- goto fail;
- }
- luks->ivgen_cipher_alg =
- qcrypto_block_luks_essiv_cipher(luks->cipher_alg,
- luks->ivgen_hash_alg,
- &local_err);
- if (local_err) {
- ret = -1;
- error_propagate(errp, local_err);
- goto fail;
- }
- } else {
- /* Note we parsed the ivhash_name earlier in the cipher_mode
- * spec string even with plain/plain64 ivgens, but we
- * will ignore it, since it is irrelevant for these ivgens.
- * This is for compat with dm-crypt which will silently
- * ignore hash names with these ivgens rather than report
- * an error about the invalid usage
- */
- luks->ivgen_cipher_alg = luks->cipher_alg;
- }
-
if (!(flags & QCRYPTO_BLOCK_OPEN_NO_IO)) {
/* Try to find which key slot our password is valid for
* and unlock the master key from that slot.
--
2.17.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH 10/12] qcrypto-luks: extract store key function
2019-09-12 9:16 [Qemu-devel] [PATCH 00/12] crypto/luks: preparation for encryption key managment Maxim Levitsky
` (8 preceding siblings ...)
2019-09-12 9:17 ` [Qemu-devel] [PATCH 09/12] qcrypto-luks: extract check and parse header Maxim Levitsky
@ 2019-09-12 9:17 ` Maxim Levitsky
2019-09-17 10:15 ` Daniel P. Berrangé
2019-09-12 9:17 ` [Qemu-devel] [PATCH 11/12] qcrypto-luks: simplify the math used for keyslot locations Maxim Levitsky
2019-09-12 9:17 ` [Qemu-devel] [PATCH 12/12] qcrypto-luks: more rigorous header checking Maxim Levitsky
11 siblings, 1 reply; 18+ messages in thread
From: Maxim Levitsky @ 2019-09-12 9:17 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Daniel P. Berrangé,
qemu-block, John Snow, Markus Armbruster, Max Reitz,
Maxim Levitsky
This function will be used later to store
new keys to the luks metadata
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
crypto/block-luks.c | 304 ++++++++++++++++++++++++++------------------
1 file changed, 181 insertions(+), 123 deletions(-)
diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index 24c1da3739..c6045da33e 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -623,6 +623,176 @@ qcrypto_block_luks_parse_header(QCryptoBlockLUKS *luks, Error **errp)
return 0;
}
+/*
+ * Given a key slot, user password, and the master key,
+ * will store the encrypted master key there, and update the
+ * in-memory header. User must then write the in-memory header
+ *
+ * Returns:
+ * 0 if the keyslot was written successfully
+ * with the provided password
+ * -1 if a fatal error occurred while storing the key
+ */
+static int
+qcrypto_block_luks_store_key(QCryptoBlock *block,
+ unsigned int slot_idx,
+ const char *password,
+ uint8_t *masterkey,
+ uint64_t iter_time,
+ QCryptoBlockWriteFunc writefunc,
+ void *opaque,
+ Error **errp)
+{
+ QCryptoBlockLUKS *luks = block->opaque;
+ QCryptoBlockLUKSKeySlot *slot = &luks->header.key_slots[slot_idx];
+ g_autofree uint8_t *splitkey = NULL;
+ size_t splitkeylen;
+ g_autofree uint8_t *slotkey = NULL;
+ g_autoptr(QCryptoCipher) cipher = NULL;
+ g_autoptr(QCryptoIVGen) ivgen = NULL;
+ Error *local_err = NULL;
+ uint64_t iters;
+ int ret = -1;
+
+ if (qcrypto_random_bytes(slot->salt,
+ QCRYPTO_BLOCK_LUKS_SALT_LEN,
+ errp) < 0) {
+ goto cleanup;
+ }
+
+ splitkeylen = luks->header.master_key_len * slot->stripes;
+
+ /*
+ * Determine how many iterations are required to
+ * hash the user password while consuming 1 second of compute
+ * time
+ */
+ iters = qcrypto_pbkdf2_count_iters(luks->hash_alg,
+ (uint8_t *)password, strlen(password),
+ slot->salt,
+ QCRYPTO_BLOCK_LUKS_SALT_LEN,
+ luks->header.master_key_len,
+ &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ goto cleanup;
+ }
+
+ if (iters > (ULLONG_MAX / iter_time)) {
+ error_setg_errno(errp, ERANGE,
+ "PBKDF iterations %llu too large to scale",
+ (unsigned long long)iters);
+ goto cleanup;
+ }
+
+ /* iter_time was in millis, but count_iters reported for secs */
+ iters = iters * iter_time / 1000;
+
+ if (iters > UINT32_MAX) {
+ error_setg_errno(errp, ERANGE,
+ "PBKDF iterations %llu larger than %u",
+ (unsigned long long)iters, UINT32_MAX);
+ goto cleanup;
+ }
+
+ slot->iterations =
+ MAX(iters, QCRYPTO_BLOCK_LUKS_MIN_SLOT_KEY_ITERS);
+
+
+ /*
+ * Generate a key that we'll use to encrypt the master
+ * key, from the user's password
+ */
+ slotkey = g_new0(uint8_t, luks->header.master_key_len);
+ if (qcrypto_pbkdf2(luks->hash_alg,
+ (uint8_t *)password, strlen(password),
+ slot->salt,
+ QCRYPTO_BLOCK_LUKS_SALT_LEN,
+ slot->iterations,
+ slotkey, luks->header.master_key_len,
+ errp) < 0) {
+ goto cleanup;
+ }
+
+
+ /*
+ * Setup the encryption objects needed to encrypt the
+ * master key material
+ */
+ cipher = qcrypto_cipher_new(luks->cipher_alg,
+ luks->cipher_mode,
+ slotkey, luks->header.master_key_len,
+ errp);
+ if (!cipher) {
+ goto cleanup;
+ }
+
+ ivgen = qcrypto_ivgen_new(luks->ivgen_alg,
+ luks->ivgen_cipher_alg,
+ luks->ivgen_hash_alg,
+ slotkey, luks->header.master_key_len,
+ errp);
+ if (!ivgen) {
+ goto cleanup;
+ }
+
+ /*
+ * Before storing the master key, we need to vastly
+ * increase its size, as protection against forensic
+ * disk data recovery
+ */
+ splitkey = g_new0(uint8_t, splitkeylen);
+
+ if (qcrypto_afsplit_encode(luks->hash_alg,
+ luks->header.master_key_len,
+ slot->stripes,
+ masterkey,
+ splitkey,
+ errp) < 0) {
+ goto cleanup;
+ }
+
+ /*
+ * Now we encrypt the split master key with the key generated
+ * from the user's password, before storing it
+ */
+ if (qcrypto_block_cipher_encrypt_helper(cipher, block->niv, ivgen,
+ QCRYPTO_BLOCK_LUKS_SECTOR_SIZE,
+ 0,
+ splitkey,
+ splitkeylen,
+ errp) < 0) {
+ goto cleanup;
+ }
+
+ /* Write out the slot's master key material. */
+ if (writefunc(block,
+ slot->key_offset_sector *
+ QCRYPTO_BLOCK_LUKS_SECTOR_SIZE,
+ splitkey, splitkeylen,
+ opaque,
+ errp) != splitkeylen) {
+ goto cleanup;
+ }
+
+ slot->active = QCRYPTO_BLOCK_LUKS_KEY_SLOT_ENABLED;
+
+ if (qcrypto_block_luks_store_header(block, writefunc, opaque, errp) < 0) {
+ goto cleanup;
+ }
+
+ ret = 0;
+
+cleanup:
+ if (slotkey) {
+ memset(slotkey, 0, luks->header.master_key_len);
+ }
+ if (splitkey) {
+ memset(splitkey, 0, splitkeylen);
+ }
+ return ret;
+}
+
/*
* Given a key slot, and user password, this will attempt to unlock
* the master encryption key from the key slot.
@@ -954,12 +1124,8 @@ qcrypto_block_luks_create(QCryptoBlock *block,
QCryptoBlockCreateOptionsLUKS luks_opts;
Error *local_err = NULL;
g_autofree uint8_t *masterkey = NULL;
- g_autofree uint8_t *slotkey = NULL;
- g_autofree uint8_t *splitkey = NULL;
size_t splitkeylen = 0;
size_t i;
- g_autoptr(QCryptoCipher) cipher = NULL;
- g_autoptr(QCryptoIVGen) ivgen = NULL;
g_autofree char *password = NULL;
const char *cipher_alg;
const char *cipher_mode;
@@ -1182,9 +1348,7 @@ qcrypto_block_luks_create(QCryptoBlock *block,
* to use the first key slot */
splitkeylen = luks->header.master_key_len * QCRYPTO_BLOCK_LUKS_STRIPES;
for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
- luks->header.key_slots[i].active = i == 0 ?
- QCRYPTO_BLOCK_LUKS_KEY_SLOT_ENABLED :
- QCRYPTO_BLOCK_LUKS_KEY_SLOT_DISABLED;
+ luks->header.key_slots[i].active = QCRYPTO_BLOCK_LUKS_KEY_SLOT_DISABLED;
luks->header.key_slots[i].stripes = QCRYPTO_BLOCK_LUKS_STRIPES;
/* This calculation doesn't match that shown in the spec,
@@ -1198,107 +1362,6 @@ qcrypto_block_luks_create(QCryptoBlock *block,
QCRYPTO_BLOCK_LUKS_SECTOR_SIZE)) * i);
}
- if (qcrypto_random_bytes(luks->header.key_slots[0].salt,
- QCRYPTO_BLOCK_LUKS_SALT_LEN,
- errp) < 0) {
- goto error;
- }
-
- /* Again we determine how many iterations are required to
- * hash the user password while consuming 1 second of compute
- * time */
- iters = qcrypto_pbkdf2_count_iters(luks_opts.hash_alg,
- (uint8_t *)password, strlen(password),
- luks->header.key_slots[0].salt,
- QCRYPTO_BLOCK_LUKS_SALT_LEN,
- luks->header.master_key_len,
- &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
- goto error;
- }
-
- if (iters > (ULLONG_MAX / luks_opts.iter_time)) {
- error_setg_errno(errp, ERANGE,
- "PBKDF iterations %llu too large to scale",
- (unsigned long long)iters);
- goto error;
- }
-
- /* iter_time was in millis, but count_iters reported for secs */
- iters = iters * luks_opts.iter_time / 1000;
-
- if (iters > UINT32_MAX) {
- error_setg_errno(errp, ERANGE,
- "PBKDF iterations %llu larger than %u",
- (unsigned long long)iters, UINT32_MAX);
- goto error;
- }
-
- luks->header.key_slots[0].iterations =
- MAX(iters, QCRYPTO_BLOCK_LUKS_MIN_SLOT_KEY_ITERS);
-
-
- /* Generate a key that we'll use to encrypt the master
- * key, from the user's password
- */
- slotkey = g_new0(uint8_t, luks->header.master_key_len);
- if (qcrypto_pbkdf2(luks_opts.hash_alg,
- (uint8_t *)password, strlen(password),
- luks->header.key_slots[0].salt,
- QCRYPTO_BLOCK_LUKS_SALT_LEN,
- luks->header.key_slots[0].iterations,
- slotkey, luks->header.master_key_len,
- errp) < 0) {
- goto error;
- }
-
-
- /* Setup the encryption objects needed to encrypt the
- * master key material
- */
- cipher = qcrypto_cipher_new(luks_opts.cipher_alg,
- luks_opts.cipher_mode,
- slotkey, luks->header.master_key_len,
- errp);
- if (!cipher) {
- goto error;
- }
-
- ivgen = qcrypto_ivgen_new(luks_opts.ivgen_alg,
- luks->ivgen_cipher_alg,
- luks_opts.ivgen_hash_alg,
- slotkey, luks->header.master_key_len,
- errp);
- if (!ivgen) {
- goto error;
- }
-
- /* Before storing the master key, we need to vastly
- * increase its size, as protection against forensic
- * disk data recovery */
- splitkey = g_new0(uint8_t, splitkeylen);
-
- if (qcrypto_afsplit_encode(luks_opts.hash_alg,
- luks->header.master_key_len,
- luks->header.key_slots[0].stripes,
- masterkey,
- splitkey,
- errp) < 0) {
- goto error;
- }
-
- /* Now we encrypt the split master key with the key generated
- * from the user's password, before storing it */
- if (qcrypto_block_cipher_encrypt_helper(cipher, block->niv, ivgen,
- QCRYPTO_BLOCK_LUKS_SECTOR_SIZE,
- 0,
- splitkey,
- splitkeylen,
- errp) < 0) {
- goto error;
- }
-
/* The total size of the LUKS headers is the partition header + key
* slot headers, rounded up to the nearest sector, combined with
@@ -1323,23 +1386,21 @@ qcrypto_block_luks_create(QCryptoBlock *block,
goto error;
}
- if (qcrypto_block_luks_store_header(block, writefunc, opaque, errp) < 0) {
- goto error;
- }
- /* Write out the master key material, starting at the
- * sector immediately following the partition header. */
- if (writefunc(block,
- luks->header.key_slots[0].key_offset_sector *
- QCRYPTO_BLOCK_LUKS_SECTOR_SIZE,
- splitkey, splitkeylen,
- opaque,
- errp) != splitkeylen) {
+ /* populate the slot 0 with the password encrypted master key*/
+ /* This will also store the header */
+ if (qcrypto_block_luks_store_key(block,
+ 0,
+ password,
+ masterkey,
+ luks_opts.iter_time,
+ writefunc,
+ opaque,
+ errp) < 0) {
goto error;
}
memset(masterkey, 0, luks->header.master_key_len);
- memset(slotkey, 0, luks->header.master_key_len);
return 0;
@@ -1347,9 +1408,6 @@ qcrypto_block_luks_create(QCryptoBlock *block,
if (masterkey) {
memset(masterkey, 0, luks->header.master_key_len);
}
- if (slotkey) {
- memset(slotkey, 0, luks->header.master_key_len);
- }
qcrypto_block_free_cipher(block);
qcrypto_ivgen_free(block->ivgen);
--
2.17.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH 11/12] qcrypto-luks: simplify the math used for keyslot locations
2019-09-12 9:16 [Qemu-devel] [PATCH 00/12] crypto/luks: preparation for encryption key managment Maxim Levitsky
` (9 preceding siblings ...)
2019-09-12 9:17 ` [Qemu-devel] [PATCH 10/12] qcrypto-luks: extract store key function Maxim Levitsky
@ 2019-09-12 9:17 ` Maxim Levitsky
2019-09-12 9:17 ` [Qemu-devel] [PATCH 12/12] qcrypto-luks: more rigorous header checking Maxim Levitsky
11 siblings, 0 replies; 18+ messages in thread
From: Maxim Levitsky @ 2019-09-12 9:17 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Daniel P. Berrangé,
qemu-block, John Snow, Markus Armbruster, Max Reitz,
Maxim Levitsky
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
crypto/block-luks.c | 63 ++++++++++++++++++++++++++++-----------------
1 file changed, 40 insertions(+), 23 deletions(-)
diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index c6045da33e..0d155c6614 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -409,6 +409,30 @@ qcrypto_block_luks_essiv_cipher(QCryptoCipherAlgorithm cipher,
}
}
+/*
+ * Returns number of sectors needed to store the key material
+ * given number of anti forensic stripes
+ */
+static int
+qcrypto_block_luks_splitkeylen_sectors(const QCryptoBlockLUKS *luks,
+ unsigned int header_sectors,
+ unsigned int stripes)
+{
+ /*
+ * This calculation doesn't match that shown in the spec,
+ * but instead follows the cryptsetup implementation.
+ */
+
+ size_t splitkeylen = luks->header.master_key_len * stripes;
+
+ /* First align the key material size to block size*/
+ size_t splitkeylen_sectors =
+ DIV_ROUND_UP(splitkeylen, QCRYPTO_BLOCK_LUKS_SECTOR_SIZE);
+
+ /* Then also align the key material size to the size of the header */
+ return ROUND_UP(splitkeylen_sectors, header_sectors);
+}
+
/*
* Stores the main LUKS header, taking care of endianess
*/
@@ -1124,7 +1148,8 @@ qcrypto_block_luks_create(QCryptoBlock *block,
QCryptoBlockCreateOptionsLUKS luks_opts;
Error *local_err = NULL;
g_autofree uint8_t *masterkey = NULL;
- size_t splitkeylen = 0;
+ size_t header_sectors;
+ size_t split_key_sectors;
size_t i;
g_autofree char *password = NULL;
const char *cipher_alg;
@@ -1343,37 +1368,29 @@ qcrypto_block_luks_create(QCryptoBlock *block,
goto error;
}
+ /* start with the sector that follows the header*/
+ header_sectors = QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET /
+ QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
+
+ split_key_sectors =
+ qcrypto_block_luks_splitkeylen_sectors(luks,
+ header_sectors,
+ QCRYPTO_BLOCK_LUKS_STRIPES);
- /* Although LUKS has multiple key slots, we're just going
- * to use the first key slot */
- splitkeylen = luks->header.master_key_len * QCRYPTO_BLOCK_LUKS_STRIPES;
for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
- luks->header.key_slots[i].active = QCRYPTO_BLOCK_LUKS_KEY_SLOT_DISABLED;
- luks->header.key_slots[i].stripes = QCRYPTO_BLOCK_LUKS_STRIPES;
+ QCryptoBlockLUKSKeySlot *slot = &luks->header.key_slots[i];
+ slot->active = QCRYPTO_BLOCK_LUKS_KEY_SLOT_DISABLED;
- /* This calculation doesn't match that shown in the spec,
- * but instead follows the cryptsetup implementation.
- */
- luks->header.key_slots[i].key_offset_sector =
- (QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET /
- QCRYPTO_BLOCK_LUKS_SECTOR_SIZE) +
- (ROUND_UP(DIV_ROUND_UP(splitkeylen, QCRYPTO_BLOCK_LUKS_SECTOR_SIZE),
- (QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET /
- QCRYPTO_BLOCK_LUKS_SECTOR_SIZE)) * i);
+ slot->key_offset_sector = header_sectors + i * split_key_sectors;
+ slot->stripes = QCRYPTO_BLOCK_LUKS_STRIPES;
}
-
/* The total size of the LUKS headers is the partition header + key
* slot headers, rounded up to the nearest sector, combined with
* the size of each master key material region, also rounded up
* to the nearest sector */
- luks->header.payload_offset_sector =
- (QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET /
- QCRYPTO_BLOCK_LUKS_SECTOR_SIZE) +
- (ROUND_UP(DIV_ROUND_UP(splitkeylen, QCRYPTO_BLOCK_LUKS_SECTOR_SIZE),
- (QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET /
- QCRYPTO_BLOCK_LUKS_SECTOR_SIZE)) *
- QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS);
+ luks->header.payload_offset_sector = header_sectors +
+ QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS * split_key_sectors;
block->sector_size = QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
block->payload_offset = luks->header.payload_offset_sector *
--
2.17.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH 12/12] qcrypto-luks: more rigorous header checking
2019-09-12 9:16 [Qemu-devel] [PATCH 00/12] crypto/luks: preparation for encryption key managment Maxim Levitsky
` (10 preceding siblings ...)
2019-09-12 9:17 ` [Qemu-devel] [PATCH 11/12] qcrypto-luks: simplify the math used for keyslot locations Maxim Levitsky
@ 2019-09-12 9:17 ` Maxim Levitsky
2019-09-17 10:17 ` Daniel P. Berrangé
11 siblings, 1 reply; 18+ messages in thread
From: Maxim Levitsky @ 2019-09-12 9:17 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Daniel P. Berrangé,
qemu-block, John Snow, Markus Armbruster, Max Reitz,
Maxim Levitsky
Check that keyslots don't overlap with the data,
and check that keyslots don't overlap with each other.
(this is done using naive O(n^2) nested loops,
but since there are just 8 keyslots, this doesn't really matter.
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
crypto/block-luks.c | 52 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 52 insertions(+)
diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index 0d155c6614..6c53bdc428 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -530,6 +530,11 @@ qcrypto_block_luks_load_header(QCryptoBlock *block,
static int
qcrypto_block_luks_check_header(const QCryptoBlockLUKS *luks, Error **errp)
{
+ size_t i, j;
+
+ unsigned int header_sectors = QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET /
+ QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
+
if (memcmp(luks->header.magic, qcrypto_block_luks_magic,
QCRYPTO_BLOCK_LUKS_MAGIC_LEN) != 0) {
error_setg(errp, "Volume is not in LUKS format");
@@ -541,6 +546,53 @@ qcrypto_block_luks_check_header(const QCryptoBlockLUKS *luks, Error **errp)
luks->header.version);
return -1;
}
+
+ /* Check all keyslots for corruption */
+ for (i = 0 ; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS ; i++) {
+
+ const QCryptoBlockLUKSKeySlot *slot1 = &luks->header.key_slots[i];
+ unsigned int start1 = slot1->key_offset_sector;
+ unsigned int len1 =
+ qcrypto_block_luks_splitkeylen_sectors(luks,
+ header_sectors,
+ slot1->stripes);
+
+ if (slot1->stripes == 0) {
+ error_setg(errp, "Keyslot %zu is corrupted (stripes == 0)", i);
+ return -1;
+ }
+
+ if (slot1->active != QCRYPTO_BLOCK_LUKS_KEY_SLOT_DISABLED &&
+ slot1->active != QCRYPTO_BLOCK_LUKS_KEY_SLOT_ENABLED) {
+ error_setg(errp,
+ "Keyslot %zu state (active/disable) is corrupted", i);
+ return -1;
+ }
+
+ if (start1 + len1 > luks->header.payload_offset_sector) {
+ error_setg(errp,
+ "Keyslot %zu is overlapping with the encrypted payload",
+ i);
+ return -1;
+ }
+
+ for (j = i + 1 ; j < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS ; j++) {
+ const QCryptoBlockLUKSKeySlot *slot2 = &luks->header.key_slots[j];
+ unsigned int start2 = slot2->key_offset_sector;
+ unsigned int len2 =
+ qcrypto_block_luks_splitkeylen_sectors(luks,
+ header_sectors,
+ slot2->stripes);
+
+ if (start1 + len1 > start2 && start2 + len2 > start1) {
+ error_setg(errp,
+ "Keyslots %zu and %zu are overlapping in the header",
+ i, j);
+ return -1;
+ }
+ }
+
+ }
return 0;
}
--
2.17.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH 07/12] qcrypto-luks: purge unused error codes from open callback
2019-09-12 9:17 ` [Qemu-devel] [PATCH 07/12] qcrypto-luks: purge unused error codes from open callback Maxim Levitsky
@ 2019-09-17 10:01 ` Daniel P. Berrangé
0 siblings, 0 replies; 18+ messages in thread
From: Daniel P. Berrangé @ 2019-09-17 10:01 UTC (permalink / raw)
To: Maxim Levitsky
Cc: Kevin Wolf, qemu-block, John Snow, qemu-devel, Max Reitz,
Markus Armbruster
On Thu, Sep 12, 2019 at 12:17:05PM +0300, Maxim Levitsky wrote:
> These values are not used by generic crypto code anyway
>
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> ---
> crypto/block-luks.c | 26 +++++++++++++-------------
> 1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/crypto/block-luks.c b/crypto/block-luks.c
> index f3bfc921b2..ba63e9b442 100644
> --- a/crypto/block-luks.c
> +++ b/crypto/block-luks.c
> @@ -675,13 +675,13 @@ qcrypto_block_luks_open(QCryptoBlock *block,
> if (memcmp(luks->header.magic, qcrypto_block_luks_magic,
> QCRYPTO_BLOCK_LUKS_MAGIC_LEN) != 0) {
> error_setg(errp, "Volume is not in LUKS format");
> - ret = -EINVAL;
> + ret = -1;
> goto fail;
> }
The 'ret' variable should be just deleted entirely, so the
'fail:' block can directly 'return -1'.
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 :|
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH 08/12] qcrypto-luks: extract store and load header
2019-09-12 9:17 ` [Qemu-devel] [PATCH 08/12] qcrypto-luks: extract store and load header Maxim Levitsky
@ 2019-09-17 10:02 ` Daniel P. Berrangé
0 siblings, 0 replies; 18+ messages in thread
From: Daniel P. Berrangé @ 2019-09-17 10:02 UTC (permalink / raw)
To: Maxim Levitsky
Cc: Kevin Wolf, qemu-block, Markus Armbruster, qemu-devel, Max Reitz,
John Snow
On Thu, Sep 12, 2019 at 12:17:06PM +0300, Maxim Levitsky wrote:
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> ---
> crypto/block-luks.c | 158 ++++++++++++++++++++++++++------------------
> 1 file changed, 94 insertions(+), 64 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
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 :|
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH 09/12] qcrypto-luks: extract check and parse header
2019-09-12 9:17 ` [Qemu-devel] [PATCH 09/12] qcrypto-luks: extract check and parse header Maxim Levitsky
@ 2019-09-17 10:04 ` Daniel P. Berrangé
0 siblings, 0 replies; 18+ messages in thread
From: Daniel P. Berrangé @ 2019-09-17 10:04 UTC (permalink / raw)
To: Maxim Levitsky
Cc: Kevin Wolf, qemu-block, Markus Armbruster, qemu-devel, Max Reitz,
John Snow
On Thu, Sep 12, 2019 at 12:17:07PM +0300, Maxim Levitsky wrote:
> This is just to make qcrypto_block_luks_open more
> reasonable in size.
>
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> ---
> crypto/block-luks.c | 235 ++++++++++++++++++++++++--------------------
> 1 file changed, 127 insertions(+), 108 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
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 :|
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH 10/12] qcrypto-luks: extract store key function
2019-09-12 9:17 ` [Qemu-devel] [PATCH 10/12] qcrypto-luks: extract store key function Maxim Levitsky
@ 2019-09-17 10:15 ` Daniel P. Berrangé
0 siblings, 0 replies; 18+ messages in thread
From: Daniel P. Berrangé @ 2019-09-17 10:15 UTC (permalink / raw)
To: Maxim Levitsky
Cc: Kevin Wolf, qemu-block, Markus Armbruster, qemu-devel, Max Reitz,
John Snow
On Thu, Sep 12, 2019 at 12:17:08PM +0300, Maxim Levitsky wrote:
> This function will be used later to store
> new keys to the luks metadata
>
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> ---
> crypto/block-luks.c | 304 ++++++++++++++++++++++++++------------------
> 1 file changed, 181 insertions(+), 123 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
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 :|
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH 12/12] qcrypto-luks: more rigorous header checking
2019-09-12 9:17 ` [Qemu-devel] [PATCH 12/12] qcrypto-luks: more rigorous header checking Maxim Levitsky
@ 2019-09-17 10:17 ` Daniel P. Berrangé
0 siblings, 0 replies; 18+ messages in thread
From: Daniel P. Berrangé @ 2019-09-17 10:17 UTC (permalink / raw)
To: Maxim Levitsky
Cc: Kevin Wolf, qemu-block, Markus Armbruster, qemu-devel, Max Reitz,
John Snow
On Thu, Sep 12, 2019 at 12:17:10PM +0300, Maxim Levitsky wrote:
> Check that keyslots don't overlap with the data,
> and check that keyslots don't overlap with each other.
> (this is done using naive O(n^2) nested loops,
> but since there are just 8 keyslots, this doesn't really matter.
>
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> ---
> crypto/block-luks.c | 52 +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 52 insertions(+)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
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 :|
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2019-09-17 10:18 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-12 9:16 [Qemu-devel] [PATCH 00/12] crypto/luks: preparation for encryption key managment Maxim Levitsky
2019-09-12 9:16 ` [Qemu-devel] [PATCH 01/12] block-crypto: misc refactoring Maxim Levitsky
2019-09-12 9:17 ` [Qemu-devel] [PATCH 02/12] qcrypto-luks: rename some fields in QCryptoBlockLUKSHeader Maxim Levitsky
2019-09-12 9:17 ` [Qemu-devel] [PATCH 03/12] qcrypto-luks: don't overwrite cipher_mode in header Maxim Levitsky
2019-09-12 9:17 ` [Qemu-devel] [PATCH 04/12] qcrypto-luks: simplify masterkey and masterkey length Maxim Levitsky
2019-09-12 9:17 ` [Qemu-devel] [PATCH 05/12] qcrypto-luks: pass keyslot index rather that pointer to the keyslot Maxim Levitsky
2019-09-12 9:17 ` [Qemu-devel] [PATCH 06/12] qcrypto-luks: use the parsed encryption settings in QCryptoBlockLUKS Maxim Levitsky
2019-09-12 9:17 ` [Qemu-devel] [PATCH 07/12] qcrypto-luks: purge unused error codes from open callback Maxim Levitsky
2019-09-17 10:01 ` Daniel P. Berrangé
2019-09-12 9:17 ` [Qemu-devel] [PATCH 08/12] qcrypto-luks: extract store and load header Maxim Levitsky
2019-09-17 10:02 ` Daniel P. Berrangé
2019-09-12 9:17 ` [Qemu-devel] [PATCH 09/12] qcrypto-luks: extract check and parse header Maxim Levitsky
2019-09-17 10:04 ` Daniel P. Berrangé
2019-09-12 9:17 ` [Qemu-devel] [PATCH 10/12] qcrypto-luks: extract store key function Maxim Levitsky
2019-09-17 10:15 ` Daniel P. Berrangé
2019-09-12 9:17 ` [Qemu-devel] [PATCH 11/12] qcrypto-luks: simplify the math used for keyslot locations Maxim Levitsky
2019-09-12 9:17 ` [Qemu-devel] [PATCH 12/12] qcrypto-luks: more rigorous header checking Maxim Levitsky
2019-09-17 10:17 ` Daniel P. Berrangé
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).