crypto: decode PKCS#1 keys through providers

Import RSA public keys through OSSL_DECODER on OpenSSL 3 so the
resulting keys stay provider-backed. Preserve the PKCS#1 input structure
and the ASN.1 encodings accepted by the legacy decoder.

Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Assisted-by: Codex
PR-URL: #66108
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Author: panva

deps/ncrypto/ncrypto.cc

@@ -8,6 +8,9 @@
 #include <openssl/pkcs12.h>
 #include <openssl/rand.h>
 #include <openssl/x509v3.h>
+#if NCRYPTO_USE_OPENSSL3_PROVIDER
+#include <openssl/decoder.h>
+#endif
 #if NCRYPTO_USE_BORINGSSL_EVP_DO_ALL_FALLBACK
 #include <openssl/bytestring.h>
 #include <openssl/cipher.h>
@@ -3864,6 +3867,47 @@ EVPKeyPointer::operator const EC_KEY*() const {
 
 namespace {
 
+EVP_PKEY* DecodeRsaPublicKey(const unsigned char** data, size_t length) {
+#if NCRYPTO_USE_OPENSSL3_PROVIDER
+  // Borrow the EVP_PKEY constructor and its data from a context that stays
+  // alive until after the restricted decoder context is destroyed.
+  EVP_PKEY* raw = nullptr;
+  DeleteFnPtr<OSSL_DECODER_CTX, OSSL_DECODER_CTX_free> construct_ctx(
+      OSSL_DECODER_CTX_new_for_pkey(&raw,
+                                    "DER",
+                                    "type-specific",
+                                    KeyAlgorithm::RSA.name(),
+                                    EVP_PKEY_PUBLIC_KEY,
+                                    nullptr,
+                                    nullptr));
+  if (!construct_ctx) return nullptr;
+  auto* construct = OSSL_DECODER_CTX_get_construct(construct_ctx.get());
+  void* construct_data =
+      OSSL_DECODER_CTX_get_construct_data(construct_ctx.get());
+  if (construct == nullptr || construct_data == nullptr) return nullptr;
+
+  // Add only the type-specific RSA decoder: new_for_pkey() can also build
+  // chains that accept SPKI. The owning context retains the cleanup callback.
+  DeleteFnPtr<OSSL_DECODER, OSSL_DECODER_free> decoder(OSSL_DECODER_fetch(
+      nullptr, KeyAlgorithm::RSA.name(), "input=der,structure=type-specific"));
+  DeleteFnPtr<OSSL_DECODER_CTX, OSSL_DECODER_CTX_free> ctx(
+      OSSL_DECODER_CTX_new());
+  if (!decoder || !ctx ||
+      OSSL_DECODER_CTX_add_decoder(ctx.get(), decoder.get()) != 1 ||
+      OSSL_DECODER_CTX_set_input_type(ctx.get(), "DER") != 1 ||
+      OSSL_DECODER_CTX_set_selection(ctx.get(), EVP_PKEY_PUBLIC_KEY) != 1 ||
+      OSSL_DECODER_CTX_set_construct(ctx.get(), construct) != 1 ||
+      OSSL_DECODER_CTX_set_construct_data(ctx.get(), construct_data) != 1) {
+    return nullptr;
+  }
+  const int result = OSSL_DECODER_from_data(ctx.get(), data, &length);
+  EVPKeyPointer key(raw);
+  return result == 1 ? key.release() : nullptr;
+#else
+  return d2i_PublicKey(NID_rsaEncryption, nullptr, data, length);
+#endif
+}
+
 EVPKeyPointer::ParseKeyResult TryParsePublicKeyInner(const BIOPointer& bp,
                                                      const char* name,
                                                      auto&& parse) {
@@ -3991,7 +4035,7 @@ EVPKeyPointer::ParseKeyResult EVPKeyPointer::TryParsePublicKeyPEM(
           bp,
           "RSA PUBLIC KEY",
           [](const unsigned char** p, long l) {  // NOLINT(runtime/int)
-            return d2i_PublicKey(NID_rsaEncryption, nullptr, p, l);
+            return DecodeRsaPublicKey(p, l);
           })) {
     return ret;
   }
@@ -4026,7 +4070,7 @@ EVPKeyPointer::ParseKeyResult EVPKeyPointer::TryParsePublicKey(
   EVP_PKEY* key = nullptr;
 
   if (config.type == PKEncodingType::PKCS1 &&
-      (key = d2i_PublicKey(NID_rsaEncryption, nullptr, &start, buffer.len))) {
+      (key = DecodeRsaPublicKey(&start, buffer.len))) {
     return EVPKeyPointer::ParseKeyResult(EVPKeyPointer(key));
   }
 

test/cctest/test_node_crypto.cc

@@ -89,6 +89,193 @@ TEST(NodeCrypto, KeyAlgorithmNames) {
   EXPECT_FALSE(empty.isA(static_cast<const char*>(nullptr)));
 }
 
+#if NCRYPTO_USE_OPENSSL3_PROVIDER
+TEST(NodeCrypto, ProviderPkcs1PublicKeyImport) {
+  ncrypto::ClearErrorOnReturn clear_errors;
+  auto ctx = EVPKeyCtxPointer::NewFromAlgorithm(KeyAlgorithm::RSA);
+  ASSERT_TRUE(ctx);
+  ASSERT_TRUE(ctx.initForKeygen());
+  ASSERT_TRUE(ctx.setRsaKeygenBits(2048));
+  EVP_PKEY* raw = nullptr;
+  ASSERT_EQ(EVP_PKEY_keygen(ctx.get(), &raw), 1);
+  EVPKeyPointer key(raw);
+
+  for (const auto format :
+       {EVPKeyPointer::PKFormatType::PEM, EVPKeyPointer::PKFormatType::DER}) {
+    const EVPKeyPointer::PublicKeyEncodingConfig config(
+        false, format, EVPKeyPointer::PKEncodingType::PKCS1);
+    auto encoded = key.writePublicKey(config);
+    ASSERT_TRUE(encoded);
+    const BUF_MEM* mem = encoded.value;
+    ASSERT_NE(mem, nullptr);
+    const ncrypto::Buffer<const unsigned char> input{
+        reinterpret_cast<const unsigned char*>(mem->data), mem->length};
+    auto imported = EVPKeyPointer::TryParsePublicKey(config, input);
+    ASSERT_TRUE(imported);
+    EXPECT_NE(EVP_PKEY_get0_provider(imported.value.get()), nullptr);
+    EXPECT_TRUE(imported.value.isA(KeyAlgorithm::RSA));
+    EXPECT_EQ(EVP_PKEY_eq(key.get(), imported.value.get()), 1);
+  }
+}
+
+namespace {
+struct RsaLoadTestContext {
+  OSSL_FUNC_BIO_read_ex_fn* read = nullptr;
+  int selection = 0;
+  int loads = 0;
+  int frees = 0;
+};
+
+void* RsaLoadTestDecoderNew(void* context) {
+  return context;
+}
+
+void RsaLoadTestDecoderFree(void*) {}
+
+int RsaLoadTestDecode(void* context,
+                      OSSL_CORE_BIO* input,
+                      int selection,
+                      OSSL_CALLBACK* callback,
+                      void* arg,
+                      OSSL_PASSPHRASE_CALLBACK*,
+                      void*) {
+  auto* state = static_cast<RsaLoadTestContext*>(context);
+  unsigned char sentinel = 0;
+  size_t size = 0;
+  // Only exercise construction and reference ownership, not ASN.1 parsing.
+  if (!state->read(input, &sentinel, 1, &size) || size != 1 ||
+      sentinel != 0x42) {
+    return 0;
+  }
+  state->selection = selection;
+  char type[] = "RSA";
+  const OSSL_PARAM params[] = {
+      OSSL_PARAM_utf8_string(
+          OSSL_OBJECT_PARAM_DATA_TYPE, type, sizeof(type) - 1),
+      OSSL_PARAM_octet_string(
+          OSSL_OBJECT_PARAM_REFERENCE, &state, sizeof(state)),
+      OSSL_PARAM_END,
+  };
+  return callback(params, arg);
+}
+
+void* RsaLoadTestLoad(const void* reference, size_t size) {
+  if (size != sizeof(RsaLoadTestContext*)) return nullptr;
+  auto* state = *static_cast<RsaLoadTestContext* const*>(reference);
+  state->loads++;
+  return state;
+}
+
+void RsaLoadTestFree(void* context) {
+  static_cast<RsaLoadTestContext*>(context)->frees++;
+}
+
+int RsaLoadTestHas(const void*, int selection) {
+  return (selection & OSSL_KEYMGMT_SELECT_PRIVATE_KEY) == 0;
+}
+
+const OSSL_ALGORITHM* RsaLoadTestQuery(void*, int operation, int* no_cache) {
+  *no_cache = 0;
+  static const OSSL_DISPATCH decoder[] = {
+      {OSSL_FUNC_DECODER_NEWCTX,
+       reinterpret_cast<void (*)(void)>(RsaLoadTestDecoderNew)},
+      {OSSL_FUNC_DECODER_FREECTX,
+       reinterpret_cast<void (*)(void)>(RsaLoadTestDecoderFree)},
+      {OSSL_FUNC_DECODER_DECODE,
+       reinterpret_cast<void (*)(void)>(RsaLoadTestDecode)},
+      {0, nullptr},
+  };
+  static const OSSL_DISPATCH keymgmt[] = {
+      {OSSL_FUNC_KEYMGMT_LOAD,
+       reinterpret_cast<void (*)(void)>(RsaLoadTestLoad)},
+      {OSSL_FUNC_KEYMGMT_FREE,
+       reinterpret_cast<void (*)(void)>(RsaLoadTestFree)},
+      {OSSL_FUNC_KEYMGMT_HAS, reinterpret_cast<void (*)(void)>(RsaLoadTestHas)},
+      {0, nullptr},
+  };
+  static const OSSL_ALGORITHM decoders[] = {
+      {"RSA",
+       "provider=node-test-rsa-load,input=der,structure=type-specific",
+       decoder,
+       "Test RSA decoder without export"},
+      {nullptr, nullptr, nullptr, nullptr},
+  };
+  static const OSSL_ALGORITHM keymgmts[] = {
+      {"RSA",
+       "provider=node-test-rsa-load",
+       keymgmt,
+       "Test RSA reference load"},
+      {nullptr, nullptr, nullptr, nullptr},
+  };
+  if (operation == OSSL_OP_DECODER) return decoders;
+  return operation == OSSL_OP_KEYMGMT ? keymgmts : nullptr;
+}
+
+void RsaLoadTestTeardown(void* context) {
+  delete static_cast<RsaLoadTestContext*>(context);
+}
+
+int RsaLoadTestProviderInit(const OSSL_CORE_HANDLE*,
+                            const OSSL_DISPATCH* in,
+                            const OSSL_DISPATCH** out,
+                            void** context) {
+  auto state = std::make_unique<RsaLoadTestContext>();
+  for (; in->function_id != 0; in++) {
+    if (in->function_id == OSSL_FUNC_BIO_READ_EX) {
+      state->read = OSSL_FUNC_BIO_read_ex(in);
+    }
+  }
+  if (state->read == nullptr) return 0;
+  static const OSSL_DISPATCH dispatch[] = {
+      {OSSL_FUNC_PROVIDER_QUERY_OPERATION,
+       reinterpret_cast<void (*)(void)>(RsaLoadTestQuery)},
+      {OSSL_FUNC_PROVIDER_TEARDOWN,
+       reinterpret_cast<void (*)(void)>(RsaLoadTestTeardown)},
+      {0, nullptr},
+  };
+  *context = state.release();
+  *out = dispatch;
+  return 1;
+}
+}  // namespace
+
+TEST(NodeCrypto, ProviderPkcs1PublicKeyLoadWithoutExport) {
+  ncrypto::ClearErrorOnReturn clear_errors;
+  ncrypto::DeleteFnPtr<OSSL_LIB_CTX, OSSL_LIB_CTX_free> libctx(
+      OSSL_LIB_CTX_new());
+  ASSERT_TRUE(libctx);
+  ASSERT_EQ(OSSL_PROVIDER_add_builtin(
+                libctx.get(), "node-test-rsa-load", RsaLoadTestProviderInit),
+            1);
+  auto* provider = OSSL_PROVIDER_load(libctx.get(), "node-test-rsa-load");
+  auto unload_provider =
+      node::OnScopeLeave([provider] { OSSL_PROVIDER_unload(provider); });
+  ASSERT_NE(provider, nullptr);
+  auto* state = static_cast<RsaLoadTestContext*>(
+      OSSL_PROVIDER_get0_provider_ctx(provider));
+  OSSL_LIB_CTX* previous_libctx = OSSL_LIB_CTX_set0_default(libctx.get());
+  auto restore_libctx = node::OnScopeLeave(
+      [previous_libctx] { OSSL_LIB_CTX_set0_default(previous_libctx); });
+
+  // Neither decoder export nor keymgmt import is available in this provider.
+  const unsigned char sentinel[] = {0x42};
+  const EVPKeyPointer::PublicKeyEncodingConfig config(
+      false,
+      EVPKeyPointer::PKFormatType::DER,
+      EVPKeyPointer::PKEncodingType::PKCS1);
+  {
+    auto imported = EVPKeyPointer::TryParsePublicKey(config, {sentinel, 1});
+    ASSERT_TRUE(imported);
+    EXPECT_EQ(EVP_PKEY_get0_provider(imported.value.get()), provider);
+    EXPECT_TRUE(imported.value.isA(KeyAlgorithm::RSA));
+    EXPECT_EQ(state->selection, EVP_PKEY_PUBLIC_KEY);
+    EXPECT_EQ(state->loads, 1);
+    EXPECT_EQ(state->frees, 0);
+  }
+  EXPECT_EQ(state->frees, 1);
+}
+#endif
+
 TEST(NodeCrypto, UnsupportedRawExports) {
   using Error = EVPKeyPointer::RawExportError;
   EVPKeyPointer key;

test/parallel/test-crypto-rsa-pkcs1-public-key.js

@@ -0,0 +1,94 @@
+'use strict';
+
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+const assert = require('node:assert');
+const { createPrivateKey, createPublicKey, sign, verify } = require('node:crypto');
+const fixtures = require('../common/fixtures');
+const { hasOpenSSL, isBoringSSL } = require('../common/crypto');
+
+const privateKey = createPrivateKey(fixtures.readKey('rsa_private_2048.pem'));
+const publicKey = createPublicKey(privateKey);
+const der = publicKey.export({ format: 'der', type: 'pkcs1' });
+const pem = publicKey.export({ format: 'pem', type: 'pkcs1' });
+const data = Buffer.from('PKCS#1 public key import');
+const signature = sign('sha256', data, privateKey);
+
+for (const [key, format] of [
+  [der, 'der'],
+  [Buffer.concat([der, Buffer.from('trailing data')]), 'der'],
+  [pem, 'pem'],
+  [`leading data\n${pem}trailing data\n`, 'pem'],
+]) {
+  const imported = createPublicKey({ key, format, type: 'pkcs1' });
+  assert.strictEqual(imported.type, 'public');
+  assert.strictEqual(imported.asymmetricKeyType, 'rsa');
+  assert.deepStrictEqual(imported.asymmetricKeyDetails,
+                         publicKey.asymmetricKeyDetails);
+  assert.deepStrictEqual(imported.export({ format: 'der', type: 'pkcs1' }), der);
+  assert.strictEqual(imported.export({ format: 'pem', type: 'pkcs1' }), pem);
+  assert(verify('sha256', data, imported, signature));
+}
+
+// The public PKCS#1 decoder must reject truncated keys and other DER structures.
+for (const invalid of [
+  der.subarray(0, der.length - 1),
+  publicKey.export({ format: 'der', type: 'spki' }),
+]) {
+  assert.throws(() => createPublicKey({
+    key: invalid, format: 'der', type: 'pkcs1',
+  }), { name: 'Error' });
+  assert.throws(() => createPublicKey(
+    `-----BEGIN RSA PUBLIC KEY-----\n${invalid.toString('base64')}\n` +
+    '-----END RSA PUBLIC KEY-----\n',
+  ), { name: 'Error' });
+}
+
+// Public-key creation continues to recognize PKCS#1 private keys separately.
+const privateDer = privateKey.export({ format: 'der', type: 'pkcs1' });
+assert.deepStrictEqual(createPublicKey({
+  key: privateDer, format: 'der', type: 'pkcs1',
+}).export({ format: 'der', type: 'pkcs1' }), der);
+
+// Preserve the ASN.1 forms accepted by the legacy RSA BIGNUM decoder. These
+// tiny keys exercise parsing only, without performing RSA operations.
+if (!isBoringSSL) {
+  for (const hex of [
+    '30800201110201030000',  // Indefinite-length BER SEQUENCE.
+    '300702020011020103',  // Redundant modulus padding.
+    '300702810111020103',  // Non-minimal INTEGER length encoding.
+    '30800201110201030000ffff',  // BER with trailing data.
+  ]) {
+    const imported = createPublicKey({
+      key: Buffer.from(hex, 'hex'), format: 'der', type: 'pkcs1',
+    });
+    assert.strictEqual(imported.type, 'public');
+    assert.strictEqual(imported.asymmetricKeyType, 'rsa');
+  }
+
+  // OpenSSL 4 rejects empty INTEGERs in both legacy and provider decoders.
+  const emptyExponent = {
+    key: Buffer.from('30050201110200', 'hex'), format: 'der', type: 'pkcs1',
+  };
+  if (hasOpenSSL(4)) {
+    assert.throws(() => createPublicKey(emptyExponent), { name: 'Error' });
+  } else {
+    const imported = createPublicKey(emptyExponent);
+    assert.strictEqual(imported.type, 'public');
+    assert.strictEqual(imported.asymmetricKeyType, 'rsa');
+  }
+}
+
+for (const hex of [
+  '3080020111020103',  // Missing BER end-of-contents marker.
+  '30800201110201030201010000',  // Third INTEGER inside BER SEQUENCE.
+  '3006220111020103',  // Constructed INTEGER.
+  '3006020111040103',  // OCTET STRING in place of the exponent.
+  '3009020111020103020101',  // Third INTEGER inside DER SEQUENCE.
+]) {
+  assert.throws(() => createPublicKey({
+    key: Buffer.from(hex, 'hex'), format: 'der', type: 'pkcs1',
+  }), { name: 'Error' });
+}