crypto: forbid setting the PBKDF2 iter count to 0

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 <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
nodejs · Nov 27, 2019 · 10f5fa7 · 10f5fa7
1 parent 74f8196
commit 10f5fa7
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 10 deletions.
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(