crypto: forbid setting the PBKDF2 iter count to 0

tniessen · addaleax · commit 10f5fa75136b · 2019-11-28T00:36:36.000+01:00
RFC 2898 does not permit an iteration count of zero, and OpenSSL 1.1.1 will treat it as one iteration internally. Future OpenSSL versions will reject such inputs (already on master branch), but until that happens, Node.js should manually reject them. Refs: nodejs/webcrypto#29 PR-URL: #30578 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
diff --git a/doc/api/crypto.md b/doc/api/crypto.md
@@ -2294,6 +2294,10 @@ console.log(hashes); // ['DSA', 'DSA-SHA', 'DSA-SHA1', ...]
 <!-- YAML
 added: v0.5.5
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/30578
+    description: The `iterations` parameter is now restricted to positive
+                 values. Earlier releases treated other values as one.
   - version: v8.0.0
     pr-url: https://github.com/nodejs/node/pull/11305
     description: The `digest` parameter is always required now.
@@ -2369,6 +2373,10 @@ negative performance implications for some applications; see the
 <!-- YAML
 added: v0.9.3
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/30578
+    description: The `iterations` parameter is now restricted to positive
+                 values. Earlier releases treated other values as one.
   - version: v6.0.0
     pr-url: https://github.com/nodejs/node/pull/4047
     description: Calling this function without passing the `digest` parameter
diff --git a/lib/internal/crypto/pbkdf2.js b/lib/internal/crypto/pbkdf2.js
@@ -66,7 +66,7 @@ function check(password, salt, iterations, keylen, digest) {
 
   password = getArrayBufferView(password, 'password');
   salt = getArrayBufferView(salt, 'salt');
-  validateUint32(iterations, 'iterations');
+  validateUint32(iterations, 'iterations', true);
   validateUint32(keylen, 'keylen');
 
   return { password, salt, iterations, keylen, digest };
diff --git a/test/parallel/test-crypto-pbkdf2.js b/test/parallel/test-crypto-pbkdf2.js
@@ -68,15 +68,17 @@ assert.throws(
   }
 );
 
-assert.throws(
-  () => crypto.pbkdf2Sync('password', 'salt', -1, 20, 'sha1'),
-  {
-    code: 'ERR_OUT_OF_RANGE',
-    name: 'RangeError',
-    message: 'The value of "iterations" is out of range. ' +
-             'It must be >= 0 && < 4294967296. Received -1'
-  }
-);
+for (const iterations of [-1, 0]) {
+  assert.throws(
+    () => crypto.pbkdf2Sync('password', 'salt', iterations, 20, 'sha1'),
+    {
+      code: 'ERR_OUT_OF_RANGE',
+      name: 'RangeError',
+      message: 'The value of "iterations" is out of range. ' +
+               `It must be >= 1 && < 4294967296. Received ${iterations}`
+    }
+  );
+}
 
 ['str', null, undefined, [], {}].forEach((notNumber) => {
   assert.throws(
