diff --git a/doc/api/crypto.md b/doc/api/crypto.md index a9a96267991b96..fa0804b86fe15c 100644 --- a/doc/api/crypto.md +++ b/doc/api/crypto.md @@ -1587,8 +1587,8 @@ changes: description: The default encoding for `password` if it is a string changed from `binary` to `utf8`. --> -- `password` {string} -- `salt` {string} +- `password` {string|Buffer|TypedArray} +- `salt` {string|Buffer|TypedArray} - `iterations` {number} - `keylen` {number} - `digest` {string} @@ -1602,8 +1602,10 @@ applied to derive a key of the requested byte length (`keylen`) from the `password`, `salt` and `iterations`. The supplied `callback` function is called with two arguments: `err` and -`derivedKey`. If an error occurs, `err` will be set; otherwise `err` will be -null. The successfully generated `derivedKey` will be passed as a [`Buffer`][]. +`derivedKey`. If an error occurs while deriving the key, `err` will be set; +otherwise `err` will be null. By default, the successfully generated +`derivedKey` will be passed to the callback as a [`Buffer`][]. An error will be +thrown if any of the input arguments specify invalid values or types. The `iterations` argument must be a number set as high as possible. The higher the number of iterations, the more secure the derived key will be, @@ -1623,6 +1625,18 @@ crypto.pbkdf2('secret', 'salt', 100000, 64, 'sha512', (err, derivedKey) => { }); ``` +The `crypto.DEFAULT_ENCODING` may be used to change the way the `derivedKey` +is passed to the callback: + +```js +const crypto = require('crypto'); +crypto.DEFAULT_ENCODING = 'hex'; +crypto.pbkdf2('secret', 'salt', 100000, 512, 'sha512', (err, derivedKey) => { + if (err) throw err; + console.log(derivedKey); // '3745e48...aa39b34' +}); +``` + An array of supported digest functions can be retrieved using [`crypto.getHashes()`][]. @@ -1643,8 +1657,8 @@ changes: description: The default encoding for `password` if it is a string changed from `binary` to `utf8`. --> -- `password` {string} -- `salt` {string} +- `password` {string|Buffer|TypedArray} +- `salt` {string|Buffer|TypedArray} - `iterations` {number} - `keylen` {number} - `digest` {string} @@ -1673,6 +1687,16 @@ const key = crypto.pbkdf2Sync('secret', 'salt', 100000, 64, 'sha512'); console.log(key.toString('hex')); // '3745e48...08d59ae' ``` +The `crypto.DEFAULT_ENCODING` may be used to change the way the `derivedKey` +is returned: + +```js +const crypto = require('crypto'); +crypto.DEFAULT_ENCODING = 'hex'; +const key = crypto.pbkdf2Sync('secret', 'salt', 100000, 512, 'sha512'); +console.log(key); // '3745e48...aa39b34' +``` + An array of supported digest functions can be retrieved using [`crypto.getHashes()`][]. diff --git a/doc/api/errors.md b/doc/api/errors.md index 5b8fb7875a7e4b..b78c6b36d634db 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -637,6 +637,11 @@ Used when the native call from `process.cpuUsage` cannot be processed properly. Used when an invalid value for the `format` argument has been passed to the `crypto.ECDH()` class `getPublicKey()` method. + +### ERR_CRYPTO_INVALID_DIGEST + +Used when an invalid [crypto digest algorithm][] is specified. + ### ERR_DNS_SET_SERVERS_FAILED @@ -1355,6 +1360,7 @@ closed. [Node.js Error Codes]: #nodejs-error-codes [V8's stack trace API]: https://github.com/v8/v8/wiki/Stack-Trace-API [WHATWG URL API]: url.html#url_the_whatwg_url_api +[crypto digest algorithm]: crypto.html#crypto_crypto_gethashes [domains]: domain.html [event emitter-based]: events.html#events_class_eventemitter [file descriptors]: https://en.wikipedia.org/wiki/File_descriptor diff --git a/lib/internal/crypto/pbkdf2.js b/lib/internal/crypto/pbkdf2.js index 5398321ece2aef..2fc211a87d7635 100644 --- a/lib/internal/crypto/pbkdf2.js +++ b/lib/internal/crypto/pbkdf2.js @@ -5,9 +5,13 @@ const { getDefaultEncoding, toBuf } = require('internal/crypto/util'); +const { isArrayBufferView } = require('internal/util/types'); const { PBKDF2 } = process.binding('crypto'); +const { + INT_MAX +} = process.binding('constants').crypto; function pbkdf2(password, salt, iterations, keylen, digest, callback) { if (typeof digest === 'function') { @@ -34,10 +38,39 @@ function _pbkdf2(password, salt, iterations, keylen, digest, callback) { password = toBuf(password); salt = toBuf(salt); + if (!isArrayBufferView(password)) { + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'password', + ['string', 'Buffer', 'TypedArray']); + } + + if (!isArrayBufferView(salt)) { + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'salt', + ['string', 'Buffer', 'TypedArray']); + } + + if (typeof iterations !== 'number') + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'iterations', 'number'); + + if (iterations < 0) + throw new errors.RangeError('ERR_OUT_OF_RANGE', 'iterations'); + + if (typeof keylen !== 'number') + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'keylen', 'number'); + + if (keylen < 0 || + !Number.isFinite(keylen) || + keylen > INT_MAX) { + throw new errors.RangeError('ERR_OUT_OF_RANGE', 'keylen'); + } + const encoding = getDefaultEncoding(); - if (encoding === 'buffer') - return PBKDF2(password, salt, iterations, keylen, digest, callback); + if (encoding === 'buffer') { + const ret = PBKDF2(password, salt, iterations, keylen, digest, callback); + if (ret === -1) + throw new errors.TypeError('ERR_CRYPTO_INVALID_DIGEST', digest); + return ret; + } // at this point, we need to handle encodings. if (callback) { @@ -46,9 +79,12 @@ function _pbkdf2(password, salt, iterations, keylen, digest, callback) { ret = ret.toString(encoding); callback(er, ret); } - PBKDF2(password, salt, iterations, keylen, digest, next); + if (PBKDF2(password, salt, iterations, keylen, digest, next) === -1) + throw new errors.TypeError('ERR_CRYPTO_INVALID_DIGEST', digest); } else { - var ret = PBKDF2(password, salt, iterations, keylen, digest); + const ret = PBKDF2(password, salt, iterations, keylen, digest); + if (ret === -1) + throw new errors.TypeError('ERR_CRYPTO_INVALID_DIGEST', digest); return ret.toString(encoding); } } diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 975ae13d536661..a719570841e090 100755 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -157,6 +157,7 @@ E('ERR_CRYPTO_ECDH_INVALID_FORMAT', 'Invalid ECDH format: %s'); E('ERR_CRYPTO_HASH_DIGEST_NO_UTF16', 'hash.digest() does not support UTF-16'); E('ERR_CRYPTO_HASH_FINALIZED', 'Digest already called'); E('ERR_CRYPTO_HASH_UPDATE_FAILED', 'Hash update failed'); +E('ERR_CRYPTO_INVALID_DIGEST', 'Invalid digest: %s'); E('ERR_CRYPTO_SIGN_KEY_REQUIRED', 'No key provided to sign'); E('ERR_DNS_SET_SERVERS_FAILED', (err, servers) => `c-ares failed to set servers: "${err}" [${servers}]`); diff --git a/src/node_constants.cc b/src/node_constants.cc index 787f44a1f1643e..ba33d65d1dc087 100644 --- a/src/node_constants.cc +++ b/src/node_constants.cc @@ -1180,6 +1180,7 @@ void DefineCryptoConstants(Local target) { "defaultCipherList", default_cipher_list); #endif + NODE_DEFINE_CONSTANT(target, INT_MAX); } void DefineZlibConstants(Local target) { diff --git a/src/node_crypto.cc b/src/node_crypto.cc index fa4815a9890299..3e8dd4205f2c5e 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -5330,7 +5330,6 @@ void PBKDF2(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); const EVP_MD* digest = nullptr; - const char* type_error = nullptr; char* pass = nullptr; char* salt = nullptr; int passlen = -1; @@ -5341,54 +5340,19 @@ void PBKDF2(const FunctionCallbackInfo& args) { PBKDF2Request* req = nullptr; Local obj; - if (args.Length() != 5 && args.Length() != 6) { - type_error = "Bad parameter"; - goto err; - } - - THROW_AND_RETURN_IF_NOT_BUFFER(args[0], "Pass phrase"); passlen = Buffer::Length(args[0]); - if (passlen < 0) { - type_error = "Bad password"; - goto err; - } - - THROW_AND_RETURN_IF_NOT_BUFFER(args[1], "Salt"); pass = node::Malloc(passlen); memcpy(pass, Buffer::Data(args[0]), passlen); saltlen = Buffer::Length(args[1]); - if (saltlen < 0) { - type_error = "Bad salt"; - goto err; - } salt = node::Malloc(saltlen); memcpy(salt, Buffer::Data(args[1]), saltlen); - if (!args[2]->IsNumber()) { - type_error = "Iterations not a number"; - goto err; - } - iter = args[2]->Int32Value(); - if (iter < 0) { - type_error = "Bad iterations"; - goto err; - } - - if (!args[3]->IsNumber()) { - type_error = "Key length not a number"; - goto err; - } raw_keylen = args[3]->NumberValue(); - if (raw_keylen < 0.0 || isnan(raw_keylen) || isinf(raw_keylen) || - raw_keylen > INT_MAX) { - type_error = "Bad key length"; - goto err; - } keylen = static_cast(raw_keylen); @@ -5396,8 +5360,10 @@ void PBKDF2(const FunctionCallbackInfo& args) { node::Utf8Value digest_name(env->isolate(), args[4]); digest = EVP_get_digestbyname(*digest_name); if (digest == nullptr) { - type_error = "Bad digest name"; - goto err; + free(salt); + free(pass); + args.GetReturnValue().Set(-1); + return; } } @@ -5443,12 +5409,6 @@ void PBKDF2(const FunctionCallbackInfo& args) { else args.GetReturnValue().Set(argv[1]); } - return; - - err: - free(salt); - free(pass); - return env->ThrowTypeError(type_error); } diff --git a/test/parallel/test-crypto-pbkdf2.js b/test/parallel/test-crypto-pbkdf2.js index c495b9306fd471..3cefa84e712a49 100644 --- a/test/parallel/test-crypto-pbkdf2.js +++ b/test/parallel/test-crypto-pbkdf2.js @@ -6,6 +6,8 @@ if (!common.hasCrypto) const assert = require('assert'); const crypto = require('crypto'); +const { INT_MAX } = process.binding('constants').crypto; + // // Test PBKDF2 with RFC 6070 test vectors (except #4) // @@ -63,33 +65,17 @@ common.expectsError( } ); -// Should not work with Infinity key length -assert.throws(function() { - crypto.pbkdf2('password', 'salt', 1, Infinity, 'sha256', - common.mustNotCall()); -}, /^TypeError: Bad key length$/); - -// Should not work with negative Infinity key length -assert.throws(function() { - crypto.pbkdf2('password', 'salt', 1, -Infinity, 'sha256', - common.mustNotCall()); -}, /^TypeError: Bad key length$/); - -// Should not work with NaN key length -assert.throws(function() { - crypto.pbkdf2('password', 'salt', 1, NaN, 'sha256', common.mustNotCall()); -}, /^TypeError: Bad key length$/); - -// Should not work with negative key length -assert.throws(function() { - crypto.pbkdf2('password', 'salt', 1, -1, 'sha256', common.mustNotCall()); -}, /^TypeError: Bad key length$/); - -// Should not work with key length that does not fit into 32 signed bits -assert.throws(function() { - crypto.pbkdf2('password', 'salt', 1, 4073741824, 'sha256', - common.mustNotCall()); -}, /^TypeError: Bad key length$/); +[Infinity, -Infinity, NaN, -1, 4073741824, INT_MAX + 1].forEach((i) => { + common.expectsError( + () => { + crypto.pbkdf2('password', 'salt', 1, i, 'sha256', + common.mustNotCall()); + }, { + code: 'ERR_OUT_OF_RANGE', + type: RangeError, + message: 'The "keylen" argument is out of range' + }); +}); // Should not get FATAL ERROR with empty password and salt // https://github.com/nodejs/node/issues/8571 @@ -114,3 +100,106 @@ common.expectsError( type: TypeError, message: 'The "digest" argument must be one of type string or null' }); + +[1, {}, [], true, undefined, null].forEach((i) => { + common.expectsError( + () => crypto.pbkdf2(i, 'salt', 8, 8, 'sha256', common.mustNotCall()), + { + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: 'The "password" argument must be one of type string, ' + + 'Buffer, or TypedArray' + } + ); + + common.expectsError( + () => crypto.pbkdf2('pass', i, 8, 8, 'sha256', common.mustNotCall()), + { + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: 'The "salt" argument must be one of type string, ' + + 'Buffer, or TypedArray' + } + ); + + common.expectsError( + () => crypto.pbkdf2Sync(i, 'salt', 8, 8, 'sha256'), + { + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: 'The "password" argument must be one of type string, ' + + 'Buffer, or TypedArray' + } + ); + + common.expectsError( + () => crypto.pbkdf2Sync('pass', i, 8, 8, 'sha256'), + { + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: 'The "salt" argument must be one of type string, ' + + 'Buffer, or TypedArray' + } + ); +}); + +['test', {}, [], true, undefined, null].forEach((i) => { + common.expectsError( + () => crypto.pbkdf2('pass', 'salt', i, 8, 'sha256', common.mustNotCall()), + { + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: 'The "iterations" argument must be of type number' + } + ); + + common.expectsError( + () => crypto.pbkdf2Sync('pass', 'salt', i, 8, 'sha256'), + { + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: 'The "iterations" argument must be of type number' + } + ); +}); + +// Any TypedArray should work for password and salt +crypto.pbkdf2(new Uint8Array(10), 'salt', 8, 8, 'sha256', common.mustCall()); +crypto.pbkdf2('pass', new Uint8Array(10), 8, 8, 'sha256', common.mustCall()); +crypto.pbkdf2(new Uint16Array(10), 'salt', 8, 8, 'sha256', common.mustCall()); +crypto.pbkdf2('pass', new Uint16Array(10), 8, 8, 'sha256', common.mustCall()); +crypto.pbkdf2(new Uint32Array(10), 'salt', 8, 8, 'sha256', common.mustCall()); +crypto.pbkdf2('pass', new Uint32Array(10), 8, 8, 'sha256', common.mustCall()); +crypto.pbkdf2(new Float32Array(10), 'salt', 8, 8, 'sha256', common.mustCall()); +crypto.pbkdf2('pass', new Float32Array(10), 8, 8, 'sha256', common.mustCall()); +crypto.pbkdf2(new Float64Array(10), 'salt', 8, 8, 'sha256', common.mustCall()); +crypto.pbkdf2('pass', new Float64Array(10), 8, 8, 'sha256', common.mustCall()); + +crypto.pbkdf2Sync(new Uint8Array(10), 'salt', 8, 8, 'sha256'); +crypto.pbkdf2Sync('pass', new Uint8Array(10), 8, 8, 'sha256'); +crypto.pbkdf2Sync(new Uint16Array(10), 'salt', 8, 8, 'sha256'); +crypto.pbkdf2Sync('pass', new Uint16Array(10), 8, 8, 'sha256'); +crypto.pbkdf2Sync(new Uint32Array(10), 'salt', 8, 8, 'sha256'); +crypto.pbkdf2Sync('pass', new Uint32Array(10), 8, 8, 'sha256'); +crypto.pbkdf2Sync(new Float32Array(10), 'salt', 8, 8, 'sha256'); +crypto.pbkdf2Sync('pass', new Float32Array(10), 8, 8, 'sha256'); +crypto.pbkdf2Sync(new Float64Array(10), 'salt', 8, 8, 'sha256'); +crypto.pbkdf2Sync('pass', new Float64Array(10), 8, 8, 'sha256'); + +common.expectsError( + () => crypto.pbkdf2('pass', 'salt', 8, 8, 'md55', common.mustNotCall()), + { + code: 'ERR_CRYPTO_INVALID_DIGEST', + type: TypeError, + message: 'Invalid digest: md55' + } +); + +common.expectsError( + () => crypto.pbkdf2Sync('pass', 'salt', 8, 8, 'md55'), + { + code: 'ERR_CRYPTO_INVALID_DIGEST', + type: TypeError, + message: 'Invalid digest: md55' + } +);