From 91e9c7ab0415428d3050c845178affd8ca95a4d4 Mon Sep 17 00:00:00 2001 From: Tom Gallacher Date: Fri, 9 Oct 2015 14:39:38 +0100 Subject: [PATCH] crypto: pbkdf2 digest parameter is now required This change will remove the default behaviour of defaulting to SHA1 as its digest function, instead this will break backwards compatibility and force implementors to choose a secure digest that is suited for them at that time. --- doc/api/crypto.markdown | 8 ++++---- lib/crypto.js | 6 +++--- src/node_crypto.cc | 3 ++- test/parallel/test-crypto-binary-default.js | 8 +++++--- test/parallel/test-crypto-domains.js | 2 +- test/parallel/test-crypto-pbkdf2.js | 16 +++++++++------- test/parallel/test-domain-crypto.js | 2 +- 7 files changed, 25 insertions(+), 20 deletions(-) diff --git a/doc/api/crypto.markdown b/doc/api/crypto.markdown index 0379179de3593c..834632d48d6acb 100644 --- a/doc/api/crypto.markdown +++ b/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: diff --git a/lib/crypto.js b/lib/crypto.js index b32d9aff90b72b..6db77b32155c31 100644 --- a/lib/crypto.js +++ b/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') diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 6d5403b563118f..d786f178833d53 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -4831,7 +4831,8 @@ void PBKDF2(const FunctionCallbackInfo& args) { } if (digest == nullptr) { - digest = EVP_sha1(); + type_error = "Bad digest name"; + goto err; } obj = env->NewInternalFieldObject(); diff --git a/test/parallel/test-crypto-binary-default.js b/test/parallel/test-crypto-binary-default.js index bebc91dc8ed0b1..8ec04ec4158cbe 100644 --- a/test/parallel/test-crypto-binary-default.js +++ b/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); } diff --git a/test/parallel/test-crypto-domains.js b/test/parallel/test-crypto-domains.js index ca93d20fdf25b3..9c60441f83316d 100644 --- a/test/parallel/test-crypto-domains.js +++ b/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'); }); diff --git a/test/parallel/test-crypto-pbkdf2.js b/test/parallel/test-crypto-pbkdf2.js index 51759ca8357ee7..9244a3ed54c6ce 100644 --- a/test/parallel/test-crypto-pbkdf2.js +++ b/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'; }); diff --git a/test/parallel/test-domain-crypto.js b/test/parallel/test-domain-crypto.js index e76e8d08791c87..a7b7562c4f1bb3 100644 --- a/test/parallel/test-domain-crypto.js +++ b/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() {});