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

README.md

@@ -407,6 +407,13 @@ it watches. Instead, it performs full iterative resolution:
 4. **Authoritative query**: Queries all discovered authoritative nameservers
    directly for the requested records.
 
+In steps 2 and 3 the servers are asked one at a time in a random order, chosen
+anew each time, so no one root server gets every first query. A server that does
+not reply, refuses the query, or gives an error reply such as SERVFAIL or a
+referral that leads no closer to the name is passed over for the next one. When
+a referral names a zone's nameservers without their addresses, the addresses of
+all of them are looked up, so that each can be asked.
+
 This approach ensures:
 
 - Independence from any upstream resolver's cache or filtering.

TODO.md

@@ -19,6 +19,10 @@ 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
   letter case or with a trailing dot, is watched once (closes #207).
 - 2026-10-01: README checked against the code and corrected: metrics, CORS,
@@ -131,5 +135,4 @@ trial run of the finished image: https://git.eeqj.de/sneak/dnswatcher/issues/149
 
 - 1.0 readiness: run it with a real config and read the logs:
   https://git.eeqj.de/sneak/dnswatcher/issues/66
-- fixed root server order: https://git.eeqj.de/sneak/dnswatcher/issues/138
 - review toward 1.0: https://git.eeqj.de/sneak/dnswatcher/issues/144

internal/livednstest/livednstest.go

@@ -9,11 +9,11 @@
 //
 //  1. Bounded concurrency. Tests run in parallel and the build hosts
 //     have many cores, so without a limit every test starts its own
-//     iterative resolution at the same instant and they all hit the
+//     iterative resolution at the same instant and they all send their
-//     first root server within a few milliseconds of each other. Root
+//     first queries to the root servers within a few milliseconds of
-//     servers rate-limit that, which shows up as a different arbitrary
+//     each other. Root servers rate-limit that, which shows up as a
-//     subset of tests failing on each run. Run caps how many live
+//     different arbitrary subset of tests failing on each run. Run caps
-//     operations are in flight at once in one test binary.
+//     how many live operations are in flight at once in one test binary.
 //
 //  2. Retry with exponential backoff. Each live operation gets several
 //     attempts with its own timeout. An attempt is retried when it

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(

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,
@@ -36,3 +47,24 @@ func (r *Resolver) QueryEachNS(
 ) (map[string]*NameserverResponse, error) {
 	return r.queryEachNS(ctx, nameservers, hostname)
 }
+
+// ResolveNSIPs exports resolveNSIPs for testing.
+func (r *Resolver) ResolveNSIPs(
+	ctx context.Context,
+	nsNames []string,
+) []string {
+	return r.resolveNSIPs(ctx, nsNames)
+}
+
+// RootServerList exports rootServerList for testing.
+func RootServerList() []string {
+	return rootServerList()
+}
+
+// Shuffled exports shuffled for testing.
+func Shuffled(
+	servers []string,
+	shuffle func(n int, swap func(i, j int)),
+) []string {
+	return shuffled(servers, shuffle)
+}

internal/resolver/iterative.go

@@ -4,7 +4,9 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"math/rand/v2"
 	"net"
+	"slices"
 	"sort"
 	"strings"
 	"time"
@@ -105,9 +107,8 @@ func (r *Resolver) retryTCP(
 	return resp
 }
 
-// queryDNS sends a DNS query to a specific server IP.
+// queryDNS sends a DNS query to a specific server IP, never asking it
-// Tries non-recursive first, falls back to recursive on
+// for recursion. A reply of REFUSED is returned as ErrRefused.
-// REFUSED (handles DNS interception environments).
 func (r *Resolver) queryDNS(
 	ctx context.Context,
 	serverIP string,
@@ -131,25 +132,12 @@ func (r *Resolver) queryDNS(
 	}
 
 	if resp.Rcode == dns.RcodeRefused {
-		msg.RecursionDesired = true
+		return nil, fmt.Errorf(
-
+			"query %s @%s: %w", name, serverIP, ErrRefused,
-		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,
-			)
-		}
 	}
 
-	resp = r.retryTCP(ctx, msg, addr, resp)
+	return r.retryTCP(ctx, msg, addr, resp), nil
-
-	return resp, nil
 }
 
 func extractNSSet(rrs []dns.RR) []string {
@@ -259,9 +247,27 @@ func (r *Resolver) followDelegation(
 	return nil, ErrNoNameservers
 }
 
-// queryServers asks servers, the servers of zone, about name until one
+// shuffled returns a copy of servers in the order shuffle puts them
-// gives a usable reply. A server that times out, refuses or gives a
+// in. The resolver passes rand.Shuffle, so each time it walks a list of
-// reply that is not usable is passed over for the next.
+// servers it starts at a random one, and no one server gets every
+// first query.
+func shuffled(
+	servers []string,
+	shuffle func(n int, swap func(i, j int)),
+) []string {
+	order := slices.Clone(servers)
+	shuffle(len(order), func(i, j int) {
+		order[i], order[j] = order[j], order[i]
+	})
+
+	return order
+}
+
+// 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. 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,7 +277,9 @@ func (r *Resolver) queryServers(
 ) (*dns.Msg, error) {
 	var lastErr error
 
-	for _, ip := range servers {
+	refused := 0
+
+	for _, ip := range shuffled(servers, rand.Shuffle) {
 		if checkCtx(ctx) != nil {
 			return nil, ErrContextCanceled
 		}
@@ -287,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)
 }
 
@@ -340,6 +366,10 @@ func nsSetFrom(resp *dns.Msg, domain string) []string {
 	return extractNSSet(resp.Answer)
 }
 
+// resolveNSIPs returns the addresses of every nameserver in nsNames
+// whose name resolves, for a referral that carries none. The walk can
+// then go on to the zone's other nameservers when one gives no usable
+// reply.
 func (r *Resolver) resolveNSIPs(
 	ctx context.Context,
 	nsNames []string,
@@ -351,10 +381,6 @@ func (r *Resolver) resolveNSIPs(
 		if err == nil {
 			ips = append(ips, resolved...)
 		}
-
-		if len(ips) > 0 {
-			break
-		}
 	}
 
 	return ips
@@ -510,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

internal/resolver/iterative_test.go

@@ -1,6 +1,8 @@
 package resolver_test
 
 import (
+	"math/rand/v2"
+	"slices"
 	"testing"
 
 	"github.com/miekg/dns"
@@ -235,3 +237,30 @@ func TestExtractRecordValue_LetterCase(t *testing.T) {
 		})
 	}
 }
+
+// TestShuffled shuffles the root servers with many seeds. Every order
+// must hold each root server once, so each is tried before a
+// resolution fails; each root server must come first for some seed, so
+// no one root server gets every first query; and the list passed in
+// must be left as it was.
+func TestShuffled(t *testing.T) {
+	t.Parallel()
+
+	const seeds = 1000
+
+	roots := resolver.RootServerList()
+	before := slices.Clone(roots)
+	first := make(map[string]bool)
+
+	for seed := range uint64(seeds) {
+		rng := rand.New(rand.NewPCG(seed, 0)) //nolint:gosec // seeded on purpose
+		order := resolver.Shuffled(roots, rng.Shuffle)
+
+		assert.ElementsMatch(t, roots, order)
+
+		first[order[0]] = true
+	}
+
+	assert.Len(t, first, len(roots))
+	assert.Equal(t, before, roots)
+}

internal/resolver/livedns_test.go

@@ -383,3 +383,37 @@ func liveResolveIPsAllowingEmpty(
 
 	return out
 }
+
+// liveResolveNSIPs looks up the addresses of the nameservers named
+// names, retrying until there are at least atLeast of them: a name
+// whose lookup got no reply is left out of the result, not an error.
+func liveResolveNSIPs(
+	t *testing.T,
+	r *resolver.Resolver,
+	names []string,
+	atLeast int,
+) []string {
+	t.Helper()
+
+	var out []string
+
+	livednstest.Retry(
+		t,
+		"ResolveNSIPs("+strings.Join(names, ", ")+")",
+		func(ctx context.Context) error {
+			ips := r.ResolveNSIPs(ctx, names)
+			if len(ips) < atLeast {
+				return fmt.Errorf(
+					"%w: %d addresses, expected at least %d",
+					livednstest.ErrNoAnswer, len(ips), atLeast,
+				)
+			}
+
+			out = ips
+
+			return nil
+		},
+	)
+
+	return out
+}

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"
 
@@ -139,6 +140,28 @@ func TestFindAuthoritativeNameservers_CloudflareDomain(
 	}
 }
 
+// TestResolveNSIPs_EveryNameserver looks up the addresses of two of
+// google.com's nameservers together, as the walk does when a referral
+// names a zone's nameservers without their addresses, and compares them
+// with each looked up alone. Together they must give the addresses of
+// both, not only of the first that resolves, so that when one gives no
+// usable reply the walk goes on to the other.
+func TestResolveNSIPs_EveryNameserver(t *testing.T) {
+	t.Parallel()
+
+	r := newTestResolver(t)
+	names := []string{"ns3.google.com.", "ns4.google.com."}
+	want := make([]string, 0, len(names))
+
+	for _, name := range names {
+		want = append(want, liveResolveNSIPs(t, r, []string{name}, 1)...)
+	}
+
+	got := liveResolveNSIPs(t, r, names, len(want))
+
+	assert.ElementsMatch(t, want, got)
+}
+
 // ----------------------------------------------------------------
 // QueryNameserver tests
 // ----------------------------------------------------------------
@@ -273,6 +296,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()