Permalink
Browse files

crypto: refactor argument validation for pbkdf2

Move input argument validation to js, using internal/errors.

Also update docs

* `password` and `salt` may be Buffers or any TypedArrays
* `crypto.DEFAULT_ENCODING` changes the returned derivedKey type

PR-URL: #15746
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
  • Loading branch information...
jasnell committed Oct 3, 2017
1 parent 4eb9365 commit 7124b466d9c10e12b3bd9a59910032e033f35493
Showing with 198 additions and 81 deletions.
  1. +30 −6 doc/api/crypto.md
  2. +6 −0 doc/api/errors.md
  3. +40 −4 lib/internal/crypto/pbkdf2.js
  4. +1 −0 lib/internal/errors.js
  5. +1 −0 src/node_constants.cc
  6. +4 −44 src/node_crypto.cc
  7. +116 −27 test/parallel/test-crypto-pbkdf2.js
View
@@ -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()`][].
View
@@ -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.
<a id="ERR_CRYPTO_INVALID_DIGEST"></a>
### ERR_CRYPTO_INVALID_DIGEST
Used when an invalid [crypto digest algorithm][] is specified.
<a id="ERR_DNS_SET_SERVERS_FAILED"></a>
### 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
@@ -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);
}
}
View
@@ -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}]`);
View
@@ -1180,6 +1180,7 @@ void DefineCryptoConstants(Local<Object> target) {
"defaultCipherList",
default_cipher_list);
#endif
NODE_DEFINE_CONSTANT(target, INT_MAX);
}
void DefineZlibConstants(Local<Object> target) {
View
@@ -5330,7 +5330,6 @@ void PBKDF2(const FunctionCallbackInfo<Value>& 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,63 +5340,30 @@ void PBKDF2(const FunctionCallbackInfo<Value>& args) {
PBKDF2Request* req = nullptr;
Local<Object> 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<int>(raw_keylen);
if (args[4]->IsString()) {
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<Value>& args) {
else
args.GetReturnValue().Set(argv[1]);
}
return;
err:
free(salt);
free(pass);
return env->ThrowTypeError(type_error);
}
Oops, something went wrong.

0 comments on commit 7124b46

Please sign in to comment.