src: fix error handling on async crypto operations

RafaelGSS · panva · tniessen · RafaelGSS · commit f4494d38f1e0 · 2025-05-13T15:25:41.000-03:00
Fixes: https://hackerone.com/reports/2817648 Co-Authored-By: Filip Skokan <panva.ip@gmail.com> Co-Authored-By: Tobias Nießen <tniessen@tnie.de> Backport-PR-URL: nodejs-private/node-private#688 CVE-ID: CVE-2025-23166 PR-URL: nodejs-private/node-private#709
diff --git a/src/crypto/crypto_dh.cc b/src/crypto/crypto_dh.cc
@@ -523,10 +523,10 @@ 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) {
   *out = StatelessDiffieHellmanThreadsafe(params.private_key.GetAsymmetricKey(),
                                           params.public_key.GetAsymmetricKey());
   return true;
diff --git a/src/crypto/crypto_dh.h b/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,
diff --git a/src/crypto/crypto_ec.cc b/src/crypto/crypto_ec.cc
@@ -450,7 +450,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();
diff --git a/src/crypto/crypto_ec.h b/src/crypto/crypto_ec.h
@@ -78,10 +78,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,
diff --git a/src/crypto/crypto_hash.cc b/src/crypto/crypto_hash.cc
@@ -502,10 +502,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) {
   EVPMDCtxPointer ctx(EVP_MD_CTX_new());
 
   if (!ctx || EVP_DigestInit_ex(ctx.get(), params.digest, nullptr) <= 0 ||
diff --git a/src/crypto/crypto_hash.h b/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,
diff --git a/src/crypto/crypto_hkdf.cc b/src/crypto/crypto_hkdf.cc
@@ -96,10 +96,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*>(
diff --git 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::MaybeLocal<v8::Value> EncodeOutput(Environment* env,
                                                 const HKDFConfig& params,
diff --git a/src/crypto/crypto_hmac.cc b/src/crypto/crypto_hmac.cc
@@ -221,10 +221,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) {
   HMACCtxPointer ctx(HMAC_CTX_new());
 
   if (!ctx || !HMAC_Init_ex(ctx.get(),
diff --git 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::MaybeLocal<v8::Value> EncodeOutput(Environment* env,
                                                 const HmacConfig& params,
diff --git a/src/crypto/crypto_pbkdf2.cc b/src/crypto/crypto_pbkdf2.cc
@@ -111,7 +111,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>{
diff --git 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::MaybeLocal<v8::Value> EncodeOutput(Environment* env,
                                                 const PBKDF2Config& params,
diff --git a/src/crypto/crypto_random.cc b/src/crypto/crypto_random.cc
@@ -64,10 +64,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) return false;
   ByteSource::Builder buf(1);
diff --git 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::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,
diff --git a/src/crypto/crypto_scrypt.cc b/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) {
diff --git 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::MaybeLocal<v8::Value> EncodeOutput(Environment* env,
                                                 const ScryptConfig& params,
diff --git a/src/crypto/crypto_sig.cc b/src/crypto/crypto_sig.cc
@@ -703,11 +703,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;
   EVPMDCtxPointer context(EVP_MD_CTX_new());
   EVP_PKEY_CTX* ctx = nullptr;
 
@@ -717,14 +717,14 @@ bool SignTraits::DeriveBits(
     case SignConfiguration::kSign:
       if (!EVP_DigestSignInit(
               context.get(), &ctx, params.digest, nullptr, key.get())) {
-        crypto::CheckThrow(env, SignBase::Error::kSignInit);
+        if (can_throw) crypto::CheckThrow(env, SignBase::Error::kSignInit);
         return false;
       }
       break;
     case SignConfiguration::kVerify:
       if (!EVP_DigestVerifyInit(
               context.get(), &ctx, params.digest, nullptr, key.get())) {
-        crypto::CheckThrow(env, SignBase::Error::kSignInit);
+        if (can_throw) crypto::CheckThrow(env, SignBase::Error::kSignInit);
         return false;
       }
       break;
@@ -738,7 +738,7 @@ bool SignTraits::DeriveBits(
       ? Just<int>(params.salt_length) : Nothing<int>();
 
   if (!ApplyRSAOptions(key, ctx, padding, salt_length)) {
-    crypto::CheckThrow(env, SignBase::Error::kSignPrivateKey);
+    if (can_throw) crypto::CheckThrow(env, SignBase::Error::kSignPrivateKey);
     return false;
   }
 
@@ -752,7 +752,8 @@ bool SignTraits::DeriveBits(
             &len,
             params.data.data<unsigned char>(),
             params.data.size())) {
-          crypto::CheckThrow(env, SignBase::Error::kSignPrivateKey);
+          if (can_throw)
+            crypto::CheckThrow(env, SignBase::Error::kSignPrivateKey);
           return false;
         }
         ByteSource::Builder buf(len);
@@ -772,13 +773,15 @@ bool SignTraits::DeriveBits(
                 params.data.data<unsigned char>(),
                 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<unsigned char>(), &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
@@ -146,10 +146,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,
diff --git a/src/crypto/crypto_util.h b/src/crypto/crypto_util.h
@@ -467,9 +467,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())
diff --git a/test/parallel/test-crypto-async-sign-verify.js b/test/parallel/test-crypto-async-sign-verify.js
