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 <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

1 files changed, 31 insertions, 21 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