feat(proxy): CIDR rules + HTTP Host header peeking on port 80#39
Merged
nnemirovsky merged 6 commits intomainfrom May 8, 2026
Merged
feat(proxy): CIDR rules + HTTP Host header peeking on port 80#39nnemirovsky merged 6 commits intomainfrom
nnemirovsky merged 6 commits intomainfrom
Conversation
Two related improvements that together let the policy engine match hostname-based rules for IP-only CONNECT traffic on port 80, mirroring the SNI-peek path that already exists for TLS ports. CIDR destinations A rule's destination is now treated as a CIDR when it contains a `/`, and as a glob otherwise. compileRules calls net.ParseCIDR for the CIDR branch and stores the resulting IPNet on compiledRule alongside the existing optional Glob. matchDestination dispatches between IP containment and glob matching depending on which is non-nil. Invalid CIDR masks fail at compile time rather than silently matching nothing. A CIDR rule only matches destinations that parse as an IP, so hostnames cannot accidentally match 0.0.0.0/0. HTTP Host header peeking New isPlainHTTPPort guard (80, 8080) and ctxKeyHTTPHostDeferred extend the same defer-then-peek mechanism that SNI uses. When the SOCKS5 CONNECT carries a bare IP and the verdict is not already Allow / Deny, Resolve sets the HTTP-host-deferred flag. handleConnect then sends CONNECT success early, peeks the client's request bytes via peekHTTPHost, parses the request line + headers via http.ReadRequest, extracts the Host value, and re-evaluates policy against the hostname. Bytes consumed during the peek are prepended to the relay reader so the upstream sees the full request. The peek is bounded to 8 KiB and bails out fast when the first byte is not in [A-Z], so non-HTTP traffic on port 80 falls back cleanly. Operator motivation: tailscaled's DERP latency probes hit dozens of bare DERP IPs on port 80. With the new path, a single *.tailscale.com rule covers them, instead of one approval prompt per IP.
There was a problem hiding this comment.
Pull request overview
This PR enhances the proxy’s policy matching in two places: (1) supports CIDR-form destinations in policy rules, and (2) adds an HTTP/1.x Host-header peek path for IP-only SOCKS5 CONNECT traffic on plain-HTTP ports to enable hostname-based policy matching (similar to the existing TLS SNI peek path).
Changes:
- Treat rule destinations containing
/as CIDRs (compiled vianet.ParseCIDR) and match by IP containment; otherwise continue using glob matching. - Add HTTP Host deferral + peeking for ports 80/8080 to recover hostname from HTTP/1.x headers and re-run policy evaluation before dialing.
- Add new unit tests for CIDR compilation/matching and HTTP host extraction/peeking, plus documentation updates in
CLAUDE.md.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/proxy/server.go | Adds HTTP Host deferral and pre-dial Host peek policy re-evaluation path. |
| internal/proxy/http_host.go | Implements bounded HTTP/1.x request/header peek + Host extraction. |
| internal/proxy/http_host_test.go | Unit tests for Host extraction and peek behavior (caps, truncation, non-HTTP fast bail). |
| internal/policy/engine.go | Adds CIDR-aware rule compilation and destination matching dispatch (CIDR vs glob). |
| internal/policy/engine_test.go | Tests for CIDR compilation, invalid mask rejection, containment matching, and end-to-end evaluation. |
| CLAUDE.md | Documents destination matching semantics and hostname recovery behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Three review comments addressed. 1. Reword isPlainHTTPPort comment to drop the "Telegram approval prompts" phrasing. The helper is product-neutral and the doc should match — operators reading the code may not be using the Telegram channel at all. 2. Replace strings.Split(dest, ":")[0] with request.DestAddr.IP.String() in both the SNI and HTTP-host policy-check paths. The split approach mishandled IPv6 destinations because DestAddr.String() emits IPv6 as "[::1]:80" and splitting on ":" yielded partial values that corrupted logs and stored an incorrect key into the reverse-DNS cache. The .IP field is already the parsed net.IP, so .String() returns the address-only form regardless of family. Same one-line bug existed in the pre-existing SNI helper; fixing both at once because the PR introduced the second instance. 3. Reword peekHTTPHost docstring to say "plain HTTP (e.g. ports 80, 8080)" rather than "plain HTTP on port 80", matching what the caller actually enables via isPlainHTTPPort.
Copilot round 2: the new HTTP-host peek path re-evaluated policy using the client-supplied Host header, but the subsequent dial still went to the original SOCKS5 destination IP. Without a binding between Host and dest, an agent could connect to an arbitrary IP:80 and claim Host: <permitted-name> to slip past hostname-based policy. TLS catches this naturally because SNI/cert mismatch fails the upstream handshake; plain HTTP has no equivalent integrity check. New hostResolvesToIP attests the Host -> dest binding before the verdict is upgraded: - Reverse-cache hit for the dest IP whose stored hostname equals the Host claim is accepted as attested. The cache is populated by the agent's own prior DNS query, which is the strongest available signal that host -> dest is real. - Otherwise a forward DNS lookup runs with a 2s timeout. The dest IP must appear in the result set. - Confirmed mismatch (DNS resolved but dest is not in the result) and lookup failure both deny outright. Falling back to IP-based Ask would surface the spoofed hostname in the broker prompt, which is exactly the manipulation the attacker wanted. Tests cover the cache-attestation happy path, cache-different-host rejection, and nil-input guards.
Three review comments addressed. 1. Gate HTTP-host deferral on broker presence. The peek inside handleConnect must send SOCKS5 RepSuccess before it can read the request bytes, which means a deferred Ask-with-no-broker would manifest as success-then-reset on the client side instead of a clean RepHostUnreachable. Resolve now skips the defer when no broker is configured, falling through to the IP-based path so the Ask->Deny collapse happens before SOCKS5 success goes out, matching how go-socks5 reports failure for the non-deferred Ask-without-broker case. 2. Reorder spoofing check after policy evaluation. Before, every extracted Host header triggered a forward DNS lookup even when policy would deny it, which leaked denied hostnames to the resolver and added avoidable latency. Now the verdict is computed first; Deny short-circuits without any lookup, and Ask-without-broker also short-circuits. The DNS forward check only runs for Allow and Ask-with-broker outcomes where the verdict could result in keeping the connection. 3. Make the spoofing-guard tests hermetic. Split the cache attestation into attestHostFromCache (pure cache, no network) and add an injectable lookupIP field on Server so hostResolvesToIP tests stub the resolver instead of hitting the real one. The previous mismatch test indirectly performed a real net.DefaultResolver.LookupIP, which is slow and flaky in restricted CI environments. New tests cover lookup-match, lookup-mismatch (spoofing), and lookup-error paths through the stub plus the cache-only paths through the split helper.
Copilot round 4: when peekHTTPHost failed to recover a Host (binary protocol on port 80, truncated headers, peek timeout, malformed HTTP), the deferred path was returning allow=true with the buffered bytes for relay. The deferral runs only for non-Allow / non-Deny verdicts (i.e. Ask), so this silently upgraded Ask into an unconditional allow for any port-80 traffic that didn't parse as HTTP. Now the peek-failure branch denies and closes the connection. We already sent SOCKS5 RepSuccess to enable the peek, so the client observes a closed connection rather than a clean SOCKS5 reject; that is acceptable because the alternative is a real bypass and a non-HTTP probe on a deferred port is the suspect pattern we want to block anyway.
Round 5 review correcting an over-correction from round 4 plus an
IPv6 parsing edge case.
1. Host peek failure no longer collapses Ask to Deny. Round 4
switched the bypass to outright Deny, but that converted any
non-HTTP traffic on a deferred port-80 connection into Deny
regardless of the original IP-level Ask verdict, contrary to
the "fall back cleanly" intent. Now the failure path attaches
a per-request RequestPolicyChecker bound to the IP destination
and returns allow=true with the buffered bytes; the dial step
calls CheckAndConsume which broker-prompts the operator for
the bare IP. Approval => connection proceeds; denial => dial
fails and the connection closes. This mirrors the non-deferred
Ask-with-broker handling and preserves the Ask semantic. The
deferral guard already requires broker != nil, so the checker
has somewhere to send the prompt.
2. extractHTTPHost now uses net.SplitHostPort to strip the port
from a Host header. The previous strings.LastIndex(":") logic
correctly handled bracketed IPv6 ("[::1]:80") and DNS hosts
("example.com:80") but mishandled bare IPv6 ("2001:db8::1"),
stripping the final hextet as if it were a port. SplitHostPort
errors on bare IPv6 ("too many colons in address"), and the
error path now falls back to the trimmed host unchanged. Added
a regression test for the bare-IPv6 input.
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
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.
Summary
Two related improvements that together let the policy engine match hostname-based rules for IP-only CONNECT traffic on port 80, mirroring the SNI-peek path that already exists for TLS ports.
1. CIDR destinations
A rule's destination is now treated as a CIDR when it contains a
/, and as a glob otherwise. Before this change, a rule like192.168.0.0/24compiled to a glob that matched only the literal string192.168.0.0/24, never an IP inside the range. Operators ran into this when adding allow rules for tailscale's DERP /24s and seeing zero traffic actually allowed.compileRulesnow callsnet.ParseCIDRfor the CIDR branch and stores the resulting*net.IPNetoncompiledRulealongside the existing optionalGlob.matchDestinationdispatches between IP containment and glob matching depending on which is non-nil. Invalid CIDR masks fail at compile time rather than silently matching nothing. A CIDR rule only matches destinations that parse as an IP, so hostnames cannot accidentally match0.0.0.0/0.2. HTTP Host header peeking
New
isPlainHTTPPortguard (80, 8080) andctxKeyHTTPHostDeferredextend the same defer-then-peek mechanism that SNI already uses. When the SOCKS5 CONNECT carries a bare IP and the verdict is not alreadyAllow/Deny,Resolvesets the HTTP-host-deferred flag.handleConnectthen sends CONNECT success early, peeks the client's request bytes viapeekHTTPHost, parses the request line + headers viahttp.ReadRequest, extracts theHost:value, and re-evaluates policy against the hostname. Bytes consumed during the peek are prepended to the relay reader so the upstream sees the full request. The peek is bounded to 8 KiB and bails out fast when the first byte is not in[A-Z], so non-HTTP traffic on port 80 falls back cleanly.Operator motivation
tailscaled's DERP latency probes hit dozens of bare DERP IPs on port 80. With the new path, a single*.tailscale.comrule covers them, instead of one approval prompt per IP. The pattern generalizes to any HTTP probe / health check that addresses by IP rather than name.Test plan
go build ./...go test ./...clean across all packages, including new focused tests:extractHTTPHosthappy path / port stripping / IPv6 in brackets / missing Host / binary garbagepeekHTTPHostfull request / non-HTTP first byte fast-bail / truncated headers /maxBytescapcompileRulesCIDR branch (IPv4 + IPv6 + /32) and rejection of invalid masksmatchDestinationcontainment vs hostname-string mismatchEvaluateend-to-end with a CIDR allow rulegofumpt -lcleangolangci-lint run ./internal/...clean