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;
