From 6348b46b38abeb1547e50bcfc3a570acc76e3682 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Fri, 31 Jul 2026 14:46:09 -0600 Subject: [PATCH] Keep TLS session resumption alive across config reloads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit symphony already configures rustls for session resumption (a session cache for TLS 1.2 IDs, a ticketer for TLS 1.3 tickets), and it works — but only until the first config reload. Both of those live on the rustls ServerConfig, and build_route_table created a TlsConfigCache per call. Every route-table rebuild therefore minted a brand-new ServerConfig for every route, including ones whose cert had not changed at all, which threw away the session cache and generated fresh random ticket keys. Every ticket already handed to a client became undecryptable. Nothing errors — clients just quietly go back to full handshakes. That matters because reloads are routine: adding or removing a route, or an on-disk cert renewal picked up by the file watcher, rebuilds the whole table. Measured against the current build: a TLS 1.3 client resumes on its second connection, then fails to resume after an updateConfig() that changes nothing. Fix: the proxy owns the TlsConfigCache and threads it through every build_route_table, so an unchanged cert maps to the same Arc and keeps its session state. Keying on the cert bytes gives the right lifetime for free — sessions live as long as the cert they were issued under, and a rotation retires them. Two details worth keeping: - The sweep (retain_used, mark-and-sweep over keys touched during a build) is the caller's and runs at the commit point in proxy.rs, 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 sweeping against a table that never went live would retire configs the still-running table is serving from. Over-retaining for one generation is the safe direction. - Ticket keys stay per-config, not process-global. A shared ticketer would let a ticket minted under one tenant's cert resume against another tenant's route. Tests: unit tests for config identity across rebuilds (with the per-build cache as an explicit control) and for the sweep; an end-to-end spec covering TLS 1.2 and 1.3 resumption, survival across a reload, and correct non-resumption across a cert rotation. The reload assertions fail on the previous behaviour. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 11 ++- __test__/session-resumption.spec.ts | 122 ++++++++++++++++++++++++++++ __test__/util.ts | 53 ++++++++++++ package.json | 2 +- src/proxy.rs | 27 +++++- src/router.rs | 81 +++++++++++++++--- src/tls.rs | 47 ++++++++++- 7 files changed, 323 insertions(+), 20 deletions(-) create mode 100644 __test__/session-resumption.spec.ts diff --git a/CLAUDE.md b/CLAUDE.md index 584a95c..df95b12 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -128,7 +128,16 @@ 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`. `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 deduplication (TlsConfigCache) -Routes that share the same cert+mTLS combination share a single `Arc` 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` 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. + +Two properties keep that honest: + +- **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 sweeping against a table that never went live would retire configs the still-running table is serving from. 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. + +Two 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`, `ListenerTlsSpec`, etc.), and never stored in the struct. The struct only holds types that are provably `Send + Sync`. diff --git a/__test__/session-resumption.spec.ts b/__test__/session-resumption.spec.ts new file mode 100644 index 0000000..4f6856c --- /dev/null +++ b/__test__/session-resumption.spec.ts @@ -0,0 +1,122 @@ +/** + * 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>; + 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. Asserted so the sweep isn't "fixed" into retaining forever. + 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); + }); +}); diff --git a/__test__/util.ts b/__test__/util.ts index 2c31423..83c7d59 100644 --- a/__test__/util.ts +++ b/__test__/util.ts @@ -442,6 +442,59 @@ 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 { + 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; + const finish = () => { + if (settled) return; + settled = true; + clearTimeout(timer); + socket.end(); + resolve(result); + }; + socket.once('session', (s: Buffer) => { + result.session = s; + finish(); + }); + const 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 { return new Promise((resolve, reject) => { diff --git a/package.json b/package.json index 46f8c02..52b7387 100644 --- a/package.json +++ b/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", diff --git a/src/proxy.rs b/src/proxy.rs index 5b20416..f151131 100644 --- a/src/proxy.rs +++ b/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; @@ -281,6 +282,9 @@ pub struct SymphonyProxyWrap { listener_states: Vec, // Interior mutability for start/stop shutdown_tx: Mutex>>, + // 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, js_emit: Arc>, // Dedicated multi-thread runtime for all proxy I/O. // napi's tokio_rt runs a single-threaded executor; spawning proxy tasks there @@ -386,8 +390,10 @@ impl SymphonyProxyWrap { .iter() .map(parse_route_spec) .collect::>>()?; - 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 = emit_fn @@ -441,6 +447,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, @@ -565,8 +572,17 @@ 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(¤t)) - .map_err(|e| napi::Error::from_reason(e.to_string()))?; + let mut cache = self + .tls_cache + .lock() + .map_err(|_| napi::Error::from_reason("tls config cache poisoned"))?; + let table = + build_route_table(&specs, &self.default_listener_tls, Some(¤t), &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 retiring + // configs for a table that never goes live would drop the running table's session + // state. The sweep happens at the commit point below. Some(table) } else { None @@ -611,6 +627,11 @@ 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. + if let Ok(mut cache) = self.tls_cache.lock() { + cache.retain_used(); + } } if let Some((protection_updates, parsed)) = validated_protection { // Phase 3: store all (infallible — validation above guarantees each port is valid). diff --git a/src/router.rs b/src/router.rs index c9d96d6..9c82285 100644 --- a/src/router.rs +++ b/src/router.rs @@ -352,12 +352,18 @@ impl ListenerTlsSpec { /// a route's cert fails to build, its last-good route is carried forward from `previous` if /// present, so a transient rotation mismatch keeps serving the old (still-valid) cert instead /// of dropping the SNI. +/// +/// `cache` is the proxy's long-lived `TlsConfigCache`, deliberately *not* created here: an +/// unchanged cert must map to the same `Arc` across rebuilds so its TLS session +/// state survives the reload. Sweeping it (`retain_used`) is the *caller's* job, once it commits +/// the returned table — this table is only one half of an all-or-nothing `updateConfig`, and +/// retiring configs for a table that never goes live would strand the running table's sessions. pub fn build_route_table( specs: &[RouteSpec], listener_tls: &ListenerTlsSpec, previous: Option<&RouteTable>, + cache: &mut TlsConfigCache, ) -> crate::error::Result { - let mut cache = TlsConfigCache::new(); let mut exact: HashMap, Route> = HashMap::new(); let mut wildcard: Vec<(Arc, Route)> = Vec::new(); let mut monitored_balancers: Vec> = Vec::new(); @@ -392,7 +398,7 @@ pub fn build_route_table( // Isolate per-route failures: a single route whose cert can't be built (e.g. a // rotated key no longer matching an inlined chain → rustls KeyMismatch) must not // abort the whole table and take every other tenant on the port down with it. - let route = match build_route(spec, listener_tls, &mut cache) { + let route = match build_route(spec, listener_tls, cache) { Ok(route) => route, Err(e) => { // Log only on the good→bad transition: a persistently-broken cert would @@ -787,6 +793,59 @@ UlqL1DcgX6Szi9w/p7B4BZO9iA== } } + fn config_ptr(table: &RouteTable, sni: &str) -> *const ServerConfig { + Arc::as_ptr(table.resolve(Some(sni)).expect("route present").tls_config.as_ref().unwrap()) + } + + // A rebuild must hand an unchanged cert back the *same* ServerConfig. Identity is the whole + // point: the TLS session cache and ticket keys live on that allocation, so a fresh one + // silently invalidates every ticket already issued to clients. + #[test] + fn unchanged_cert_keeps_its_server_config_across_rebuilds() { + let specs = vec![tls_route("tenant.example.com", CERT_A, KEY_A)]; + let mut cache = TlsConfigCache::new(); + + let first = build_route_table(&specs, &ListenerTlsSpec::empty(), None, &mut cache) + .expect("initial build"); + cache.retain_used(); + let second = build_route_table(&specs, &ListenerTlsSpec::empty(), Some(&first), &mut cache) + .expect("rebuild"); + cache.retain_used(); + + assert_eq!( + config_ptr(&first, "tenant.example.com"), + config_ptr(&second, "tenant.example.com"), + "an unchanged cert must reuse its ServerConfig, or resumption dies on every reload" + ); + + // Control: a cache that does not outlive the build is exactly the bug — a new allocation. + let detached = + build_route_table(&specs, &ListenerTlsSpec::empty(), None, &mut TlsConfigCache::new()) + .expect("build with a fresh cache"); + assert_ne!( + config_ptr(&first, "tenant.example.com"), + config_ptr(&detached, "tenant.example.com"), + "a per-build cache mints a new config — the behaviour the shared cache exists to avoid" + ); + } + + // Retention is scoped to what the committed table actually references, so a cert that + // rotates out is not held for the life of the process. + #[test] + fn sweep_retires_configs_no_route_references() { + let mut cache = TlsConfigCache::new(); + let specs = vec![tls_route("tenant.example.com", CERT_A, KEY_A)]; + + build_route_table(&specs, &ListenerTlsSpec::empty(), None, &mut cache).expect("build"); + cache.retain_used(); + assert_eq!(cache.len(), 1); + + // The route goes away; nothing asks for its config, so the sweep drops it. + build_route_table(&[], &ListenerTlsSpec::empty(), None, &mut cache).expect("empty build"); + cache.retain_used(); + assert!(cache.is_empty(), "a config no live route references must not be retained"); + } + // A single route with a mismatched cert/key must not abort the whole table: the // healthy co-tenant on the same listener stays routable, the bad one is dropped. #[test] @@ -796,7 +855,7 @@ UlqL1DcgX6Szi9w/p7B4BZO9iA== tls_route("bad.example.com", CERT_A, KEY_B), // KeyMismatch ]; - let table = build_route_table(&specs, &ListenerTlsSpec::empty(), None) + let table = build_route_table(&specs, &ListenerTlsSpec::empty(), None, &mut TlsConfigCache::new()) .expect("a single bad route must not fail the whole build"); assert!( @@ -820,7 +879,7 @@ UlqL1DcgX6Szi9w/p7B4BZO9iA== tls_route("*.acme.com", CERT_A, KEY_A), ]; - let table = build_route_table(&specs, &ListenerTlsSpec::empty(), None).expect("build"); + let table = build_route_table(&specs, &ListenerTlsSpec::empty(), None, &mut TlsConfigCache::new()).expect("build"); assert!( table.resolve(Some("api.acme.com")).is_none(), @@ -838,7 +897,7 @@ UlqL1DcgX6Szi9w/p7B4BZO9iA== fn dropped_wildcard_route_does_not_fall_through_to_default() { let specs = vec![tls_route("*.acme.com", CERT_A, KEY_B)]; // KeyMismatch — dropped - let table = build_route_table(&specs, &ListenerTlsSpec::empty(), None).expect("build"); + let table = build_route_table(&specs, &ListenerTlsSpec::empty(), None, &mut TlsConfigCache::new()).expect("build"); assert!( table.resolve(Some("api.acme.com")).is_none(), @@ -858,7 +917,7 @@ UlqL1DcgX6Szi9w/p7B4BZO9iA== tls_route("api.acme.com", CERT_A, KEY_A), // valid, more specific ]; - let table = build_route_table(&specs, &ListenerTlsSpec::empty(), None).expect("build"); + let table = build_route_table(&specs, &ListenerTlsSpec::empty(), None, &mut TlsConfigCache::new()).expect("build"); assert!( table.resolve(Some("api.acme.com")).is_some(), @@ -877,12 +936,12 @@ UlqL1DcgX6Szi9w/p7B4BZO9iA== fn transient_failure_retains_last_good_on_hot_swap() { // First build with a valid cert → the live table. let good = vec![tls_route("tenant.example.com", CERT_A, KEY_A)]; - let live = build_route_table(&good, &ListenerTlsSpec::empty(), None).expect("initial build"); + let live = build_route_table(&good, &ListenerTlsSpec::empty(), None, &mut TlsConfigCache::new()).expect("initial build"); assert!(live.resolve(Some("tenant.example.com")).is_some()); // Hot-swap where the same SNI now presents a mismatched pair (mid-rotation). let mismatched = vec![tls_route("tenant.example.com", CERT_A, KEY_B)]; - let swapped = build_route_table(&mismatched, &ListenerTlsSpec::empty(), Some(&live)) + let swapped = build_route_table(&mismatched, &ListenerTlsSpec::empty(), Some(&live), &mut TlsConfigCache::new()) .expect("hot-swap must not fail"); assert!( swapped.resolve(Some("tenant.example.com")).is_some(), @@ -894,7 +953,7 @@ UlqL1DcgX6Szi9w/p7B4BZO9iA== ); // With no previous table (initial build), the same bad route is dropped. - let fresh = build_route_table(&mismatched, &ListenerTlsSpec::empty(), None).expect("build"); + let fresh = build_route_table(&mismatched, &ListenerTlsSpec::empty(), None, &mut TlsConfigCache::new()).expect("build"); assert!( fresh.resolve(Some("tenant.example.com")).is_none(), "with no prior route there is nothing to retain — the SNI is dropped" @@ -912,14 +971,14 @@ UlqL1DcgX6Szi9w/p7B4BZO9iA== let mut good = tls_route("tenant.example.com", CERT_A, KEY_A); good.source_address_mode = SourceAddressMode::XForwardedFor; good.protocol = RouteProtocol::Http; - let live = build_route_table(&[good], &ListenerTlsSpec::empty(), None).expect("initial build"); + let live = build_route_table(&[good], &ListenerTlsSpec::empty(), None, &mut TlsConfigCache::new()).expect("initial build"); assert!(live.resolve(Some("tenant.example.com")).is_some()); // Hot-swap drops the protocol declaration — a permanent rejection, not a transient one. let mut broken = tls_route("tenant.example.com", CERT_A, KEY_A); broken.source_address_mode = SourceAddressMode::XForwardedFor; // protocol left at its default (Opaque) — undeclared. - let swapped = build_route_table(&[broken], &ListenerTlsSpec::empty(), Some(&live)).expect("hot-swap must not fail"); + let swapped = build_route_table(&[broken], &ListenerTlsSpec::empty(), Some(&live), &mut TlsConfigCache::new()).expect("hot-swap must not fail"); assert!( swapped.resolve(Some("tenant.example.com")).is_none(), "a permanent protocol-declaration rejection must drop the route, not silently carry the previous one forward" diff --git a/src/tls.rs b/src/tls.rs index cd74b83..7d7eb15 100644 --- a/src/tls.rs +++ b/src/tls.rs @@ -1,7 +1,7 @@ use crate::error::{Result, SymphonyError}; use crate::mtls::SymphonyClientVerifier; use rustls::ServerConfig; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::sync::Arc; use zeroize::Zeroizing; @@ -19,16 +19,45 @@ pub struct MtlsSpec { pub require_client_cert: bool, } +/// key: (cert_sha256, mtls_sha256_or_zeros, http2) +type CacheKey = ([u8; 32], [u8; 32], bool); + /// Builds and deduplicates Arc instances. /// Routes sharing identical cert + mTLS config share one allocation. +/// +/// The cache **outlives a single route-table build** — it is owned by the proxy and threaded +/// through every `build_route_table`. That is load-bearing for TLS session resumption, not just +/// an allocation saving: each `ServerConfig` owns its own session store and ticket keys (see +/// `build_server_config`), so minting a new one for an unchanged cert silently invalidates every +/// outstanding client ticket. Reloads are frequent — a route add/remove or an on-disk cert +/// renewal rebuilds the whole table — so a per-build cache means clients almost never get to +/// resume. Keying on the cert bytes gives exactly the right lifetime: session state survives as +/// long as the cert it was issued under, and rotating a cert retires it. pub struct TlsConfigCache { - // key: (cert_sha256, mtls_sha256_or_zeros, http2) -> Arc - cache: HashMap<([u8; 32], [u8; 32], bool), Arc>, + cache: HashMap>, + /// Keys touched since the last `retain_used()` — the mark half of mark-and-sweep. + used: HashSet, } impl TlsConfigCache { pub fn new() -> Self { - Self { cache: HashMap::new() } + Self { cache: HashMap::new(), used: HashSet::new() } + } + + /// Drop every entry not requested since the previous sweep, retiring rotated-out certs. + /// Callers run this once they *commit* the table they just built — see `build_route_table`. + pub fn retain_used(&mut self) { + let used = std::mem::take(&mut self.used); + self.cache.retain(|k, _| used.contains(k)); + } + + /// Number of live `ServerConfig`s held. + pub fn len(&self) -> usize { + self.cache.len() + } + + pub fn is_empty(&self) -> bool { + self.cache.is_empty() } pub fn get_or_build( @@ -50,11 +79,14 @@ impl TlsConfigCache { let cache_key = (cert_key, mtls_key, http2); if let Some(cfg) = self.cache.get(&cache_key) { + // Mark before returning: a hit is exactly as much "still in use" as a miss. + self.used.insert(cache_key); return Ok(cfg.clone()); } let cfg = build_server_config(cert, mtls, http2)?; self.cache.insert(cache_key, cfg.clone()); + self.used.insert(cache_key); Ok(cfg) } } @@ -103,6 +135,13 @@ fn build_server_config(cert: &CertSpec, mtls: Option<&MtlsSpec>, http2: bool) -> // - session_storage: handles TLS 1.2 session ID resumption. // - ticketer: handles TLS 1.3 PSK-based session ticket resumption (primary // path for modern clients). + // + // Both live *on this ServerConfig*: the cache entries and the ticketer's random keys die + // with it. Rebuilding a config for an unchanged cert therefore looks like a no-op but + // invalidates every ticket already handed out — which is why TlsConfigCache is owned by the + // proxy and survives route-table rebuilds instead of being created per build. Keep them + // per-config rather than process-global: a ticket minted under one tenant's cert should not + // be resumable against another tenant's route. if http2 { cfg.alpn_protocols = vec![b"h2".to_vec(), b"http/1.1".to_vec()]; }