nodejs/node · #65602

tls: read the peer certificate chain without consuming it

tgies · merged Aug 31, 20266 files · 148 + / 25
lib/internal/tls/wrap.js1 + / 1
@@ -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.cc10 + / 0
@@ -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.h2 + / 0
@@ -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.cc39 + / 22
@@ -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.h9 + / 2
@@ -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.jsadded87 + / 0
@@ -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()));+}));