Skip to content

Commit

Permalink
crypto: make generatePrime/checkPrime interruptible
Browse files Browse the repository at this point in the history
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: #56449
  • Loading branch information
jasnell committed Jan 4, 2025
1 parent 804d41f commit c06d35e
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 11 deletions.
14 changes: 14 additions & 0 deletions doc/api/crypto.md
Original file line number Diff line number Diff line change
Expand Up @@ -3940,6 +3940,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
Expand Down Expand Up @@ -3981,6 +3988,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
Expand Down
42 changes: 31 additions & 11 deletions src/crypto/crypto_random.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -150,13 +169,16 @@ 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);
BignumCtxPointer ctx(BN_CTX_new());

if (BN_generate_prime_ex2(params.prime.get(),
params.bits,
params.safe ? 1 : 0,
params.add.get(),
params.rem.get(),
cb.get(),
ctx.get()) == 0) {
return false;
}

Expand Down Expand Up @@ -189,12 +211,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;
Expand Down
16 changes: 16 additions & 0 deletions test/parallel/test-crypto-prime.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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)));
}

0 comments on commit c06d35e

Please sign in to comment.