From 937856bf0f83be5c481a9b1c53aded584befefb8 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 14 Oct 2026 11:01:44 -0700 Subject: [PATCH] fix(tls): omit implicit SNI for absent and null servernames Pass an explicit empty native name when tls.connect has no servername, while preserving pending setServername state and hostname verification. Keep HTTP/2 authority defaults in the HTTP/2 adapter and report false on established client sockets without SNI. Supersedes stale oven-sh/bun#33476 by robobun; thanks @robobun. Ports openclaw/bun#171. Related to the unwanted SNI in oven-sh/bun#20727. --- src/js/node/http2.ts | 22 +-- src/js/node/net.ts | 1 + src/js/node/tls.ts | 11 +- ...-tls-connect-hostname-verification.test.ts | 14 ++ test/js/node/tls/node-tls-connect.test.ts | 5 + .../node/tls/node-tls-servername-fixture.mjs | 132 ++++++++++++++++++ 6 files changed, 161 insertions(+), 24 deletions(-) create mode 100644 test/js/node/tls/node-tls-servername-fixture.mjs diff --git a/src/js/node/http2.ts b/src/js/node/http2.ts index 72b86355d452..94cb1b6becb1 100644 --- a/src/js/node/http2.ts +++ b/src/js/node/http2.ts @@ -5885,22 +5885,12 @@ class ClientHttp2Session extends Http2Session { } else { // Like node, https only: https://github.com/nodejs/node/blob/v26.3.0/lib/internal/http2/core.js#L3638 if (protocol === "https:") initializeOptions(options); - socket = connectWithProtocol( - protocol, - options - ? { - host, - port: String(port), - ALPNProtocols: ["h2"], - ...options, - } - : { - host, - port: String(port), - ALPNProtocols: ["h2"], - }, - onConnect.bind(this), - ); + const connectOptions = { host, port: String(port), ALPNProtocols: ["h2"], ...options }; + // HTTP/2 derives SNI from its authority; tls.connect itself never derives it. + if (protocol === "https:" && !connectOptions.servername && !net.isIP(host)) { + connectOptions.servername = host; + } + socket = connectWithProtocol(protocol, connectOptions, onConnect.bind(this)); this[bunHTTP2Socket] = socket; } this.#encrypted = socket instanceof TLSSocket; diff --git a/src/js/node/net.ts b/src/js/node/net.ts index 6caa13d8f86e..61106d90ba06 100644 --- a/src/js/node/net.ts +++ b/src/js/node/net.ts @@ -1063,6 +1063,7 @@ function onClientHandshake( self._securePending = false; self._secureEstablished = true; self[kVerifyError] = verifyError ?? null; + self.servername = self.servername || false; self.alpnProtocol = socket.alpnProtocol; // Node has no try/catch around these emits; a listener throw reaches // InternalCallbackScope as uncaughtException. reportError mirrors that diff --git a/src/js/node/tls.ts b/src/js/node/tls.ts index 6cf042c09846..882c1a51009a 100644 --- a/src/js/node/tls.ts +++ b/src/js/node/tls.ts @@ -1161,15 +1161,10 @@ TLSSocket.prototype.getX509Certificate = function getX509Certificate(this: $Node return this._handle?.getX509Certificate?.(); }; -TLSSocket.prototype[buntls] = function (this: $Node.TLSSocket, port: unknown, host: string | null | undefined) { +TLSSocket.prototype[buntls] = function (this: $Node.TLSSocket) { const ctx = this[ksecureContext]; - // RFC 6066 forbids IP literals in SNI. Match Node.js: only default servername to host - // when host is not an IP. For IP hosts, pass "" so the native layer skips SNI instead of - // falling back to the connection host. - let servername = this.servername || this[kservername] || ctx?.servername; - if (servername === undefined) { - servername = host && !net.isIP(host) ? host : ""; - } + // An empty native serverName suppresses its connection-host fallback; tls.connect does not infer SNI. + const servername = this.servername || this[kservername] || ""; return { ALPNProtocols: this.ALPNProtocols, checkServerIdentity: this[kcheckServerIdentity], diff --git a/test/js/node/tls/node-tls-connect-hostname-verification.test.ts b/test/js/node/tls/node-tls-connect-hostname-verification.test.ts index 2f0219de2e3f..02ad4e7950ef 100644 --- a/test/js/node/tls/node-tls-connect-hostname-verification.test.ts +++ b/test/js/node/tls/node-tls-connect-hostname-verification.test.ts @@ -8,6 +8,11 @@ import net, { type AddressInfo } from "node:net"; import path from "node:path"; import { describe, test } from "node:test"; import tls from "node:tls"; +import { + expected as expectedServername, + observe as observeServername, + cases as servernameCases, +} from "./node-tls-servername-fixture.mjs"; // Server presents a cert for CN=agent1 (no SAN), signed by ca1. // A client connecting to host "localhost" with ca1 trusted will pass chain @@ -605,3 +610,12 @@ describe("Bun.connect TLS hostname verification", () => { } }); }); + +// The same fixture runs directly under Node.js 24.21.0. +describe("TLS servername selection", () => { + for (const scenario of servernameCases) { + test(`${scenario.transport} host=${scenario.host} servername=${String(scenario.servername)}`, async () => { + assert.deepStrictEqual(await observeServername(scenario), expectedServername(scenario)); + }); + } +}); diff --git a/test/js/node/tls/node-tls-connect.test.ts b/test/js/node/tls/node-tls-connect.test.ts index 06a4714c615d..ebacf297f8f1 100644 --- a/test/js/node/tls/node-tls-connect.test.ts +++ b/test/js/node/tls/node-tls-connect.test.ts @@ -717,6 +717,7 @@ for (const { name, connect } of tests) { let socket = connect({ port: 443, host: "bun.sh", + servername: "bun.sh", rejectUnauthorized: false, }); @@ -736,6 +737,7 @@ for (const { name, connect } of tests) { // Test using port and host itNetwork("should process port and host correctly", done => { let socket = connect(443, "bun.sh", { + servername: "bun.sh", rejectUnauthorized: false, }); @@ -760,6 +762,7 @@ for (const { name, connect } of tests) { 443, "bun.sh", { + servername: "bun.sh", rejectUnauthorized: false, }, () => { @@ -778,6 +781,7 @@ for (const { name, connect } of tests) { // Additional tests to ensure the callback is optional and handled correctly itNetwork("should handle the absence of a callback gracefully", done => { let socket = connect(443, "bun.sh", { + servername: "bun.sh", rejectUnauthorized: false, }); @@ -803,6 +807,7 @@ for (const { name, connect } of tests) { { port: 443, host: "bun.sh", + servername: "bun.sh", }, () => { socket.setTimeout(1000, () => { diff --git a/test/js/node/tls/node-tls-servername-fixture.mjs b/test/js/node/tls/node-tls-servername-fixture.mjs new file mode 100644 index 000000000000..2944b6dde223 --- /dev/null +++ b/test/js/node/tls/node-tls-servername-fixture.mjs @@ -0,0 +1,132 @@ +import assert from "node:assert/strict"; +import { once } from "node:events"; +import { readFileSync } from "node:fs"; +import http2 from "node:http2"; +import https from "node:https"; +import net from "node:net"; +import tls from "node:tls"; +import { fileURLToPath } from "node:url"; + +const key = readFileSync(new URL("./fixtures/agent1-key.pem", import.meta.url)); +const cert = readFileSync(new URL("./fixtures/agent1-cert.pem", import.meta.url)); +const ca = readFileSync(new URL("./fixtures/ca1-cert.pem", import.meta.url)); + +export const cases = [ + ...["127.0.0.1", "localhost"].flatMap(host => + ["omitted", undefined, null, "", "files.proxy.test"].map(servername => ({ host, servername, transport: "tls" })), + ), + ...["socket", "context"].flatMap(transport => + [undefined, null, "", "files.proxy.test"].map(servername => ({ host: "127.0.0.1", servername, transport })), + ), + ...["https", "http2"].flatMap(transport => + ["127.0.0.1", "localhost"].flatMap(host => + [undefined, null, "", "files.proxy.test"].map(servername => ({ host, servername, transport })), + ), + ), +]; + +export function expected({ host, servername, transport }) { + const name = + servername && servername !== "omitted" + ? servername + : (transport === "http2" || (transport === "https" && servername !== "")) && host === "localhost" + ? host + : false; + return { servername: name, clientServername: name, identity: name || host, authorized: true }; +} + +export async function observe({ host, servername, transport }) { + const isHttps = transport === "https"; + const isHttp2 = transport === "http2"; + const server = isHttp2 + ? http2.createSecureServer({ key, cert }) + : isHttps + ? https.createServer({ key, cert }, (req, res) => res.end(JSON.stringify({ servername: req.socket.servername }))) + : tls.createServer({ key, cert }, socket => socket.end(JSON.stringify({ servername: socket.servername }))); + if (isHttp2) + server.on("stream", stream => stream.end(JSON.stringify({ servername: stream.session.socket.servername }))); + server.listen(0, "127.0.0.1"); + await once(server, "listening"); + let socket; + let raw; + let agent; + let session; + try { + const port = server.address().port; + let identity; + const options = { + host, + port, + ca, + lookup(_host, _options, callback) { + if (_options.all) callback(null, [{ address: "127.0.0.1", family: 4 }]); + else callback(null, "127.0.0.1", 4); + }, + checkServerIdentity(hostname) { + identity = hostname; + }, + ...(servername === "omitted" ? {} : { servername }), + }; + if (transport === "context") options.secureContext = tls.createSecureContext({ ca }); + if (transport === "socket") { + raw = net.connect({ host: "127.0.0.1", port }); + await once(raw, "connect"); + options.socket = raw; + } + let clientServername; + let authorized; + let body = ""; + if (isHttp2) { + session = http2.connect(`https://${host}:${port}`, options); + await once(session, "connect"); + socket = session.socket; + clientServername = socket.servername; + authorized = socket.authorized; + const request = session.request(); + request.setEncoding("utf8"); + request.on("data", chunk => (body += chunk)); + request.end(); + await once(request, "end"); + } else if (isHttps) { + agent = new https.Agent(options); + await new Promise((resolve, reject) => { + const req = https.get({ host, port, agent }, res => { + socket = res.socket; + clientServername = socket.servername; + authorized = socket.authorized; + res.setEncoding("utf8"); + res.on("data", chunk => (body += chunk)); + res.on("error", reject); + res.on("end", resolve); + }); + req.on("error", reject); + }); + } else { + socket = tls.connect(options); + socket.on("secureConnect", () => { + clientServername = socket.servername; + authorized = socket.authorized; + }); + socket.setEncoding("utf8"); + socket.on("data", chunk => (body += chunk)); + await once(socket, "close"); + } + return { ...JSON.parse(body), clientServername, identity, authorized }; + } finally { + if (session) { + session.close(); + await once(session, "close"); + } else socket?.destroy(); + raw?.destroy(); + agent?.destroy(); + await new Promise((resolve, reject) => server.close(error => (error ? reject(error) : resolve()))); + } +} + +if (process.argv[1] === fileURLToPath(import.meta.url)) { + for (const scenario of cases) { + const result = await observe(scenario); + console.log(JSON.stringify({ ...scenario, result })); + assert.deepEqual(result, expected(scenario)); + } +}