FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

crypto: refactor argument validation for pbkdf2 · nodejs/node@7124b46 · GitHub

/ node Public

Commit 7124b46

Browse files
jasnell
committed
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>
1 parent 4eb9365 commit 7124b46

7 files changed

Lines changed: 198 additions & 81 deletions

File tree

‎doc/api/crypto.md‎

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1587,8 +1587,8 @@ changes:
15871587
description: The default encoding for `password` if it is a string changed
15881588
from `binary` to `utf8`.
15891589
-->
1590-
- `password` {string}
1591-
- `salt` {string}
1590+
- `password` {string|Buffer|TypedArray}
1591+
- `salt` {string|Buffer|TypedArray}
15921592
- `iterations` {number}
15931593
- `keylen` {number}
15941594
- `digest` {string}
@@ -1602,8 +1602,10 @@ applied to derive a key of the requested byte length (`keylen`) from the
16021602
`password`, `salt` and `iterations`.
16031603

16041604
The supplied `callback` function is called with two arguments: `err` and
1605-
`derivedKey`. If an error occurs, `err` will be set; otherwise `err` will be
1606-
null. The successfully generated `derivedKey` will be passed as a [`Buffer`][].
1605+
`derivedKey`. If an error occurs while deriving the key, `err` will be set;
1606+
otherwise `err` will be null. By default, the successfully generated
1607+
`derivedKey` will be passed to the callback as a [`Buffer`][]. An error will be
1608+
thrown if any of the input arguments specify invalid values or types.
16071609

16081610
The `iterations` argument must be a number set as high as possible. The
16091611
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) => {
16231625
});
16241626
```
16251627

1628+
The `crypto.DEFAULT_ENCODING` may be used to change the way the `derivedKey`
1629+
is passed to the callback:
1630+
1631+
```js
1632+
const crypto = require('crypto');
1633+
crypto.DEFAULT_ENCODING = 'hex';
1634+
crypto.pbkdf2('secret', 'salt', 100000, 512, 'sha512', (err, derivedKey) => {
1635+
if (err) throw err;
1636+
console.log(derivedKey); // '3745e48...aa39b34'
1637+
});
1638+
```
1639+
16261640
An array of supported digest functions can be retrieved using
16271641
[`crypto.getHashes()`][].
16281642

@@ -1643,8 +1657,8 @@ changes:
16431657
description: The default encoding for `password` if it is a string changed
16441658
from `binary` to `utf8`.
16451659
-->
1646-
- `password` {string}
1647-
- `salt` {string}
1660+
- `password` {string|Buffer|TypedArray}
1661+
- `salt` {string|Buffer|TypedArray}
16481662
- `iterations` {number}
16491663
- `keylen` {number}
16501664
- `digest` {string}
@@ -1673,6 +1687,16 @@ const key = crypto.pbkdf2Sync('secret', 'salt', 100000, 64, 'sha512');
16731687
console.log(key.toString('hex')); // '3745e48...08d59ae'
16741688
```
16751689

1690+
The `crypto.DEFAULT_ENCODING` may be used to change the way the `derivedKey`
1691+
is returned:
1692+
1693+
```js
1694+
const crypto = require('crypto');
1695+
crypto.DEFAULT_ENCODING = 'hex';
1696+
const key = crypto.pbkdf2Sync('secret', 'salt', 100000, 512, 'sha512');
1697+
console.log(key); // '3745e48...aa39b34'
1698+
```
1699+
16761700
An array of supported digest functions can be retrieved using
16771701
[`crypto.getHashes()`][].
16781702

‎doc/api/errors.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -637,6 +637,11 @@ Used when the native call from `process.cpuUsage` cannot be processed properly.
637637
Used when an invalid value for the `format` argument has been passed to the
638638
`crypto.ECDH()` class `getPublicKey()` method.
639639

640+
<a id="ERR_CRYPTO_INVALID_DIGEST"></a>
641+
### ERR_CRYPTO_INVALID_DIGEST
642+
643+
Used when an invalid [crypto digest algorithm][] is specified.
644+
640645
<a id="ERR_DNS_SET_SERVERS_FAILED"></a>
641646
### ERR_DNS_SET_SERVERS_FAILED
642647

@@ -1355,6 +1360,7 @@ closed.
13551360
[Node.js Error Codes]: #nodejs-error-codes
13561361
[V8's stack trace API]: https://github.com/v8/v8/wiki/Stack-Trace-API
13571362
[WHATWG URL API]: url.html#url_the_whatwg_url_api
1363+
[crypto digest algorithm]: crypto.html#crypto_crypto_gethashes
13581364
[domains]: domain.html
13591365
[event emitter-based]: events.html#events_class_eventemitter
13601366
[file descriptors]: https://en.wikipedia.org/wiki/File_descriptor

‎lib/internal/crypto/pbkdf2.js‎

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,13 @@ const {
55
getDefaultEncoding,
66
toBuf
77
} = require('internal/crypto/util');
8+
const { isArrayBufferView } = require('internal/util/types');
89
const {
910
PBKDF2
1011
} = process.binding('crypto');
12+
const {
13+
INT_MAX
14+
} = process.binding('constants').crypto;
1115

1216
function pbkdf2(password, salt, iterations, keylen, digest, callback) {
1317
if (typeof digest === 'function') {
@@ -34,10 +38,39 @@ function _pbkdf2(password, salt, iterations, keylen, digest, callback) {
3438
password = toBuf(password);
3539
salt = toBuf(salt);
3640

41+
if (!isArrayBufferView(password)) {
42+
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'password',
43+
['string', 'Buffer', 'TypedArray']);
44+
}
45+
46+
if (!isArrayBufferView(salt)) {
47+
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'salt',
48+
['string', 'Buffer', 'TypedArray']);
49+
}
50+
51+
if (typeof iterations !== 'number')
52+
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'iterations', 'number');
53+
54+
if (iterations < 0)
55+
throw new errors.RangeError('ERR_OUT_OF_RANGE', 'iterations');
56+
57+
if (typeof keylen !== 'number')
58+
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'keylen', 'number');
59+
60+
if (keylen < 0 ||
61+
!Number.isFinite(keylen) ||
62+
keylen > INT_MAX) {
63+
throw new errors.RangeError('ERR_OUT_OF_RANGE', 'keylen');
64+
}
65+
3766
const encoding = getDefaultEncoding();
3867

39-
if (encoding === 'buffer')
40-
return PBKDF2(password, salt, iterations, keylen, digest, callback);
68+
if (encoding === 'buffer') {
69+
const ret = PBKDF2(password, salt, iterations, keylen, digest, callback);
70+
if (ret === -1)
71+
throw new errors.TypeError('ERR_CRYPTO_INVALID_DIGEST', digest);
72+
return ret;
73+
}
4174

4275
// at this point, we need to handle encodings.
4376
if (callback) {
@@ -46,9 +79,12 @@ function _pbkdf2(password, salt, iterations, keylen, digest, callback) {
4679
ret = ret.toString(encoding);
4780
callback(er, ret);
4881
}
49-
PBKDF2(password, salt, iterations, keylen, digest, next);
82+
if (PBKDF2(password, salt, iterations, keylen, digest, next) === -1)
83+
throw new errors.TypeError('ERR_CRYPTO_INVALID_DIGEST', digest);
5084
} else {
51-
var ret = PBKDF2(password, salt, iterations, keylen, digest);
85+
const ret = PBKDF2(password, salt, iterations, keylen, digest);
86+
if (ret === -1)
87+
throw new errors.TypeError('ERR_CRYPTO_INVALID_DIGEST', digest);
5288
return ret.toString(encoding);
5389
}
5490
}

‎lib/internal/errors.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ E('ERR_CRYPTO_ECDH_INVALID_FORMAT', 'Invalid ECDH format: %s');
157157
E('ERR_CRYPTO_HASH_DIGEST_NO_UTF16', 'hash.digest() does not support UTF-16');
158158
E('ERR_CRYPTO_HASH_FINALIZED', 'Digest already called');
159159
E('ERR_CRYPTO_HASH_UPDATE_FAILED', 'Hash update failed');
160+
E('ERR_CRYPTO_INVALID_DIGEST', 'Invalid digest: %s');
160161
E('ERR_CRYPTO_SIGN_KEY_REQUIRED', 'No key provided to sign');
161162
E('ERR_DNS_SET_SERVERS_FAILED', (err, servers) =>
162163
`c-ares failed to set servers: "${err}" [${servers}]`);

‎src/node_constants.cc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1180,6 +1180,7 @@ void DefineCryptoConstants(Local<Object> target) {
11801180
"defaultCipherList",
11811181
default_cipher_list);
11821182
#endif
1183+
NODE_DEFINE_CONSTANT(target, INT_MAX);
11831184
}
11841185

11851186
void DefineZlibConstants(Local<Object> target) {

‎src/node_crypto.cc‎

Lines changed: 4 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -5330,7 +5330,6 @@ void PBKDF2(const FunctionCallbackInfo<Value>& args) {
53305330
Environment* env = Environment::GetCurrent(args);
53315331

53325332
const EVP_MD* digest = nullptr;
5333-
const char* type_error = nullptr;
53345333
char* pass = nullptr;
53355334
char* salt = nullptr;
53365335
int passlen = -1;
@@ -5341,63 +5340,30 @@ void PBKDF2(const FunctionCallbackInfo<Value>& args) {
53415340
PBKDF2Request* req = nullptr;
53425341
Local<Object> obj;
53435342

5344-
if (args.Length() != 5 && args.Length() != 6) {
5345-
type_error = "Bad parameter";
5346-
goto err;
5347-
}
5348-
5349-
THROW_AND_RETURN_IF_NOT_BUFFER(args[0], "Pass phrase");
53505343
passlen = Buffer::Length(args[0]);
5351-
if (passlen < 0) {
5352-
type_error = "Bad password";
5353-
goto err;
5354-
}
5355-
5356-
THROW_AND_RETURN_IF_NOT_BUFFER(args[1], "Salt");
53575344

53585345
pass = node::Malloc(passlen);
53595346
memcpy(pass, Buffer::Data(args[0]), passlen);
53605347

53615348
saltlen = Buffer::Length(args[1]);
5362-
if (saltlen < 0) {
5363-
type_error = "Bad salt";
5364-
goto err;
5365-
}
53665349

53675350
salt = node::Malloc(saltlen);
53685351
memcpy(salt, Buffer::Data(args[1]), saltlen);
53695352

5370-
if (!args[2]->IsNumber()) {
5371-
type_error = "Iterations not a number";
5372-
goto err;
5373-
}
5374-
53755353
iter = args[2]->Int32Value();
5376-
if (iter < 0) {
5377-
type_error = "Bad iterations";
5378-
goto err;
5379-
}
5380-
5381-
if (!args[3]->IsNumber()) {
5382-
type_error = "Key length not a number";
5383-
goto err;
5384-
}
53855354

53865355
raw_keylen = args[3]->NumberValue();
5387-
if (raw_keylen < 0.0 || isnan(raw_keylen) || isinf(raw_keylen) ||
5388-
raw_keylen > INT_MAX) {
5389-
type_error = "Bad key length";
5390-
goto err;
5391-
}
53925356

53935357
keylen = static_cast<int>(raw_keylen);
53945358

53955359
if (args[4]->IsString()) {
53965360
node::Utf8Value digest_name(env->isolate(), args[4]);
53975361
digest = EVP_get_digestbyname(*digest_name);
53985362
if (digest == nullptr) {
5399-
type_error = "Bad digest name";
5400-
goto err;
5363+
free(salt);
5364+
free(pass);
5365+
args.GetReturnValue().Set(-1);
5366+
return;
54015367
}
54025368
}
54035369

@@ -5443,12 +5409,6 @@ void PBKDF2(const FunctionCallbackInfo<Value>& args) {
54435409
else
54445410
args.GetReturnValue().Set(argv[1]);
54455411
}
5446-
return;
5447-
5448-
err:
5449-
free(salt);
5450-
free(pass);
5451-
return env->ThrowTypeError(type_error);
54525412
}
54535413

54545414

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL