resolver: ask a referral's nameservers that come without addresses (closes #221)

Looking up a nameserver's own address followed only the addresses a
referral gave, so a nameserver whose zone is delegated without them,
such as a.ntpns.org of pool.ntp.org, never resolved. The walk to a
name's nameservers looked addresses up only when a referral gave none.
Both now go through queryZone, which asks the nameservers whose
addresses the referral gives first and, if none of them gives a usable
reply, looks up and asks the others. maxLookupDepth stops lookups three
deep, so delegations that point at each other still end; when the limit
is why no address was found, the error is ErrLookupDepthExceeded, not
"no address".

Model: opus-5-5

internal/resolver/errors.go

@@ -33,6 +33,13 @@ var (
 		"CNAME chain depth exceeded",
 	)
 
+	// ErrLookupDepthExceeded is returned when nameserver addresses
+	// were not looked up because lookups were already maxLookupDepth
+	// deep, one inside another.
+	ErrLookupDepthExceeded = errors.New(
+		"lookups of nameserver addresses go too deep",
+	)
+
 	// ErrContextCanceled wraps context cancellation for the
 	// resolver's iterative queries.
 	ErrContextCanceled = errors.New("context canceled")

internal/resolver/export_test.go

@@ -61,14 +61,16 @@ func (r *Resolver) ResolveNSIPs(
 	ctx context.Context,
 	nsNames []string,
 ) []string {
-	return r.resolveNSIPs(ctx, nsNames, 1)
+	ips, _ := r.resolveNSIPs(ctx, nsNames, 1)
+
+	return ips
 }
 
 // MaxLookupDepth exports maxLookupDepth for testing.
 const MaxLookupDepth = maxLookupDepth
 
-// QueryNameservers exports queryNameservers for testing.
+// QueryZone exports queryZone for testing.
-func (r *Resolver) QueryNameservers(
+func (r *Resolver) QueryZone(
 	ctx context.Context,
 	given []string,
 	withoutAddresses []string,
@@ -77,7 +79,7 @@ func (r *Resolver) QueryNameservers(
 	qtype uint16,
 	depth int,
 ) (*dns.Msg, error) {
-	return r.queryNameservers(
+	return r.queryZone(
 		ctx, given, withoutAddresses, zone, name, qtype, depth,
 	)
 }

internal/resolver/iterative.go

@@ -215,7 +215,7 @@ func (r *Resolver) followDelegation(
 			return nil, ErrContextCanceled
 		}
 
-		resp, err := r.queryNameservers(
+		resp, err := r.queryZone(
 			ctx, servers, withoutAddresses, zone, domain, dns.TypeNS, 0,
 		)
 		if err != nil {
@@ -387,14 +387,15 @@ func referralNameservers(resp *dns.Msg) ([]string, []string) {
 	return given, withoutAddresses
 }
 
-// queryNameservers asks the servers of zone about name as queryServers
+// queryZone asks the servers of zone about name as queryServers does:
-// does: first those at given, the addresses a referral gave, and only
+// first those at given, the addresses a referral gave, and only when
-// when none of them gives a usable reply, the nameservers named
+// 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.
+// up. When the limit is why none was found, here or in a lookup this
-func (r *Resolver) queryNameservers(
+// one started, the error is ErrLookupDepthExceeded.
+func (r *Resolver) queryZone(
 	ctx context.Context,
 	given []string,
 	withoutAddresses []string,
@@ -416,11 +417,22 @@ func (r *Resolver) queryNameservers(
 		}
 	}
 
-	if len(withoutAddresses) == 0 || depth >= maxLookupDepth {
+	if len(withoutAddresses) == 0 {
 		return nil, err
 	}
 
-	lookedUp := r.resolveNSIPs(ctx, withoutAddresses, depth+1)
+	if depth >= maxLookupDepth {
+		return nil, fmt.Errorf(
+			"addresses of the nameservers of %s not looked up: %w",
+			zone, ErrLookupDepthExceeded,
+		)
+	}
+
+	lookedUp, limitErr := r.resolveNSIPs(ctx, withoutAddresses, depth+1)
+	if limitErr != nil {
+		return nil, limitErr
+	}
+
 	if len(lookedUp) == 0 {
 		return nil, err
 	}
@@ -431,22 +443,34 @@ func (r *Resolver) queryNameservers(
 // resolveNSIPs returns the addresses of every nameserver in nsNames
 // 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.
+// gives no usable reply. When none resolves and the depth limit
+// stopped one of the lookups, it returns that lookup's error.
 func (r *Resolver) resolveNSIPs(
 	ctx context.Context,
 	nsNames []string,
 	depth int,
-) []string {
+) ([]string, error) {
-	var ips []string
+	var (
+		ips      []string
+		limitErr error
+	)
 
 	for _, ns := range nsNames {
 		resolved, err := r.resolveARecord(ctx, ns, depth)
-		if err == nil {
+
+		switch {
+		case err == nil:
 			ips = append(ips, resolved...)
+		case errors.Is(err, ErrLookupDepthExceeded):
+			limitErr = err
 		}
 	}
 
-	return ips
+	if len(ips) > 0 {
+		return ips, nil
+	}
+
+	return nil, limitErr
 }
 
 // resolveNSIterative queries for NS records using iterative
@@ -525,7 +549,7 @@ func (r *Resolver) resolveARecord(
 			return nil, ErrContextCanceled
 		}
 
-		resp, err := r.queryNameservers(
+		resp, err := r.queryZone(
 			ctx, servers, withoutAddresses, zone, hostname, dns.TypeA,
 			depth,
 		)

internal/resolver/resolver_test.go

@@ -2,6 +2,7 @@ package resolver_test
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"log/slog"
 	"net"
@@ -177,13 +178,13 @@ func TestResolveNSIPs_ZoneDelegatedWithoutAddresses(t *testing.T) {
 	}
 }
 
-// TestQueryNameservers_GivenAddressesFail asks the servers of ntp.org
+// TestQueryZone_GivenAddressesFail asks the servers of ntp.org about
-// about pool.ntp.org, as the walk to a name under ntp.org does after the
+// pool.ntp.org, as the walk to a name under ntp.org does after the org
-// org servers' referral. That referral names four nameservers and gives
+// servers' referral. That referral names four nameservers and gives an
-// an address for ns1.everett.org alone; here the given address is
+// 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) {
+func TestQueryZone_GivenAddressesFail(t *testing.T) {
 	t.Parallel()
 
 	r := newTestResolver(t)
@@ -192,12 +193,11 @@ func TestQueryNameservers_GivenAddressesFail(t *testing.T) {
 
 	livednstest.Retry(
 		t,
-		"QueryNameservers(192.0.2.1 and three ntp.org nameservers, "+
+		"QueryZone(192.0.2.1 and three ntp.org nameservers, pool.ntp.org)",
-			"pool.ntp.org)",
 		func(ctx context.Context) error {
 			var err error
 
-			resp, err = r.QueryNameservers(
+			resp, err = r.QueryZone(
 				ctx, []string{"192.0.2.1"},
 				[]string{"anyns.pch.net.", "dns1.udel.edu.", "dns2.udel.edu."},
 				"ntp.org.", "pool.ntp.org.", dns.TypeNS, 0,
@@ -210,7 +210,7 @@ func TestQueryNameservers_GivenAddressesFail(t *testing.T) {
 	assert.NotEmpty(t, resolver.NSSetFrom(resp, "pool.ntp.org."))
 }
 
-// TestQueryNameservers_LookupDepth asks the servers of g.ntpns.org, a
+// TestQueryZone_LookupDepth asks the servers of g.ntpns.org, a
 // nameserver of pool.ntp.org, for its address, as looking that address
 // up does when anyns.pch.net, one of the servers of ntpns.org, gives the
 // referral to g.ntpns.org without addresses. Their addresses are looked
@@ -219,8 +219,8 @@ func TestQueryNameservers_GivenAddressesFail(t *testing.T) {
 // addresses. From depth 1, where looking up g.ntpns.org's address
 // starts, that makes three lookups and the address is found. From one
 // below maxLookupDepth, the bitnames.com lookup would be past the limit,
-// so nothing can be asked.
+// so nothing can be asked, and the error says the limit is why.
-func TestQueryNameservers_LookupDepth(t *testing.T) {
+func TestQueryZone_LookupDepth(t *testing.T) {
 	t.Parallel()
 
 	r := newTestResolver(t)
@@ -230,11 +230,11 @@ func TestQueryNameservers_LookupDepth(t *testing.T) {
 
 	livednstest.Retry(
 		t,
-		"QueryNameservers(a.ntpns.org without its address, g.ntpns.org)",
+		"QueryZone(a.ntpns.org without its address, g.ntpns.org)",
 		func(ctx context.Context) error {
 			var err error
 
-			resp, err = r.QueryNameservers(
+			resp, err = r.QueryZone(
 				ctx, nil, withoutAddresses, "g.ntpns.org.", "g.ntpns.org.",
 				dns.TypeA, 1,
 			)
@@ -245,11 +245,28 @@ func TestQueryNameservers_LookupDepth(t *testing.T) {
 
 	assert.NotEmpty(t, resp.Answer)
 
-	_, err := r.QueryNameservers(
+	var limitErr error
-		t.Context(), nil, withoutAddresses, "g.ntpns.org.", "g.ntpns.org.",
+
-		dns.TypeA, resolver.MaxLookupDepth-1,
+	// Any other error is live DNS not answering, and is retried.
+	livednstest.Retry(
+		t,
+		"QueryZone(a.ntpns.org without its address, g.ntpns.org, "+
+			"one below the limit)",
+		func(ctx context.Context) error {
+			_, limitErr = r.QueryZone(
+				ctx, nil, withoutAddresses, "g.ntpns.org.", "g.ntpns.org.",
+				dns.TypeA, resolver.MaxLookupDepth-1,
+			)
+			if limitErr == nil ||
+				errors.Is(limitErr, resolver.ErrLookupDepthExceeded) {
+				return nil
+			}
+
+			return limitErr
+		},
 	)
-	require.ErrorIs(t, err, resolver.ErrNoNameservers)
+
+	require.ErrorIs(t, limitErr, resolver.ErrLookupDepthExceeded)
 }
 
 // ----------------------------------------------------------------
@@ -275,6 +292,20 @@ func TestQueryNameserver_BasicA(t *testing.T) {
 	)
 }
 
+// TestQueryNameserver_ZoneDelegatedWithoutAddresses asks a.ntpns.org, a
+// nameserver of pool.ntp.org, about pool.ntp.org, as the watcher does.
+// The org servers delegate ntpns.org without the addresses of its
+// nameservers, so finding a.ntpns.org's address needs a lookup inside
+// the one QueryNameserver starts.
+func TestQueryNameserver_ZoneDelegatedWithoutAddresses(t *testing.T) {
+	t.Parallel()
+
+	r := newTestResolver(t)
+	resp := liveQueryNameserver(t, r, "a.ntpns.org.", "pool.ntp.org", "A")
+
+	assert.Equal(t, resolver.StatusOK, resp.Status)
+}
+
 func TestQueryNameserver_AAAA(t *testing.T) {
 	t.Parallel()