src: fix error handling on async crypto operations

RafaelGSS · panva · tniessen · RafaelGSS · commit 6c57465920cf · 2025-05-13T12:19:56.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#710
diff --git a/src/crypto/crypto_dh.cc b/src/crypto/crypto_dh.cc
@@ -705,10 +705,10 @@ Maybe<bool> 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
@@ -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<bool> EncodeOutput(
       Environment* env,
diff --git a/src/crypto/crypto_ec.cc b/src/crypto/crypto_ec.cc
@@ -480,7 +480,8 @@ Maybe<bool> 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
@@ -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<bool> EncodeOutput(
       Environment* env,
diff --git a/src/crypto/crypto_hash.cc b/src/crypto/crypto_hash.cc
@@ -501,10 +501,10 @@ Maybe<bool> 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) {
   EVPMDCtxPointer ctx(EVP_MD_CTX_new());
 
   if (UNLIKELY(!ctx ||
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::Maybe<bool> EncodeOutput(
       Environment* env,
diff --git a/src/crypto/crypto_hkdf.cc b/src/crypto/crypto_hkdf.cc
@@ -100,10 +100,10 @@ Maybe<bool> 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
@@ -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<bool> EncodeOutput(
       Environment* env,
diff --git a/src/crypto/crypto_hmac.cc b/src/crypto/crypto_hmac.cc
@@ -220,10 +220,10 @@ Maybe<bool> 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
@@ -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<bool> EncodeOutput(
       Environment* env,
diff --git a/src/crypto/crypto_pbkdf2.cc b/src/crypto/crypto_pbkdf2.cc
@@ -111,10 +111,10 @@ Maybe<bool> 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
@@ -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<bool> EncodeOutput(
       Environment* env,
diff --git a/src/crypto/crypto_random.cc b/src/crypto/crypto_random.cc
@@ -56,10 +56,10 @@ Maybe<bool> 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<bool> 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<bool> 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
@@ -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<bool> 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<bool> 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<bool> EncodeOutput(
       Environment* env,
diff --git a/src/crypto/crypto_scrypt.cc b/src/crypto/crypto_scrypt.cc
@@ -124,10 +124,10 @@ Maybe<bool> 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
@@ -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<bool> EncodeOutput(
       Environment* env,
diff --git a/src/crypto/crypto_sig.cc b/src/crypto/crypto_sig.cc
@@ -713,11 +713,11 @@ Maybe<bool> 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;
   EVPMDCtxPointer context(EVP_MD_CTX_new());
   EVP_PKEY_CTX* ctx = nullptr;
 
@@ -729,7 +729,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 +740,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;
@@ -758,7 +758,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;
   }
 
@@ -772,7 +772,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);
@@ -781,7 +782,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;
         }
         *out = std::move(buf).release(len);
@@ -792,13 +794,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
@@ -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<bool> EncodeOutput(
       Environment* env,
diff --git a/src/crypto/crypto_util.h b/src/crypto/crypto_util.h
@@ -503,9 +503,10 @@ class DeriveBitsJob final : public CryptoJob<DeriveBitsTraits> {
             std::move(params)) {}
 
   void DoThreadPoolWork() override {
+    ClearErrorOnReturn clear_error_on_return;
     if (!DeriveBitsTraits::DeriveBits(
             AsyncWrap::env(),
-            *CryptoJob<DeriveBitsTraits>::params(), &out_)) {
+            *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
