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

crypto: preserve OpenSSL errors from KDF failures · nodejs/node@58d13b6 · GitHub

/ node Public

Commit 58d13b6

Browse files
panva
authored and
aduh95
committed
crypto: preserve OpenSSL errors from KDF failures
The ncrypto KDF helpers cleared the OpenSSL error queue on return, and the traits insert their own message, which makes DeriveBitsJob skip errors->Capture(). Argon2, HKDF, PBKDF2 and scrypt failures were therefore bare Errors with no code and no opensslErrorStack. Drop the guard, which DeriveBitsJob already provides, and capture before inserting since Capture() clears the store. Signed-off-by: Filip Skokan <panva.ip@gmail.com> PR-URL: #64776 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
1 parent 478a719 commit 58d13b6

7 files changed

Lines changed: 41 additions & 16 deletions

File tree

‎deps/ncrypto/ncrypto.cc‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2642,8 +2642,6 @@ DataPointer hkdf(const Digest& md,
26422642
const Buffer<const unsigned char>& info,
26432643
const Buffer<const unsigned char>& salt,
26442644
size_t length) {
2645-
ClearErrorOnReturn clearErrorOnReturn;
2646-
26472645
if (!checkHkdfLength(md, length) || info.len > INT_MAX ||
26482646
salt.len > INT_MAX) {
26492647
return {};
@@ -2714,8 +2712,6 @@ DataPointer scrypt(const Buffer<const char>& pass,
27142712
uint64_t p,
27152713
uint64_t maxmem,
27162714
size_t length) {
2717-
ClearErrorOnReturn clearErrorOnReturn;
2718-
27192715
if (pass.len > INT_MAX || salt.len > INT_MAX) {
27202716
return {};
27212717
}
@@ -2742,8 +2738,6 @@ DataPointer pbkdf2(const Digest& md,
27422738
const Buffer<const unsigned char>& salt,
27432739
uint32_t iterations,
27442740
size_t length) {
2745-
ClearErrorOnReturn clearErrorOnReturn;
2746-
27472741
if (pass.len > INT_MAX || salt.len > INT_MAX || length > INT_MAX) {
27482742
return {};
27492743
}
@@ -2775,8 +2769,6 @@ DataPointer argon2(const Buffer<const char>& pass,
27752769
const Buffer<const unsigned char>& secret,
27762770
const Buffer<const unsigned char>& ad,
27772771
Argon2Type type) {
2778-
ClearErrorOnReturn clearErrorOnReturn;
2779-
27802772
std::string_view algorithm;
27812773
switch (type) {
27822774
case Argon2Type::ARGON2I:

‎src/crypto/crypto_argon2.cc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ bool Argon2Traits::DeriveBits(Environment* env,
147147
config.type);
148148

149149
if (!dp) {
150+
errors->Capture();
150151
errors->Insert(NodeCryptoError::ARGON2_FAILED);
151152
return false;
152153
}

‎src/crypto/crypto_hkdf.cc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ bool HKDFTraits::DeriveBits(Environment* env,
127127
},
128128
params.length);
129129
if (!dp) {
130+
errors->Capture();
130131
errors->Insert(NodeCryptoError::HKDF_FAILED);
131132
return false;
132133
}

‎src/crypto/crypto_pbkdf2.cc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ bool PBKDF2Traits::DeriveBits(Environment* env,
135135
params.length);
136136

137137
if (!dp) {
138+
errors->Capture();
138139
errors->Insert(NodeCryptoError::PBKDF2_FAILED);
139140
return false;
140141
}

‎src/crypto/crypto_scrypt.cc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ bool ScryptTraits::DeriveBits(Environment* env,
132132
params.length);
133133

134134
if (!dp) {
135+
errors->Capture();
135136
errors->Insert(NodeCryptoError::SCRYPT_FAILED);
136137
return false;
137138
}

‎test/parallel/test-crypto-argon2-job.js‎

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,19 +29,31 @@ const empty = Buffer.alloc(0);
2929

3030
// Parameters that OpenSSL's Argon2 KDF rejects.
3131
const badParams = [
32-
{ lanes: 0, keylen: 32, memcost: 16, iter: 1 }, // lanes < 1
33-
{ lanes: 1, keylen: 32, memcost: 0, iter: 1 }, // memcost == 0
34-
{ lanes: 1, keylen: 32, memcost: 16, iter: 0 }, // iter == 0
32+
{ lanes: 0, keylen: 32, memcost: 16, iter: 1,
33+
code: 'ERR_OSSL_INVALID_THREAD_POOL_SIZE', reason: /invalid thread pool size/ },
34+
{ lanes: 1, keylen: 32, memcost: 0, iter: 1,
35+
code: 'ERR_OSSL_INVALID_MEMORY_SIZE', reason: /invalid memory size/ },
36+
{ lanes: 1, keylen: 32, memcost: 16, iter: 0,
37+
code: 'ERR_OSSL_INVALID_ITERATION_COUNT', reason: /invalid iteration count/ },
3538
];
3639

37-
for (const { lanes, keylen, memcost, iter } of badParams) {
40+
function assertError(err, { code, reason }) {
41+
assert.ok(err);
42+
assert.match(err.message, /Argon2 derivation failed/);
43+
assert.strictEqual(err.code, code);
44+
assert.ok(err.opensslErrorStack.some((msg) => reason.test(msg)),
45+
`did not find ${reason} in ${err.opensslErrorStack}`);
46+
}
47+
48+
for (const params of badParams) {
49+
const { lanes, keylen, memcost, iter } = params;
50+
3851
{
3952
const job = new Argon2Job(
4053
kCryptoJobSync, pass, salt, lanes, keylen, memcost, iter,
4154
empty, empty, kTypeArgon2id);
4255
const { 0: err, 1: result } = job.run();
43-
assert.ok(err);
44-
assert.match(err.message, /Argon2 derivation failed/);
56+
assertError(err, params);
4557
assert.strictEqual(result, undefined);
4658
}
4759

@@ -50,8 +62,7 @@ for (const { lanes, keylen, memcost, iter } of badParams) {
5062
kCryptoJobAsync, pass, salt, lanes, keylen, memcost, iter,
5163
empty, empty, kTypeArgon2id);
5264
job.ondone = common.mustCall((err, result) => {
53-
assert.ok(err);
54-
assert.match(err.message, /Argon2 derivation failed/);
65+
assertError(err, params);
5566
assert.strictEqual(result, undefined);
5667
});
5768
job.run();

‎test/parallel/test-crypto-no-algorithm.js‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,24 @@ if (isMainThread) {
2626
`did not find ${expected} in ${err.opensslErrorStack}`);
2727
}
2828
}));
29+
30+
const derivations = [
31+
['HKDF', () => crypto.hkdfSync('sha256', Buffer.alloc(32), Buffer.alloc(8),
32+
Buffer.alloc(0), 32)],
33+
['PBKDF2', () => crypto.pbkdf2Sync('secret', Buffer.alloc(16), 1000, 32,
34+
'sha256')],
35+
];
36+
for (const { 0: name, 1: derive } of derivations) {
37+
try {
38+
derive();
39+
} catch (err) {
40+
assert.match(err.message, /derivation failed/);
41+
assert.strictEqual(err.code, 'ERR_OSSL_EVP_UNSUPPORTED', `${name}: ${err.code}`);
42+
const expected = /digital envelope routines::unsupported/;
43+
assert(err.opensslErrorStack.some((msg) => expected.test(msg)),
44+
`${name}: did not find ${expected} in ${err.opensslErrorStack}`);
45+
}
46+
}
2947
}
3048

3149
{

0 commit comments

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