Block H2 TLS coalescing

Due to wildcard certificates, browsers can reuse an open connection for
a.testserver.host for b.testserver.host. On other servers that's fine,
but in our case we use a/b to change actual behaviour at the TLS stage,
so this isn't safe! We reject with 421 which should trigger automatic
browser behaviour to use a separate connection instead. Critical case is
things like http2.testserver.host (fine) then
http2--expired.testserver.host (should get a different cert - can't
reuse the same TLS connection).

Author: pimterry

src/http-handler.ts

@@ -7,6 +7,7 @@ import { HttpRequest, HttpResponse } from './endpoints/http-index.js';
 import { handleWebSocketUpgrade } from './ws-handler.js';
 import { resolveEndpointChain } from './endpoint-chain.js';
 import { getDocsHtml } from './docs-page.js';
+import { TLS_CLIENT_HELLO } from './tls-client-hello.js';
 
 function stopRawDataCapture(req: HttpRequest): void {
     if (req.httpVersion === '2.0') {
@@ -68,6 +69,24 @@ function createHttpRequestHandler(options: {
             }
         }
 
+        // Reject H2 connection coalescing: if :authority doesn't match the TLS SNI,
+        // return 421 so the browser opens a new connection with the correct SNI.
+        if (isHttps && req.httpVersion === '2.0') {
+            const authority = req.headers[':authority']?.toString();
+            if (authority) {
+                const hostWithoutPort = authority.replace(/:\d+$/, '').toLowerCase();
+                const sni = socket.stream?.[TLS_CLIENT_HELLO]?.serverName?.toLowerCase();
+
+                if (sni && sni !== hostWithoutPort) {
+                    res.writeHead(421, { 'content-type': 'text/plain' });
+                    res.end(
+                        `Misdirected Request: TLS connection was established for ${sni} but got a request for ${hostWithoutPort}`
+                    );
+                    return;
+                }
+            }
+        }
+
         const url = new URL(req.url!, `${protocol}://${
             req.headers[':authority'] ?? req.headers['host']
         }`);

src/process-connection.ts

@@ -6,6 +6,7 @@ import {
     parseProxyProtocol,
     detectProxyProtocol,
 } from './proxy-protocol.js';
+import { TLS_CLIENT_HELLO } from './tls-client-hello.js';
 
 const FRAME_HEADER_SIZE = 9;
 
@@ -55,6 +56,7 @@ class DataCapturingStream extends stream.Duplex {
         this.localPort = (wrapped as any).localPort;
         this.encrypted = (wrapped as any).encrypted ?? false;
         this[PROXY_PROTOCOL] = wrapped[PROXY_PROTOCOL];
+        this[TLS_CLIENT_HELLO] = wrapped[TLS_CLIENT_HELLO];
 
         wrapped.on('error', (err) => this.emit('error', err));
         wrapped.on('close', () => this.emit('close'));

test/tls-coalescing.spec.ts

@@ -0,0 +1,52 @@
+import * as net from 'net';
+import * as http2 from 'http2';
+import { expect } from 'chai';
+import { DestroyableServer, makeDestroyable } from 'destroyable-server';
+
+import { createServer } from '../src/server.js';
+
+describe("TLS connection coalescing prevention", () => {
+
+    let server: DestroyableServer;
+    let serverPort: number;
+
+    beforeEach(async () => {
+        server = makeDestroyable(await createServer());
+        await new Promise<void>((resolve) => server.listen(resolve));
+        serverPort = (server.address() as net.AddressInfo).port;
+    });
+
+    afterEach(async () => {
+        await server.destroy();
+    });
+
+    async function h2Request(servername: string, authority: string): Promise<number | undefined> {
+        const client = http2.connect(`https://localhost:${serverPort}`, {
+            rejectUnauthorized: false,
+            servername
+        });
+
+        const status = await new Promise<number | undefined>((resolve, reject) => {
+            const req = client.request({ ':authority': authority, ':path': '/status/200' });
+            req.on('response', (headers) => resolve(headers[':status']));
+            req.on('error', reject);
+            req.end();
+        });
+        client.close();
+
+        return status;
+    }
+
+    it("allows requests where :authority matches the SNI", async () => {
+        expect(await h2Request('http2.localhost', 'http2.localhost')).to.equal(200);
+    });
+
+    it("returns 421 when :authority does not match SNI", async () => {
+        expect(await h2Request('http2.localhost', 'http2--expired.localhost')).to.equal(421);
+    });
+
+    it("returns 421 when :authority includes a port but hostname doesn't match SNI", async () => {
+        expect(await h2Request('http2.localhost', `localhost:${serverPort}`)).to.equal(421);
+    });
+
+});

test/tls-required.spec.ts

@@ -107,7 +107,8 @@ describe("TLS-required endpoints over plain HTTP", () => {
 
     it("does not redirect HTTP/2+TLS requests for TLS subdomains", async () => {
         const client = http2.connect(`https://localhost:${serverPort}`, {
-            rejectUnauthorized: false
+            rejectUnauthorized: false,
+            servername: 'http2.localhost'
         });
 
         const response = await new Promise<{ status: number | undefined }>((resolve, reject) => {