crypto: honor prime checks and support empty SEC1 passphrases

deps/ncrypto/ncrypto.cc

@@ -724,6 +724,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());
@@ -741,7 +743,17 @@ 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++) {
+    // BN_check_prime() may return without invoking the callback (e.g. small
+    // primes resolved by trial division), so check cancellation here too.
+    if (i > 0 && innerCb != nullptr && !innerCb(1, -1)) return -1;
+    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(
@@ -4496,14 +4508,17 @@ 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<char> passphrase) {
   if (passphrase.len > static_cast<size_t>(INT_MAX)) return false;
 
   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);
@@ -4513,7 +4528,7 @@ bool WriteEncryptedTraditionalPEM(BIO* bio,
   DERView der_view{der_storage.get<const unsigned char>(), der_len};
   return PEM_ASN1_write_bio(
              WriteDERView,
-             PEM_STRING_RSA,
+             label,
              bio,
              &der_view,
              cipher,
@@ -4727,8 +4742,13 @@ Result<BIOPointer, bool> 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(),
@@ -4821,13 +4841,23 @@ Result<BIOPointer, bool> 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<BIOPointer, bool>(false);

test/cctest/test_node_crypto.cc

@@ -491,6 +491,82 @@ 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, 3);
+  EXPECT_EQ(completed_checks, kMinimumPrimeChecks);
+  ERR_clear_error();
+}
+
+TEST(NodeCrypto, BignumPrimeCheckCancellationSmallPrime) {
+  // 311 is resolved by trial division, so OpenSSL never invokes the callback.
+  auto candidate = ncrypto::BignumPointer::New();
+  ASSERT_TRUE(candidate);
+  ASSERT_EQ(BN_set_word(candidate.get(), 311), 1);
+
+  int invocations = 0;
+  const int result = candidate.isPrime(INT_MAX, [&](int, int) {
+    invocations++;
+    return false;
+  });
+  EXPECT_EQ(result, -1);
+  EXPECT_EQ(invocations, 1);
+}
+#endif
+
 TEST(NodeCrypto, EcKeyComponents) {
   ncrypto::ClearErrorOnReturn clear_errors;
   ncrypto::BignumPointer x;

test/parallel/test-crypto-key-export-sec1-passphrase.js

@@ -0,0 +1,46 @@
+'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');
+  // A wrong CBC password can occasionally yield valid padding, in which case
+  // ASN.1 decoding fails instead.
+  assert.throws(() => createPrivateKey({
+    key: pem,
+    passphrase: Buffer.from('wrong'),
+  }), { code: /^ERR_OSSL_(BAD_DECRYPT|UNSUPPORTED)$/ });
+  assert.throws(() => createPrivateKey({ key: pem }), {
+    code: 'ERR_OSSL_CRYPTO_INTERRUPTED_OR_CANCELLED',
+  });
+}