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

crypto: remove Argon2 KDF derivation from its job setup · nodejs/node@28a7874 · GitHub

/ node Public

Commit 28a7874

Browse files
panva
authored and
aduh95
committed
crypto: remove Argon2 KDF derivation from its job setup
Signed-off-by: Filip Skokan <panva.ip@gmail.com> PR-URL: #62863 Backport-PR-URL: #63173 Fixes: #62861 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 parent 2ca42c8 commit 28a7874

4 files changed

Lines changed: 140 additions & 16 deletions

File tree

‎src/crypto/crypto_argon2.cc‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -104,20 +104,6 @@ Maybe<void> Argon2Traits::AdditionalConfig(
104104
config->type =
105105
static_cast<ncrypto::Argon2Type>(args[offset + 8].As<Uint32>()->Value());
106106

107-
if (!ncrypto::argon2(config->pass,
108-
config->salt,
109-
config->lanes,
110-
config->keylen,
111-
config->memcost,
112-
config->iter,
113-
config->version,
114-
config->secret,
115-
config->ad,
116-
config->type)) {
117-
THROW_ERR_CRYPTO_INVALID_ARGON2_PARAMS(env);
118-
return Nothing<void>();
119-
}
120-
121107
return JustVoid();
122108
}
123109

‎src/node_errors.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@ void OOMErrorHandler(const char* location, const v8::OOMDetails& details);
5353
V(ERR_CRYPTO_CUSTOM_ENGINE_NOT_SUPPORTED, Error) \
5454
V(ERR_CRYPTO_INCOMPATIBLE_KEY_OPTIONS, Error) \
5555
V(ERR_CRYPTO_INITIALIZATION_FAILED, Error) \
56-
V(ERR_CRYPTO_INVALID_ARGON2_PARAMS, TypeError) \
5756
V(ERR_CRYPTO_INVALID_AUTH_TAG, TypeError) \
5857
V(ERR_CRYPTO_INVALID_COUNTER, TypeError) \
5958
V(ERR_CRYPTO_INVALID_CURVE, TypeError) \
@@ -195,7 +194,6 @@ ERRORS_WITH_CODE(V)
195194
V(ERR_CRYPTO_INCOMPATIBLE_KEY_OPTIONS, \
196195
"The selected key encoding is incompatible with the key type") \
197196
V(ERR_CRYPTO_INITIALIZATION_FAILED, "Initialization failed") \
198-
V(ERR_CRYPTO_INVALID_ARGON2_PARAMS, "Invalid Argon2 params") \
199197
V(ERR_CRYPTO_INVALID_AUTH_TAG, "Invalid authentication tag") \
200198
V(ERR_CRYPTO_INVALID_COUNTER, "Invalid counter") \
201199
V(ERR_CRYPTO_INVALID_CURVE, "Invalid EC curve name") \

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

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
// Flags: --expose-internals --no-warnings
2+
'use strict';
3+
const common = require('../common');
4+
if (!common.hasCrypto)
5+
common.skip('missing crypto');
6+
7+
const { hasOpenSSL } = require('../common/crypto');
8+
9+
if (!hasOpenSSL(3, 2))
10+
common.skip('requires OpenSSL >= 3.2');
11+
12+
// Exercises the native Argon2 job directly via internalBinding, bypassing
13+
// the JS validators, to ensure that if invalid parameters ever reach the
14+
// native layer they produce a clean error from the KDF rather than crashing,
15+
// in both sync and async modes.
16+
17+
const assert = require('node:assert');
18+
const { internalBinding } = require('internal/test/binding');
19+
const {
20+
Argon2Job,
21+
kCryptoJobAsync,
22+
kCryptoJobSync,
23+
kTypeArgon2id,
24+
} = internalBinding('crypto');
25+
26+
const pass = Buffer.from('password');
27+
const salt = Buffer.alloc(16, 0x02);
28+
const empty = Buffer.alloc(0);
29+
30+
// Parameters that OpenSSL's Argon2 KDF rejects.
31+
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
35+
];
36+
37+
for (const { lanes, keylen, memcost, iter } of badParams) {
38+
{
39+
const job = new Argon2Job(
40+
kCryptoJobSync, pass, salt, lanes, keylen, memcost, iter,
41+
empty, empty, kTypeArgon2id);
42+
const { 0: err, 1: result } = job.run();
43+
assert.ok(err);
44+
assert.match(err.message, /Deriving bits failed/);
45+
assert.strictEqual(result, undefined);
46+
}
47+
48+
{
49+
const job = new Argon2Job(
50+
kCryptoJobAsync, pass, salt, lanes, keylen, memcost, iter,
51+
empty, empty, kTypeArgon2id);
52+
job.ondone = common.mustCall((err, result) => {
53+
assert.ok(err);
54+
assert.match(err.message, /Deriving bits failed/);
55+
assert.strictEqual(result, undefined);
56+
});
57+
job.run();
58+
}
59+
}

‎test/pummel/test-crypto-argon2-nonblocking-constructor.js‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
// Flags: --expose-internals --no-warnings
2+
'use strict';
3+
const common = require('../common');
4+
if (!common.hasCrypto)
5+
common.skip('missing crypto');
6+
7+
const { hasOpenSSL } = require('../common/crypto');
8+
9+
if (!hasOpenSSL(3, 2))
10+
common.skip('requires OpenSSL >= 3.2');
11+
12+
// Regression test for https://github.com/nodejs/node/issues/62861.
13+
// `AdditionalConfig` used to invoke the full Argon2 KDF synchronously inside
14+
// `new Argon2Job(...)` for the purpose of getting a parameter validation
15+
// error.
16+
//
17+
// The fix removes the redundant KDF call from the constructor. This test
18+
// asserts that the constructor returns in a small fraction of the time a
19+
// full sync job takes. Pre-fix the constructor ran the KDF, and job.run()
20+
// then ran it a second time in DeriveBits; post-fix the constructor does
21+
// no KDF work at all.
22+
23+
const assert = require('node:assert');
24+
const { internalBinding } = require('internal/test/binding');
25+
const {
26+
Argon2Job,
27+
kCryptoJobAsync,
28+
kCryptoJobSync,
29+
kTypeArgon2id,
30+
} = internalBinding('crypto');
31+
32+
const pass = Buffer.from('password');
33+
const salt = Buffer.alloc(16, 0x02);
34+
const empty = Buffer.alloc(0);
35+
36+
// Tuned so a single-threaded Argon2id derivation is expensive enough that
37+
// scheduler/GC noise is negligible compared to it.
38+
const kdfArgs = [
39+
pass,
40+
salt,
41+
1, // lanes
42+
32, // keylen
43+
65536, // memcost
44+
20, // iter
45+
empty, // secret
46+
empty, // associatedData
47+
kTypeArgon2id,
48+
];
49+
50+
// For each mode, measure the constructor and the derivation separately and
51+
// assert that the constructor is a small fraction of the derivation. Pre-fix
52+
// the constructor ran a full KDF, so ctorNs was comparable to runNs. Post-fix
53+
// the constructor does no KDF work and must be orders of magnitude faster.
54+
//
55+
// For async mode the derivation happens on the thread pool, so runNs is
56+
// measured from the start of run() until ondone fires.
57+
58+
// Sync: run() derives on the main thread and returns when done.
59+
{
60+
const ctorStart = process.hrtime.bigint();
61+
const job = new Argon2Job(kCryptoJobSync, ...kdfArgs);
62+
const ctorNs = process.hrtime.bigint() - ctorStart;
63+
const runStart = process.hrtime.bigint();
64+
const { 0: err } = job.run();
65+
const runNs = process.hrtime.bigint() - runStart;
66+
assert.strictEqual(err, undefined);
67+
assert.ok(ctorNs * 10n < runNs);
68+
}
69+
70+
// Async: run() dispatches to the thread pool; measure until ondone fires.
71+
{
72+
const ctorStart = process.hrtime.bigint();
73+
const job = new Argon2Job(kCryptoJobAsync, ...kdfArgs);
74+
const ctorNs = process.hrtime.bigint() - ctorStart;
75+
const runStart = process.hrtime.bigint();
76+
job.ondone = common.mustSucceed(() => {
77+
const runNs = process.hrtime.bigint() - runStart;
78+
assert.ok(ctorNs * 10n < runNs);
79+
});
80+
job.run();
81+
}

0 commit comments

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