diff --git a/node.gyp b/node.gyp index dd9bddd08ba2da..c4bf4411cfdcc1 100644 --- a/node.gyp +++ b/node.gyp @@ -1360,6 +1360,9 @@ 'defines': [ 'HAVE_OPENSSL=1', ], + 'sources': [ + 'test/cctest/test_node_crypto.cc', + ] }], [ 'node_use_openssl=="true" and experimental_quic==1', { 'defines': [ diff --git a/src/crypto/crypto_context.cc b/src/crypto/crypto_context.cc index d0210079166fb4..856b34e54a9a5c 100644 --- a/src/crypto/crypto_context.cc +++ b/src/crypto/crypto_context.cc @@ -186,12 +186,15 @@ int SSL_CTX_use_certificate_chain(SSL_CTX* ctx, issuer); } -static X509_STORE* NewRootCertStore() { +} // namespace + +X509_STORE* NewRootCertStore() { static std::vector 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], @@ -209,7 +212,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); @@ -224,7 +229,6 @@ static X509_STORE* NewRootCertStore() { return store; } -} // namespace void GetRootCertificates(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); diff --git a/src/crypto/crypto_context.h b/src/crypto/crypto_context.h index e36a76c5a8812a..5a2126c2ea3b1f 100644 --- a/src/crypto/crypto_context.h +++ b/src/crypto/crypto_context.h @@ -21,6 +21,8 @@ void GetRootCertificates( void IsExtraRootCertsFileLoaded( const v8::FunctionCallbackInfo& args); +X509_STORE* NewRootCertStore(); + class SecureContext final : public BaseObject { public: using GetSessionCb = SSL_SESSION* (*)(SSL*, const unsigned char*, int, int*); diff --git a/test/cctest/test_node_crypto.cc b/test/cctest/test_node_crypto.cc new file mode 100644 index 00000000000000..c0ce5f25496cd3 --- /dev/null +++ b/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); +}