From 986fb5b02d77e07593f92b678b3a108e360d5e5d Mon Sep 17 00:00:00 2001
From: James M Snell <jasnell@gmail.com>
Date: Mon, 26 Oct 2020 11:25:42 -0700
Subject: [PATCH 1/2] crypto: fixup scrypt regressions
Fixes a handful of regressions in scrypt support following
the refactor.
Fixes: https://github.com/nodejs/node/issues/35815
---
lib/internal/crypto/webcrypto.js | 2 +-
src/crypto/crypto_scrypt.cc | 3 ++-
src/crypto/crypto_util.cc | 7 ++++++-
src/node_crypto.cc | 5 ++++-
test/parallel/test-crypto-scrypt.js | 10 +++-------
test/parallel/test-webcrypto-derivebits.js | 4 ++--
test/parallel/test-webcrypto-derivekey.js | 4 ++--
test/sequential/test-async-wrap-getasyncid.js | 2 +-
8 files changed, 21 insertions(+), 16 deletions(-)
diff --git a/lib/internal/crypto/webcrypto.js b/lib/internal/crypto/webcrypto.js
index 0a7101bfcc78c2..81ff9ca5191004 100644
--- a/lib/internal/crypto/webcrypto.js
+++ b/lib/internal/crypto/webcrypto.js
@@ -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);
diff --git a/src/crypto/crypto_scrypt.cc b/src/crypto/crypto_scrypt.cc
index 1af9853a6710e3..39d6b3fd0d8d6a 100644
--- a/src/crypto/crypto_scrypt.cc
+++ b/src/crypto/crypto_scrypt.cc
@@ -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 {
@@ -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(),
diff --git a/src/crypto/crypto_util.cc b/src/crypto/crypto_util.cc
index c8a02cc335b66a..1ee51315fa41cd 100644
--- a/src/crypto/crypto_util.cc
+++ b/src/crypto/crypto_util.cc
@@ -236,7 +236,12 @@ ByteSource& ByteSource::operator=(ByteSource&& other) noexcept {
}
std::unique_ptr<BackingStore> ByteSource::ReleaseToBackingStore() {
- CHECK_NOT_NULL(allocated_data_);
+ CHECK_IMPLIES(size_ == 0, allocated_data_ == nullptr);
+ if (size_ == 0) {
+ return ArrayBuffer::NewBackingStore(
+ nullptr, 0, [](void* data, size_t length, void* deleter_data) {},
+ nullptr);
+ }
std::unique_ptr<BackingStore> ptr = ArrayBuffer::NewBackingStore(
allocated_data_,
size(),
diff --git a/src/node_crypto.cc b/src/node_crypto.cc
index 0c1dbcc52d748c..d25387f1425daa 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -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
diff --git a/test/parallel/test-crypto-scrypt.js b/test/parallel/test-crypto-scrypt.js
index 6c19dee23291bb..aec86d64891518 100644
--- a/test/parallel/test-crypto-scrypt.js
+++ b/test/parallel/test-crypto-scrypt.js
@@ -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 = [
@@ -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);
@@ -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);
diff --git a/test/parallel/test-webcrypto-derivebits.js b/test/parallel/test-webcrypto-derivebits.js
index a8d4d6e673862f..0556b8c15ceba3 100644
--- a/test/parallel/test-webcrypto-derivebits.js
+++ b/test/parallel/test-webcrypto-derivebits.js
@@ -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(
@@ -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);
diff --git a/test/parallel/test-webcrypto-derivekey.js b/test/parallel/test-webcrypto-derivekey.js
index 9434da1fb672fb..f072fe01682792 100644
--- a/test/parallel/test-webcrypto-derivekey.js
+++ b/test/parallel/test-webcrypto-derivekey.js
@@ -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(
@@ -144,7 +144,7 @@ if (typeof internalBinding('crypto').scrypt === 'function') {
}
const kTests = [
- ['hello', 'there', 10, 'SHA-256',
+ ['hello', 'there',
'30ddda6feabaac788eb81cc38f496cd5d9a165d320c537ea05331fe720db1061']
];
diff --git a/test/sequential/test-async-wrap-getasyncid.js b/test/sequential/test-async-wrap-getasyncid.js
index 22fde0916544d6..e37e613b747dc2 100644
--- a/test/sequential/test-async-wrap-getasyncid.js
+++ b/test/sequential/test-async-wrap-getasyncid.js
@@ -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');
}));
From 9e4594b2ece07adb33893a13260b74b086d92dcb Mon Sep 17 00:00:00 2001
From: James M Snell <jasnell@gmail.com>
Date: Mon, 26 Oct 2020 12:12:52 -0700
Subject: [PATCH 2/2] fixup! crypto: fixup scrypt regressions
---
src/crypto/crypto_util.cc | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/src/crypto/crypto_util.cc b/src/crypto/crypto_util.cc
index 1ee51315fa41cd..1b86c54a6f4690 100644
--- a/src/crypto/crypto_util.cc
+++ b/src/crypto/crypto_util.cc
@@ -236,12 +236,9 @@ ByteSource& ByteSource::operator=(ByteSource&& other) noexcept {
}
std::unique_ptr<BackingStore> ByteSource::ReleaseToBackingStore() {
- CHECK_IMPLIES(size_ == 0, allocated_data_ == nullptr);
- if (size_ == 0) {
- return ArrayBuffer::NewBackingStore(
- nullptr, 0, [](void* data, size_t length, void* deleter_data) {},
- nullptr);
- }
+ // 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(),