tls: implement tls.getCACertificates()

[joyeecheung]

To accompany --use-system-ca, this adds a new API that allows querying various kinds of CA certificates.

• If the first argument type is "default" or undefined, it returns the CA certificates that will be used by Node.js TLS clients by default, which includes the Mozilla CA if --use-bundled-ca is enabled or --use-openssl-ca is not enabled, and the system certificates if --use-system-ca is enabled, and the extra certificates if NODE_EXTRA_CA_CERTS is used.
• If type is "system" this returns the system certificates, regardless of whether --use-system-ca is enabeld or not.
• If type is "bundled" this is the same as tls.rootCertificates and returns the Mozilla CA certificates.
• If type is "extra" this returns the certificates parsed from the path specified by NODE_EXTRA_CA_CERTS.

Drive-by: remove the inaccurate description in tls.rootCertificates about including system certificates, since it in fact does not include them, and also it is contradicting the previous description about tls.rootCertificates always returning the Mozilla CA store and staying the same across platforms.

---

I've considered just adding the certificates to tls.rootCertificates but ended up going with a new API instead with the following considerations:

1. tls.rootCertificates has been documented explicitly as:

   An immutable array of strings representing the root certificates (in PEM format)
   from the bundled Mozilla CA store as supplied by the current Node.js version.
   The bundled CA store, as supplied by Node.js, is a snapshot of Mozilla CA store
   that is fixed at release time. It is identical on all supported platforms.

   And users might have been relying on this description and do not expect any other additional certificates to be added in.

2. Historically, tls.rootCertificates has not been affected by --use-openssl-ca or NODE_EXTRA_CA_CERTS, either, which further enforces the documented invariant. Users might get caught off-guard if we start to add system certificates etc. into this array, especially considering the certificates may contain private information in the issuer and subject part (e.g. employer or other identity information), at least if it's not done in a semver-major.

3. Unlike the bundled certificates, CA certificates from the system store may or may not be root certificates. So the name tls.rootCertificates is somewhat misleading if it's repurposed to include the intermediate CA certificates.

[nodejs-github-bot]

Review requested:

• @nodejs/crypto
• @nodejs/net

[codecov]

Codecov Report

Attention: Patch coverage is 90.97744% with 12 lines in your changes missing coverage. Please review.

Project coverage is 90.24%. Comparing base (8c2df73) to head (85599bf).
Report is 90 commits behind head on main.

Files with missing lines	Patch %	Lines
src/crypto/crypto_context.cc	77.77%	6 Missing and 6 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #57107      +/-   ##
==========================================
- Coverage   90.34%   90.24%   -0.11%     
==========================================
  Files         629      630       +1     
  Lines      184403   185278     +875     
  Branches    36040    36232     +192     
==========================================
+ Hits       166607   167197     +590     
- Misses      10919    11089     +170     
- Partials     6877     6992     +115     

Files with missing lines	Coverage Δ
lib/tls.js	96.00% <100.00%> (+0.84%)	⬆️
src/crypto/crypto_context.cc	69.34% <77.77%> (+4.25%)	⬆️

... and 67 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/65274/

[jasnell]

Mostly just a nit so feel free to ignore... it would be nice to be able to allocate the array at once rather than using multiple individual array.push(...) calls.

[joyeecheung]

I think this is different in JS - in JS if the array is preallocated, this becomes a holey array. using array.push keeps it compact, which can have perf implications. See https://v8.dev/blog/elements-kinds

[jasnell]

Since you're using X509View here anyway, it would be nice to move away from the raw pointers in this std::vector. Can we make this an std::vector<ncrypto::X509View>& ?

[joyeecheung]

I've considered this but it seems a bit too complicated for this PR, because we get the raw pointers from the function-local static storage for thread safety. Passing in std::vector<ncrypto::X509View> means doing another loop to convert them into vectors of X509View first, and then this must be called for all the caller before X509sToArrayOfStrings. It seems like a lot of extra code/slight overhead for no particular gains. Unless ncrypto provides some kind of helper to convert vectors of X509 into vectors of X509View automatically.

[joyeecheung]

There are some failures in the dynamically linked OpenSSL builds that I cannot reproduce locally - I am not sure if it has something to do with the specific configuration of those containers in the CI. Retrying after I did some fix ups in the tests - if it's still failing I'll need to log into the containers to see what's going on.

[joyeecheung]

I managed to reproduce the container errors locally, FWIW this is what I used to build the them locally https://gist.github.com/joyeecheung/2a27e16ee47a018504b79b77257937fb - there is some memory error, but I built it on a plane and did not save the build, so will need to rebuild and try debugging later when I am back from vacation.

[joyeecheung]

Somehow it seems uv_fs_req_cleanup can't be called on the unused uv_fs_t, though I recall that uv_fs_req_cleanup was able to handle that...? Anyway this seems to fix the crash, running the CI to verify, though this might be better as a separate PR because it technically fixes --use-system-ca as well.