Skip to content

Commit

Permalink
crypto: increase maxmem range from 32 to 53 bits
Browse files Browse the repository at this point in the history
Fixes: #28755

Backport-PR-URL: #29316
PR-URL: #28799
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
tniessen authored and BethGriggs committed Oct 1, 2019
1 parent a3eda28 commit 7735824
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 8 deletions.
6 changes: 6 additions & 0 deletions doc/api/crypto.md
Expand Up @@ -2352,6 +2352,9 @@ request.
<!-- YAML
added: v10.5.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/28799
description: The `maxmem` value can now be any safe integer.
- version: v10.9.0
pr-url: https://github.com/nodejs/node/pull/21525
description: The `cost`, `blockSize` and `parallelization` option names
Expand Down Expand Up @@ -2407,6 +2410,9 @@ crypto.scrypt('secret', 'salt', 64, { N: 1024 }, (err, derivedKey) => {
<!-- YAML
added: v10.5.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/28799
description: The `maxmem` value can now be any safe integer.
- version: v10.9.0
pr-url: https://github.com/nodejs/node/pull/21525
description: The `cost`, `blockSize` and `parallelization` option names
Expand Down
12 changes: 7 additions & 5 deletions lib/internal/crypto/scrypt.js
Expand Up @@ -2,12 +2,12 @@

const { AsyncWrap, Providers } = internalBinding('async_wrap');
const { Buffer } = require('buffer');
const { scrypt: _scrypt } = process.binding('crypto');
const { validateUint32 } = require('internal/validators');
const { scrypt: _scrypt } = internalBinding('crypto');
const { validateInteger, validateUint32 } = require('internal/validators');
const {
ERR_CRYPTO_SCRYPT_INVALID_PARAMETER,
ERR_CRYPTO_SCRYPT_NOT_SUPPORTED,
ERR_INVALID_CALLBACK,
ERR_INVALID_CALLBACK
} = require('internal/errors').codes;
const {
getDefaultEncoding,
Expand Down Expand Up @@ -99,8 +99,10 @@ function check(password, salt, keylen, options) {
if (has_p) throw new ERR_CRYPTO_SCRYPT_INVALID_PARAMETER();
p = validateUint32(options.parallelization, 'parallelization');
}
if (options.maxmem !== undefined)
maxmem = validateUint32(options.maxmem, 'maxmem');
if (options.maxmem !== undefined) {
maxmem = options.maxmem;
validateInteger(maxmem, 'maxmem', 0);
}
if (N === 0) N = defaults.N;
if (r === 0) r = defaults.r;
if (p === 0) p = defaults.p;
Expand Down
7 changes: 4 additions & 3 deletions src/node_crypto.cc
Expand Up @@ -4754,7 +4754,7 @@ struct ScryptJob : public CryptoJob {
uint32_t N;
uint32_t r;
uint32_t p;
uint32_t maxmem;
uint64_t maxmem;
CryptoErrorVector errors;

inline explicit ScryptJob(Environment* env) : CryptoJob(env) {}
Expand Down Expand Up @@ -4809,7 +4809,7 @@ void Scrypt(const FunctionCallbackInfo<Value>& args) {
CHECK(args[3]->IsUint32()); // N
CHECK(args[4]->IsUint32()); // r
CHECK(args[5]->IsUint32()); // p
CHECK(args[6]->IsUint32()); // maxmem
CHECK(args[6]->IsNumber()); // maxmem
CHECK(args[7]->IsObject() || args[7]->IsUndefined()); // wrap object
std::unique_ptr<ScryptJob> job(new ScryptJob(env));
job->keybuf_data = reinterpret_cast<unsigned char*>(Buffer::Data(args[0]));
Expand All @@ -4819,7 +4819,8 @@ void Scrypt(const FunctionCallbackInfo<Value>& args) {
job->N = args[3].As<Uint32>()->Value();
job->r = args[4].As<Uint32>()->Value();
job->p = args[5].As<Uint32>()->Value();
job->maxmem = args[6].As<Uint32>()->Value();
Local<Context> ctx = env->isolate()->GetCurrentContext();
job->maxmem = static_cast<uint64_t>(args[6]->IntegerValue(ctx).ToChecked());
if (!job->Validate()) {
// EVP_PBE_scrypt() does not always put errors on the error stack
// and therefore ToResult() may or may not return an exception
Expand Down
15 changes: 15 additions & 0 deletions test/parallel/test-crypto-scrypt.js
Expand Up @@ -217,3 +217,18 @@ for (const { args, expected } of badargs) {
common.expectsError(() => crypto.scrypt('', '', 42, {}), expected);
common.expectsError(() => crypto.scrypt('', '', 42, {}, {}), expected);
}

{
// Values for maxmem that do not fit in 32 bits but that are still safe
// integers should be allowed.
crypto.scrypt('', '', 4, { maxmem: 2 ** 52 },
common.mustCall((err, actual) => {
assert.ifError(err);
assert.strictEqual(actual.toString('hex'), 'd72c87d0');
}));

// Values that exceed Number.isSafeInteger should not be allowed.
common.expectsError(() => crypto.scryptSync('', '', 0, { maxmem: 2 ** 53 }), {
code: 'ERR_OUT_OF_RANGE'
});
}

0 comments on commit 7735824

Please sign in to comment.