nodejs/node · #65602
tls: read the peer certificate chain without consuming it
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()));+}));