diff --git a/TODO.md b/TODO.md index dc9e0a3..8972d49 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: a zone's server that answers SERVFAIL or a referral leading no + closer is passed over for the next, as one that times out is (closes #197). - 2026-10-01: when none of a configured name's nameservers answered, the port state saved for its addresses is kept, not removed (closes #193). - 2026-10-01: `ResolveIPAddresses` returns an error, not no addresses, when no diff --git a/internal/resolver/errors.go b/internal/resolver/errors.go index ab05678..91eea16 100644 --- a/internal/resolver/errors.go +++ b/internal/resolver/errors.go @@ -15,6 +15,13 @@ var ( // so whether the name has addresses is unknown. ErrNoNameserverAnswered = errors.New("no nameserver answered") + // ErrUnusableReply is returned when a server replied with an + // error such as SERVFAIL, or with a referral that leads no + // closer to the name asked about. + ErrUnusableReply = errors.New( + "reply is an error or a referral that leads no closer", + ) + // ErrCNAMEDepthExceeded is returned when a CNAME chain // exceeds MaxCNAMEDepth. ErrCNAMEDepthExceeded = errors.New( diff --git a/internal/resolver/export_test.go b/internal/resolver/export_test.go index 9569a3a..73c3a33 100644 --- a/internal/resolver/export_test.go +++ b/internal/resolver/export_test.go @@ -11,6 +11,11 @@ func ExtractRecordValue(rr dns.RR) string { return extractRecordValue(rr) } +// UsableReply exports usableReply for testing. +func UsableReply(resp *dns.Msg, zone string, name string) bool { + return usableReply(resp, zone, name) +} + // CollectIPs exports collectIPs for testing. func CollectIPs( results map[string]*NameserverResponse, diff --git a/internal/resolver/iterative.go b/internal/resolver/iterative.go index a4c88ab..7c5cfa7 100644 --- a/internal/resolver/iterative.go +++ b/internal/resolver/iterative.go @@ -207,13 +207,16 @@ func (r *Resolver) followDelegation( domain string, servers []string, ) ([]string, error) { + // servers are the root servers, the servers of zone ".". + zone := "." + for range maxDelegation { if checkCtx(ctx) != nil { return nil, ErrContextCanceled } resp, err := r.queryServers( - ctx, servers, domain, dns.TypeNS, + ctx, servers, zone, domain, dns.TypeNS, ) if err != nil { return nil, err @@ -250,14 +253,19 @@ func (r *Resolver) followDelegation( } servers = nextServers + zone = referralZone(resp) } return nil, ErrNoNameservers } +// queryServers asks servers, the servers of zone, about name until one +// gives a usable reply. A server that times out, refuses or gives a +// reply that is not usable is passed over for the next. func (r *Resolver) queryServers( ctx context.Context, servers []string, + zone string, name string, qtype uint16, ) (*dns.Msg, error) { @@ -269,6 +277,12 @@ func (r *Resolver) queryServers( } resp, err := r.queryDNS(ctx, ip, name, qtype) + if err == nil && !usableReply(resp, zone, name) { + err = fmt.Errorf( + "query %s @%s: %w", name, ip, ErrUnusableReply, + ) + } + if err == nil { return resp, nil } @@ -279,6 +293,38 @@ func (r *Resolver) queryServers( return nil, fmt.Errorf("all servers failed: %w", lastErr) } +// usableReply reports whether resp, a reply from one of the servers of +// zone to a query about name, is usable. An error reply such as SERVFAIL +// is not. Nor is a referral, unless it refers the query to a zone below +// zone that name is in: a server that refers it back to zone, up or +// sideways does not serve zone as it should. +func usableReply(resp *dns.Msg, zone string, name string) bool { + if resp.Rcode != dns.RcodeSuccess && resp.Rcode != dns.RcodeNameError { + return false + } + + child := referralZone(resp) + if resp.Authoritative || len(resp.Answer) > 0 || child == "" { + return true + } + + return child != zone && dns.IsSubDomain(zone, child) && + dns.IsSubDomain(child, name) +} + +// referralZone returns the zone a referral refers the query to: the +// owner name of the NS records in resp's authority section, or "" when +// there are none. +func referralZone(resp *dns.Msg) string { + for _, rr := range resp.Ns { + if ns, ok := rr.(*dns.NS); ok { + return strings.ToLower(ns.Hdr.Name) + } + } + + return "" +} + func (r *Resolver) resolveNSIPs( ctx context.Context, nsNames []string, @@ -312,6 +358,7 @@ func (r *Resolver) resolveNSIterative( domain = dns.Fqdn(domain) servers := rootServerList() + zone := "." for range maxDelegation { if checkCtx(ctx) != nil { @@ -319,7 +366,7 @@ func (r *Resolver) resolveNSIterative( } resp, err := r.queryServers( - ctx, servers, domain, dns.TypeNS, + ctx, servers, zone, domain, dns.TypeNS, ) if err != nil { return nil, err @@ -344,6 +391,7 @@ func (r *Resolver) resolveNSIterative( } servers = nextServers + zone = referralZone(resp) } return nil, ErrNoNameservers @@ -361,6 +409,7 @@ func (r *Resolver) resolveARecord( hostname = dns.Fqdn(hostname) servers := rootServerList() + zone := "." for range maxDelegation { if checkCtx(ctx) != nil { @@ -368,7 +417,7 @@ func (r *Resolver) resolveARecord( } resp, err := r.queryServers( - ctx, servers, hostname, dns.TypeA, + ctx, servers, zone, hostname, dns.TypeA, ) if err != nil { return nil, fmt.Errorf( @@ -406,6 +455,7 @@ func (r *Resolver) resolveARecord( } servers = nextServers + zone = referralZone(resp) } return nil, fmt.Errorf( diff --git a/internal/resolver/iterative_test.go b/internal/resolver/iterative_test.go index f3440e7..cc1b30b 100644 --- a/internal/resolver/iterative_test.go +++ b/internal/resolver/iterative_test.go @@ -41,6 +41,97 @@ func TestCollectIPs_FailedIsNoAnswer(t *testing.T) { assert.Empty(t, ips) } +// exampleCom is the zone most cases of TestUsableReply are about. +const exampleCom = "example.com." + +// nsRecord builds an NS record that names a server of zone. +func nsRecord(zone string) *dns.NS { + return &dns.NS{ + Hdr: dns.RR_Header{ + Name: zone, Rrtype: dns.TypeNS, Class: dns.ClassINET, + }, + Ns: "ns1.example.net.", + } +} + +// referralTo builds a reply that refers the query to the servers of +// zone. +func referralTo(zone string) *dns.Msg { + msg := new(dns.Msg) + msg.Ns = []dns.RR{nsRecord(zone)} + + return msg +} + +// TestUsableReply checks which replies from one of a zone's servers are +// used. A reply that is not usable moves the query on to the zone's +// next server. +func TestUsableReply(t *testing.T) { + t.Parallel() + + servfail := new(dns.Msg) + servfail.Rcode = dns.RcodeServerFailure + + answer := new(dns.Msg) + answer.Authoritative = true + answer.Answer = []dns.RR{nsRecord(exampleCom)} + + nxdomain := new(dns.Msg) + nxdomain.Authoritative = true + nxdomain.Rcode = dns.RcodeNameError + + tests := []struct { + name string + resp *dns.Msg + zone string + query string + want bool + }{ + { + name: "SERVFAIL", resp: servfail, + zone: exampleCom, query: exampleCom, want: false, + }, + { + name: "answer", resp: answer, + zone: exampleCom, query: exampleCom, want: true, + }, + { + name: "NXDOMAIN", resp: nxdomain, + zone: ".", query: exampleCom, want: true, + }, + { + name: "root refers to com", resp: referralTo("com."), + zone: ".", query: exampleCom, want: true, + }, + { + name: "com refers to example.com", resp: referralTo(exampleCom), + zone: "com.", query: "www.example.com.", want: true, + }, + { + name: "referral back to the zone", resp: referralTo(exampleCom), + zone: exampleCom, query: exampleCom, want: false, + }, + { + name: "referral up to the root", resp: referralTo("."), + zone: exampleCom, query: exampleCom, want: false, + }, + { + name: "referral sideways", resp: referralTo("net."), + zone: ".", query: exampleCom, want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + assert.Equal(t, tt.want, + resolver.UsableReply(tt.resp, tt.zone, tt.query), + ) + }) + } +} + func TestExtractRecordValue_LetterCase(t *testing.T) { t.Parallel()