From: RafaelGSS Date: Mon, 12 May 2025 15:33:54 +0000 (-0300) Subject: src: fix error handling on async crypto operations X-Git-Tag: archive/raspbian/18.20.4+dfsg-1_deb12u2+rpi1^2~6 X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=13f31e41787c00adde37f709cbbf676e37a2688b;p=nodejs.git src: fix error handling on async crypto operations Fixes: https://hackerone.com/reports/2817648 Co-Authored-By: Filip Skokan Co-Authored-By: Tobias Nießen Backport-PR-URL: https://github.com/nodejs-private/node-private/pull/688 CVE-ID: CVE-2025-23166 PR-URL: https://github.com/nodejs-private/node-private/pull/710 origin: backport, https://github.com/nodejs/node/commit/6c57465920cf1b981a63031e71b1e4a73bf9beaa Gbp-Pq: Name CVE-2025-23166.patch --- diff --git a/src/crypto/crypto_dh.cc b/src/crypto/crypto_dh.cc index dd69323b8..cf9cbc973 100644 --- a/src/crypto/crypto_dh.cc +++ b/src/crypto/crypto_dh.cc @@ -705,10 +705,10 @@ Maybe DHBitsTraits::EncodeOutput( return Just(!result->IsEmpty()); } -bool DHBitsTraits::DeriveBits( - Environment* env, - const DHBitsConfig& params, - ByteSource* out) { +bool DHBitsTraits::DeriveBits(Environment* env, + const DHBitsConfig& params, + ByteSource* out, + CryptoJobMode mode) { *out = StatelessDiffieHellmanThreadsafe( params.private_key->GetAsymmetricKey(), params.public_key->GetAsymmetricKey()); diff --git a/src/crypto/crypto_dh.h b/src/crypto/crypto_dh.h index ec12548db..f7c4b6757 100644 --- a/src/crypto/crypto_dh.h +++ b/src/crypto/crypto_dh.h @@ -131,10 +131,10 @@ struct DHBitsTraits final { unsigned int offset, DHBitsConfig* params); - static bool DeriveBits( - Environment* env, - const DHBitsConfig& params, - ByteSource* out_); + static bool DeriveBits(Environment* env, + const DHBitsConfig& params, + ByteSource* out_, + CryptoJobMode mode); static v8::Maybe EncodeOutput( Environment* env, diff --git a/src/crypto/crypto_ec.cc b/src/crypto/crypto_ec.cc index 3ccea99fb..d7c77d078 100644 --- a/src/crypto/crypto_ec.cc +++ b/src/crypto/crypto_ec.cc @@ -484,7 +484,8 @@ Maybe ECDHBitsTraits::AdditionalConfig( bool ECDHBitsTraits::DeriveBits(Environment* env, const ECDHBitsConfig& params, - ByteSource* out) { + ByteSource* out, + CryptoJobMode mode) { size_t len = 0; ManagedEVPPKey m_privkey = params.private_->GetAsymmetricKey(); ManagedEVPPKey m_pubkey = params.public_->GetAsymmetricKey(); diff --git a/src/crypto/crypto_ec.h b/src/crypto/crypto_ec.h index 9782ce0bf..8c955ecdb 100644 --- a/src/crypto/crypto_ec.h +++ b/src/crypto/crypto_ec.h @@ -77,10 +77,10 @@ struct ECDHBitsTraits final { unsigned int offset, ECDHBitsConfig* params); - static bool DeriveBits( - Environment* env, - const ECDHBitsConfig& params, - ByteSource* out_); + static bool DeriveBits(Environment* env, + const ECDHBitsConfig& params, + ByteSource* out_, + CryptoJobMode mode); static v8::Maybe EncodeOutput( Environment* env, diff --git a/src/crypto/crypto_hash.cc b/src/crypto/crypto_hash.cc index 3cb39d795..eb1d5152a 100644 --- a/src/crypto/crypto_hash.cc +++ b/src/crypto/crypto_hash.cc @@ -282,10 +282,10 @@ Maybe HashTraits::AdditionalConfig( return Just(true); } -bool HashTraits::DeriveBits( - Environment* env, - const HashConfig& params, - ByteSource* out) { +bool HashTraits::DeriveBits(Environment* env, + const HashConfig& params, + ByteSource* out, + CryptoJobMode mode) { EVPMDPointer ctx(EVP_MD_CTX_new()); if (UNLIKELY(!ctx || diff --git a/src/crypto/crypto_hash.h b/src/crypto/crypto_hash.h index 2d17c3510..d67369462 100644 --- a/src/crypto/crypto_hash.h +++ b/src/crypto/crypto_hash.h @@ -68,10 +68,10 @@ struct HashTraits final { unsigned int offset, HashConfig* params); - static bool DeriveBits( - Environment* env, - const HashConfig& params, - ByteSource* out); + static bool DeriveBits(Environment* env, + const HashConfig& params, + ByteSource* out, + CryptoJobMode mode); static v8::Maybe EncodeOutput( Environment* env, diff --git a/src/crypto/crypto_hkdf.cc b/src/crypto/crypto_hkdf.cc index 7663dd693..896bcf20d 100644 --- a/src/crypto/crypto_hkdf.cc +++ b/src/crypto/crypto_hkdf.cc @@ -100,10 +100,10 @@ Maybe HKDFTraits::AdditionalConfig( return Just(true); } -bool HKDFTraits::DeriveBits( - Environment* env, - const HKDFConfig& params, - ByteSource* out) { +bool HKDFTraits::DeriveBits(Environment* env, + const HKDFConfig& params, + ByteSource* out, + CryptoJobMode mode) { EVPKeyCtxPointer ctx = EVPKeyCtxPointer(EVP_PKEY_CTX_new_id(EVP_PKEY_HKDF, nullptr)); if (!ctx || !EVP_PKEY_derive_init(ctx.get()) || diff --git a/src/crypto/crypto_hkdf.h b/src/crypto/crypto_hkdf.h index c4a537cef..acd2b670a 100644 --- a/src/crypto/crypto_hkdf.h +++ b/src/crypto/crypto_hkdf.h @@ -42,10 +42,10 @@ struct HKDFTraits final { unsigned int offset, HKDFConfig* params); - static bool DeriveBits( - Environment* env, - const HKDFConfig& params, - ByteSource* out); + static bool DeriveBits(Environment* env, + const HKDFConfig& params, + ByteSource* out, + CryptoJobMode mode); static v8::Maybe EncodeOutput( Environment* env, diff --git a/src/crypto/crypto_hmac.cc b/src/crypto/crypto_hmac.cc index a9bb00ee5..8173946b1 100644 --- a/src/crypto/crypto_hmac.cc +++ b/src/crypto/crypto_hmac.cc @@ -222,10 +222,10 @@ Maybe HmacTraits::AdditionalConfig( return Just(true); } -bool HmacTraits::DeriveBits( - Environment* env, - const HmacConfig& params, - ByteSource* out) { +bool HmacTraits::DeriveBits(Environment* env, + const HmacConfig& params, + ByteSource* out, + CryptoJobMode mode) { HMACCtxPointer ctx(HMAC_CTX_new()); if (!ctx || diff --git a/src/crypto/crypto_hmac.h b/src/crypto/crypto_hmac.h index c80cc36f1..dd490f05e 100644 --- a/src/crypto/crypto_hmac.h +++ b/src/crypto/crypto_hmac.h @@ -73,10 +73,10 @@ struct HmacTraits final { unsigned int offset, HmacConfig* params); - static bool DeriveBits( - Environment* env, - const HmacConfig& params, - ByteSource* out); + static bool DeriveBits(Environment* env, + const HmacConfig& params, + ByteSource* out, + CryptoJobMode mode); static v8::Maybe EncodeOutput( Environment* env, diff --git a/src/crypto/crypto_pbkdf2.cc b/src/crypto/crypto_pbkdf2.cc index 963d0db6c..f6d37dadc 100644 --- a/src/crypto/crypto_pbkdf2.cc +++ b/src/crypto/crypto_pbkdf2.cc @@ -111,10 +111,10 @@ Maybe PBKDF2Traits::AdditionalConfig( return Just(true); } -bool PBKDF2Traits::DeriveBits( - Environment* env, - const PBKDF2Config& params, - ByteSource* out) { +bool PBKDF2Traits::DeriveBits(Environment* env, + const PBKDF2Config& params, + ByteSource* out, + CryptoJobMode mode) { ByteSource::Builder buf(params.length); // Both pass and salt may be zero length here. diff --git a/src/crypto/crypto_pbkdf2.h b/src/crypto/crypto_pbkdf2.h index 6fda7cd31..11ffad784 100644 --- a/src/crypto/crypto_pbkdf2.h +++ b/src/crypto/crypto_pbkdf2.h @@ -55,10 +55,10 @@ struct PBKDF2Traits final { unsigned int offset, PBKDF2Config* params); - static bool DeriveBits( - Environment* env, - const PBKDF2Config& params, - ByteSource* out); + static bool DeriveBits(Environment* env, + const PBKDF2Config& params, + ByteSource* out, + CryptoJobMode mode); static v8::Maybe EncodeOutput( Environment* env, diff --git a/src/crypto/crypto_random.cc b/src/crypto/crypto_random.cc index 9850104cd..527f6a8b2 100644 --- a/src/crypto/crypto_random.cc +++ b/src/crypto/crypto_random.cc @@ -56,10 +56,10 @@ Maybe RandomBytesTraits::AdditionalConfig( return Just(true); } -bool RandomBytesTraits::DeriveBits( - Environment* env, - const RandomBytesConfig& params, - ByteSource* unused) { +bool RandomBytesTraits::DeriveBits(Environment* env, + const RandomBytesConfig& params, + ByteSource* unused, + CryptoJobMode mode) { return CSPRNG(params.buffer, params.size).is_ok(); } @@ -151,7 +151,8 @@ Maybe RandomPrimeTraits::AdditionalConfig( bool RandomPrimeTraits::DeriveBits(Environment* env, const RandomPrimeConfig& params, - ByteSource* unused) { + ByteSource* unused, + CryptoJobMode mode) { // BN_generate_prime_ex() calls RAND_bytes_ex() internally. // Make sure the CSPRNG is properly seeded. CHECK(CSPRNG(nullptr, 0).is_ok()); @@ -194,11 +195,10 @@ Maybe CheckPrimeTraits::AdditionalConfig( return Just(true); } -bool CheckPrimeTraits::DeriveBits( - Environment* env, - const CheckPrimeConfig& params, - ByteSource* out) { - +bool CheckPrimeTraits::DeriveBits(Environment* env, + const CheckPrimeConfig& params, + ByteSource* out, + CryptoJobMode mode) { BignumCtxPointer ctx(BN_CTX_new()); int ret = BN_is_prime_ex( diff --git a/src/crypto/crypto_random.h b/src/crypto/crypto_random.h index a2807ed6e..b673cbbfd 100644 --- a/src/crypto/crypto_random.h +++ b/src/crypto/crypto_random.h @@ -32,10 +32,10 @@ struct RandomBytesTraits final { unsigned int offset, RandomBytesConfig* params); - static bool DeriveBits( - Environment* env, - const RandomBytesConfig& params, - ByteSource* out_); + static bool DeriveBits(Environment* env, + const RandomBytesConfig& params, + ByteSource* out_, + CryptoJobMode mode); static v8::Maybe EncodeOutput( Environment* env, @@ -72,7 +72,8 @@ struct RandomPrimeTraits final { static bool DeriveBits( Environment* env, const RandomPrimeConfig& params, - ByteSource* out_); + ByteSource* out_, + CryptoJobMode mode); static v8::Maybe EncodeOutput( Environment* env, @@ -105,10 +106,10 @@ struct CheckPrimeTraits final { unsigned int offset, CheckPrimeConfig* params); - static bool DeriveBits( - Environment* env, - const CheckPrimeConfig& params, - ByteSource* out); + static bool DeriveBits(Environment* env, + const CheckPrimeConfig& params, + ByteSource* out, + CryptoJobMode mode); static v8::Maybe EncodeOutput( Environment* env, diff --git a/src/crypto/crypto_scrypt.cc b/src/crypto/crypto_scrypt.cc index 4dae07f13..99a6a0e7a 100644 --- a/src/crypto/crypto_scrypt.cc +++ b/src/crypto/crypto_scrypt.cc @@ -114,10 +114,10 @@ Maybe ScryptTraits::AdditionalConfig( return Just(true); } -bool ScryptTraits::DeriveBits( - Environment* env, - const ScryptConfig& params, - ByteSource* out) { +bool ScryptTraits::DeriveBits(Environment* env, + const ScryptConfig& params, + ByteSource* out, + CryptoJobMode mode) { ByteSource::Builder buf(params.length); // Both the pass and salt may be zero-length at this point diff --git a/src/crypto/crypto_scrypt.h b/src/crypto/crypto_scrypt.h index 3d185637f..9ea9d75d8 100644 --- a/src/crypto/crypto_scrypt.h +++ b/src/crypto/crypto_scrypt.h @@ -57,10 +57,10 @@ struct ScryptTraits final { unsigned int offset, ScryptConfig* params); - static bool DeriveBits( - Environment* env, - const ScryptConfig& params, - ByteSource* out); + static bool DeriveBits(Environment* env, + const ScryptConfig& params, + ByteSource* out, + CryptoJobMode mode); static v8::Maybe EncodeOutput( Environment* env, diff --git a/src/crypto/crypto_sig.cc b/src/crypto/crypto_sig.cc index 64e788dce..188103424 100644 --- a/src/crypto/crypto_sig.cc +++ b/src/crypto/crypto_sig.cc @@ -695,12 +695,13 @@ Maybe SignTraits::AdditionalConfig( return Just(true); } -bool SignTraits::DeriveBits( - Environment* env, - const SignConfiguration& params, - ByteSource* out) { - ClearErrorOnReturn clear_error_on_return; +bool SignTraits::DeriveBits(Environment* env, + const SignConfiguration& params, + ByteSource* out, + CryptoJobMode mode) { + bool can_throw = mode == CryptoJobMode::kCryptoJobSync; EVPMDPointer context(EVP_MD_CTX_new()); + EVP_PKEY_CTX* ctx = nullptr; switch (params.mode) { @@ -711,7 +712,7 @@ bool SignTraits::DeriveBits( params.digest, nullptr, params.key.get())) { - crypto::CheckThrow(env, SignBase::Error::kSignInit); + if (can_throw) crypto::CheckThrow(env, SignBase::Error::kSignInit); return false; } break; @@ -722,7 +723,7 @@ bool SignTraits::DeriveBits( params.digest, nullptr, params.key.get())) { - crypto::CheckThrow(env, SignBase::Error::kSignInit); + if (can_throw) crypto::CheckThrow(env, SignBase::Error::kSignInit); return false; } break; @@ -740,7 +741,7 @@ bool SignTraits::DeriveBits( ctx, padding, salt_length)) { - crypto::CheckThrow(env, SignBase::Error::kSignPrivateKey); + if (can_throw) crypto::CheckThrow(env, SignBase::Error::kSignPrivateKey); return false; } @@ -754,7 +755,8 @@ bool SignTraits::DeriveBits( &len, params.data.data(), params.data.size())) { - crypto::CheckThrow(env, SignBase::Error::kSignPrivateKey); + if (can_throw) + crypto::CheckThrow(env, SignBase::Error::kSignPrivateKey); return false; } ByteSource::Builder buf(len); @@ -763,7 +765,8 @@ bool SignTraits::DeriveBits( &len, params.data.data(), params.data.size())) { - crypto::CheckThrow(env, SignBase::Error::kSignPrivateKey); + if (can_throw) + crypto::CheckThrow(env, SignBase::Error::kSignPrivateKey); return false; } *out = std::move(buf).release(len); @@ -774,13 +777,15 @@ bool SignTraits::DeriveBits( params.data.data(), params.data.size()) || !EVP_DigestSignFinal(context.get(), nullptr, &len)) { - crypto::CheckThrow(env, SignBase::Error::kSignPrivateKey); + if (can_throw) + crypto::CheckThrow(env, SignBase::Error::kSignPrivateKey); return false; } ByteSource::Builder buf(len); if (!EVP_DigestSignFinal( context.get(), buf.data(), &len)) { - crypto::CheckThrow(env, SignBase::Error::kSignPrivateKey); + if (can_throw) + crypto::CheckThrow(env, SignBase::Error::kSignPrivateKey); return false; } diff --git a/src/crypto/crypto_sig.h b/src/crypto/crypto_sig.h index 1a4cda422..6b3c8d915 100644 --- a/src/crypto/crypto_sig.h +++ b/src/crypto/crypto_sig.h @@ -147,10 +147,10 @@ struct SignTraits final { unsigned int offset, SignConfiguration* params); - static bool DeriveBits( - Environment* env, - const SignConfiguration& params, - ByteSource* out); + static bool DeriveBits(Environment* env, + const SignConfiguration& params, + ByteSource* out, + CryptoJobMode mode); static v8::Maybe EncodeOutput( Environment* env, diff --git a/src/crypto/crypto_util.h b/src/crypto/crypto_util.h index bf19334cf..e528de912 100644 --- a/src/crypto/crypto_util.h +++ b/src/crypto/crypto_util.h @@ -498,9 +498,10 @@ class DeriveBitsJob final : public CryptoJob { std::move(params)) {} void DoThreadPoolWork() override { + ClearErrorOnReturn clear_error_on_return; if (!DeriveBitsTraits::DeriveBits( AsyncWrap::env(), - *CryptoJob::params(), &out_)) { + *CryptoJob::params(), &out_, this->mode())) { CryptoErrorStore* errors = CryptoJob::errors(); errors->Capture(); if (errors->Empty()) diff --git a/test/parallel/test-crypto-async-sign-verify.js b/test/parallel/test-crypto-async-sign-verify.js index 4e3c32fdc..5924d36e4 100644 --- a/test/parallel/test-crypto-async-sign-verify.js +++ b/test/parallel/test-crypto-async-sign-verify.js @@ -141,3 +141,29 @@ test('dsa_public.pem', 'dsa_private.pem', 'sha256', false, }) .catch(common.mustNotCall()); } + +{ + const untrustedKey = `-----BEGIN PUBLIC KEY----- +MCowBQYDK2VuAyEA6pwGRbadNQAI/tYN8+/p/0/hbsdHfOEGr1ADiLVk/Gc= +-----END PUBLIC KEY-----`; + const data = crypto.randomBytes(32); + const signature = crypto.randomBytes(16); + + const expected = common.hasOpenSSL3 ? + /operation not supported for this keytype/ : /no default digest/; + + crypto.verify(undefined, data, untrustedKey, signature, common.mustCall((err) => { + assert.ok(err); + assert.match(err.message, expected); + })); +} + +{ + const { privateKey } = crypto.generateKeyPairSync('rsa', { + modulusLength: 512 + }); + crypto.sign('sha512', 'message', privateKey, common.mustCall((err) => { + assert.ok(err); + assert.match(err.message, /digest too big for rsa key/); + })); +}