src: mark/pop OpenSSL errors in NewRootCertStore

node.gyp

@@ -1361,6 +1361,9 @@
           'defines': [
             'HAVE_OPENSSL=1',
           ],
+          'sources': [
+            'test/cctest/test_node_crypto.cc',
+          ]
         }],
         [ 'node_use_openssl=="true" and experimental_quic==1', {
           'defines': [
@@ -1385,7 +1388,7 @@
            ]
         }],
         ['OS=="solaris"', {
-          'ldflags': [ '-I<(SHARED_INTERMEDIATE_DIR)' ]
+          'ldflags': [ '-I<(SHARED_INTERMEDIATE_DIR)' ],
         }],
         # Skip cctest while building shared lib node for Windows
         [ 'OS=="win" and node_shared=="true"', {

src/crypto/crypto_context.cc

@@ -187,12 +187,15 @@ int SSL_CTX_use_certificate_chain(SSL_CTX* ctx,
                                        issuer);
 }
 
-static X509_STORE* NewRootCertStore() {
+}  // namespace
+
+X509_STORE* NewRootCertStore() {
   static std::vector<X509*> root_certs_vector;
   static Mutex root_certs_vector_mutex;
   Mutex::ScopedLock lock(root_certs_vector_mutex);
 
-  if (root_certs_vector.empty()) {
+  if (root_certs_vector.empty() &&
+      per_process::cli_options->ssl_openssl_cert_store == false) {
     for (size_t i = 0; i < arraysize(root_certs); i++) {
       X509* x509 =
           PEM_read_bio_X509(NodeBIO::NewFixed(root_certs[i],
@@ -210,7 +213,9 @@ static X509_STORE* NewRootCertStore() {
 
   X509_STORE* store = X509_STORE_new();
   if (*system_cert_path != '\0') {
+    ERR_set_mark();
     X509_STORE_load_locations(store, system_cert_path, nullptr);
+    ERR_pop_to_mark();
   }
 
   Mutex::ScopedLock cli_lock(node::per_process::cli_options_mutex);
@@ -225,7 +230,6 @@ static X509_STORE* NewRootCertStore() {
 
   return store;
 }
-}  // namespace
 
 void GetRootCertificates(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);

src/crypto/crypto_context.h

@@ -21,6 +21,8 @@ void GetRootCertificates(
 void IsExtraRootCertsFileLoaded(
     const v8::FunctionCallbackInfo<v8::Value>& args);
 
+X509_STORE* NewRootCertStore();
+
 class SecureContext final : public BaseObject {
  public:
   ~SecureContext() override;

test/cctest/test_node_crypto.cc

@@ -0,0 +1,23 @@
+// This simulates specifying the configuration option --openssl-system-ca-path
+// and settting it to a file that does not exist.
+#define NODE_OPENSSL_SYSTEM_CERT_PATH "/missing/ca.pem"
+
+#include "crypto/crypto_context.h"
+#include "node_options.h"
+#include "openssl/err.h"
+#include "gtest/gtest.h"
+
+/*
+ * This test verifies that a call to NewRootCertDir with the build time
+ * configuration option --openssl-system-ca-path set to an missing file, will
+ * not leave any OpenSSL errors on the OpenSSL error stack.
+ * See https://github.com/nodejs/node/issues/35456 for details.
+ */
+TEST(NodeCrypto, NewRootCertStore) {
+  node::per_process::cli_options->ssl_openssl_cert_store = true;
+  X509_STORE* store = node::crypto::NewRootCertStore();
+  ASSERT_TRUE(store);
+  ASSERT_EQ(ERR_peek_error(), 0UL) << "NewRootCertStore should not have left "
+                                      "any errors on the OpenSSL error stack\n";
+  X509_STORE_free(store);
+}