From a586275399acd5d88cc6ec0564c71b34b0903cd1 Mon Sep 17 00:00:00 2001 From: Nikita Nemirovsky Date: Wed, 8 Apr 2026 01:49:37 +0800 Subject: [PATCH] fix(proxy): forward DNS for all non-denied domains, remove ChannelGateMiddleware Two fixes: 1. DNS interceptor now only blocks domains with explicit deny rules. Previously, any domain not matching an allow rule got NXDOMAIN, silently preventing connections from reaching the SOCKS5 approval flow. Now DNS resolves for all domains, and policy enforcement (allow/ask/deny) happens at the SOCKS5 CONNECT level where the Telegram approval broker can prompt the user. 2. Removed ChannelGateMiddleware that blocked all /api/* routes when no HTTP channel was enabled. The API is now accessible whenever SLUICE_API_TOKEN is set, regardless of channel configuration. Bearer token auth is sufficient protection. --- cmd/sluice/main.go | 5 ++-- internal/api/server_test.go | 50 +---------------------------------- internal/policy/engine.go | 19 +++++++++++++ internal/proxy/dns.go | 28 ++++++++++---------- internal/proxy/dns_test.go | 18 +++++++++---- internal/proxy/server_test.go | 7 +++-- 6 files changed, 54 insertions(+), 73 deletions(-) diff --git a/cmd/sluice/main.go b/cmd/sluice/main.go index 4d88b76..f624da7 100644 --- a/cmd/sluice/main.go +++ b/cmd/sluice/main.go @@ -861,11 +861,10 @@ func startAPIServer(addr string, apiSrv *api.Server, st *store.Store, mcpHandler } // oapi-codegen wraps handlers bottom-up: last middleware in the slice // becomes the outermost layer. List channel gate first, then auth, so - // the request hits auth before the channel gate. This ensures bad tokens - // get 401 before the channel gate reveals whether HTTP channel is enabled. + // Bearer token auth protects all /api/* routes. The API is accessible + // whenever SLUICE_API_TOKEN is set, regardless of which channels are enabled. apiHandler := api.HandlerWithOptions(apiSrv, api.ChiServerOptions{ Middlewares: []api.MiddlewareFunc{ - api.ChannelGateMiddleware(st), api.BearerAuthMiddleware, }, }) diff --git a/internal/api/server_test.go b/internal/api/server_test.go index d833510..272b7f8 100644 --- a/internal/api/server_test.go +++ b/internal/api/server_test.go @@ -45,11 +45,10 @@ func enableHTTPChannel(t *testing.T, st *store.Store) { // oapi-codegen wraps handlers bottom-up: last middleware in the slice becomes // outermost. Channel gate goes first (innermost), auth second (outermost), // so auth rejects before channel gate reveals channel state. -func newTestHandler(t *testing.T, srv *api.Server, st *store.Store) http.Handler { +func newTestHandler(t *testing.T, srv *api.Server, _ *store.Store) http.Handler { t.Helper() return api.HandlerWithOptions(srv, api.ChiServerOptions{ Middlewares: []api.MiddlewareFunc{ - api.ChannelGateMiddleware(st), api.BearerAuthMiddleware, }, }) @@ -210,53 +209,6 @@ func TestAuth_BadTokenBeforeChannelCheck(t *testing.T) { // --- Channel gate middleware tests --- -func TestChannelGate_Disabled(t *testing.T) { - st := newTestStore(t) - // Default store has only Telegram channel (type=0), no HTTP channel - srv := api.NewServer(st, nil, nil, "") - - t.Setenv("SLUICE_API_TOKEN", "secret") - handler := newTestHandler(t, srv, st) - - req := httptest.NewRequest("GET", "/api/status", nil) - req.Header.Set("Authorization", "Bearer secret") - rec := httptest.NewRecorder() - handler.ServeHTTP(rec, req) - - if rec.Code != http.StatusForbidden { - t.Errorf("expected 403, got %d", rec.Code) - } - - var resp api.ErrorResponse - if err := json.NewDecoder(rec.Body).Decode(&resp); err != nil { - t.Fatalf("decode: %v", err) - } - if resp.Error != "HTTP channel is not enabled" { - t.Errorf("unexpected error: %q", resp.Error) - } - if resp.Code == nil || *resp.Code != "channel_disabled" { - t.Errorf("unexpected code: %v", resp.Code) - } -} - -func TestChannelGate_Enabled(t *testing.T) { - st := newTestStore(t) - enableHTTPChannel(t, st) - srv := api.NewServer(st, nil, nil, "") - - t.Setenv("SLUICE_API_TOKEN", "secret") - handler := newTestHandler(t, srv, st) - - req := httptest.NewRequest("GET", "/api/status", nil) - req.Header.Set("Authorization", "Bearer secret") - rec := httptest.NewRecorder() - handler.ServeHTTP(rec, req) - - if rec.Code != http.StatusOK { - t.Errorf("expected 200, got %d", rec.Code) - } -} - // --- Approval endpoint tests --- func TestGetApiApprovals_Empty(t *testing.T) { diff --git a/internal/policy/engine.go b/internal/policy/engine.go index bff0930..87823fd 100644 --- a/internal/policy/engine.go +++ b/internal/policy/engine.go @@ -381,6 +381,25 @@ func (e *Engine) IsDenied(dest string, port int) bool { return matchRules(e.compiled.denyRules, dest, port) } +// IsDeniedDomain checks if a domain matches any deny rule regardless of port +// or protocol. Used by the DNS interceptor to block resolution for explicitly +// denied domains while allowing all others through (policy enforcement happens +// at the SOCKS5 CONNECT level). +func (e *Engine) IsDeniedDomain(dest string) bool { + dest = normalizeDestination(dest) + e.mu.RLock() + defer e.mu.RUnlock() + if e.compiled == nil { + return false + } + for _, r := range e.compiled.denyRules { + if r.glob.Match(dest) { + return true + } + } + return false +} + // IsRestricted checks whether a destination and port match any explicit deny // or ask rule. Unlike Evaluate, this does not fall back to the default verdict. // Used for DNS rebinding checks where the original FQDN was already allowed diff --git a/internal/proxy/dns.go b/internal/proxy/dns.go index d0c2e8b..6c06b93 100644 --- a/internal/proxy/dns.go +++ b/internal/proxy/dns.go @@ -239,11 +239,18 @@ func (d *DNSInterceptor) HandleQuery(query []byte) ([]byte, error) { } domain := questions[0].Name - verdict := d.evaluate(domain) + + // Only block DNS for explicitly denied domains. All other verdicts + // (allow, ask, default) are forwarded. Policy enforcement for the + // actual connection happens at the SOCKS5 CONNECT level, not DNS. + // Blocking DNS for "ask" or default-deny domains would prevent the + // connection from ever reaching the approval flow. + eng := d.engine.Load() + denied := eng.IsDeniedDomain(domain) if d.audit != nil { verdictStr := "allow" - if verdict != policy.Allow { + if denied { verdictStr = "deny" } if logErr := d.audit.Log(audit.Event{ @@ -257,7 +264,7 @@ func (d *DNSInterceptor) HandleQuery(query []byte) ([]byte, error) { } } - if verdict != policy.Allow { + if denied { return BuildNXDOMAIN(query) } @@ -275,17 +282,10 @@ func (d *DNSInterceptor) HandleQuery(query []byte) ([]byte, error) { return resp, nil } -// evaluate checks the DNS domain against the policy engine. Uses -// EvaluateWithProtocol with protocol "dns" so dns-specific rules match. -func (d *DNSInterceptor) evaluate(domain string) policy.Verdict { - eng := d.engine.Load() - // Use EvaluateWithProtocol with "dns" so protocol-scoped rules match. - // DNS follows the same deny-then-allow-then-default semantics as - // regular evaluation, not the UDP default-deny semantics, because - // DNS queries are a known protocol with meaningful domain-level policy. - v := eng.EvaluateWithProtocol(domain, 53, ProtoDNS.String()) - return v -} +// NOTE: the old evaluate() method has been removed. DNS resolution is now +// allowed for all domains except those with explicit deny rules. Policy +// enforcement (allow/ask/deny) happens at the SOCKS5 CONNECT level where +// the approval broker can send Telegram notifications for "ask" verdicts. // forwardToResolver sends the query to the upstream DNS resolver and returns // the response. diff --git a/internal/proxy/dns_test.go b/internal/proxy/dns_test.go index a2dac50..d7456bc 100644 --- a/internal/proxy/dns_test.go +++ b/internal/proxy/dns_test.go @@ -447,6 +447,9 @@ func TestDNSInterceptor_DeniedDomain(t *testing.T) { eng, err := policy.LoadFromBytes([]byte(` [policy] default = "deny" + +[[deny]] +destination = "denied.example.com" `)) if err != nil { t.Fatal(err) @@ -464,7 +467,7 @@ default = "deny" t.Fatalf("HandleQuery: %v", err) } - // Response should be NXDOMAIN. + // Response should be NXDOMAIN (explicit deny rule). respID := binary.BigEndian.Uint16(resp[0:2]) if respID != 0x2222 { t.Errorf("response ID = 0x%04x, want 0x2222", respID) @@ -543,7 +546,9 @@ protocols = ["dns"] t.Errorf("evil.google.com RCODE = %d, want %d (NXDOMAIN)", rcode, dnsRcodeNXDomain) } - // Unmatched domain (default deny). + // Unmatched domain (default deny). DNS should still resolve so the + // connection reaches the SOCKS5 layer where the ask/deny flow runs. + // Only explicitly denied domains get NXDOMAIN at the DNS level. query = buildDNSQuery(0x5555, "other.com", dnsTypeA) resp, err = interceptor.HandleQuery(query) if err != nil { @@ -551,8 +556,8 @@ protocols = ["dns"] } flags = binary.BigEndian.Uint16(resp[2:4]) rcode = flags & 0x000F - if rcode != dnsRcodeNXDomain { - t.Errorf("other.com RCODE = %d, want %d (NXDOMAIN)", rcode, dnsRcodeNXDomain) + if rcode == dnsRcodeNXDomain { + t.Errorf("other.com RCODE = %d (NXDOMAIN), want forwarded (non-denied domains resolve via DNS)", rcode) } } @@ -877,10 +882,13 @@ func TestParseDNSNameEdgeCases(t *testing.T) { }) } -// TestHandleQueryDenyRule tests that a deny rule causes NXDOMAIN. +// TestHandleQueryDenyRule tests that an explicit deny rule causes NXDOMAIN. func TestHandleQueryDenyRule(t *testing.T) { eng, _ := policy.LoadFromBytes([]byte(`[policy] default = "deny" + +[[deny]] +destination = "blocked.example.com" `)) var enginePtr atomic.Pointer[policy.Engine] diff --git a/internal/proxy/server_test.go b/internal/proxy/server_test.go index c9e2c59..d5db5d2 100644 --- a/internal/proxy/server_test.go +++ b/internal/proxy/server_test.go @@ -1203,8 +1203,8 @@ protocols = ["dns"] } func TestUDPAssociateDNSInterceptionNXDOMAIN(t *testing.T) { - // DNS interceptor should return NXDOMAIN for denied domains without - // contacting the upstream resolver. + // DNS interceptor should return NXDOMAIN for explicitly denied domains + // without contacting the upstream resolver. eng, err := policy.LoadFromBytes([]byte(` [policy] default = "deny" @@ -1213,6 +1213,9 @@ default = "deny" destination = "allowed.example.com" ports = [53] protocols = ["dns"] + +[[deny]] +destination = "denied.example.com" `)) if err != nil { t.Fatal(err)