* [PATCH v2 00/13] crypto/luks: preparation for encryption key managment
@ 2019-09-25 21:35 Maxim Levitsky
2019-09-25 21:35 ` [PATCH v2 01/13] block-crypto: misc refactoring Maxim Levitsky
` (12 more replies)
0 siblings, 13 replies; 16+ messages in thread
From: Maxim Levitsky @ 2019-09-25 21:35 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.
V2:
I addressed all the review comments
I also added another minor patch to improve an error messsage
when trying to create too large file, for which I have an
open bug that waits to be closed.
Its also is form of refactoring, and thus I guess it makes
sense to include it here.
Best regards,
Maxim Levitsky
Maxim Levitsky (13):
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
LUKS: better error message when creating too large files
block/crypto.c | 33 +-
crypto/block-luks.c | 1025 +++++++++++++++++++++++++------------------
2 files changed, 617 insertions(+), 441 deletions(-)
--
2.17.2
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 01/13] block-crypto: misc refactoring
2019-09-25 21:35 [PATCH v2 00/13] crypto/luks: preparation for encryption key managment Maxim Levitsky
@ 2019-09-25 21:35 ` Maxim Levitsky
2019-09-27 10:15 ` Daniel P. Berrangé
2019-09-25 21:35 ` [PATCH v2 02/13] qcrypto-luks: rename some fields in QCryptoBlockLUKSHeader Maxim Levitsky
` (11 subsequent siblings)
12 siblings, 1 reply; 16+ messages in thread
From: Maxim Levitsky @ 2019-09-25 21:35 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] 16+ messages in thread
* [PATCH v2 02/13] qcrypto-luks: rename some fields in QCryptoBlockLUKSHeader
2019-09-25 21:35 [PATCH v2 00/13] crypto/luks: preparation for encryption key managment Maxim Levitsky
2019-09-25 21:35 ` [PATCH v2 01/13] block-crypto: misc refactoring Maxim Levitsky
@ 2019-09-25 21:35 ` Maxim Levitsky
2019-09-25 21:35 ` [PATCH v2 03/13] qcrypto-luks: don't overwrite cipher_mode in header Maxim Levitsky
` (10 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Maxim Levitsky @ 2019-09-25 21:35 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] 16+ messages in thread
* [PATCH v2 03/13] qcrypto-luks: don't overwrite cipher_mode in header
2019-09-25 21:35 [PATCH v2 00/13] crypto/luks: preparation for encryption key managment Maxim Levitsky
2019-09-25 21:35 ` [PATCH v2 01/13] block-crypto: misc refactoring Maxim Levitsky
2019-09-25 21:35 ` [PATCH v2 02/13] qcrypto-luks: rename some fields in QCryptoBlockLUKSHeader Maxim Levitsky
@ 2019-09-25 21:35 ` Maxim Levitsky
2019-09-25 21:35 ` [PATCH v2 04/13] qcrypto-luks: simplify masterkey and masterkey length Maxim Levitsky
` (9 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Maxim Levitsky @ 2019-09-25 21:35 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] 16+ messages in thread
* [PATCH v2 04/13] qcrypto-luks: simplify masterkey and masterkey length
2019-09-25 21:35 [PATCH v2 00/13] crypto/luks: preparation for encryption key managment Maxim Levitsky
` (2 preceding siblings ...)
2019-09-25 21:35 ` [PATCH v2 03/13] qcrypto-luks: don't overwrite cipher_mode in header Maxim Levitsky
@ 2019-09-25 21:35 ` Maxim Levitsky
2019-09-25 21:35 ` [PATCH v2 05/13] qcrypto-luks: pass keyslot index rather that pointer to the keyslot Maxim Levitsky
` (8 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Maxim Levitsky @ 2019-09-25 21:35 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] 16+ messages in thread
* [PATCH v2 05/13] qcrypto-luks: pass keyslot index rather that pointer to the keyslot
2019-09-25 21:35 [PATCH v2 00/13] crypto/luks: preparation for encryption key managment Maxim Levitsky
` (3 preceding siblings ...)
2019-09-25 21:35 ` [PATCH v2 04/13] qcrypto-luks: simplify masterkey and masterkey length Maxim Levitsky
@ 2019-09-25 21:35 ` Maxim Levitsky
2019-09-25 21:35 ` [PATCH v2 06/13] qcrypto-luks: use the parsed encryption settings in QCryptoBlockLUKS Maxim Levitsky
` (7 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Maxim Levitsky @ 2019-09-25 21:35 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] 16+ messages in thread
* [PATCH v2 06/13] qcrypto-luks: use the parsed encryption settings in QCryptoBlockLUKS
2019-09-25 21:35 [PATCH v2 00/13] crypto/luks: preparation for encryption key managment Maxim Levitsky
` (4 preceding siblings ...)
2019-09-25 21:35 ` [PATCH v2 05/13] qcrypto-luks: pass keyslot index rather that pointer to the keyslot Maxim Levitsky
@ 2019-09-25 21:35 ` Maxim Levitsky
2019-09-25 21:35 ` [PATCH v2 07/13] qcrypto-luks: purge unused error codes from open callback Maxim Levitsky
` (6 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Maxim Levitsky @ 2019-09-25 21:35 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] 16+ messages in thread
* [PATCH v2 07/13] qcrypto-luks: purge unused error codes from open callback
2019-09-25 21:35 [PATCH v2 00/13] crypto/luks: preparation for encryption key managment Maxim Levitsky
` (5 preceding siblings ...)
2019-09-25 21:35 ` [PATCH v2 06/13] qcrypto-luks: use the parsed encryption settings in QCryptoBlockLUKS Maxim Levitsky
@ 2019-09-25 21:35 ` Maxim Levitsky
2019-09-25 21:35 ` [PATCH v2 08/13] qcrypto-luks: extract store and load header Maxim Levitsky
` (5 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Maxim Levitsky @ 2019-09-25 21:35 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 | 45 +++++++++++++--------------------------------
1 file changed, 13 insertions(+), 32 deletions(-)
diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index f3bfc921b2..b8f9b9c20a 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -622,9 +622,7 @@ 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;
@@ -648,13 +646,11 @@ qcrypto_block_luks_open(QCryptoBlock *block,
/* 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;
+ if (readfunc(block, 0,
+ (uint8_t *)&luks->header,
+ sizeof(luks->header),
+ opaque,
+ errp) < 0) {
goto fail;
}
@@ -675,13 +671,11 @@ 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;
goto fail;
}
if (luks->header.version != QCRYPTO_BLOCK_LUKS_VERSION) {
error_setg(errp, "LUKS version %" PRIu32 " is not supported",
luks->header.version);
- ret = -ENOTSUP;
goto fail;
}
@@ -697,7 +691,6 @@ qcrypto_block_luks_open(QCryptoBlock *block,
*/
ivgen_name = strchr(cipher_mode, '-');
if (!ivgen_name) {
- ret = -EINVAL;
error_setg(errp, "Unexpected cipher mode string format %s",
cipher_mode);
goto fail;
@@ -715,7 +708,6 @@ 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;
error_propagate(errp, local_err);
goto fail;
}
@@ -724,7 +716,6 @@ qcrypto_block_luks_open(QCryptoBlock *block,
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;
}
@@ -735,7 +726,6 @@ qcrypto_block_luks_open(QCryptoBlock *block,
luks->header.master_key_len,
&local_err);
if (local_err) {
- ret = -ENOTSUP;
error_propagate(errp, local_err);
goto fail;
}
@@ -744,7 +734,6 @@ qcrypto_block_luks_open(QCryptoBlock *block,
qcrypto_block_luks_hash_name_lookup(luks->header.hash_spec,
&local_err);
if (local_err) {
- ret = -ENOTSUP;
error_propagate(errp, local_err);
goto fail;
}
@@ -752,14 +741,12 @@ qcrypto_block_luks_open(QCryptoBlock *block,
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 (luks->ivgen_alg == QCRYPTO_IVGEN_ALG_ESSIV) {
if (!ivhash_name) {
- ret = -EINVAL;
error_setg(errp, "Missing IV generator hash specification");
goto fail;
}
@@ -768,7 +755,6 @@ qcrypto_block_luks_open(QCryptoBlock *block,
luks->ivgen_hash_alg,
&local_err);
if (local_err) {
- ret = -ENOTSUP;
error_propagate(errp, local_err);
goto fail;
}
@@ -795,7 +781,6 @@ qcrypto_block_luks_open(QCryptoBlock *block,
masterkey,
readfunc, opaque,
errp) < 0) {
- ret = -EACCES;
goto fail;
}
@@ -813,19 +798,16 @@ qcrypto_block_luks_open(QCryptoBlock *block,
luks->header.master_key_len,
errp);
if (!block->ivgen) {
- ret = -ENOTSUP;
goto fail;
}
- ret = qcrypto_block_init_cipher(block,
- luks->cipher_alg,
- luks->cipher_mode,
- masterkey,
- luks->header.master_key_len,
- n_threads,
- errp);
- if (ret < 0) {
- ret = -ENOTSUP;
+ if (qcrypto_block_init_cipher(block,
+ luks->cipher_alg,
+ luks->cipher_mode,
+ masterkey,
+ luks->header.master_key_len,
+ n_threads,
+ errp) < 0) {
goto fail;
}
}
@@ -834,14 +816,13 @@ qcrypto_block_luks_open(QCryptoBlock *block,
block->payload_offset = luks->header.payload_offset_sector *
block->sector_size;
-
return 0;
fail:
qcrypto_block_free_cipher(block);
qcrypto_ivgen_free(block->ivgen);
g_free(luks);
- return ret;
+ return -1;
}
--
2.17.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 08/13] qcrypto-luks: extract store and load header
2019-09-25 21:35 [PATCH v2 00/13] crypto/luks: preparation for encryption key managment Maxim Levitsky
` (6 preceding siblings ...)
2019-09-25 21:35 ` [PATCH v2 07/13] qcrypto-luks: purge unused error codes from open callback Maxim Levitsky
@ 2019-09-25 21:35 ` Maxim Levitsky
2019-09-25 21:35 ` [PATCH v2 09/13] qcrypto-luks: extract check and parse header Maxim Levitsky
` (4 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Maxim Levitsky @ 2019-09-25 21:35 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 | 155 ++++++++++++++++++++++++++------------------
1 file changed, 93 insertions(+), 62 deletions(-)
diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index b8f9b9c20a..47371edf13 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.
@@ -622,7 +713,6 @@ qcrypto_block_luks_open(QCryptoBlock *block,
{
QCryptoBlockLUKS *luks = NULL;
Error *local_err = NULL;
- size_t i;
g_autofree uint8_t *masterkey = NULL;
char *ivgen_name, *ivhash_name;
g_autofree char *password = NULL;
@@ -644,30 +734,10 @@ 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 */
- if (readfunc(block, 0,
- (uint8_t *)&luks->header,
- sizeof(luks->header),
- opaque,
- errp) < 0) {
+ if (qcrypto_block_luks_load_header(block, readfunc, opaque, errp) < 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) {
error_setg(errp, "Volume is not in LUKS format");
@@ -1216,46 +1286,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] 16+ messages in thread
* [PATCH v2 09/13] qcrypto-luks: extract check and parse header
2019-09-25 21:35 [PATCH v2 00/13] crypto/luks: preparation for encryption key managment Maxim Levitsky
` (7 preceding siblings ...)
2019-09-25 21:35 ` [PATCH v2 08/13] qcrypto-luks: extract store and load header Maxim Levitsky
@ 2019-09-25 21:35 ` Maxim Levitsky
2019-09-25 21:35 ` [PATCH v2 10/13] qcrypto-luks: extract store key function Maxim Levitsky
` (3 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Maxim Levitsky @ 2019-09-25 21:35 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>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
crypto/block-luks.c | 223 +++++++++++++++++++++++++-------------------
1 file changed, 125 insertions(+), 98 deletions(-)
diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index 47371edf13..fa799fd21d 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,11 +835,8 @@ qcrypto_block_luks_open(QCryptoBlock *block,
Error **errp)
{
QCryptoBlockLUKS *luks = NULL;
- Error *local_err = NULL;
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) {
@@ -738,107 +858,14 @@ qcrypto_block_luks_open(QCryptoBlock *block,
goto fail;
}
- if (memcmp(luks->header.magic, qcrypto_block_luks_magic,
- QCRYPTO_BLOCK_LUKS_MAGIC_LEN) != 0) {
- error_setg(errp, "Volume is not in LUKS format");
- goto fail;
- }
- if (luks->header.version != QCRYPTO_BLOCK_LUKS_VERSION) {
- error_setg(errp, "LUKS version %" PRIu32 " is not supported",
- luks->header.version);
- 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) {
- 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) {
- error_propagate(errp, local_err);
- goto fail;
- }
- }
-
- luks->cipher_mode = qcrypto_block_luks_cipher_mode_lookup(cipher_mode,
- &local_err);
- if (local_err) {
- 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) {
- 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) {
- error_propagate(errp, local_err);
+ if (qcrypto_block_luks_check_header(luks, errp) < 0) {
goto fail;
}
- luks->ivgen_alg = qcrypto_block_luks_ivgen_name_lookup(ivgen_name,
- &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
+ if (qcrypto_block_luks_parse_header(luks, errp) < 0) {
goto fail;
}
- if (luks->ivgen_alg == QCRYPTO_IVGEN_ALG_ESSIV) {
- if (!ivhash_name) {
- 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) {
- 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] 16+ messages in thread
* [PATCH v2 10/13] qcrypto-luks: extract store key function
2019-09-25 21:35 [PATCH v2 00/13] crypto/luks: preparation for encryption key managment Maxim Levitsky
` (8 preceding siblings ...)
2019-09-25 21:35 ` [PATCH v2 09/13] qcrypto-luks: extract check and parse header Maxim Levitsky
@ 2019-09-25 21:35 ` Maxim Levitsky
2019-09-25 21:35 ` [PATCH v2 11/13] qcrypto-luks: simplify the math used for keyslot locations Maxim Levitsky
` (2 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Maxim Levitsky @ 2019-09-25 21:35 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>
Reviewed-by: Daniel P. Berrangé <berrange@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 fa799fd21d..6d4e9eb348 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.
@@ -944,12 +1114,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;
@@ -1172,9 +1338,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,
@@ -1188,107 +1352,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
@@ -1313,23 +1376,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;
@@ -1337,9 +1398,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] 16+ messages in thread
* [PATCH v2 11/13] qcrypto-luks: simplify the math used for keyslot locations
2019-09-25 21:35 [PATCH v2 00/13] crypto/luks: preparation for encryption key managment Maxim Levitsky
` (9 preceding siblings ...)
2019-09-25 21:35 ` [PATCH v2 10/13] qcrypto-luks: extract store key function Maxim Levitsky
@ 2019-09-25 21:35 ` Maxim Levitsky
2019-09-25 21:35 ` [PATCH v2 12/13] qcrypto-luks: more rigorous header checking Maxim Levitsky
2019-09-25 21:35 ` [PATCH v2 13/13] LUKS: better error message when creating too large files Maxim Levitsky
12 siblings, 0 replies; 16+ messages in thread
From: Maxim Levitsky @ 2019-09-25 21:35 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 6d4e9eb348..a53d5d1916 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
*/
@@ -1114,7 +1138,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;
@@ -1333,37 +1358,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] 16+ messages in thread
* [PATCH v2 12/13] qcrypto-luks: more rigorous header checking
2019-09-25 21:35 [PATCH v2 00/13] crypto/luks: preparation for encryption key managment Maxim Levitsky
` (10 preceding siblings ...)
2019-09-25 21:35 ` [PATCH v2 11/13] qcrypto-luks: simplify the math used for keyslot locations Maxim Levitsky
@ 2019-09-25 21:35 ` Maxim Levitsky
2019-09-25 21:35 ` [PATCH v2 13/13] LUKS: better error message when creating too large files Maxim Levitsky
12 siblings, 0 replies; 16+ messages in thread
From: Maxim Levitsky @ 2019-09-25 21:35 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>
Reviewed-by: Daniel P. Berrangé <berrange@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 a53d5d1916..4861db810c 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] 16+ messages in thread
* [PATCH v2 13/13] LUKS: better error message when creating too large files
2019-09-25 21:35 [PATCH v2 00/13] crypto/luks: preparation for encryption key managment Maxim Levitsky
` (11 preceding siblings ...)
2019-09-25 21:35 ` [PATCH v2 12/13] qcrypto-luks: more rigorous header checking Maxim Levitsky
@ 2019-09-25 21:35 ` Maxim Levitsky
12 siblings, 0 replies; 16+ messages in thread
From: Maxim Levitsky @ 2019-09-25 21:35 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Daniel P. Berrangé,
qemu-block, John Snow, Markus Armbruster, Max Reitz,
Maxim Levitsky
Currently if you attampt to create too large file with luks you
get the following error message:
Formatting 'test.luks', fmt=luks size=17592186044416 key-secret=sec0
qemu-img: test.luks: Could not resize file: File too large
While for raw format the error message is
qemu-img: test.img: The image size is too large for file format 'raw'
The reason for this is that qemu-img checks for errono of the failure,
and presents the later error when it is -EFBIG
However crypto generic code 'swallows' the errno and replaces it
with -EIO.
As an attempt to make it better, we can make luks driver,
detect -EFBIG and in this case present a better error message,
which is what this patch does
The new error message is:
qemu-img: error creating test.luks: The requested file size is too large
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1534898
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
block/crypto.c | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/block/crypto.c b/block/crypto.c
index 6e822c6e50..19c2ac602c 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -102,10 +102,12 @@ static ssize_t block_crypto_create_init_func(QCryptoBlock *block,
Error **errp)
{
struct BlockCryptoCreateData *data = opaque;
+ Error *local_error = NULL;
+ int ret;
if (data->size > INT64_MAX || headerlen > INT64_MAX - data->size) {
- error_setg(errp, "The requested file size is too large");
- return -EFBIG;
+ ret = -EFBIG;
+ goto error;
}
/*
@@ -115,6 +117,21 @@ static ssize_t block_crypto_create_init_func(QCryptoBlock *block,
*/
return blk_truncate(data->blk, data->size + headerlen, data->prealloc,
errp);
+
+ if (ret >= 0) {
+ return ret;
+ }
+
+error:
+ if (ret == -EFBIG) {
+ /* Replace the error message with a better one */
+ error_free(local_error);
+ error_setg(errp, "The requested file size is too large");
+ } else {
+ error_propagate(errp, local_error);
+ }
+
+ return ret;
}
--
2.17.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2 01/13] block-crypto: misc refactoring
2019-09-25 21:35 ` [PATCH v2 01/13] block-crypto: misc refactoring Maxim Levitsky
@ 2019-09-27 10:15 ` Daniel P. Berrangé
2019-09-27 16:00 ` Maxim Levitsky
0 siblings, 1 reply; 16+ messages in thread
From: Daniel P. Berrangé @ 2019-09-27 10:15 UTC (permalink / raw)
To: Maxim Levitsky
Cc: Kevin Wolf, qemu-block, John Snow, qemu-devel, Markus Armbruster,
Max Reitz
On Thu, Sep 26, 2019 at 12:35:15AM +0300, Maxim Levitsky wrote:
> * 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,
The parameters need re-indenting when you change this name.
> @@ -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)
And here
I dropped this one from the pull request, so please bundle it into your
other luks series
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] 16+ messages in thread
* Re: [PATCH v2 01/13] block-crypto: misc refactoring
2019-09-27 10:15 ` Daniel P. Berrangé
@ 2019-09-27 16:00 ` Maxim Levitsky
0 siblings, 0 replies; 16+ messages in thread
From: Maxim Levitsky @ 2019-09-27 16:00 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Kevin Wolf, qemu-block, John Snow, qemu-devel, Markus Armbruster,
Max Reitz
On Fri, 2019-09-27 at 11:15 +0100, Daniel P. Berrangé wrote:
> On Thu, Sep 26, 2019 at 12:35:15AM +0300, Maxim Levitsky wrote:
> > * 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,
>
> The parameters need re-indenting when you change this name.
I am just blind thats all :-(
I checked all the patches for the indention and still missed it :-(
Will do of course.
Best regards,
Maxim Levitsky
>
> > @@ -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)
>
> And here
>
> I dropped this one from the pull request, so please bundle it into your
> other luks series
>
> Regards,
> Daniel
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2019-09-27 17:40 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-25 21:35 [PATCH v2 00/13] crypto/luks: preparation for encryption key managment Maxim Levitsky
2019-09-25 21:35 ` [PATCH v2 01/13] block-crypto: misc refactoring Maxim Levitsky
2019-09-27 10:15 ` Daniel P. Berrangé
2019-09-27 16:00 ` Maxim Levitsky
2019-09-25 21:35 ` [PATCH v2 02/13] qcrypto-luks: rename some fields in QCryptoBlockLUKSHeader Maxim Levitsky
2019-09-25 21:35 ` [PATCH v2 03/13] qcrypto-luks: don't overwrite cipher_mode in header Maxim Levitsky
2019-09-25 21:35 ` [PATCH v2 04/13] qcrypto-luks: simplify masterkey and masterkey length Maxim Levitsky
2019-09-25 21:35 ` [PATCH v2 05/13] qcrypto-luks: pass keyslot index rather that pointer to the keyslot Maxim Levitsky
2019-09-25 21:35 ` [PATCH v2 06/13] qcrypto-luks: use the parsed encryption settings in QCryptoBlockLUKS Maxim Levitsky
2019-09-25 21:35 ` [PATCH v2 07/13] qcrypto-luks: purge unused error codes from open callback Maxim Levitsky
2019-09-25 21:35 ` [PATCH v2 08/13] qcrypto-luks: extract store and load header Maxim Levitsky
2019-09-25 21:35 ` [PATCH v2 09/13] qcrypto-luks: extract check and parse header Maxim Levitsky
2019-09-25 21:35 ` [PATCH v2 10/13] qcrypto-luks: extract store key function Maxim Levitsky
2019-09-25 21:35 ` [PATCH v2 11/13] qcrypto-luks: simplify the math used for keyslot locations Maxim Levitsky
2019-09-25 21:35 ` [PATCH v2 12/13] qcrypto-luks: more rigorous header checking Maxim Levitsky
2019-09-25 21:35 ` [PATCH v2 13/13] LUKS: better error message when creating too large files Maxim Levitsky
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.