From 017eede779992d25fafedab316d38171dc2267e6 Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 1 Oct 2026 22:29:23 +0000 Subject: [PATCH] resolver: try servers in a random order on each resolution (closes #138) Every resolution walked the root servers in a fixed order, so a.root-servers.net got every first query and its timeouts were paid on every lookup. Each list of servers the resolver walks, the root servers and the nameservers of each zone below them, is now walked in a random order from the standard library's rand.Shuffle, chosen anew each time. Failover is unchanged: after a failure the next server is tried, and the walk fails only when every server has. The shuffle is passed in, so the tests check the order with a seeded source; which server a live query reached is not observable, so no test fails if the walk stops shuffling. Model: opus-5-5 --- README.md | 4 ++++ TODO.md | 3 ++- internal/resolver/export_test.go | 13 +++++++++++++ internal/resolver/iterative.go | 26 ++++++++++++++++++++++++-- internal/resolver/iterative_test.go | 29 +++++++++++++++++++++++++++++ 5 files changed, 72 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index b25ef86..9a8b627 100644 --- a/README.md +++ b/README.md @@ -399,6 +399,10 @@ 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, until one replies; the lookup fails only when none does. No one +root server gets every first query. + This approach ensures: - Independence from any upstream resolver's cache or filtering. diff --git a/TODO.md b/TODO.md index 10f64bb..10acba0 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: 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-01: `ResolveIPAddresses` returns an error, not no addresses, when no nameserver of the name's zone answered (closes #190). - 2026-10-01: `make fmt` and `make fmt-check` cover Markdown with prettier, run @@ -120,5 +122,4 @@ trial run of the finished image: https://git.eeqj.de/sneak/dnswatcher/issues/149 - README accuracy sweep: https://git.eeqj.de/sneak/dnswatcher/issues/108 - README sections required by policy: https://git.eeqj.de/sneak/dnswatcher/issues/173 -- fixed root server order: https://git.eeqj.de/sneak/dnswatcher/issues/138 - review toward 1.0: https://git.eeqj.de/sneak/dnswatcher/issues/144 diff --git a/internal/resolver/export_test.go b/internal/resolver/export_test.go index 9569a3a..a479a24 100644 --- a/internal/resolver/export_test.go +++ b/internal/resolver/export_test.go @@ -26,3 +26,16 @@ func (r *Resolver) QueryEachNS( ) (map[string]*NameserverResponse, error) { return r.queryEachNS(ctx, nameservers, hostname) } + +// 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) +} diff --git a/internal/resolver/iterative.go b/internal/resolver/iterative.go index a4c88ab..56a4497 100644 --- a/internal/resolver/iterative.go +++ b/internal/resolver/iterative.go @@ -4,7 +4,9 @@ import ( "context" "errors" "fmt" + "math/rand/v2" "net" + "slices" "sort" "strings" "time" @@ -255,6 +257,24 @@ func (r *Resolver) followDelegation( return nil, ErrNoNameservers } +// shuffled returns a copy of servers in the order shuffle puts them +// in. The resolver passes rand.Shuffle, so each time it walks a list of +// 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 the servers in a random order and returns the +// first reply; it fails only when every server has failed. func (r *Resolver) queryServers( ctx context.Context, servers []string, @@ -263,7 +283,7 @@ func (r *Resolver) queryServers( ) (*dns.Msg, error) { var lastErr error - for _, ip := range servers { + for _, ip := range shuffled(servers, rand.Shuffle) { if checkCtx(ctx) != nil { return nil, ErrContextCanceled } @@ -279,13 +299,15 @@ func (r *Resolver) queryServers( return nil, fmt.Errorf("all servers failed: %w", lastErr) } +// resolveNSIPs returns the addresses of one of the nameservers, trying +// their names in a random order until one resolves. func (r *Resolver) resolveNSIPs( ctx context.Context, nsNames []string, ) []string { var ips []string - for _, ns := range nsNames { + for _, ns := range shuffled(nsNames, rand.Shuffle) { resolved, err := r.resolveARecord(ctx, ns) if err == nil { ips = append(ips, resolved...) diff --git a/internal/resolver/iterative_test.go b/internal/resolver/iterative_test.go index f3440e7..65b1e93 100644 --- a/internal/resolver/iterative_test.go +++ b/internal/resolver/iterative_test.go @@ -1,6 +1,8 @@ package resolver_test import ( + "math/rand/v2" + "slices" "testing" "github.com/miekg/dns" @@ -92,3 +94,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) +}