PROXY protocol v2 + downstream JA3/JA4 fingerprint forwarding#23
Open
kriszyp wants to merge 4 commits into
Open
PROXY protocol v2 + downstream JA3/JA4 fingerprint forwarding#23kriszyp wants to merge 4 commits into
kriszyp wants to merge 4 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
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.
Member
Author
|
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:
Nothing to action on those two. — KrAIs (Claude Opus 4.8) |
Adds two related edge-proxy capabilities (issue #9): 1. PROXY protocol v2 (binary, TLV-capable) header emission alongside the existing v1 text encoder, selectable per route via sourceAddressHeader: 'proxyProtocolV2'. v1 stays the UDS default since Harper core's own UDS reader currently parses v1 only. 2. forwardFingerprint ('ja3' | 'ja4' | 'none') forwards the ClientHello fingerprint symphony already computes to the upstream so backends can act on it. Carrier follows the mode: a PROXY v2 TLV (0xE0=JA3 / 0xE1=JA4, in HAProxy's private range) under proxyProtocolV2 — which works in passthrough since it prefixes the raw TLS bytes — otherwise an injected X-JA3/X-JA4 HTTP header for terminated L7 upstreams. Fingerprint-agnostic: JA4 rides the same path as JA3 (built on the JA4 branch, #2/#20). proxy_conn.rs threads the fingerprint + the connection's local_addr (the v2 destination) into apply_source_header; inject_x_forwarded_for generalized to inject_request_headers for XFF + fingerprint headers in one request rewrite. Drive-by: toJsRoute/resolveConnection dropped the http2 field entirely — it was declared in types + parsed in Rust but never forwarded by the JS wrapper; fixed alongside forwardFingerprint. Also unstuck a stale server.spec version assertion (0.4.0 → 0.5.0). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… client copies Cross-model review (Codex + domain pass) surfaced three issues in the header carrier, all fixed by enforcing the invariant that HTTP-header injection is only valid for a plaintext HTTP/1 upstream and injected trust-signal headers must be authoritative: - Passthrough / h2 corruption: injecting X-JA3/X-Forwarded-For into raw TLS ciphertext (terminateTls:false) or an HTTP/2 frame stream broke the upstream. apply_source_header now gates all header injection on l7_http1 (terminated TLS AND non-h2 negotiated ALPN) and no-ops otherwise; the PROXY v2 TLV carrier is unaffected (it prefixes the bytes). This also fixes the pre-existing latent X-Forwarded-For-in-passthrough corruption symmetrically. - Spoofing: inject_request_headers now strips any client-supplied copy of an injected header (case-insensitive) before writing symphony's value, so a client can't pre-seed X-JA3/X-JA4 (or X-Forwarded-For). Tests: passthrough no-op (end-to-end round-trip intact), h2 negotiation + no injection into the h2 preface, client-header strip unit test. Broadened the config warning to cover http2 routes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Per gemini-code-assist review: refuse (io::Error) an oversized TLV value or total header instead of silently truncating the u16 length field. Never hit by JA3/JA4 (short), but the encoder is generic. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
kriszyp
force-pushed
the
kris/proxy-v2-fingerprint-9
branch
from
73c04c0 to
18bea07
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 tomainwhen #20 merges.What it does
write_proxy_v2_header) beside the v1 text one, selected per route viasourceAddressHeader: 'proxyProtocolV2'. v2 adds the TLV section used below. Stays opt-in; UDS default remains v1 (Harper core's own UDS reader parses v1 only).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) underproxyProtocolV2— works in passthrough since it prefixes the raw bytes — otherwise an injectedX-JA3/X-JA4header 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_headergates all HTTP-header injection onl7_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_headersstrips any client-supplied copy of an injected header (anti-spoof, so a client can't pre-seedX-JA3).Notes for the reviewer
toJsRoute/resolveConnectionints/proxy.tsnever forwarded thehttp2field — it was declared in types and parsed in Rust but silently dropped by the JS wrapper, sohttp2: truewas 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 carryinghttp2: truewill now genuinely negotiate h2. Confirmed with Kris that it stays bundled here rather than splitting out.agyleg 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-JA3injection, passthrough no-op, and h2 negotiation + no-injection. Full suite green (Rust 22, Node 39).Generated by an LLM (Claude Opus 4.8).