crypto: segfault in X509Certificate.toLegacyObject() / tls.TLSSocket.getPeerCertificate() for EC certs on curves without a NIST name (e.g. brainpool), regression in v23.7.0 #66681

Version

v23.7.0 and every later release I tested, including v24.0.0 and v24.21.0 (newest 24.x at the time of writing). main has the same code. Last good: v23.6.1 (and v22.x, v20.x)

Platform

Linux x64 (glibc). Not platform specific as far as I can tell.

Subsystem

No response

What steps will reproduce the bug?

import { X509Certificate } from 'node:crypto';
const pem = `-----BEGIN CERTIFICATE-----
MIIBFDCBvKADAgECAgEBMAoGCCqGSM49BAMCMBQxEjAQBgNVBAMMCWxvY2FsaG9z
dDAeFw0yNjEwMTIxMjU1NDZaFw0yNjEwMTMxMzU1NDZaMBQxEjAQBgNVBAMMCWxv
Y2FsaG9zdDBaMBQGByqGSM49AgEGCSskAwMCCAEBBwNCAAQcoLg3iHP+AvE19WXd
myS0iU3s0ROJmPy/2Q0bF4YwEFBMuYKwNa39vmmNP3zIN6Gib9OKE3OY0b6xyBuD
dS6pMAoGCCqGSM49BAMCA0cAMEQCIBrZpi0P8YS2XEUwKSynJsunnTyVJQ9vOwpr
ByVIRFQrAiAxekTmNX9T/EbkY4HLodsDddq5U316rAX8kD63VAtK9w==
-----END CERTIFICATE-----
`; // self-signed, EC public key on brainpoolP256r1
const c = new X509Certificate(pem);
console.log(c.publicKey.asymmetricKeyDetails); // { namedCurve: 'brainpoolP256r1' }
console.log(c.toLegacyObject().nistCurve);     // crashes here

The same crash happens in a real TLS connection. When the peer's certificate chain is trusted (for example through NODE_EXTRA_CA_CERTS), tls.connect() crashes right after the handshake completes, without the application calling anything on the socket.
getPeerCertificate() crashes too.
When verification fails, Node skips the peer-certificate read and the process survives.

How often does it reproduce? Is there a required condition?

Always.

What is the expected behavior? Why is that the expected behavior?

toLegacyObject() / getPeerCertificate() return the legacy object, with nistCurve: undefined for curves that have no NIST name. This is what v23.6.1 and v22.x do.

What do you see instead?

Segmentation fault (core dumped), exit code 139. The native backtrace from our production cores ends in strlen.

Bisect

v23.6.1 and v23.7.0 both bundle OpenSSL 3.0.15, so an OpenSSL change is not the trigger.
Release-to-release bisect only, not a commit bisect.

Additional information

The v23.7.0 changelog includes the ncrypto EC refactors (#56526, #56421). GetCurveName in src/crypto/crypto_x509.cc changed shape:

// v23.6.1
template <const char* (*nid2string)(int nid)>
MaybeLocal<Value> GetCurveName(Environment* env, const int nid) {
  const char* name = nid2string(nid);
  return name != nullptr ? MaybeLocal<Value>(OneByteString(env->isolate(), name))
                         : MaybeLocal<Value>(Undefined(env->isolate()));
}

// v23.7.0 and later, including main
template <const char* (*nid2string)(int nid)>
MaybeLocal<Value> GetCurveName(Environment* env, const int nid) {
  std::string_view name = nid2string(nid);   // <-- nid2string() can return NULL
  return name.size() ? ... : Undefined(...);
}

It is instantiated with EC_curve_nid2nist, which returns NULL for any curve without a NIST name (brainpool, for example).
std::string_view(const char*) calls strlen() on its argument, so NULL crashes. This matches the strlen frame in our cores.
I have not built Node to test a fix.
A likely fix is to keep the null check:

const char* s = nid2string(nid);
std::string_view name = s != nullptr ? std::string_view(s) : std::string_view();

I have not checked whether other string_view-from-C-string conversions introduced in the same refactor have the same problem.

Impact

Brainpool curves are mandatory in the German healthcare PKI (gematik TI connectors use brainpoolP256r1 certificates). Any Node >= 23.7 client that trusts such a server's chain crashes on connect.