From c3f2a7ab16f066b73b04098dc5e627f7cee3a477 Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 1 Oct 2026 23:59:33 +0000 Subject: [PATCH] resolver: never resend a refused query asking for recursion (closes #206) queryDNS resent a query that a server refused, this time asking for recursion, so on a network that intercepts DNS the answers could come from a recursive resolver without anyone knowing. A refusal is now only a refusal, and the server is passed over for the next. When every server of a zone refuses, the error says so. When every root server refuses, the error is ErrIntercepted: root servers refuse no query, so something on the network is answering in their place. FindAuthoritativeNameservers stops at that error instead of trying each parent name, so the watcher's log line says it. A live test asks Quad9, which refuses a query not asking for recursion, so that the resend cannot come back unnoticed. Model: opus-5-5 --- TODO.md | 2 + internal/resolver/errors.go | 5 + internal/resolver/export_test.go | 11 +++ internal/resolver/iterative.go | 55 +++++++---- internal/resolver/resolver_test.go | 149 +++++++++++++++++++++++++++++ 5 files changed, 201 insertions(+), 21 deletions(-) diff --git a/TODO.md b/TODO.md index dca30de..e786f05 100644 --- a/TODO.md +++ b/TODO.md @@ -19,6 +19,8 @@ trial run of the finished image: https://git.eeqj.de/sneak/dnswatcher/issues/149 # Completed Steps +- 2026-10-02: a query a server refuses is not resent asking for recursion, and + every root server refusing is reported as DNS interception (closes #206). - 2026-10-02: a name listed more than once in `DNSWATCHER_TARGETS`, in any letter case or with a trailing dot, is watched once (closes #207). - 2026-10-01: README checked against the code and corrected: metrics, CORS, diff --git a/internal/resolver/errors.go b/internal/resolver/errors.go index 91eea16..5e019c0 100644 --- a/internal/resolver/errors.go +++ b/internal/resolver/errors.go @@ -22,6 +22,11 @@ var ( "reply is an error or a referral that leads no closer", ) + // ErrIntercepted is returned when every root server refused a + // query. Root servers refuse no query, so the refusals came from + // something on the network answering in their place. + ErrIntercepted = errors.New("this network intercepts DNS queries") + // ErrCNAMEDepthExceeded is returned when a CNAME chain // exceeds MaxCNAMEDepth. ErrCNAMEDepthExceeded = errors.New( diff --git a/internal/resolver/export_test.go b/internal/resolver/export_test.go index 5c59228..55da36c 100644 --- a/internal/resolver/export_test.go +++ b/internal/resolver/export_test.go @@ -28,6 +28,17 @@ func CollectIPs( return collectIPs(results) } +// QueryServers exports queryServers for testing. +func (r *Resolver) QueryServers( + ctx context.Context, + servers []string, + zone string, + name string, + qtype uint16, +) (*dns.Msg, error) { + return r.queryServers(ctx, servers, zone, name, qtype) +} + // QueryEachNS exports queryEachNS for testing. func (r *Resolver) QueryEachNS( ctx context.Context, diff --git a/internal/resolver/iterative.go b/internal/resolver/iterative.go index d495193..141a7d9 100644 --- a/internal/resolver/iterative.go +++ b/internal/resolver/iterative.go @@ -105,9 +105,8 @@ func (r *Resolver) retryTCP( return resp } -// queryDNS sends a DNS query to a specific server IP. -// Tries non-recursive first, falls back to recursive on -// REFUSED (handles DNS interception environments). +// queryDNS sends a DNS query to a specific server IP, never asking it +// for recursion. A reply of REFUSED is returned as ErrRefused. func (r *Resolver) queryDNS( ctx context.Context, serverIP string, @@ -131,25 +130,12 @@ func (r *Resolver) queryDNS( } if resp.Rcode == dns.RcodeRefused { - msg.RecursionDesired = true - - resp, err = r.tryExchange(ctx, msg, addr) - if err != nil { - return nil, fmt.Errorf( - "query %s @%s: %w", name, serverIP, err, - ) - } - - if resp.Rcode == dns.RcodeRefused { - return nil, fmt.Errorf( - "query %s @%s: %w", name, serverIP, ErrRefused, - ) - } + return nil, fmt.Errorf( + "query %s @%s: %w", name, serverIP, ErrRefused, + ) } - resp = r.retryTCP(ctx, msg, addr, resp) - - return resp, nil + return r.retryTCP(ctx, msg, addr, resp), nil } func extractNSSet(rrs []dns.RR) []string { @@ -261,7 +247,9 @@ func (r *Resolver) followDelegation( // queryServers asks servers, the servers of zone, about name until one // gives a usable reply. A server that times out, refuses or gives a -// reply that is not usable is passed over for the next. +// reply that is not usable is passed over for the next. When every +// server refused, the error says so, and when they are the root +// servers it is ErrIntercepted. func (r *Resolver) queryServers( ctx context.Context, servers []string, @@ -271,6 +259,8 @@ func (r *Resolver) queryServers( ) (*dns.Msg, error) { var lastErr error + refused := 0 + for _, ip := range servers { if checkCtx(ctx) != nil { return nil, ErrContextCanceled @@ -287,9 +277,27 @@ func (r *Resolver) queryServers( return resp, nil } + if errors.Is(err, ErrRefused) { + refused++ + } + lastErr = err } + if refused == len(servers) && zone == "." { + return nil, fmt.Errorf( + "every root server refused a query for %s: %w", + name, ErrIntercepted, + ) + } + + if refused == len(servers) { + return nil, fmt.Errorf( + "every server of %s refused a query for %s: %w", + zone, name, ErrRefused, + ) + } + return nil, fmt.Errorf("all servers failed: %w", lastErr) } @@ -510,6 +518,11 @@ func (r *Resolver) FindAuthoritativeNameservers( return nsNames, nil } + + // The root servers would refuse every parent name too. + if errors.Is(err, ErrIntercepted) { + return nil, err + } } return nil, ErrNoNameservers diff --git a/internal/resolver/resolver_test.go b/internal/resolver/resolver_test.go index dd585c8..9e6f335 100644 --- a/internal/resolver/resolver_test.go +++ b/internal/resolver/resolver_test.go @@ -11,6 +11,7 @@ import ( "testing" "time" + "github.com/miekg/dns" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -273,6 +274,154 @@ func TestQueryNameserver_Refused(t *testing.T) { assert.Equal(t, "server returned REFUSED", resp.Error) } +// TestQueryNameserverIP_RecursiveResolverRefused asks Quad9, a public +// recursive resolver, about google.com at both of its addresses. Quad9 +// refuses a query that does not ask for recursion and answers one that +// does. The resolver never asks for recursion, so it must be reported +// as refusing, never as answering. +func TestQueryNameserverIP_RecursiveResolverRefused(t *testing.T) { + t.Parallel() + + r := newTestResolver(t) + + for _, ip := range []string{"9.9.9.9", "149.112.112.112"} { + var resp *resolver.NameserverResponse + + livednstest.Retry( + t, + "QueryNameserverIP("+ip+", google.com)", + func(ctx context.Context) error { + var err error + + resp, err = r.QueryNameserverIP( + ctx, ip, ip, "google.com", + ) + if err != nil { + return err + } + + // A timeout or a network error is no reply at all. + if resp.Status == resolver.StatusTimeout || + strings.HasPrefix(resp.Error, "network error") { + return fmt.Errorf( + "%w: %s: %s", + livednstest.ErrNoAnswer, ip, resp.Error, + ) + } + + return nil + }, + ) + + assert.Equal(t, resolver.StatusError, resp.Status, ip) + assert.Equal(t, "server returned REFUSED", resp.Error, ip) + } +} + +// googleNameserverIPv4s returns the IPv4 addresses of google.com's +// nameservers. The resolver asks servers only at their IPv4 addresses. +func googleNameserverIPv4s(t *testing.T, r *resolver.Resolver) []string { + t.Helper() + + var servers []string + + for _, ns := range liveFindAuthoritative(t, r, "google.com") { + for _, ip := range liveResolveIPs(t, r, ns) { + if net.ParseIP(ip).To4() != nil { + servers = append(servers, ip) + } + } + } + + return servers +} + +// TestQueryServers_EveryServerRefused asks all of google.com's +// nameservers about cloudflare.com, a zone they do not serve, which +// they all refuse. The error says every server refused; it is not +// ErrIntercepted, which only the root servers refusing shows. +func TestQueryServers_EveryServerRefused(t *testing.T) { + t.Parallel() + + r := newTestResolver(t) + servers := googleNameserverIPv4s(t, r) + + var err error + + livednstest.Retry( + t, + "QueryServers(google.com servers, cloudflare.com)", + func(ctx context.Context) error { + _, err = r.QueryServers( + ctx, servers, "google.com.", "cloudflare.com.", + dns.TypeNS, + ) + + // When not every server refused, one may have given no + // reply at all, so the attempt is tried again. + if err != nil && + !strings.HasPrefix(err.Error(), "every server of") { + return fmt.Errorf( + "%w: %w", livednstest.ErrNoAnswer, err, + ) + } + + return nil + }, + ) + + require.ErrorIs(t, err, resolver.ErrRefused) + require.NotErrorIs(t, err, resolver.ErrIntercepted) + require.EqualError( + t, err, + "every server of google.com. refused a query for "+ + "cloudflare.com.: dns query refused", + ) +} + +// TestQueryServers_EveryRootServerRefused passes google.com's +// nameservers to QueryServers as the servers of the root zone. They +// refuse a query about cloudflare.com, as root servers would if +// something on the network answered in their place, so the error is +// ErrIntercepted. +func TestQueryServers_EveryRootServerRefused(t *testing.T) { + t.Parallel() + + r := newTestResolver(t) + servers := googleNameserverIPv4s(t, r) + + var err error + + livednstest.Retry( + t, + "QueryServers(google.com servers as root servers, cloudflare.com)", + func(ctx context.Context) error { + _, err = r.QueryServers( + ctx, servers, ".", "cloudflare.com.", dns.TypeNS, + ) + + // When not every server refused, one may have given no + // reply at all, so the attempt is tried again. Both errors + // for every server refusing say "refused a query for". + if err != nil && + !strings.Contains(err.Error(), "refused a query for") { + return fmt.Errorf( + "%w: %w", livednstest.ErrNoAnswer, err, + ) + } + + return nil + }, + ) + + require.ErrorIs(t, err, resolver.ErrIntercepted) + require.EqualError( + t, err, + "every root server refused a query for cloudflare.com.: "+ + "this network intercepts DNS queries", + ) +} + func TestQueryNameserver_RecordsSorted(t *testing.T) { t.Parallel()