tls: reject trust-store certificates that carry an unhandled critical extension

packages/bun-usockets/src/crypto/openssl.c

@@ -1133,6 +1133,66 @@ void us_internal_ssl_set_inline_reject(SSL *ssl) {
   SSL_set_verify(ssl, SSL_VERIFY_PEER, us_inline_reject_verify_callback);
 }
 
+static int us_x509_stack_contains(const STACK_OF(X509) *sk, const X509 *x) {
+  for (size_t i = 0; i < sk_X509_num(sk); i++) {
+    if (sk_X509_value(sk, i) == x) return 1;
+  }
+  return 0;
+}
+
+/* BoringSSL's check_chain_extensions() only covers the untrusted part of the
+ * chain: a certificate taken from the trust store (the anchor, an intermediate
+ * configured via `ca`, a pinned self-signed certificate) is accepted even when
+ * it carries a critical extension BoringSSL does not implement. OpenSSL checks
+ * the whole chain, so node rejects that with
+ * X509_V_ERR_UNHANDLED_CRITICAL_EXTENSION; apply the same check to the
+ * store-sourced certificates of a chain that verified otherwise. Those are
+ * told apart by identity: the chain holds the store's own X509 objects, never
+ * the peer's copies of them. */
+static int us_trusted_chain_certs_error(X509_STORE_CTX *ctx) {
+  const STACK_OF(X509) *chain = X509_STORE_CTX_get0_chain(ctx);
+  const STACK_OF(X509) *peer_certs = X509_STORE_CTX_get0_untrusted(ctx);
+  const X509 *leaf = X509_STORE_CTX_get0_cert(ctx);
+  for (size_t i = 0; i < sk_X509_num(chain); i++) {
+    X509 *x = sk_X509_value(chain, i);
+    if (x == leaf || us_x509_stack_contains(peer_certs, x)) continue;
+    if (X509_get_extension_flags(x) & EXFLAG_CRITICAL) {
+      return X509_V_ERR_UNHANDLED_CRITICAL_EXTENSION;
+    }
+  }
+  return X509_V_OK;
+}
+
+/* X509_verify_cert plus the check above. Every chain bun verifies goes through
+ * here: installed on each SSL_CTX by us_internal_ssl_ctx_set_chain_verifier,
+ * called directly by the QUIC client's custom_verify (quic.c). Same contract
+ * as X509_verify_cert: the verdict is left in the ctx error, the return value
+ * says whether verification may proceed. */
+int us_internal_verify_cert_chain(X509_STORE_CTX *ctx) {
+  int ok = X509_verify_cert(ctx);
+  if (!ok || X509_STORE_CTX_get_error(ctx) != X509_V_OK) return ok;
+  int err = us_trusted_chain_certs_error(ctx);
+  if (err == X509_V_OK) return ok;
+  X509_STORE_CTX_set_error(ctx, err);
+  /* Hand the error to the same verify callback BoringSSL gives its own errors
+   * to (us_verify_callback keeps the handshake going and leaves the verdict to
+   * JS, us_inline_reject_verify_callback records it); with no callback it is
+   * fatal, exactly like a BoringSSL-detected error. */
+  SSL *ssl = X509_STORE_CTX_get_ex_data(ctx, SSL_get_ex_data_X509_STORE_CTX_idx());
+  int (*verify_cb)(int, X509_STORE_CTX *) = ssl ? SSL_get_verify_callback(ssl) : NULL;
+  return verify_cb ? verify_cb(0, ctx) : 0;
+}
+
+static int us_cert_verify_callback(X509_STORE_CTX *ctx, void *arg) {
+  return us_internal_verify_cert_chain(ctx);
+}
+
+/* Also called from the Rust node:quic TLS context builder, which creates its
+ * SSL_CTX outside us_ssl_ctx_build_raw. */
+void us_internal_ssl_ctx_set_chain_verifier(SSL_CTX *ctx) {
+  SSL_CTX_set_cert_verify_callback(ctx, us_cert_verify_callback, NULL);
+}
+
 /* Drop the strdup'd passphrase. Called as soon as private-key load completes
  * (the only consumer of the passwd_cb), so the secret never outlives ctx
  * construction and SSL_CTX_free() is sufficient on every later path. Also
@@ -1167,6 +1227,7 @@ SSL_CTX *us_ssl_ctx_build_raw(struct us_bun_socket_context_options_t options,
   /* Default options we rely on — changing these breaks the BIO logic. */
   SSL_CTX_set_read_ahead(ssl_context, 1);
   SSL_CTX_set_mode(ssl_context, SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER);
+  us_internal_ssl_ctx_set_chain_verifier(ssl_context);
   /* BoringSSL ships with SSL_MODE_NO_AUTO_CHAIN set; Node clears it so a
    * leaf-only `cert` presents the intermediates found in the context's store
    * (crypto_context.cc#L1640). It only runs when the configured chain is 1. */

packages/bun-usockets/src/quic.c

@@ -29,6 +29,7 @@ extern SSL_CTX *us_ssl_ctx_build_raw(
     enum create_bun_socket_error_t *err);
 extern X509_STORE *us_get_default_ca_store(void);
 extern struct us_bun_verify_error_t us_ssl_socket_verify_error_from_ssl(SSL *ssl);
+extern int us_internal_verify_cert_chain(X509_STORE_CTX *ctx);
 
 #define US_QUIC_READ_BUF (16 * 1024)
 
@@ -1157,9 +1158,10 @@ static enum ssl_verify_result_t us_quic_client_verify(SSL *ssl, uint8_t *out_ale
     if (!qs) return ssl_verify_invalid;
     if (!qs->reject_unauthorized) return ssl_verify_ok;
 
-    /* custom_verify bypasses BoringSSL's built-in chain check, so run
-     * X509_verify_cert against the SSL_CTX store ourselves, then match the
-     * leaf against the SNI hostname. */
+    /* custom_verify bypasses BoringSSL's built-in chain check (and with it the
+     * cert_verify_callback the TCP contexts use), so run the shared chain
+     * verifier against the SSL_CTX store ourselves, then match the leaf against
+     * the SNI hostname. */
     STACK_OF(X509) *chain = SSL_get_peer_full_cert_chain(ssl);
     if (!chain || sk_X509_num(chain) == 0) return ssl_verify_invalid;
     X509 *leaf = sk_X509_value(chain, 0);
@@ -1169,7 +1171,7 @@ static enum ssl_verify_result_t us_quic_client_verify(SSL *ssl, uint8_t *out_ale
     int ok = 0;
     if (X509_STORE_CTX_init(vctx, store, leaf, chain) == 1) {
         X509_STORE_CTX_set_default(vctx, "ssl_server");
-        ok = X509_verify_cert(vctx) == 1;
+        ok = us_internal_verify_cert_chain(vctx) == 1;
     }
     X509_STORE_CTX_free(vctx);
     if (!ok) return ssl_verify_invalid;

src/runtime/node/quic/tls.rs

@@ -406,6 +406,11 @@ fn load_ca_store(ctx: *mut ssl::SSL_CTX, pem: &[u8]) -> Result<(), &'static str>
     Ok(())
 }
 
+unsafe extern "C" {
+    /// The chain verifier every usockets TLS context installs; see `crypto/openssl.c`.
+    fn us_internal_ssl_ctx_set_chain_verifier(ctx: *mut ssl::SSL_CTX);
+}
+
 impl TlsContext {
     pub(super) fn new(config: &TlsConfig) -> Result<Self, &'static str> {
         // SAFETY: each SSL_CTX call below is paired with the matching free on
@@ -420,6 +425,7 @@ impl TlsContext {
                 ctx: owned,
                 _alpn: None,
             };
+            us_internal_ssl_ctx_set_chain_verifier(ctx);
 
             if let Some(policy) = config.ciphers.as_deref().and_then(tls13_policy_for_ciphers) {
                 if ssl::SSL_CTX_set_compliance_policy(ctx, policy) != 1 {

test/js/node/quic/quic-sni.test.ts

@@ -1,4 +1,4 @@
-import { expect, test } from "bun:test";
+import { describe, expect, test } from "bun:test";
 import { bunEnv, bunExe } from "harness";
 import { createPrivateKey, X509Certificate } from "node:crypto";
 import { readFileSync } from "node:fs";
@@ -221,3 +221,41 @@ test("connect() with the default verifyPeer refuses an unverifiable certificate
     exitCode: 0,
   });
 });
+
+// node:quic builds its TLS contexts apart from node:tls, so it needs its own
+// check that a trust anchor carrying a critical extension BoringSSL does not
+// implement is rejected the way node (OpenSSL) rejects it. Fixtures are shared
+// with test/js/node/tls/node-tls-cert.test.ts: two anchors for one CA key that
+// differ only in that extension, and a leaf that chains to either.
+describe("trust anchor with an unimplemented critical extension", () => {
+  const fixturesDir = join(import.meta.dir, "..", "tls", "fixtures");
+  const fixture = (name: string) => readFileSync(join(fixturesDir, `critical-ext-anchor-${name}.pem`));
+  const leafIdentity = { keys: [createPrivateKey(fixture("leaf-key"))], certs: [fixture("leaf-cert")] };
+
+  async function validationOutcome(ca: Buffer) {
+    await using server = await listen(ignoreErrors, { sni: { "*": leafIdentity }, alpn: ["quic-test"] });
+    const session = await connect(server.address, {
+      alpn: "quic-test",
+      servername: "localhost",
+      ca,
+      verifyPeer: "manual",
+    });
+    try {
+      const info = await session.opened;
+      return { code: info.validationErrorCode, reason: info.validationErrorReason };
+    } finally {
+      await session.close();
+    }
+  }
+
+  test("reports node's validation error for the anchor with the extension", async () => {
+    expect(await validationOutcome(fixture("ca-cert"))).toEqual({
+      code: "UNSPECIFIED",
+      reason: "unhandled critical extension",
+    });
+  });
+
+  test("validates the same leaf under the anchor without the extension", async () => {
+    expect(await validationOutcome(fixture("ca-plain-cert"))).toEqual({ code: undefined, reason: undefined });
+  });
+});

test/js/node/tls/fetch-tls-cert.test.ts

@@ -395,3 +395,28 @@ it("fetch applies tls.sigalgs even when it is the only TLS option", async () =>
     tlsServer.close();
   }
 });
+
+// Same fixtures as node-tls-cert.test.ts: one CA key behind two anchors that
+// differ only in an unimplemented critical extension, and a leaf for
+// DNS:localhost that chains to either. The HTTP client builds its TLS contexts
+// separately from node:tls, so check that it enforces the anchor check too.
+it("fetch rejects a chain whose trust anchor carries an unimplemented critical extension", async () => {
+  const fixture = (name: string) =>
+    readFileSync(join(import.meta.dir, "fixtures", `critical-ext-anchor-${name}.pem`), "utf8");
+  using bunServer = Bun.serve({
+    port: 0,
+    tls: { cert: fixture("leaf-cert"), key: fixture("leaf-key") },
+    fetch: () => new Response("ok"),
+  });
+  const outcome = (ca: string) =>
+    fetch(`https://localhost:${bunServer.port}/`, { tls: { ca } }).then(
+      response => response.status,
+      (error: Error & { code?: string }) => ({ code: error.code, message: error.message }),
+    );
+
+  expect(await outcome(fixture("ca-cert"))).toEqual({
+    code: "UNHANDLED_CRITICAL_EXTENSION",
+    message: "unhandled critical extension",
+  });
+  expect(await outcome(fixture("ca-plain-cert"))).toBe(200);
+});

test/js/node/tls/fixtures/critical-ext-anchor-ca-cert.pem

@@ -0,0 +1,12 @@
+-----BEGIN CERTIFICATE-----
+MIIBujCCAWCgAwIBAgIUGyB77F/DR2Bv16qFcRU2WCVws0IwCgYIKoZIzj0EAwIw
+JjEkMCIGA1UEAwwbY3JpdGljYWwtZXh0LWFuY2hvciB0ZXN0IENBMCAXDTI2MDgx
+NDE0MTEyNloYDzIxMjYwNzIxMTQxMTI2WjAmMSQwIgYDVQQDDBtjcml0aWNhbC1l
+eHQtYW5jaG9yIHRlc3QgQ0EwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAARKhPjK
+H+73IH4q+8wExzDmDvjQ3AtL6TfrwNynbNeih9Of8vzMmkefoCM/guLd2ArtYpEQ
+0iijwfHBGS7qHhvRo2owaDAPBgNVHRMBAf8EBTADAQH/MA4GA1UdDwEB/wQEAwIC
+hDAUBgNVHREEDTALgglsb2NhbGhvc3QwEAYHKgMEBQYHYwEB/wQCBQAwHQYDVR0O
+BBYEFH6ModKD6hbHTRl/J07KbcZVqOgmMAoGCCqGSM49BAMCA0gAMEUCIHSywASb
+77AAEwOLYkP+1ym0CxuEyYyIdUhC0O7IajqzAiEAnsNsOsaVfBjl5vGEDFVyLjRW
+y9jFeJVju3wyVkcplrs=
+-----END CERTIFICATE-----

test/js/node/tls/fixtures/critical-ext-anchor-ca-key.pem

@@ -0,0 +1,5 @@
+-----BEGIN EC PRIVATE KEY-----
+MHcCAQEEIPbi6t21tb9WdhVbgIBP6BJ2IAax253FZuHGlF+5qjTWoAoGCCqGSM49
+AwEHoUQDQgAESoT4yh/u9yB+KvvMBMcw5g740NwLS+k368Dcp2zXoofTn/L8zJpH
+n6AjP4Li3dgK7WKRENIoo8HxwRku6h4b0Q==
+-----END EC PRIVATE KEY-----

test/js/node/tls/fixtures/critical-ext-anchor-ca-plain-cert.pem

@@ -0,0 +1,11 @@
+-----BEGIN CERTIFICATE-----
+MIIBqDCCAU6gAwIBAgIUF06h0eqvGKgFpJSmoHLS2DFYU6YwCgYIKoZIzj0EAwIw
+JjEkMCIGA1UEAwwbY3JpdGljYWwtZXh0LWFuY2hvciB0ZXN0IENBMCAXDTI2MDgx
+NDE0MTEyN1oYDzIxMjYwNzIxMTQxMTI3WjAmMSQwIgYDVQQDDBtjcml0aWNhbC1l
+eHQtYW5jaG9yIHRlc3QgQ0EwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAARKhPjK
+H+73IH4q+8wExzDmDvjQ3AtL6TfrwNynbNeih9Of8vzMmkefoCM/guLd2ArtYpEQ
+0iijwfHBGS7qHhvRo1gwVjAPBgNVHRMBAf8EBTADAQH/MA4GA1UdDwEB/wQEAwIC
+hDAUBgNVHREEDTALgglsb2NhbGhvc3QwHQYDVR0OBBYEFH6ModKD6hbHTRl/J07K
+bcZVqOgmMAoGCCqGSM49BAMCA0gAMEUCIEbodZtPCLG7FT6iUnqqvo8j/wrMi0DB
+ae3+YUgWbfk7AiEAks6wk8ALaj3UiPKKp8g7Xoyb3poXTMYFF1NDQJzV9Ck=
+-----END CERTIFICATE-----

test/js/node/tls/fixtures/critical-ext-anchor-leaf-cert.pem

@@ -0,0 +1,11 @@
+-----BEGIN CERTIFICATE-----
+MIIBnTCCAUSgAwIBAgIBATAKBggqhkjOPQQDAjAmMSQwIgYDVQQDDBtjcml0aWNh
+bC1leHQtYW5jaG9yIHRlc3QgQ0EwIBcNMjYwODE0MTQxMTI3WhgPMjEyNjA3MjEx
+NDExMjdaMBQxEjAQBgNVBAMMCWxvY2FsaG9zdDBZMBMGByqGSM49AgEGCCqGSM49
+AwEHA0IABHYBLKnLv8wKSwCxFU0lQ6qiK+qLA0eDYIgin75LvT4Akzehc4LxQt1+
+tQdMVcEZX5KXc4I0WQ+Veuhp/xOwNqOjczBxMAkGA1UdEwQCMAAwDgYDVR0PAQH/
+BAQDAgeAMBQGA1UdEQQNMAuCCWxvY2FsaG9zdDAdBgNVHQ4EFgQUdvmUfJyJm2Ib
+al2RbAy881Mhyh8wHwYDVR0jBBgwFoAUfoyh0oPqFsdNGX8nTsptxlWo6CYwCgYI
+KoZIzj0EAwIDRwAwRAIgcYcBb1zGfeRjFGbTiTfGhIpGPGUUctWZBJWbmOlalrEC
+IByOtE+knj4bDzUL6MkUCSocL+0x7SIDfSwPZ5nhA4DH
+-----END CERTIFICATE-----

test/js/node/tls/fixtures/critical-ext-anchor-leaf-key.pem

@@ -0,0 +1,5 @@
+-----BEGIN EC PRIVATE KEY-----
+MHcCAQEEIO+ZH7QUAMS8WLBBF83SMJJ8A4TdP0gbYoX2jgUgyZewoAoGCCqGSM49
+AwEHoUQDQgAEdgEsqcu/zApLALEVTSVDqqIr6osDR4NgiCKfvku9PgCTN6FzgvFC
+3X61B0xVwRlfkpdzgjRZD5V66Gn/E7A2ow==
+-----END EC PRIVATE KEY-----

test/js/node/tls/node-tls-cert.test.ts

@@ -3,7 +3,7 @@ import { once } from "events";
 import { readFileSync } from "fs";
 import { bunEnv, bunExe, invalidTls, tmpdirSync } from "harness";
 import type { AddressInfo } from "node:net";
-import type { Server, TLSSocket } from "node:tls";
+import type { Server, TlsOptions, TLSSocket } from "node:tls";
 import { join } from "path";
 import tls from "tls";
 const clientTls = {
@@ -728,3 +728,127 @@ describe("tls ciphers should work", () => {
     );
   });
 });
+
+// BoringSSL only runs its per-certificate extension checks on the untrusted
+// part of a chain, so a trust-store certificate (the anchor, or a self-signed
+// certificate pinned via `ca`) carrying a critical extension it does not
+// implement used to verify cleanly. OpenSSL checks the whole chain, so node
+// reports X509 error 34 ("unhandled critical extension", code UNSPECIFIED).
+//
+// Fixtures: one CA key behind two self-signed CA certificates with the same
+// subject, one carrying an extension nobody implements
+// (critical-ext-anchor-ca-cert) and one without it
+// (critical-ext-anchor-ca-plain-cert), plus a leaf for DNS:localhost issued
+// with that key. The same leaf therefore chains to either anchor and the
+// extension is the only difference between the cases. Regenerate with:
+//
+//   # ext.cnf: [req] distinguished_name=dn / prompt=no; [dn] CN=critical-ext-anchor test CA
+//   #   [ca] and [ca_plain]: basicConstraints=critical,CA:TRUE
+//   #                        keyUsage=critical,digitalSignature,keyCertSign
+//   #                        subjectAltName=DNS:localhost
+//   #   [ca] additionally:   1.2.3.4.5.6.7.99=critical,DER:0500
+//   #   [leaf]: basicConstraints=CA:FALSE keyUsage=critical,digitalSignature subjectAltName=DNS:localhost
+//   openssl ecparam -name prime256v1 -genkey -noout -out critical-ext-anchor-ca-key.pem
+//   openssl req -new -x509 -key critical-ext-anchor-ca-key.pem -config ext.cnf -extensions ca -days 36500 -out critical-ext-anchor-ca-cert.pem
+//   openssl req -new -x509 -key critical-ext-anchor-ca-key.pem -config ext.cnf -extensions ca_plain -days 36500 -out critical-ext-anchor-ca-plain-cert.pem
+//   openssl ecparam -name prime256v1 -genkey -noout -out critical-ext-anchor-leaf-key.pem
+//   openssl req -new -key critical-ext-anchor-leaf-key.pem -subj /CN=localhost -out leaf.csr
+//   openssl x509 -req -in leaf.csr -CA critical-ext-anchor-ca-plain-cert.pem -CAkey critical-ext-anchor-ca-key.pem \
+//     -set_serial 1 -days 36500 -extfile ext.cnf -extensions leaf -out critical-ext-anchor-leaf-cert.pem
+describe("trust anchor with an unimplemented critical extension", () => {
+  const fixture = (name: string) =>
+    readFileSync(join(import.meta.dir, "fixtures", `critical-ext-anchor-${name}.pem`), "utf8");
+  const anchorWithExtension = fixture("ca-cert");
+  const anchorKey = fixture("ca-key");
+  const anchorWithoutExtension = fixture("ca-plain-cert");
+  const leaf = { cert: fixture("leaf-cert"), key: fixture("leaf-key") };
+
+  type Verdict = { authorized: boolean; authorizationError: unknown };
+
+  async function withServer<T>(options: TlsOptions, run: (port: number, server: Server) => Promise<T>) {
+    const server = tls.createServer(options, socket => socket.end());
+    server.on("tlsClientError", () => {});
+    await once(server.listen(0, "127.0.0.1"), "listening");
+    try {
+      return await run((server.address() as AddressInfo).port, server);
+    } finally {
+      server.close();
+    }
+  }
+
+  function clientVerdict(port: number, ca: string): Promise<Verdict> {
+    const { promise, resolve, reject } = Promise.withResolvers<Verdict>();
+    const socket = tls.connect(
+      { port, host: "127.0.0.1", servername: "localhost", ca, rejectUnauthorized: false },
+      () => {
+        resolve({ authorized: socket.authorized, authorizationError: socket.authorizationError });
+        socket.end();
+      },
+    );
+    socket.on("error", reject);
+    return promise;
+  }
+
+  const rejected: Verdict = { authorized: false, authorizationError: "UNSPECIFIED" };
+  const accepted: Verdict = { authorized: true, authorizationError: null };
+
+  it("rejects a leaf issued by such an anchor, but accepts it under the same anchor without the extension", async () => {
+    await withServer(leaf, async port => {
+      expect(await clientVerdict(port, anchorWithExtension)).toEqual(rejected);
+      expect(await clientVerdict(port, anchorWithoutExtension)).toEqual(accepted);
+    });
+  });
+
+  it("rejects a pinned self-signed certificate carrying the extension", async () => {
+    await withServer({ cert: anchorWithExtension, key: anchorKey }, async port => {
+      expect(await clientVerdict(port, anchorWithExtension)).toEqual(rejected);
+    });
+    await withServer({ cert: anchorWithoutExtension, key: anchorKey }, async port => {
+      expect(await clientVerdict(port, anchorWithoutExtension)).toEqual(accepted);
+    });
+  });
+
+  it("fails the connection with node's error when rejectUnauthorized is on", async () => {
+    await withServer(leaf, async port => {
+      const { promise, resolve, reject } = Promise.withResolvers<NodeJS.ErrnoException>();
+      const socket = tls.connect({ port, host: "127.0.0.1", servername: "localhost", ca: anchorWithExtension }, () => {
+        socket.end();
+        reject(new Error("handshake succeeded"));
+      });
+      socket.on("error", resolve);
+      const error = await promise;
+      expect({ code: error.code, message: error.message }).toEqual({
+        code: "UNSPECIFIED",
+        message: "unhandled critical extension",
+      });
+    });
+  });
+
+  it("applies to the CA a server verifies client certificates against", async () => {
+    // The same leaf serves as the client certificate; only the server's `ca` differs.
+    async function serverVerdict(ca: string): Promise<Verdict> {
+      return withServer({ ...leaf, ca, requestCert: true, rejectUnauthorized: false }, async (port, server) => {
+        const verdict = once(server, "secureConnection").then(([socket]: [TLSSocket]) => ({
+          authorized: socket.authorized,
+          authorizationError: socket.authorizationError,
+        }));
+        const client = tls.connect({
+          port,
+          host: "127.0.0.1",
+          servername: "localhost",
+          ...leaf,
+          ca: anchorWithoutExtension,
+        });
+        const failed = once(client, "error").then(([error]) => Promise.reject(error));
+        try {
+          return await Promise.race([verdict, failed]);
+        } finally {
+          client.destroy();
+        }
+      });
+    }
+
+    expect(await serverVerdict(anchorWithExtension)).toEqual(rejected);
+    expect(await serverVerdict(anchorWithoutExtension)).toEqual(accepted);
+  });
+});