crypto: handle incomplete RSA private keys

Treat missing private RSA parameters as an export failure instead of
passing null BIGNUM pointers to the JWK encoder. Also stop constructing
a usable RSA view when reading an optional parameter itself fails.

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

Author: panva

deps/ncrypto/ncrypto.cc

@@ -5685,12 +5685,14 @@ Rsa::Rsa(const EVP_PKEY* pkey) : Rsa() {
       !GetPKeyBnParam(pkey, OSSL_PKEY_PARAM_RSA_E, &e_)) {
     return;
   }
-  GetOptionalPKeyBnParam(pkey, OSSL_PKEY_PARAM_RSA_D, &d_);
-  GetOptionalPKeyBnParam(pkey, OSSL_PKEY_PARAM_RSA_FACTOR1, &p_);
-  GetOptionalPKeyBnParam(pkey, OSSL_PKEY_PARAM_RSA_FACTOR2, &q_);
-  GetOptionalPKeyBnParam(pkey, OSSL_PKEY_PARAM_RSA_EXPONENT1, &dp_);
-  GetOptionalPKeyBnParam(pkey, OSSL_PKEY_PARAM_RSA_EXPONENT2, &dq_);
-  GetOptionalPKeyBnParam(pkey, OSSL_PKEY_PARAM_RSA_COEFFICIENT1, &qi_);
+  if (!GetOptionalPKeyBnParam(pkey, OSSL_PKEY_PARAM_RSA_D, &d_) ||
+      !GetOptionalPKeyBnParam(pkey, OSSL_PKEY_PARAM_RSA_FACTOR1, &p_) ||
+      !GetOptionalPKeyBnParam(pkey, OSSL_PKEY_PARAM_RSA_FACTOR2, &q_) ||
+      !GetOptionalPKeyBnParam(pkey, OSSL_PKEY_PARAM_RSA_EXPONENT1, &dp_) ||
+      !GetOptionalPKeyBnParam(pkey, OSSL_PKEY_PARAM_RSA_EXPONENT2, &dq_) ||
+      !GetOptionalPKeyBnParam(pkey, OSSL_PKEY_PARAM_RSA_COEFFICIENT1, &qi_)) {
+    return;
+  }
 
   if (type == EVP_PKEY_RSA_PSS) {
     MarkPopErrorOnReturn pop_errors;

src/crypto/crypto_rsa.cc

@@ -315,6 +315,13 @@ bool ExportJWKRsaKey(Environment* env,
 
   if (key.GetKeyType() == kKeyTypePrivate) {
     auto pvt_key = rsa.getPrivateKey();
+    if (pub_key.d == nullptr || pvt_key.p == nullptr || pvt_key.q == nullptr ||
+        pvt_key.dp == nullptr || pvt_key.dq == nullptr ||
+        pvt_key.qi == nullptr) {
+      THROW_ERR_CRYPTO_OPERATION_FAILED(env,
+                                        "Failed to export RSA private key");
+      return false;
+    }
     if (SetEncodedValue(env, target, env->jwk_d_string(), pub_key.d)
             .IsNothing() ||
         SetEncodedValue(env, target, env->jwk_p_string(), pvt_key.p)

test/cctest/test_node_crypto_env.cc

@@ -1,11 +1,14 @@
 #include <ncrypto.h>
 #include "crypto/crypto_bio.h"
+#include "crypto/crypto_keys.h"
+#include "crypto/crypto_rsa.h"
 #include "gtest/gtest.h"
 #include "node_options.h"
 #include "node_test_fixture.h"
 #include "openssl/err.h"
 
 using v8::Local;
+using v8::Object;
 using v8::String;
 
 /*
@@ -31,3 +34,27 @@ TEST_F(NodeCryptoEnv, LoadBIO) {
   ASSERT_EQ(ERR_peek_error(), 0UL) << "There should not have left "
                                       "any errors on the OpenSSL error stack\n";
 }
+
+#if NCRYPTO_USE_OPENSSL3_PROVIDER
+TEST_F(NodeCryptoEnv, ExportIncompleteRsaPrivateKeyAsJwk) {
+  v8::HandleScope handle_scope(isolate_);
+  Argv argv;
+  Env env{handle_scope, argv};
+
+  ncrypto::Rsa rsa;
+  auto n = ncrypto::BignumPointer::New();
+  auto e = ncrypto::BignumPointer::New();
+  ASSERT_TRUE(n.setWord(3233));
+  ASSERT_TRUE(e.setWord(17));
+  ASSERT_TRUE(rsa.setPublicKey(std::move(n), std::move(e)));
+
+  auto pkey = ncrypto::EVPKeyPointer::NewRSA(rsa);
+  ASSERT_TRUE(pkey);
+  auto key = node::crypto::KeyObjectData::CreateAsymmetric(
+      node::crypto::kKeyTypePrivate, std::move(pkey));
+
+  v8::TryCatch try_catch(isolate_);
+  EXPECT_FALSE(node::crypto::ExportJWKRsaKey(*env, key, Object::New(isolate_)));
+  EXPECT_TRUE(try_catch.HasCaught());
+}
+#endif