src: fix CAs missing from secure contexts
-
Adds CAs from
NODE_EXTRA_CA_CERTS
toroot_certs_vector
innode_crypto.cc
so that the extra certificates are always added to SecureContext instances. The extra certificates were omitted ifcrl
orpfx
were specified as options tocreateSecureContext
. -
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 orNODE_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), orvcbuild 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 theroot_certs_vector
in the oldNewRootCertStore()
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 toAddRootCertsFromFile
to make it possible to directly modifyroot_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.