diff --git a/README.md b/README.md index 4f14c1d..cdfa817 100644 --- a/README.md +++ b/README.md @@ -411,8 +411,12 @@ 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. +a referral names a zone's nameservers without their addresses, or gives +addresses for only some of them, the addresses of the others are looked up, so +that each can be asked. This holds both in the walk to a name's nameservers and +in the lookup of a nameserver's own address. Such a lookup may need others in +turn; lookups go at most two deep, one inside another, so delegations that point +at each other still end. This approach ensures: diff --git a/TODO.md b/TODO.md index 927a598..3d322a6 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: every nameserver a referral names without an address is looked up, + two deep at most, so `pool.ntp.org`'s nameservers resolve (closes #221). - 2026-10-02: a resolver test that reads one record type from a nameserver's answer asks again when that type is missing from it (closes #218). - 2026-10-02: a plain `docker build .` of a clone stamps its tag or short diff --git a/internal/resolver/export_test.go b/internal/resolver/export_test.go index 17c57d7..a592f24 100644 --- a/internal/resolver/export_test.go +++ b/internal/resolver/export_test.go @@ -48,12 +48,22 @@ func (r *Resolver) QueryEachNS( return r.queryEachNS(ctx, nameservers, hostname, recordTypes()) } -// ResolveNSIPs exports resolveNSIPs for testing. +// ResolveNSIPs exports resolveNSIPs for testing, looking each name up +// as a lookup that no other lookup started. func (r *Resolver) ResolveNSIPs( ctx context.Context, nsNames []string, ) []string { - return r.resolveNSIPs(ctx, nsNames) + return r.resolveNSIPs(ctx, nsNames, 1) +} + +// ReferralServers exports referralServers for testing, for a referral +// met in the walk to a domain's nameservers. +func (r *Resolver) ReferralServers( + ctx context.Context, + resp *dns.Msg, +) []string { + return r.referralServers(ctx, resp, 0) } // RootServerList exports rootServerList for testing. diff --git a/internal/resolver/iterative.go b/internal/resolver/iterative.go index 1f06a24..d9b6582 100644 --- a/internal/resolver/iterative.go +++ b/internal/resolver/iterative.go @@ -19,6 +19,14 @@ const ( maxRetries = 2 maxDelegation = 20 timeoutMultiplier = 2 + + // maxLookupDepth is how many lookups of nameserver addresses may be + // under way one inside another. Looking up a nameserver's address + // can meet a referral that names nameservers without their + // addresses, which are then looked up in turn; without a limit, + // delegations that point at each other would never end. Each level + // multiplies the queries sent. + maxLookupDepth = 2 ) // ErrRefused is returned when a DNS server refuses a query. @@ -229,13 +237,7 @@ func (r *Resolver) followDelegation( return r.resolveNSIterative(ctx, domain) } - glue := extractGlue(resp.Extra) - nextServers := glueIPs(authNS, glue) - - if len(nextServers) == 0 { - nextServers = r.resolveNSIPs(ctx, authNS) - } - + nextServers := r.referralServers(ctx, resp, 0) if len(nextServers) == 0 { return nil, ErrNoNameservers } @@ -366,18 +368,50 @@ func nsSetFrom(resp *dns.Msg, domain string) []string { return extractNSSet(resp.Answer) } +// referralServers returns the IPv4 addresses of every nameserver that +// resp, a referral, names: the addresses resp gives for it, or else the +// addresses its name resolves to. depth is how many lookups of a +// nameserver's address the referral was met in, 0 in the walk to a +// domain's nameservers; at maxLookupDepth, the nameservers resp gives +// no addresses for are left out. +func (r *Resolver) referralServers( + ctx context.Context, + resp *dns.Msg, + depth int, +) []string { + glue := extractGlue(resp.Extra) + + var ips, withoutAddresses []string + + for _, ns := range extractNSSet(resp.Ns) { + nsIPs := glueIPs([]string{ns}, glue) + if len(nsIPs) == 0 { + withoutAddresses = append(withoutAddresses, ns) + } + + ips = append(ips, nsIPs...) + } + + if depth < maxLookupDepth { + ips = append(ips, r.resolveNSIPs(ctx, withoutAddresses, depth+1)...) + } + + return ips +} + // 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. +// whose name resolves, each looked up at depth (see resolveARecord). +// 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, + depth int, ) []string { var ips []string for _, ns := range nsNames { - resolved, err := r.resolveARecord(ctx, ns) + resolved, err := r.resolveARecord(ctx, ns, depth) if err == nil { ips = append(ips, resolved...) } @@ -438,11 +472,14 @@ func (r *Resolver) resolveNSIterative( return nil, ErrNoNameservers } -// resolveARecord resolves a hostname to IPv4 addresses using -// iterative resolution through the delegation chain. +// resolveARecord resolves a hostname, a nameserver's name, to IPv4 +// addresses using iterative resolution through the delegation chain. +// depth is how many lookups of a nameserver's address are under way, +// this one included: 1 for a lookup that no other lookup started. func (r *Resolver) resolveARecord( ctx context.Context, hostname string, + depth int, ) ([]string, error) { if checkCtx(ctx) != nil { return nil, ErrContextCanceled @@ -485,13 +522,8 @@ func (r *Resolver) resolveARecord( break } - glue := extractGlue(resp.Extra) - nextServers := glueIPs(authNS, glue) - + nextServers := r.referralServers(ctx, resp, depth) if len(nextServers) == 0 { - // Resolve NS IPs iteratively — but guard - // against infinite recursion by using only - // already-resolved servers. break } @@ -584,7 +616,7 @@ func (r *Resolver) queryNameserver( return nil, ErrContextCanceled } - nsIPs, err := r.resolveARecord(ctx, nsHostname) + nsIPs, err := r.resolveARecord(ctx, nsHostname, 1) if err != nil { return nil, fmt.Errorf("resolving NS %s: %w", nsHostname, err) } diff --git a/internal/resolver/resolver_test.go b/internal/resolver/resolver_test.go index a07c718..42ccfed 100644 --- a/internal/resolver/resolver_test.go +++ b/internal/resolver/resolver_test.go @@ -162,6 +162,109 @@ func TestResolveNSIPs_EveryNameserver(t *testing.T) { assert.ElementsMatch(t, want, got) } +// TestResolveNSIPs_ZoneDelegatedWithoutAddresses looks up the address +// of a.ntpns.org, a nameserver of pool.ntp.org. The org servers delegate +// ntpns.org to nameservers in other zones and give none of their +// addresses, so those are looked up on the way. +func TestResolveNSIPs_ZoneDelegatedWithoutAddresses(t *testing.T) { + t.Parallel() + + r := newTestResolver(t) + ips := liveResolveNSIPs(t, r, []string{"a.ntpns.org."}, 1) + + for _, ip := range ips { + assert.NotNil(t, net.ParseIP(ip), "should be valid IP: %s", ip) + } +} + +// TestResolveNSIPs_DelegationsPointAtEachOther looks up the address of +// ns1.desec.io. The io servers delegate desec.io to ns1.desec.io, with +// its address, and to ns2.desec.org, without; the org servers delegate +// desec.org to ns2.desec.org, with its address, and to ns1.desec.io, +// without. Looking up either address starts a lookup of the other, and +// the lookup must still end. +func TestResolveNSIPs_DelegationsPointAtEachOther(t *testing.T) { + t.Parallel() + + r := newTestResolver(t) + ips := liveResolveNSIPs(t, r, []string{"ns1.desec.io."}, 1) + + for _, ip := range ips { + assert.NotNil(t, net.ParseIP(ip), "should be valid IP: %s", ip) + } +} + +// givenAddresses returns the IPv4 addresses that resp, a referral, +// gives for the nameservers it names. +func givenAddresses(resp *dns.Msg) []string { + var ips []string + + for _, rr := range resp.Extra { + if a, ok := rr.(*dns.A); ok { + ips = append(ips, a.A.String()) + } + } + + return ips +} + +// TestReferralServers_SomeWithoutAddresses asks the org servers about +// ntp.org, as the walk to a name under ntp.org does. Their referral +// names four nameservers and gives an address for ns1.everett.org +// alone. The addresses of the other three are looked up, so that the +// walk can ask them too. +func TestReferralServers_SomeWithoutAddresses(t *testing.T) { + t.Parallel() + + r := newTestResolver(t) + + var referral *dns.Msg + + livednstest.Retry( + t, + "referral for ntp.org from the org servers", + func(ctx context.Context) error { + fromRoot, err := r.QueryServers( + ctx, resolver.RootServerList(), ".", "ntp.org.", + dns.TypeNS, + ) + if err != nil { + return err + } + + referral, err = r.QueryServers( + ctx, givenAddresses(fromRoot), "org.", "ntp.org.", + dns.TypeNS, + ) + + return err + }, + ) + + given := givenAddresses(referral) + require.NotEmpty(t, given, "the referral gives no addresses") + + var servers []string + + livednstest.Retry( + t, + "ReferralServers(referral for ntp.org)", + func(ctx context.Context) error { + servers = r.ReferralServers(ctx, referral) + if len(servers) <= len(given) { + return fmt.Errorf( + "%w: %d addresses, the referral gives %d", + livednstest.ErrNoAnswer, len(servers), len(given), + ) + } + + return nil + }, + ) + + assert.Subset(t, servers, given) +} + // ---------------------------------------------------------------- // QueryNameserver tests // ----------------------------------------------------------------