Skip to content

Commit 399807c

Browse files
authored
feat(proxy): CIDR rules + HTTP Host header peeking on port 80 (#39)
* feat(proxy): cidr destinations + http host header peeking 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. * fix(proxy): copilot review round 1 on PR #39 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. * fix(proxy): spoofing guard for http host peek 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. * fix(proxy): copilot review round 3 on PR #39 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. * fix(proxy): deny http host peek failures instead of allowing through 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. * fix(proxy): preserve ask semantic on host peek failure + ipv6 host parse 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.
1 parent 61f4c00 commit 399807c

6 files changed

Lines changed: 736 additions & 14 deletions

File tree

CLAUDE.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,10 @@ See `internal/proxy/request_policy.go`, `internal/policy/engine.go` (`EvaluateDe
274274

275275
`LoadFromStore` reads rules from SQLite, compiles glob patterns into regexes, produces read-only Engine snapshot. `Evaluate(dest, port)` checks deny first, then allow, then ask, falling back to default verdict. Mutations go through the store, then a new Engine is compiled and atomically swapped via `srv.StoreEngine()`. SIGHUP also rebuilds the binding resolver and swaps it via `srv.StoreResolver()`.
276276

277+
**Destination matching: glob and CIDR.** A rule's `destination` is interpreted as a CIDR when it contains a `/` (e.g. `192.168.0.0/16`, `2001:db8::/32`) and as a glob otherwise (e.g. `*.tailscale.com`, `api.openai.com`, `10.0.0.5`). CIDR rules use IP containment via `net.IPNet.Contains`; glob rules use the existing `[^.]*` / `(.*\.)?` matcher. A CIDR rule only matches destinations that parse as an IP, so `example.com` cannot accidentally match `0.0.0.0/0`; conversely a glob rule only matches its compiled string pattern, so `192.168.0.*` works for the 256 hosts in `192.168.0.0/24` but does not magically extend to other subnets. Compile errors are loud (invalid CIDR mask fails `compileRules` rather than silently matching nothing).
278+
279+
**Hostname recovery for IP-only CONNECT requests.** Two peek paths run before dial when the SOCKS5 layer received a bare IP and a hostname rule could plausibly match: `[SNI-DEFER]` for TLS ports (443, 8443, 993, 995, 465) reads the TLS ClientHello and extracts SNI; `[HTTP-HOST-DEFER]` for plain HTTP ports (80, 8080) reads the request prefix up to `\r\n\r\n` and extracts the `Host:` header. Both feed the recovered hostname back into `EvaluateDetailed` and update `ctxKeyFQDN` so the dial uses the hostname for upstream selection. The HTTP path is what makes `*.tailscale.com:80` rules match tailscale's bare-IP DERP latency probes without flooding the approval channel with one prompt per IP. Bytes consumed during the peek are prepended to the relay reader via `io.MultiReader` so the upstream sees the full request.
280+
277281
**Unscoped rules match all transports.** A rule without a `protocols` field (the common case for CLI-added rules like `sluice policy add allow cloudflare.com --ports 443`) matches TCP, UDP, and QUIC traffic. `EvaluateUDP` and `EvaluateQUICDetailed` first check protocol-scoped rules (`matchRulesStrictProto` with `protocols=["udp"]`/`["quic"]`) and fall back to unscoped rules (`matchRulesUnscoped`) before the engine's configured default verdict. UDP and QUIC use the same default as TCP; there is no hidden "UDP default-deny" override. `EvaluateUDP` collapses an Ask default to Deny because per-packet approval is impractical, while `EvaluateQUICDetailed` preserves Ask for the QUIC per-request approval flow. Protocol-scoped rules (`protocols=["tcp"]`, `["udp"]`, `["quic"]`, etc.) still apply only to their declared protocol. DNS has its own evaluation path via `IsDeniedDomain`, so the unscoped-rule fallback for UDP/QUIC does not affect DNS query handling.
278282

279283
### Protocol detection

internal/policy/engine.go

Lines changed: 49 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,35 @@ func isUDPFamilyProto(proto string) bool {
2727
}
2828

2929
type compiledRule struct {
30+
// Exactly one of glob or cidr is set per rule. cidr is set when
31+
// the rule's destination is a CIDR like "192.168.1.0/24" or a
32+
// bare IP-with-mask like "10.0.0.1/32"; glob is set for hostname
33+
// patterns and bare IPs without a mask. The matchDestination
34+
// method dispatches on which is non-nil.
3035
glob *Glob
36+
cidr *net.IPNet
3137
ports map[int]bool
3238
protocols map[string]bool
3339
}
3440

41+
// matchDestination reports whether dest matches this rule's destination.
42+
// CIDR rules use IP containment (the rule "10.0.0.0/8" matches "10.1.2.3").
43+
// Glob rules use the existing glob matcher. A CIDR rule matches only when
44+
// dest parses as an IP — a hostname like "example.com" can never match a
45+
// CIDR rule even if the rule's CIDR happens to cover the IP that hostname
46+
// resolves to elsewhere, because policy evaluation happens with the
47+
// destination string the SOCKS5 layer received.
48+
func (r compiledRule) matchDestination(dest string) bool {
49+
if r.cidr != nil {
50+
ip := net.ParseIP(dest)
51+
if ip == nil {
52+
return false
53+
}
54+
return r.cidr.Contains(ip)
55+
}
56+
return r.glob != nil && r.glob.Match(dest)
57+
}
58+
3559
// portToProtocol maps well-known ports to protocol names for protocol-scoped
3660
// rule matching. Returns "" for non-standard ports where the protocol is
3761
// ambiguous.
@@ -256,11 +280,6 @@ func compileRules(rules []Rule) ([]compiledRule, error) {
256280
if r.Destination == "" {
257281
return nil, fmt.Errorf("rule has empty destination")
258282
}
259-
dest := canonicalizeDestination(r.Destination)
260-
g, err := CompileGlob(dest)
261-
if err != nil {
262-
return nil, fmt.Errorf("compile rule %q: %w", r.Destination, err)
263-
}
264283
ports := make(map[int]bool, len(r.Ports))
265284
for _, p := range r.Ports {
266285
if p < 1 || p > 65535 {
@@ -272,6 +291,24 @@ func compileRules(rules []Rule) ([]compiledRule, error) {
272291
for _, p := range r.Protocols {
273292
protocols[p] = true
274293
}
294+
// A destination containing "/" is unambiguously CIDR intent.
295+
// Globs and DNS hostnames do not contain forward slashes, so
296+
// the slash is a clean discriminator. Use net.ParseCIDR so
297+
// invalid masks fail loudly at compile time rather than
298+
// silently matching nothing at runtime.
299+
if strings.Contains(r.Destination, "/") {
300+
_, ipnet, err := net.ParseCIDR(r.Destination)
301+
if err != nil {
302+
return nil, fmt.Errorf("rule %q: invalid CIDR: %w", r.Destination, err)
303+
}
304+
out = append(out, compiledRule{cidr: ipnet, ports: ports, protocols: protocols})
305+
continue
306+
}
307+
dest := canonicalizeDestination(r.Destination)
308+
g, err := CompileGlob(dest)
309+
if err != nil {
310+
return nil, fmt.Errorf("compile rule %q: %w", r.Destination, err)
311+
}
275312
out = append(out, compiledRule{glob: g, ports: ports, protocols: protocols})
276313
}
277314
return out, nil
@@ -313,7 +350,7 @@ func matchRules(rules []compiledRule, dest string, port int) bool {
313350
// transport-agnostic rules that TCP matches via matchRulesWithProto.
314351
func matchRulesStrictProto(rules []compiledRule, dest string, port int, proto string) bool {
315352
for _, r := range rules {
316-
if !r.glob.Match(dest) {
353+
if !r.matchDestination(dest) {
317354
continue
318355
}
319356
if len(r.ports) > 0 && !r.ports[port] {
@@ -337,7 +374,7 @@ func matchRulesStrictProto(rules []compiledRule, dest string, port int, proto st
337374
// and is unaffected.
338375
func matchRulesUnscoped(rules []compiledRule, dest string, port int) bool {
339376
for _, r := range rules {
340-
if !r.glob.Match(dest) {
377+
if !r.matchDestination(dest) {
341378
continue
342379
}
343380
if len(r.ports) > 0 && !r.ports[port] {
@@ -361,7 +398,7 @@ func matchRulesUnscoped(rules []compiledRule, dest string, port int) bool {
361398
// TCP-based connection, mirroring how EvaluateUDP/EvaluateQUIC treat "udp".
362399
func matchRulesWithProto(rules []compiledRule, dest string, port int, proto string) bool {
363400
for _, r := range rules {
364-
if !r.glob.Match(dest) {
401+
if !r.matchDestination(dest) {
365402
continue
366403
}
367404
if len(r.ports) > 0 && !r.ports[port] {
@@ -416,7 +453,7 @@ func (e *Engine) IsDeniedDomain(dest string) bool {
416453
return false
417454
}
418455
for _, r := range e.compiled.denyRules {
419-
if r.glob.Match(dest) {
456+
if r.matchDestination(dest) {
420457
return true
421458
}
422459
}
@@ -478,15 +515,15 @@ func (e *Engine) CouldBeAllowed(dest string, includeAsk bool) bool {
478515
// only deny that protocol, so DNS must still be resolved for other
479516
// protocols to work.
480517
for _, r := range e.compiled.denyRules {
481-
if len(r.ports) == 0 && len(r.protocols) == 0 && r.glob.Match(dest) {
518+
if len(r.ports) == 0 && len(r.protocols) == 0 && r.matchDestination(dest) {
482519
return false
483520
}
484521
}
485522

486523
// If any allow rule matches (ignoring ports), the destination
487524
// might be allowed on some port.
488525
for _, r := range e.compiled.allowRules {
489-
if r.glob.Match(dest) {
526+
if r.matchDestination(dest) {
490527
return true
491528
}
492529
}
@@ -495,7 +532,7 @@ func (e *Engine) CouldBeAllowed(dest string, includeAsk bool) bool {
495532
// but only when an approval broker is available.
496533
if includeAsk {
497534
for _, r := range e.compiled.askRules {
498-
if r.glob.Match(dest) {
535+
if r.matchDestination(dest) {
499536
return true
500537
}
501538
}

internal/policy/engine_test.go

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2239,3 +2239,82 @@ func TestMatchSourceString(t *testing.T) {
22392239
t.Errorf("MatchSource(99).String() = %q, want %q", MatchSource(99).String(), "unknown")
22402240
}
22412241
}
2242+
2243+
func TestCompileRules_CIDR(t *testing.T) {
2244+
rules := []Rule{
2245+
{Destination: "192.168.0.0/16", Ports: []int{443}},
2246+
{Destination: "10.0.0.5/32", Ports: []int{443}},
2247+
{Destination: "2001:db8::/32", Ports: []int{443}},
2248+
}
2249+
out, err := compileRules(rules)
2250+
if err != nil {
2251+
t.Fatalf("compile failed: %v", err)
2252+
}
2253+
if len(out) != 3 {
2254+
t.Fatalf("got %d rules, want 3", len(out))
2255+
}
2256+
for i, r := range out {
2257+
if r.cidr == nil {
2258+
t.Errorf("rule %d: cidr is nil", i)
2259+
}
2260+
if r.glob != nil {
2261+
t.Errorf("rule %d: glob should be nil for CIDR rule", i)
2262+
}
2263+
}
2264+
}
2265+
2266+
func TestCompileRules_InvalidCIDRRejected(t *testing.T) {
2267+
rules := []Rule{{Destination: "10.0.0.0/99"}}
2268+
_, err := compileRules(rules)
2269+
if err == nil {
2270+
t.Fatal("expected error on invalid CIDR mask")
2271+
}
2272+
}
2273+
2274+
func TestMatchDestination_CIDRContainment(t *testing.T) {
2275+
rules := []Rule{
2276+
{Destination: "192.168.0.0/16", Ports: []int{443}},
2277+
}
2278+
out, err := compileRules(rules)
2279+
if err != nil {
2280+
t.Fatalf("compile failed: %v", err)
2281+
}
2282+
r := out[0]
2283+
cases := []struct {
2284+
ip string
2285+
want bool
2286+
}{
2287+
{"192.168.1.5", true},
2288+
{"192.168.255.255", true},
2289+
{"192.169.0.1", false},
2290+
{"10.0.0.1", false},
2291+
// Hostname strings never match CIDR rules even if the host
2292+
// would resolve to a covered IP. Policy is evaluated against
2293+
// the destination string the SOCKS5 layer received, and we
2294+
// don't perform implicit DNS at this layer.
2295+
{"example.com", false},
2296+
}
2297+
for _, c := range cases {
2298+
if got := r.matchDestination(c.ip); got != c.want {
2299+
t.Errorf("matchDestination(%q) = %v, want %v", c.ip, got, c.want)
2300+
}
2301+
}
2302+
}
2303+
2304+
func TestEvaluate_CIDRRule(t *testing.T) {
2305+
eng, err := LoadFromBytes([]byte(`
2306+
default = "deny"
2307+
[[allow]]
2308+
destination = "10.0.0.0/8"
2309+
ports = [443]
2310+
`))
2311+
if err != nil {
2312+
t.Fatalf("load failed: %v", err)
2313+
}
2314+
if v := eng.Evaluate("10.5.6.7", 443); v != Allow {
2315+
t.Errorf("10.5.6.7:443 should be Allow, got %v", v)
2316+
}
2317+
if v := eng.Evaluate("11.5.6.7", 443); v != Deny {
2318+
t.Errorf("11.5.6.7:443 should be Deny (default), got %v", v)
2319+
}
2320+
}

internal/proxy/http_host.go

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
package proxy
2+
3+
import (
4+
"bufio"
5+
"bytes"
6+
"io"
7+
"net"
8+
"net/http"
9+
"strings"
10+
)
11+
12+
// peekHTTPHost reads enough bytes from r to parse the HTTP/1.x request line
13+
// and Host header, returning the peeked buffer and the host. Like peekSNI
14+
// but for plain HTTP (e.g. ports 80, 8080). The caller prepends the buffer
15+
// to subsequent reads so the upstream sees the full request.
16+
//
17+
// Returns an empty host when the bytes are not a valid HTTP/1.x request
18+
// (binary protocol, partial data, malformed). In that case the caller should
19+
// fall back to IP-based policy. Reads are bounded by maxBytes to avoid
20+
// hanging on slow clients or very long header sets.
21+
func peekHTTPHost(r io.Reader, maxBytes int) ([]byte, string, error) {
22+
buf := make([]byte, 0, maxBytes)
23+
tmp := make([]byte, 4096)
24+
25+
for len(buf) < maxBytes {
26+
// Cap each read so a single big chunk does not push buf
27+
// past maxBytes.
28+
want := maxBytes - len(buf)
29+
if want > len(tmp) {
30+
want = len(tmp)
31+
}
32+
n, err := r.Read(tmp[:want])
33+
if n > 0 {
34+
buf = append(buf, tmp[:n]...)
35+
}
36+
// Quick reject: HTTP/1.x request lines start with a method like
37+
// GET/POST/HEAD/etc. Method tokens are uppercase ASCII letters.
38+
// If the first byte is not in the [A-Z] range, this is not HTTP.
39+
// Returning early on the first read avoids waiting maxBytes worth
40+
// of data for a binary protocol that happens to be on port 80.
41+
if len(buf) >= 1 && (buf[0] < 'A' || buf[0] > 'Z') {
42+
return buf, "", nil
43+
}
44+
// Look for end of headers. http.ReadRequest needs the full header
45+
// section before it returns; calling it on partial data yields
46+
// io.ErrUnexpectedEOF, which we treat as "keep reading".
47+
if idx := bytes.Index(buf, []byte("\r\n\r\n")); idx >= 0 {
48+
host, ok := extractHTTPHost(buf[:idx+4])
49+
if ok {
50+
return buf, host, nil
51+
}
52+
return buf, "", nil
53+
}
54+
if err != nil {
55+
if len(buf) > 0 {
56+
return buf, "", nil
57+
}
58+
return nil, "", err
59+
}
60+
}
61+
return buf, "", nil
62+
}
63+
64+
// extractHTTPHost parses an HTTP/1.x request prefix terminated by \r\n\r\n
65+
// and returns the Host header value with any port stripped. The fast-path
66+
// uses net/http's parser, which handles obs-fold, mixed case, multiple
67+
// Host header rules, and request-line validation in one pass.
68+
func extractHTTPHost(prefix []byte) (string, bool) {
69+
req, err := http.ReadRequest(bufio.NewReader(bytes.NewReader(prefix)))
70+
if err != nil {
71+
return "", false
72+
}
73+
host := req.Host
74+
if host == "" {
75+
host = req.Header.Get("Host")
76+
}
77+
host = strings.TrimSpace(host)
78+
if host == "" {
79+
return "", false
80+
}
81+
// net.SplitHostPort handles every shape Host can legitimately
82+
// take: "example.com:80" -> ("example.com", "80"),
83+
// "[::1]:80" -> ("::1", "80"), "[::1]" with no port -> error,
84+
// and bare IPv6 like "2001:db8::1" -> error ("too many colons").
85+
// Falling back to the trimmed host on error avoids the previous
86+
// LastIndex(":") approach that mishandled bare IPv6 by stripping
87+
// the final hextet as if it were a port.
88+
if h, _, err := net.SplitHostPort(host); err == nil {
89+
host = h
90+
}
91+
host = strings.TrimPrefix(host, "[")
92+
host = strings.TrimSuffix(host, "]")
93+
if host == "" {
94+
return "", false
95+
}
96+
return host, true
97+
}

0 commit comments

Comments
 (0)