diff --git a/TODO.md b/TODO.md index 6b45c81..f2f1fba 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-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..56c7f76 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,59 @@ 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, + ) + + // 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", + ) +} + func TestQueryNameserver_RecordsSorted(t *testing.T) { t.Parallel()