Repository navigation
feat: config-driven MITM interception for HTTPS and plain HTTP - #8
TiaraBasori wants to merge 5 commits into
Conversation
Adds a component/mitm engine behind a new top-level 'mitm:' config block: a generated CA (persisted under store-dir) mints per-host leaf certs, connections are gated on SNI peeked from the ClientHello before any TLS state exists, and gated traffic is terminated and re-originated through the same rule-resolved proxy adapter, so interception never changes which proxy carries a connection. Rewrites are declarative: ordered policies with host+path patterns, 302 redirect, 4xx reject, and request header set/add/remove. HTTP/1.1 and HTTP/2 are handled on both legs. The inner leg negotiates ALPN and serves h2 through a small frame layer (hpack via golang.org/x/net/http2/hpack, already a direct dependency); the outer leg dials per request burst and bridges an h2 client to an HTTP/1.1 upstream or vice versa. Ungated SNI, wrong ports, and gate misses return the conn untouched for the normal path; reject and redirect policies never dial upstream. Request bodies buffer up to 1 MiB per h2 stream and larger uploads are refused 413 rather than buffered without bound. The bind layer exposes MITMState, ExportMITMCa and ResetMITMCa so the client can show, export and reset the trust profile the user installs.
The custom-trust-cert seeding referenced cfg from outside its scope and used x509 without importing it; go1.23 masked both because the tuic package stops the build before hub/executor compiles. Caught by the CI-toolchain build (go1.26.9).
|
CI run 38510720399 sits in Verification already done locally with the CI toolchain (go1.26.9):
|
Connection lifecycle: - tunnel: the 5s dialCtx no longer bounds the whole intercepted connection; serve switches to a long-lived context and each upstream dial re-arms its own DefaultTCPTimeout, so keep-alive legs survive past the tunnel's dial budget (was: guaranteed 502s after 5s) - dispatcher: h1 keep-alive works end to end — resp.Close from the upstream leg (which we force to Connection:close ourselves) no longer closes the client leg, the upstream body's Close is decoupled from Response.Write's internal close so FlushBody can stream it, and the client keep-alive signal rides leg.reqClose - dial metadata is an explicit fresh copy (Pure() can alias), the deferred handshake is reported on the claimed path, and gated conns leave an interception trace in the log Protocol correctness: - rewrite url-patterns match the request path with segment boundaries (was: pattern path ignored, policies fired on every path) - host-only redirect targets (redirect: mirror.example.com) build scheme://host + original path + query (was: same-origin loop) - plain vs TLS upstream is chosen by the gate scheme, not the port; overlapping http/https port lists are rejected at load - SNI peek grows the bufio buffer before the second Peek, so >4KB ClientHellos (post-quantum key shares) are intercepted - WebSocket upgrade requests are refused with 501 instead of corrupting the request loop with a forwarded 101 Resource safety: - panic recovery around the per-connection intercept path - request/response heads capped at 64KB (431 on overflow) on both legs - per-operation deadlines: streaming bodies are never truncated by a sticky deadline and a non-reading client cannot pin a goroutine - 413 drains a bounded amount before answering so well-behaved transports see the status h2 legs: - flow control: streams register at the tracked initial window with RFC 9113 6.9.2 delta math; negative windows are connection errors (was: grant(sid,0) deadlocked every bodied request) - CONTINUATION flood capped at 64KB with open-block and same-stream checks (CVE-2024-27316 shape) - MAX_CONCURRENT_STREAMS=32 advertised with REFUSED_STREAM beyond it, plus an 8MB connection-level buffered-body budget with deferred window grants for real backpressure - terminal responses (reject/redirect/502) complete with END_STREAM - upstream RST_STREAM fails the stream, not the connection - frame hardening: zero/overflow WINDOW_UPDATE, wrong-length SETTINGS, unknown-stream DATA all GOAWAY; PING echoed with length enforced - upstream leg offers h2 via ALPN (was unreachable) with an EnableHTTP2 e2e bridging test CA engine and config: - SaveState is atomic (temp + rename) and LoadState only regenerates when BOTH files are absent — a transient read error no longer silently destroys an installed CA - gates canonicalized (lowercase, trailing dot) and non-ASCII (IDN) rejected with punycode guidance; skip-domain bare entries now cover subdomains like host gates - value-prefix prepends to the client's existing header value; the docs example (which paired value+value-prefix and failed Normalize) is fixed and pinned by a round-trip test - header names validated as RFC 7230 tokens, pattern hosts validated at load, gate/skip lists capped at 256 - leaf minting moved out of the engine mutex and eviction replaces the oldest quarter instead of nuking the cache - reload with empty store-dir reuses the running engine instead of rotating the CA on every ApplyConfig - custom-trust-cert seeding appends per-cert with honest logging; ExportMITMCa returns the DER fingerprint (matches MITMState) Tests: 21 unit + 30 h2 frame/e2e + ~20 dispatcher e2e additions (big-ClientHello, plain-HTTP, path matching, redirect host-only, 413/431, upgrade 501, h2-upstream bridging, keep-alive longevity, slow-chunked streaming, docs example round-trip).
|
Pushed a hardening pass ( Must-fixes found by review, now fixed
Also: panic recovery on the per-conn path, 64KB header caps (431), per-operation deadlines, WebSocket upgrades refused 501, h2 ALPN to upstream (was unreachable) with an EnableHTTP2 e2e, gate canonicalization (case/trailing-dot/IDN-reject), docs example fixed (it paired Local verification (go1.26.9, CI toolchain): |
TiaraBasori
left a comment
TiaraBasori
left a comment
There was a problem hiding this comment.
Hermes Agent Review (round 2)
Full independent pass over the head commit (also verified locally with go1.26.9: go build ./..., go vet, full ./component/mitm + ./config suite green — no flakes; and probed adversarially with purpose-built tests, all removed after).
Verdict: Changes required (self-review — cannot formally request changes on own PR) — 4 P0s, all reproduced, plus wiring/spec issues. The P0s: the feature never activates in production (dispatcher stored into a write-only var), every h2-origin response body >64 KiB truncates (WINDOW_UPDATE misroute), a 0-length RST_STREAM panics the process, and the h2 inner leg wedges after 32 keep-alive GETs (s.active leak).
✅ Held up well under the second pass
- CA persistence (atomic save/load, fail-closed on broken state) — probed corrupt/half-missing/dir-shaped states, all correct.
- Gate semantics, path-prefix matching (segment boundaries verified against traversal/encoded shapes), header-field validation, big-ClientHello Grow fix, h1 keep-alive and 413/431 flows — all behave as documented.
- Goroutine stability across 30 sequential intercepted requests: clean.
Details in the inline comments, ordered by severity.
| tunnel.UpdateMITM(nil) | ||
| return | ||
| } | ||
| tunnel.UpdateMITM(dispatcher) |
There was a problem hiding this comment.
🔴 P0 — the dispatcher is stored into a write-only variable; MITM never activates in production.
tunnel.UpdateMITM(dispatcher) stores into tunnel.mitmDispatcher (tunnel.go:69) — and nothing ever reads that variable. The hot path reads mitm.ActiveDispatcher() (tunnel.go:780), which loads component/mitm's own atomic (dispatcher.go:584), and mitm.UpdateMITM — the only writer of that store — is called only from bind/clash/mitm_ca_test.go. grep -rn 'mitm.UpdateMITM' --include='*.go' . outside tests returns nothing.
Consequences, all silent:
- interception never runs (hot path always sees
nil); every e2e still passes because they driveInnerHandle/NewDispatcherdirectly; MITMState/ExportMITMCa/ResetMITMCaalways report disabled;StableDispatcher(mitm.ActiveDispatcher(), …)always getsnilasprevious, so the "reuse engine across reloads" path never executes;- the disable path (executor.go:420/458
tunnel.UpdateMITM(nil)) looks like it turns MITM off but cannot.
Fix: call mitm.UpdateMITM(dispatcher) / mitm.UpdateMITM(nil) here (or drop tunnel.mitmDispatcher entirely and have UpdateMITM in tunnel delegate to mitm.UpdateMITM). Add one integration test that goes through executor.ApplyConfig → mitm.ActiveDispatcher() != nil so this class of wiring break cannot re-land.
| var wu [4]byte | ||
| wu[0], wu[1], wu[2], wu[3] = byte(len(payload)>>24)&0x7f, byte(len(payload)>>16), byte(len(payload)>>8), byte(len(payload)) | ||
| _ = c.writeFrame(h2FrameWindowUpdate, 0, 0, wu[:]) | ||
| _ = c.writeFrame(h2FrameWindowUpdate, 0, c.streamID, wu[:]) |
There was a problem hiding this comment.
🔴 P0 — stream-level WINDOW_UPDATE goes to the wrong stream; every h2-origin response body > 64 KiB is truncated.
RoundTrip bumps c.streamID += 2 (line 1216) before the response arrives, so by the time response DATA flows, c.streamID points at the next stream. The per-stream grant here lands on a phantom id instead of the stream that carried the bytes.
Reproduced against a real Go httptest h2 origin through the full dispatcher: a 100 KiB body delivers exactly 65535 bytes then unexpected EOF / upstream PROTOCOL_ERROR, and the leg emits zero WINDOW_UPDATEs for stream 1 (conn-window grants do arrive). TestE2EH2UpstreamBridgedEndToEnd passes only because its body is 9 bytes.
Fix: track the in-flight stream id explicitly (e.g. c.activeStream set in RoundTrip, cleared in FinishBody), and grant on that. Please add a big-body (>64 KiB) e2e — this is the single most common real-world shape.
| // Per-stream error only: fail the in-flight round trip with the | ||
| // code; the connection itself stays usable unless the peer also | ||
| // sends GOAWAY (W5). | ||
| code := uint32(f.payload[0])<<24 | uint32(f.payload[1])<<16 | uint32(f.payload[2])<<8 | uint32(f.payload[3]) |
There was a problem hiding this comment.
🔴 P0 — remote panic / process kill on a 0-length RST_STREAM.
f.payload[0..3] is read before the len(f.payload) == 4 check. A upstream sending RST_STREAM with an empty payload crashes the read loop:
panic: runtime error: index out of range [0] with length 0
component/mitm.(*h2ClientConn).readLoop(...) h2.go:1058
Verified with a scripted upstream. In this kernel a panic in a serve goroutine takes the process (the whole VPN) down. Swap the order: check len(f.payload) == 4 first, read the code inside that branch (which the else already does).
| _ = s.writeFrame(h2FrameRSTStream, 0, streamID, []byte{0, 0, 0, h2ErrorCodeRefused}) | ||
| return | ||
| } | ||
| s.active++ |
There was a problem hiding this comment.
🔴 P0 — s.active leaks for every END_STREAM-on-HEADERS request; the h2 inner leg wedges after 32 keep-alive GETs.
finishHeaders increments s.active, then for endStream requests delivers immediately and returns without any stream entry in s.streams — so finalizeData/releaseStream never run for it and the counter is never decremented. The common case (every GET, every HEAD) leaks one unit per request.
Reproduced: 40 sequential GETs on one h2 conn — request 33 is refused with RST_STREAM(REFUSED_STREAM) and the connection is wedged from then on. A gated h2 keep-alive browser session dies after 32 requests.
Fix: decrement for the endStream path too (e.g. s.active-- after deliverRequest, or track completion in the serve loop), plus a regression test doing >h2MaxConcurrentStreams sequential GETs on one connection.
| func (s *h2Server) finishHeaders(streamID uint32, endStream bool) { | ||
| fields, err := s.decodeHeaderBlock(s.hdrBuf, streamID) | ||
| if err != nil { | ||
| _ = s.writeFrame(h2FrameRSTStream, 0, streamID, []byte{0, 0, 0, h2ErrorCodeInternal}) |
There was a problem hiding this comment.
🟠 HPACK decode failure must be a CONNECTION error, not RST_STREAM.
On decodeHeaderBlock error the leg sends RST_STREAM(INTERNAL) and keeps reading (verified: the connection still accepts a valid request afterwards). RFC 9113 §4.3 requires COMPRESSION_ERROR — after a failed block the dynamic table state is undefined, so subsequent requests decode against a possibly-desynced table (cross-stream header confusion on a MITM leg is request-smuggling-shaped). TestH2DecodeErrorsDoNotPanic currently accepts RST or GOAWAY, which masks this — it should assert GOAWAY + connection close.
| // cached leaf and drop ours so all connections share one identity. | ||
| return cached, nil | ||
| } | ||
| e.insertLeafLocked(normalized, leaf) |
There was a problem hiding this comment.
Regenerate can race MintLeaf: a mint that snapshotted the pre-reset CA outside the lock inserts an old-CA leaf into the fresh cache after the reset, and leafFresh is time-only — the stale-CA leaf is then served for up to leafValidity (7 days), breaking the just-installed new profile for that host. Cheap fix: store the owning CA fingerprint in HostCertificate and re-check it against the current CA before serving/inserting.
| // proxyPass dials the upstream for one request, writes it, and streams the | ||
| // response back. upstreamHost is empty for direct forwards; policies that | ||
| // rewrite to another host pass the rewritten host so the dial uses it. | ||
| func (d *Dispatcher) proxyPass(ctx context.Context, leg *clientLeg, req *http.Request, host, scheme string, dial DialFunc, dstPort uint16, upstreamHost string) bool { |
There was a problem hiding this comment.
💡 proxyPass dials the upstream per request — an h1 keep-alive client pays TCP + TLS + full proxy handshake for every request, and an h2 upstream conn is closed after exactly one response (the W5 connection-reuse machinery only ever serves one round trip). The PR body describes "per request burst" dialing; the code is per request. Fine for a first cut, but ad-block style hot paths will feel this — worth a follow-up pooling note in the docs at least.
| } | ||
|
|
||
| func (u *http1Upstream) RoundTrip(ctx context.Context, req *http.Request) (*http.Response, error) { | ||
| _ = ctx |
There was a problem hiding this comment.
💡 http1Upstream.RoundTrip discards ctx (_ = ctx): a client-cancelled request (navigated away) leaves the upstream leg pinned up to the 120s idle deadline. Even without full cancellation plumbing, a shorter cap for the head phase (like outerHandshakeTimeout) would shrink the orphan window.
| # - 8000-9999 | ||
|
|
||
| mitm: | ||
| enable: false |
There was a problem hiding this comment.
💡 QUIC/HTTP3 bypasses interception entirely (PR body acknowledges; the docs don't). Users enabling this for ad-block will see it silently stop working on h3-upgraded sites. A one-line warning here ("block UDP 443 to keep h3 from bypassing MITM") would save a support thread.
| if err != nil { | ||
| // H10 policy: if the failure is inside h2.go's client leg, keep | ||
| // the NextProtos fix and record why the bridge is not green. | ||
| t.Skipf("h2 upstream bridge failed inside h2.go: %v", err) |
There was a problem hiding this comment.
💡 t.Skipf on h2-bridge failure is a escape hatch that can silently rot the outer-leg h2 path (and did mask the >64 KiB truncation). Now that the bridge exists, prefer hard failure.
P0 (all reproduced before fixing): - executor wiring: tunnel.UpdateMITM stored into a write-only tunnel-local var; the hot path reads mitm.ActiveDispatcher. Delegate to mitm.UpdateMITM so interception/CA export/disable actually reach the running core. Integration test pins it. - h2 client leg: stream-level WINDOW_UPDATE went to c.streamID (the NEXT stream, already bumped) instead of the in-flight one, so response bodies > 64 KiB truncated at exactly 65535 bytes. Grant on c.streamID-2. Big-body (>64 KiB) e2e added against a real h2 origin. - h2 client leg: 0-length RST_STREAM indexed f.payload[0..3] before the length check -> read-loop panic took down the process. Check length first. - h2 server leg: s.active leaked for every END_STREAM-on-HEADERS request (all GETs), wedging the connection at 32 keep-alive GETs. Release the slot on the fully-delivered path. Regression test drives 72 sequential GETs. P1: - HPACK decode failure is now COMPRESSION_ERROR + close (dynamic table state undefined after a failed block; RST-and-continue risked cross-stream header confusion). - server-leg DATA writes bounded by a per-chunk flow-control deadline (takeWithDeadline; channel-based wakeup replacing cond.Broadcast) + flow.Shutdown on readLoop exit/Close, so a stalled client can no longer park the serve goroutine forever. - SNI peek bounded by innerHandshakeTimeout; a stalled pre-ClientHello client releases the tunnel conn slot instead of pinning it. P2/P3: - HEADERS on stream 0 / even IDs GOAWAY (PROTOCOL_ERROR) instead of being answered from stream 0. - plain-HTTP gate no longer requires DNSMode==DNSMapping (fake-ip and sniffer-replaced metadata never carry it; port-80 interception was unreachable on default configs). Host quality enforced downstream by MatchHost in the cleartext path. - port overlap validation now compares EFFECTIVE sets: https-ports: ["80"] (http standard-port-only) is rejected instead of loading into a config where every :80 conn fails a TLS handshake. - HostCertificate records its signing-CA fingerprint; leaves minted by a previous CA are refused and re-minted after Regenerate/LoadState (race replay test). - docs: note that QUIC/HTTP3 bypasses MITM and how to force h2. - pre-existing race: h2ClientConn round-trip state (respOnce/respReady/ respErr/status/header/pipes) reset by RoundTrip while the readLoop published a failure; guarded by rtMu (found by -race). 12 new regression tests, all reproducing the pre-fix defects.
|
All findings fixed in 02d516a (pushed): P0
P1 P2/P3 Verification: |
- idleTimeout became an atomic (idleTimeoutNow accessor): tests shorten it while leg goroutines from earlier tests are still reading it — the mutation is test-only and benign, but a plain var data-races. Product behavior unchanged. - TestE2EOversizedPostRejected413: the exact overflow write that fails races the interceptor's 413+close on loopback buffering; tolerate a broken pipe past the drain bound (>=16 of 17 chunks) and go read the answer. Verified: 6 consecutive full -race runs of ./component/mitm, all green, 0 warnings.
|
Follow-up 94b46d7: the late
Verified with 6 consecutive full |
1 participant
Summary
component/mitmengine behind a top-levelmitm:config block: a generated CA mints per-host leaf certificates, connections are gated on the ClientHello SNI (peeked, never consumed), and gated traffic is terminated and re-originated through the same rule-resolved proxy adapter — interception never changes which proxy carries a connection.golang.org/x/net/http2/hpack, already a direct dependency; frame + flow control implemented locally so the Apple SDK build stays free of a full HTTP/2 stack); the outer leg dials per request burst and bridges h2↔HTTP/1.1 in either direction.host+pathurl-patterns, 302 redirect (bare-host redirects keep path/query), 4xx reject, request-headerset/add/removewithvalue-prefixcomposition.MITMState/ExportMITMCa/ResetMITMCaso the client can display, export and reset the trust profile the user installs.Motivation
Anywhere (NodePassProject) ships a built-in MITM (declarative rules + JS over terminated TLS) as a core feature of a native client. ProjectClash's kernel is a mihomo-lineage fork where the sniffer can only observe SNI/Host — there is no way to rewrite HTTP on hosts the user controls. This PR brings the interception model over, adapted to this kernel's architecture: the tun lives in the Go core (the NE feed sits on top of it), so MITM must be a core component; a Swift-side MITM would never see the streams.
Design notes that drove the implementation:
crypto/tlsis only constructed after the host is known to be gated.metadata.Pure()(fake-IP host restored, dst IP cleared, DNS re-resolved by the core). MITM changes what is said to the origin, never how it gets there.Changes
component/mitm/— new package:mitm.go(CA engine: RSA-2048 root, P-256 leaves, persist/load/regenerate,Equal()-based key-pair verification),config.go(normalize/validate: host gatesexample.com/+.example.com, skip rules, port ranges via the existingutils.IntRanges, rewrite policies; rejects wildcards, bare IPs, header injection, javascript: redirects, loops through >64 policies),dispatcher.go(tunnel entry: gate → SNI peek → terminate → reverse-proxy loop; upstream TLS honorstls.custom-trust-cert),h2.go(h2 server + client legs: frame codec, flow control with cond-based window waits, CONTINUATION assembly, padded frames, one serial response at a time).tunnel/tunnel.go—mitmDispatcheratomic slot +UpdateMITM; the TCP hot path asks MITM afterresolveMetadataContextso the dial func has the selected proxy.config/config.go—RawMITM+parseMITM(ports reuse the sniffer's range parser;+.example.comnormalizes to the leading-dot form).hub/executor/executor.go—updateMITMalongsideupdateSniffer, fail-closed on init errors, custom-trust-cert seeding.bind/clash/mitm_ca.go— CA exports for the App, under the standardrecoverAppCallguard.docs/config.yaml— annotatedmitm:example in the existing style.Test Plan
component/mitmunit tests: CA mint/verify against own pool, leaf cache identity + eviction bound, save/load round-trip (incl. 0600 key perms), key-mismatch rejection, gate semantics (suffix match withoutnotexample.com/evil.testfalse-positives, skip-domain precedence), normalize rejection matrix, first-match-wins, port ranges.go build ./...andgo vet ./...with the CI toolchain (go1.26.9), main module andbind/clash.xcframeworkjob) — gomobile-bindable APIs only (string/[]byte/JSON structs).Notes for Reviewers
net/http-transport-shaped dependencies; if reviewers prefer,http.Server's h2 could serve the inner leg viah2c-style wrapping at the cost of tying the leg to stdlib server semantics — the frame layer here is ~600 lines and gives explicit control over buffering and stream limits.updateMITMis called afterupdateSnifferon every reload; a broken CA store logs a warning and disables MITM rather than poisoning the hot path (fail closed).