Repository navigation
fix(mitm): TPROXY OUTPUT-based recipe for local traffic (validated e2e on VPS) - #4156
Conversation
…e on VPS) The MITM use case is LOCAL outbound traffic (IDE agents on the same host), which TPROXY in PREROUTING does NOT intercept — PREROUTING only sees forwarded traffic. The previous builder (#4139) emitted a PREROUTING-only recipe; a VPS spike proved it never delivered local connections to the socket. Rework the builder to the OUTPUT-based recipe: mark new local outbound conns in mangle OUTPUT → an ip rule reroutes marked packets to local delivery (lo) → the mangle PREROUTING TPROXY target assigns them to the IP_TRANSPARENT listener. Add an optional bypassMark (the SO_MARK the proxy sets on its own upstream conns) so the OUTPUT rule can exclude the proxy's traffic (anti-loop). revert stays the exact inverse in reverse order. Validated e2e on the VPS (kernel 6.8.0), isolated to a test port + dedicated fwmark so production 443/80 was untouched: client CONNECTED and the listener saw the original destination preserved (198.51.100.7:9999). 11/11 unit tests (commands + setup wiring) green.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Code Review
This pull request updates the TPROXY transparent capture mode to use an OUTPUT-based recipe, enabling the interception of local outbound traffic. It introduces a bypassMark configuration option to prevent infinite routing loops by excluding the proxy's own upstream connections. The command builders and unit tests have been updated to support this new 4-command flow. The review feedback suggests adding validation to ensure that firewall marks do not exceed the 32-bit unsigned integer limit (0xffffffff) to prevent runtime command failures, along with corresponding unit tests to verify this boundary check.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if (cfg.bypassMark !== undefined) { | ||
| if (!Number.isInteger(cfg.bypassMark) || cfg.bypassMark < 1) { | ||
| return `bypassMark must be a positive integer when set, got ${cfg.bypassMark}`; | ||
| } | ||
| if (cfg.bypassMark === cfg.mark) return "bypassMark must differ from mark (anti-loop)"; | ||
| } |
There was a problem hiding this comment.
In Linux, firewall marks (fwmark) are represented as 32-bit unsigned integers (up to 4294967295 or 0xffffffff). If a value larger than this is passed, the iptables or ip commands will fail at runtime with an error. It is safer to validate that bypassMark (and ideally mark and routeTable as well) does not exceed 0xffffffff.
| if (cfg.bypassMark !== undefined) { | |
| if (!Number.isInteger(cfg.bypassMark) || cfg.bypassMark < 1) { | |
| return `bypassMark must be a positive integer when set, got ${cfg.bypassMark}`; | |
| } | |
| if (cfg.bypassMark === cfg.mark) return "bypassMark must differ from mark (anti-loop)"; | |
| } | |
| if (cfg.bypassMark !== undefined) { | |
| if (!Number.isInteger(cfg.bypassMark) || cfg.bypassMark < 1 || cfg.bypassMark > 0xffffffff) { | |
| return 'bypassMark must be a positive 32-bit unsigned integer when set, got ' + cfg.bypassMark; | |
| } | |
| if (cfg.bypassMark === cfg.mark) return "bypassMark must differ from mark (anti-loop)"; | |
| } |
| assert.match(validateTproxyConfig({ ...CFG, onPort: 70000 }) ?? "", /onPort/i); | ||
| assert.match(validateTproxyConfig({ ...CFG, mark: 0 }) ?? "", /mark/i); | ||
| assert.match(validateTproxyConfig({ ...CFG, routeTable: -1 }) ?? "", /table/i); | ||
| assert.match(validateTproxyConfig({ ...CFG, bypassMark: CFG.mark }) ?? "", /bypassMark/i); |
There was a problem hiding this comment.
Add a unit test to verify that validateTproxyConfig correctly rejects bypassMark values that exceed the 32-bit unsigned integer limit (0xffffffff).
assert.match(validateTproxyConfig({ ...CFG, bypassMark: CFG.mark }) ?? "", /bypassMark/i);
assert.match(validateTproxyConfig({ ...CFG, bypassMark: 0xffffffff + 1 }) ?? "", /bypassMark/i);The OUTPUT-based TPROXY recipe (#4156) marks all new local outbound conns to the target port — including the proxy's OWN upstream connections when it forwards, which would re-enter TPROXY (infinite loop). setSocketMark(fd, mark) lets the listener stamp SO_MARK on its upstream sockets so the mangle OUTPUT rule (-m mark ! --mark <bypass>) excludes them. Viability validated on the VPS (root): setSocketMark works on a fd, and a Node outbound socket's fd is reachable via socket._handle.fd, so the listener can mark its forward connections. The loader now requires both addon functions; tests + the addon build updated. 8/8 wrapper tests green.
…e on VPS) (cheaper-inference#4156) Integrated into release/v3.8.29
Summary
The TPROXY command builder (#4139) emitted a PREROUTING-only recipe. A VPS spike proved that recipe never intercepts local outbound traffic — and local outbound is the real MITM use case (IDE agents run on the same host).
PREROUTINGonly sees forwarded traffic; local traffic must be captured via theOUTPUTchain.Rework to the OUTPUT-based recipe, validated end-to-end on the VPS:
Flow: mark new local outbound conns in
mangle OUTPUT→ theip rulereroutes marked packets to local delivery (lo) → on re-entry thePREROUTINGTPROXY target assigns them to the IP_TRANSPARENT listener (#4148).revertstays the exact inverse in reverse order (crash-safe — the Fase 1 /repairMitm()invariant).bypassMark(optional, anti-loop): the SO_MARK the proxy sets on its own upstream connections; the OUTPUT rule excludes it so the proxy's forwarded traffic isn't re-intercepted. (The listener wiring that sets SO_MARK is a follow-up.)Validated e2e on the VPS (Hard Rule #18)
Kernel
6.8.0-124, isolated to a test port (9999) + dedicated fwmark (0x2333) so production 443/80 traffic was never touched:(The earlier PREROUTING-only recipe, by contrast, never delivered the SYN to the socket — confirmed even with a pure-C IP_TRANSPARENT listener, proving the socket was fine and the recipe was the gap.)
Follow-ups (camada 4)
bypassMarkexcludes them.repairMitm()callingrevertTproxy()once a TPROXY config is persisted.Test Plan
node --test tproxy-commands.test.ts tproxy-setup.test.ts— 11/11 (recipe, anti-loop exclusion, exact-inverse revert, execFile-safe, validation)typecheck:core— 0 errors ·eslint— 0🤖 Generated with Claude Code