From 1c0f9dc8108c082090e1cba8b41875db91eabeae 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. 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 | 53 ++++++++++++++++++++++++++++ 5 files changed, 105 insertions(+), 21 deletions(-) diff --git a/TODO.md b/TODO.md index 6b45c81..3211304 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-01: 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-01: a certificate within the expiry warning period is warned about on every TLS check, where some checks used to skip it at random (closes #204). - 2026-10-01: a domain's NS set is its delegation from the parent zone's 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..ccee30c 100644 --- a/internal/resolver/resolver_test.go +++ b/internal/resolver/resolver_test.go @@ -2,6 +2,7 @@ package resolver_test import ( "context" + "errors" "fmt" "log/slog" "net" @@ -11,6 +12,7 @@ import ( "testing" "time" + "github.com/miekg/dns" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -273,6 +275,57 @@ func TestQueryNameserver_Refused(t *testing.T) { assert.Equal(t, "server returned REFUSED", resp.Error) } +// 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) + + // The resolver asks servers only at their IPv4 addresses. + 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) + } + } + } + + 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, + ) + + // A server that did not reply at all did not refuse. + if err != nil && !errors.Is(err, resolver.ErrRefused) { + 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", + ) +} + func TestQueryNameserver_RecordsSorted(t *testing.T) { t.Parallel()