diff --git a/TODO.md b/TODO.md index 49e335e..8dd73b2 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: the resolver tries root servers, and every other server list it walks, in a random order each time, not always from the top (closes #138). - 2026-10-02: a name listed more than once in `DNSWATCHER_TARGETS`, in any 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 868174c..2b4ddbf 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 0c526e5..aacccb4 100644 --- a/internal/resolver/iterative.go +++ b/internal/resolver/iterative.go @@ -107,9 +107,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, @@ -133,25 +132,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 { @@ -279,7 +265,9 @@ func shuffled( // queryServers asks servers, the servers of zone, about name in a random // order 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. +// or gives a 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, @@ -289,6 +277,8 @@ func (r *Resolver) queryServers( ) (*dns.Msg, error) { var lastErr error + refused := 0 + for _, ip := range shuffled(servers, rand.Shuffle) { if checkCtx(ctx) != nil { return nil, ErrContextCanceled @@ -305,9 +295,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) } @@ -528,6 +536,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 abb6b92..ce2617a 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" @@ -295,6 +296,195 @@ 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", + ) +} + +// TestQueryServers_NotEveryRootServerRefused passes google.com's +// nameservers and 192.0.2.1 to QueryServers as the servers of the root +// zone. The google.com nameservers refuse a query about cloudflare.com, +// but nothing answers at 192.0.2.1, a documentation address, so not +// every server refused, wherever 192.0.2.1 falls in the random order: +// the error is not ErrIntercepted and does not say every server refused. +func TestQueryServers_NotEveryRootServerRefused(t *testing.T) { + t.Parallel() + + r := newTestResolver(t) + servers := googleNameserverIPv4s(t, r) + servers = append(servers, "192.0.2.1") + + var err error + + livednstest.Retry( + t, + "QueryServers(google.com servers and 192.0.2.1, cloudflare.com)", + func(ctx context.Context) error { + _, err = r.QueryServers( + ctx, servers, ".", "cloudflare.com.", dns.TypeNS, + ) + + // An attempt that ran out of time may not have asked every + // server, so it is tried again. + if ctx.Err() != nil { + return fmt.Errorf( + "%w: %w", livednstest.ErrNoAnswer, err, + ) + } + + return nil + }, + ) + + require.Error(t, err) + require.NotErrorIs(t, err, resolver.ErrIntercepted) + // Both errors for every server refusing say "refused a query for". + require.NotContains(t, err.Error(), "refused a query for") +} + func TestQueryNameserver_RecordsSorted(t *testing.T) { t.Parallel()