fix(policy): use engine default verdict for QUIC (#29)

* fix(policy): use engine default verdict for QUIC instead of hardcoded deny

EvaluateQUICDetailed was hardcoded to return Deny as the default
verdict, ignoring the engine's configured default. When default is
"ask", QUIC traffic to unmatched destinations was silently dropped
instead of triggering approval. Now uses e.Default so QUIC respects
the same default as TCP.

* docs: add QUIC full flow fixes plan

* feat(proxy): extract SNI from QUIC Initial packets for hostname-based policy

* feat(proxy): deduplicate QUIC broker requests with bounded packet buffer

* fix(proxy): verify and document QUIC response relay path

* fix(test): use IPv4-only listeners in httptest servers

* test(e2e): add comprehensive protocol coverage for WebSocket, gRPC, QUIC, DNS, IMAP/SMTP

* chore: mark Task 6 acceptance criteria verified

* docs: update CLAUDE.md with QUIC SNI extraction and move plan to completed

* fix: address review phase 1 findings

* fix: address review phase 2 code smell findings

* fix(proxy): address review phase 4 critical findings

* fix(proxy): handle real-world QUIC Initial packets in SNI extraction

* fix(proxy): handle real-world QUIC frame parsing and document SNI fragmentation

* feat(cli): add --protocols flag to policy add command

* feat(proxy): accumulate CRYPTO data across QUIC Initial packets to extract SNI

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

* style: fix golangci-lint issues in QUIC SNI code

* fix(e2e): remove broken WS credential injection test (upstream limitation)

* fix(proxy): forward modified headers on WebSocket upgrade

Bumps go-mitmproxy fork to include the header forwarding fix so that
addon-modified headers (credential injection, custom headers) reach the
upstream WS server during the handshake.

Restores TestWebSocket_CredentialInjectionInUpgradeHeaders which now
passes end-to-end.

* chore: bump go-mitmproxy fork to drop Chinese comment

* fix(test): address SSH jump host test flakiness

Author: nnemirovsky

CLAUDE.md

@@ -11,7 +11,7 @@ go test ./... -v -timeout 30s
 
 ## E2e Tests
 
-End-to-end tests live in `e2e/` and use build tags. They start a real sluice binary, configure policies, make connections through the proxy, and verify credential injection, MCP gateway flows, and audit log integrity.
+End-to-end tests live in `e2e/` and use build tags. They start a real sluice binary, configure policies, make connections through the proxy, and verify credential injection, MCP gateway flows, and audit log integrity. Protocol coverage: HTTP/HTTPS, SSH, MCP, WebSocket, gRPC, QUIC/HTTP3, DNS, and IMAP/SMTP.
 
 Build tags:
 - `e2e` -- required for all e2e tests
@@ -150,6 +150,7 @@ Extends phantom swap to handle OAuth credentials bidirectionally. Static credent
 - `internal/vault/phantom.go` -- `GeneratePhantomToken` for MITM phantom strings
 - `internal/proxy/oauth_index.go` -- Token URL index for response matching
 - `internal/proxy/oauth_response.go` -- Response interception, phantom swap, async vault persistence
+- `internal/proxy/quic_sni.go` -- `ExtractQUICSNI` decrypts QUIC Initial to extract SNI hostname
 - `internal/container/docker.go` -- `InjectEnvVars` implementation for Docker backend
 - `internal/container/types.go` -- `ContainerManager` interface with `InjectEnvVars`
 - `internal/store/migrations/000002_credential_meta.up.sql` -- Schema for credential metadata
@@ -165,23 +166,29 @@ Extends phantom swap to handle OAuth credentials bidirectionally. Static credent
 | SSH | Jump host, key from vault | N/A | Per-connection (channels belong to one session) |
 | IMAP/SMTP | AUTH command proxy, phantom password swap | N/A | Per-connection (one mailbox session) |
 | DNS | N/A | Deny-only (NXDOMAIN). See DNS design note below. | Per-query deny, other verdicts resolved at SOCKS5 |
-| QUIC/HTTP3 | HTTP/3 MITM via quic-go | Full HTTP/3 request/response | Per-request (each HTTP/3 request triggers policy check) |
+| QUIC/HTTP3 | HTTP/3 MITM via quic-go, SNI from Initial packet | Full HTTP/3 request/response | Per-request (each HTTP/3 request triggers policy check) |
 | APNS | Connection-level allow/deny (port 5223) | N/A | Per-connection |
 
 **Per-request policy evaluation** applies to HTTP/HTTPS, gRPC-over-HTTP/2, and QUIC/HTTP3. Policy is re-evaluated for every HTTP request (or HTTP/2 stream, or HTTP/3 request), so "Allow Once" permits a single request and subsequent requests on the same connection re-trigger the approval flow. When a per-request approval resolves to "Always Allow" or "Always Deny", the `RequestPolicyChecker` persists the new rule to the policy store via its `PersistRuleFunc` callback and swaps in a freshly compiled engine, so subsequent requests match via the fast path instead of re-entering the approval flow. A fast path skips per-request checks when the SOCKS5 CONNECT matched an explicit allow rule (`RuleMatch`, not default verdict) so normally allowed destinations incur no extra overhead. WebSocket, SSH, and IMAP/SMTP remain connection-level on purpose: per-message or per-command policy on those would blow past the broker's 5/min per-destination rate limit and break normal usage.
 
 **MITM library:** HTTPS interception uses go-mitmproxy (`github.com/lqqyt2423/go-mitmproxy`). The `SluiceAddon` struct in `internal/proxy/addon.go` implements go-mitmproxy's `Addon` interface. `Requestheaders` fires per HTTP/2 stream, giving true per-request policy for gRPC and other HTTP/2 traffic. `Request` handles credential injection (three-pass phantom swap). `Response` handles OAuth token interception.
 
-**QUIC per-request:** `EvaluateQUICDetailed` returns Ask when an ask rule matches. The UDP dispatch loop creates a `RequestPolicyChecker` and passes it to `buildHandler`, which calls `CheckAndConsume` per HTTP/3 request.
+**QUIC per-request:** `EvaluateQUICDetailed` returns Ask when an ask rule matches and falls back to the engine's configured default verdict (not hardcoded Deny). The UDP dispatch loop creates a `RequestPolicyChecker` and passes it to `buildHandler`, which calls `CheckAndConsume` per HTTP/3 request. When the default verdict is "allow", a per-request checker is still attached (with seed credits of 1) so long-lived QUIC sessions re-evaluate policy on subsequent requests.
 
-See `internal/proxy/request_policy.go`, `internal/policy/engine.go` (`EvaluateDetailed`, `EvaluateQUICDetailed`), and `internal/proxy/addon.go` (`SluiceAddon`).
+**QUIC SNI extraction:** Hostname recovery uses `ExtractQUICSNI()` to decrypt the QUIC Initial packet and extract SNI from the embedded TLS ClientHello. QUIC Initial packets encrypt the ClientHello, but the encryption keys are derived from the Destination Connection ID (DCID) visible in the packet header (RFC 9001 Section 5). Supports both QUIC v1 and v2 salts. Falls back to DNS reverse cache lookup, then raw IP if extraction fails.
+
+**QUIC broker dedup:** `pendingQUICSessions` in `server.go` prevents duplicate Telegram approval prompts when multiple UDP packets arrive for the same destination during the approval wait. Packets are buffered (max 32 per session). When approval resolves, buffered packets are flushed (if allowed) or discarded (if denied).
+
+See `internal/proxy/request_policy.go`, `internal/policy/engine.go` (`EvaluateDetailed`, `EvaluateQUICDetailed`), `internal/proxy/quic_sni.go` (`ExtractQUICSNI`), and `internal/proxy/addon.go` (`SluiceAddon`).
 
 ## Implementation Details
 
 ### Policy engine
 
 `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.
@@ -192,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
 

cmd/sluice/flagutil.go

@@ -33,6 +33,28 @@ func parsePortsList(s string) ([]int, error) {
 	return ports, nil
 }
 
+// parseProtocolsList parses a comma-separated string of protocol names into
+// a []string. An empty input returns (nil, nil). Whitespace around each
+// entry is trimmed and the name is lowercased.
+//
+// Validation against the known protocol set is deferred to the store layer
+// (validateProtocols) which runs during AddRule/ImportTOML. This keeps
+// the canonical list in one place.
+func parseProtocolsList(s string) ([]string, error) {
+	if s == "" {
+		return nil, nil
+	}
+	var protocols []string
+	for _, ps := range strings.Split(s, ",") {
+		ps = strings.TrimSpace(strings.ToLower(ps))
+		if ps == "" {
+			return nil, fmt.Errorf("empty protocol name in list")
+		}
+		protocols = append(protocols, ps)
+	}
+	return protocols, nil
+}
+
 // reorderFlagsBeforePositional returns a copy of args with all flag
 // arguments moved before any positional arguments, so that Go's stdlib
 // flag parser (which stops at the first non-flag) still sees every flag.

cmd/sluice/policy.go

@@ -89,7 +89,7 @@ func handlePolicyList(args []string) error {
 
 func handlePolicyAdd(args []string) error {
 	if len(args) == 0 {
-		return fmt.Errorf("usage: sluice policy add <allow|deny|ask> <destination> [--ports 443,80] [--name \"reason\"]")
+		return fmt.Errorf("usage: sluice policy add <allow|deny|ask> <destination> [--ports 443,80] [--protocols quic,udp] [--name \"reason\"]")
 	}
 
 	verdict := args[0]
@@ -100,13 +100,14 @@ func handlePolicyAdd(args []string) error {
 	fs := flag.NewFlagSet("policy add", flag.ContinueOnError)
 	dbPath := fs.String("db", "data/sluice.db", "path to SQLite database")
 	portsStr := fs.String("ports", "", "comma-separated port list (e.g. 443,80)")
+	protocolsStr := fs.String("protocols", "", "comma-separated protocol list (e.g. quic,udp)")
 	note := fs.String("name", "", "human-readable name")
 	if err := fs.Parse(args[1:]); err != nil {
 		return err
 	}
 
 	if fs.NArg() == 0 {
-		return fmt.Errorf("usage: sluice policy add <allow|deny|ask> <destination> [--ports 443,80] [--name \"reason\"]")
+		return fmt.Errorf("usage: sluice policy add <allow|deny|ask> <destination> [--ports 443,80] [--protocols quic,udp] [--name \"reason\"]")
 	}
 	destination := fs.Arg(0)
 
@@ -119,13 +120,18 @@ func handlePolicyAdd(args []string) error {
 		return err
 	}
 
+	protocols, err := parseProtocolsList(*protocolsStr)
+	if err != nil {
+		return err
+	}
+
 	db, err := store.New(*dbPath)
 	if err != nil {
 		return fmt.Errorf("open store: %w", err)
 	}
 	defer func() { _ = db.Close() }()
 
-	id, err := db.AddRule(verdict, store.RuleOpts{Destination: destination, Ports: ports, Name: *note})
+	id, err := db.AddRule(verdict, store.RuleOpts{Destination: destination, Ports: ports, Protocols: protocols, Name: *note})
 	if err != nil {
 		return fmt.Errorf("add rule: %w", err)
 	}

cmd/sluice/policy_test.go

@@ -305,6 +305,68 @@ func TestHandlePolicyAddWithGlob(t *testing.T) {
 	}
 }
 
+func TestHandlePolicyAddWithProtocols(t *testing.T) {
+	dir := t.TempDir()
+	dbPath := filepath.Join(dir, "test.db")
+
+	output := capturePolicyOutput(t, func() {
+		if err := handlePolicyAdd([]string{"allow", "--db", dbPath, "--protocols", "quic,udp", "--ports", "443", "cdn.example.com"}); err != nil {
+			t.Fatalf("handlePolicyAdd with protocols: %v", err)
+		}
+	})
+
+	if !strings.Contains(output, "added allow rule") {
+		t.Errorf("expected 'added allow rule' in output: %s", output)
+	}
+
+	db, err := store.New(dbPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer func() { _ = db.Close() }()
+
+	rules, _ := db.ListRules(store.RuleFilter{Verdict: "allow"})
+	if len(rules) != 1 {
+		t.Fatalf("expected 1 rule, got %d", len(rules))
+	}
+	if len(rules[0].Protocols) != 2 {
+		t.Fatalf("expected 2 protocols, got %v", rules[0].Protocols)
+	}
+	protos := make(map[string]bool)
+	for _, p := range rules[0].Protocols {
+		protos[p] = true
+	}
+	if !protos["quic"] || !protos["udp"] {
+		t.Errorf("expected quic and udp, got %v", rules[0].Protocols)
+	}
+}
+
+func TestHandlePolicyAddInvalidProtocol(t *testing.T) {
+	dir := t.TempDir()
+	dbPath := filepath.Join(dir, "test.db")
+
+	err := handlePolicyAdd([]string{"allow", "--db", dbPath, "--protocols", "htp", "example.com"})
+	if err == nil {
+		t.Fatal("expected error for invalid protocol")
+	}
+	if !strings.Contains(err.Error(), "unknown protocol") {
+		t.Errorf("expected 'unknown protocol' in error, got: %v", err)
+	}
+}
+
+func TestHandlePolicyAddEmptyProtocol(t *testing.T) {
+	dir := t.TempDir()
+	dbPath := filepath.Join(dir, "test.db")
+
+	err := handlePolicyAdd([]string{"allow", "--db", dbPath, "--protocols", "quic,,udp", "example.com"})
+	if err == nil {
+		t.Fatal("expected error for empty protocol name")
+	}
+	if !strings.Contains(err.Error(), "empty protocol name") {
+		t.Errorf("expected 'empty protocol name' in error, got: %v", err)
+	}
+}
+
 // --- handlePolicyRemove tests ---
 
 func TestHandlePolicyRemoveValid(t *testing.T) {