From a0616695e92f9889b8d2fd35f635eb00e1ca4e8b Mon Sep 17 00:00:00 2001 From: Brad Anderson Date: Mon, 26 Oct 2026 14:24:46 -0400 Subject: [PATCH] fix: preserve OpenSSL provider key behavior Signed-off-by: Brad Anderson --- src/ncrypto.cpp | 69 +++++++++++++++----- tests/basic.cpp | 169 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 221 insertions(+), 17 deletions(-) diff --git a/src/ncrypto.cpp b/src/ncrypto.cpp index 6e711af..d226d67 100644 --- a/src/ncrypto.cpp +++ b/src/ncrypto.cpp @@ -717,6 +717,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()); @@ -735,7 +737,14 @@ int BignumPointer::isPrime(int nchecks, &innerCb); } #if NCRYPTO_USE_OPENSSL3_PROVIDER - return BN_check_prime(get(), ctx.get(), cb.get()); + const int minimum_checks = bitLength() > 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( @@ -3670,6 +3679,8 @@ int WriteDERView(const void* x, unsigned char** out) { bool WriteEncryptedTraditionalPEM(BIO* bio, const EVP_PKEY* pkey, + const char* structure, + const char* label, const EVP_CIPHER* cipher, Buffer passphrase) { if (passphrase.len > static_cast(INT_MAX)) return false; @@ -3677,7 +3688,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, OSSL_KEYMGMT_SELECT_KEYPAIR, "DER", structure, nullptr)); if (!ctx) return false; const int result = OSSL_ENCODER_to_data(ctx.get(), &der, &der_len); @@ -3687,7 +3698,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, @@ -3700,14 +3711,29 @@ bool WriteEncryptedTraditionalPEM(BIO* bio, bool ECKeyHasMissingOid(const EVPKeyPointer& key) { if (key.id() != EVP_PKEY_EC) return false; - const Ec ec(key.get()); - const EC_GROUP* group = ec.getGroup(); - if (group == nullptr || - EC_GROUP_get_asn1_flag(group) != OPENSSL_EC_NAMED_CURVE) { + std::array encoding{}; + size_t encoding_len = 0; + if (EVP_PKEY_get_utf8_string_param(key.get(), + OSSL_PKEY_PARAM_EC_ENCODING, + encoding.data(), + encoding.size(), + &encoding_len) != 1 || + std::string_view(encoding.data(), encoding_len) != + OSSL_PKEY_EC_ENCODING_GROUP) { + return false; + } + + std::array group_name{}; + size_t group_name_len = 0; + if (EVP_PKEY_get_utf8_string_param(key.get(), + OSSL_PKEY_PARAM_GROUP_NAME, + group_name.data(), + group_name.size(), + &group_name_len) != 1) { return false; } - const int nid = EC_GROUP_get_curve_name(group); + const int nid = OBJ_txt2nid(group_name.data()); if (nid == NID_undef) return true; const ASN1_OBJECT* asn1 = OBJ_nid2obj(nid); @@ -3898,8 +3924,8 @@ Result EVPKeyPointer::writePrivateKey( const EVP_CIPHER* cipher = config.format == PKFormatType::PEM ? config.cipher : nullptr; if (cipher != nullptr && passphrase.len == 0) { - err = - !WriteEncryptedTraditionalPEM(bio.get(), get(), cipher, passphrase); + err = !WriteEncryptedTraditionalPEM( + bio.get(), get(), "pkcs1", PEM_STRING_RSA, cipher, passphrase); } else { err = !WriteEncodedPKey(bio.get(), get(), @@ -3979,13 +4005,22 @@ Result EVPKeyPointer::writePrivateKey( #if NCRYPTO_USE_OPENSSL3_PROVIDER const EVP_CIPHER* cipher = config.format == PKFormatType::PEM ? config.cipher : 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(), + "type-specific", + PEM_STRING_ECPRIVATEKEY, + cipher, + passphrase); + } else { + err = !WriteEncodedPKey(bio.get(), + get(), + OSSL_KEYMGMT_SELECT_ALL, + config.format, + "type-specific", + cipher, + passphrase); + } #else #if OPENSSL_VERSION_MAJOR >= 3 const EC_KEY* ec = EVP_PKEY_get0_EC_KEY(get()); diff --git a/tests/basic.cpp b/tests/basic.cpp index 8c86c84..08df7cf 100644 --- a/tests/basic.cpp +++ b/tests/basic.cpp @@ -2,7 +2,11 @@ #include #include +#include +#include #include +#include +#include using namespace ncrypto; @@ -136,6 +140,85 @@ static EVPKeyPointer createTestEcKey() { return key; } +#if NCRYPTO_USE_OPENSSL3_PROVIDER +static EVPKeyPointer createProviderKey(int id, + int curve = NID_undef, + int encoding = OPENSSL_EC_NAMED_CURVE) { + auto ctx = EVPKeyCtxPointer::NewFromID(id); + if (!ctx || !ctx.initForKeygen()) return {}; + if (id == EVP_PKEY_RSA && !ctx.setRsaKeygenBits(2048)) return {}; + if (id == EVP_PKEY_EC && !ctx.setEcParameters(curve, encoding)) return {}; + + EVP_PKEY* raw_key = nullptr; + if (EVP_PKEY_keygen(ctx.get(), &raw_key) != 1) return {}; + return EVPKeyPointer(raw_key); +} + +static Buffer bioBuffer(const BIOPointer& bio) { + BUF_MEM* mem = bio; + return { + .data = reinterpret_cast(mem->data), + .len = mem->length, + }; +} + +static DataPointer copyPassphrase( + const std::vector& passphrase) { + return DataPointer::Copy(Buffer{ + .data = passphrase.data(), + .len = passphrase.size(), + }); +} + +static void expectPrivateKeyRoundTrips(const EVPKeyPointer& key, + EVPKeyPointer::PKEncodingType type) { + using Format = EVPKeyPointer::PKFormatType; + + for (const auto format : {Format::DER, Format::PEM}) { + EVPKeyPointer::PrivateKeyEncodingConfig config(false, format, type); + auto encoded = key.writePrivateKey(config); + ASSERT_TRUE(encoded.has_value); + + auto parsed = + EVPKeyPointer::TryParsePrivateKey(config, bioBuffer(encoded.value)); + ASSERT_TRUE(parsed.has_value); + EXPECT_EQ(parsed.value.id(), key.id()); + } + + const std::array, 2> passphrases = { + std::vector{}, + std::vector{'b', 'i', 0, 'n', 'a', 'r', 'y'}, + }; + for (const auto& passphrase : passphrases) { + EVPKeyPointer::PrivateKeyEncodingConfig config(false, Format::PEM, type); + config.cipher = EVP_aes_256_cbc(); + config.passphrase = copyPassphrase(passphrase); + + auto encoded = key.writePrivateKey(config); + ASSERT_TRUE(encoded.has_value); + const auto encoded_buffer = bioBuffer(encoded.value); + + auto parsed = EVPKeyPointer::TryParsePrivateKey(config, encoded_buffer); + ASSERT_TRUE(parsed.has_value); + EXPECT_EQ(parsed.value.id(), key.id()); + + EVPKeyPointer::PrivateKeyEncodingConfig absent(false, Format::PEM, type); + auto absent_result = + EVPKeyPointer::TryParsePrivateKey(absent, encoded_buffer); + EXPECT_FALSE(absent_result.has_value); + ERR_clear_error(); + + EVPKeyPointer::PrivateKeyEncodingConfig wrong(false, Format::PEM, type); + wrong.passphrase = copyPassphrase({'w', 'r', 'o', 'n', 'g'}); + auto wrong_result = + EVPKeyPointer::TryParsePrivateKey(wrong, encoded_buffer); + EXPECT_FALSE(wrong_result.has_value); + EXPECT_EQ(wrong_result.error, EVPKeyPointer::PKParseError::FAILED); + ERR_clear_error(); + } +} +#endif + TEST(Ec, getDegree) { auto ecKey = createTestEcKey(); ASSERT_TRUE(ecKey); @@ -290,6 +373,92 @@ TEST(EVPKeyPointer, cloneEmpty) { ASSERT_FALSE(cloned); } +#if NCRYPTO_USE_OPENSSL3_PROVIDER +TEST(EVPKeyPointer, traditionalRsaPrivateKeyRoundTrips) { + auto key = createProviderKey(EVP_PKEY_RSA); + ASSERT_TRUE(key); + expectPrivateKeyRoundTrips(key, EVPKeyPointer::PKEncodingType::PKCS1); +} + +TEST(EVPKeyPointer, traditionalEcPrivateKeyRoundTrips) { + auto key = createProviderKey(EVP_PKEY_EC, NID_X9_62_prime256v1); + ASSERT_TRUE(key); + expectPrivateKeyRoundTrips(key, EVPKeyPointer::PKEncodingType::SEC1); +} + +TEST(EVPKeyPointer, explicitEcParametersPreserveSpki) { + using Format = EVPKeyPointer::PKFormatType; + using Type = EVPKeyPointer::PKEncodingType; + const EVPKeyPointer::PublicKeyEncodingConfig config( + false, Format::DER, Type::SPKI); + + for (const auto [curve, expected_size] : + {std::pair{NID_ipsec3, size_t{210}}, + std::pair{NID_ipsec4, size_t{238}}}) { + if (OBJ_nid2sn(curve) == nullptr) continue; + + auto explicit_key = + createProviderKey(EVP_PKEY_EC, curve, OPENSSL_EC_EXPLICIT_CURVE); + ASSERT_TRUE(explicit_key); + auto encoded = explicit_key.writePublicKey(config); + ASSERT_TRUE(encoded.has_value); + const auto original = bioBuffer(encoded.value); + EXPECT_EQ(original.len, expected_size); + + auto parsed = EVPKeyPointer::TryParsePublicKey(config, original); + ASSERT_TRUE(parsed.has_value); + auto reencoded = parsed.value.writePublicKey(config); + ASSERT_TRUE(reencoded.has_value); + const auto round_trip = bioBuffer(reencoded.value); + ASSERT_EQ(round_trip.len, original.len); + EXPECT_EQ(memcmp(round_trip.data, original.data, original.len), 0); + + auto named_key = + createProviderKey(EVP_PKEY_EC, curve, OPENSSL_EC_NAMED_CURVE); + ASSERT_TRUE(named_key); + auto missing_oid = named_key.writePublicKey(config); + EXPECT_FALSE(missing_oid.has_value); + ERR_clear_error(); + } +} + +static bool checkMersennePrime(int exponent, int requested_checks) { + BignumPointer candidate(BN_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; +} + +TEST(BignumPointer, repeatedProviderPrimeChecks) { + EXPECT_TRUE(checkMersennePrime(127, 65)); + EXPECT_TRUE(checkMersennePrime(2203, 129)); +} + +TEST(BignumPointer, providerPrimeCheckCancellation) { + BignumPointer candidate(BN_new()); + ASSERT_TRUE(candidate); + ASSERT_EQ(BN_set_word(candidate.get(), 65537), 1); + + int callbacks = 0; + const int result = candidate.isPrime(65, [&callbacks](int, int) { + callbacks++; + return false; + }); + EXPECT_EQ(result, -1); + EXPECT_EQ(callbacks, 1); + ERR_clear_error(); +} +#endif + // ============================================================================ // KDF tests