From f1c2aab89cbd0005c928f838390b5844aef1e6cc Mon Sep 17 00:00:00 2001 From: Brad Anderson Date: Mon, 26 Oct 2026 16:23:36 -0400 Subject: [PATCH] crypto: fix ncrypto provider behavior Honor requested primality check counts with provider-backed OpenSSL. Support empty passphrases for encrypted SEC1 PEM output. Assisted-by: Codex Signed-off-by: Brad Anderson --- deps/ncrypto/ncrypto.cc | 51 ++++++++++++---- test/cctest/test_node_crypto.cc | 61 +++++++++++++++++++ .../test-crypto-key-export-sec1-passphrase.js | 44 +++++++++++++ 3 files changed, 144 insertions(+), 12 deletions(-) create mode 100644 test/parallel/test-crypto-key-export-sec1-passphrase.js diff --git a/deps/ncrypto/ncrypto.cc b/deps/ncrypto/ncrypto.cc index 5c20b482878..a13084dab7c 100644 --- a/deps/ncrypto/ncrypto.cc +++ b/deps/ncrypto/ncrypto.cc @@ -705,6 +705,8 @@ BignumPointer BignumPointer::clone() { int BignumPointer::isPrime(int nchecks, BignumPointer::PrimeCheckCallback innerCb) const { BignumCtxPointer ctx(BN_CTX_new()); + if (!ctx) [[unlikely]] + return -1; BignumGenCallbackPointer cb(nullptr); if (innerCb != nullptr) { cb = BignumGenCallbackPointer(BN_GENCB_new()); @@ -722,7 +724,14 @@ int BignumPointer::isPrime(int nchecks, &innerCb); } #if NCRYPTO_USE_OPENSSL_PROVIDER - return BN_check_prime(get(), ctx.get(), cb.get()); + const int minimum_checks = GetBitCount(get()) > 2048 ? 128 : 64; + const int repetitions = + nchecks > minimum_checks ? 1 + (nchecks - 1) / minimum_checks : 1; + for (int i = 0; i < repetitions; i++) { + const int result = BN_check_prime(get(), ctx.get(), cb.get()); + if (result != 1) return result; + } + return 1; #elif NCRYPTO_USE_BORINGSSL int is_probably_prime = 0; if (BN_primality_test( @@ -4219,6 +4228,9 @@ int WriteDERView(const void* x, unsigned char** out) { bool WriteEncryptedTraditionalPEM(BIO* bio, const EVP_PKEY* pkey, + int selection, + const char* structure, + const char* label, const EVP_CIPHER* cipher, Buffer passphrase) { if (passphrase.len > static_cast(INT_MAX)) return false; @@ -4226,7 +4238,7 @@ bool WriteEncryptedTraditionalPEM(BIO* bio, unsigned char* der = nullptr; size_t der_len = 0; OSSLEncoderCtxPointer ctx(OSSL_ENCODER_CTX_new_for_pkey( - pkey, OSSL_KEYMGMT_SELECT_KEYPAIR, "DER", "pkcs1", nullptr)); + pkey, selection, "DER", structure, nullptr)); if (!ctx) return false; const int result = OSSL_ENCODER_to_data(ctx.get(), &der, &der_len); @@ -4236,7 +4248,7 @@ bool WriteEncryptedTraditionalPEM(BIO* bio, DERView der_view{der_storage.get(), der_len}; return PEM_ASN1_write_bio( WriteDERView, - PEM_STRING_RSA, + label, bio, &der_view, cipher, @@ -4447,8 +4459,13 @@ Result EVPKeyPointer::writePrivateKey( const EVP_CIPHER* cipher = config.format == PKFormatType::PEM ? config.cipher.get() : nullptr; if (cipher != nullptr && passphrase.len == 0) { - err = - !WriteEncryptedTraditionalPEM(bio.get(), get(), cipher, passphrase); + err = !WriteEncryptedTraditionalPEM(bio.get(), + get(), + OSSL_KEYMGMT_SELECT_KEYPAIR, + "pkcs1", + PEM_STRING_RSA, + cipher, + passphrase); } else { err = !WriteEncodedPKey(bio.get(), get(), @@ -4539,13 +4556,23 @@ Result EVPKeyPointer::writePrivateKey( #if NCRYPTO_USE_OPENSSL_PROVIDER const EVP_CIPHER* cipher = config.format == PKFormatType::PEM ? config.cipher.get() : nullptr; - err = !WriteEncodedPKey(bio.get(), - get(), - OSSL_KEYMGMT_SELECT_ALL, - config.format, - "type-specific", - cipher, - passphrase); + if (cipher != nullptr && passphrase.len == 0) { + err = !WriteEncryptedTraditionalPEM(bio.get(), + get(), + OSSL_KEYMGMT_SELECT_ALL, + "type-specific", + PEM_STRING_ECPRIVATEKEY, + cipher, + passphrase); + } else { + err = !WriteEncodedPKey(bio.get(), + get(), + OSSL_KEYMGMT_SELECT_ALL, + config.format, + "type-specific", + cipher, + passphrase); + } #elif NCRYPTO_USE_BORINGSSL EC_KEY* ec = EVP_PKEY_get0_EC_KEY(get()); if (ec == nullptr) return Result(false); diff --git a/test/cctest/test_node_crypto.cc b/test/cctest/test_node_crypto.cc index 7658e1810cd..73a6503d75c 100644 --- a/test/cctest/test_node_crypto.cc +++ b/test/cctest/test_node_crypto.cc @@ -387,6 +387,67 @@ TEST(NodeCrypto, RsaPrimeProduct) { EXPECT_FALSE(rsa.checkPrimeProduct()); } +#if NCRYPTO_USE_OPENSSL_PROVIDER +namespace { +constexpr int kMinimumPrimeChecks = 64; + +bool CheckMersennePrime(int exponent, int requested_checks) { + auto candidate = ncrypto::BignumPointer::New(); + if (!candidate || BN_set_bit(candidate.get(), exponent) != 1 || + BN_sub_word(candidate.get(), 1) != 1) { + return false; + } + + int completed_checks = 0; + const int result = candidate.isPrime( + requested_checks, [&completed_checks](int event, int iteration) { + if (event == 1 && iteration >= 0) completed_checks++; + return true; + }); + return result == 1 && completed_checks >= requested_checks; +} +} // namespace + +TEST(NodeCrypto, BignumPrimeChecksHonorRequestedCount) { + EXPECT_TRUE(CheckMersennePrime(127, kMinimumPrimeChecks + 1)); + EXPECT_TRUE(CheckMersennePrime(2203, 129)); +} + +TEST(NodeCrypto, BignumPrimeCheckCancellation) { + auto candidate = ncrypto::BignumPointer::New(); + ASSERT_TRUE(candidate); + ASSERT_EQ(BN_set_bit(candidate.get(), 127), 1); + ASSERT_EQ(BN_sub_word(candidate.get(), 1), 1); + + int invocations = 0; + int completed_checks = 0; + bool valid_events = true; + const int result = + candidate.isPrime(kMinimumPrimeChecks + 1, [&](int event, int iteration) { + if (event != 1) { + valid_events = false; + return false; + } + if (iteration == -1) { + invocations++; + return true; + } + if (iteration != completed_checks % kMinimumPrimeChecks) { + valid_events = false; + return false; + } + if (completed_checks == kMinimumPrimeChecks) return false; + completed_checks++; + return true; + }); + EXPECT_EQ(result, -1); + EXPECT_TRUE(valid_events); + EXPECT_EQ(invocations, 2); + EXPECT_EQ(completed_checks, kMinimumPrimeChecks); + ERR_clear_error(); +} +#endif + TEST(NodeCrypto, EcKeyComponents) { ncrypto::ClearErrorOnReturn clear_errors; ncrypto::BignumPointer x; diff --git a/test/parallel/test-crypto-key-export-sec1-passphrase.js b/test/parallel/test-crypto-key-export-sec1-passphrase.js new file mode 100644 index 00000000000..ed02ec2144e --- /dev/null +++ b/test/parallel/test-crypto-key-export-sec1-passphrase.js @@ -0,0 +1,44 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +const { hasFIPS, isBoringSSL } = require('../common/crypto'); + +if (isBoringSSL) + common.skip('BoringSSL does not use OpenSSL provider encoders'); +if (hasFIPS(3)) + common.skip('SEC1 private key encoding is not supported in FIPS mode'); + +const assert = require('assert'); +const { + createPrivateKey, + generateKeyPairSync, +} = require('crypto'); + +const { privateKey } = generateKeyPairSync('ec', { + namedCurve: 'prime256v1', +}); + +for (const passphrase of [ + Buffer.alloc(0), + Buffer.from([0x62, 0x69, 0x00, 0x6e, 0x61, 0x72, 0x79]), +]) { + const pem = privateKey.export({ + type: 'sec1', + format: 'pem', + cipher: 'aes-128-cbc', + passphrase, + }); + assert.match(pem.toString(), /^-----BEGIN EC PRIVATE KEY-----\n/); + assert.strictEqual(createPrivateKey({ key: pem, passphrase }) + .asymmetricKeyType, 'ec'); + assert.throws(() => createPrivateKey({ + key: pem, + passphrase: Buffer.from('wrong'), + }), { code: 'ERR_OSSL_BAD_DECRYPT' }); + assert.throws(() => createPrivateKey({ key: pem }), { + code: 'ERR_OSSL_CRYPTO_INTERRUPTED_OR_CANCELLED', + }); +}