tls: read the peer certificate chain without consuming it

Project: nodejs/node | Language: JavaScript, C++ | Concepts: memory-management, api-design, error-handling, testing-strategy | Difficulty: senior
Link: https://github.com/nodejs/node/pull/65602

The problem

In Node.js v26.8.0, calling getPeerCertificate(true) on the server stopped returning the full client certificate issuer chain; the issuerCertificate property became undefined. This was due to the internal logic destructively reading the peer certificate chain: after the first read, the chain was gone, so subsequent calls returned incomplete data.

Why it's non-trivial

A naive fix might simply try to cache or re-read the certificate chain, but the underlying OpenSSL stack (SSL_get_peer_cert_chain()) gives access to memory it owns, and mutating or freeing it can break future reads or corrupt state. The code previously used sk_X509_delete() to walk the chain, which removed entries as it went, breaking later reads. Fixing this requires careful, non-destructive access, and a way to check for the presence of a peer certificate without building expensive JS objects.

The approach

1. Replace destructive read with non-destructive traversal

Previously, the code would consume the issuer chain by deleting entries as it built the JS objects. The new approach clones each certificate using X509_dup (or equivalent), leaving the original OpenSSL stack untouched.

+  Local<Object> issuer;
+  if (issuer_chain != nullptr && start < sk_X509_num(issuer_chain)) {
+    X509Pointer issuer_cert =
+        X509View(sk_X509_value(issuer_chain, start)).clone();
+    if (!issuer_cert) [[unlikely]] {
+      return MaybeLocal<Object>();
+    }
+    if (!NewWithIssuers(env,
+                        std::make_shared<ManagedX509>(std::move(issuer_cert)),
+                        issuer_chain,
+                        start + 1)
+             .ToLocal(&issuer)) [[unlikely]] {
+      return MaybeLocal<Object>();
+    }
+  }

What this does:
Instead of deleting from the OpenSSL stack, this clones each issuer certificate and recursively builds the JS issuer chain. This preserves the original chain for future reads.

2. Add a lightweight hasPeerCertificate binding

Previously, the server checked for the presence of a peer certificate by calling getPeerX509Certificate(), which was expensive and (before this PR) destructive. Now, a new binding is added to check for a certificate's presence without building JS objects or walking the chain.

+void TLSWrap::HasPeerCertificate(const FunctionCallbackInfo<Value>& args) {
+  TLSWrap* w;
+  ASSIGN_OR_RETURN_UNWRAP(&w, args.This());
+  bool has_peer_cert = static_cast<bool>(X509Pointer::PeerFrom(w->ssl_));
+  args.GetReturnValue().Set(has_peer_cert);
+}

What this does:
This function checks if a peer certificate exists and returns a boolean, avoiding the cost of constructing certificate objects.

The JS side is updated to use this new method:

-    } else if (!this._handle.getPeerX509Certificate()) {
+    } else if (!this._handle.hasPeerCertificate()) {

What this does:
Switches the server's peer certificate presence check to the new, efficient method.

3. Update and expand tests

A new test is added to verify that repeated reads of the peer certificate chain (using both getPeerCertificate and getPeerX509Certificate) always return the full chain, regardless of call order or repetition.

+function checkPeerCertificate(socket, chain, side) {
+  assert.strictEqual(socket.authorized, true, side);
+  const [leaf, intermediate, root] = fingerprints(chain);
+
+  // Two rounds, alternating the read methods, so a chain consumed by one read
+  // would be observed by the next.
+  for (let round = 0; round < 2; round++) {
+    const x509 = socket.getPeerX509Certificate();
+    assert.strictEqual(x509.fingerprint256, leaf, side);
+    assert.strictEqual(x509.issuerCertificate.fingerprint256,
+                       intermediate, side);
+
+    const detailed = socket.getPeerCertificate(true);
+    assert.strictEqual(detailed.fingerprint256, leaf, side);
+    assert.strictEqual(detailed.issuerCertificate.fingerprint256,
+                       intermediate, side);
+    assert.strictEqual(
+      detailed.issuerCertificate.issuerCertificate.fingerprint256, root, side);
+
+    assert.strictEqual(socket.getPeerCertificate().fingerprint256, leaf, side);
+  }
+}

What this does:
Ensures that repeated and interleaved calls to both certificate-reading methods always return the full, correct chain.

From the review thread

tgies: BTW, the argument for adding the hasPeerCertificate() binding even after making getPeerX509Certificate() nondestructive is mainly that it's expensive and unnecessary to call getPeerX509Certificate() here anyway. That builds the whole cert chain and the JS X509Certificate wrapper objects on every handshake just so we can test != null.

No notable review discussion on this PR.

Takeaways

• Never destructively consume shared data structures unless you own them. When working with pointers or references from external libraries (like OpenSSL), always ensure your reads are non-destructive unless explicitly allowed.
• Optimize for the common case: If you only need to check for the presence of data, provide a lightweight method instead of building full objects or structures.
• Regression tests should cover repeated and interleaved API calls: Bugs can surface when APIs are called multiple times or in unexpected orders; tests should verify consistent behavior in these scenarios.