tls: include OpenSSL CAs in default CA list

[Archkon]

Fixes: #64258

[nodejs-github-bot]

Review requested:

• @nodejs/crypto
• @nodejs/net

[Archkon]

@joyeecheung
I’m not sure whether we should add support for the OpenSSL CA flag when developers pass the default type.

The existing behavior with OpenSSL CAs is that, during TLS connection setup, OpenSSL performs hash-based lookups for certificates. However, that behavior is not synchronized with what tls.getCACertificates('default') returns.

Another possibility would be to allow the default type to accept a second lookup argument, but that seems to duplicate the purpose of adding a separate openssl type.

Could you review the code by the way ? I really don't know how to support subject parse which maybe implement a
OpenSSL/RFC2253 subject parser ?

[joyeecheung]

Another possibility would be to allow the default type to accept a second lookup argument, but that seems to duplicate the purpose of adding a separate openssl type.

There's also NODE_EXTRA_CA_CERTS to consider:

// Flags: --use-openssl-ca
tls.getCACertificates('default');   // Either warn or error about no issuer name to filter for
tls.getCACertificates('default', cert);  // Does not warn and perform a similar hash lookup for system certs, + extra certs from NODE_EXTRA_CA_CERTS if any
tls.getCACertificates('openssl', cert);  // Similar, but no certs from NODE_EXTRA_CA_CERTS

[Archkon]

Another possibility would be to allow the default type to accept a second lookup argument, but that seems to duplicate the purpose of adding a separate openssl type.

There's also NODE_EXTRA_CA_CERTS to consider:

// Flags: --use-openssl-ca
tls.getCACertificates('default');   // Either warn or error about no issuer name to filter for
tls.getCACertificates('default', cert);  // Does not warn and perform a similar hash lookup for system certs, + extra certs from NODE_EXTRA_CA_CERTS if any
tls.getCACertificates('openssl', cert);  // Similar, but no certs from NODE_EXTRA_CA_CERTS

@joyeecheung
So are we only allowing this API to accept certificates as valid arguments, while dropping support for subject name ? And one more thing, I need calrification about how to deal with developers who call tls.getCACertificates('openssl', cert);without --use-openssl-ca as flag to pass? allow or not?

[codecov]

Codecov Report

❌ Patch coverage is 79.37500% with 33 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.22%. Comparing base (116302d) to head (fc25c33).

Files with missing lines	Patch %	Lines
src/crypto/crypto_context.cc	68.29%	13 Missing and 13 partials ⚠️
lib/tls.js	91.02%	6 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #64269      +/-   ##
==========================================
- Coverage   90.25%   90.22%   -0.03%     
==========================================
  Files         741      741              
  Lines      241339   241483     +144     
  Branches    45476    45506      +30     
==========================================
+ Hits       217810   217870      +60     
- Misses      15084    15147      +63     
- Partials     8445     8466      +21     

Files with missing lines	Coverage Δ
lib/tls.js	92.64% <91.02%> (-0.50%)	⬇️
src/crypto/crypto_context.cc	72.23% <68.29%> (-0.04%)	⬇️

... and 29 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.