docs(forwarder): document the subnet-route SSRF-guard scoping + lock the SOCKS5 reply ATYP reject#252
Conversation
…the SOCKS5 reply ATYP reject The exit-egress SSRF guard (exit_dst_is_forbidden) gates only ExitNode-class flows by design: a Subnet flow (a destination covered by a narrower advertised route) skips it, because a subnet route legitimately targets a private range -- that range IS the routed subnet. This is NOT an attacker-controllable SSRF (the flow class derives from the operator's own advertised_routes, never the peer), but a deployer who advertises a sensitive internal range as a forwarded subnet route WILL have the forwarder dial into it. Add a deployer note to AGENTS.md so operators don't advertise link-local / CGNAT / loopback as forwardable subnets. Also lock, with an adversarial test, the SOCKS5 connect-reply parser's reject-before-unbounded-read: a hostile reply with a non-IPv4 bound-address type (ATYP=0x03 domain + an attacker-chosen 0xFF length) is rejected at the `head[3] != 0x01` IPv4-only guard BEFORE any variable-length read, so a malicious proxy can't drive an oversized/length-driven read. The parser was already correct; the test pins it against a future refactor that might add an ATYP-dispatched read. Docs + test only; no behavior change. cargo test -p ts_forwarder (32 dialer + the rest), clippy -D warnings clean, fmt, cargo run -p checks all green. Signed-off-by: Sergio <sergio@geiser.cloud>
… bound) Belt-and-suspenders per review: wrap the dial in a 100ms timeout so 'reject before the variable-length read' is proven by timing, independent of the Io-vs-ProxyHandshake error-type discrimination — a future refactor collapsing both into ProxyHandshake would otherwise weaken the assertion. Signed-off-by: Sergio <sergio@geiser.cloud>
|
Warning Review limit reached
More reviews will be available in 44 minutes and 29 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Release 0.39.0. Bundles the parity + anti-leak batch: magicsock CallMeMaybe immediate-ping (#246), peerless-STUN-stop (#247), STUN SOFTWARE+FINGERPRINT (#241); MagicDNS RD/RA+compression (#242) + SERVFAIL-not-NXDOMAIN (#248); DERP send rate-limit (#249); IPv4 fragment handling (#253); control Hostinfo/ProtoPortRange/NetInfo wire fixes (#244/#245); netcheck StunProber deletion (#250); forwarder subnet-SSRF doc+test (#252); control panic-hardening (#254).
1 participant
What
Two related hardening items in the exit-egress forwarder (no behavior change):
Deployer note (AGENTS.md): the exit-egress SSRF guard (
exit_dst_is_forbidden) is scoped toExitNode-class flows by design. ASubnetflow (a destination covered by a narrower advertised route) skips it, because a subnet route legitimately targets a private range — that range is the routed subnet. This is not an attacker-controllable SSRF (the flow class derives from the operator's own advertised routes, never the peer), but a deployer who advertises a sensitive internal range as a forwarded subnet route will have the forwarder dial into it. The note warns operators not to advertise link-local (169.254/16), the tailnet CGNAT range (100.64/10), or loopback as forwardable subnets. (0.0.0.0/0as an exit node stays behind the guard.)Adversarial test (ts_forwarder/src/dialer.rs): lock the SOCKS5 connect-reply parser's reject-before-unbounded-read. A hostile reply with a non-IPv4 bound-address type (
ATYP=0x03domain + an attacker-chosen0xFFlength) is rejected at thehead[3] != 0x01IPv4-only guard before any variable-length read, so a malicious proxy can't drive a length-driven/oversized read. The parser was already correct; the test pins it.Why the test is strong (per security review)
The fake proxy sends
[0x05,0x00,0x00,0x03,0xff](REP=success, ATYP=domain, len=255) then nothing. The assertion is discriminating: the ATYP guard returnsProxyHandshakeexplicitly, while everyread_exactmaps EOF/io errors toDialError::Io(notProxyHandshake) — so a regression that tried to read the (never-sent) domain would surfaceIo(EOF)and fail thematches!(ProxyHandshake)assertion. Atokio::time::timeout(100ms)wrapper adds a timing bound so "reject before the read" is also proven independent of the error-type mapping (belt-and-suspenders).Verification
Docs + test only;
git diff --statshows AGENTS.md + dialer.rsmod testsadditions, 0 production-code deletions.cargo test -p geiserx_ts_forwarder(32 dialer + the rest),clippy -D warnings(0),fmt --check,cargo run -p checks(anti-leak guard) all green.Created using Claude Code (Opus 4.8)