[Qemu-devel] [PATCH 0/6] crypto: misc tweaks & improvements to pbkdf code

[Qemu-devel] [PATCH 0/6] crypto: misc tweaks & improvements to pbkdf code

[Daniel P. Berrange]

The core goal of this series was to make the pbkdf2 iteration
time configurable. In doing so a number of other improvements
were identified, and some updates done to match latest cryptsetup
behaviour / recommendations.

Daniel P. Berrange (6):
  crypto: make PBKDF iterations configurable for LUKS format
  crypto: clear out buffer after timing pbkdf algorithm
  crypto: use correct derived key size when timing pbkdf
  crypto: remove bogus /= 2 for pbkdf iterations
  crypto: increase default pbkdf2 time for luks to 2 seconds
  crypto: support more hash algorithms for pbkdf

 block/crypto.c            |  6 +++++
 crypto/block-luks.c       | 35 ++++++++++++++++++---------
 crypto/pbkdf-gcrypt.c     | 12 +++++++++-
 crypto/pbkdf-nettle.c     | 61 ++++++++++++++++++++++++++++++++++++++++-------
 crypto/pbkdf.c            | 23 ++++++++++++------
 include/crypto/pbkdf.h    |  6 ++++-
 qapi/crypto.json          |  6 ++++-
 tests/test-crypto-pbkdf.c | 54 ++++++++++++++++++++++++++++++++++++++++-
 8 files changed, 172 insertions(+), 31 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH 1/6] crypto: make PBKDF iterations configurable for LUKS format

[Daniel P. Berrange]

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 limits. This adds a new 'iter-time'
property that allows the user to choose the iteration
wallclock time.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 block/crypto.c      |  6 ++++++
 crypto/block-luks.c | 32 +++++++++++++++++++++++---------
 qapi/crypto.json    |  6 +++++-
 3 files changed, 34 insertions(+), 10 deletions(-)

diff --git a/block/crypto.c b/block/crypto.c
index 7f61e12..7aa7eb5 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 aba4455..a5d9ebc 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -917,8 +917,12 @@ 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_iter_time) {
+        luks_opts.iter_time = 1000;
+    }
     if (!luks_opts.has_cipher_alg) {
         luks_opts.cipher_alg = QCRYPTO_CIPHER_ALG_AES_256;
     }
@@ -1064,7 +1068,7 @@ 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 =
+    iters = luks_opts.iter_time *
         qcrypto_pbkdf2_count_iters(luks_opts.hash_alg,
                                    masterkey, luks->header.key_bytes,
                                    luks->header.master_key_salt,
@@ -1074,15 +1078,19 @@ qcrypto_block_luks_create(QCryptoBlock *block,
         error_propagate(errp, local_err);
         goto error;
     }
-
+    /* iter_time was in millis, but count_iters reported for secs */
+    iters /= 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
      * in total time to check all keys */
-    luks->header.master_key_iterations /= 8;
+    iters /= 8;
+    if (iters > UINT32_MAX) {
+        error_setg(errp, "Too many PBKDF iterations for LUKS format");
+        goto error;
+    }
     luks->header.master_key_iterations = MAX(
-        luks->header.master_key_iterations,
-        QCRYPTO_BLOCK_LUKS_MIN_MASTER_KEY_ITERS);
+        iters, QCRYPTO_BLOCK_LUKS_MIN_MASTER_KEY_ITERS);
 
 
     /* Hash the master key, saving the result in the LUKS
@@ -1131,7 +1139,7 @@ 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 =
+    iters = luks_opts.iter_time *
         qcrypto_pbkdf2_count_iters(luks_opts.hash_alg,
                                    (uint8_t *)password, strlen(password),
                                    luks->header.key_slots[0].salt,
@@ -1141,12 +1149,18 @@ qcrypto_block_luks_create(QCryptoBlock *block,
         error_propagate(errp, local_err);
         goto error;
     }
+
+    /* iter_time was in millis, but count_iters reported for secs */
+    iters /= 1000;
     /* Why /= 2 ?  That matches cryptsetup, but there's no
      * explanation why they chose /= 2... */
-    luks->header.key_slots[0].iterations /= 2;
+    iters /= 2;
+    if (iters > UINT32_MAX) {
+        error_setg(errp, "Too many PBKDF iterations for LUKS format");
+        goto error;
+    }
     luks->header.key_slots[0].iterations = MAX(
-        luks->header.key_slots[0].iterations,
-        QCRYPTO_BLOCK_LUKS_MIN_SLOT_KEY_ITERS);
+        iters, QCRYPTO_BLOCK_LUKS_MIN_SLOT_KEY_ITERS);
 
 
     /* Generate a key that we'll use to encrypt the master
diff --git a/qapi/crypto.json b/qapi/crypto.json
index 34d2583..1527f4b 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'}}
 
 
 ##
-- 
2.7.4

[Qemu-devel] [PATCH 2/6] crypto: clear out buffer after timing pbkdf algorithm

[Daniel P. Berrange]

The 'out' buffer will hold a key derived from master
password, so it is best practice to clear this buffer
when no longer required.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 crypto/pbkdf.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/crypto/pbkdf.c b/crypto/pbkdf.c
index 695cc35..35dccc2 100644
--- a/crypto/pbkdf.c
+++ b/crypto/pbkdf.c
@@ -67,13 +67,14 @@ int qcrypto_pbkdf2_count_iters(QCryptoHashAlgorithm hash,
                                const uint8_t *salt, size_t nsalt,
                                Error **errp)
 {
+    int ret = -1;
     uint8_t out[32];
     long long int 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 @@ int 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;
@@ -103,8 +104,12 @@ int qcrypto_pbkdf2_count_iters(QCryptoHashAlgorithm hash,
     if (iterations > INT32_MAX) {
         error_setg(errp, "Iterations %lld too large for a 32-bit int",
                    iterations);
-        return -1;
+        goto cleanup;
     }
 
-    return iterations;
+    ret = iterations;
+
+ cleanup:
+    memset(out, 0, sizeof(out));
+    return ret;
 }
-- 
2.7.4

[Qemu-devel] [PATCH 3/6] crypto: use correct derived key size when timing pbkdf

[Daniel P. Berrange]

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.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 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 a5d9ebc..11047fa 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -1073,6 +1073,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);
@@ -1144,6 +1145,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 35dccc2..0b902a8 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,
 int 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)
 {
     int ret = -1;
-    uint8_t out[32];
+    uint8_t *out;
     long long int iterations = (1 << 15);
     unsigned long long delta_ms, start_ms, end_ms;
 
+    out = g_new0(uint8_t, nout);
+
     while (1) {
         if (qcrypto_pbkdf2_get_thread_cpu(&start_ms, errp) < 0) {
             goto cleanup;
@@ -80,7 +83,7 @@ int qcrypto_pbkdf2_count_iters(QCryptoHashAlgorithm hash,
                            key, nkey,
                            salt, nsalt,
                            iterations,
-                           out, sizeof(out),
+                           out, nout,
                            errp) < 0) {
             goto cleanup;
         }
@@ -110,6 +113,7 @@ int 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 e9e4cec..6b7c54b 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
  */
 int 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 8ceceb1..a651dc5 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));
-- 
2.7.4

[Qemu-devel] [PATCH 4/6] crypto: remove bogus /= 2 for pbkdf iterations

[Daniel P. Berrange]

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
by cryptsetup had this division and it was recently
removed too.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 crypto/block-luks.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index 11047fa..7d5893a 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -1154,9 +1154,6 @@ qcrypto_block_luks_create(QCryptoBlock *block,
 
     /* iter_time was in millis, but count_iters reported for secs */
     iters /= 1000;
-    /* Why /= 2 ?  That matches cryptsetup, but there's no
-     * explanation why they chose /= 2... */
-    iters /= 2;
     if (iters > UINT32_MAX) {
         error_setg(errp, "Too many PBKDF iterations for LUKS format");
         goto error;
-- 
2.7.4

[Qemu-devel] [PATCH 5/6] crypto: increase default pbkdf2 time for luks to 2 seconds

[Daniel P. Berrange]

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.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 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 7d5893a..c843983 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 1527f4b..e7a2ba2 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',
-- 
2.7.4

[Qemu-devel] [PATCH 6/6] crypto: support more hash algorithms for pbkdf

[Daniel P. Berrange]

Currently pbkdf is only supported with SHA1 and SHA256. Expand
this to support all algorithms known to QEMU.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 crypto/pbkdf-gcrypt.c     | 12 +++++++++-
 crypto/pbkdf-nettle.c     | 61 ++++++++++++++++++++++++++++++++++++++++-------
 tests/test-crypto-pbkdf.c | 53 +++++++++++++++++++++++++++++++++++++++-
 3 files changed, 115 insertions(+), 11 deletions(-)

diff --git a/crypto/pbkdf-gcrypt.c b/crypto/pbkdf-gcrypt.c
index 34af3a9..2d35262 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,13 +49,19 @@ 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;
 
     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 d681a60..f9ca79f 100644
--- a/crypto/pbkdf-nettle.c
+++ b/crypto/pbkdf-nettle.c
@@ -20,6 +20,7 @@
 
 #include "qemu/osdep.h"
 #include <nettle/pbkdf2.h>
+#include <nettle/hmac.h>
 #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,24 +47,62 @@ 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;
     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 a651dc5..d937aff 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,
@@ -280,6 +279,58 @@ static QCryptoPbkdfTestData test_data[] = {
         .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,
         .iterations = 1200,
-- 
2.7.4

Re: [Qemu-devel] [PATCH 1/6] crypto: make PBKDF iterations configurable for LUKS format

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 3691 bytes --]

On 09/08/2016 11:27 AM, Daniel P. Berrange wrote:
> 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 limits. This adds a new 'iter-time'

s/limits/iterations/ ?

> property that allows the user to choose the iteration
> wallclock time.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  block/crypto.c      |  6 ++++++
>  crypto/block-luks.c | 32 +++++++++++++++++++++++---------
>  qapi/crypto.json    |  6 +++++-
>  3 files changed, 34 insertions(+), 10 deletions(-)
> 

> +++ b/crypto/block-luks.c
> @@ -917,8 +917,12 @@ 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_iter_time) {
> +        luks_opts.iter_time = 1000;
> +    }
>      if (!luks_opts.has_cipher_alg) {
>          luks_opts.cipher_alg = QCRYPTO_CIPHER_ALG_AES_256;
>      }
> @@ -1064,7 +1068,7 @@ 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 =
> +    iters = luks_opts.iter_time *
>          qcrypto_pbkdf2_count_iters(luks_opts.hash_alg,

luks_opts.iter_time is a user-provided 64-bit value, so this
multiplication can overflow...

>                                     masterkey, luks->header.key_bytes,
>                                     luks->header.master_key_salt,
> @@ -1074,15 +1078,19 @@ qcrypto_block_luks_create(QCryptoBlock *block,
>          error_propagate(errp, local_err);
>          goto error;
>      }
> -
> +    /* iter_time was in millis, but count_iters reported for secs */
> +    iters /= 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
>       * in total time to check all keys */
> -    luks->header.master_key_iterations /= 8;
> +    iters /= 8;
> +    if (iters > UINT32_MAX) {
> +        error_setg(errp, "Too many PBKDF iterations for LUKS format");

...and your check here is too late to catch it.

> @@ -1131,7 +1139,7 @@ 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 =
> +    iters = luks_opts.iter_time *
>          qcrypto_pbkdf2_count_iters(luks_opts.hash_alg,

And again.

> +++ 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

I think this is usually written '(since 2.8)' rather than 'Since 2.8'.
Marc-Andre may have more input on exactly what format he is expecting
when his patches enable doc generation from .json source.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 2/6] crypto: clear out buffer after timing pbkdf algorithm

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 879 bytes --]

On 09/08/2016 11:27 AM, Daniel P. Berrange wrote:
> The 'out' buffer will hold a key derived from master
> password, so it is best practice to clear this buffer
> when no longer required.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  crypto/pbkdf.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

It still doesn't prevent the memory from being copied elsewhere (such as
the stack being paged out), unless we go to extraordinary lengths to
explicitly request volatile memory that can't be paged out.  I don't
know if we need to worry about that, though.  Do any of our crypto
libraries provide APIs for allocating local-use-only memory for
sensitive data?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 3/6] crypto: use correct derived key size when timing pbkdf

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1822 bytes --]

On 09/08/2016 11:27 AM, Daniel P. Berrange wrote:
> 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.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  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(-)
> 

>  int 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)
>  {
>      int ret = -1;
> -    uint8_t out[32];
> +    uint8_t *out;
>      long long int iterations = (1 << 15);
>      unsigned long long delta_ms, start_ms, end_ms;
>  
> +    out = g_new0(uint8_t, nout);

Why g_new0()? Aren't we going to immediately be overwriting its
contents, in which case g_new() is slightly faster?

Also, this changes from stack to heap for the sensitive memory, which
changes the analysis of how likely the memory is to be paged out
somewhere where we don't need spare copies floating around (if that is
indeed something we want to worry about).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 4/6] crypto: remove bogus /= 2 for pbkdf iterations

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 632 bytes --]

On 09/08/2016 11:27 AM, Daniel P. Berrange wrote:
> 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
> by cryptsetup had this division and it was recently

s/by/why/ ?

> removed too.

s/too/there, too/ ?

> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  crypto/block-luks.c | 3 ---
>  1 file changed, 3 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 5/6] crypto: increase default pbkdf2 time for luks to 2 seconds

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1013 bytes --]

On 09/08/2016 11:27 AM, Daniel P. Berrange wrote:
> 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.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  crypto/block-luks.c | 2 +-
>  qapi/crypto.json    | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 

> +++ 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

Possible merge conflicts if you address my comments in earlier patches,
but those are trivial.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 6/6] crypto: support more hash algorithms for pbkdf

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1360 bytes --]

On 09/08/2016 11:27 AM, Daniel P. Berrange wrote:
> Currently pbkdf is only supported with SHA1 and SHA256. Expand
> this to support all algorithms known to QEMU.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  crypto/pbkdf-gcrypt.c     | 12 +++++++++-
>  crypto/pbkdf-nettle.c     | 61 ++++++++++++++++++++++++++++++++++++++++-------
>  tests/test-crypto-pbkdf.c | 53 +++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 115 insertions(+), 11 deletions(-)
> 

>  
>      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]);

Can this access beyond bounds if hash > G_N_ELEMENTS(hash_map)?


> +++ b/crypto/pbkdf-nettle.c

>  
>      default:
>          error_setg_errno(errp, ENOSYS,
> -                         "PBKDF does not support hash algorithm %d", hash);
> +                         "PBKDF does not support hash algorithm %s",
> +                         QCryptoHashAlgorithm_lookup[hash]);

and again

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 0/6] crypto: misc tweaks & improvements to pbkdf code

[no-reply]

Hi,

Your series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 1473352047-908-1-git-send-email-berrange@redhat.com
Subject: [Qemu-devel] [PATCH 0/6] crypto: misc tweaks & improvements to pbkdf code

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git show --no-patch --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
15cfa9d crypto: support more hash algorithms for pbkdf
eb171ab crypto: increase default pbkdf2 time for luks to 2 seconds
b16d6e2 crypto: remove bogus /= 2 for pbkdf iterations
b48082f crypto: use correct derived key size when timing pbkdf
a61fe5a crypto: clear out buffer after timing pbkdf algorithm
cce4ddf crypto: make PBKDF iterations configurable for LUKS format

=== OUTPUT BEGIN ===
Checking PATCH 1/6: crypto: make PBKDF iterations configurable for LUKS format...
Checking PATCH 2/6: crypto: clear out buffer after timing pbkdf algorithm...
Checking PATCH 3/6: crypto: use correct derived key size when timing pbkdf...
Checking PATCH 4/6: crypto: remove bogus /= 2 for pbkdf iterations...
Checking PATCH 5/6: crypto: increase default pbkdf2 time for luks to 2 seconds...
Checking PATCH 6/6: crypto: support more hash algorithms for pbkdf...
ERROR: if this code is redundant consider removing it
#211: FILE: tests/test-crypto-pbkdf.c:332:
+#if 0

total: 1 errors, 0 warnings, 185 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

Re: [Qemu-devel] [PATCH 6/6] crypto: support more hash algorithms for pbkdf

[Daniel P. Berrange]

On Thu, Sep 08, 2016 at 12:57:38PM -0500, Eric Blake wrote:
> On 09/08/2016 11:27 AM, Daniel P. Berrange wrote:
> > Currently pbkdf is only supported with SHA1 and SHA256. Expand
> > this to support all algorithms known to QEMU.
> > 
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> >  crypto/pbkdf-gcrypt.c     | 12 +++++++++-
> >  crypto/pbkdf-nettle.c     | 61 ++++++++++++++++++++++++++++++++++++++++-------
> >  tests/test-crypto-pbkdf.c | 53 +++++++++++++++++++++++++++++++++++++++-
> >  3 files changed, 115 insertions(+), 11 deletions(-)
> > 
> 
> >  
> >      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]);
> 
> Can this access beyond bounds if hash > G_N_ELEMENTS(hash_map)?

I'm relying on fact that 'hash' came from QAPIs string -> enum
convertor - nothing in QEMU lets users provide raw integer enum
values - so will be guaranteed to be a valid enum value.

> > +++ b/crypto/pbkdf-nettle.c
> 
> >  
> >      default:
> >          error_setg_errno(errp, ENOSYS,
> > -                         "PBKDF does not support hash algorithm %d", hash);
> > +                         "PBKDF does not support hash algorithm %s",
> > +                         QCryptoHashAlgorithm_lookup[hash]);
> 
> and again


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 1/6] crypto: make PBKDF iterations configurable for LUKS format

[Daniel P. Berrange]

On Thu, Sep 08, 2016 at 12:44:55PM -0500, Eric Blake wrote:
> On 09/08/2016 11:27 AM, Daniel P. Berrange wrote:
> > 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 limits. This adds a new 'iter-time'
> 
> s/limits/iterations/ ?
> 
> > property that allows the user to choose the iteration
> > wallclock time.
> > 
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> >  block/crypto.c      |  6 ++++++
> >  crypto/block-luks.c | 32 +++++++++++++++++++++++---------
> >  qapi/crypto.json    |  6 +++++-
> >  3 files changed, 34 insertions(+), 10 deletions(-)
> > 
> 
> > +++ b/crypto/block-luks.c
> > @@ -917,8 +917,12 @@ 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_iter_time) {
> > +        luks_opts.iter_time = 1000;
> > +    }
> >      if (!luks_opts.has_cipher_alg) {
> >          luks_opts.cipher_alg = QCRYPTO_CIPHER_ALG_AES_256;
> >      }
> > @@ -1064,7 +1068,7 @@ 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 =
> > +    iters = luks_opts.iter_time *
> >          qcrypto_pbkdf2_count_iters(luks_opts.hash_alg,
> 
> luks_opts.iter_time is a user-provided 64-bit value, so this
> multiplication can overflow...

Oh doh, there I was thinkig it was just a 32bit int...

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 2/6] crypto: clear out buffer after timing pbkdf algorithm

[Daniel P. Berrange]

On Thu, Sep 08, 2016 at 12:47:43PM -0500, Eric Blake wrote:
> On 09/08/2016 11:27 AM, Daniel P. Berrange wrote:
> > The 'out' buffer will hold a key derived from master
> > password, so it is best practice to clear this buffer
> > when no longer required.
> > 
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> >  crypto/pbkdf.c | 15 ++++++++++-----
> >  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> 
> It still doesn't prevent the memory from being copied elsewhere (such as
> the stack being paged out), unless we go to extraordinary lengths to
> explicitly request volatile memory that can't be paged out.  I don't
> know if we need to worry about that, though.  Do any of our crypto
> libraries provide APIs for allocating local-use-only memory for
> sensitive data?

AFAICT, while gcrypt uses such APIs internally, it doesn't expose them
to users. Nettle avoids malloc entirely in its API. So if we wanted
that we'd basically need to roll our own.  I don't think this is a big
deal, but I just wanted to add the memset() as a sanity check really.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|