src: fix error handling on async crypto operations

Fixes: https://hackerone.com/reports/2817648

Co-Authored-By: Filip Skokan <panva.ip@gmail.com>
Co-Authored-By: Tobias Nießen <tniessen@tnie.de>
Co-Authored-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
CVE-ID: CVE-2025-23166
PR-URL: https://github.com/nodejs-private/node-private/pull/688

src/crypto/crypto_dh.cc

@@ -512,15 +512,15 @@ MaybeLocal<Value> DHBitsTraits::EncodeOutput(Environment* env,
   return out->ToArrayBuffer(env);
 }
 
-bool DHBitsTraits::DeriveBits(
-    Environment* env,
-    const DHBitsConfig& params,
-    ByteSource* out) {
+bool DHBitsTraits::DeriveBits(Environment* env,
+                              const DHBitsConfig& params,
+                              ByteSource* out,
+                              CryptoJobMode mode) {
   auto dp = DHPointer::stateless(params.private_key.GetAsymmetricKey(),
                                  params.public_key.GetAsymmetricKey());
   if (!dp) {
-    bool can_throw =
-        per_process::v8_initialized && Isolate::TryGetCurrent() != nullptr;
+    bool can_throw = mode == CryptoJobMode::kCryptoJobSync;
+
     if (can_throw) {
       unsigned long err = ERR_get_error();  // NOLINT(runtime/int)
       if (err) ThrowCryptoError(env, err, "diffieHellman failed");

src/crypto/crypto_dh.h

@@ -103,10 +103,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::MaybeLocal<v8::Value> EncodeOutput(Environment* env,
                                                 const DHBitsConfig& params,

src/crypto/crypto_ec.cc

@@ -434,7 +434,8 @@ Maybe<void> ECDHBitsTraits::AdditionalConfig(
 
 bool ECDHBitsTraits::DeriveBits(Environment* env,
                                 const ECDHBitsConfig& params,
-                                ByteSource* out) {
+                                ByteSource* out,
+                                CryptoJobMode mode) {
   size_t len = 0;
   const auto& m_privkey = params.private_.GetAsymmetricKey();
   const auto& m_pubkey = params.public_.GetAsymmetricKey();

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::MaybeLocal<v8::Value> EncodeOutput(Environment* env,
                                                 const ECDHBitsConfig& params,

src/crypto/crypto_hash.cc

@@ -489,10 +489,10 @@ Maybe<void> HashTraits::AdditionalConfig(
   return JustVoid();
 }
 
-bool HashTraits::DeriveBits(
-    Environment* env,
-    const HashConfig& params,
-    ByteSource* out) {
+bool HashTraits::DeriveBits(Environment* env,
+                            const HashConfig& params,
+                            ByteSource* out,
+                            CryptoJobMode mode) {
   auto ctx = EVPMDCtxPointer::New();
 
   if (!ctx.digestInit(params.digest) || !ctx.digestUpdate(params.in))

src/crypto/crypto_hash.h

@@ -70,10 +70,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::MaybeLocal<v8::Value> EncodeOutput(Environment* env,
                                                 const HashConfig& params,

src/crypto/crypto_hkdf.cc

@@ -97,10 +97,10 @@ Maybe<void> HKDFTraits::AdditionalConfig(
   return JustVoid();
 }
 
-bool HKDFTraits::DeriveBits(
-    Environment* env,
-    const HKDFConfig& params,
-    ByteSource* out) {
+bool HKDFTraits::DeriveBits(Environment* env,
+                            const HKDFConfig& params,
+                            ByteSource* out,
+                            CryptoJobMode mode) {
   auto dp = ncrypto::hkdf(params.digest,
                           ncrypto::Buffer<const unsigned char>{
                               .data = reinterpret_cast<const unsigned char*>(

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::MaybeLocal<v8::Value> EncodeOutput(Environment* env,
                                                 const HKDFConfig& params,

src/crypto/crypto_hmac.cc

@@ -233,10 +233,10 @@ Maybe<void> HmacTraits::AdditionalConfig(
   return JustVoid();
 }
 
-bool HmacTraits::DeriveBits(
-    Environment* env,
-    const HmacConfig& params,
-    ByteSource* out) {
+bool HmacTraits::DeriveBits(Environment* env,
+                            const HmacConfig& params,
+                            ByteSource* out,
+                            CryptoJobMode mode) {
   auto ctx = HMACCtxPointer::New();
 
   ncrypto::Buffer<const void> key_buf{

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::MaybeLocal<v8::Value> EncodeOutput(Environment* env,
                                                 const HmacConfig& params,

src/crypto/crypto_pbkdf2.cc

@@ -112,7 +112,8 @@ Maybe<void> PBKDF2Traits::AdditionalConfig(
 
 bool PBKDF2Traits::DeriveBits(Environment* env,
                               const PBKDF2Config& params,
-                              ByteSource* out) {
+                              ByteSource* out,
+                              CryptoJobMode mode) {
   // Both pass and salt may be zero length here.
   auto dp = ncrypto::pbkdf2(params.digest,
                             ncrypto::Buffer<const char>{

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::MaybeLocal<v8::Value> EncodeOutput(Environment* env,
                                                 const PBKDF2Config& params,

src/crypto/crypto_random.cc

@@ -65,10 +65,10 @@ Maybe<void> RandomBytesTraits::AdditionalConfig(
   return JustVoid();
 }
 
-bool RandomBytesTraits::DeriveBits(
-    Environment* env,
-    const RandomBytesConfig& params,
-    ByteSource* unused) {
+bool RandomBytesTraits::DeriveBits(Environment* env,
+                                   const RandomBytesConfig& params,
+                                   ByteSource* unused,
+                                   CryptoJobMode mode) {
   return ncrypto::CSPRNG(params.buffer, params.size);
 }
 
@@ -154,7 +154,8 @@ Maybe<void> RandomPrimeTraits::AdditionalConfig(
 
 bool RandomPrimeTraits::DeriveBits(Environment* env,
                                    const RandomPrimeConfig& params,
-                                   ByteSource* unused) {
+                                   ByteSource* unused,
+                                   CryptoJobMode mode) {
   return params.prime.generate(
       BignumPointer::PrimeConfig{
           .bits = params.bits,
@@ -190,10 +191,10 @@ Maybe<void> CheckPrimeTraits::AdditionalConfig(
   return JustVoid();
 }
 
-bool CheckPrimeTraits::DeriveBits(
-    Environment* env,
-    const CheckPrimeConfig& params,
-    ByteSource* out) {
+bool CheckPrimeTraits::DeriveBits(Environment* env,
+                                  const CheckPrimeConfig& params,
+                                  ByteSource* out,
+                                  CryptoJobMode mode) {
   int ret = params.candidate.isPrime(params.checks, getPrimeCheckCallback(env));
   if (ret < 0) [[unlikely]]
     return false;

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::MaybeLocal<v8::Value> EncodeOutput(Environment* env,
                                                 const RandomBytesConfig& params,
@@ -67,10 +67,10 @@ struct RandomPrimeTraits final {
       unsigned int offset,
       RandomPrimeConfig* params);
 
-  static bool DeriveBits(
-      Environment* env,
-      const RandomPrimeConfig& params,
-      ByteSource* out_);
+  static bool DeriveBits(Environment* env,
+                         const RandomPrimeConfig& params,
+                         ByteSource* out_,
+                         CryptoJobMode mode);
 
   static v8::MaybeLocal<v8::Value> EncodeOutput(Environment* env,
                                                 const RandomPrimeConfig& params,
@@ -101,10 +101,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::MaybeLocal<v8::Value> EncodeOutput(Environment* env,
                                                 const CheckPrimeConfig& params,

src/crypto/crypto_scrypt.cc

@@ -113,10 +113,10 @@ Maybe<void> ScryptTraits::AdditionalConfig(
   return JustVoid();
 }
 
-bool ScryptTraits::DeriveBits(
-    Environment* env,
-    const ScryptConfig& params,
-    ByteSource* out) {
+bool ScryptTraits::DeriveBits(Environment* env,
+                              const ScryptConfig& params,
+                              ByteSource* out,
+                              CryptoJobMode mode) {
   // If the params.length is zero-length, just return an empty buffer.
   // It's useless, yes, but allowed via the API.
   if (params.length == 0) {

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::MaybeLocal<v8::Value> EncodeOutput(Environment* env,
                                                 const ScryptConfig& params,

src/crypto/crypto_sig.cc

@@ -636,11 +636,11 @@ Maybe<void> SignTraits::AdditionalConfig(
   return JustVoid();
 }
 
-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;
   auto context = EVPMDCtxPointer::New();
   if (!context) [[unlikely]]
     return false;
@@ -657,7 +657,7 @@ bool SignTraits::DeriveBits(
   })();
 
   if (!ctx.has_value()) [[unlikely]] {
-    crypto::CheckThrow(env, SignBase::Error::Init);
+    if (can_throw) crypto::CheckThrow(env, SignBase::Error::Init);
     return false;
   }
 
@@ -671,7 +671,7 @@ bool SignTraits::DeriveBits(
           : std::nullopt;
 
   if (!ApplyRSAOptions(key, *ctx, padding, salt_length)) {
-    crypto::CheckThrow(env, SignBase::Error::PrivateKey);
+    if (can_throw) crypto::CheckThrow(env, SignBase::Error::PrivateKey);
     return false;
   }
 
@@ -680,7 +680,7 @@ bool SignTraits::DeriveBits(
       if (key.isOneShotVariant()) {
         auto data = context.signOneShot(params.data);
         if (!data) [[unlikely]] {
-          crypto::CheckThrow(env, SignBase::Error::PrivateKey);
+          if (can_throw) crypto::CheckThrow(env, SignBase::Error::PrivateKey);
           return false;
         }
         DCHECK(!data.isSecure());
@@ -688,7 +688,7 @@ bool SignTraits::DeriveBits(
       } else {
         auto data = context.sign(params.data);
         if (!data) [[unlikely]] {
-          crypto::CheckThrow(env, SignBase::Error::PrivateKey);
+          if (can_throw) crypto::CheckThrow(env, SignBase::Error::PrivateKey);
           return false;
         }
         DCHECK(!data.isSecure());

src/crypto/crypto_sig.h

@@ -137,10 +137,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::MaybeLocal<v8::Value> EncodeOutput(Environment* env,
                                                 const SignConfiguration& params,

src/crypto/crypto_util.h

@@ -410,9 +410,11 @@ class DeriveBitsJob final : public CryptoJob<DeriveBitsTraits> {
             std::move(params)) {}
 
   void DoThreadPoolWork() override {
-    if (!DeriveBitsTraits::DeriveBits(
-            AsyncWrap::env(),
-            *CryptoJob<DeriveBitsTraits>::params(), &out_)) {
+    ncrypto::ClearErrorOnReturn clear_error_on_return;
+    if (!DeriveBitsTraits::DeriveBits(AsyncWrap::env(),
+                                      *CryptoJob<DeriveBitsTraits>::params(),
+                                      &out_,
+                                      this->mode())) {
       CryptoErrorStore* errors = CryptoJob<DeriveBitsTraits>::errors();
       errors->Capture();
       if (errors->Empty())

test/parallel/test-crypto-async-sign-verify.js

@@ -3,6 +3,7 @@ const common = require('../common');
 if (!common.hasCrypto)
   common.skip('missing crypto');
 
+const { hasOpenSSL3 } = require('../common/crypto');
 const assert = require('assert');
 const util = require('util');
 const crypto = require('crypto');
@@ -141,3 +142,28 @@ 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 = 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/);
+  }));
+}