tls: read the peer certificate chain without consuming it

lib/internal/tls/wrap.js

@@ -1322,7 +1322,7 @@ function onServerSocketSecure() {
 
       if (this._rejectUnauthorized)
         this.destroy();
-    } else if (!this._handle.getPeerX509Certificate()) {
+    } else if (!this._handle.hasPeerCertificate()) {
       // Ncrypto reports X509_V_OK for TLS 1.3 resumption without a peer
       // certificate, as it uses PSKs. Require one to authorize the socket.
       this.authorizationError = 'UNABLE_TO_GET_ISSUER_CERT';

src/crypto/crypto_tls.cc

@@ -1765,6 +1765,13 @@ void TLSWrap::GetPeerX509Certificate(const FunctionCallbackInfo<Value>& args) {
     args.GetReturnValue().Set(ret);
 }
 
+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);
+}
+
 void TLSWrap::GetCertificate(const FunctionCallbackInfo<Value>& args) {
   TLSWrap* w;
   ASSIGN_OR_RETURN_UNWRAP(&w, args.This());
@@ -2288,6 +2295,8 @@ void TLSWrap::Initialize(
       isolate, t, "getPeerCertificate", GetPeerCertificate);
   SetProtoMethodNoSideEffect(
       isolate, t, "getPeerX509Certificate", GetPeerX509Certificate);
+  SetProtoMethodNoSideEffect(
+      isolate, t, "hasPeerCertificate", HasPeerCertificate);
   SetProtoMethodNoSideEffect(isolate, t, "getPeerFinished", GetPeerFinished);
   SetProtoMethodNoSideEffect(isolate, t, "getProtocol", GetProtocol);
   SetProtoMethodNoSideEffect(isolate, t, "getSession", GetSession);
@@ -2347,6 +2356,7 @@ void TLSWrap::RegisterExternalReferences(ExternalReferenceRegistry* registry) {
   registry->Register(GetFinished);
   registry->Register(GetPeerCertificate);
   registry->Register(GetPeerX509Certificate);
+  registry->Register(HasPeerCertificate);
   registry->Register(GetPeerFinished);
   registry->Register(GetProtocol);
   registry->Register(GetSession);

src/crypto/crypto_tls.h

@@ -229,6 +229,8 @@ class TLSWrap : public AsyncWrap,
       const v8::FunctionCallbackInfo<v8::Value>& args);
   static void GetPeerX509Certificate(
       const v8::FunctionCallbackInfo<v8::Value>& args);
+  static void HasPeerCertificate(
+      const v8::FunctionCallbackInfo<v8::Value>& args);
   static void GetPeerFinished(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void GetProtocol(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void GetServername(const v8::FunctionCallbackInfo<v8::Value>& args);

src/crypto/crypto_x509.cc

@@ -920,14 +920,22 @@ void X509Certificate::IsX509Certificate(
 
 MaybeLocal<Object> X509Certificate::New(Environment* env,
                                         X509Pointer cert,
-                                        STACK_OF(X509) * issuer_chain) {
+                                        const STACK_OF(X509) * issuer_chain) {
   std::shared_ptr<ManagedX509> mcert(new ManagedX509(std::move(cert)));
   return New(env, std::move(mcert), issuer_chain);
 }
 
 MaybeLocal<Object> X509Certificate::New(Environment* env,
                                         std::shared_ptr<ManagedX509> cert,
-                                        STACK_OF(X509) * issuer_chain) {
+                                        const STACK_OF(X509) * issuer_chain) {
+  return NewWithIssuers(env, std::move(cert), issuer_chain, 0);
+}
+
+MaybeLocal<Object> X509Certificate::NewWithIssuers(
+    Environment* env,
+    std::shared_ptr<ManagedX509> cert,
+    const STACK_OF(X509) * issuer_chain,
+    int start) {
   EscapableHandleScope scope(env->isolate());
   Local<Object> obj;
   if (!GetConstructorTemplate(env)
@@ -937,20 +945,23 @@ MaybeLocal<Object> X509Certificate::New(Environment* env,
     return MaybeLocal<Object>();
   }
 
-  Local<Object> issuer_chain_obj;
-  if (issuer_chain != nullptr && sk_X509_num(issuer_chain)) {
-    X509Pointer cert(X509_dup(sk_X509_value(issuer_chain, 0)));
-    sk_X509_delete(issuer_chain, 0);
-    auto maybeObj =
-        sk_X509_num(issuer_chain)
-            ? X509Certificate::New(env, std::move(cert), issuer_chain)
-            : X509Certificate::New(env, std::move(cert));
-    if (!maybeObj.ToLocal(&issuer_chain_obj)) [[unlikely]] {
+  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>();
     }
   }
 
-  new X509Certificate(env, obj, std::move(cert), issuer_chain_obj);
+  new X509Certificate(env, obj, std::move(cert), issuer);
   return scope.Escape(obj);
 }
 
@@ -967,23 +978,29 @@ MaybeLocal<Object> X509Certificate::GetPeerCert(Environment* env,
                                                 GetPeerCertificateFlag flag) {
   ClearErrorOnReturn clear_error_on_return;
 
+  // The peer chain is owned by the SSL session and must not be modified. Its
+  // first entry is the peer certificate on the client but not on the server.
+  const STACK_OF(X509)* ssl_certs = SSL_get_peer_cert_chain(ssl.get());
+  int issuers_start = 0;
+
   X509Pointer cert;
   if ((flag & GetPeerCertificateFlag::SERVER) ==
       GetPeerCertificateFlag::SERVER) {
     cert = X509Pointer::PeerFrom(ssl);
   }
-
-  STACK_OF(X509)* ssl_certs = SSL_get_peer_cert_chain(ssl.get());
-  if (!cert && (ssl_certs == nullptr || sk_X509_num(ssl_certs) == 0))
-    return MaybeLocal<Object>();
-
-  if (!cert) [[unlikely]] {
-    cert.reset(sk_X509_value(ssl_certs, 0));
-    sk_X509_delete(ssl_certs, 0);
+  if (!cert) {
+    if (ssl_certs == nullptr || sk_X509_num(ssl_certs) == 0)
+      return MaybeLocal<Object>();
+    cert = X509View(sk_X509_value(ssl_certs, 0)).clone();
+    if (!cert) [[unlikely]]
+      return MaybeLocal<Object>();
+    issuers_start = 1;
   }
 
-  return sk_X509_num(ssl_certs) ? New(env, std::move(cert), ssl_certs)
-                                : New(env, std::move(cert));
+  return NewWithIssuers(env,
+                        std::make_shared<ManagedX509>(std::move(cert)),
+                        ssl_certs,
+                        issuers_start);
 }
 
 v8::MaybeLocal<v8::Value> X509Certificate::toObject(Environment* env) {

src/crypto/crypto_x509.h

@@ -60,12 +60,12 @@ class X509Certificate final : public BaseObject {
   static v8::MaybeLocal<v8::Object> New(
       Environment* env,
       ncrypto::X509Pointer cert,
-      STACK_OF(X509) * issuer_chain = nullptr);
+      const STACK_OF(X509) * issuer_chain = nullptr);
 
   static v8::MaybeLocal<v8::Object> New(
       Environment* env,
       std::shared_ptr<ManagedX509> cert,
-      STACK_OF(X509)* issuer_chain = nullptr);
+      const STACK_OF(X509) * issuer_chain = nullptr);
 
   static v8::MaybeLocal<v8::Object> GetCert(Environment* env,
                                             const ncrypto::SSLPointer& ssl);
@@ -121,6 +121,13 @@ class X509Certificate final : public BaseObject {
                   std::shared_ptr<ManagedX509> cert,
                   v8::Local<v8::Object> issuer_chain = v8::Local<v8::Object>());
 
+  // Like New(), but reads the issuer chain from issuer_chain[start] upward.
+  static v8::MaybeLocal<v8::Object> NewWithIssuers(
+      Environment* env,
+      std::shared_ptr<ManagedX509> cert,
+      const STACK_OF(X509) * issuer_chain,
+      int start);
+
   std::shared_ptr<ManagedX509> cert_;
   BaseObjectPtr<X509Certificate> issuer_cert_;
 };

test/parallel/test-tls-peer-certificate-repeated-reads.js

@@ -0,0 +1,87 @@
+'use strict';
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+// Reading the peer certificate must not consume the chain held by the SSL
+// session. getPeerX509Certificate() and getPeerCertificate() have to keep
+// returning the full chain however often, and in whichever order, they are
+// called on either end of the connection (see the #65579 regression, where an
+// internal getPeerX509Certificate() call left getPeerCertificate(true) with
+// only the leaf). On the server this also covers the peer certificate check
+// that runs before 'secureConnection' is emitted.
+
+const assert = require('assert');
+const { X509Certificate } = require('crypto');
+const tls = require('tls');
+const fixtures = require('../common/fixtures');
+
+// Each peer presents a distinct leaf -> intermediate -> root chain, so the
+// certificate read back has two issuers above the leaf.
+const serverChain = [
+  'leaf-from-intermediate-cert.pem',
+  'intermediate-ca.pem',
+  'fake-startcom-root-cert.pem',
+].map((name) => fixtures.readKey(name));
+const clientChain = [
+  'agent10-cert.pem',
+  'ca4-cert.pem',
+  'ca2-cert.pem',
+].map((name) => fixtures.readKey(name));
+
+function fingerprints(chain) {
+  return chain.map((pem) => new X509Certificate(pem).fingerprint256);
+}
+
+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);
+  }
+}
+
+const server = tls.createServer({
+  key: fixtures.readKey('leaf-from-intermediate-key.pem'),
+  cert: Buffer.concat(serverChain),
+  ca: clientChain[2],
+  requestCert: true,
+}, common.mustCall((socket) => {
+  checkPeerCertificate(socket, clientChain, 'server');
+  socket.end();
+}));
+
+server.listen(0, common.mustCall(() => {
+  const socket = tls.connect({
+    port: server.address().port,
+    key: fixtures.readKey('agent10-key.pem'),
+    cert: Buffer.concat(clientChain),
+    ca: serverChain[2],
+  }, common.mustCall(() => {
+    checkPeerCertificate(socket, serverChain, 'client');
+
+    // The client receives the server chain verbatim, so its X509 certificate
+    // links all the way to the root, exercising the recursive issuer build
+    // more than one level deep.
+    const [, , root] = fingerprints(serverChain);
+    const x509 = socket.getPeerX509Certificate();
+    assert.strictEqual(x509.issuerCertificate.issuerCertificate.fingerprint256,
+                       root);
+  }));
+  socket.on('close', common.mustCall(() => server.close()));
+}));