http: fix use-after-free in onHandshake when checkServerIdentity rejects (u9sf0l)

src/boringssl.zig

@@ -116,10 +116,48 @@ pub fn ip2String(ip: *boring.ASN1_OCTET_STRING, outIP: *[INET6_ADDRSTRLEN + 1]u8
     return outIP[0..size];
 }
 
+/// Matches a DNS name pattern (possibly with a leading `*.` wildcard) against
+/// `hostname`. Mirrors Node.js `check()` in lib/tls.js for a single pattern.
+fn matchDnsName(pattern: []const u8, hostname: []const u8) bool {
+    if (pattern.len == 0) return false;
+    if (!X509.isSafeAltName(pattern, false)) return false;
+
+    if (pattern[0] == '*') {
+        // RFC 6125 Section 6.4.3: Wildcard must match exactly one label.
+        // Enforce "*." prefix (wildcard must be leftmost and followed by a dot).
+        if (pattern.len >= 2 and pattern[1] == '.') {
+            const suffix = pattern[2..];
+            // Disallow "*.tld" (suffix must contain at least one dot for proper domain hierarchy)
+            if (strings.containsChar(suffix, '.')) {
+                // Host must be at least "label.suffix" (suffix_len + 1 for dot + at least 1 char for label)
+                if (hostname.len > suffix.len + 1) {
+                    const dot_index = hostname.len - suffix.len - 1;
+                    // The character before suffix must be a dot, and there must be no other
+                    // dots in the prefix (single-label wildcard only).
+                    if (hostname[dot_index] == '.' and !strings.containsChar(hostname[0..dot_index], '.')) {
+                        const host_suffix = hostname[dot_index + 1 ..];
+                        // RFC 4343: DNS names are case-insensitive
+                        if (strings.eqlCaseInsensitiveASCII(suffix, host_suffix, true)) {
+                            return true;
+                        }
+                    }
+                }
+            }
+        }
+    }
+    // RFC 4343: DNS names are case-insensitive
+    return strings.eqlCaseInsensitiveASCII(pattern, hostname, true);
+}
+
 pub fn checkX509ServerIdentity(
     x509: *boring.X509,
     hostname: []const u8,
 ) bool {
+    const host_is_ip = strings.isIPAddress(hostname);
+    // Node.js: CN is consulted only when the certificate carries no
+    // DNS / IP / URI subjectAltName entries. Track whether any were seen.
+    var has_identifier_san = false;
+
     // we check with native code if the cert is valid
     const index = boring.X509_get_ext_by_NID(x509, boring.NID_subject_alt_name, -1);
     if (index >= 0) {
@@ -130,7 +168,7 @@ pub fn checkX509ServerIdentity(
                 return false;
             }
 
-            if (strings.isIPAddress(hostname)) {
+            if (host_is_ip) {
                 // we safely ensure buffer size with max len + 1
                 var canonicalIPBuf: [INET6_ADDRSTRLEN + 1]u8 = undefined;
                 var certIPBuf: [INET6_ADDRSTRLEN + 1]u8 = undefined;
@@ -143,12 +181,17 @@ pub fn checkX509ServerIdentity(
                     for (0..boring.sk_GENERAL_NAME_num(names)) |i| {
                         const gen = boring.sk_GENERAL_NAME_value(names, i);
                         if (gen) |name| {
-                            if (name.name_type == .GEN_IPADD) {
-                                if (ip2String(name.d.ip, &certIPBuf)) |cert_ip| {
-                                    if (strings.eql(host_ip, cert_ip)) {
-                                        return true;
+                            switch (name.name_type) {
+                                .GEN_DNS, .GEN_URI => has_identifier_san = true,
+                                .GEN_IPADD => {
+                                    has_identifier_san = true;
+                                    if (ip2String(name.d.ip, &certIPBuf)) |cert_ip| {
+                                        if (strings.eql(host_ip, cert_ip)) {
+                                            return true;
+                                        }
                                     }
-                                }
+                                },
+                                else => {},
                             }
                         }
                     }
@@ -160,48 +203,48 @@ pub fn checkX509ServerIdentity(
                     for (0..boring.sk_GENERAL_NAME_num(names)) |i| {
                         const gen = boring.sk_GENERAL_NAME_value(names, i);
                         if (gen) |name| {
-                            if (name.name_type == .GEN_DNS) {
-                                const dnsName = name.d.dNSName;
-                                var dnsNameSlice = dnsName.data[0..@as(usize, @intCast(dnsName.length))];
-                                // ignore empty dns names (should never happen)
-                                if (dnsNameSlice.len > 0) {
-                                    if (X509.isSafeAltName(dnsNameSlice, false)) {
-                                        if (dnsNameSlice[0] == '*') {
-                                            // RFC 6125 Section 6.4.3: Wildcard must match exactly one label
-                                            // Enforce "*." prefix (wildcards must be leftmost and followed by a dot)
-                                            if (dnsNameSlice.len >= 2 and dnsNameSlice[1] == '.') {
-                                                const suffix = dnsNameSlice[2..];
-                                                // Disallow "*.tld" (suffix must contain at least one dot for proper domain hierarchy)
-                                                if (std.mem.indexOfScalar(u8, suffix, '.') != null) {
-                                                    // Host must be at least "label.suffix" (suffix_len + 1 for dot + at least 1 char for label)
-                                                    if (hostname.len > suffix.len + 1) {
-                                                        const dot_index = hostname.len - suffix.len - 1;
-                                                        // The character before suffix must be a dot, and there must be no other dots
-                                                        // in the prefix (single-label wildcard only)
-                                                        if (hostname[dot_index] == '.' and std.mem.indexOfScalar(u8, hostname[0..dot_index], '.') == null) {
-                                                            const host_suffix = hostname[dot_index + 1 ..];
-                                                            // RFC 4343: DNS names are case-insensitive
-                                                            if (strings.eqlCaseInsensitiveASCII(suffix, host_suffix, true)) {
-                                                                return true;
-                                                            }
-                                                        }
-                                                    }
-                                                }
-                                            }
-                                        }
-                                        // RFC 4343: DNS names are case-insensitive
-                                        if (strings.eqlCaseInsensitiveASCII(dnsNameSlice, hostname, true)) {
-                                            return true;
-                                        }
+                            switch (name.name_type) {
+                                .GEN_IPADD, .GEN_URI => has_identifier_san = true,
+                                .GEN_DNS => {
+                                    has_identifier_san = true;
+                                    const dnsName = name.d.dNSName;
+                                    const dnsNameSlice = dnsName.data[0..@as(usize, @intCast(dnsName.length))];
+                                    if (matchDnsName(dnsNameSlice, hostname)) {
+                                        return true;
                                     }
-                                }
+                                },
+                                else => {},
                             }
                         }
                     }
                 }
             }
         }
     }
+
+    // Node.js tls.checkServerIdentity: when the certificate has no
+    // DNS/IP/URI subjectAltName entries, fall back to the Subject
+    // Common Name. Never for IP-literal hosts (RFC 2818 §3.1).
+    if (!host_is_ip and !has_identifier_san) {
+        if (boring.X509_get_subject_name(x509)) |subject| {
+            var last: c_int = -1;
+            while (true) {
+                const entry_idx = boring.X509_NAME_get_index_by_NID(subject, boring.NID_commonName, last);
+                if (entry_idx < 0) break;
+                last = entry_idx;
+                const entry = boring.X509_NAME_get_entry(subject, entry_idx) orelse continue;
+                const data = boring.X509_NAME_ENTRY_get_data(entry) orelse continue;
+                const cn_ptr = boring.ASN1_STRING_get0_data(data);
+                const cn_len = boring.ASN1_STRING_length(data);
+                if (cn_ptr == null or cn_len <= 0) continue;
+                const cn = cn_ptr[0..@intCast(cn_len)];
+                if (matchDnsName(cn, hostname)) {
+                    return true;
+                }
+            }
+        }
+    }
+
     return false;
 }
 

src/http/HTTPContext.zig

@@ -417,9 +417,12 @@ pub fn NewHTTPContext(comptime ssl: bool) type {
                             // if checkServerIdentity returns false, we dont call firstCall — the connection was rejected
                             const ssl_ptr = @as(*BoringSSL.SSL, @ptrCast(socket.getNativeHandle()));
                             if (!client.checkServerIdentity(comptime ssl, socket, handshake_error, ssl_ptr, true)) {
-                                client.flags.did_have_handshaking_error = true;
-                                client.unregisterAbortTracker();
-                                if (!socket.isClosed()) terminateSocket(socket);
+                                // checkServerIdentity already called closeAndFail() → fail()
+                                // → result callback, which may have destroyed the
+                                // AsyncHTTP that embeds `client`. Socket is terminated
+                                // and the abort tracker is unregistered there, so the
+                                // only safe action is to return without touching
+                                // `client` again.
                                 return;
                             }
                         }

src/http/ProxyTunnel.zig

@@ -164,17 +164,17 @@ fn onHandshake(this: *HTTPClient, handshake_success: bool, ssl_error: uws.us_bun
                     .ssl => |socket| {
                         if (!this.checkServerIdentity(true, socket, handshake_error, ssl_ptr, false)) {
                             log("ProxyTunnel onHandshake checkServerIdentity failed", .{});
-                            this.flags.did_have_handshaking_error = true;
-
-                            this.unregisterAbortTracker();
+                            // checkServerIdentity already called closeAndFail()
+                            // → fail() → result callback, which may have
+                            // destroyed the AsyncHTTP that embeds `this`. Do not
+                            // touch `this` after a `false` return.
                             return;
                         }
                     },
                     .tcp => |socket| {
                         if (!this.checkServerIdentity(false, socket, handshake_error, ssl_ptr, false)) {
                             log("ProxyTunnel onHandshake checkServerIdentity failed", .{});
-                            this.flags.did_have_handshaking_error = true;
-                            this.unregisterAbortTracker();
+                            // see .ssl arm — `this` may be freed here.
                             return;
                         }
                     },

src/js/node/_http_client.ts

@@ -779,6 +779,10 @@ function ClientRequest(input, options, cb) {
     validateInteger(mergedTlsOptions.secureOptions, "options.secureOptions");
     this._ensureTls().secureOptions = mergedTlsOptions.secureOptions;
   }
+  if (mergedTlsOptions.checkServerIdentity !== undefined) {
+    validateFunction(mergedTlsOptions.checkServerIdentity, "options.checkServerIdentity");
+    this._ensureTls().checkServerIdentity = mergedTlsOptions.checkServerIdentity;
+  }
   this[kPath] = options.path || "/";
   if (cb) {
     this.once("response", cb);

src/js/node/_http_server.ts

@@ -197,6 +197,19 @@ function emitListeningNextTick(self, hostname, port) {
   }
 }
 
+// Node.js only requests a client certificate when `requestCert: true`.
+// The uSockets SSL context treats `ca` alone as "verify peer", so without
+// these two flags an `https.Server({ ca })` would reject every client that
+// doesn't present a cert. Mirror tls.Server (net.ts): default `requestCert`
+// to false and, when not requesting, force `rejectUnauthorized` to false so
+// the CA is loaded into the trust store without requiring a client cert.
+function normalizeServerTls(tls) {
+  const requestCert = !!tls.requestCert;
+  tls.requestCert = requestCert;
+  tls.rejectUnauthorized = requestCert ? tls.rejectUnauthorized !== false : false;
+  return tls;
+}
+
 function Server(options, callback): void {
   if (!(this instanceof Server)) return new Server(options, callback);
   EventEmitter.$call(this);
@@ -251,14 +264,16 @@ function Server(options, callback): void {
     }
 
     if (this[isTlsSymbol]) {
-      this[tlsSymbol] = {
+      this[tlsSymbol] = normalizeServerTls({
         serverName,
         key,
         cert,
         ca,
         passphrase,
         secureOptions,
-      };
+        requestCert: options.requestCert,
+        rejectUnauthorized: options.rejectUnauthorized,
+      });
     } else {
       this[tlsSymbol] = null;
     }
@@ -385,7 +400,7 @@ Server.prototype.listen = function () {
 
       const otherTLS = arguments[0].tls;
       if (otherTLS && $isObject(otherTLS)) {
-        tls = otherTLS;
+        tls = normalizeServerTls({ ...otherTLS });
       }
     } else if (typeof arguments[0] === "string" && !(Number(arguments[0]) >= 0)) {
       // (path[...][, cb])