Proposal: crypto: Make the digest parameter required for pbkdf2.

doc/api/crypto.markdown

@@ -618,11 +618,11 @@ Example (obtaining a shared secret):
     /* alice_secret and bob_secret should be the same */
     console.log(alice_secret == bob_secret);
 
-## crypto.pbkdf2(password, salt, iterations, keylen[, digest], callback)
+## crypto.pbkdf2(password, salt, iterations, keylen, digest, callback)
 
-Asynchronous PBKDF2 function.  Applies the selected HMAC digest function
-(default: SHA1) to derive a key of the requested length from the password,
-salt and number of iterations.  The callback gets two arguments:
+Asynchronous PBKDF2 function.  Applies the selected HMAC digest function to
+derive a key of the requested length from the password, salt and number of
+iterations.  The callback gets two arguments:
 `(err, derivedKey)`.
 
 Example:

lib/crypto.js

@@ -537,9 +537,9 @@ exports.pbkdf2 = function(password,
                           keylen,
                           digest,
                           callback) {
-  if (typeof digest === 'function') {
-    callback = digest;
-    digest = undefined;
+
+  if (typeof digest !== 'string') {
+    throw new Error('No digest provided to pbkdf2');
   }
 
   if (typeof callback !== 'function')

src/node_crypto.cc

@@ -4831,7 +4831,8 @@ void PBKDF2(const FunctionCallbackInfo<Value>& args) {
   }
 
   if (digest == nullptr) {
-    digest = EVP_sha1();
+    type_error = "Bad digest name";
+    goto err;
   }
 
   obj = env->NewInternalFieldObject();

test/parallel/test-crypto-binary-default.js

@@ -627,12 +627,14 @@ assert.strictEqual(rsaVerify.verify(rsaPubPem, rsaSignature, 'hex'), true);
 // Test PBKDF2 with RFC 6070 test vectors (except #4)
 //
 function testPBKDF2(password, salt, iterations, keylen, expected) {
-  var actual = crypto.pbkdf2Sync(password, salt, iterations, keylen);
+  var actual = crypto.pbkdf2Sync(password, salt, iterations, keylen, 'sha1');
   assert.equal(actual, expected);
 
-  crypto.pbkdf2(password, salt, iterations, keylen, function(err, actual) {
+  function assertCb(err, actual) {
     assert.equal(actual, expected);
-  });
+  }
+
+  crypto.pbkdf2(password, salt, iterations, keylen, 'sha1', assertCb);
 }
 
 

test/parallel/test-crypto-domains.js

@@ -25,7 +25,7 @@ d.run(function() {
   one();
 
   function one() {
-    crypto.pbkdf2('a', 'b', 1, 8, function() {
+    crypto.pbkdf2('a', 'b', 1, 8, 'sha1', function() {
       two();
       throw new Error('pbkdf2');
     });

test/parallel/test-crypto-pbkdf2.js

@@ -12,12 +12,14 @@ var crypto = require('crypto');
 // Test PBKDF2 with RFC 6070 test vectors (except #4)
 //
 function testPBKDF2(password, salt, iterations, keylen, expected) {
-  var actual = crypto.pbkdf2Sync(password, salt, iterations, keylen);
+  var actual = crypto.pbkdf2Sync(password, salt, iterations, keylen, 'sha1');
   assert.equal(actual.toString('binary'), expected);
 
-  crypto.pbkdf2(password, salt, iterations, keylen, function(err, actual) {
+  function assertCb(err, actual) {
     assert.equal(actual.toString('binary'), expected);
-  });
+  }
+
+  crypto.pbkdf2(password, salt, iterations, keylen, 'sha1', assertCb);
 }
 
 
@@ -62,28 +64,28 @@ assert.throws(function() {
 
 // Should not work with Infinity key length
 assert.throws(function() {
-  crypto.pbkdf2('password', 'salt', 1, Infinity, assert.fail);
+  crypto.pbkdf2('password', 'salt', 1, Infinity, 'sha1', assert.fail);
 }, function(err) {
   return err instanceof Error && err.message === 'Bad key length';
 });
 
 // Should not work with negative Infinity key length
 assert.throws(function() {
-  crypto.pbkdf2('password', 'salt', 1, -Infinity, assert.fail);
+  crypto.pbkdf2('password', 'salt', 1, -Infinity, 'sha1', assert.fail);
 }, function(err) {
   return err instanceof Error && err.message === 'Bad key length';
 });
 
 // Should not work with NaN key length
 assert.throws(function() {
-  crypto.pbkdf2('password', 'salt', 1, NaN, assert.fail);
+  crypto.pbkdf2('password', 'salt', 1, NaN, 'sha1', assert.fail);
 }, function(err) {
   return err instanceof Error && err.message === 'Bad key length';
 });
 
 // Should not work with negative key length
 assert.throws(function() {
-  crypto.pbkdf2('password', 'salt', 1, -1, assert.fail);
+  crypto.pbkdf2('password', 'salt', 1, -1, 'sha1', assert.fail);
 }, function(err) {
   return err instanceof Error && err.message === 'Bad key length';
 });

test/parallel/test-domain-crypto.js

@@ -14,4 +14,4 @@ crypto.randomBytes(8);
 crypto.randomBytes(8, function() {});
 crypto.pseudoRandomBytes(8);
 crypto.pseudoRandomBytes(8, function() {});
-crypto.pbkdf2('password', 'salt', 8, 8, function() {});
+crypto.pbkdf2('password', 'salt', 8, 8, 'sha1', function() {});