Skip to content

src: fix CAs missing from secure contexts

  1. Adds CAs from NODE_EXTRA_CA_CERTS to root_certs_vector in node_crypto.cc so that the extra certificates are always added to SecureContext instances. The extra certificates were omitted if crl or pfx were specified as options to createSecureContext.

  2. tls.rootCertificates restored to previous behavior of returning built-in Node.js certificates (+NODE_EXTRA_CA_CERTS) when --openssl-use-def-ca-store CLI option or NODE_OPENSSL_SYSTEM_CERT_PATH compiler define are set.

Fixes: https://github.com/nodejs/node/issues/32229 Fixes: https://github.com/nodejs/node/issues/32010 Refs: https://github.com/nodejs/node/pull/32075

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Notes for reviewers
  • The call to X509_up_ref when building the root_certs_vector in the old NewRootCertStore() function was a double reference counting bug. X509_STORE_add_cert increments the reference count.
  • Scopes were used to keep the root_certs_vector_mutex lock to an absolute minimum and avoid any unnecessary function calls within the lock.
  • AddCertsFromFile was changed to AddRootCertsFromFile to make it possible to directly modify root_certs_vector. The function is not used anywhere else.
  • tls.rootCertificates returning the built-in node.js root CAs when --openssl-use-def-ca-store is set was the behavior v13.11.0 and has been restored in this PR. Returning the OpenSSL certificates from the file system caused syncronous IO and leaving the behavior of v13.11.0 of returning a blank array created a high risk of a breaking change and was non-deterministic when a file system certificate was cached.
  • Please carefully review changes for thread-safety. Fairly certain I got it right, but there may be some nuances to node.js multithreading I'm unaware of.

Merge request reports

Loading