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

[kriszyp]

Follow-up on #20, addressing @hdbjeff's review comments (1, 2) that were still open when it merged. Four findings:

🔒 Medium (security) — fragmented ClientHello bypassed the fingerprint blocklists

A single stream.peek() returns only the currently-buffered bytes, and the parser tolerated truncation. A client could send just enough ClientHello to expose its SNI, stall, and make symphony compute a different or empty JA3/JA4 — while rustls later read and accepted the complete handshake, slipping past ja3Blocklist/ja4Blocklist.

peek() now reassembles the full declared ClientHello (reads the record+handshake length, re-peeks into a growing buffer, bounded by MAX_CLIENT_HELLO = 16 KB and a 5 s REASSEMBLY_TIMEOUT) and reports PeekInfo.complete. protection.rs fails closed — BlockReason::IncompleteHandshake — when a fingerprint blocklist is configured and the hello couldn't be fully reassembled. Without enforcement configured, fingerprinting stays best-effort and an incomplete hello isn't blocked.

Medium — invalid blocklist entries silently accepted

ja4Blocklist just lowercased arbitrary strings; a typo started fine but could never match. Now validated (is_valid_ja4) at config parse — a malformed entry is a construction error naming the offender. Applied the same to ja3Blocklist (32-hex) for parity.

Low — GREASE signature algorithms produced noncanonical JA4

Sig-algs were added to JA4_c without is_grease filtering (ciphers/extensions already filter). A GREASE sig value shifted the fingerprint away from standard JA4 tooling. Now filtered.

Perf — hot-path allocation churn in compute_ja4

.map(|v| format!("{v:04x}")).collect::<Vec<_>>().join(",") ran per-token on the peek hot path (every connection). Replaced with a single write!-into-preallocated-String helper (join_hex4) for the cipher/extension/sig-alg lists.

Where to look

• Fail-closed semantics (protection.rs): the IncompleteHandshake block only triggers when ja3_blocklist/ja4_blocklist is non-empty — a plain proxy with no fingerprint enforcement never drops a slow/partial hello on this path.
• Reassembly bound/timeout (sni.rs): MSG_PEEK doesn't consume, so socket readiness stays asserted; the loop paces with a 10 ms poll and is capped by the 5 s outer timeout. A multi-TLS-record-spanning hello that we can't fully frame parses as incomplete → fail-closed under enforcement.

Testing

• Rust (52): real-socket reassembly (fragmented→complete, stalled-partial→!complete via injectable timeout), fail-closed decision matrix, GREASE sig-alg JA4 parity, declared_client_hello_len, join_hex4, is_valid_ja4.
• Node (55): ja3Blocklist/ja4Blocklist malformed-entry rejection + well-formed acceptance.
• Full suite green on Linux.

Note

Pre-existing clippy::all violations in http_listener.rs/http_proxy.rs (empty line after doc comment) are on main already and untouched here; symphony CI doesn't gate clippy. Happy to sweep them separately.

Generated by an LLM (Claude Fable 5) pairing with Kris.

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request introduces ClientHello reassembly to prevent blocklist bypasses via fragmentation, adds fail-closed logic for incomplete handshakes when fingerprint blocklists are active, and implements validation for JA3/JA4 blocklist entries at configuration time. It also optimizes JA4 fingerprint generation and ignores GREASE signature algorithms. The review feedback correctly identifies a potential Denial of Service (DoS) vulnerability where non-TLS or non-ClientHello connections can cause CPU-intensive polling during reassembly, and suggests failing fast. Additionally, it recommends validating the handshake message type in declared_client_hello_len to avoid processing incorrect handshake lengths.

[hdbjeff]

Request changes. The GREASE filtering, allocation cleanup, JA3 validation, and fail-closed decision itself look correct, but the original fragmentation bypass is not fully closed and the reassembler introduces a new pre-protection DoS path.

High — Invalid traffic causes five seconds of unbounded pre-protection polling

For plain HTTP, a non-ClientHello handshake record, or another malformed prefix, declared_client_hello_len returns None. peek_reassemble then grows its buffer to 16 KiB and performs a ready MSG_PEEK approximately every 10 ms until the
five-second timeout.

This runs before protection checks and before the active-connection metric is incremented. As a result, maxConnections does not bound these pending tasks. An unauthenticated client can create large numbers of one-byte connections, consuming
file descriptors, task memory, and roughly 500 polls per connection.

Please:

• Fail immediately once byte 0 proves the input is not a TLS handshake record.
• Fail immediately once byte 5 proves the handshake is not a ClientHello.
• Include pre-peek connections in admission accounting, or use a semaphore covering the entire handler.
• Add tests proving non-TLS and non-ClientHello inputs return promptly.

This also means the two existing Gemini comments are valid. Checking the message type only inside declared_client_hello_len is insufficient unless the caller distinguishes invalid input from “need more bytes” and stops polling.

Medium — TLS-record fragmentation still bypasses JA3/JA4 enforcement

The implementation handles one TLS record divided across TCP writes, but not a ClientHello divided across multiple TLS records.

declared_client_hello_len calculates the required wire length as:

5-byte record header + 4-byte handshake header + handshake length

That omits the additional five-byte header for each continuation TLS record. peek_reassemble can therefore reach the calculated length and set complete=true while the final ClientHello payload bytes are still missing. The parser also
treats the continuation-record header as ClientHello body or extension data because it parses the raw TCP buffer as one record.

This produces a different or empty JA3/JA4, but the fail-closed check is skipped because complete is true. Rustls can subsequently reassemble and accept the valid multi-record handshake, preserving the original blocklist bypass.

Please reassemble TLS record payloads until the complete declared handshake payload is available, stripping each record header before parsing. Add a regression test that constructs one ClientHello across at least two TLS handshake records and
verifies that its known blocklist entry is enforced.

The current peek_reassembles_fragmented_client_hello test only divides one TLS record across two TCP writes, so it does not cover this case.

Low — JA4 validation still accepts entries that cannot match

is_valid_ja4 accepts q and d protocol prefixes and arbitrary two-digit versions. Symphony’s implementation always emits t and only versions 00, 10, 11, 12, or 13. Such entries pass configuration but can never match, which is
the failure mode the validation change was intended to prevent.

Validation also occurs before lowercase normalization, so a fully uppercase fingerprint can be rejected despite the documented case-insensitive behavior.

Please normalize before validation and validate against the values compute_ja4 can actually produce. Tests should cover:

• Fully uppercase input and successful matching after normalization.
• Rejection of q/d until those transports are supported.
• Rejection of versions Symphony cannot emit.

[hdbjeff]

Request changes. The GREASE filtering, allocation cleanup, JA3 validation, and fail-closed decision itself look correct, but the original fragmentation bypass is not fully closed and the reassembler introduces a new pre-protection DoS path.

[hdbjeff]

Proposed Fixes for Open Review Findings on PR #28

Below are the complete, production-ready Rust implementations to fully address the remaining High, Medium, and Low findings on your review.

---

1. High — Pre-Protection Admission Accounting (src/proxy_conn.rs)

Because peeking can run for up to 5 seconds before active metrics are incremented, a client can bypass the maxConnections limit by opening thousands of slow connections.

We should track the connection as active immediately upon entering handle(). If the connection is subsequently blocked, the ActiveGuard will naturally decrement the counters on drop. If it is allowed, we associate the protection state for release.

// Proposed inline replacement in src/proxy_conn.rs

pub async fn handle(stream: TcpStream, peer_addr: SocketAddr, ctx: Arc<ConnContext>) {
	let peer_ip = peer_addr.ip();
	let local_addr = stream.local_addr().ok();

	// Track connection as active immediately during peeking/protection checks
	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(),
		protection: None,
		peer_ip,
	};

	// ── 1. Peek: extract SNI + JA3 ───────────────────────────────────────────
	let peek_info = sni::peek(&stream).await;

	// ── 2. Protection checks ─────────────────────────────────────────────────
	if let Some(protection) = &ctx.protection {
		match protection.check(peer_ip, &peek_info) {
			crate::protection::Decision::Block(reason) => {
				ctx.listener_metrics.inc_blocked();
				ctx.global_metrics.inc_blocked();
				emit(&ctx.js_emit, JsEvent::Blocked {
					ip: peer_ip.to_string(),
					reason: reason.as_str().to_string(),
					listener: ctx.listener_addr.clone(),
					ja3: peek_info.ja3.clone(),
					ja4: peek_info.ja4.clone(),
				});
				return; // active_guard drops here, automatically decrementing active connections!
			}
			crate::protection::Decision::AllowBypassed => {}
			crate::protection::Decision::Allow => {
				active_guard.protection = Some(protection.clone());
			}
		}
	}

	// ── 3. Route lookup ───────────────────────────────────────────────────────
	let table = ctx.route_table.0.load();
	let sni_str = peek_info.sni.as_deref();
	let route = match table.resolve(sni_str) {
		Some(r) => r.clone(),
		None => {
			ctx.listener_metrics.inc_error();
			return; // active_guard drops and decrements active connections
		}
	};
    // ... (rest of route/upstreams logic remains unchanged, remove the old active count increment block below)

---

2. Medium — Multi-Record TLS Fragmentation Reassembly (src/sni.rs)

The current reassembly logic fails if a ClientHello is fragmented across multiple TLS records (each with its own 5-byte header). The parser will treat continuation record headers as payload, corrupting the fingerprint and skipping the blocklist.

We should parse successive record headers, strip them, and reassemble the raw handshake payload. If complete, we wrap it in a single synthetic TLS record so that parse_client_hello continues to work seamlessly.

// 1. Add this helper function at the bottom of src/sni.rs

/// Parses consecutive TLS Handshake records and attempts to reassemble
/// the underlying Handshake payload.
///
/// Returns `Some((synthetic_record, complete))` where `complete` indicates whether
/// the complete declared Handshake message has been reassembled.
fn reassemble_handshake(buf: &[u8]) -> Option<(Vec<u8>, bool)> {
	if buf.len() < 5 {
		return None; // need more bytes to read first record header
	}
	if buf[0] != 0x16 {
		return Some((Vec::new(), false)); // invalid record type (will fail fast)
	}

	let mut pos = 0;
	let mut handshake_payload = Vec::new();
	let mut declared_handshake_len = None;

	while pos < buf.len() {
		if pos + 5 > buf.len() {
			// Incomplete record header — need more bytes from the stream
			return Some((handshake_payload, false));
		}
		let record_type = buf[pos];
		if record_type != 0x16 {
			// Not a handshake record anymore; corrupt or invalid
			return Some((handshake_payload, false));
		}
		let record_len = ((buf[pos + 3] as usize) << 8) | (buf[pos + 4] as usize);
		pos += 5;

		let payload_end = pos + record_len;
		if payload_end > buf.len() {
			// Current record payload is truncated in the peek buffer — need more bytes
			let available = buf.len() - pos;
			handshake_payload.extend_from_slice(&buf[pos..pos + available]);
			return Some((handshake_payload, false));
		}

		let record_payload = &buf[pos..payload_end];
		pos = payload_end;

		// Extract handshake header from the first record's payload
		if declared_handshake_len.is_none() {
			if record_payload.len() < 4 {
				// Need more bytes to parse handshake header
				return Some((handshake_payload, false));
			}
			let hs_type = record_payload[0];
			if hs_type != 0x01 {
				// Not a ClientHello
				return Some((handshake_payload, false));
			}
			let hs_len = ((record_payload[1] as usize) << 16)
				| ((record_payload[2] as usize) << 8)
				| (record_payload[3] as usize);
			declared_handshake_len = Some(hs_len);
		}

		handshake_payload.extend_from_slice(record_payload);

		if let Some(target) = declared_handshake_len {
			let needed = target + 4; // 4 bytes handshake header
			if handshake_payload.len() >= needed {
				// We have fully reassembled the handshake payload!
				// Construct a single synthetic TLS record wrapping this reassembled handshake message
				let final_payload = &handshake_payload[..needed];
				let mut synthetic_record = vec![0x16, 0x03, 0x03]; // Handshake, TLS 1.2 legacy version
				let len_bytes = (final_payload.len() as u16).to_be_bytes();
				synthetic_record.extend_from_slice(&len_bytes);
				synthetic_record.extend_from_slice(final_payload);
				return Some((synthetic_record, true));
			}
		}
	}

	Some((handshake_payload, false))
}

// 2. Update `peek_reassemble` in src/sni.rs to utilize this helper:

async fn peek_reassemble(stream: &TcpStream) -> Option<(Vec<u8>, bool)> {
	let mut buf = vec![0u8; INITIAL_PEEK];
	loop {
		let n = match stream.peek(&mut buf).await {
			Ok(0) => return None, // peer closed before sending a ClientHello
			Ok(n) => n,
			Err(_) => return None,
		};
		// Fail fast if the data is clearly not a TLS ClientHello
		if n >= 1 && buf[0] != 0x16 {
			return Some((buf[..n].to_vec(), false));
		}
		if n >= 6 && buf[5] != 0x01 {
			return Some((buf[..n].to_vec(), false));
		}

		// Reassemble the TLS records
		if let Some((reassembled, complete)) = reassemble_handshake(&buf[..n]) {
			if complete {
				return Some((reassembled, true));
			}
			if reassembled.len() > MAX_CLIENT_HELLO {
				// Exceeds maximum size cap
				return Some((reassembled, false));
			}
		}

		if n >= MAX_CLIENT_HELLO {
			return Some((buf[..n].to_vec(), false));
		}
		if buf.len() < MAX_CLIENT_HELLO {
			buf.resize((buf.len() * 2).min(MAX_CLIENT_HELLO), 0);
		}
		tokio::time::sleep(PEEK_POLL_INTERVAL).await;
	}
}

---

3. Low — Hardened JA4 Validation (src/proxy.rs)

Symphony only supports TCP and emits `t` with versions `00`, `10`, `11`, `12`, or `13`. Additionally, validation occurs before lowercase normalization, rejecting valid uppercase strings. > > We should restrict the protocol/version matches and normalize to lowercase prior to validating.

// Proposed inline replacement in src/proxy.rs

/// Validate a JA4 core-TLS fingerprint string: `t<ver2><sni1><cc2><ec2><alpn2>_<12hex>_<12hex>`
/// (36 chars), matching what `sni::compute_ja4` emits.
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 must be 't' (TCP)
	let protocol_ok = a[0] == b't';
	
	// Version must be "00", "10", "11", "12", or "13"
	let version = &a[1..3];
	let version_ok = matches!(version, b"00" | b"10" | b"11" | b"12" | b"13");

	protocol_ok
		&& version_ok
		&& 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)
}

// And update the blocklist parsing to normalize first:

	if let Some(ja4s) = &prot.ja4_blocklist {
		for s in ja4s {
			let normalized = s.to_lowercase();
			// Validate normalized representation
			if !is_valid_ja4(&normalized) {
				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(normalized.into_boxed_str());
		}
	}

[kriszyp]

Thanks for the thorough second pass — all three findings are addressed in ef729c1:

High (pre-protection DoS polling): peek_reassemble now fails fast — as soon as the buffered prefix proves it can never become a ClientHello (wrong record content-type or handshake msg_type), it returns immediately instead of polling for up to 5s. Verified with two tests asserting non-TLS and non-ClientHello input return in well under a second.

I went with a slightly different fix for the admission-accounting half: rather than a separate semaphore, handle() now counts the connection toward maxConnections/activeConnections from acceptance — before peek() runs — so the peek/reassembly phase itself is admission-controlled by the existing counter instead of running uncounted.

Medium (multi-record fragmentation bypass): this was the real gap — the old code assumed a ClientHello always fits in one TLS record. sni.rs now has scan_handshake_records, which walks however many 0x16 Handshake records are buffered, strips each record's own 5-byte header, and concatenates the handshake-layer payload before parsing. Added a regression test (peek_reassembles_client_hello_split_across_multiple_tls_records) that splits a hello across two real TLS records over an actual socket and asserts the JA4 matches the single-record encoding exactly — the case your review called out as untested.

Low (JA4 validation): is_valid_ja4 now only accepts what compute_ja4 can actually emit — transport t only (no q/d), versions 00/10/11/12/13 only — and validation happens after lowercasing, so a well-formed uppercase entry is accepted while a q13.../t99... entry that could never match is rejected.

Full Rust (60) + Node (57) suites green.

🤖 Generated with Claude Code — Claude Sonnet 5