Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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<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.

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<u8>`, `ListenerTlsSpec`, etc.), and never stored in the struct. The struct only holds types that are provably `Send + Sync`.
Expand Down
122 changes: 122 additions & 0 deletions __test__/session-resumption.spec.ts
Original file line number Diff line number Diff line change
@@ -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<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. 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);
});
});
53 changes: 53 additions & 0 deletions __test__/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<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;
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<Buffer> {
return new Promise((resolve, reject) => {
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
27 changes: 24 additions & 3 deletions src/proxy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -281,6 +282,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
Expand Down Expand Up @@ -386,8 +390,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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(&current))
.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(&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 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
Expand Down Expand Up @@ -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).
Expand Down
Loading
Loading