From 59b060be184aff59cfa101c937c8139e66f452f2 Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Mon, 12 Sep 2016 12:50:12 +0100 Subject: [PATCH 1/8] crypto: use uint64_t for pbkdf iteration count parameters The qcrypto_pbkdf_count_iters method uses a 64 bit int but then checks its value against INT32_MAX before returning it. This bounds check is premature, because the calling code may well scale the iteration count by some value. It is thus better to return a 64-bit integer and let the caller do range checking. For consistency the qcrypto_pbkdf method is also changed to accept a 64bit int, though this is somewhat academic since nettle is limited to taking an 'int' while gcrypt is limited to taking a 'long int'. Reviewed-by: Eric Blake Signed-off-by: Daniel P. Berrange --- crypto/block-luks.c | 52 +++++++++++++++++++++++++----------------- crypto/pbkdf-gcrypt.c | 9 +++++++- crypto/pbkdf-nettle.c | 8 ++++++- crypto/pbkdf-stub.c | 2 +- crypto/pbkdf.c | 16 ++++--------- include/crypto/pbkdf.h | 10 ++++---- 6 files changed, 57 insertions(+), 40 deletions(-) diff --git a/crypto/block-luks.c b/crypto/block-luks.c index aba4455646..bc086acdab 100644 --- a/crypto/block-luks.c +++ b/crypto/block-luks.c @@ -917,6 +917,7 @@ qcrypto_block_luks_create(QCryptoBlock *block, const char *hash_alg; char *cipher_mode_spec = NULL; QCryptoCipherAlgorithm ivcipheralg = 0; + uint64_t iters; memcpy(&luks_opts, &options->u.luks, sizeof(luks_opts)); if (!luks_opts.has_cipher_alg) { @@ -1064,12 +1065,11 @@ qcrypto_block_luks_create(QCryptoBlock *block, /* Determine how many iterations we need to hash the master * key, in order to have 1 second of compute time used */ - luks->header.master_key_iterations = - qcrypto_pbkdf2_count_iters(luks_opts.hash_alg, - masterkey, luks->header.key_bytes, - luks->header.master_key_salt, - QCRYPTO_BLOCK_LUKS_SALT_LEN, - &local_err); + iters = qcrypto_pbkdf2_count_iters(luks_opts.hash_alg, + masterkey, luks->header.key_bytes, + luks->header.master_key_salt, + QCRYPTO_BLOCK_LUKS_SALT_LEN, + &local_err); if (local_err) { error_propagate(errp, local_err); goto error; @@ -1079,11 +1079,15 @@ qcrypto_block_luks_create(QCryptoBlock *block, * explanation why they chose /= 8... Probably so that * if all 8 keyslots are active we only spend 1 second * in total time to check all keys */ - luks->header.master_key_iterations /= 8; - luks->header.master_key_iterations = MAX( - luks->header.master_key_iterations, - QCRYPTO_BLOCK_LUKS_MIN_MASTER_KEY_ITERS); - + iters /= 8; + if (iters > UINT32_MAX) { + error_setg_errno(errp, ERANGE, + "PBKDF iterations %llu larger than %u", + (unsigned long long)iters, UINT32_MAX); + goto error; + } + iters = MAX(iters, QCRYPTO_BLOCK_LUKS_MIN_MASTER_KEY_ITERS); + luks->header.master_key_iterations = iters; /* Hash the master key, saving the result in the LUKS * header. This hash is used when opening the encrypted @@ -1131,22 +1135,28 @@ qcrypto_block_luks_create(QCryptoBlock *block, /* Again we determine how many iterations are required to * hash the user password while consuming 1 second of compute * time */ - luks->header.key_slots[0].iterations = - qcrypto_pbkdf2_count_iters(luks_opts.hash_alg, - (uint8_t *)password, strlen(password), - luks->header.key_slots[0].salt, - QCRYPTO_BLOCK_LUKS_SALT_LEN, - &local_err); + 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, + &local_err); if (local_err) { error_propagate(errp, local_err); goto error; } /* Why /= 2 ? That matches cryptsetup, but there's no * explanation why they chose /= 2... */ - luks->header.key_slots[0].iterations /= 2; - luks->header.key_slots[0].iterations = MAX( - luks->header.key_slots[0].iterations, - QCRYPTO_BLOCK_LUKS_MIN_SLOT_KEY_ITERS); + iters /= 2; + + 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 diff --git a/crypto/pbkdf-gcrypt.c b/crypto/pbkdf-gcrypt.c index 34af3a97e9..44cf31aff4 100644 --- a/crypto/pbkdf-gcrypt.c +++ b/crypto/pbkdf-gcrypt.c @@ -38,7 +38,7 @@ bool qcrypto_pbkdf2_supports(QCryptoHashAlgorithm hash) int qcrypto_pbkdf2(QCryptoHashAlgorithm hash, const uint8_t *key, size_t nkey, const uint8_t *salt, size_t nsalt, - unsigned int iterations, + uint64_t iterations, uint8_t *out, size_t nout, Error **errp) { @@ -49,6 +49,13 @@ int qcrypto_pbkdf2(QCryptoHashAlgorithm hash, }; int ret; + if (iterations > ULONG_MAX) { + error_setg_errno(errp, ERANGE, + "PBKDF iterations %llu must be less than %lu", + (long long unsigned)iterations, ULONG_MAX); + return -1; + } + if (hash >= G_N_ELEMENTS(hash_map) || hash_map[hash] == GCRY_MD_NONE) { error_setg(errp, "Unexpected hash algorithm %d", hash); diff --git a/crypto/pbkdf-nettle.c b/crypto/pbkdf-nettle.c index d681a606f9..db81517adc 100644 --- a/crypto/pbkdf-nettle.c +++ b/crypto/pbkdf-nettle.c @@ -38,10 +38,16 @@ bool qcrypto_pbkdf2_supports(QCryptoHashAlgorithm hash) int qcrypto_pbkdf2(QCryptoHashAlgorithm hash, const uint8_t *key, size_t nkey, const uint8_t *salt, size_t nsalt, - unsigned int iterations, + uint64_t iterations, uint8_t *out, size_t nout, Error **errp) { + if (iterations > UINT_MAX) { + error_setg_errno(errp, ERANGE, + "PBKDF iterations %llu must be less than %u", + (long long unsigned)iterations, UINT_MAX); + return -1; + } switch (hash) { case QCRYPTO_HASH_ALG_SHA1: pbkdf2_hmac_sha1(nkey, key, diff --git a/crypto/pbkdf-stub.c b/crypto/pbkdf-stub.c index 266a5051b7..a15044da42 100644 --- a/crypto/pbkdf-stub.c +++ b/crypto/pbkdf-stub.c @@ -32,7 +32,7 @@ int qcrypto_pbkdf2(QCryptoHashAlgorithm hash G_GNUC_UNUSED, size_t nkey G_GNUC_UNUSED, const uint8_t *salt G_GNUC_UNUSED, size_t nsalt G_GNUC_UNUSED, - unsigned int iterations G_GNUC_UNUSED, + uint64_t iterations G_GNUC_UNUSED, uint8_t *out G_GNUC_UNUSED, size_t nout G_GNUC_UNUSED, Error **errp) diff --git a/crypto/pbkdf.c b/crypto/pbkdf.c index 695cc35df1..929458b312 100644 --- a/crypto/pbkdf.c +++ b/crypto/pbkdf.c @@ -62,13 +62,13 @@ static int qcrypto_pbkdf2_get_thread_cpu(unsigned long long *val_ms, #endif } -int qcrypto_pbkdf2_count_iters(QCryptoHashAlgorithm hash, - const uint8_t *key, size_t nkey, - const uint8_t *salt, size_t nsalt, - Error **errp) +uint64_t qcrypto_pbkdf2_count_iters(QCryptoHashAlgorithm hash, + const uint8_t *key, size_t nkey, + const uint8_t *salt, size_t nsalt, + Error **errp) { uint8_t out[32]; - long long int iterations = (1 << 15); + uint64_t iterations = (1 << 15); unsigned long long delta_ms, start_ms, end_ms; while (1) { @@ -100,11 +100,5 @@ int qcrypto_pbkdf2_count_iters(QCryptoHashAlgorithm hash, iterations = iterations * 1000 / delta_ms; - if (iterations > INT32_MAX) { - error_setg(errp, "Iterations %lld too large for a 32-bit int", - iterations); - return -1; - } - return iterations; } diff --git a/include/crypto/pbkdf.h b/include/crypto/pbkdf.h index e9e4ceca83..6f4ac85b5c 100644 --- a/include/crypto/pbkdf.h +++ b/include/crypto/pbkdf.h @@ -122,7 +122,7 @@ bool qcrypto_pbkdf2_supports(QCryptoHashAlgorithm hash); int qcrypto_pbkdf2(QCryptoHashAlgorithm hash, const uint8_t *key, size_t nkey, const uint8_t *salt, size_t nsalt, - unsigned int iterations, + uint64_t iterations, uint8_t *out, size_t nout, Error **errp); @@ -144,9 +144,9 @@ int qcrypto_pbkdf2(QCryptoHashAlgorithm hash, * * Returns: number of iterations in 1 second, -1 on error */ -int qcrypto_pbkdf2_count_iters(QCryptoHashAlgorithm hash, - const uint8_t *key, size_t nkey, - const uint8_t *salt, size_t nsalt, - Error **errp); +uint64_t qcrypto_pbkdf2_count_iters(QCryptoHashAlgorithm hash, + const uint8_t *key, size_t nkey, + const uint8_t *salt, size_t nsalt, + Error **errp); #endif /* QCRYPTO_PBKDF_H */ From 3bd18890cab82735ae2565fa50aa122e1b4a0ef0 Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Tue, 6 Sep 2016 18:43:00 +0100 Subject: [PATCH 2/8] crypto: make PBKDF iterations configurable for LUKS format As protection against bruteforcing passphrases, the PBKDF algorithm is tuned by counting the number of iterations needed to produce 1 second of running time. If the machine that the image will be used on is much faster than the machine where the image is created, it can be desirable to raise the number of iterations. This change adds a new 'iter-time' property that allows the user to choose the iteration wallclock time. Reviewed-by: Eric Blake Signed-off-by: Daniel P. Berrange --- block/crypto.c | 6 ++++++ crypto/block-luks.c | 24 ++++++++++++++++++++++++ qapi/crypto.json | 6 +++++- 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/block/crypto.c b/block/crypto.c index 7f61e12686..7aa7eb553e 100644 --- a/block/crypto.c +++ b/block/crypto.c @@ -33,6 +33,7 @@ #define BLOCK_CRYPTO_OPT_LUKS_IVGEN_ALG "ivgen-alg" #define BLOCK_CRYPTO_OPT_LUKS_IVGEN_HASH_ALG "ivgen-hash-alg" #define BLOCK_CRYPTO_OPT_LUKS_HASH_ALG "hash-alg" +#define BLOCK_CRYPTO_OPT_LUKS_ITER_TIME "iter-time" typedef struct BlockCrypto BlockCrypto; @@ -183,6 +184,11 @@ static QemuOptsList block_crypto_create_opts_luks = { .type = QEMU_OPT_STRING, .help = "Name of encryption hash algorithm", }, + { + .name = BLOCK_CRYPTO_OPT_LUKS_ITER_TIME, + .type = QEMU_OPT_NUMBER, + .help = "Time to spend in PBKDF in milliseconds", + }, { /* end of list */ } }, }; diff --git a/crypto/block-luks.c b/crypto/block-luks.c index bc086acdab..91a4172287 100644 --- a/crypto/block-luks.c +++ b/crypto/block-luks.c @@ -920,6 +920,9 @@ qcrypto_block_luks_create(QCryptoBlock *block, uint64_t iters; memcpy(&luks_opts, &options->u.luks, sizeof(luks_opts)); + if (!luks_opts.has_iter_time) { + luks_opts.iter_time = 1000; + } if (!luks_opts.has_cipher_alg) { luks_opts.cipher_alg = QCRYPTO_CIPHER_ALG_AES_256; } @@ -1075,6 +1078,16 @@ qcrypto_block_luks_create(QCryptoBlock *block, 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; + /* Why /= 8 ? That matches cryptsetup, but there's no * explanation why they chose /= 8... Probably so that * if all 8 keyslots are active we only spend 1 second @@ -1144,6 +1157,17 @@ qcrypto_block_luks_create(QCryptoBlock *block, 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; + /* Why /= 2 ? That matches cryptsetup, but there's no * explanation why they chose /= 2... */ iters /= 2; diff --git a/qapi/crypto.json b/qapi/crypto.json index 34d2583154..2b6118f660 100644 --- a/qapi/crypto.json +++ b/qapi/crypto.json @@ -185,6 +185,9 @@ # Currently defaults to 'sha256' # @hash-alg: #optional the master key hash algorithm # Currently defaults to 'sha256' +# @iter-time: #optional number of milliseconds to spend in +# PBKDF passphrase processing. Currently defaults +# to 1000. (since 2.8) # Since: 2.6 ## { 'struct': 'QCryptoBlockCreateOptionsLUKS', @@ -193,7 +196,8 @@ '*cipher-mode': 'QCryptoCipherMode', '*ivgen-alg': 'QCryptoIVGenAlgorithm', '*ivgen-hash-alg': 'QCryptoHashAlgorithm', - '*hash-alg': 'QCryptoHashAlgorithm'}} + '*hash-alg': 'QCryptoHashAlgorithm', + '*iter-time': 'int'}} ## From 8813800b7d995d8b54ef0a1e16d41fc13d8b5f3a Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Wed, 7 Sep 2016 12:38:07 +0100 Subject: [PATCH 3/8] crypto: clear out buffer after timing pbkdf algorithm The 'out' buffer will hold a key derived from master password, so it is best practice to clear this buffer when no longer required. At this time, the code isn't worrying about locking buffers into RAM to prevent swapping sensitive data to disk. Reviewed-by: Eric Blake Signed-off-by: Daniel P. Berrange --- crypto/pbkdf.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/crypto/pbkdf.c b/crypto/pbkdf.c index 929458b312..e3915058fb 100644 --- a/crypto/pbkdf.c +++ b/crypto/pbkdf.c @@ -67,13 +67,14 @@ uint64_t qcrypto_pbkdf2_count_iters(QCryptoHashAlgorithm hash, const uint8_t *salt, size_t nsalt, Error **errp) { + uint64_t ret = -1; uint8_t out[32]; uint64_t iterations = (1 << 15); unsigned long long delta_ms, start_ms, end_ms; while (1) { if (qcrypto_pbkdf2_get_thread_cpu(&start_ms, errp) < 0) { - return -1; + goto cleanup; } if (qcrypto_pbkdf2(hash, key, nkey, @@ -81,10 +82,10 @@ uint64_t qcrypto_pbkdf2_count_iters(QCryptoHashAlgorithm hash, iterations, out, sizeof(out), errp) < 0) { - return -1; + goto cleanup; } if (qcrypto_pbkdf2_get_thread_cpu(&end_ms, errp) < 0) { - return -1; + goto cleanup; } delta_ms = end_ms - start_ms; @@ -100,5 +101,9 @@ uint64_t qcrypto_pbkdf2_count_iters(QCryptoHashAlgorithm hash, iterations = iterations * 1000 / delta_ms; - return iterations; + ret = iterations; + + cleanup: + memset(out, 0, sizeof(out)); + return ret; } From e74aabcffb74e6c15de05255480d43771ec63d8b Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Wed, 7 Sep 2016 12:43:29 +0100 Subject: [PATCH 4/8] crypto: use correct derived key size when timing pbkdf Currently when timing the pbkdf algorithm a fixed key size of 32 bytes is used. This results in inaccurate timings for certain hashes depending on their digest size. For example when using sha1 with aes-256, this causes us to measure time for the master key digest doing 2 sha1 operations per iteration, instead of 1. Instead we should pass in the desired key size to the timing routine that matches the key size that will be used for real later. Reviewed-by: Eric Blake Signed-off-by: Daniel P. Berrange --- crypto/block-luks.c | 2 ++ crypto/pbkdf.c | 10 +++++++--- include/crypto/pbkdf.h | 6 +++++- tests/test-crypto-pbkdf.c | 1 + 4 files changed, 15 insertions(+), 4 deletions(-) diff --git a/crypto/block-luks.c b/crypto/block-luks.c index 91a4172287..9269aaf488 100644 --- a/crypto/block-luks.c +++ b/crypto/block-luks.c @@ -1072,6 +1072,7 @@ qcrypto_block_luks_create(QCryptoBlock *block, masterkey, luks->header.key_bytes, luks->header.master_key_salt, QCRYPTO_BLOCK_LUKS_SALT_LEN, + QCRYPTO_BLOCK_LUKS_DIGEST_LEN, &local_err); if (local_err) { error_propagate(errp, local_err); @@ -1152,6 +1153,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, &local_err); if (local_err) { error_propagate(errp, local_err); diff --git a/crypto/pbkdf.c b/crypto/pbkdf.c index e3915058fb..f22e71d183 100644 --- a/crypto/pbkdf.c +++ b/crypto/pbkdf.c @@ -65,13 +65,16 @@ static int qcrypto_pbkdf2_get_thread_cpu(unsigned long long *val_ms, uint64_t qcrypto_pbkdf2_count_iters(QCryptoHashAlgorithm hash, const uint8_t *key, size_t nkey, const uint8_t *salt, size_t nsalt, + size_t nout, Error **errp) { uint64_t ret = -1; - uint8_t out[32]; + uint8_t *out; uint64_t iterations = (1 << 15); unsigned long long delta_ms, start_ms, end_ms; + out = g_new(uint8_t, nout); + while (1) { if (qcrypto_pbkdf2_get_thread_cpu(&start_ms, errp) < 0) { goto cleanup; @@ -80,7 +83,7 @@ uint64_t qcrypto_pbkdf2_count_iters(QCryptoHashAlgorithm hash, key, nkey, salt, nsalt, iterations, - out, sizeof(out), + out, nout, errp) < 0) { goto cleanup; } @@ -104,6 +107,7 @@ uint64_t qcrypto_pbkdf2_count_iters(QCryptoHashAlgorithm hash, ret = iterations; cleanup: - memset(out, 0, sizeof(out)); + memset(out, 0, nout); + g_free(out); return ret; } diff --git a/include/crypto/pbkdf.h b/include/crypto/pbkdf.h index 6f4ac85b5c..ef209b3e03 100644 --- a/include/crypto/pbkdf.h +++ b/include/crypto/pbkdf.h @@ -133,6 +133,7 @@ int qcrypto_pbkdf2(QCryptoHashAlgorithm hash, * @nkey: the length of @key in bytes * @salt: a random salt * @nsalt: length of @salt in bytes + * @nout: size of desired derived key * @errp: pointer to a NULL-initialized error object * * Time the PBKDF2 algorithm to determine how many @@ -140,13 +141,16 @@ int qcrypto_pbkdf2(QCryptoHashAlgorithm hash, * key from a user password provided in @key in 1 * second of compute time. The result of this can * be used as a the @iterations parameter of a later - * call to qcrypto_pbkdf2(). + * call to qcrypto_pbkdf2(). The value of @nout should + * match that value that will later be provided with + * a call to qcrypto_pbkdf2(). * * Returns: number of iterations in 1 second, -1 on error */ uint64_t qcrypto_pbkdf2_count_iters(QCryptoHashAlgorithm hash, const uint8_t *key, size_t nkey, const uint8_t *salt, size_t nsalt, + size_t nout, Error **errp); #endif /* QCRYPTO_PBKDF_H */ diff --git a/tests/test-crypto-pbkdf.c b/tests/test-crypto-pbkdf.c index 8ceceb1827..a651dc50a3 100644 --- a/tests/test-crypto-pbkdf.c +++ b/tests/test-crypto-pbkdf.c @@ -358,6 +358,7 @@ static void test_pbkdf_timing(void) iters = qcrypto_pbkdf2_count_iters(QCRYPTO_HASH_ALG_SHA256, key, sizeof(key), salt, sizeof(salt), + 32, &error_abort); g_assert(iters >= (1 << 15)); From acd0dfd0c252a06ec6f2146fea01b66b7bc68cfc Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Wed, 7 Sep 2016 13:17:07 +0100 Subject: [PATCH 5/8] crypto: remove bogus /= 2 for pbkdf iterations When calculating iterations for pbkdf of the key slot data, we had a /= 2, which was copied from identical code in cryptsetup. It was always unclear & undocumented why cryptsetup had this division and it was recently removed there, too. Reviewed-by: Eric Blake Signed-off-by: Daniel P. Berrange --- crypto/block-luks.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/crypto/block-luks.c b/crypto/block-luks.c index 9269aaf488..3ab3250e3d 100644 --- a/crypto/block-luks.c +++ b/crypto/block-luks.c @@ -1170,10 +1170,6 @@ qcrypto_block_luks_create(QCryptoBlock *block, /* iter_time was in millis, but count_iters reported for secs */ iters = iters * luks_opts.iter_time / 1000; - /* Why /= 2 ? That matches cryptsetup, but there's no - * explanation why they chose /= 2... */ - iters /= 2; - if (iters > UINT32_MAX) { error_setg_errno(errp, ERANGE, "PBKDF iterations %llu larger than %u", From 2ab66cd577d6d0ec3c44b14cc823e76ea5a4397c Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Wed, 7 Sep 2016 12:48:32 +0100 Subject: [PATCH 6/8] crypto: increase default pbkdf2 time for luks to 2 seconds cryptsetup recently increased the default pbkdf2 time to 2 seconds to partially mitigate improvements in hardware performance wrt brute-forcing the pbkdf algorithm. This updates QEMU defaults to match. Reviewed-by: Eric Blake Signed-off-by: Daniel P. Berrange --- crypto/block-luks.c | 2 +- qapi/crypto.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crypto/block-luks.c b/crypto/block-luks.c index 3ab3250e3d..a848232034 100644 --- a/crypto/block-luks.c +++ b/crypto/block-luks.c @@ -921,7 +921,7 @@ qcrypto_block_luks_create(QCryptoBlock *block, memcpy(&luks_opts, &options->u.luks, sizeof(luks_opts)); if (!luks_opts.has_iter_time) { - luks_opts.iter_time = 1000; + luks_opts.iter_time = 2000; } if (!luks_opts.has_cipher_alg) { luks_opts.cipher_alg = QCRYPTO_CIPHER_ALG_AES_256; diff --git a/qapi/crypto.json b/qapi/crypto.json index 2b6118f660..6933b13bd0 100644 --- a/qapi/crypto.json +++ b/qapi/crypto.json @@ -187,7 +187,7 @@ # Currently defaults to 'sha256' # @iter-time: #optional number of milliseconds to spend in # PBKDF passphrase processing. Currently defaults -# to 1000. (since 2.8) +# to 2000. (since 2.8) # Since: 2.6 ## { 'struct': 'QCryptoBlockCreateOptionsLUKS', From 533008f4f382490f817a0c313f2d32f6173c08c7 Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Wed, 7 Sep 2016 13:12:28 +0100 Subject: [PATCH 7/8] crypto: support more hash algorithms for pbkdf Currently pbkdf is only supported with SHA1 and SHA256. Expand this to support all algorithms known to QEMU. Reviewed-by: Eric Blake Signed-off-by: Daniel P. Berrange --- crypto/pbkdf-gcrypt.c | 12 +++++++- crypto/pbkdf-nettle.c | 63 +++++++++++++++++++++++++++++++++------ tests/test-crypto-pbkdf.c | 53 +++++++++++++++++++++++++++++++- 3 files changed, 117 insertions(+), 11 deletions(-) diff --git a/crypto/pbkdf-gcrypt.c b/crypto/pbkdf-gcrypt.c index 44cf31aff4..40289858bf 100644 --- a/crypto/pbkdf-gcrypt.c +++ b/crypto/pbkdf-gcrypt.c @@ -28,7 +28,11 @@ bool qcrypto_pbkdf2_supports(QCryptoHashAlgorithm hash) switch (hash) { case QCRYPTO_HASH_ALG_MD5: case QCRYPTO_HASH_ALG_SHA1: + case QCRYPTO_HASH_ALG_SHA224: case QCRYPTO_HASH_ALG_SHA256: + case QCRYPTO_HASH_ALG_SHA384: + case QCRYPTO_HASH_ALG_SHA512: + case QCRYPTO_HASH_ALG_RIPEMD160: return true; default: return false; @@ -45,7 +49,11 @@ int qcrypto_pbkdf2(QCryptoHashAlgorithm hash, static const int hash_map[QCRYPTO_HASH_ALG__MAX] = { [QCRYPTO_HASH_ALG_MD5] = GCRY_MD_MD5, [QCRYPTO_HASH_ALG_SHA1] = GCRY_MD_SHA1, + [QCRYPTO_HASH_ALG_SHA224] = GCRY_MD_SHA224, [QCRYPTO_HASH_ALG_SHA256] = GCRY_MD_SHA256, + [QCRYPTO_HASH_ALG_SHA384] = GCRY_MD_SHA384, + [QCRYPTO_HASH_ALG_SHA512] = GCRY_MD_SHA512, + [QCRYPTO_HASH_ALG_RIPEMD160] = GCRY_MD_RMD160, }; int ret; @@ -58,7 +66,9 @@ int qcrypto_pbkdf2(QCryptoHashAlgorithm hash, if (hash >= G_N_ELEMENTS(hash_map) || hash_map[hash] == GCRY_MD_NONE) { - error_setg(errp, "Unexpected hash algorithm %d", hash); + error_setg_errno(errp, ENOSYS, + "PBKDF does not support hash algorithm %s", + QCryptoHashAlgorithm_lookup[hash]); return -1; } diff --git a/crypto/pbkdf-nettle.c b/crypto/pbkdf-nettle.c index db81517adc..6fb2671656 100644 --- a/crypto/pbkdf-nettle.c +++ b/crypto/pbkdf-nettle.c @@ -20,6 +20,7 @@ #include "qemu/osdep.h" #include +#include #include "qapi/error.h" #include "crypto/pbkdf.h" @@ -28,7 +29,11 @@ bool qcrypto_pbkdf2_supports(QCryptoHashAlgorithm hash) { switch (hash) { case QCRYPTO_HASH_ALG_SHA1: + case QCRYPTO_HASH_ALG_SHA224: case QCRYPTO_HASH_ALG_SHA256: + case QCRYPTO_HASH_ALG_SHA384: + case QCRYPTO_HASH_ALG_SHA512: + case QCRYPTO_HASH_ALG_RIPEMD160: return true; default: return false; @@ -42,30 +47,70 @@ int qcrypto_pbkdf2(QCryptoHashAlgorithm hash, uint8_t *out, size_t nout, Error **errp) { + union { + struct hmac_md5_ctx md5; + struct hmac_sha1_ctx sha1; + struct hmac_sha224_ctx sha224; + struct hmac_sha256_ctx sha256; + struct hmac_sha384_ctx sha384; + struct hmac_sha512_ctx sha512; + struct hmac_ripemd160_ctx ripemd160; + } ctx; + if (iterations > UINT_MAX) { error_setg_errno(errp, ERANGE, "PBKDF iterations %llu must be less than %u", (long long unsigned)iterations, UINT_MAX); return -1; } + switch (hash) { + case QCRYPTO_HASH_ALG_MD5: + hmac_md5_set_key(&ctx.md5, nkey, key); + PBKDF2(&ctx.md5, hmac_md5_update, hmac_md5_digest, + MD5_DIGEST_SIZE, iterations, nsalt, salt, nout, out); + break; + case QCRYPTO_HASH_ALG_SHA1: - pbkdf2_hmac_sha1(nkey, key, - iterations, - nsalt, salt, - nout, out); + hmac_sha1_set_key(&ctx.sha1, nkey, key); + PBKDF2(&ctx.sha1, hmac_sha1_update, hmac_sha1_digest, + SHA1_DIGEST_SIZE, iterations, nsalt, salt, nout, out); + break; + + case QCRYPTO_HASH_ALG_SHA224: + hmac_sha224_set_key(&ctx.sha224, nkey, key); + PBKDF2(&ctx.sha224, hmac_sha224_update, hmac_sha224_digest, + SHA224_DIGEST_SIZE, iterations, nsalt, salt, nout, out); break; case QCRYPTO_HASH_ALG_SHA256: - pbkdf2_hmac_sha256(nkey, key, - iterations, - nsalt, salt, - nout, out); + hmac_sha256_set_key(&ctx.sha256, nkey, key); + PBKDF2(&ctx.sha256, hmac_sha256_update, hmac_sha256_digest, + SHA256_DIGEST_SIZE, iterations, nsalt, salt, nout, out); + break; + + case QCRYPTO_HASH_ALG_SHA384: + hmac_sha384_set_key(&ctx.sha384, nkey, key); + PBKDF2(&ctx.sha384, hmac_sha384_update, hmac_sha384_digest, + SHA384_DIGEST_SIZE, iterations, nsalt, salt, nout, out); + break; + + case QCRYPTO_HASH_ALG_SHA512: + hmac_sha512_set_key(&ctx.sha512, nkey, key); + PBKDF2(&ctx.sha512, hmac_sha512_update, hmac_sha512_digest, + SHA512_DIGEST_SIZE, iterations, nsalt, salt, nout, out); + break; + + case QCRYPTO_HASH_ALG_RIPEMD160: + hmac_ripemd160_set_key(&ctx.ripemd160, nkey, key); + PBKDF2(&ctx.ripemd160, hmac_ripemd160_update, hmac_ripemd160_digest, + RIPEMD160_DIGEST_SIZE, iterations, nsalt, salt, nout, out); break; default: error_setg_errno(errp, ENOSYS, - "PBKDF does not support hash algorithm %d", hash); + "PBKDF does not support hash algorithm %s", + QCryptoHashAlgorithm_lookup[hash]); return -1; } return 0; diff --git a/tests/test-crypto-pbkdf.c b/tests/test-crypto-pbkdf.c index a651dc50a3..d937aff6b2 100644 --- a/tests/test-crypto-pbkdf.c +++ b/tests/test-crypto-pbkdf.c @@ -261,7 +261,6 @@ static QCryptoPbkdfTestData test_data[] = { "\xcc\x4a\x5e\x6d\xca\x04\xec\x58", .nout = 32 }, -#if 0 { .path = "/crypto/pbkdf/nonrfc/sha512/iter1200", .hash = QCRYPTO_HASH_ALG_SHA512, @@ -279,6 +278,58 @@ static QCryptoPbkdfTestData test_data[] = { "\x76\x14\x80\xf3\xe3\x7a\x22\xb9", .nout = 32 }, + { + .path = "/crypto/pbkdf/nonrfc/sha224/iter1200", + .hash = QCRYPTO_HASH_ALG_SHA224, + .iterations = 1200, + .key = "XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX" + "XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX" + "XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX" + "XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX", + .nkey = 129, + .salt = "pass phrase exceeds block size", + .nsalt = 30, + .out = "\x13\x3b\x88\x0c\x0e\x52\xa2\x41" + "\x49\x33\x35\xa6\xc3\x83\xae\x23" + "\xf6\x77\x43\x9e\x5b\x30\x92\x3e" + "\x4a\x3a\xaa\x24\x69\x3c\xed\x20", + .nout = 32 + }, + { + .path = "/crypto/pbkdf/nonrfc/sha384/iter1200", + .hash = QCRYPTO_HASH_ALG_SHA384, + .iterations = 1200, + .key = "XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX" + "XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX" + "XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX" + "XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX", + .nkey = 129, + .salt = "pass phrase exceeds block size", + .nsalt = 30, + .out = "\xfe\xe3\xe1\x84\xc9\x25\x3e\x10" + "\x47\xc8\x7d\x53\xc6\xa5\xe3\x77" + "\x29\x41\x76\xbd\x4b\xe3\x9b\xac" + "\x05\x6c\x11\xdd\x17\xc5\x93\x80", + .nout = 32 + }, + { + .path = "/crypto/pbkdf/nonrfc/ripemd160/iter1200", + .hash = QCRYPTO_HASH_ALG_RIPEMD160, + .iterations = 1200, + .key = "XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX" + "XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX" + "XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX" + "XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX", + .nkey = 129, + .salt = "pass phrase exceeds block size", + .nsalt = 30, + .out = "\xd6\xcb\xd8\xa7\xdb\x0c\xa2\x2a" + "\x23\x5e\x47\xaf\xdb\xda\xa8\xef" + "\xe4\x01\x0d\x6f\xb5\x33\xc8\xbd" + "\xce\xbf\x91\x14\x8b\x5c\x48\x41", + .nout = 32 + }, +#if 0 { .path = "/crypto/pbkdf/nonrfc/whirlpool/iter1200", .hash = QCRYPTO_HASH_ALG_WHIRLPOOL, From b57482d7a0fe669aeb6f0c3c3503d143b9db89dd Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Wed, 14 Sep 2016 10:18:09 +0100 Subject: [PATCH 8/8] crypto: add trace points for TLS cert verification It is very useful to know about TLS cert verification status when debugging, so add a trace point for it. Signed-off-by: Daniel P. Berrange --- crypto/tlssession.c | 10 ++++++++-- crypto/trace-events | 1 + 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/crypto/tlssession.c b/crypto/tlssession.c index 2de42c61cb..96a02deb69 100644 --- a/crypto/tlssession.c +++ b/crypto/tlssession.c @@ -351,16 +351,22 @@ qcrypto_tls_session_check_credentials(QCryptoTLSSession *session, { if (object_dynamic_cast(OBJECT(session->creds), TYPE_QCRYPTO_TLS_CREDS_ANON)) { + trace_qcrypto_tls_session_check_creds(session, "nop"); return 0; } else if (object_dynamic_cast(OBJECT(session->creds), TYPE_QCRYPTO_TLS_CREDS_X509)) { if (session->creds->verifyPeer) { - return qcrypto_tls_session_check_certificate(session, - errp); + int ret = qcrypto_tls_session_check_certificate(session, + errp); + trace_qcrypto_tls_session_check_creds(session, + ret == 0 ? "pass" : "fail"); + return ret; } else { + trace_qcrypto_tls_session_check_creds(session, "skip"); return 0; } } else { + trace_qcrypto_tls_session_check_creds(session, "error"); error_setg(errp, "Unexpected credential type %s", object_get_typename(OBJECT(session->creds))); return -1; diff --git a/crypto/trace-events b/crypto/trace-events index 8181843723..dc6ddd30d6 100644 --- a/crypto/trace-events +++ b/crypto/trace-events @@ -17,3 +17,4 @@ qcrypto_tls_creds_x509_load_cert_list(void *creds, const char *file) "TLS creds # crypto/tlssession.c qcrypto_tls_session_new(void *session, void *creds, const char *hostname, const char *aclname, int endpoint) "TLS session new session=%p creds=%p hostname=%s aclname=%s endpoint=%d" +qcrypto_tls_session_check_creds(void *session, const char *status) "TLS session check creds session=%p status=%s"