diff --git a/src/crypto/crypto_argon2.cc b/src/crypto/crypto_argon2.cc index ca1af21f084d71..7bb995ca51c0df 100644 --- a/src/crypto/crypto_argon2.cc +++ b/src/crypto/crypto_argon2.cc @@ -104,20 +104,6 @@ Maybe Argon2Traits::AdditionalConfig( config->type = static_cast(args[offset + 8].As()->Value()); - if (!ncrypto::argon2(config->pass, - config->salt, - config->lanes, - config->keylen, - config->memcost, - config->iter, - config->version, - config->secret, - config->ad, - config->type)) { - THROW_ERR_CRYPTO_INVALID_ARGON2_PARAMS(env); - return Nothing(); - } - return JustVoid(); } diff --git a/src/node_errors.h b/src/node_errors.h index 6734e7807f5280..ed8e7eed9e3d9b 100644 --- a/src/node_errors.h +++ b/src/node_errors.h @@ -53,7 +53,6 @@ void OOMErrorHandler(const char* location, const v8::OOMDetails& details); V(ERR_CRYPTO_CUSTOM_ENGINE_NOT_SUPPORTED, Error) \ V(ERR_CRYPTO_INCOMPATIBLE_KEY_OPTIONS, Error) \ V(ERR_CRYPTO_INITIALIZATION_FAILED, Error) \ - V(ERR_CRYPTO_INVALID_ARGON2_PARAMS, TypeError) \ V(ERR_CRYPTO_INVALID_AUTH_TAG, TypeError) \ V(ERR_CRYPTO_INVALID_COUNTER, TypeError) \ V(ERR_CRYPTO_INVALID_CURVE, TypeError) \ @@ -198,7 +197,6 @@ ERRORS_WITH_CODE(V) V(ERR_CRYPTO_INCOMPATIBLE_KEY_OPTIONS, \ "The selected key encoding is incompatible with the key type") \ V(ERR_CRYPTO_INITIALIZATION_FAILED, "Initialization failed") \ - V(ERR_CRYPTO_INVALID_ARGON2_PARAMS, "Invalid Argon2 params") \ V(ERR_CRYPTO_INVALID_AUTH_TAG, "Invalid authentication tag") \ V(ERR_CRYPTO_INVALID_COUNTER, "Invalid counter") \ V(ERR_CRYPTO_INVALID_CURVE, "Invalid EC curve name") \ diff --git a/test/parallel/test-crypto-argon2-job.js b/test/parallel/test-crypto-argon2-job.js new file mode 100644 index 00000000000000..34db5e4660c11c --- /dev/null +++ b/test/parallel/test-crypto-argon2-job.js @@ -0,0 +1,59 @@ +// Flags: --expose-internals --no-warnings +'use strict'; +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +const { hasOpenSSL } = require('../common/crypto'); + +if (!hasOpenSSL(3, 2)) + common.skip('requires OpenSSL >= 3.2'); + +// Exercises the native Argon2 job directly via internalBinding, bypassing +// the JS validators, to ensure that if invalid parameters ever reach the +// native layer they produce a clean error from the KDF rather than crashing, +// in both sync and async modes. + +const assert = require('node:assert'); +const { internalBinding } = require('internal/test/binding'); +const { + Argon2Job, + kCryptoJobAsync, + kCryptoJobSync, + kTypeArgon2id, +} = internalBinding('crypto'); + +const pass = Buffer.from('password'); +const salt = Buffer.alloc(16, 0x02); +const empty = Buffer.alloc(0); + +// Parameters that OpenSSL's Argon2 KDF rejects. +const badParams = [ + { lanes: 0, keylen: 32, memcost: 16, iter: 1 }, // lanes < 1 + { lanes: 1, keylen: 32, memcost: 0, iter: 1 }, // memcost == 0 + { lanes: 1, keylen: 32, memcost: 16, iter: 0 }, // iter == 0 +]; + +for (const { lanes, keylen, memcost, iter } of badParams) { + { + const job = new Argon2Job( + kCryptoJobSync, pass, salt, lanes, keylen, memcost, iter, + empty, empty, kTypeArgon2id); + const { 0: err, 1: result } = job.run(); + assert.ok(err); + assert.match(err.message, /Argon2 derivation failed/); + assert.strictEqual(result, undefined); + } + + { + const job = new Argon2Job( + kCryptoJobAsync, pass, salt, lanes, keylen, memcost, iter, + empty, empty, kTypeArgon2id); + job.ondone = common.mustCall((err, result) => { + assert.ok(err); + assert.match(err.message, /Argon2 derivation failed/); + assert.strictEqual(result, undefined); + }); + job.run(); + } +} diff --git a/test/pummel/test-crypto-argon2-nonblocking-constructor.js b/test/pummel/test-crypto-argon2-nonblocking-constructor.js new file mode 100644 index 00000000000000..37cb5363d43c02 --- /dev/null +++ b/test/pummel/test-crypto-argon2-nonblocking-constructor.js @@ -0,0 +1,81 @@ +// Flags: --expose-internals --no-warnings +'use strict'; +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +const { hasOpenSSL } = require('../common/crypto'); + +if (!hasOpenSSL(3, 2)) + common.skip('requires OpenSSL >= 3.2'); + +// Regression test for https://github.com/nodejs/node/issues/62861. +// `AdditionalConfig` used to invoke the full Argon2 KDF synchronously inside +// `new Argon2Job(...)` for the purpose of getting a parameter validation +// error. +// +// The fix removes the redundant KDF call from the constructor. This test +// asserts that the constructor returns in a small fraction of the time a +// full sync job takes. Pre-fix the constructor ran the KDF, and job.run() +// then ran it a second time in DeriveBits; post-fix the constructor does +// no KDF work at all. + +const assert = require('node:assert'); +const { internalBinding } = require('internal/test/binding'); +const { + Argon2Job, + kCryptoJobAsync, + kCryptoJobSync, + kTypeArgon2id, +} = internalBinding('crypto'); + +const pass = Buffer.from('password'); +const salt = Buffer.alloc(16, 0x02); +const empty = Buffer.alloc(0); + +// Tuned so a single-threaded Argon2id derivation is expensive enough that +// scheduler/GC noise is negligible compared to it. +const kdfArgs = [ + pass, + salt, + 1, // lanes + 32, // keylen + 65536, // memcost + 20, // iter + empty, // secret + empty, // associatedData + kTypeArgon2id, +]; + +// For each mode, measure the constructor and the derivation separately and +// assert that the constructor is a small fraction of the derivation. Pre-fix +// the constructor ran a full KDF, so ctorNs was comparable to runNs. Post-fix +// the constructor does no KDF work and must be orders of magnitude faster. +// +// For async mode the derivation happens on the thread pool, so runNs is +// measured from the start of run() until ondone fires. + +// Sync: run() derives on the main thread and returns when done. +{ + const ctorStart = process.hrtime.bigint(); + const job = new Argon2Job(kCryptoJobSync, ...kdfArgs); + const ctorNs = process.hrtime.bigint() - ctorStart; + const runStart = process.hrtime.bigint(); + const { 0: err } = job.run(); + const runNs = process.hrtime.bigint() - runStart; + assert.strictEqual(err, undefined); + assert.ok(ctorNs * 10n < runNs); +} + +// Async: run() dispatches to the thread pool; measure until ondone fires. +{ + const ctorStart = process.hrtime.bigint(); + const job = new Argon2Job(kCryptoJobAsync, ...kdfArgs); + const ctorNs = process.hrtime.bigint() - ctorStart; + const runStart = process.hrtime.bigint(); + job.ondone = common.mustSucceed(() => { + const runNs = process.hrtime.bigint() - runStart; + assert.ok(ctorNs * 10n < runNs); + }); + job.run(); +}