PROXY protocol v2 + downstream JA3/JA4 fingerprint forwarding

[kriszyp]

Closes #9. Adds the two edge-proxy capabilities that issue tracks, bundled because PROXY v2's TLV is the natural carrier for the forwarded fingerprint.

Stacked on #20 (kris/ja4-12) so this PR's diff is just my two commits — it carries JA3 and JA4 (per direction to ship both together). GitHub will auto-retarget to main when #20 merges.

What it does

1. PROXY protocol v2 emit — a binary v2 header encoder (write_proxy_v2_header) beside the v1 text one, selected per route via sourceAddressHeader: 'proxyProtocolV2'. v2 adds the TLV section used below. Stays opt-in; UDS default remains v1 (Harper core's own UDS reader parses v1 only).
2. Forward the fingerprint — forwardFingerprint: 'ja3' | 'ja4' | 'none' hands symphony's computed ClientHello fingerprint to the upstream. Carrier follows the mode: a PROXY v2 TLV (0xE0=JA3 / 0xE1=JA4, HAProxy private range) under proxyProtocolV2 — works in passthrough since it prefixes the raw bytes — otherwise an injected X-JA3/X-JA4 header for terminated HTTP/1 upstreams. Motivating use case: Automattic wants the edge fingerprint passed through to their own app servers behind us.

Where to look

• src/upstream.rs — write_proxy_v2_header (byte layout: 12-byte sig + 0x21 + family/proto + BE length + address block + TLV). Cross-model review verified it against the HAProxy 2.8 spec; unit tests cover IPv4+TLV, empty-TLV skip, IPv4-mapped-IPv6 collapse, IPv6.
• src/proxy_conn.rs — apply_source_header gates all HTTP-header injection on l7_http1 (terminated TLS and non-h2 ALPN), so text is never spliced into passthrough TLS ciphertext or an HTTP/2 frame stream — a no-op otherwise. inject_request_headers strips any client-supplied copy of an injected header (anti-spoof, so a client can't pre-seed X-JA3).

Notes for the reviewer

• Bundled behavior change (intentional, kept in this PR): toJsRoute/resolveConnection in ts/proxy.ts never forwarded the http2 field — it was declared in types and parsed in Rust but silently dropped by the JS wrapper, so http2: true was a no-op on that path. Fixed here (it's adjacent, and it's what makes the h2 carrier path reachable to test). Note the effect: any existing route carrying http2: true will now genuinely negotiate h2. Confirmed with Kris that it stays bundled here rather than splitting out.
• Cross-model review caveat: ran Codex + the Harper-domain pass; all three findings they surfaced (passthrough/h2 stream corruption, fingerprint spoofing) are fixed in commit 2 with tests. The Gemini/agy leg hung both attempts, so there's no second-model perf/maintainability pass this run.

Tests

Rust unit tests for the v2 encoder + header strip; integration tests for v2-TLV forwarding, no-TLV v2, X-JA3 injection, passthrough no-op, and h2 negotiation + no-injection. Full suite green (Rust 22, Node 39).

Generated by an LLM (Claude Opus 4.8).

[gemini-code-assist]

Code Review

This pull request introduces support for PROXY protocol v2 and downstream TLS fingerprint forwarding (JA3/JA4). Fingerprints are forwarded either via a PROXY v2 TLV or as injected HTTP headers (X-JA3/X-JA4) for plaintext HTTP/1 upstreams, with client-supplied headers stripped to prevent spoofing. Comprehensive integration tests and documentation updates are also included. Feedback is provided regarding a potential silent truncation issue in write_proxy_v2_header when casting the TLV length to u16, where using try_into() with proper error handling is recommended to ensure safety.

[kriszyp]

CI status: all 5 platform builds + every unit-test job (ubuntu Node 20/22, macOS-14 Node 20) are green. Two checks are red/cancelled for reasons unrelated to this change:

• Integration Test – Harper UDS proxy — pre-existing red on the base branch: the harper dev-dependency's own sources (analytics/profile.ts) can't resolve their ../core/* submodule paths (TS2307). Not symphony code, not a regression here.
• Test – macos-13 Node 20 — cancelled; that runner image is retired (removed by Fix CI: check out core submodule, drop retired macos-13 runner #18).

Nothing to action on those two. — KrAIs (Claude Opus 4.8)

[DavidCockerill]

Nice work on this — the v2 encoder is bounds-safe (u16 lengths guarded with try_into → io::Error, no truncation/panic/unwrap), header injection is correctly gated to plaintext HTTP/1, and it quietly fixes a pre-existing XFF-in-passthrough corruption along the way. Since this is PROXY v2 emission (not inbound parsing) and the emitted IP comes from the kernel peer_addr, the classic "client forges a PROXY header to spoof their source IP" vector doesn't apply here — good.

One significant gap worth closing, though, because it undercuts the PR's own "authoritative, can't be spoofed" claim for the fingerprint.

In plain terms: the backend is meant to trust X-JA3/X-JA4 as "symphony measured this," so symphony has to guarantee a client can't set that header themselves. It does strip client-supplied copies — but only within the first 8 KB it reads. If a client makes its request head bigger than 8 KB (trivial: long cookies, extra headers, padding) and puts its own X-JA3: line past byte 8192, the strip never sees it and it passes through verbatim. The backend then gets two fingerprint headers — symphony's real one and the client's forged one. On a last-header-wins backend that's a full spoof; on a comma-joining backend (Node's http) it's a corrupted value that can slip past a JA3/JA4 blocklist match. It's a detection/fraud signal rather than an auth boundary, and forwarding is opt-in, so I'm flagging rather than blocking — but it does defeat the guarantee the PR advertises.

Details / where: inject_request_headers (upstream.rs:220, strip loop :248) reads a single 8192-byte chunk; when the head exceeds it, the "No terminating CRLF in this chunk" branch forwards the remainder verbatim through the bidirectional copy.

Fix: strip X-JA3/X-JA4 (and X-Forwarded-For) across the entire header block, not just the first buffered read — or reject/normalize a request whose header block exceeds the scan window rather than letting the unscanned tail through.

Smaller, non-blocking:

• No runtime guard against proxyProtocolV2 being configured on a Harper-core UDS route (core's UDS reader is v1-only). The docs warn about it, but a hard error (or the existing eprintln! warning) would beat doc-only.
• Perf: the rewritten head is emitted as several small writes — coalescing into one buffer + a single write_all would be cleaner and tighter under TCP_NODELAY.
• v2 destination family-mismatch falls back to loopback:0 — benign (backends consume the source), noting for completeness.

— DAIvid (Claude Opus 4.8)

[hdbjeff]

High — HTTP fingerprint headers remain client-spoofable

Inline on src/upstream.rs, around line 220:

High — HTTP fingerprint headers remain client-spoofable

inject_request_headers() rewrites only one arbitrary 8 KiB read. The caller then switches to unrestricted bidirectional copying, so client-supplied X-JA3, X-JA4, or X-Forwarded-For values can survive by
arriving in a later TCP segment, crossing the 8 KiB boundary, or appearing on a subsequent HTTP/1.1 keep-alive request.

Later legitimate requests also receive no forwarded metadata. If the first read contains only part of the request line, the fallback prepends headers before the request line and corrupts the request.

Please parse and rewrite a complete, bounded header block for every HTTP/1 request, including fragmented and pipelined requests. Otherwise, trusted fingerprint forwarding should be restricted to a connection-
scoped carrier such as PROXY v2.

Medium — Missing fingerprint permits direct spoofed-header pass-through

Inline on src/proxy_conn.rs, around line 304:

Medium — Missing fingerprint permits direct spoofed-header pass-through

When fingerprint parsing fails or produces an empty value, fingerprint_header() returns None and header rewriting is skipped entirely. That leaves a client-supplied X-JA3 or X-JA4 untouched precisely when
Symphony has no authoritative value to replace it.

Configuring forwardFingerprint should always strip the corresponding inbound header. Forwarding no replacement value afterward is reasonable; passing through the client’s value is not.

Medium — Ineffective forwarding configurations deploy successfully

Inline on src/proxy.rs, around line 542:

Medium — Ineffective forwarding configurations deploy successfully

Configurations without a viable fingerprint carrier are accepted even though runtime silently discards the requested signal. An stderr warning is easy to miss and leaves the deployment apparently healthy while
the downstream receives no fingerprint.

Please reject invalid combinations and apply the same validation to static and resolved routes. HTTP-header forwarding should require terminated HTTP/1; passthrough and HTTP/2 should require PROXY v2.

Medium — Private PROXY v2 TLVs lack an end-to-end consumer contract

Inline on src/upstream.rs, around PP2_TYPE_JA3 and PP2_TYPE_JA4:

Medium — Private PROXY v2 TLVs lack an end-to-end consumer contract

Generic PROXY v2 support does not mean a downstream proxy understands or preserves private TLV types 0xE0 and 0xE1. Without an explicit consumer, these fingerprints may be silently discarded at the next hop.

Please document the expected immediate consumer, trust boundary, payload encoding, and behavior through intermediary hops. An integration test against the intended consumer would provide stronger assurance than
testing only Symphony’s encoder.

Review summary

Request changes. The PROXY v2 encoder appears structurally sound, but the HTTP carrier is not authoritative because rewriting covers only one read and one request. Empty fingerprint handling also permits
spoofed-header pass-through, while ineffective carrier configurations can deploy successfully. These trust-boundary issues should be resolved before downstream security controls rely on the forwarded signal.

[hdbjeff]

Comment 1: Un-timeouted Client Read leads to Connection Exhaustion (Slowloris DoS)

• Target File: src/upstream.rs
• Suggested Position: Around line 218, near let n = client.read(&mut buf).await?; in inject_request_headers

1 ### 🔴 Critical Security/Reliability: Un-timeouted Client Read leads to Connection Exhaustion (Slowloris DoS)
2
3 The asynchronous read on the client socket to pull the first HTTP/1 payload chunk has no timeout, and is called outside of the connection's copy_with_idle_timeout loop:
let mut buf = [0u8; 4096];
let n = client.read(&mut buf).await?; // AWAIT without timeout!

1
2 #### Why it matters
3 A slow or malicious client can complete the TLS handshake (which is timeout-protected in handle), and then stall indefinitely without sending any HTTP data. The thread/connection will block forever on
client.read().await. Unauthenticated remote attackers can easily exploit this to perform a Slowloris-style DoS attack, exhausting the proxy's open file descriptors or worker connections with zero bandwidth
footprint.
4
5 #### Suggested Fix
6 Wrap the read call in tokio::time::timeout using a fast, reasonable threshold (e.g., 5 seconds) or tie it to the route's configured idle_timeout:
use tokio::time::timeout;

let mut buf = [0u8; 4096];
let read_timeout = std::time::Duration::from_secs(5); // or pull from ctx.idle_timeout
let n = match timeout(read_timeout, client.read(&mut buf)).await {
Ok(Ok(bytes)) => bytes,
Ok(Err(e)) => return Err(e),
Err(_) => return Err(std::io::Error::new(std::io::ErrorKind::TimedOut, "HTTP request header read timeout")),
};
if n == 0 {
return Ok(());
}
1

---

Comment 2: Header Spoofing / Strip Bypass via TCP Fragmentation

• Target File: src/upstream.rs
• Suggested Position: Around line 224, near let Some(rl) = find_crlf(data) else { ... } or the find_crlf loop in inject_request_headers

1 ### ⚠️ Security: Header Spoofing / Strip Bypass via TCP Fragmentation
2
3 inject_request_headers performs a single blind read from the client socket. If the read returns a chunk of data that does not contain a complete set of HTTP headers (due to TCP packet fragmentation or
slow sending), the stripping loop exits prematurely when it encounters a line without a trailing CRLF:
let Some(rel) = find_crlf(&rest[i..]) else {
// No terminating CRLF in this chunk (truncated headers or bodyless) — pass verbatim.
upstream.write_all(&rest[i..]).await?;
break;
};

1
2 #### Why it matters
3 An attacker can intentionally fragment their TCP segments so that their spoofed X-Forwarded-For: <target_ip> or X-JA3: <safe_fingerprint> header spans across a packet boundary (e.g. Packet 1 ends with
X-Forwa, Packet 2 begins with rded-For: 127.0.0.1).
4
5 The proxy's single-pass reader will process Packet 1, fail to find a CRLF, write the fragment, and hand the stream over to the bidirectional copy loop—which then blindly passes Packet 2. The downstream backend
will receive the reconstructed, spoofed header, bypassing IP/fingerprint-based security rules on downstream backends.
6
7 #### Suggested Fix
8 Do not process headers from a single blind read. Instead, buffer incoming client bytes in a loop until the end-of-header delimiter (\r\n\r\n) is reached, capped at a standard L7 size limit (e.g., 8KB),
before parsing or stripping.

---

Comment 3: Header Spoofing Bypass via Oversized Headers

• Target File: src/upstream.rs
• Suggested Position: Around line 215, near let mut buf = [0u8; 4096]; in inject_request_headers

1 ### ⚠️ Security: Header Spoofing Bypass via Oversized Headers
2
3 The client read buffer in inject_request_headers is hardcoded to 4096 bytes:
let mut buf = [0u8; 4096];

1 Any request headers that lie beyond the first 4096 bytes are not captured during this phase and are instead transferred verbatim by the subsequent bidirectional copy loop.
2
3 #### Why it matters
4 Modern backends and web servers (such as Node.js, Nginx, or Apache) typically allow headers up to 8KB or 16KB. An attacker can inflate their request with 4096 bytes of dummy headers (e.g., X-Dummy: AAAA...),
followed by a spoofed X-Forwarded-For or X-JA3 header. The proxy will only clean the first 4096 bytes, letting the spoofed headers slip through to the backend undetected.
5
6 #### Suggested Fix
7 Increase the buffer limit to 8192 bytes (to match standard web server header limits) or dynamically read up to the target limit while looking for the end of headers.

[kriszyp]

Addressed the two trust-boundary findings on the HTTP/1 injection path in 86588b1.

Root cause: the original inject_request_headers hand-rolled a single-read parser and ran once before the bidirectional copy. symphony already has bounded HTTP/1 framing in http_proxy.rs (used by the plaintext :80 listener); the fix reuses it instead.

🔴 Critical — un-timeouted read (Slowloris): the header rewrite is now the client→upstream half of the bidirectional copy, wrapped in the same idle timeout as the rest of the connection. A client that completes the TLS handshake and then stalls without sending a request is dropped when the idle timeout elapses. New integration test: drops a client that stalls post-TLS without sending a request.

⚠️ High — spoofable / partial-read corruption: replaced the single-read parser with rewrite_request_stream, which frames a complete, bounded (64 KB) header block for every request — including headers fragmented across TCP segments, requests past the old fixed read size, and subsequent pipelined / keep-alive requests. Each request has client-supplied X-JA3/X-JA4/X-Forwarded-For stripped and the trusted values injected. Bodies are framed (Content-Length / chunked) only to locate the next request; Transfer-Encoding+Content-Length together, obs-fold continuations, and other malformed framing are refused rather than forwarded ambiguously. A partial first read no longer prepends headers before an incomplete request line — the whole block is framed first.

Related (empty-fingerprint pass-through): a configured fingerprint header is now stripped from the client's request even when symphony has no authoritative value to substitute, so a client can't smuggle its own value through precisely when we have nothing to replace it with.

Tests: Rust unit tests cover cross-read fragmentation, keep-alive, the 64 KB bound, partial-request-line, TE+CL / obs-fold rejection, and strip-without-value; JS integration tests cover post-TLS stall-drop, end-to-end spoof-strip, and per-request keep-alive strip. Full Rust + JS unit suites pass locally.

The remaining Medium findings (config-time validation of ineffective carrier combinations; PROXY v2 private-TLV consumer contract/docs) are separate from these two and not included here — flagging for triage.

— KrAIs (Claude Opus 4.8)

[heskew]

The two flagged findings are genuinely closed here — the 8192-byte strip window is gone (full header-block parse, all requests rewritten) and the client read is now inside the idle timeout. Verified. But a focused second pass on the new parser surfaced a residual spoof path in the tunnel handling that undercuts the same anti-spoof invariant, plus a compat regression — both inline. (This second pass was Codex standing in for the Gemini leg, which failed on this PR.)

Also, one thing the new tunnel classification now leans on: is_upgrade only inspects the first Connection header, so a client that splits Connection: keep-alive / Connection: Upgrade across two fields is read as no-tunnel — a valid WebSocket/h2c client breaks after the 101. Worth folding a multi-field Connection parse in while this code is open.

_{🤖 Draft prepared with Claude (cross-model review pipeline; Codex second-model pass); reviewed and posted by @heskew.}

[kriszyp]

@heskew both your findings (plus the multi-field Connection note) were addressed in the final commit before Kris had this merged: tunnel passthrough is now gated on the upstream's actual accept (response pump with request/response correspondence — the rejected-but-kept-alive pipelining scenario has a dedicated regression test), and header parsing went byte-level so obs-text values forward intact. Happy to follow up post-merge if anything looks off.

🤖 Claude Fable 5, on Kris's behalf

[hdbjeff]

I reviewed the final implementation and test coverage.

The previously reported HTTP-header spoofing paths are genuinely closed:

• Complete bounded header parsing across fragmented reads.
• Rewriting every keep-alive and pipelined request.
• Stripping configured headers even when no authoritative fingerprint is available.
• Keeping the initial read under the connection timeout.
• Waiting for the upstream’s 101/2xx response before entering raw Upgrade/CONNECT passthrough.
• Multi-field Connection handling.
• Preserving obs-text header values.

I found two new reliability issues:

Medium — Request metadata queue is unbounded

proxy_http1_rewriting uses mpsc::unbounded_channel, and the request pump queues one RequestMeta for every forwarded request. If an upstream continues reading requests but delays its responses, a client can pipeline requests indefinitely
and grow this queue without bound.

Please use a bounded channel or cap outstanding pipelined requests so request forwarding receives backpressure. A test should demonstrate that the request pump stops advancing when the response side falls behind.

Medium — Response framing only examines the first TE/CL field

The response parser uses header_field, which returns only the first matching field. Consequently:

• A response with Transfer-Encoding: gzip followed by Transfer-Encoding: chunked is treated as read-to-close rather than chunked.
• Conflicting or malformed Content-Length fields are not rejected.
• On a keep-alive connection, subsequent responses can be consumed as part of the incorrectly framed response.

Please combine all Transfer-Encoding fields in wire order, require the final coding to be chunked, and validate all Content-Length occurrences consistently. Add response-side tests for split TE, conflicting CL, malformed CL, and a following
keep-alive response.

Existing findings still unresolved

The two findings explicitly deferred in the follow-up remain open:

• Ineffective fingerprint-carrier configurations still deploy successfully. Static routes only emit an stderr warning, and resolved routes have no equivalent warning or validation.
• The private PROXY v2 TLVs still lack an integration test or concrete end-to-end consumer contract.

Test accuracy

The documented 64 KiB header limit is not exact: read_header_block checks for the delimiter before checking the size, and reads in 4096-byte chunks. A complete header somewhat above 64 KiB can therefore be accepted. The existing 70 KiB test
does not exercise MAX_HEADER_SIZE + 1.

[hdbjeff]

Follow-up issues from the post-merge review are now tracked:

• Bound HTTP/1 request metadata queue under pipelining #31 — bound the HTTP/1 request metadata queue under pipelining.
• Correct repeated Transfer-Encoding and Content-Length response framing #32 — correctly frame responses with repeated or malformed Transfer-Encoding / Content-Length fields.
• Reject ineffective fingerprint-forwarding carrier configurations #30 — reject ineffective fingerprint-forwarding carrier configurations for both static and resolved routes.
• PROXY v2 fingerprint/mTLS forwarding: end-to-end integration test against the real Harper consumer #29 — define and integration-test the private PROXY v2 TLV contract against the real Harper consumer.

The original fragmented/oversized/keep-alive header-spoofing fixes, empty-fingerprint stripping, timeout coverage, rejected Upgrade/CONNECT handling, multi-field Connection parsing, and obs-text handling were verified as closed in the final #23 code. The four issues above capture the remaining deferred work and new follow-up findings.