Skip to content

Commit

Permalink
crypto: fixup scrypt regressions
Browse files Browse the repository at this point in the history
Fixes a handful of regressions in scrypt support following
the refactor.

Fixes: #35815

PR-URL: #35821
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
jasnell authored and targos committed Nov 3, 2020
1 parent 8c2b179 commit 79a8fb6
Show file tree
Hide file tree
Showing 8 changed files with 18 additions and 16 deletions.
2 changes: 1 addition & 1 deletion lib/internal/crypto/webcrypto.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ async function deriveBits(algorithm, baseKey, length) {
.pbkdf2DeriveBits(algorithm, baseKey, length);
case 'NODE-SCRYPT':
return lazyRequire('internal/crypto/scrypt')
.asyncScryptDeriveBits(algorithm, baseKey, length);
.scryptDeriveBits(algorithm, baseKey, length);
case 'NODE-DH':
return lazyRequire('internal/crypto/diffiehellman')
.asyncDeriveBitsDH(algorithm, baseKey, length);
Expand Down
3 changes: 2 additions & 1 deletion src/crypto/crypto_scrypt.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ ScryptConfig::ScryptConfig(ScryptConfig&& other) noexcept
N(other.N),
r(other.r),
p(other.p),
maxmem(other.maxmem),
length(other.length) {}

ScryptConfig& ScryptConfig::operator=(ScryptConfig&& other) noexcept {
Expand Down Expand Up @@ -127,7 +128,7 @@ bool ScryptTraits::DeriveBits(
ByteSource buf = ByteSource::Allocated(data, params.length);
unsigned char* ptr = reinterpret_cast<unsigned char*>(data);

// Botht the pass and salt may be zero-length at this point
// Both the pass and salt may be zero-length at this point

if (!EVP_PBE_scrypt(
params.pass.get(),
Expand Down
4 changes: 3 additions & 1 deletion src/crypto/crypto_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,9 @@ ByteSource& ByteSource::operator=(ByteSource&& other) noexcept {
}

std::unique_ptr<BackingStore> ByteSource::ReleaseToBackingStore() {
CHECK_NOT_NULL(allocated_data_);
// It's ok for allocated_data_ to be nullptr but
// only if size_ is not zero.
CHECK_IMPLIES(size_ > 0, allocated_data_ != nullptr);
std::unique_ptr<BackingStore> ptr = ArrayBuffer::NewBackingStore(
allocated_data_,
size(),
Expand Down
5 changes: 4 additions & 1 deletion src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,16 @@ void Initialize(Local<Object> target,
PBKDF2Job::Initialize(env, target);
Random::Initialize(env, target);
RSAAlg::Initialize(env, target);
ScryptJob::Initialize(env, target);
SecureContext::Initialize(env, target);
Sign::Initialize(env, target);
SPKAC::Initialize(env, target);
Timing::Initialize(env, target);
Util::Initialize(env, target);
Verify::Initialize(env, target);

#ifndef OPENSSL_NO_SCRYPT
ScryptJob::Initialize(env, target);
#endif
}

} // namespace crypto
Expand Down
10 changes: 3 additions & 7 deletions test/parallel/test-crypto-scrypt.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const assert = require('assert');
const crypto = require('crypto');

const { internalBinding } = require('internal/test/binding');
if (typeof internalBinding('crypto').scrypt !== 'function')
if (typeof internalBinding('crypto').ScryptJob !== 'function')
common.skip('no scrypt support');

const good = [
Expand Down Expand Up @@ -156,9 +156,7 @@ for (const options of good) {

for (const options of bad) {
const expected = {
code: 'ERR_CRYPTO_SCRYPT_INVALID_PARAMETER',
message: 'Invalid scrypt parameter',
name: 'Error',
message: /Invalid scrypt param/,
};
assert.throws(() => crypto.scrypt('pass', 'salt', 1, options, () => {}),
expected);
Expand All @@ -168,9 +166,7 @@ for (const options of bad) {

for (const options of toobig) {
const expected = {
message: new RegExp('error:[^:]+:digital envelope routines:' +
'(?:EVP_PBE_scrypt|scrypt_alg):memory limit exceeded'),
name: 'Error',
message: /Invalid scrypt param/
};
assert.throws(() => crypto.scrypt('pass', 'salt', 1, options, () => {}),
expected);
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-webcrypto-derivebits.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ const { internalBinding } = require('internal/test/binding');
}

// Test Scrypt bit derivation
if (typeof internalBinding('crypto').scrypt === 'function') {
if (typeof internalBinding('crypto').ScryptJob === 'function') {
async function test(pass, salt, length, expected) {
const ec = new TextEncoder();
const key = await subtle.importKey(
Expand All @@ -111,7 +111,7 @@ if (typeof internalBinding('crypto').scrypt === 'function') {
{ name: 'NODE-SCRYPT' },
false, ['deriveBits']);
const secret = await subtle.deriveBits({
name: 'SCRYPT',
name: 'NODE-SCRYPT',
salt: ec.encode(salt),
}, key, length);
assert.strictEqual(Buffer.from(secret).toString('hex'), expected);
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-webcrypto-derivekey.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ const { internalBinding } = require('internal/test/binding');
}

// Test Scrypt bit derivation
if (typeof internalBinding('crypto').scrypt === 'function') {
if (typeof internalBinding('crypto').ScryptJob === 'function') {
async function test(pass, salt, expected) {
const ec = new TextEncoder();
const key = await subtle.importKey(
Expand All @@ -144,7 +144,7 @@ if (typeof internalBinding('crypto').scrypt === 'function') {
}

const kTests = [
['hello', 'there', 10, 'SHA-256',
['hello', 'there',
'30ddda6feabaac788eb81cc38f496cd5d9a165d320c537ea05331fe720db1061']
];

Expand Down
2 changes: 1 addition & 1 deletion test/sequential/test-async-wrap-getasyncid.js
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ if (common.hasCrypto) { // eslint-disable-line node-core/crypto-check
testInitialized(this, 'RandomBytesJob');
}));

if (typeof internalBinding('crypto').scrypt === 'function') {
if (typeof internalBinding('crypto').ScryptJob === 'function') {
crypto.scrypt('password', 'salt', 8, common.mustCall(function() {
testInitialized(this, 'ScryptJob');
}));
Expand Down

0 comments on commit 79a8fb6

Please sign in to comment.