From 8d178657abc38291891c7ff1d4f9f6ee925c14bb Mon Sep 17 00:00:00 2001
From: James M Snell <jasnell@gmail.com>
Date: Fri, 3 Jan 2025 15:22:12 -0800
Subject: [PATCH] crypto: make generatePrime/checkPrime interruptible

The `generatePrime` and `checkPrime` functions in the `crypto`
module are only somewhat interruptible. This change makes it
possible to interrupt these more reliably. Note that generating
overly large primes can still take a long time and may not be
interruptible as this mechanism relies on a callback to check
for stopping conditions but OpenSSL may perform a long running
operation without calling the callback right away.

Fixes: https://github.com/nodejs/node/issues/56449
PR-URL: https://github.com/nodejs/node/pull/56460
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
---
 doc/api/crypto.md                  | 14 +++++++++++
 src/crypto/crypto_random.cc        | 40 ++++++++++++++++++++++--------
 test/parallel/test-crypto-prime.js | 16 ++++++++++++
 3 files changed, 59 insertions(+), 11 deletions(-)

diff --git a/doc/api/crypto.md b/doc/api/crypto.md
index ecd379f694e441..966ba964ba2efe 100644
--- a/doc/api/crypto.md
+++ b/doc/api/crypto.md
@@ -3934,6 +3934,13 @@ By default, the prime is encoded as a big-endian sequence of octets
 in an {ArrayBuffer}. If the `bigint` option is `true`, then a {bigint}
 is provided.
 
+The `size` of the prime will have a direct impact on how long it takes to
+generate the prime. The larger the size, the longer it will take. Because
+we use OpenSSL's `BN_generate_prime_ex` function, which provides only
+minimal control over our ability to interrupt the generation process,
+it is not recommended to generate overly large primes, as doing so may make
+the process unresponsive.
+
 ### `crypto.generatePrimeSync(size[, options])`
 
 <!-- YAML
@@ -3975,6 +3982,13 @@ By default, the prime is encoded as a big-endian sequence of octets
 in an {ArrayBuffer}. If the `bigint` option is `true`, then a {bigint}
 is provided.
 
+The `size` of the prime will have a direct impact on how long it takes to
+generate the prime. The larger the size, the longer it will take. Because
+we use OpenSSL's `BN_generate_prime_ex` function, which provides only
+minimal control over our ability to interrupt the generation process,
+it is not recommended to generate overly large primes, as doing so may make
+the process unresponsive.
+
 ### `crypto.getCipherInfo(nameOrNid[, options])`
 
 <!-- YAML
diff --git a/src/crypto/crypto_random.cc b/src/crypto/crypto_random.cc
index b59e394d9a7e2c..2c90a796e1195b 100644
--- a/src/crypto/crypto_random.cc
+++ b/src/crypto/crypto_random.cc
@@ -28,6 +28,25 @@ using v8::Uint32;
 using v8::Value;
 
 namespace crypto {
+namespace {
+using BNGENCBPointer = DeleteFnPtr<BN_GENCB, BN_GENCB_free>;
+
+BNGENCBPointer getBN_GENCB(Environment* env) {
+  // The callback is used to check if the operation should be stopped.
+  // Currently, the only check we perform is if env->is_stopping()
+  // is true.
+  BNGENCBPointer cb(BN_GENCB_new());
+  BN_GENCB_set(
+      cb.get(),
+      [](int a, int b, BN_GENCB* cb) -> int {
+        Environment* env = static_cast<Environment*>(BN_GENCB_get_arg(cb));
+        return env->is_stopping() ? 0 : 1;
+      },
+      env);
+  return cb;
+}
+
+}  // namespace
 MaybeLocal<Value> RandomBytesTraits::EncodeOutput(
     Environment* env, const RandomBytesConfig& params, ByteSource* unused) {
   return v8::Undefined(env->isolate());
@@ -150,13 +169,14 @@ bool RandomPrimeTraits::DeriveBits(Environment* env,
   // Make sure the CSPRNG is properly seeded.
   CHECK(ncrypto::CSPRNG(nullptr, 0));
 
-  if (BN_generate_prime_ex(
-          params.prime.get(),
-          params.bits,
-          params.safe ? 1 : 0,
-          params.add.get(),
-          params.rem.get(),
-          nullptr) == 0) {
+  BNGENCBPointer cb = getBN_GENCB(env);
+
+  if (BN_generate_prime_ex(params.prime.get(),
+                           params.bits,
+                           params.safe ? 1 : 0,
+                           params.add.get(),
+                           params.rem.get(),
+                           cb.get()) == 0) {
     return false;
   }
 
@@ -189,12 +209,10 @@ bool CheckPrimeTraits::DeriveBits(
     ByteSource* out) {
 
   BignumCtxPointer ctx(BN_CTX_new());
+  BNGENCBPointer cb = getBN_GENCB(env);
 
   int ret = BN_is_prime_ex(
-            params.candidate.get(),
-            params.checks,
-            ctx.get(),
-            nullptr);
+      params.candidate.get(), params.checks, ctx.get(), cb.get());
   if (ret < 0) return false;
   ByteSource::Builder buf(1);
   buf.data<char>()[0] = ret;
diff --git a/test/parallel/test-crypto-prime.js b/test/parallel/test-crypto-prime.js
index 209c8251f78053..2e7edb9074d090 100644
--- a/test/parallel/test-crypto-prime.js
+++ b/test/parallel/test-crypto-prime.js
@@ -14,6 +14,8 @@ const {
   checkPrimeSync,
 } = require('crypto');
 
+const { Worker } = require('worker_threads');
+
 const { promisify } = require('util');
 const pgeneratePrime = promisify(generatePrime);
 const pCheckPrime = promisify(checkPrime);
@@ -295,3 +297,17 @@ assert.throws(() => {
     checkPrime(prime, common.mustSucceed(assert));
   }));
 }
+
+{
+  // Verify that generatePrime can be reasonably interrupted.
+  const worker = new Worker(`
+    const { generatePrime } = require('crypto');
+    generatePrime(2048, () => {
+      throw new Error('should not be called');
+    });
+    process.exit(42);
+  `, { eval: true });
+
+  worker.on('error', common.mustNotCall());
+  worker.on('exit', common.mustCall((exitCode) => assert.strictEqual(exitCode, 42)));
+}