Keep TLS session resumption alive across config reloads

CLAUDE.md

@@ -129,7 +129,19 @@ Each tokio worker thread gets its own listening socket on the same address via `
 Each suspended connection gets a `tokio::sync::oneshot::channel`. The sender is stored in a `DashMap<u64, Sender>`. `resolveConnection()` removes the sender and fires it — synchronous from the JS side (no async needed). `oneshot` is used rather than `mpsc` because exactly one resolution is possible per connection. If no resolution arrives within `suspendTimeoutMs`, the `timeout(rx.await)` in `proxy_conn.rs` returns an error and the TCP stream is dropped.
 
 ### Per-route Arc<ServerConfig> deduplication (TlsConfigCache)
-Routes that share the same cert+mTLS combination share a single `Arc<ServerConfig>` allocation. The cache key is `(sha256(cert_pem + key_pem), sha256(mtls_ca_pem))`. Built at config-parse time in `tls.rs::TlsConfigCache`. Important for deployments where many routes share a wildcard cert.
+Routes that share the same cert+mTLS combination share a single `Arc<ServerConfig>` allocation. The cache key is `(sha256(cert_pem + key_pem), sha256(mtls_ca_pem), http2)`. Important for deployments where many routes share a wildcard cert.
+
+The cache is owned by `SymphonyProxyWrap` and **threaded through every `build_route_table`** rather than created per build. That is what makes TLS session resumption survive a config reload, and it is the non-obvious part: `build_server_config` gives each `ServerConfig` its own `session_storage` (TLS 1.2 session IDs) and its own `Ticketer` (TLS 1.3 ticket keys), so minting a fresh config for an *unchanged* cert silently invalidates every ticket already issued. Nothing errors — clients just quietly fall back to full handshakes. Since a route add/remove or an on-disk cert renewal rebuilds the whole table, a per-build cache meant clients almost never resumed. Keying on the cert bytes gives the right lifetime for free: session state lives exactly as long as the cert it was issued under, and a rotation retires it.
+
+Three properties keep that honest:
+
+- **What survives a sweep is decided by the refcount, not the mark set.** `retain_used()` keeps an entry when it was marked during the build *or* when `Arc::strong_count > 1`, and the second clause is the load-bearing one: the carry-forward branch in `build_route_table` (mid-rotation cert/key mismatch) reuses the previous table's `Arc` without going through `get_or_build`, so a live route can be unmarked. Since the table is swapped in before the sweep runs, a committed table's configs always have `strong_count >= 2`. `clear_used()` at the top of a build evicts on the same rule inverted (`strong_count == 1` — cache-only), which is what bounds a reload that fails validation over and over.
+- **The sweep is the caller's, at the commit point.** `retain_used()` (mark-and-sweep over the keys touched during a build) runs in `proxy.rs` *after* the new table is swapped in — never inside `build_route_table`. `updateConfig` is all-or-nothing: a route build can succeed and then be discarded when the protection half fails validation, and a sweep driven by that build's marks names none of the live table's configs. The refcount clause above spares them regardless, so this is the belt to that suspenders — but it is what keeps the placement from depending on it. Over-retaining for one generation is the safe direction; under-retaining costs live session state.
+- **Ticket keys stay per-config, not process-global.** A process-wide ticketer would let a ticket minted under one tenant's cert resume against another tenant's route. Sharing across configs is the obvious "optimisation" here and it is the wrong one.
+
+Scope: this is one proxy's *route table* cache, and it lives as long as that `SymphonyProxy` does. A reload that only changes routes keeps it; anything that changes a listener signature — including an on-disk rotation of a listener `defaultCert`/mTLS CA, since `server.ts` computes `listenerSig` over the *resolved* listeners — recreates the proxy, and every route on that port starts over with fresh ticketers. Hoisting the cache to process scope would fix that (it is keyed on cert bytes, so it cannot leak a ticket across tenants) and is not done here. `suspended.rs::build_resolved_route` is outside it too: it builds from a throwaway cache per resolution, so a JS-resolved suspended connection re-parses its cert and gets a fresh ticketer — those clients never resume.
+
+Two further things are deliberately *not* solved: rustls' ring `Ticketer` rotates keys on its own ~6h schedule (independent of reloads), and ticket keys are per-process, so during host-manager's overlapping-process upgrade window clients that land on the new process fall back to a full handshake until they hold one of its tickets. Both degrade to a full handshake, never to an error. `__test__/session-resumption.spec.ts` covers all of this end-to-end (TLS 1.2 and 1.3, reload, and rotation).
 
 ### Send + Sync in napi classes
 napi `Buffer` contains raw pointers (`*mut napi_env__`, `*mut napi_ref__`) that are not `Sync`. The `SymphonyProxyWrap` struct must be `Send + Sync`. Solution: napi types (`Buffer`, `JsCertConfig`, etc.) are only used as constructor/method *parameters*, immediately converted to plain Rust (`Vec<u8>`, `ListenerTlsSpec`, etc.), and never stored in the struct. The struct only holds types that are provably `Send + Sync`.

__test__/session-resumption.spec.ts

@@ -0,0 +1,124 @@
+/**
+ * TLS session resumption — that symphony offers it at all, and that a config reload doesn't
+ * silently take it away.
+ *
+ * The second property is the fragile one. Resumption state (the TLS 1.2 session cache and the
+ * TLS 1.3 ticket keys) lives on the rustls `ServerConfig`, so anything that rebuilds a config
+ * for an unchanged cert invalidates every ticket already handed out — with no error anywhere,
+ * just a quiet return to full handshakes. Reloads are routine in production (a route add or an
+ * on-disk cert renewal rebuilds the whole route table), so without a route-table-outliving
+ * config cache, clients would rarely get to resume at all.
+ *
+ * Requires the native addon:
+ *   npm run build:debug
+ */
+
+import assert from 'node:assert/strict';
+import { after, before, describe, it } from 'node:test';
+import { SymphonyProxy } from '../ts/proxy.js';
+import { generateSelfSignedCert, getFreePort, startEchoServer, tlsHandshake, sleep } from './util.js';
+
+describe('TLS session resumption', () => {
+	const cert = generateSelfSignedCert('localhost');
+	let proxyPort: number;
+	let echo: Awaited<ReturnType<typeof startEchoServer>>;
+	let proxy: SymphonyProxy;
+
+	const routes = () => [
+		{
+			sni: 'localhost',
+			upstreams: [{ kind: 'tcp' as const, host: '127.0.0.1', port: echo.port }],
+			terminateTls: true,
+			cert: { certChain: cert.cert, privateKey: cert.key },
+		},
+	];
+
+	before(async () => {
+		echo = await startEchoServer();
+		proxyPort = await getFreePort();
+		proxy = new SymphonyProxy({ listeners: [{ host: '127.0.0.1', port: proxyPort }], routes: routes() });
+		await proxy.start();
+		await sleep(50);
+	});
+
+	after(async () => {
+		await proxy.stop();
+		await echo.close();
+	});
+
+	for (const maxVersion of ['TLSv1.3', 'TLSv1.2'] as const) {
+		describe(maxVersion, () => {
+			it('issues a session and resumes it on a second connection', async () => {
+				const first = await tlsHandshake({ port: proxyPort, servername: 'localhost', caCert: cert.cert, maxVersion });
+				assert.equal(first.protocol, maxVersion);
+				assert.equal(first.reused, false, 'the first handshake cannot be a resumption');
+				assert.ok(first.session, 'the server must issue a session to resume against');
+
+				const second = await tlsHandshake({
+					port: proxyPort,
+					servername: 'localhost',
+					caCert: cert.cert,
+					maxVersion,
+					session: first.session,
+				});
+				assert.equal(second.reused, true, 'the offered session must be accepted');
+			});
+
+			// The regression this file exists for: a reload that leaves the cert untouched must
+			// leave outstanding sessions resumable. It reproduces as reused === false.
+			it('keeps sessions resumable across an updateConfig() that does not change the cert', async () => {
+				const first = await tlsHandshake({ port: proxyPort, servername: 'localhost', caCert: cert.cert, maxVersion });
+				assert.ok(first.session);
+
+				await proxy.updateConfig({ routes: routes() });
+				await sleep(50);
+
+				const afterReload = await tlsHandshake({
+					port: proxyPort,
+					servername: 'localhost',
+					caCert: cert.cert,
+					maxVersion,
+					session: first.session,
+				});
+				assert.equal(
+					afterReload.reused,
+					true,
+					'a reload must not mint a new ServerConfig for an unchanged cert — that discards the ticket keys'
+				);
+			});
+		});
+	}
+
+	// A rotated cert is a different identity: its predecessor's session state going away is
+	// correct, not a regression. This asserts the client-visible behaviour only — a ticket minted
+	// under the old cert's ticketer cannot decrypt under the new one whether or not the old config
+	// is still cached. Eviction itself is proven in `router.rs::sweep_retires_configs_no_route_references`.
+	it('does not resume a session across a cert rotation', async () => {
+		const first = await tlsHandshake({
+			port: proxyPort,
+			servername: 'localhost',
+			caCert: cert.cert,
+			maxVersion: 'TLSv1.3',
+		});
+		assert.ok(first.session);
+
+		const rotated = generateSelfSignedCert('localhost');
+		await proxy.updateConfig({
+			routes: [{ ...routes()[0], cert: { certChain: rotated.cert, privateKey: rotated.key } }],
+		});
+		await sleep(50);
+
+		const afterRotation = await tlsHandshake({
+			port: proxyPort,
+			servername: 'localhost',
+			caCert: rotated.cert,
+			maxVersion: 'TLSv1.3',
+			session: first.session,
+		});
+		assert.equal(afterRotation.reused, false, 'a session issued under the old cert must not resume under the new one');
+
+		// Restore the original cert so ordering between tests stays irrelevant.
+		await proxy.updateConfig({ routes: routes() });
+		await sleep(50);
+	});
+});

__test__/util.ts

@@ -442,6 +442,67 @@ export function tlsAlpn(opts: {
 	});
 }
 
+export interface TlsHandshakeResult {
+	/** True when the server accepted the offered session (ticket or session ID). */
+	reused: boolean;
+	protocol: string | null;
+	/** The session to offer on a subsequent connect, or undefined if the server issued none. */
+	session?: Buffer;
+}
+
+/**
+ * Complete one TLS handshake and report whether it resumed, plus the session to reuse next time.
+ * TLS 1.3 tickets arrive *after* the handshake (the 'session' event), so a fresh connection has
+ * to linger briefly for one; a resumed connection is reported as soon as the handshake completes.
+ */
+export function tlsHandshake(opts: {
+	port: number;
+	host?: string;
+	servername: string;
+	caCert?: string;
+	session?: Buffer;
+	maxVersion?: 'TLSv1.2' | 'TLSv1.3';
+	/** How long to wait for a post-handshake session ticket. */
+	ticketWaitMs?: number;
+}): Promise<TlsHandshakeResult> {
+	return new Promise((resolve, reject) => {
+		const { port, host = '127.0.0.1', servername, caCert, session, maxVersion, ticketWaitMs = 1000 } = opts;
+		const socket = tls.connect(
+			{ port, host, servername, ca: caCert, rejectUnauthorized: false, session, minVersion: 'TLSv1.2', maxVersion },
+			() => {
+				const result: TlsHandshakeResult = {
+					reused: socket.isSessionReused(),
+					protocol: socket.getProtocol(),
+					// TLS 1.2 hands the session over at handshake time; 1.3 does not.
+					session: socket.getSession() ?? undefined,
+				};
+				let settled = false;
+				let timer: NodeJS.Timeout | undefined;
+				const finish = () => {
+					if (settled) return;
+					settled = true;
+					clearTimeout(timer);
+					socket.end();
+					resolve(result);
+				};
+				// Only 1.3 is worth waiting on: under 1.2 the session is already in hand, and the
+				// 'session' event may have fired before this callback ran, so waiting would just
+				// burn the full ticket timeout on every fresh connection.
+				if (result.protocol !== 'TLSv1.3' && result.session) {
+					finish();
+					return;
+				}
+				socket.once('session', (s: Buffer) => {
+					result.session = s;
+					finish();
+				});
+				timer = setTimeout(finish, result.reused ? 100 : ticketWaitMs);
+			},
+		);
+		socket.on('error', reject);
+	});
+}
+
 /** Send data through a raw TCP connection and return the echoed response. */
 export function tcpRoundTrip(opts: { port: number; host?: string; data: Buffer | string }): Promise<Buffer> {
 	return new Promise((resolve, reject) => {

package.json

@@ -31,7 +31,7 @@
 		"artifacts": "napi artifacts",
 		"prepublishOnly": "napi prepublish -t npm",
 		"version": "napi version",
-		"test": "tsc -p tsconfig.test.json && node --test --test-force-exit dist-test/__test__/protection.spec.js dist-test/__test__/proxy.spec.js dist-test/__test__/proxy-protocol-v2.spec.js dist-test/__test__/mtls.spec.js dist-test/__test__/suspended.spec.js dist-test/__test__/http-listener.spec.js dist-test/__test__/server.spec.js dist-test/__test__/h2-dispatch.spec.js dist-test/__test__/metrics.spec.js dist-test/__test__/copy-buffers.spec.js dist-test/__test__/route-protocol.spec.js",
+		"test": "tsc -p tsconfig.test.json && node --test --test-force-exit dist-test/__test__/protection.spec.js dist-test/__test__/proxy.spec.js dist-test/__test__/proxy-protocol-v2.spec.js dist-test/__test__/mtls.spec.js dist-test/__test__/suspended.spec.js dist-test/__test__/http-listener.spec.js dist-test/__test__/server.spec.js dist-test/__test__/h2-dispatch.spec.js dist-test/__test__/metrics.spec.js dist-test/__test__/copy-buffers.spec.js dist-test/__test__/route-protocol.spec.js dist-test/__test__/session-resumption.spec.js",
 		"test:integration": "tsc -p tsconfig.test.json && node --test --test-force-exit dist-test/__test__/harper-integration.spec.js",
 		"benchmark": "tsc -p tsconfig.test.json && node dist-test/__test__/benchmark.js",
 		"benchmark:throughput": "tsc -p tsconfig.test.json && node dist-test/__test__/benchmark-throughput.js",

src/proxy.rs

@@ -10,6 +10,7 @@ use crate::router::{
 	RouteProtocol, RouteSpec, SourceAddressMode, UpstreamSpec,
 };
 use crate::suspended::{build_resolved_route, ResolveSpec, ResolveUpstream, SuspendedRegistry};
+use crate::tls::TlsConfigCache;
 use ipnetwork::IpNetwork;
 use napi::bindgen_prelude::*;
 use napi::threadsafe_function::ThreadsafeFunction;
@@ -300,6 +301,9 @@ pub struct SymphonyProxyWrap {
 	listener_states: Vec<ListenerState>,
 	// Interior mutability for start/stop
 	shutdown_tx: Mutex<Option<broadcast::Sender<()>>>,
+	// Outlives every route-table build so an unchanged cert keeps the same ServerConfig — and
+	// with it the TLS session cache and ticket keys — across hot-swaps.
+	tls_cache: Mutex<TlsConfigCache>,
 	js_emit: Arc<ThreadsafeFunction<JsEvent>>,
 	// Dedicated multi-thread runtime for all proxy I/O.
 	// napi's tokio_rt runs a single-threaded executor; spawning proxy tasks there
@@ -405,8 +409,10 @@ impl SymphonyProxyWrap {
 			.iter()
 			.map(parse_route_spec)
 			.collect::<Result<Vec<_>>>()?;
-		let table = build_route_table(&specs, &default_listener_tls, None)
+		let mut tls_cache = TlsConfigCache::new();
+		let table = build_route_table(&specs, &default_listener_tls, None, &mut tls_cache)
 			.map_err(|e| napi::Error::from_reason(e.to_string()))?;
+		tls_cache.retain_used();
 
 		// Set up threadsafe event emitter
 		let js_emit: ThreadsafeFunction<JsEvent> = emit_fn
@@ -460,6 +466,7 @@ impl SymphonyProxyWrap {
 			global_metrics: Arc::new(GlobalMetrics::default()),
 			listener_states,
 			shutdown_tx: Mutex::new(None),
+			tls_cache: Mutex::new(tls_cache),
 			js_emit: Arc::new(js_emit),
 			rt: Mutex::new(Some(rt)),
 			rt_handle,
@@ -584,8 +591,20 @@ impl SymphonyProxyWrap {
 			// rebuild (mid-rotation KeyMismatch) retains its last-good cert instead of
 			// dropping the SNI from the live table.
 			let current = self.route_table.0.load();
-			let table = build_route_table(&specs, &self.default_listener_tls, Some(&current))
-				.map_err(|e| napi::Error::from_reason(e.to_string()))?;
+			// Recover a poisoned lock instead of propagating: the cache holds no invariant a
+			// panicking build can break — `clear_used` reconciles it at the start of the next
+			// build — and refusing the lock forever would turn one panicked reload into a proxy
+			// that can never pick up a renewed cert again.
+			let mut cache = self.tls_cache.lock().unwrap_or_else(|e| e.into_inner());
+			let table =
+				build_route_table(&specs, &self.default_listener_tls, Some(&current), &mut cache)
+					.map_err(|e| napi::Error::from_reason(e.to_string()))?;
+			drop(cache);
+			// Not swept here: a later validation failure aborts the whole update, and a sweep
+			// against marks from a table that never goes live names none of the running table's
+			// configs. The refcount clause in `retain_used` would spare them anyway (the live
+			// table still holds them) — sweeping only at the commit point below means the
+			// placement doesn't depend on that.
 			Some(table)
 		} else {
 			None
@@ -630,6 +649,9 @@ impl SymphonyProxyWrap {
 		// All validation passed — apply atomically (routes then protection, neither applied on any error above).
 		if let Some(table) = new_route_table {
 			self.route_table.swap(table);
+			// Committed — now retire the ServerConfigs (and their session state) that no route
+			// in the new table asked for, i.e. rotated-out certs.
+			self.tls_cache.lock().unwrap_or_else(|e| e.into_inner()).retain_used();
 		}
 		if let Some((protection_updates, parsed)) = validated_protection {
 			// Phase 3: store all (infallible — validation above guarantees each port is valid).