From 36ba46a4bd341fb9e845802f425c3e3c2ea3ba83 Mon Sep 17 00:00:00 2001 From: Nikita Nemirovsky Date: Fri, 8 May 2026 20:12:40 +0800 Subject: [PATCH 1/6] 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. --- CLAUDE.md | 4 + internal/policy/engine.go | 61 +++++++++++--- internal/policy/engine_test.go | 79 +++++++++++++++++ internal/proxy/http_host.go | 95 +++++++++++++++++++++ internal/proxy/http_host_test.go | 123 +++++++++++++++++++++++++++ internal/proxy/server.go | 140 +++++++++++++++++++++++++++++++ 6 files changed, 490 insertions(+), 12 deletions(-) create mode 100644 internal/proxy/http_host.go create mode 100644 internal/proxy/http_host_test.go diff --git a/CLAUDE.md b/CLAUDE.md index bf8817f..1099f72 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -274,6 +274,10 @@ See `internal/proxy/request_policy.go`, `internal/policy/engine.go` (`EvaluateDe `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()`. +**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). + +**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. + **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. ### Protocol detection diff --git a/internal/policy/engine.go b/internal/policy/engine.go index a146ac8..d67b346 100644 --- a/internal/policy/engine.go +++ b/internal/policy/engine.go @@ -27,11 +27,35 @@ func isUDPFamilyProto(proto string) bool { } type compiledRule struct { + // Exactly one of glob or cidr is set per rule. cidr is set when + // the rule's destination is a CIDR like "192.168.1.0/24" or a + // bare IP-with-mask like "10.0.0.1/32"; glob is set for hostname + // patterns and bare IPs without a mask. The matchDestination + // method dispatches on which is non-nil. glob *Glob + cidr *net.IPNet ports map[int]bool protocols map[string]bool } +// matchDestination reports whether dest matches this rule's destination. +// CIDR rules use IP containment (the rule "10.0.0.0/8" matches "10.1.2.3"). +// Glob rules use the existing glob matcher. A CIDR rule matches only when +// dest parses as an IP — a hostname like "example.com" can never match a +// CIDR rule even if the rule's CIDR happens to cover the IP that hostname +// resolves to elsewhere, because policy evaluation happens with the +// destination string the SOCKS5 layer received. +func (r compiledRule) matchDestination(dest string) bool { + if r.cidr != nil { + ip := net.ParseIP(dest) + if ip == nil { + return false + } + return r.cidr.Contains(ip) + } + return r.glob != nil && r.glob.Match(dest) +} + // portToProtocol maps well-known ports to protocol names for protocol-scoped // rule matching. Returns "" for non-standard ports where the protocol is // ambiguous. @@ -256,11 +280,6 @@ func compileRules(rules []Rule) ([]compiledRule, error) { if r.Destination == "" { return nil, fmt.Errorf("rule has empty destination") } - dest := canonicalizeDestination(r.Destination) - g, err := CompileGlob(dest) - if err != nil { - return nil, fmt.Errorf("compile rule %q: %w", r.Destination, err) - } ports := make(map[int]bool, len(r.Ports)) for _, p := range r.Ports { if p < 1 || p > 65535 { @@ -272,6 +291,24 @@ func compileRules(rules []Rule) ([]compiledRule, error) { for _, p := range r.Protocols { protocols[p] = true } + // A destination containing "/" is unambiguously CIDR intent. + // Globs and DNS hostnames do not contain forward slashes, so + // the slash is a clean discriminator. Use net.ParseCIDR so + // invalid masks fail loudly at compile time rather than + // silently matching nothing at runtime. + if strings.Contains(r.Destination, "/") { + _, ipnet, err := net.ParseCIDR(r.Destination) + if err != nil { + return nil, fmt.Errorf("rule %q: invalid CIDR: %w", r.Destination, err) + } + out = append(out, compiledRule{cidr: ipnet, ports: ports, protocols: protocols}) + continue + } + dest := canonicalizeDestination(r.Destination) + g, err := CompileGlob(dest) + if err != nil { + return nil, fmt.Errorf("compile rule %q: %w", r.Destination, err) + } out = append(out, compiledRule{glob: g, ports: ports, protocols: protocols}) } return out, nil @@ -313,7 +350,7 @@ func matchRules(rules []compiledRule, dest string, port int) bool { // transport-agnostic rules that TCP matches via matchRulesWithProto. func matchRulesStrictProto(rules []compiledRule, dest string, port int, proto string) bool { for _, r := range rules { - if !r.glob.Match(dest) { + if !r.matchDestination(dest) { continue } if len(r.ports) > 0 && !r.ports[port] { @@ -337,7 +374,7 @@ func matchRulesStrictProto(rules []compiledRule, dest string, port int, proto st // and is unaffected. func matchRulesUnscoped(rules []compiledRule, dest string, port int) bool { for _, r := range rules { - if !r.glob.Match(dest) { + if !r.matchDestination(dest) { continue } if len(r.ports) > 0 && !r.ports[port] { @@ -361,7 +398,7 @@ func matchRulesUnscoped(rules []compiledRule, dest string, port int) bool { // TCP-based connection, mirroring how EvaluateUDP/EvaluateQUIC treat "udp". func matchRulesWithProto(rules []compiledRule, dest string, port int, proto string) bool { for _, r := range rules { - if !r.glob.Match(dest) { + if !r.matchDestination(dest) { continue } if len(r.ports) > 0 && !r.ports[port] { @@ -416,7 +453,7 @@ func (e *Engine) IsDeniedDomain(dest string) bool { return false } for _, r := range e.compiled.denyRules { - if r.glob.Match(dest) { + if r.matchDestination(dest) { return true } } @@ -478,7 +515,7 @@ func (e *Engine) CouldBeAllowed(dest string, includeAsk bool) bool { // only deny that protocol, so DNS must still be resolved for other // protocols to work. for _, r := range e.compiled.denyRules { - if len(r.ports) == 0 && len(r.protocols) == 0 && r.glob.Match(dest) { + if len(r.ports) == 0 && len(r.protocols) == 0 && r.matchDestination(dest) { return false } } @@ -486,7 +523,7 @@ func (e *Engine) CouldBeAllowed(dest string, includeAsk bool) bool { // If any allow rule matches (ignoring ports), the destination // might be allowed on some port. for _, r := range e.compiled.allowRules { - if r.glob.Match(dest) { + if r.matchDestination(dest) { return true } } @@ -495,7 +532,7 @@ func (e *Engine) CouldBeAllowed(dest string, includeAsk bool) bool { // but only when an approval broker is available. if includeAsk { for _, r := range e.compiled.askRules { - if r.glob.Match(dest) { + if r.matchDestination(dest) { return true } } diff --git a/internal/policy/engine_test.go b/internal/policy/engine_test.go index 18acca8..9db832f 100644 --- a/internal/policy/engine_test.go +++ b/internal/policy/engine_test.go @@ -2239,3 +2239,82 @@ func TestMatchSourceString(t *testing.T) { t.Errorf("MatchSource(99).String() = %q, want %q", MatchSource(99).String(), "unknown") } } + +func TestCompileRules_CIDR(t *testing.T) { + rules := []Rule{ + {Destination: "192.168.0.0/16", Ports: []int{443}}, + {Destination: "10.0.0.5/32", Ports: []int{443}}, + {Destination: "2001:db8::/32", Ports: []int{443}}, + } + out, err := compileRules(rules) + if err != nil { + t.Fatalf("compile failed: %v", err) + } + if len(out) != 3 { + t.Fatalf("got %d rules, want 3", len(out)) + } + for i, r := range out { + if r.cidr == nil { + t.Errorf("rule %d: cidr is nil", i) + } + if r.glob != nil { + t.Errorf("rule %d: glob should be nil for CIDR rule", i) + } + } +} + +func TestCompileRules_InvalidCIDRRejected(t *testing.T) { + rules := []Rule{{Destination: "10.0.0.0/99"}} + _, err := compileRules(rules) + if err == nil { + t.Fatal("expected error on invalid CIDR mask") + } +} + +func TestMatchDestination_CIDRContainment(t *testing.T) { + rules := []Rule{ + {Destination: "192.168.0.0/16", Ports: []int{443}}, + } + out, err := compileRules(rules) + if err != nil { + t.Fatalf("compile failed: %v", err) + } + r := out[0] + cases := []struct { + ip string + want bool + }{ + {"192.168.1.5", true}, + {"192.168.255.255", true}, + {"192.169.0.1", false}, + {"10.0.0.1", false}, + // Hostname strings never match CIDR rules even if the host + // would resolve to a covered IP. Policy is evaluated against + // the destination string the SOCKS5 layer received, and we + // don't perform implicit DNS at this layer. + {"example.com", false}, + } + for _, c := range cases { + if got := r.matchDestination(c.ip); got != c.want { + t.Errorf("matchDestination(%q) = %v, want %v", c.ip, got, c.want) + } + } +} + +func TestEvaluate_CIDRRule(t *testing.T) { + eng, err := LoadFromBytes([]byte(` +default = "deny" +[[allow]] +destination = "10.0.0.0/8" +ports = [443] +`)) + if err != nil { + t.Fatalf("load failed: %v", err) + } + if v := eng.Evaluate("10.5.6.7", 443); v != Allow { + t.Errorf("10.5.6.7:443 should be Allow, got %v", v) + } + if v := eng.Evaluate("11.5.6.7", 443); v != Deny { + t.Errorf("11.5.6.7:443 should be Deny (default), got %v", v) + } +} diff --git a/internal/proxy/http_host.go b/internal/proxy/http_host.go new file mode 100644 index 0000000..944db7b --- /dev/null +++ b/internal/proxy/http_host.go @@ -0,0 +1,95 @@ +package proxy + +import ( + "bufio" + "bytes" + "io" + "net/http" + "strings" +) + +// peekHTTPHost reads enough bytes from r to parse the HTTP/1.x request line +// and Host header, returning the peeked buffer and the host. Like peekSNI +// but for plain HTTP on port 80. The caller prepends the buffer to subsequent +// reads so the upstream sees the full request. +// +// Returns an empty host when the bytes are not a valid HTTP/1.x request +// (binary protocol, partial data, malformed). In that case the caller should +// fall back to IP-based policy. Reads are bounded by maxBytes to avoid +// hanging on slow clients or very long header sets. +func peekHTTPHost(r io.Reader, maxBytes int) ([]byte, string, error) { + buf := make([]byte, 0, maxBytes) + tmp := make([]byte, 4096) + + for len(buf) < maxBytes { + // Cap each read so a single big chunk does not push buf + // past maxBytes. + want := maxBytes - len(buf) + if want > len(tmp) { + want = len(tmp) + } + n, err := r.Read(tmp[:want]) + if n > 0 { + buf = append(buf, tmp[:n]...) + } + // Quick reject: HTTP/1.x request lines start with a method like + // GET/POST/HEAD/etc. Method tokens are uppercase ASCII letters. + // If the first byte is not in the [A-Z] range, this is not HTTP. + // Returning early on the first read avoids waiting maxBytes worth + // of data for a binary protocol that happens to be on port 80. + if len(buf) >= 1 && (buf[0] < 'A' || buf[0] > 'Z') { + return buf, "", nil + } + // Look for end of headers. http.ReadRequest needs the full header + // section before it returns; calling it on partial data yields + // io.ErrUnexpectedEOF, which we treat as "keep reading". + if idx := bytes.Index(buf, []byte("\r\n\r\n")); idx >= 0 { + host, ok := extractHTTPHost(buf[:idx+4]) + if ok { + return buf, host, nil + } + return buf, "", nil + } + if err != nil { + if len(buf) > 0 { + return buf, "", nil + } + return nil, "", err + } + } + return buf, "", nil +} + +// extractHTTPHost parses an HTTP/1.x request prefix terminated by \r\n\r\n +// and returns the Host header value with any port stripped. The fast-path +// uses net/http's parser, which handles obs-fold, mixed case, multiple +// Host header rules, and request-line validation in one pass. +func extractHTTPHost(prefix []byte) (string, bool) { + req, err := http.ReadRequest(bufio.NewReader(bytes.NewReader(prefix))) + if err != nil { + return "", false + } + host := req.Host + if host == "" { + host = req.Header.Get("Host") + } + host = strings.TrimSpace(host) + if host == "" { + return "", false + } + // Strip port if present. IPv6 hosts in Host headers appear as + // "[::1]:80" so only strip the trailing :port when there is no + // closing bracket after the last colon. + if i := strings.LastIndex(host, ":"); i >= 0 { + if !strings.Contains(host[i:], "]") { + host = host[:i] + } + } + // IPv6 hosts may still be wrapped in [] — strip those. + host = strings.TrimPrefix(host, "[") + host = strings.TrimSuffix(host, "]") + if host == "" { + return "", false + } + return host, true +} diff --git a/internal/proxy/http_host_test.go b/internal/proxy/http_host_test.go new file mode 100644 index 0000000..02a0323 --- /dev/null +++ b/internal/proxy/http_host_test.go @@ -0,0 +1,123 @@ +package proxy + +import ( + "bytes" + "errors" + "io" + "strings" + "testing" +) + +func TestExtractHTTPHost_BasicGet(t *testing.T) { + prefix := []byte("GET /derp/probe HTTP/1.1\r\nHost: derp10b.tailscale.com\r\nUser-Agent: tailscale\r\n\r\n") + host, ok := extractHTTPHost(prefix) + if !ok { + t.Fatal("expected ok=true") + } + if host != "derp10b.tailscale.com" { + t.Errorf("got %q, want derp10b.tailscale.com", host) + } +} + +func TestExtractHTTPHost_StripsPort(t *testing.T) { + prefix := []byte("GET / HTTP/1.1\r\nHost: example.com:8080\r\n\r\n") + host, ok := extractHTTPHost(prefix) + if !ok || host != "example.com" { + t.Errorf("got %q ok=%v, want example.com ok=true", host, ok) + } +} + +func TestExtractHTTPHost_IPv6WithPort(t *testing.T) { + // IPv6 in Host header: [::1]:80. Should strip the :80, leave ::1 (no brackets). + prefix := []byte("GET / HTTP/1.1\r\nHost: [::1]:80\r\n\r\n") + host, ok := extractHTTPHost(prefix) + if !ok || host != "::1" { + t.Errorf("got %q ok=%v, want ::1 ok=true", host, ok) + } +} + +func TestExtractHTTPHost_MissingHost(t *testing.T) { + // HTTP/1.0 allowed missing Host. Should return ok=false rather than + // silently allowing an empty hostname through to policy. + prefix := []byte("GET / HTTP/1.0\r\n\r\n") + host, ok := extractHTTPHost(prefix) + if ok { + t.Errorf("got %q ok=%v, want ok=false", host, ok) + } +} + +func TestExtractHTTPHost_BinaryGarbage(t *testing.T) { + // Random bytes that happen to start with 'G' but are not HTTP. + // The parser should reject and we fall back to IP-based policy. + prefix := []byte{'G', 0x00, 0xff, 0x10, '\r', '\n', '\r', '\n'} + host, ok := extractHTTPHost(prefix) + if ok { + t.Errorf("got %q ok=%v, want ok=false on garbage", host, ok) + } +} + +func TestPeekHTTPHost_FullRequest(t *testing.T) { + body := "GET /probe HTTP/1.1\r\nHost: derp.tailscale.com\r\nAccept: */*\r\n\r\n" + r := strings.NewReader(body) + buf, host, err := peekHTTPHost(r, 4096) + if err != nil { + t.Fatalf("err: %v", err) + } + if host != "derp.tailscale.com" { + t.Errorf("got host %q", host) + } + if !bytes.Equal(buf, []byte(body)) { + t.Errorf("buf should preserve all read bytes; got %d bytes", len(buf)) + } +} + +func TestPeekHTTPHost_NotHTTP(t *testing.T) { + // First byte is 0x16 (TLS handshake) — should bail out fast with + // empty host, returning the buffered bytes for replay. + r := bytes.NewReader([]byte{0x16, 0x03, 0x01, 0x00, 0x42}) + buf, host, err := peekHTTPHost(r, 4096) + if err != nil { + t.Fatalf("err: %v", err) + } + if host != "" { + t.Errorf("expected empty host on non-HTTP, got %q", host) + } + if len(buf) == 0 { + t.Errorf("expected peeked bytes to be returned for replay") + } +} + +func TestPeekHTTPHost_Truncated(t *testing.T) { + // Headers never terminate. peek should return empty host without + // hanging once it hits EOF, with the buffered bytes for replay. + body := "GET / HTTP/1.1\r\nHost: example.com\r\n" + r := strings.NewReader(body) + buf, host, err := peekHTTPHost(r, 4096) + if err != nil && !errors.Is(err, io.EOF) { + t.Fatalf("unexpected err: %v", err) + } + if host != "" { + t.Errorf("expected empty host on truncated headers, got %q", host) + } + if len(buf) == 0 { + t.Errorf("expected buffered bytes for replay") + } +} + +func TestPeekHTTPHost_RespectsMaxBytes(t *testing.T) { + // Long Host header value that exceeds maxBytes triggers the cap. + // peek must return without blocking even if no \r\n\r\n is found. + long := strings.Repeat("X", 8192) + body := "GET / HTTP/1.1\r\nHost: " + long + r := strings.NewReader(body) + buf, host, err := peekHTTPHost(r, 1024) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if host != "" { + t.Errorf("expected empty host when headers exceed cap, got %q", host) + } + if len(buf) > 1024 { + t.Errorf("buffer exceeded maxBytes: %d", len(buf)) + } +} diff --git a/internal/proxy/server.go b/internal/proxy/server.go index 2ded07a..3d9cefd 100644 --- a/internal/proxy/server.go +++ b/internal/proxy/server.go @@ -108,6 +108,7 @@ const ( ctxKeyFallbackAddrs contextKey = "fallbackAddrs" ctxKeyFQDN contextKey = "fqdn" ctxKeySNIDeferred contextKey = "sniDeferred" // true when policy check deferred for SNI peeking + ctxKeyHTTPHostDeferred contextKey = "httpHostDeferred" // true when policy check deferred for HTTP Host peeking ctxKeyPerRequestPolicy contextKey = "perRequestPolicy" // *RequestPolicyChecker for per-HTTP-request policy checks ctxKeySkipPerRequest contextKey = "skipPerRequest" // true when connection matched an explicit allow rule ) @@ -138,6 +139,20 @@ func isTLSPort(port int) bool { } } +// isPlainHTTPPort returns true for ports that typically carry plain +// (non-TLS) HTTP/1.x traffic. Used to enable Host-header peeking on +// SOCKS5 CONNECT requests that arrive with a bare IP — without the +// peek, hostname-based allow rules cannot match the connection and +// the operator gets a flood of IP-based Telegram approval prompts. +func isPlainHTTPPort(port int) bool { + switch port { + case 80, 8080: + return true + default: + return false + } +} + // policyResolver performs DNS resolution only for destinations that could be // allowed by policy. Definitely-denied destinations return nil IP (no DNS // lookup) to prevent leaks. For potentially-allowed destinations, DNS failures @@ -312,6 +327,23 @@ func (r *policyRuleSet) Allow(ctx context.Context, req *socks5.Request) (context return ctx, true } + // HTTP Host deferral: same idea as SNI deferral but for plain HTTP. + // When the SOCKS5 layer received a bare IP and a hostname-based rule + // could plausibly match, defer the policy check until the connect + // handler can peek the request's Host header. Without this, every + // new IP behind a hostname rule (e.g. tailscale's DERP latency + // probes hitting dozens of derp[N].tailscale.com IPs) generates an + // approval prompt that can't be silenced short of allowing each IP. + if ipOnly && verdict != policy.Allow && verdict != policy.Deny && isPlainHTTPPort(port) { + log.Printf("[HTTP-HOST-DEFER] %s:%d (deferring policy for Host header peek)", dest, port) + proto := DetectProtocol(port) + ctx = context.WithValue(ctx, ctxKeyProtocol, proto) + ctx = context.WithValue(ctx, ctxKeyFQDN, dest) + ctx = context.WithValue(ctx, ctxKeyHTTPHostDeferred, true) + ctx = context.WithValue(ctx, ctxKeyEngine, eng) + return ctx, true + } + // Determine the effective outcome. allowed := false effectiveVerdict := verdict @@ -1342,6 +1374,39 @@ func (s *Server) handleConnect(ctx context.Context, writer io.Writer, request *s return s.relayData(clientReader, writer, target) } + // HTTP Host-deferred connections: peek the request's Host header + // (mirrors the SNI flow above but for plaintext HTTP on port 80). + // Hostname recovery here lets `*.tailscale.com:80` rules match the + // dozens of bare-IP DERP probes that would otherwise spam the + // approval channel. + if deferred, _ := ctx.Value(ctxKeyHTTPHostDeferred).(bool); deferred { + bindAddr := &net.TCPAddr{IP: request.DestAddr.IP, Port: request.DestAddr.Port} + if sendErr := socks5.SendReply(writer, statute.RepSuccess, bindAddr); sendErr != nil { + return fmt.Errorf("failed to send reply: %w", sendErr) + } + if conn, ok := writer.(net.Conn); ok { + conn.SetReadDeadline(time.Now().Add(10 * time.Second)) //nolint:errcheck + } + + var allow bool + clientReader, ctx, allow = s.httpHostPolicyCheckBeforeDial(ctx, request) + if !allow { + return nil + } + + if conn, ok := writer.(net.Conn); ok { + conn.SetReadDeadline(time.Time{}) //nolint:errcheck + } + + target, err := s.dial(ctx, "tcp", request.DestAddr.String()) + if err != nil { + return fmt.Errorf("connect to %v failed: %w", request.RawDestAddr, err) + } + defer target.Close() //nolint:errcheck + + return s.relayData(clientReader, writer, target) + } + // Normal (non-deferred) path: dial first, then relay. target, err := s.dial(ctx, "tcp", request.DestAddr.String()) if err != nil { @@ -1496,6 +1561,81 @@ func (s *Server) sniPolicyCheckBeforeDial(ctx context.Context, request *socks5.R } } +// httpHostPolicyCheckBeforeDial peeks the first bytes from the client to +// extract the HTTP/1.x Host header, re-evaluates policy with the recovered +// hostname, and updates the context FQDN so the dial uses the hostname for +// upstream selection. Mirrors sniPolicyCheckBeforeDial; the only differences +// are which peek function runs and which protocol's "host pinning" semantic +// applies to the recovered name. +func (s *Server) httpHostPolicyCheckBeforeDial(ctx context.Context, request *socks5.Request) (io.Reader, context.Context, bool) { + buf, host, err := peekHTTPHost(request.Reader, 8192) + if err != nil || host == "" { + hexPrefix := "" + if len(buf) >= 6 { + hexPrefix = fmt.Sprintf(" first6=%02x", buf[:6]) + } + log.Printf("[HTTP-HOST-PEEK] no Host extracted (err=%v, bufLen=%d, host=%q%s)", err, len(buf), host, hexPrefix) + if len(buf) > 0 { + return io.MultiReader(bytes.NewReader(buf), request.Reader), ctx, true + } + return request.Reader, ctx, true + } + + host = strings.TrimRight(host, ".") + dest := request.DestAddr.String() + ipStr := strings.Split(dest, ":")[0] + port := request.DestAddr.Port + + log.Printf("[HTTP-HOST] %s -> %s:%d (recovered hostname via HTTP Host header)", ipStr, host, port) + + ctx = context.WithValue(ctx, ctxKeyFQDN, host) + if s.dnsInterceptor != nil { + s.dnsInterceptor.StoreReverse(ipStr, host) + } + + eng, _ := ctx.Value(ctxKeyEngine).(*policy.Engine) + if eng == nil { + eng = s.rules.engine.Load() + } + verdict, matchSource := eng.EvaluateDetailed(host, port) + reader := io.MultiReader(bytes.NewReader(buf), request.Reader) + + switch verdict { + case policy.Allow: + log.Printf("[HTTP-HOST->ALLOW] %s:%d (hostname %s matched allow rule)", ipStr, port, host) + if matchSource == policy.RuleMatch { + ctx = context.WithValue(ctx, ctxKeySkipPerRequest, true) + } else if s.rules.broker == nil { + ctx = context.WithValue(ctx, ctxKeySkipPerRequest, true) + } else { + checker := NewRequestPolicyChecker( + s.rules.engine, s.rules.broker, + WithPersist(s.rules.buildPersistFunc()), + ) + ctx = context.WithValue(ctx, ctxKeyPerRequestPolicy, checker) + } + return reader, ctx, true + case policy.Deny: + log.Printf("[HTTP-HOST->DENY] %s:%d (hostname %s matched deny rule)", ipStr, port, host) + return nil, ctx, false + case policy.Ask: + if s.rules.broker == nil { + log.Printf("[HTTP-HOST->DENY] %s:%d (hostname %s: ask treated as deny, no broker)", ipStr, port, host) + return nil, ctx, false + } + log.Printf("[HTTP-HOST->DEFER] %s:%d (hostname %s: approval deferred to per-request)", ipStr, port, host) + checker := NewRequestPolicyChecker( + s.rules.engine, s.rules.broker, + WithPersist(s.rules.buildPersistFunc()), + ) + ctx = context.WithValue(ctx, ctxKeyPerRequestPolicy, checker) + return reader, ctx, true + default: + log.Printf("[HTTP-HOST->DENY] %s:%d (hostname %s: default deny)", ipStr, port, host) + return nil, ctx, false + } +} + // then dispatches datagrams to the DNSInterceptor (port 53) or UDPRelay // (all other ports). The handler blocks until the TCP control connection // closes, at which point all UDP sessions are cleaned up. From e9be0331f2f1b54c9fed18c82a3e30792cf8ddf4 Mon Sep 17 00:00:00 2001 From: Nikita Nemirovsky Date: Fri, 8 May 2026 20:39:16 +0800 Subject: [PATCH 2/6] fix(proxy): copilot review round 1 on PR #39 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- internal/proxy/http_host.go | 4 ++-- internal/proxy/server.go | 23 +++++++++++++++++------ 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/internal/proxy/http_host.go b/internal/proxy/http_host.go index 944db7b..c936a1f 100644 --- a/internal/proxy/http_host.go +++ b/internal/proxy/http_host.go @@ -10,8 +10,8 @@ import ( // peekHTTPHost reads enough bytes from r to parse the HTTP/1.x request line // and Host header, returning the peeked buffer and the host. Like peekSNI -// but for plain HTTP on port 80. The caller prepends the buffer to subsequent -// reads so the upstream sees the full request. +// but for plain HTTP (e.g. ports 80, 8080). The caller prepends the buffer +// to subsequent reads so the upstream sees the full request. // // Returns an empty host when the bytes are not a valid HTTP/1.x request // (binary protocol, partial data, malformed). In that case the caller should diff --git a/internal/proxy/server.go b/internal/proxy/server.go index 3d9cefd..c04696e 100644 --- a/internal/proxy/server.go +++ b/internal/proxy/server.go @@ -141,9 +141,10 @@ func isTLSPort(port int) bool { // isPlainHTTPPort returns true for ports that typically carry plain // (non-TLS) HTTP/1.x traffic. Used to enable Host-header peeking on -// SOCKS5 CONNECT requests that arrive with a bare IP — without the +// SOCKS5 CONNECT requests that arrive with a bare IP. Without the // peek, hostname-based allow rules cannot match the connection and -// the operator gets a flood of IP-based Telegram approval prompts. +// the policy engine resolves to its default verdict (often Ask) on +// every distinct IP behind a hostname rule. func isPlainHTTPPort(port int) bool { switch port { case 80, 8080: @@ -1500,8 +1501,13 @@ func (s *Server) sniPolicyCheckBeforeDial(ctx context.Context, request *socks5.R } sni = strings.TrimRight(sni, ".") - dest := request.DestAddr.String() - ipStr := strings.Split(dest, ":")[0] + // request.DestAddr.IP is the parsed net.IP, so .String() yields a + // clean address-only form without the trailing port. The previous + // strings.Split(dest, ":") approach mishandled IPv6 destinations: + // request.DestAddr.String() emits IPv6 as "[::1]:80", and splitting + // on ":" yields "[" or partial values that corrupt logs and the + // reverse-DNS cache key. + ipStr := request.DestAddr.IP.String() port := request.DestAddr.Port log.Printf("[SNI] %s -> %s:%d (recovered hostname via TLS ClientHello)", ipStr, sni, port) @@ -1582,8 +1588,13 @@ func (s *Server) httpHostPolicyCheckBeforeDial(ctx context.Context, request *soc } host = strings.TrimRight(host, ".") - dest := request.DestAddr.String() - ipStr := strings.Split(dest, ":")[0] + // request.DestAddr.IP is the parsed net.IP, so .String() yields a + // clean address-only form without the trailing port. The previous + // strings.Split(dest, ":") approach mishandled IPv6 destinations: + // request.DestAddr.String() emits IPv6 as "[::1]:80", and splitting + // on ":" yields "[" or partial values that corrupt logs and the + // reverse-DNS cache key. + ipStr := request.DestAddr.IP.String() port := request.DestAddr.Port log.Printf("[HTTP-HOST] %s -> %s:%d (recovered hostname via HTTP Host header)", ipStr, host, port) From 424d19c77571e94db6fe1fab532afb113d6d741f Mon Sep 17 00:00:00 2001 From: Nikita Nemirovsky Date: Fri, 8 May 2026 20:49:36 +0800 Subject: [PATCH 3/6] 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: 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. --- internal/proxy/http_host_test.go | 40 +++++++++++++++++++++ internal/proxy/server.go | 61 ++++++++++++++++++++++++++++++++ 2 files changed, 101 insertions(+) diff --git a/internal/proxy/http_host_test.go b/internal/proxy/http_host_test.go index 02a0323..682a894 100644 --- a/internal/proxy/http_host_test.go +++ b/internal/proxy/http_host_test.go @@ -2,8 +2,10 @@ package proxy import ( "bytes" + "context" "errors" "io" + "net" "strings" "testing" ) @@ -121,3 +123,41 @@ func TestPeekHTTPHost_RespectsMaxBytes(t *testing.T) { t.Errorf("buffer exceeded maxBytes: %d", len(buf)) } } + +func TestHostResolvesToIP_ReverseCacheAttestation(t *testing.T) { + // Server with a DNS interceptor whose reverse cache has been + // populated for derp.tailscale.com -> 192.0.2.10. A subsequent + // HTTP host-peek for that exact pair should return true without + // hitting the resolver. + di := NewDNSInterceptor(nil, nil, "") + di.StoreReverse("192.0.2.10", "derp.tailscale.com") + s := &Server{dnsInterceptor: di} + if !s.hostResolvesToIP(context.Background(), "derp.tailscale.com", net.ParseIP("192.0.2.10")) { + t.Fatal("attested cache hit should be accepted") + } +} + +func TestHostResolvesToIP_ReverseCacheDifferentHost(t *testing.T) { + // Cache says 192.0.2.10 -> attacker.example.com. A spoof attempt + // claiming Host: bank.example.com on the same IP must NOT be + // accepted off the cache. + di := NewDNSInterceptor(nil, nil, "") + di.StoreReverse("192.0.2.10", "attacker.example.com") + s := &Server{dnsInterceptor: di} + // Lookup will fail or return something that does not match + // 192.0.2.10 (the literal IP is unlikely to be a registered + // hostname). Either way the cache must NOT attest the spoof. + if s.hostResolvesToIP(context.Background(), "bank.example.com", net.ParseIP("192.0.2.10")) { + t.Fatal("cache hit for a different host must not attest a different-Host claim") + } +} + +func TestHostResolvesToIP_NilInputs(t *testing.T) { + s := &Server{} + if s.hostResolvesToIP(context.Background(), "", net.ParseIP("1.2.3.4")) { + t.Error("empty host must not be considered attested") + } + if s.hostResolvesToIP(context.Background(), "example.com", nil) { + t.Error("nil dest IP must not be considered attested") + } +} diff --git a/internal/proxy/server.go b/internal/proxy/server.go index c04696e..4417082 100644 --- a/internal/proxy/server.go +++ b/internal/proxy/server.go @@ -1597,6 +1597,26 @@ func (s *Server) httpHostPolicyCheckBeforeDial(ctx context.Context, request *soc ipStr := request.DestAddr.IP.String() port := request.DestAddr.Port + // Spoofing guard. The Host header is client-controlled and not + // cryptographically bound to the destination IP. With TLS, an + // SNI/cert mismatch fails the upstream handshake and the bypass + // attempt dies on its own; for plain HTTP there is no equivalent + // integrity check, so an agent could connect to an arbitrary IP + // and claim Host: to slip past hostname-based + // policy. Forward-resolve the recovered host and require the + // dial target to be one of the resolved IPs before trusting the + // Host. On confirmed mismatch (DNS resolved but the dest IP is + // not in the result set) deny outright — surfacing this as Ask + // in the broker would just channel the spoofed hostname back to + // the operator, which is exactly the manipulation the attacker + // wanted. DNS lookup failure is also treated as deny because + // sluice cannot distinguish a transient resolver hiccup from a + // poisoned resolver, and the safer default is the strict one. + if !s.hostResolvesToIP(ctx, host, request.DestAddr.IP) { + log.Printf("[HTTP-HOST->DENY] %s:%d (Host %q does not resolve to %s; possible spoofing)", ipStr, port, host, ipStr) + return nil, ctx, false + } + log.Printf("[HTTP-HOST] %s -> %s:%d (recovered hostname via HTTP Host header)", ipStr, host, port) ctx = context.WithValue(ctx, ctxKeyFQDN, host) @@ -1647,6 +1667,47 @@ func (s *Server) httpHostPolicyCheckBeforeDial(ctx context.Context, request *soc } } +// hostResolvesToIP reports whether forward-resolving host yields the given +// destination IP. Used by the HTTP Host peek path to defeat Host-spoofing +// attempts where an agent connects to an arbitrary IP and claims a +// permitted hostname in the request. Returns false on lookup error so +// the caller treats unverifiable Host claims as suspect rather than +// trusting the client. +// +// The lookup is bounded by hostResolveTimeout. The first DNS interceptor +// reverse cache hit short-circuits the lookup: if sluice's own DNS layer +// previously saw a query for host that returned dest, the binding is +// already attested and we do not need to repeat the resolve. +func (s *Server) hostResolvesToIP(ctx context.Context, host string, dest net.IP) bool { + if dest == nil || host == "" { + return false + } + // If sluice's DNS interceptor saw a prior query for this host that + // returned dest, the binding is already attested by an actual DNS + // answer the agent received and we do not need to re-resolve. + // The reverse cache stores IP -> hostname, so a hit on dest equals + // host means the agent itself just resolved host -> dest. + if s.dnsInterceptor != nil { + if cached := s.dnsInterceptor.ReverseLookup(dest.String()); cached != "" && strings.EqualFold(strings.TrimRight(cached, "."), host) { + return true + } + } + lookupCtx, cancel := context.WithTimeout(ctx, hostResolveTimeout) + defer cancel() + ips, err := net.DefaultResolver.LookupIP(lookupCtx, "ip", host) + if err != nil { + return false + } + for _, ip := range ips { + if ip.Equal(dest) { + return true + } + } + return false +} + +const hostResolveTimeout = 2 * time.Second + // then dispatches datagrams to the DNSInterceptor (port 53) or UDPRelay // (all other ports). The handler blocks until the TCP control connection // closes, at which point all UDP sessions are cleaned up. From 191cba11464170cf79c4a3c851f966c5432206f2 Mon Sep 17 00:00:00 2001 From: Nikita Nemirovsky Date: Fri, 8 May 2026 21:03:15 +0800 Subject: [PATCH 4/6] 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. --- internal/proxy/http_host_test.go | 95 ++++++++++++++++++++------ internal/proxy/server.go | 113 +++++++++++++++++++++---------- 2 files changed, 151 insertions(+), 57 deletions(-) diff --git a/internal/proxy/http_host_test.go b/internal/proxy/http_host_test.go index 682a894..75d6482 100644 --- a/internal/proxy/http_host_test.go +++ b/internal/proxy/http_host_test.go @@ -124,40 +124,93 @@ func TestPeekHTTPHost_RespectsMaxBytes(t *testing.T) { } } -func TestHostResolvesToIP_ReverseCacheAttestation(t *testing.T) { - // Server with a DNS interceptor whose reverse cache has been - // populated for derp.tailscale.com -> 192.0.2.10. A subsequent - // HTTP host-peek for that exact pair should return true without - // hitting the resolver. +func TestAttestHostFromCache_Match(t *testing.T) { + // Cache populated by a prior agent DNS query: derp.tailscale.com + // resolved to 192.0.2.10. Subsequent Host: derp.tailscale.com + // arriving on a SOCKS5 CONNECT to 192.0.2.10 is attested without + // any further DNS work. di := NewDNSInterceptor(nil, nil, "") di.StoreReverse("192.0.2.10", "derp.tailscale.com") s := &Server{dnsInterceptor: di} - if !s.hostResolvesToIP(context.Background(), "derp.tailscale.com", net.ParseIP("192.0.2.10")) { - t.Fatal("attested cache hit should be accepted") + if !s.attestHostFromCache("derp.tailscale.com", net.ParseIP("192.0.2.10")) { + t.Fatal("cache hit should attest") } } -func TestHostResolvesToIP_ReverseCacheDifferentHost(t *testing.T) { - // Cache says 192.0.2.10 -> attacker.example.com. A spoof attempt - // claiming Host: bank.example.com on the same IP must NOT be - // accepted off the cache. +func TestAttestHostFromCache_DifferentHost(t *testing.T) { + // Cache says 192.0.2.10 -> attacker.example.com. A claim of + // Host: bank.example.com on that IP must NOT be attested off + // the cache, even though the IP is in the cache. di := NewDNSInterceptor(nil, nil, "") di.StoreReverse("192.0.2.10", "attacker.example.com") s := &Server{dnsInterceptor: di} - // Lookup will fail or return something that does not match - // 192.0.2.10 (the literal IP is unlikely to be a registered - // hostname). Either way the cache must NOT attest the spoof. - if s.hostResolvesToIP(context.Background(), "bank.example.com", net.ParseIP("192.0.2.10")) { + if s.attestHostFromCache("bank.example.com", net.ParseIP("192.0.2.10")) { t.Fatal("cache hit for a different host must not attest a different-Host claim") } } -func TestHostResolvesToIP_NilInputs(t *testing.T) { - s := &Server{} - if s.hostResolvesToIP(context.Background(), "", net.ParseIP("1.2.3.4")) { - t.Error("empty host must not be considered attested") +func TestAttestHostFromCache_NilInputs(t *testing.T) { + s := &Server{dnsInterceptor: NewDNSInterceptor(nil, nil, "")} + if s.attestHostFromCache("", net.ParseIP("1.2.3.4")) { + t.Error("empty host must not attest") } - if s.hostResolvesToIP(context.Background(), "example.com", nil) { - t.Error("nil dest IP must not be considered attested") + if s.attestHostFromCache("example.com", nil) { + t.Error("nil dest IP must not attest") + } + emptyServer := &Server{} + if emptyServer.attestHostFromCache("example.com", net.ParseIP("1.2.3.4")) { + t.Error("nil DNS interceptor must not attest") + } +} + +// stubLookup returns a canned result regardless of the host. Used to +// exercise hostResolvesToIP without performing any real DNS queries. +func stubLookup(ips ...string) func(context.Context, string, string) ([]net.IP, error) { + out := make([]net.IP, len(ips)) + for i, s := range ips { + out[i] = net.ParseIP(s) + } + return func(_ context.Context, _, _ string) ([]net.IP, error) { + return out, nil + } +} + +func TestHostResolvesToIP_LookupMatch(t *testing.T) { + // Cache empty, stubbed resolver returns the dest IP among the + // answers. Should be attested. + s := &Server{ + dnsInterceptor: NewDNSInterceptor(nil, nil, ""), + lookupIP: stubLookup("203.0.113.5", "192.0.2.10"), + } + if !s.hostResolvesToIP(context.Background(), "good.example.com", net.ParseIP("192.0.2.10")) { + t.Fatal("forward-lookup match should attest") + } +} + +func TestHostResolvesToIP_LookupMismatch(t *testing.T) { + // Stub returns a result set that does NOT include the dest IP. + // Spoofing claim — must not attest. + s := &Server{ + dnsInterceptor: NewDNSInterceptor(nil, nil, ""), + lookupIP: stubLookup("203.0.113.5"), + } + if s.hostResolvesToIP(context.Background(), "spoof.example.com", net.ParseIP("192.0.2.10")) { + t.Fatal("forward-lookup mismatch must not attest") + } +} + +func TestHostResolvesToIP_LookupError(t *testing.T) { + // Resolver returns an error (NXDOMAIN, timeout, etc). Treated + // as deny-equivalent — sluice cannot tell a transient failure + // from a poisoned resolver, so the strict default applies. + errResolver := func(_ context.Context, _, _ string) ([]net.IP, error) { + return nil, net.UnknownNetworkError("test stub error") + } + s := &Server{ + dnsInterceptor: NewDNSInterceptor(nil, nil, ""), + lookupIP: errResolver, + } + if s.hostResolvesToIP(context.Background(), "example.com", net.ParseIP("192.0.2.10")) { + t.Fatal("lookup error must not attest") } } diff --git a/internal/proxy/server.go b/internal/proxy/server.go index 4417082..e353112 100644 --- a/internal/proxy/server.go +++ b/internal/proxy/server.go @@ -98,6 +98,12 @@ type Server struct { // mutations. oauthMetasMu sync.Mutex oauthMetasCache []store.CredentialMeta + + // lookupIP is the forward DNS lookup the HTTP Host spoofing + // guard uses when the reverse cache cannot attest the binding. + // nil means "use net.DefaultResolver.LookupIP". Tests inject a + // stub so the unit test does not perform a real DNS query. + lookupIP func(ctx context.Context, network, host string) ([]net.IP, error) } type contextKey string @@ -335,7 +341,16 @@ func (r *policyRuleSet) Allow(ctx context.Context, req *socks5.Request) (context // new IP behind a hostname rule (e.g. tailscale's DERP latency // probes hitting dozens of derp[N].tailscale.com IPs) generates an // approval prompt that can't be silenced short of allowing each IP. - if ipOnly && verdict != policy.Allow && verdict != policy.Deny && isPlainHTTPPort(port) { + // + // Require a broker before deferring. 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. When no broker is configured, fall 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. + if ipOnly && verdict != policy.Allow && verdict != policy.Deny && isPlainHTTPPort(port) && r.broker != nil { log.Printf("[HTTP-HOST-DEFER] %s:%d (deferring policy for Host header peek)", dest, port) proto := DetectProtocol(port) ctx = context.WithValue(ctx, ctxKeyProtocol, proto) @@ -1597,6 +1612,30 @@ func (s *Server) httpHostPolicyCheckBeforeDial(ctx context.Context, request *soc ipStr := request.DestAddr.IP.String() port := request.DestAddr.Port + // Evaluate policy on the recovered hostname BEFORE running the + // spoofing check. Two reasons: + // 1. If the verdict is Deny, the connection is rejected + // regardless of whether the Host claim is real, and we save + // a forward DNS lookup that would otherwise leak the + // hostname to the resolver. + // 2. If the verdict is the no-broker default (Ask collapsed to + // Deny), we skip the lookup for the same reason. + eng, _ := ctx.Value(ctxKeyEngine).(*policy.Engine) + if eng == nil { + eng = s.rules.engine.Load() + } + verdict, matchSource := eng.EvaluateDetailed(host, port) + reader := io.MultiReader(bytes.NewReader(buf), request.Reader) + + if verdict == policy.Deny { + log.Printf("[HTTP-HOST->DENY] %s:%d (hostname %s matched deny rule)", ipStr, port, host) + return nil, ctx, false + } + if verdict == policy.Ask && s.rules.broker == nil { + log.Printf("[HTTP-HOST->DENY] %s:%d (hostname %s: ask treated as deny, no broker)", ipStr, port, host) + return nil, ctx, false + } + // Spoofing guard. The Host header is client-controlled and not // cryptographically bound to the destination IP. With TLS, an // SNI/cert mismatch fails the upstream handshake and the bypass @@ -1624,13 +1663,6 @@ func (s *Server) httpHostPolicyCheckBeforeDial(ctx context.Context, request *soc s.dnsInterceptor.StoreReverse(ipStr, host) } - eng, _ := ctx.Value(ctxKeyEngine).(*policy.Engine) - if eng == nil { - eng = s.rules.engine.Load() - } - verdict, matchSource := eng.EvaluateDetailed(host, port) - reader := io.MultiReader(bytes.NewReader(buf), request.Reader) - switch verdict { case policy.Allow: log.Printf("[HTTP-HOST->ALLOW] %s:%d (hostname %s matched allow rule)", ipStr, port, host) @@ -1646,14 +1678,11 @@ func (s *Server) httpHostPolicyCheckBeforeDial(ctx context.Context, request *soc ctx = context.WithValue(ctx, ctxKeyPerRequestPolicy, checker) } return reader, ctx, true - case policy.Deny: - log.Printf("[HTTP-HOST->DENY] %s:%d (hostname %s matched deny rule)", ipStr, port, host) - return nil, ctx, false case policy.Ask: - if s.rules.broker == nil { - log.Printf("[HTTP-HOST->DENY] %s:%d (hostname %s: ask treated as deny, no broker)", ipStr, port, host) - return nil, ctx, false - } + // Deny + Ask-without-broker were filtered out before the + // spoofing check above. Reaching this branch means there is a + // broker, so attach a per-request checker and let the approval + // flow run on the first HTTP request. log.Printf("[HTTP-HOST->DEFER] %s:%d (hostname %s: approval deferred to per-request)", ipStr, port, host) checker := NewRequestPolicyChecker( s.rules.engine, s.rules.broker, @@ -1667,34 +1696,46 @@ func (s *Server) httpHostPolicyCheckBeforeDial(ctx context.Context, request *soc } } -// hostResolvesToIP reports whether forward-resolving host yields the given -// destination IP. Used by the HTTP Host peek path to defeat Host-spoofing -// attempts where an agent connects to an arbitrary IP and claims a -// permitted hostname in the request. Returns false on lookup error so -// the caller treats unverifiable Host claims as suspect rather than -// trusting the client. -// -// The lookup is bounded by hostResolveTimeout. The first DNS interceptor -// reverse cache hit short-circuits the lookup: if sluice's own DNS layer -// previously saw a query for host that returned dest, the binding is -// already attested and we do not need to repeat the resolve. +// attestHostFromCache reports whether sluice's DNS interceptor reverse +// cache binds dest to host. The cache is populated when the DNS layer +// answers a query the agent itself made, so a hit is the strongest +// available signal that host -> dest is real and not attacker-claimed. +// Pure-cache check; never touches the network. Returned separately +// from hostResolvesToIP so the cache logic can be unit-tested without +// pulling in a live resolver. +func (s *Server) attestHostFromCache(host string, dest net.IP) bool { + if dest == nil || host == "" || s.dnsInterceptor == nil { + return false + } + cached := s.dnsInterceptor.ReverseLookup(dest.String()) + if cached == "" { + return false + } + return strings.EqualFold(strings.TrimRight(cached, "."), host) +} + +// hostResolvesToIP reports whether host can be attested as binding to +// dest, either via the DNS interceptor's reverse cache or via a fresh +// forward DNS lookup bounded by hostResolveTimeout. Used by the HTTP +// Host peek path to defeat Host-spoofing attempts where an agent +// connects to an arbitrary IP and claims a permitted hostname in the +// request. Returns false on lookup error or mismatch so the caller +// treats unverifiable Host claims as suspect rather than trusting +// the client. func (s *Server) hostResolvesToIP(ctx context.Context, host string, dest net.IP) bool { if dest == nil || host == "" { return false } - // If sluice's DNS interceptor saw a prior query for this host that - // returned dest, the binding is already attested by an actual DNS - // answer the agent received and we do not need to re-resolve. - // The reverse cache stores IP -> hostname, so a hit on dest equals - // host means the agent itself just resolved host -> dest. - if s.dnsInterceptor != nil { - if cached := s.dnsInterceptor.ReverseLookup(dest.String()); cached != "" && strings.EqualFold(strings.TrimRight(cached, "."), host) { - return true - } + if s.attestHostFromCache(host, dest) { + return true + } + resolver := s.lookupIP + if resolver == nil { + resolver = net.DefaultResolver.LookupIP } lookupCtx, cancel := context.WithTimeout(ctx, hostResolveTimeout) defer cancel() - ips, err := net.DefaultResolver.LookupIP(lookupCtx, "ip", host) + ips, err := resolver(lookupCtx, "ip", host) if err != nil { return false } From 1cc33f421aa7bc4c8bf544b0295c880a0ffe1318 Mon Sep 17 00:00:00 2001 From: Nikita Nemirovsky Date: Fri, 8 May 2026 21:12:14 +0800 Subject: [PATCH 5/6] 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. --- internal/proxy/server.go | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/internal/proxy/server.go b/internal/proxy/server.go index e353112..b078fe3 100644 --- a/internal/proxy/server.go +++ b/internal/proxy/server.go @@ -1591,15 +1591,22 @@ func (s *Server) sniPolicyCheckBeforeDial(ctx context.Context, request *socks5.R func (s *Server) httpHostPolicyCheckBeforeDial(ctx context.Context, request *socks5.Request) (io.Reader, context.Context, bool) { buf, host, err := peekHTTPHost(request.Reader, 8192) if err != nil || host == "" { + // Deny when the peek fails to recover a Host. The deferral + // path runs only for non-Allow / non-Deny IP verdicts (i.e. + // Ask), so passing through with allow=true would silently + // upgrade Ask into an unconditional allow for any port-80 + // traffic that does not yield a parsable Host header + // (binary protocols, truncated headers, peek timeout, + // malformed HTTP). Denying is the strict default that + // preserves the original Ask semantic without re-entering + // SOCKS5 (we already sent RepSuccess to enable the peek); + // the agent observes a closed connection and can retry. hexPrefix := "" if len(buf) >= 6 { hexPrefix = fmt.Sprintf(" first6=%02x", buf[:6]) } - log.Printf("[HTTP-HOST-PEEK] no Host extracted (err=%v, bufLen=%d, host=%q%s)", err, len(buf), host, hexPrefix) - if len(buf) > 0 { - return io.MultiReader(bytes.NewReader(buf), request.Reader), ctx, true - } - return request.Reader, ctx, true + log.Printf("[HTTP-HOST->DENY] no Host extracted (err=%v, bufLen=%d, host=%q%s)", err, len(buf), host, hexPrefix) + return nil, ctx, false } host = strings.TrimRight(host, ".") From 6a332984bac3498e9b7f689c7549a41147d0af44 Mon Sep 17 00:00:00 2001 From: Nikita Nemirovsky Date: Fri, 8 May 2026 21:21:46 +0800 Subject: [PATCH 6/6] 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. --- internal/proxy/http_host.go | 18 +++++++------- internal/proxy/http_host_test.go | 12 ++++++++++ internal/proxy/server.go | 41 ++++++++++++++++++++++---------- 3 files changed, 51 insertions(+), 20 deletions(-) diff --git a/internal/proxy/http_host.go b/internal/proxy/http_host.go index c936a1f..af0149a 100644 --- a/internal/proxy/http_host.go +++ b/internal/proxy/http_host.go @@ -4,6 +4,7 @@ import ( "bufio" "bytes" "io" + "net" "net/http" "strings" ) @@ -77,15 +78,16 @@ func extractHTTPHost(prefix []byte) (string, bool) { if host == "" { return "", false } - // Strip port if present. IPv6 hosts in Host headers appear as - // "[::1]:80" so only strip the trailing :port when there is no - // closing bracket after the last colon. - if i := strings.LastIndex(host, ":"); i >= 0 { - if !strings.Contains(host[i:], "]") { - host = host[:i] - } + // net.SplitHostPort handles every shape Host can legitimately + // take: "example.com:80" -> ("example.com", "80"), + // "[::1]:80" -> ("::1", "80"), "[::1]" with no port -> error, + // and bare IPv6 like "2001:db8::1" -> error ("too many colons"). + // Falling back to the trimmed host on error avoids the previous + // LastIndex(":") approach that mishandled bare IPv6 by stripping + // the final hextet as if it were a port. + if h, _, err := net.SplitHostPort(host); err == nil { + host = h } - // IPv6 hosts may still be wrapped in [] — strip those. host = strings.TrimPrefix(host, "[") host = strings.TrimSuffix(host, "]") if host == "" { diff --git a/internal/proxy/http_host_test.go b/internal/proxy/http_host_test.go index 75d6482..fe2c81a 100644 --- a/internal/proxy/http_host_test.go +++ b/internal/proxy/http_host_test.go @@ -38,6 +38,18 @@ func TestExtractHTTPHost_IPv6WithPort(t *testing.T) { } } +func TestExtractHTTPHost_UnbracketedIPv6(t *testing.T) { + // Bare IPv6 without brackets is invalid per RFC but seen in the + // wild. The previous LastIndex(":") logic chopped the final + // hextet as if it were a port. SplitHostPort errors on this + // shape, so the original host should pass through unmodified. + prefix := []byte("GET / HTTP/1.1\r\nHost: 2001:db8::1\r\n\r\n") + host, ok := extractHTTPHost(prefix) + if !ok || host != "2001:db8::1" { + t.Errorf("got %q ok=%v, want 2001:db8::1 ok=true (bare IPv6 must not be truncated)", host, ok) + } +} + func TestExtractHTTPHost_MissingHost(t *testing.T) { // HTTP/1.0 allowed missing Host. Should return ok=false rather than // silently allowing an empty hostname through to policy. diff --git a/internal/proxy/server.go b/internal/proxy/server.go index b078fe3..b16551c 100644 --- a/internal/proxy/server.go +++ b/internal/proxy/server.go @@ -1591,22 +1591,39 @@ func (s *Server) sniPolicyCheckBeforeDial(ctx context.Context, request *socks5.R func (s *Server) httpHostPolicyCheckBeforeDial(ctx context.Context, request *socks5.Request) (io.Reader, context.Context, bool) { buf, host, err := peekHTTPHost(request.Reader, 8192) if err != nil || host == "" { - // Deny when the peek fails to recover a Host. The deferral - // path runs only for non-Allow / non-Deny IP verdicts (i.e. - // Ask), so passing through with allow=true would silently - // upgrade Ask into an unconditional allow for any port-80 - // traffic that does not yield a parsable Host header - // (binary protocols, truncated headers, peek timeout, - // malformed HTTP). Denying is the strict default that - // preserves the original Ask semantic without re-entering - // SOCKS5 (we already sent RepSuccess to enable the peek); - // the agent observes a closed connection and can retry. + // Peek failed: binary protocol on port 80, truncated + // headers, peek timeout, or otherwise unparsable HTTP. We + // must NOT fall through with allow=true unconditionally — + // the deferral path runs for Ask verdicts, so a free pass + // here would silently upgrade Ask into an allow. We must + // also NOT collapse the verdict to outright deny, because + // the original Ask semantic for the IP destination still + // needs to be honored when an operator is at the broker. + // + // Attach a per-request checker bound to the IP and return + // allow=true with the buffered bytes. The downstream dial + // step calls CheckAndConsume on the checker, which + // broker-prompts the operator for the bare IP. If they + // approve, dial proceeds; if they deny, dial fails and the + // connection closes. This mirrors what the non-deferred + // Ask-with-broker path does when an Ask verdict reaches + // dial — exactly the behavior we deferred away from. The + // deferral guard above already required broker != nil, so + // the checker has somewhere to send the prompt. hexPrefix := "" if len(buf) >= 6 { hexPrefix = fmt.Sprintf(" first6=%02x", buf[:6]) } - log.Printf("[HTTP-HOST->DENY] no Host extracted (err=%v, bufLen=%d, host=%q%s)", err, len(buf), host, hexPrefix) - return nil, ctx, false + log.Printf("[HTTP-HOST-PEEK] no Host extracted, falling back to IP-based ask flow (err=%v, bufLen=%d, host=%q%s)", err, len(buf), host, hexPrefix) + checker := NewRequestPolicyChecker( + s.rules.engine, s.rules.broker, + WithPersist(s.rules.buildPersistFunc()), + ) + ctx = context.WithValue(ctx, ctxKeyPerRequestPolicy, checker) + if len(buf) > 0 { + return io.MultiReader(bytes.NewReader(buf), request.Reader), ctx, true + } + return request.Reader, ctx, true } host = strings.TrimRight(host, ".")