From ce372c37127c1b8199ca1732d58b623664250e7e Mon Sep 17 00:00:00 2001
From: Rob Austein <sra@hactrn.net>
Date: Fri, 5 Jun 2015 15:00:53 -0400
Subject: Get feedback cycle right in PBKDF2 iteration.

---
 .gitignore          |  1 +
 pbkdf2.c            | 38 +++++++++++++++++++++-----------------
 tests/test-pbkdf2.c |  3 ++-
 3 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/.gitignore b/.gitignore
index 20d13e4..c200d8d 100644
--- a/.gitignore
+++ b/.gitignore
@@ -7,4 +7,5 @@ config.log
 config.status
 tests/test-aes-key-wrap
 tests/test-hash
+tests/test-pbkdf2
 tests/test-rsa
diff --git a/pbkdf2.c b/pbkdf2.c
index 5b70201..1ed8332 100644
--- a/pbkdf2.c
+++ b/pbkdf2.c
@@ -86,8 +86,12 @@ hal_error_t hal_pbkdf2(const hal_hash_descriptor_t * const descriptor,
                        uint8_t       * derived_key,          size_t derived_key_length,
                        unsigned iterations_desired)
 {
-  uint8_t statebuf[1024];       /* C99 may let us clean this up */
-  uint32_t block_count;
+  uint8_t ac[HAL_MAX_HASH_DIGEST_LENGTH], mac[HAL_MAX_HASH_DIGEST_LENGTH];
+  uint8_t statebuf[1024];
+  unsigned iteration;
+  hal_error_t err;
+  uint32_t block;
+  int i;
 
   if (descriptor == NULL || password == NULL || salt == NULL ||
       derived_key == NULL || derived_key_length == 0 ||
@@ -95,11 +99,16 @@ hal_error_t hal_pbkdf2(const hal_hash_descriptor_t * const descriptor,
     return HAL_ERROR_BAD_ARGUMENTS;
 
   assert(sizeof(statebuf) >= descriptor->hmac_state_length);
+  assert(sizeof(ac)       >= descriptor->digest_length);
+  assert(sizeof(mac)      >= descriptor->digest_length);
 
   /* Output length check per RFC 2989 5.2. */
   if ((uint64_t) derived_key_length > ((uint64_t) 0xFFFFFFFF) * descriptor->block_length)
     return HAL_ERROR_UNSUPPORTED_KEY;
 
+  memset(ac,  0, sizeof(ac));
+  memset(mac, 0, sizeof(mac));
+
   /*
    * We probably should check here to see whether the password is
    * longer than the HMAC block size, and, if so, we should hash the
@@ -113,38 +122,33 @@ hal_error_t hal_pbkdf2(const hal_hash_descriptor_t * const descriptor,
    * Generate output blocks until we reach the requested length.
    */
 
-  for (block_count = 1; ; block_count++) {
-
-    uint8_t accumulator[HAL_MAX_HASH_DIGEST_LENGTH], mac[HAL_MAX_HASH_DIGEST_LENGTH];
-    unsigned iteration;
-    hal_error_t err;
-    int i;
+  for (block = 1; ; block++) {
 
     /*
-     * Initialize the accumulator with the HMAC of the salt
-     * concatenated with the block count.
+     * Initial HMAC is of the salt concatenated with the block count.
+     * This seeds the accumulator, and constitutes iteration one.
      */
 
     if ((err = do_hmac(descriptor, password, password_length, salt, salt_length,
-                       block_count, accumulator, sizeof(accumulator))) != HAL_OK)
+                       block, mac, sizeof(mac))) != HAL_OK)
       return err;
 
+    memcpy(ac, mac, descriptor->digest_length);
+
     /*
      * Now iterate however many times the caller requested, XORing the
      * result back into the accumulator on each iteration.
-     * Initializing the accumulator counts as iteration 1, so we start
-     * with iteration 2.
      */
 
     for (iteration = 2; iteration <= iterations_desired; iteration++) {
 
       if ((err = do_hmac(descriptor, password, password_length,
-                         accumulator, descriptor->digest_length,
+                         ac, descriptor->digest_length,
                          0, mac, sizeof(mac))) != HAL_OK)
         return err;
 
       for (i = 0; i < descriptor->digest_length; i++)
-        accumulator[i] ^= mac[i];
+        ac[i] ^= mac[i];
     }
 
     /*
@@ -153,12 +157,12 @@ hal_error_t hal_pbkdf2(const hal_hash_descriptor_t * const descriptor,
      */
 
     if (derived_key_length > descriptor->digest_length) {
-      memcpy(derived_key, accumulator, descriptor->digest_length);
+      memcpy(derived_key, ac, descriptor->digest_length);
       derived_key        += descriptor->digest_length;
       derived_key_length -= descriptor->digest_length;
     }
     else {
-      memcpy(derived_key, accumulator, derived_key_length);
+      memcpy(derived_key, ac, derived_key_length);
       return HAL_OK;
     }
   }
diff --git a/tests/test-pbkdf2.c b/tests/test-pbkdf2.c
index 67940b4..477dfe2 100644
--- a/tests/test-pbkdf2.c
+++ b/tests/test-pbkdf2.c
@@ -153,7 +153,8 @@ static const uint8_t pbkdf2_tc_6_DK[] = { /* 16 bytes */
 
 static void print_hex(const uint8_t * const val, const size_t len)
 {
-  for (size_t i = 0; i < len; i++)
+  size_t i;
+  for (i = 0; i < len; i++)
     printf(" %02x", val[i]);
 }
 
-- 
cgit v1.2.3