Skip to content

Commit

Permalink
crypto: upgrade pbkdf2 without digest to an error
Browse files Browse the repository at this point in the history
Commit a116358 added a deprecation warning when pbkdf2 was called without an
explicit `digest` argument. This was because the default digest is `sha1`,
which is not-recommended from a security point of view. This upgrades it
to a runtime error when `digest` is undefined per the plan discussed in
the original issue.

Ref: a116358

PR-URL: #11305
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
  • Loading branch information
jasnell committed Feb 13, 2017
1 parent 8bcc122 commit 9f74184
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 15 deletions.
9 changes: 6 additions & 3 deletions doc/api/deprecations.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,13 @@ to the `constants` property exposed by the relevant module. For instance,
<a id="DEP0009"></a>
### DEP0009: crypto.pbkdf2 without digest

Type: Runtime
Type: End-of-life

Use of the [`crypto.pbkdf2()`][] API without specifying a digest is deprecated.
Please specify a digest.
Use of the [`crypto.pbkdf2()`][] API without specifying a digest was deprecated
in Node.js 6.0 because the method defaulted to using the non-recommendend
`'SHA1'` digest. Previously, a deprecation warning was printed. Starting in
Node.js 8.0.0, calling `crypto.pbkdf2()` or `crypto.pbkdf2Sync()` with an
undefined `digest` will throw a `TypeError`.

<a id="DEP0010"></a>
### DEP0010: crypto.createCredentials
Expand Down
16 changes: 6 additions & 10 deletions lib/crypto.js
Original file line number Diff line number Diff line change
Expand Up @@ -537,11 +537,6 @@ ECDH.prototype.getPublicKey = function getPublicKey(encoding, format) {
};


const pbkdf2DeprecationWarning =
internalUtil.deprecate(() => {}, 'crypto.pbkdf2 without specifying' +
' a digest is deprecated. Please specify a digest', 'DEP0009');


exports.pbkdf2 = function(password,
salt,
iterations,
Expand All @@ -551,7 +546,6 @@ exports.pbkdf2 = function(password,
if (typeof digest === 'function') {
callback = digest;
digest = undefined;
pbkdf2DeprecationWarning();
}

if (typeof callback !== 'function')
Expand All @@ -562,15 +556,17 @@ exports.pbkdf2 = function(password,


exports.pbkdf2Sync = function(password, salt, iterations, keylen, digest) {
if (typeof digest === 'undefined') {
digest = undefined;
pbkdf2DeprecationWarning();
}
return pbkdf2(password, salt, iterations, keylen, digest);
};


function pbkdf2(password, salt, iterations, keylen, digest, callback) {

if (digest === undefined) {
throw new TypeError(
'The "digest" argument is required and must not be undefined');
}

password = toBuf(password);
salt = toBuf(salt);

Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-crypto-domains.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,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');
});
Expand Down
8 changes: 8 additions & 0 deletions test/parallel/test-crypto-pbkdf2.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,3 +96,11 @@ assert.doesNotThrow(() => {
assert.ifError(e);
}));
});

assert.throws(() => {
crypto.pbkdf2('password', 'salt', 8, 8, function() {});
}, /^TypeError: The "digest" argument is required and must not be undefined$/);

assert.throws(() => {
crypto.pbkdf2Sync('password', 'salt', 8, 8);
}, /^TypeError: The "digest" argument is required and must not be undefined$/);
2 changes: 1 addition & 1 deletion test/parallel/test-domain-crypto.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,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() {});

0 comments on commit 9f74184

Please sign in to comment.