tls: apply the CA of SNI contexts without a certificate

`CertCbDone()` only installed an SNI context's CA store and client CA
list when `setSniContext()` succeeded, which requires the context to
have a certificate. A context added with `server.addContext(host,
{ ca })` was therefore ignored for client certificate verification,
and clients for that host were verified against the default context's
CA instead.

Apply the SNI context's CA whenever it configured one through
`addCACert()`, even without a certificate. A context with neither a
certificate nor its own CA still keeps the default trust store, rather
than switching to the bundled root certificates. Also report a failure
of `setSniContext()` instead of silently continuing with the default
credentials.

Refs: https://hackerone.com/reports/4103553
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: nodejs-private/node-private#1020
Reviewed-By: Tim Perry <pimterry@gmail.com>
CVE-ID: CVE-2026-123867

Author: mcollina

src/crypto/crypto_context.cc

@@ -1731,6 +1731,7 @@ inline void SecureContext::Reset() {
   ctx_.reset();
   cert_.reset();
   issuer_.reset();
+  has_own_ca_ = false;
 }
 
 SecureContext::~SecureContext() {
@@ -2066,6 +2067,7 @@ void SecureContext::SetAllowPartialTrustChain(
 void SecureContext::SetCACert(const BIOPointer& bio) {
   ClearErrorOnReturn clear_error_on_return;
   AddCACertificates(env(), ctx_.get(), bio, &own_cert_store_cache_);
+  has_own_ca_ = true;
 }
 
 void SecureContext::AddCACert(const FunctionCallbackInfo<Value>& args) {

src/crypto/crypto_context.h

@@ -52,6 +52,9 @@ class SecureContext final : public BaseObject {
   static SecureContext* Create(Environment* env);
 
   const ncrypto::SSLCtxPointer& ctx() const { return ctx_; }
+  // True if CA certificates were added with addCACert(), as opposed to the
+  // context relying on the default root store.
+  bool has_own_ca() const { return has_own_ca_; }
 
   // Non-const ctx() that allows for non-default initialization of
   // the SecureContext.
@@ -204,6 +207,7 @@ class SecureContext final : public BaseObject {
   bool client_cert_engine_provided_ = false;
   ncrypto::EnginePointer private_key_engine_;
 #endif  // !OPENSSL_NO_ENGINE
+  bool has_own_ca_ = false;
 
   unsigned char ticket_key_name_[16];
   unsigned char ticket_key_aes_[16];

src/crypto/crypto_tls.cc

@@ -1722,11 +1722,19 @@ void TLSWrap::CertCbDone(const FunctionCallbackInfo<Value>& args) {
     // Store the SNI context for later use.
     w->sni_context_ = BaseObjectPtr<SecureContext>(sc);
 
-    if (w->ssl_.setSniContext(w->sni_context_->ctx()) && !w->SetCACerts(sc)) {
-      // Not clear why sometimes we throw error, and sometimes we call
-      // onerror(). Both cause .destroy(), but onerror does a bit more.
-      unsigned long err = ERR_get_error();  // NOLINT(runtime/int)
-      return ThrowCryptoError(env, err, "CertCbDone");
+    // An SNI context without a certificate keeps the default context's
+    // certificate, but a `ca` it configured must still replace the default
+    // trust store for client certificates. A context with neither keeps the
+    // default context's trust store.
+    const bool has_cert = SSL_CTX_get0_certificate(sc->ctx().get()) != nullptr;
+    if (has_cert || sc->has_own_ca()) {
+      if ((has_cert && !w->ssl_.setSniContext(w->sni_context_->ctx())) ||
+          !w->SetCACerts(sc)) {
+        // Not clear why sometimes we throw error, and sometimes we call
+        // onerror(). Both cause .destroy(), but onerror does a bit more.
+        unsigned long err = ERR_get_error();  // NOLINT(runtime/int)
+        return ThrowCryptoError(env, err, "CertCbDone");
+      }
     }
     // setSniContext copies the cert via SSL_use_certificate which does not
     // carry over pre-compressed certificate data (comp_cert). If the new

test/parallel/test-tls-sni-context-ca-only.js

@@ -0,0 +1,88 @@
+'use strict';
+
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+// An SNI context that sets `ca` but no certificate must still replace the
+// default trust store for client certificates on that host. A context that
+// sets neither keeps the default trust store.
+
+const assert = require('assert');
+const tls = require('tls');
+const fixtures = require('../common/fixtures');
+
+const serverCreds = {
+  key: fixtures.readKey('agent2-key.pem'),
+  cert: fixtures.readKey('agent2-cert.pem'),
+};
+const ca1 = fixtures.readKey('ca1-cert.pem');
+const ca2 = fixtures.readKey('ca2-cert.pem');
+const clients = {
+  agent1: {  // Issued by CA1.
+    key: fixtures.readKey('agent1-key.pem'),
+    cert: fixtures.readKey('agent1-cert.pem'),
+  },
+  agent3: {  // Issued by CA2.
+    key: fixtures.readKey('agent3-key.pem'),
+    cert: fixtures.readKey('agent3-cert.pem'),
+  },
+};
+
+const cases = [
+  { name: 'ca only', context: { ca: ca2 }, accepted: 'agent3' },
+  { name: 'cert, key and ca', context: { ...serverCreds, ca: ca2 },
+    accepted: 'agent3' },
+  { name: 'no ca or cert', context: { ciphers: 'DEFAULT' },
+    accepted: 'agent1' },
+];
+
+function connect(port, client, version) {
+  return new Promise((resolve) => {
+    const socket = tls.connect({
+      port,
+      servername: 'protected.example',
+      ...clients[client],
+      rejectUnauthorized: false,
+      minVersion: version,
+      maxVersion: version,
+    });
+    let data = '';
+    socket.setEncoding('utf8');
+    socket.on('data', (chunk) => data += chunk);
+    socket.on('error', () => {});
+    socket.on('close', () => resolve(data));
+  });
+}
+
+async function test(version, { name, context, accepted }) {
+  const server = tls.createServer({
+    ...serverCreds,
+    ca: ca1,
+    requestCert: true,
+    rejectUnauthorized: true,
+    minVersion: version,
+    maxVersion: version,
+  }, common.mustCall((socket) => {
+    assert.strictEqual(socket.servername, 'protected.example');
+    socket.end(socket.getPeerCertificate().subject.CN);
+  }));
+  server.on('tlsClientError', () => {});
+  server.addContext('protected.example', context);
+  await new Promise((resolve) => server.listen(0, resolve));
+
+  const { port } = server.address();
+  for (const client of Object.keys(clients)) {
+    const expected = client === accepted ? client : '';
+    assert.strictEqual(await connect(port, client, version), expected,
+                       `${version}, ${name}: ${client}`);
+  }
+  server.close();
+}
+
+(async () => {
+  for (const version of ['TLSv1.2', 'TLSv1.3']) {
+    for (const c of cases)
+      await test(version, c);
+  }
+})().then(common.mustCall());