diff --git a/README.md b/README.md index 4f14c1d..d69dbc2 100644 --- a/README.md +++ b/README.md @@ -412,7 +412,12 @@ 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. +all of them are looked up, so that each can be asked. When it gives addresses +for only some of them, those are asked first, and the others are looked up and +asked only if none of those gives a usable reply. Both hold in the walk to a +name's nameservers and in the lookup of a nameserver's own address. Such a +lookup can 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..f8380ad 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: nameservers a referral gives no address for are looked up when + needed, two deep at most; `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..9109c7a 100644 --- a/internal/resolver/export_test.go +++ b/internal/resolver/export_test.go @@ -48,12 +48,31 @@ 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) +} + +// MaxLookupDepth exports maxLookupDepth for testing. +const MaxLookupDepth = maxLookupDepth + +// QueryNameservers exports queryNameservers for testing. +func (r *Resolver) QueryNameservers( + ctx context.Context, + given []string, + withoutAddresses []string, + zone string, + name string, + qtype uint16, + depth int, +) (*dns.Msg, error) { + return r.queryNameservers( + ctx, given, withoutAddresses, zone, name, qtype, depth, + ) } // RootServerList exports rootServerList for testing. diff --git a/internal/resolver/iterative.go b/internal/resolver/iterative.go index 1f06a24..ccf6160 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. @@ -198,13 +206,15 @@ func (r *Resolver) followDelegation( // servers are the root servers, the servers of zone ".". zone := "." + var withoutAddresses []string + for range maxDelegation { if checkCtx(ctx) != nil { return nil, ErrContextCanceled } - resp, err := r.queryServers( - ctx, servers, zone, domain, dns.TypeNS, + resp, err := r.queryNameservers( + ctx, servers, withoutAddresses, zone, domain, dns.TypeNS, 0, ) if err != nil { return nil, err @@ -229,18 +239,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) - } - - if len(nextServers) == 0 { - return nil, ErrNoNameservers - } - - servers = nextServers + servers, withoutAddresses = referralNameservers(resp) zone = referralZone(resp) } @@ -366,18 +365,80 @@ func nsSetFrom(resp *dns.Msg, domain string) []string { return extractNSSet(resp.Answer) } +// referralNameservers returns the IPv4 addresses that resp, a referral, +// gives for the nameservers it names, and the names of the nameservers +// it gives no address for. +func referralNameservers(resp *dns.Msg) ([]string, []string) { + glue := extractGlue(resp.Extra) + + var given, withoutAddresses []string + + for _, ns := range extractNSSet(resp.Ns) { + ips := glueIPs([]string{ns}, glue) + if len(ips) == 0 { + withoutAddresses = append(withoutAddresses, ns) + } + + given = append(given, ips...) + } + + return given, withoutAddresses +} + +// queryNameservers asks the servers of zone about name as queryServers +// does: first those at given, the addresses a referral gave, and only +// when none of them gives a usable reply, the nameservers named +// withoutAddresses, once their addresses are looked up. depth is how +// many lookups of a nameserver's address are under way, 0 in the walk +// to a domain's nameservers; at maxLookupDepth, no address is looked +// up. +func (r *Resolver) queryNameservers( + ctx context.Context, + given []string, + withoutAddresses []string, + zone string, + name string, + qtype uint16, + depth int, +) (*dns.Msg, error) { + err := fmt.Errorf( + "no address for any nameserver of %s: %w", zone, ErrNoNameservers, + ) + + if len(given) > 0 { + var resp *dns.Msg + + resp, err = r.queryServers(ctx, given, zone, name, qtype) + if err == nil { + return resp, nil + } + } + + if len(withoutAddresses) == 0 || depth >= maxLookupDepth { + return nil, err + } + + lookedUp := r.resolveNSIPs(ctx, withoutAddresses, depth+1) + if len(lookedUp) == 0 { + return nil, err + } + + return r.queryServers(ctx, lookedUp, zone, name, qtype) +} + // 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 +499,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 @@ -452,13 +516,16 @@ func (r *Resolver) resolveARecord( servers := rootServerList() zone := "." + var withoutAddresses []string + for range maxDelegation { if checkCtx(ctx) != nil { return nil, ErrContextCanceled } - resp, err := r.queryServers( - ctx, servers, zone, hostname, dns.TypeA, + resp, err := r.queryNameservers( + ctx, servers, withoutAddresses, zone, hostname, dns.TypeA, + depth, ) if err != nil { return nil, fmt.Errorf( @@ -485,17 +552,7 @@ func (r *Resolver) resolveARecord( break } - glue := extractGlue(resp.Extra) - nextServers := glueIPs(authNS, glue) - - if len(nextServers) == 0 { - // Resolve NS IPs iteratively — but guard - // against infinite recursion by using only - // already-resolved servers. - break - } - - servers = nextServers + servers, withoutAddresses = referralNameservers(resp) zone = referralZone(resp) } @@ -584,7 +641,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..ab4ebf3 100644 --- a/internal/resolver/resolver_test.go +++ b/internal/resolver/resolver_test.go @@ -162,6 +162,85 @@ 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) + } +} + +// TestQueryNameservers_GivenAddressesFail asks the servers of ntp.org +// about pool.ntp.org, as the walk to a name under ntp.org does after the +// org servers' referral. That referral names four nameservers and gives +// an address for ns1.everett.org alone; here the given address is +// 192.0.2.1, where nothing answers, so the other three must be looked +// up and asked. +func TestQueryNameservers_GivenAddressesFail(t *testing.T) { + t.Parallel() + + r := newTestResolver(t) + + var resp *dns.Msg + + livednstest.Retry( + t, + "QueryNameservers(192.0.2.1 and three ntp.org nameservers, "+ + "pool.ntp.org)", + func(ctx context.Context) error { + var err error + + resp, err = r.QueryNameservers( + ctx, []string{"192.0.2.1"}, + []string{"anyns.pch.net.", "dns1.udel.edu.", "dns2.udel.edu."}, + "ntp.org.", "pool.ntp.org.", dns.TypeNS, 0, + ) + + return err + }, + ) + + assert.NotEmpty(t, resolver.NSSetFrom(resp, "pool.ntp.org.")) +} + +// TestQueryNameservers_LookupDepth asks the servers of desec.io about +// ns1.desec.io, giving no address and naming ns1.desec.io without one. +// Below maxLookupDepth its address is looked up and it is asked. At the +// limit it is not, so nothing can be asked: this is where lookups of +// nameserver addresses stop when delegations point at each other. +func TestQueryNameservers_LookupDepth(t *testing.T) { + t.Parallel() + + r := newTestResolver(t) + withoutAddresses := []string{"ns1.desec.io."} + + livednstest.Retry( + t, + "QueryNameservers(ns1.desec.io without its address, ns1.desec.io)", + func(ctx context.Context) error { + _, err := r.QueryNameservers( + ctx, nil, withoutAddresses, "desec.io.", "ns1.desec.io.", + dns.TypeA, 0, + ) + + return err + }, + ) + + _, err := r.QueryNameservers( + t.Context(), nil, withoutAddresses, "desec.io.", "ns1.desec.io.", + dns.TypeA, resolver.MaxLookupDepth, + ) + require.ErrorIs(t, err, resolver.ErrNoNameservers) +} + // ---------------------------------------------------------------- // QueryNameserver tests // ----------------------------------------------------------------