fix(policy): unify UDP policy with TCP, fix QUIC shared-IP dedup

Author: nnemirovsky

CLAUDE.md

@@ -187,6 +187,8 @@ 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()`.
 
+**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
 
 Two-phase detection: port-based guess first, then byte-level for non-standard ports. Standard ports (443, 22, 25, etc.) route directly on port guess. When port guess returns `ProtoGeneric`, `DetectFromClientBytes` peeks first bytes (TLS, SSH, HTTP) and `DetectFromServerBytes` reads server banner (SMTP, IMAP). Detection path signals SOCKS5 CONNECT success before reading client bytes.
@@ -197,7 +199,7 @@ Two-phase detection: port-based guess first, then byte-level for non-standard po
 
 `CouldBeAllowed(dest, includeAsk)`: when broker configured, Ask-matching destinations resolve via DNS for approval flow. When no broker, Ask treated as Deny at DNS stage to prevent leaking queries.
 
-**DNS approval design**: The DNS interceptor intentionally only blocks explicitly denied domains (returns NXDOMAIN). All other queries (allow, ask, default) are forwarded to the upstream resolver. This is by design. Policy enforcement for "ask" destinations happens at the SOCKS5 CONNECT layer, not at DNS. Blocking DNS for "ask" destinations would prevent the TCP connection from ever reaching the SOCKS5 handler where the approval flow triggers. The DNS layer populates the reverse DNS cache (IP -> hostname) so the SOCKS5 handler can recover hostnames from IP-only CONNECT requests.
+**DNS approval design**: The DNS interceptor intentionally only blocks explicitly denied domains (returns NXDOMAIN). All other queries (allow, ask, default) are forwarded to the upstream resolver. This is by design. Policy enforcement for "ask" destinations happens at the SOCKS5 CONNECT layer, not at DNS. Blocking DNS for "ask" destinations would prevent the TCP connection from ever reaching the SOCKS5 handler where the approval flow triggers. The DNS layer populates the reverse DNS cache (IP -> hostname) so the SOCKS5 handler can recover hostnames from IP-only CONNECT requests. DNS uses `IsDeniedDomain`, a separate evaluation path that is independent from the unscoped-rule matching in `EvaluateUDP` / `EvaluateQUICDetailed`. Unscoped rules therefore widen TCP/UDP/QUIC policy without changing DNS behavior.
 
 ### Audit logger
 

internal/policy/engine.go

@@ -307,9 +307,10 @@ func matchRules(rules []compiledRule, dest string, port int) bool {
 
 // matchRulesStrictProto matches rules that explicitly include the given
 // protocol in their protocols field. Rules without a protocols field are NOT
-// matched. Used by EvaluateUDP and EvaluateQUIC where only protocol-explicit
-// rules should apply, preventing unscoped TCP rules from inadvertently
-// allowing UDP/QUIC traffic.
+// matched. Used by EvaluateUDP and EvaluateQUICDetailed for their
+// protocol-scoped first-pass evaluation. Unscoped rules are handled
+// separately by matchRulesUnscoped so UDP and QUIC fall back to the same
+// 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) {
@@ -319,7 +320,7 @@ func matchRulesStrictProto(rules []compiledRule, dest string, port int, proto st
 			continue
 		}
 		// Require explicit protocol match. Rules without a protocols field
-		// are skipped to prevent TCP-intended rules from matching UDP/QUIC.
+		// are skipped here; matchRulesUnscoped handles those separately.
 		if len(r.protocols) == 0 || !r.protocols[proto] {
 			continue
 		}
@@ -328,6 +329,28 @@ func matchRulesStrictProto(rules []compiledRule, dest string, port int, proto st
 	return false
 }
 
+// matchRulesUnscoped matches rules that have NO protocols field (unscoped).
+// Used as a fallback in EvaluateUDP and EvaluateQUICDetailed after strict
+// protocol-scoped matching fails, so that unscoped rules (the common case
+// for user-configured destination rules) apply consistently across TCP,
+// UDP, and QUIC transports. DNS has its own evaluation path (IsDeniedDomain)
+// and is unaffected.
+func matchRulesUnscoped(rules []compiledRule, dest string, port int) bool {
+	for _, r := range rules {
+		if !r.glob.Match(dest) {
+			continue
+		}
+		if len(r.ports) > 0 && !r.ports[port] {
+			continue
+		}
+		if len(r.protocols) != 0 {
+			continue
+		}
+		return true
+	}
+	return false
+}
+
 // matchRulesWithProto checks compiled rules against a destination, port, and
 // optional explicit protocol. When proto is non-empty it takes precedence over
 // the port-based heuristic, allowing header-detected protocols (ws, wss, grpc)
@@ -699,25 +722,47 @@ func (e *Engine) EvaluateDetailedWithProtocol(dest string, port int, proto strin
 	return e.Default, DefaultVerdict
 }
 
-// EvaluateUDP checks a destination and port with UDP-specific semantics.
-// Only explicit allow rules produce an Allow verdict. Deny rules take priority
-// as usual. Ask rules and the engine default verdict are treated as Deny
-// because per-packet approval is impractical. This implements the UDP
-// default-deny strategy where UDP traffic requires an explicit allow rule.
+// EvaluateUDP checks a destination and port for UDP traffic. Behavior mirrors
+// the TCP Evaluate path: deny rules take priority, then allow, then the engine
+// default verdict. Ask is not a valid terminal verdict for UDP (per-packet
+// approval is impractical), so an Ask default is collapsed to Deny. Callers
+// that need Ask semantics for QUIC should use EvaluateQUICDetailed, which
+// preserves Ask for the approval flow.
+//
+// Evaluation order: UDP-scoped deny, UDP-scoped allow, unscoped deny, unscoped
+// allow, then the engine's configured default verdict. Unscoped rules (no
+// protocols field) are transport-agnostic so a user-configured
+// `allow example.com` rule applies to TCP, UDP, and QUIC consistently. DNS is
+// the only transport with a separate evaluation path (IsDeniedDomain).
 func (e *Engine) EvaluateUDP(dest string, port int) Verdict {
 	dest = normalizeDestination(dest)
 	e.mu.RLock()
 	defer e.mu.RUnlock()
 	if e.compiled == nil {
-		return Deny
+		if e.Default == Ask {
+			return Deny
+		}
+		return e.Default
 	}
 	if matchRulesStrictProto(e.compiled.denyRules, dest, port, protoNameUDP) {
 		return Deny
 	}
 	if matchRulesStrictProto(e.compiled.allowRules, dest, port, protoNameUDP) {
 		return Allow
 	}
-	return Deny
+	if matchRulesUnscoped(e.compiled.denyRules, dest, port) {
+		return Deny
+	}
+	if matchRulesUnscoped(e.compiled.allowRules, dest, port) {
+		return Allow
+	}
+	// Fall back to the engine's configured default verdict. EvaluateUDP has
+	// no Ask flow (no broker per-packet), so an Ask default collapses to
+	// Deny. Callers that need Ask must use EvaluateQUICDetailed.
+	if e.Default == Ask {
+		return Deny
+	}
+	return e.Default
 }
 
 // EvaluateQUIC checks a destination and port with QUIC-specific semantics.
@@ -738,9 +783,12 @@ func (e *Engine) EvaluateQUIC(dest string, port int) Verdict {
 
 // EvaluateQUICDetailed returns the verdict and match source for QUIC traffic.
 // Unlike EvaluateQUIC, it preserves Ask verdicts so callers can trigger the
-// approval flow for per-request policy. Evaluation order: QUIC-specific deny,
-// QUIC-specific allow, QUIC-specific ask, then generic deny, allow, ask,
-// then engine default verdict.
+// approval flow for per-request policy. Evaluation order: QUIC-scoped deny,
+// QUIC-scoped allow, QUIC-scoped ask, UDP-scoped deny, UDP-scoped allow,
+// UDP-scoped ask, unscoped deny, unscoped allow, unscoped ask, then engine
+// default verdict. Unscoped rules (no protocols field) are treated as
+// transport-agnostic so user-configured destination rules apply consistently
+// across TCP, UDP, and QUIC.
 func (e *Engine) EvaluateQUICDetailed(dest string, port int) (Verdict, MatchSource) {
 	dest = normalizeDestination(dest)
 	e.mu.RLock()
@@ -768,9 +816,16 @@ func (e *Engine) EvaluateQUICDetailed(dest string, port int) (Verdict, MatchSour
 	if matchRulesStrictProto(e.compiled.askRules, dest, port, protoNameUDP) {
 		return Ask, RuleMatch
 	}
-	// Use the engine's configured default verdict. Unscoped rules (no
-	// protocol filter) are NOT matched for QUIC because they are
-	// TCP-scoped by convention and should not inadvertently allow or
-	// deny UDP/QUIC traffic.
+	// Fall back to unscoped rules so transport-agnostic user rules apply.
+	if matchRulesUnscoped(e.compiled.denyRules, dest, port) {
+		return Deny, RuleMatch
+	}
+	if matchRulesUnscoped(e.compiled.allowRules, dest, port) {
+		return Allow, RuleMatch
+	}
+	if matchRulesUnscoped(e.compiled.askRules, dest, port) {
+		return Ask, RuleMatch
+	}
+	// Use the engine's configured default verdict.
 	return e.Default, DefaultVerdict
 }

internal/policy/engine_test.go

@@ -1249,10 +1249,11 @@ default = "ask"
 	}
 }
 
-func TestEvaluateUDP_UnscopedRulesIgnored(t *testing.T) {
-	// Rules without explicit protocols must NOT match EvaluateUDP or
-	// EvaluateQUIC. This prevents TCP-intended allow rules from
-	// inadvertently allowing UDP/QUIC traffic.
+func TestEvaluateUDP_UnscopedRulesMatch(t *testing.T) {
+	// Unscoped rules (no protocols field) match UDP and QUIC traffic as
+	// well as TCP. This keeps `sluice policy add allow example.com` working
+	// consistently across transports. Protocol-scoped rules (protocols=
+	// ["udp"] or ["quic"]) still take priority when present.
 	eng, err := LoadFromBytes([]byte(`
 [policy]
 default = "deny"
@@ -1270,14 +1271,14 @@ protocols = ["udp"]
 		t.Fatalf("load: %v", err)
 	}
 
-	// Unscoped allow rule must NOT allow UDP traffic.
-	if got := eng.EvaluateUDP("api.anthropic.com", 443); got != Deny {
-		t.Errorf("EvaluateUDP(unscoped allow) = %v, want Deny", got)
+	// Unscoped allow rule now matches UDP traffic.
+	if got := eng.EvaluateUDP("api.anthropic.com", 443); got != Allow {
+		t.Errorf("EvaluateUDP(unscoped allow) = %v, want Allow", got)
 	}
 
-	// Unscoped allow rule must NOT allow QUIC traffic.
-	if got := eng.EvaluateQUIC("api.anthropic.com", 443); got != Deny {
-		t.Errorf("EvaluateQUIC(unscoped allow) = %v, want Deny", got)
+	// Unscoped allow rule now matches QUIC traffic.
+	if got := eng.EvaluateQUIC("api.anthropic.com", 443); got != Allow {
+		t.Errorf("EvaluateQUIC(unscoped allow) = %v, want Allow", got)
 	}
 
 	// Explicitly scoped UDP rule must still work.
@@ -1289,6 +1290,199 @@ protocols = ["udp"]
 	if got := eng.Evaluate("api.anthropic.com", 443); got != Allow {
 		t.Errorf("Evaluate(unscoped allow) = %v, want Allow", got)
 	}
+
+	// Unscoped rule with a port filter still respects the port.
+	if got := eng.EvaluateUDP("api.anthropic.com", 80); got != Deny {
+		t.Errorf("EvaluateUDP(unscoped allow, wrong port) = %v, want Deny", got)
+	}
+}
+
+func TestEvaluateUDP_UnscopedAllowMatches(t *testing.T) {
+	eng, err := LoadFromBytes([]byte(`
+[policy]
+default = "deny"
+
+[[allow]]
+destination = "cloudflare.com"
+ports = [443]
+`))
+	if err != nil {
+		t.Fatalf("load: %v", err)
+	}
+	if got := eng.EvaluateUDP("cloudflare.com", 443); got != Allow {
+		t.Errorf("EvaluateUDP(unscoped allow) = %v, want Allow", got)
+	}
+}
+
+func TestEvaluateUDP_DefaultAllow(t *testing.T) {
+	// When the engine default is "allow", an unmatched UDP destination
+	// returns Allow (just like TCP Evaluate). This mirrors how the engine
+	// default is used as the terminal fallback for every transport except
+	// DNS (which has its own IsDeniedDomain path).
+	eng, err := LoadFromBytes([]byte(`
+[policy]
+default = "allow"
+
+[[deny]]
+destination = "blocked.example.com"
+ports = [443]
+`))
+	if err != nil {
+		t.Fatalf("load: %v", err)
+	}
+
+	// Unknown destination falls back to default "allow".
+	if got := eng.EvaluateUDP("unknown.example.com", 443); got != Allow {
+		t.Errorf("EvaluateUDP(default=allow, unknown) = %v, want Allow", got)
+	}
+
+	// Explicit deny still takes priority over default allow.
+	if got := eng.EvaluateUDP("blocked.example.com", 443); got != Deny {
+		t.Errorf("EvaluateUDP(default=allow, denied) = %v, want Deny", got)
+	}
+}
+
+func TestEvaluateUDP_DefaultDeny(t *testing.T) {
+	// When the engine default is "deny", an unmatched UDP destination
+	// returns Deny.
+	eng, err := LoadFromBytes([]byte(`
+[policy]
+default = "deny"
+
+[[allow]]
+destination = "allowed.example.com"
+ports = [443]
+`))
+	if err != nil {
+		t.Fatalf("load: %v", err)
+	}
+
+	// Unknown destination falls back to default "deny".
+	if got := eng.EvaluateUDP("unknown.example.com", 443); got != Deny {
+		t.Errorf("EvaluateUDP(default=deny, unknown) = %v, want Deny", got)
+	}
+
+	// Matching allow rule still produces Allow.
+	if got := eng.EvaluateUDP("allowed.example.com", 443); got != Allow {
+		t.Errorf("EvaluateUDP(default=deny, allowed) = %v, want Allow", got)
+	}
+}
+
+func TestEvaluateUDP_DefaultAsk(t *testing.T) {
+	// EvaluateUDP has no approval flow (per-packet approval is impractical),
+	// so an Ask default collapses to Deny. This mirrors the contract of
+	// EvaluateQUIC (non-detailed), which collapses Ask to Deny for callers
+	// that do not know how to drive the approval flow.
+	eng, err := LoadFromBytes([]byte(`
+[policy]
+default = "ask"
+`))
+	if err != nil {
+		t.Fatalf("load: %v", err)
+	}
+
+	if got := eng.EvaluateUDP("anything.example.com", 443); got != Deny {
+		t.Errorf("EvaluateUDP(default=ask) = %v, want Deny (collapsed)", got)
+	}
+}
+
+func TestEvaluateUDP_NilCompiled(t *testing.T) {
+	// Engine with nil compiled state returns the engine default (Ask
+	// collapses to Deny). Zero-value Engine has Default=Allow.
+	eng := &Engine{}
+	if got := eng.EvaluateUDP("anything.com", 443); got != Allow {
+		t.Errorf("EvaluateUDP(nil compiled, default=Allow) = %v, want Allow", got)
+	}
+
+	eng2 := &Engine{Default: Deny}
+	if got := eng2.EvaluateUDP("anything.com", 443); got != Deny {
+		t.Errorf("EvaluateUDP(nil compiled, default=Deny) = %v, want Deny", got)
+	}
+
+	eng3 := &Engine{Default: Ask}
+	if got := eng3.EvaluateUDP("anything.com", 443); got != Deny {
+		t.Errorf("EvaluateUDP(nil compiled, default=Ask) = %v, want Deny (collapsed)", got)
+	}
+}
+
+func TestEvaluateQUICDetailed_UnscopedAllowMatches(t *testing.T) {
+	eng, err := LoadFromBytes([]byte(`
+[policy]
+default = "deny"
+
+[[allow]]
+destination = "cloudflare.com"
+ports = [443]
+`))
+	if err != nil {
+		t.Fatalf("load: %v", err)
+	}
+	v, src := eng.EvaluateQUICDetailed("cloudflare.com", 443)
+	if v != Allow || src != RuleMatch {
+		t.Errorf("EvaluateQUICDetailed(unscoped allow) = (%v, %v), want (Allow, RuleMatch)", v, src)
+	}
+}
+
+func TestEvaluateQUICDetailed_UnscopedDenyPriority(t *testing.T) {
+	// Unscoped deny beats unscoped allow for QUIC, matching the behavior
+	// of TCP evaluation.
+	eng, err := LoadFromBytes([]byte(`
+[policy]
+default = "allow"
+
+[[allow]]
+destination = "overlap.example.com"
+ports = [443]
+
+[[deny]]
+destination = "overlap.example.com"
+ports = [443]
+`))
+	if err != nil {
+		t.Fatalf("load: %v", err)
+	}
+	v, src := eng.EvaluateQUICDetailed("overlap.example.com", 443)
+	if v != Deny || src != RuleMatch {
+		t.Errorf("EvaluateQUICDetailed(unscoped deny over unscoped allow) = (%v, %v), want (Deny, RuleMatch)", v, src)
+	}
+}
+
+func TestEvaluateQUICDetailed_ProtocolScopedTakesPriority(t *testing.T) {
+	// An explicit QUIC deny rule must beat an unscoped allow rule, and an
+	// explicit UDP allow must beat an unscoped deny for QUIC evaluation.
+	eng, err := LoadFromBytes([]byte(`
+[policy]
+default = "deny"
+
+[[allow]]
+destination = "quic-deny.example.com"
+ports = [443]
+
+[[deny]]
+destination = "quic-deny.example.com"
+ports = [443]
+protocols = ["quic"]
+
+[[deny]]
+destination = "udp-allow.example.com"
+ports = [443]
+
+[[allow]]
+destination = "udp-allow.example.com"
+ports = [443]
+protocols = ["udp"]
+`))
+	if err != nil {
+		t.Fatalf("load: %v", err)
+	}
+	// QUIC-scoped deny wins over unscoped allow.
+	if v, src := eng.EvaluateQUICDetailed("quic-deny.example.com", 443); v != Deny || src != RuleMatch {
+		t.Errorf("EvaluateQUICDetailed(quic deny) = (%v, %v), want (Deny, RuleMatch)", v, src)
+	}
+	// UDP-scoped allow wins over unscoped deny.
+	if v, src := eng.EvaluateQUICDetailed("udp-allow.example.com", 443); v != Allow || src != RuleMatch {
+		t.Errorf("EvaluateQUICDetailed(udp allow) = (%v, %v), want (Allow, RuleMatch)", v, src)
+	}
 }
 
 func TestEvaluate_TCPMetaProtocol(t *testing.T) {