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) +}