src: enable X509sToArrayOfStrings to skip errors

[StefanStojanovic]

As suggested in the linked issue, this PR relaxes the X509 to PEM conversion by skipping failures for system certs. If conversion fails, Node.js will try to give info about the failed certs if it can. This new behavior is used only on system certs, since for others, the user has control. AFAIK, there is no way to add a reliable test for this, but if someone has an idea, feel free to share it with me.

Fixes: #61636

[nodejs-github-bot]

Review requested:

• @nodejs/crypto

[codecov]

Codecov Report

❌ Patch coverage is 22.58065% with 24 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.72%. Comparing base (9a237cd) to head (88ad793).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/crypto/crypto_context.cc	22.58%	22 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #61784      +/-   ##
==========================================
- Coverage   89.73%   89.72%   -0.02%     
==========================================
  Files         675      675              
  Lines      204674   204700      +26     
  Branches    39330    39338       +8     
==========================================
  Hits       183667   183667              
- Misses      13283    13307      +24     
- Partials     7724     7726       +2     

Files with missing lines	Coverage Δ
src/crypto/crypto_context.cc	69.62% <22.58%> (-1.22%)	⬇️

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

[addaleax]

Suggested change

	std::string subject_str = "<unknown>";
	if (subject_bio) {
	auto subject_size =
	BIO_get_mem_data(subject_bio.get(), &subject_data);
	if (subject_size > 0 && subject_data) {
	subject_str = std::string(subject_data, subject_size);
	std::string_view subject_str = "<unknown>";
	if (subject_bio) {
	auto subject_size =
	BIO_get_mem_data(subject_bio.get(), &subject_data);
	if (subject_size > 0 && subject_data) {
	subject_str = std::string_view(subject_data, subject_size);

No good reason to do an extra heap allocation here

[addaleax]

Suggested change

	subject_str.c_str());
	subject_str);

[joyeecheung]

I wonder if we should make this an option to the API instead of unconditionally emitting a warning? So if the user uses tls.getCACertifcate, they can choose warn, ignore, throw (or even get a list of errors on the side) when there's an error in parsing these certificates. For NODE_USE_SYSTEM_CA/--use-system-ca, warn is a sensible behavior.