Reject iterations=0 when calling PKCS5_PBKDF2_HMAC(). BUG=https://crbug.com/534961 Change-Id: I69e2434bf8d5564711863c393ee3bafe3763cf24 Reviewed-on: https://boringssl-review.googlesource.com/5932 Reviewed-by: Adam Langley <agl@google.com>

diff --git a/crypto/evp/pbkdf.c b/crypto/evp/pbkdf.c
index be6ed86..b06b922 100644
--- a/crypto/evp/pbkdf.c
+++ b/crypto/evp/pbkdf.c

@@ -123,6 +123,22 @@
     p += cplen;
   }
   HMAC_CTX_cleanup(&hctx_tpl);
+
+  // RFC 2898 describes iterations (c) as being a "positive integer", so a
+  // value of 0 is an error.
+  //
+  // Unfortunatley not all consumers of PKCS5_PBKDF2_HMAC() check their return
+  // value, expecting it to succeed and unconditonally using |out_key|.
+  // As a precaution for such callsites in external code, the old behavior
+  // of iterations < 1 being treated as iterations == 1 is preserved, but
+  // additionally an error result is returned.
+  //
+  // TODO(eroman): Figure out how to remove this compatibility hack, or change
+  // the default to something more sensible like 2048.
+  if (iterations == 0) {
+    return 0;
+  }
+
   return 1;
 }
 

diff --git a/crypto/evp/pbkdf_test.cc b/crypto/evp/pbkdf_test.cc
index ae2f405..a39189f 100644
--- a/crypto/evp/pbkdf_test.cc
+++ b/crypto/evp/pbkdf_test.cc

@@ -149,6 +149,44 @@
   return true;
 }
 
+// Tests key derivation using iterations=0.
+//
+// RFC 2898 defines the iteration count (c) as a "positive integer". So doing a
+// key derivation with iterations=0 is ill-defined and should result in a
+// failure.
+static bool TestZeroIterations() {
+  static const char kPassword[] = "password";
+  const size_t password_len = strlen(kPassword);
+  static const uint8_t kSalt[] = {1, 2, 3, 4};
+  const size_t salt_len = sizeof(kSalt);
+  const EVP_MD *digest = EVP_sha1();
+
+  uint8_t key[10] = {0};
+  const size_t key_len = sizeof(key);
+
+  // Verify that calling with iterations=1 works.
+  if (!PKCS5_PBKDF2_HMAC(kPassword, password_len, kSalt, salt_len,
+                         1 /* iterations */, digest, key_len, key)) {
+    fprintf(stderr, "PBKDF2 failed with iterations=1\n");
+    return false;
+  }
+
+  // Flip the first key byte (so can later test if it got set).
+  const uint8_t expected_first_byte = key[0];
+  key[0] = ~key[0];
+
+  // However calling it with iterations=0 fails.
+  if (PKCS5_PBKDF2_HMAC(kPassword, password_len, kSalt, salt_len,
+                        0 /* iterations */, digest, key_len, key)) {
+    fprintf(stderr, "PBKDF2 returned zero with iterations=0\n");
+    return false;
+  }
+
+  // For backwards compatibility, the iterations == 0 case still fills in
+  // the out key.
+  return key[0] == expected_first_byte;
+}
+
 int main(void) {
   CRYPTO_library_init();
   ERR_load_crypto_strings();
@@ -173,6 +211,11 @@
     return 1;
   }
 
+  if (!TestZeroIterations()) {
+    fprintf(stderr, "TestZeroIterations failed\n");
+    return 1;
+  }
+
   printf("PASS\n");
   ERR_free_strings();
   return 0;