Harden JA3/JA4 fingerprinting: fragmented-hello bypass, blocklist validation, GREASE sig-algs, hot-path allocs

CLAUDE.md

@@ -18,7 +18,7 @@ The server also **watches the cert/key files referenced by the config** (grouped
 
 ```
 TCP accept (SO_REUSEPORT per worker thread)
-  └─ sni.rs       peek() — 1 syscall, 512-byte stack buf → PeekInfo { sni, ja3, ja4 }
+  └─ sni.rs       peek() — MSG_PEEK ClientHello (reassembled when fragmented) → PeekInfo { sni, ja3, ja4, complete }
   └─ protection.rs check() → Block (emit 'blocked', drop) | Allow
   └─ router.rs    RouteTable.resolve(sni) → Route
   └─ [suspended.rs  register, emit 'suspended', await oneshot]
@@ -53,8 +53,10 @@ TCP accept (SO_REUSEPORT per worker thread)
 
 ## Key design decisions
 
-### MSG_PEEK for SNI/JA3
-A single `stream.peek(&mut buf[..512])` reads the ClientHello without consuming any bytes. Cost: 1 syscall, 512-byte stack buffer, zero heap allocation. This gives us both the SNI (for routing) and the JA3 fingerprint (for protection) before the TLS handshake begins. The alternative — a custom TLS acceptor that extracts SNI internally — would require modifying rustls internals.
+### MSG_PEEK for SNI/JA3/JA4
+`stream.peek()` reads the ClientHello without consuming any bytes, giving SNI (routing) and the JA3/JA4 fingerprints (protection) before the TLS handshake begins. The alternative — a custom TLS acceptor that extracts SNI internally — would require modifying rustls internals.
+
+Because a single peek returns only whatever bytes are currently buffered, `peek()` **reassembles**: it reads the declared ClientHello length from the record+handshake headers and re-peeks (into a growing buffer, bounded by `MAX_CLIENT_HELLO` and `REASSEMBLY_TIMEOUT`) until the whole hello is present, setting `PeekInfo::complete`. This closes a blocklist-bypass: without it, a client fragments the ClientHello so symphony computes a different/empty fingerprint while rustls later accepts the full handshake. `protection.rs` fails closed on `!complete` when a JA3/JA4 blocklist is configured (`BlockReason::IncompleteHandshake`).
 
 ### Source-address & fingerprint forwarding (`upstream.rs` + `proxy_conn.rs`)
 Per route, `sourceAddressHeader` picks how the client IP reaches the upstream: PROXY v1 (text),

README.md

@@ -547,7 +547,16 @@ ja4Blocklist: [
 ]
 ```
 
-Matching is case-insensitive. JA4 fingerprints are always emitted as lowercase.
+Matching is case-insensitive. JA4 fingerprints are always emitted as lowercase. A blocklist
+entry that isn't a structurally valid JA4 (or JA3) string is rejected at construction rather
+than silently installed as an entry that could never match.
+
+**Fail-closed reassembly:** when a `ja3Blocklist` or `ja4Blocklist` is configured, symphony
+reassembles the full ClientHello (bounded by size and a short timeout) before fingerprinting.
+A connection whose ClientHello can't be fully reassembled — e.g. a client that fragments it to
+expose SNI while withholding later extensions — is **blocked** (`blocked` reason
+`incomplete_handshake`) rather than allowed on a partial/empty fingerprint. Without enforcement
+configured, fingerprinting stays best-effort and an incomplete hello is not blocked.
 
 **License scope:** Symphony implements **core JA4** (TLS client fingerprinting) only. Core JA4 is BSD-licensed. The JA4+ suite of variants (JA4S, JA4H, JA4SSH, etc.) carries a separate FoxIO proprietary license and is **not implemented** here.
 

__test__/protection.spec.ts

@@ -605,3 +605,69 @@ describe('Protection – updateConfig hot-swap', () => {
 		assert.equal(info.cidrBlocklist.length, 0, 'Expected cidrBlocklist to be empty after removal');
 	});
 });
+
+describe('Protection – fingerprint blocklist validation', () => {
+	const cert = generateSelfSignedCert('localhost');
+
+	function build(protection: any): SymphonyProxy {
+		return new SymphonyProxy({
+			listeners: [{ host: '127.0.0.1', port: 0, protection }],
+			routes: [
+				{
+					sni: 'localhost',
+					upstreams: [{ kind: 'tcp', host: '127.0.0.1', port: 1 }],
+					terminateTls: true,
+					cert: { certChain: cert.cert, privateKey: cert.key },
+				},
+			],
+		});
+	}
+
+	it('rejects a malformed ja4Blocklist entry at construction', () => {
+		assert.throws(() => build({ ja4Blocklist: ['not-a-ja4'] }), /invalid ja4Blocklist entry/);
+		// Wrong length / bad separators / non-hex hash are all rejected.
+		assert.throws(() => build({ ja4Blocklist: ['t13d1516h2_8daaf6152771'] }), /invalid ja4Blocklist/);
+		assert.throws(
+			() => build({ ja4Blocklist: ['t13d1516h2_8daaf615277g_02713d6af862'] }),
+			/invalid ja4Blocklist/
+		);
+	});
+
+	it('accepts a well-formed ja4Blocklist entry', () => {
+		const proxy = build({ ja4Blocklist: ['t13d1516h2_8daaf6152771_02713d6af862'] });
+		assert.ok(proxy);
+	});
+
+	it('rejects a malformed ja3Blocklist entry at construction', () => {
+		assert.throws(() => build({ ja3Blocklist: ['deadbeef'] }), /invalid ja3Blocklist entry/);
+		assert.throws(() => build({ ja3Blocklist: ['z'.repeat(32)] }), /invalid ja3Blocklist entry/);
+	});
+
+	it('accepts a well-formed ja3Blocklist entry', () => {
+		const proxy = build({ ja3Blocklist: ['0123456789abcdef0123456789abcdef'] });
+		assert.ok(proxy);
+	});
+
+	it('accepts an uppercase ja4Blocklist entry (normalized before validation)', () => {
+		// Matching is documented as case-insensitive; validation must normalize first so a
+		// fully uppercase — but otherwise well-formed — fingerprint isn't wrongly rejected.
+		const proxy = build({ ja4Blocklist: ['T13D1516H2_8DAAF6152771_02713D6AF862'] });
+		assert.ok(proxy);
+	});
+
+	it('rejects a ja4Blocklist entry symphony could never produce', () => {
+		// symphony only computes JA4 for TLS-over-TCP ('t') at versions it actually speaks
+		// (00/10/11/12/13) — a 'q'/'d' transport prefix or an unreachable version passes the
+		// old length/charset-only validation but can never match, so it must be rejected too.
+		assert.throws(
+			() => build({ ja4Blocklist: ['q13i070500_1234567890ab_abcdef012345'] }),
+			/invalid ja4Blocklist/,
+			'QUIC prefix should be rejected',
+		);
+		assert.throws(
+			() => build({ ja4Blocklist: ['t99d1516h2_8daaf6152771_02713d6af862'] }),
+			/invalid ja4Blocklist/,
+			'unreachable version should be rejected',
+		);
+	});
+});

src/protection.rs

@@ -180,6 +180,9 @@ pub enum BlockReason {
 	CidrBlocked,
 	Ja3Blocked,
 	Ja4Blocked,
+	/// A fingerprint blocklist is configured but the ClientHello could not be fully
+	/// reassembled, so the fingerprint can't be trusted — fail closed.
+	IncompleteHandshake,
 	NoSni,
 	RateLimited,
 	TooManyConnections,
@@ -193,6 +196,7 @@ impl BlockReason {
 			Self::CidrBlocked => "cidr_blocked",
 			Self::Ja3Blocked => "ja3_blocked",
 			Self::Ja4Blocked => "ja4_blocked",
+			Self::IncompleteHandshake => "incomplete_handshake",
 			Self::NoSni => "no_sni",
 			Self::RateLimited => "rate_limited",
 			Self::TooManyConnections => "too_many_connections",
@@ -252,7 +256,16 @@ impl ProtectionState {
 			}
 		}
 
-		// 3. JA3 blocklist
+		// 3. Fingerprint enforcement. When a JA3/JA4 blocklist is configured, a ClientHello
+		// we couldn't fully reassemble must fail closed: otherwise a client fragments the
+		// hello so we compute a different/empty fingerprint here while rustls accepts the
+		// complete handshake — bypassing the blocklist.
+		let fingerprint_enforced = !cfg.ja3_blocklist.is_empty() || !cfg.ja4_blocklist.is_empty();
+		if fingerprint_enforced && !peek_info.complete {
+			return Decision::Block(BlockReason::IncompleteHandshake);
+		}
+
+		// 3a. JA3 blocklist
 		if !cfg.ja3_blocklist.is_empty() && peek_info.ja3.len() == 32 {
 			if let Some(bytes) = hex_to_bytes16(&peek_info.ja3) {
 				if cfg.ja3_blocklist.contains(&bytes) {
@@ -594,7 +607,7 @@ mod tests {
 	}
 
 	fn peek_with_ja3(hex: &str) -> PeekInfo {
-		PeekInfo { ja3: hex.to_string(), ..Default::default() }
+		PeekInfo { ja3: hex.to_string(), complete: true, ..Default::default() }
 	}
 
 	#[test]
@@ -1357,4 +1370,43 @@ mod tests {
 			"IP must be admitted fresh after re-enable; stale deadline must not resurrect"
 		);
 	}
+
+	// ── Fragmented-hello fail-closed tests ──────────────────────────────────────
+
+	fn peek(ja4: &str, complete: bool) -> PeekInfo {
+		PeekInfo { sni: Some("x".into()), ja3: String::new(), ja4: ja4.into(), complete }
+	}
+
+	fn state_with_ja4_blocklist() -> Arc<ProtectionState> {
+		let mut cfg = ProtectionConfig::default();
+		cfg.ja4_blocklist.insert("t13d1516h2_8daaf6152771_02713d6af862".into());
+		ProtectionState::new(cfg)
+	}
+
+	#[test]
+	fn incomplete_hello_fails_closed_under_fingerprint_enforcement() {
+		let state = state_with_ja4_blocklist();
+		let ip = "1.2.3.4".parse().unwrap();
+		// A truncated ClientHello whose (untrusted) fingerprint happens not to match the
+		// blocklist must still be blocked — otherwise fragmentation bypasses enforcement.
+		let decision = state.check(ip, &peek("t13d0000zz_000000000000_000000000000", false));
+		assert!(matches!(decision, Decision::Block(BlockReason::IncompleteHandshake)));
+	}
+
+	#[test]
+	fn complete_hello_not_on_blocklist_is_allowed() {
+		let state = state_with_ja4_blocklist();
+		let ip = "1.2.3.5".parse().unwrap();
+		let decision = state.check(ip, &peek("t13d1516h2_ffffffffffff_ffffffffffff", true));
+		assert!(matches!(decision, Decision::Allow(_)));
+	}
+
+	#[test]
+	fn incomplete_hello_allowed_without_fingerprint_enforcement() {
+		// No blocklist configured: reassembly is best-effort only, incomplete is fine.
+		let state = ProtectionState::new(ProtectionConfig::default());
+		let ip = "1.2.3.6".parse().unwrap();
+		let decision = state.check(ip, &peek("", false));
+		assert!(matches!(decision, Decision::Allow(_)));
+	}
 }

src/proxy.rs

@@ -786,16 +786,30 @@ fn parse_protection_config(
 
 	if let Some(ja3s) = &prot.ja3_blocklist {
 		for hex in ja3s {
-			if let Some(bytes) = hex_to_bytes16(hex) {
-				cfg.ja3_blocklist.insert(bytes);
-			}
+			// A JA3 is an MD5 digest — exactly 32 hex chars. Reject typos rather than
+			// silently installing an entry that can never match.
+			let bytes = hex_to_bytes16(hex).ok_or_else(|| {
+				napi::Error::from_reason(format!(
+					"invalid ja3Blocklist entry '{hex}': expected 32 hex characters"
+				))
+			})?;
+			cfg.ja3_blocklist.insert(bytes);
 		}
 	}
 
 	if let Some(ja4s) = &prot.ja4_blocklist {
 		for s in ja4s {
-			// Normalize to lowercase for case-insensitive matching.
-			cfg.ja4_blocklist.insert(s.to_lowercase().into_boxed_str());
+			// Normalize to lowercase *before* validating — matching is case-insensitive, so an
+			// otherwise-well-formed uppercase fingerprint must not be rejected here.
+			let lower = s.to_lowercase();
+			// Validate against what `compute_ja4` can actually produce, so a malformed or
+			// unreachable entry fails at config time instead of silently never matching.
+			if !is_valid_ja4(&lower) {
+				return Err(napi::Error::from_reason(format!(
+					"invalid ja4Blocklist entry '{s}': expected JA4 format t<ver><sni><cc><ec><alpn>_<12 hex>_<12 hex> (36 chars)"
+				)));
+			}
+			cfg.ja4_blocklist.insert(lower.into_boxed_str());
 		}
 	}
 
@@ -881,6 +895,34 @@ fn num_cpus() -> u32 {
 		.unwrap_or(4)
 }
 
+/// Validate a JA4 core-TLS fingerprint string: `t<ver2><sni1><cc2><ec2><alpn2>_<12hex>_<12hex>`
+/// (36 chars). Expects lowercase input (call sites normalize before validating). Restricted to
+/// exactly what `sni::compute_ja4` can produce — not the full JA4 spec — since a blocklist entry
+/// symphony itself can never emit (a `q`/`d` transport prefix, or a TLS version it doesn't speak)
+/// would pass validation yet can never match, defeating the point of validating it at all.
+fn is_valid_ja4(s: &str) -> bool {
+	let b = s.as_bytes();
+	if b.len() != 36 || b[10] != b'_' || b[23] != b'_' {
+		return false;
+	}
+	let a = &b[..10];
+	// protocol: symphony only speaks TLS-over-TCP, so only 't' (never 'q'=QUIC, 'd'=DTLS).
+	// tls version: only the 2-digit strings compute_ja4's ver_str match can emit.
+	// sni presence (d=domain/i=ip), cipher count (2 digits), extension count (2 digits),
+	// alpn first/last (2 alphanumeric).
+	a[0] == b't'
+		&& matches!((a[1], a[2]), (b'0', b'0') | (b'1', b'0') | (b'1', b'1') | (b'1', b'2') | (b'1', b'3'))
+		&& matches!(a[3], b'd' | b'i')
+		&& a[4].is_ascii_digit()
+		&& a[5].is_ascii_digit()
+		&& a[6].is_ascii_digit()
+		&& a[7].is_ascii_digit()
+		&& a[8].is_ascii_alphanumeric()
+		&& a[9].is_ascii_alphanumeric()
+		&& b[11..23].iter().all(u8::is_ascii_hexdigit)
+		&& b[24..36].iter().all(u8::is_ascii_hexdigit)
+}
+
 fn hex_to_bytes16(hex: &str) -> Option<[u8; 16]> {
 	if hex.len() != 32 {
 		return None;
@@ -1010,4 +1052,43 @@ mod tests {
 		};
 		assert!(parse_protection_config(&prot).is_ok(), "absent burst must be accepted");
 	}
+
+	#[test]
+	fn valid_ja4_accepts_well_formed() {
+		assert!(is_valid_ja4("t13d1516h2_8daaf6152771_02713d6af862"));
+		assert!(is_valid_ja4("t00i070500_1234567890ab_abcdef012345")); // version 00, no-SNI variant
+		for ver in ["00", "10", "11", "12", "13"] {
+			let s = format!("t{ver}d1516h2_8daaf6152771_02713d6af862");
+			assert!(is_valid_ja4(&s), "version {ver} should be accepted: {s}");
+		}
+	}
+
+	#[test]
+	fn valid_ja4_rejects_malformed() {
+		assert!(!is_valid_ja4(""), "empty");
+		assert!(!is_valid_ja4("t13d1516h2_8daaf6152771"), "missing part C");
+		assert!(!is_valid_ja4("t13d1516h2_8daaf6152771_02713d6af86"), "part C too short");
+		assert!(!is_valid_ja4("t13d1516h2_8daaf6152771_02713d6af862x"), "too long");
+		assert!(!is_valid_ja4("t13d1516h2-8daaf6152771-02713d6af862"), "wrong separators");
+		assert!(!is_valid_ja4("t13d1516h2_8daaf615277g_02713d6af862"), "non-hex in part B");
+		assert!(!is_valid_ja4("x13d1516h2_8daaf6152771_02713d6af862"), "bad protocol char");
+		assert!(!is_valid_ja4("t1xd1516h2_8daaf6152771_02713d6af862"), "non-digit version");
+		// Uppercase must be rejected here — call sites are expected to normalize before
+		// validating; is_valid_ja4 itself only matches the lowercase output compute_ja4 emits.
+		assert!(!is_valid_ja4("T13D1516H2_8DAAF6152771_02713D6AF862"), "uppercase");
+	}
+
+	#[test]
+	fn valid_ja4_rejects_transports_and_versions_symphony_cannot_emit() {
+		// symphony only speaks TLS-over-TCP: 'q' (QUIC) and 'd' (DTLS) transport prefixes can
+		// never match a fingerprint symphony itself computes.
+		assert!(!is_valid_ja4("q13i070500_1234567890ab_abcdef012345"), "QUIC prefix");
+		assert!(!is_valid_ja4("d13i070500_1234567890ab_abcdef012345"), "DTLS prefix");
+		// compute_ja4's ver_str only ever emits 00/10/11/12/13 — any other 2-digit value can
+		// never match either.
+		for ver in ["01", "02", "14", "20", "99"] {
+			let s = format!("t{ver}d1516h2_8daaf6152771_02713d6af862");
+			assert!(!is_valid_ja4(&s), "version {ver} should be rejected (unreachable): {s}");
+		}
+	}
 }

src/proxy_conn.rs

@@ -60,14 +60,28 @@ pub async fn handle(stream: TcpStream, peer_addr: SocketAddr, ctx: Arc<ConnConte
 	// stream is consumed by the TLS handshake.
 	let local_addr = stream.local_addr().ok();
 
+	// ── 0. Admission: count this connection from acceptance, for its whole lifetime ────
+	// `maxConnections` is enforced at accept() against this same global counter (listener.rs).
+	// If we only counted a connection as active *after* peek()/protection::check() returned,
+	// the up-to-`REASSEMBLY_TIMEOUT` (5s) peek phase would run uncounted — a flood of slow or
+	// invalid (non-TLS, stalled) ClientHellos could hold open far more than maxConnections
+	// pending tasks/fds at once. Guard from the top so peek is admission-controlled too.
+	ctx.listener_metrics.inc_active();
+	ctx.global_metrics.inc_active();
+	let mut active_guard = ActiveGuard {
+		global: ctx.global_metrics.clone(),
+		listener: ctx.listener_metrics.clone(),
+		ip_state: None,
+	};
+
 	// ── 1. Peek: extract SNI + JA3 ───────────────────────────────────────────
 	let peek_info = sni::peek(&stream).await;
 
 	// ── 2. Protection checks ─────────────────────────────────────────────────
 	// On Allow, check() returns the Arc<IpState> that incremented the active counter.
-	// We hold it in ip_state_held so the ActiveGuard can decrement the SAME entry even
-	// if eviction removes it from the map between admission and connection close (fix 6).
-	let ip_state_held: Option<Arc<IpState>> = if let Some(protection) = &ctx.protection {
+	// We hold it on active_guard so it decrements the SAME entry even if eviction removes
+	// it from the map between admission and connection close (fix 6).
+	if let Some(protection) = &ctx.protection {
 		match protection.check(peer_ip, &peek_info) {
 			crate::protection::Decision::Block(reason) => {
 				ctx.listener_metrics.inc_blocked();
@@ -79,28 +93,16 @@ pub async fn handle(stream: TcpStream, peer_addr: SocketAddr, ctx: Arc<ConnConte
 					ja3: peek_info.ja3.clone(),
 					ja4: peek_info.ja4.clone(),
 				});
-				return;
+				return; // active_guard drop decrements the active counters it already bumped
 			}
 			// Allowlisted: active counter was not incremented.
-			crate::protection::Decision::AllowBypassed => None,
-			crate::protection::Decision::Allow(state) => Some(state),
+			crate::protection::Decision::AllowBypassed => {}
+			crate::protection::Decision::Allow(state) => active_guard.ip_state = Some(state),
 		}
-	} else {
-		None
-	};
-
-	// ── Connection is allowed — track it ─────────────────────────────────────
-	ctx.listener_metrics.inc_active();
-	ctx.global_metrics.inc_active();
+	}
 
 	// RAII: decrement counts on scope exit.
-	// Decrement the active counter through the held Arc<IpState> so it always hits the
-	// same entry that was incremented, regardless of concurrent eviction.
-	let _active_guard = ActiveGuard {
-		global: ctx.global_metrics.clone(),
-		listener: ctx.listener_metrics.clone(),
-		ip_state: ip_state_held,
-	};
+	let _active_guard = active_guard;
 
 	// ── 3. Route lookup ───────────────────────────────────────────────────────
 	let table = ctx.route_table.0.load();