diff --git a/TODO.md b/TODO.md index 596779e..aa422dc 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 domain's NS set is its delegation from the parent zone's + servers, not whichever of its own servers answered first (closes #200). - 2026-10-01: README has Getting Started, Rationale and TODO sections, and its Architecture section is now Design, in the order policy sets (closes #173). - 2026-10-01: a zone's server that answers SERVFAIL or a referral leading no diff --git a/internal/resolver/export_test.go b/internal/resolver/export_test.go index 73c3a33..5c59228 100644 --- a/internal/resolver/export_test.go +++ b/internal/resolver/export_test.go @@ -16,6 +16,11 @@ func UsableReply(resp *dns.Msg, zone string, name string) bool { return usableReply(resp, zone, name) } +// NSSetFrom exports nsSetFrom for testing. +func NSSetFrom(resp *dns.Msg, domain string) []string { + return nsSetFrom(resp, domain) +} + // CollectIPs exports collectIPs for testing. func CollectIPs( results map[string]*NameserverResponse, diff --git a/internal/resolver/iterative.go b/internal/resolver/iterative.go index 7c5cfa7..d495193 100644 --- a/internal/resolver/iterative.go +++ b/internal/resolver/iterative.go @@ -222,9 +222,9 @@ func (r *Resolver) followDelegation( return nil, err } - ansNS := extractNSSet(resp.Answer) - if len(ansNS) > 0 { - return ansNS, nil + nsSet := nsSetFrom(resp, domain) + if len(nsSet) > 0 { + return nsSet, nil } // An authoritative reply comes from the servers of the zone @@ -325,6 +325,21 @@ func referralZone(resp *dns.Msg) string { return "" } +// nsSetFrom returns the NS set of domain that resp, a reply to a query +// for domain's NS records, gives: the delegation in a referral to domain +// itself, or else the NS records in the answer; empty when it gives +// neither. A referral to domain comes from its parent zone's servers, +// which all hold the same delegation, so the set does not depend on +// which of them answered. domain's own servers, which can disagree about +// their NS records, are then not asked. +func nsSetFrom(resp *dns.Msg, domain string) []string { + if referralZone(resp) == domain { + return extractNSSet(resp.Ns) + } + + return extractNSSet(resp.Answer) +} + func (r *Resolver) resolveNSIPs( ctx context.Context, nsNames []string, @@ -372,7 +387,7 @@ func (r *Resolver) resolveNSIterative( return nil, err } - nsNames := extractNSSet(resp.Answer) + nsNames := nsSetFrom(resp, domain) if len(nsNames) > 0 { return nsNames, nil } @@ -465,7 +480,8 @@ func (r *Resolver) resolveARecord( // FindAuthoritativeNameservers traces the delegation chain from // root servers to discover all authoritative nameservers for the -// given domain. For a name that is not a zone apex it tries each +// given domain, as the delegation from its parent zone's servers lists +// them. For a name that is not a zone apex it tries each // parent name in turn, so it returns the nameservers of the zone the // name is in. func (r *Resolver) FindAuthoritativeNameservers( @@ -631,9 +647,10 @@ func (r *Resolver) querySingleType( // A reply with no answer that lists other nameservers, from a server // that does not hold the name's zone, is a referral and says nothing - // about the name's records. A parent zone's servers send one when - // every server of the name's own zone failed and - // FindAuthoritativeNameservers moved on to the parent name. + // about the name's records. A server named in the delegation that + // does not hold the zone may send one, as do a parent zone's servers + // when FindAuthoritativeNameservers found no delegation for the + // name's zone and moved on to a parent name. if !msg.Authoritative && len(msg.Answer) == 0 && len(extractNSSet(msg.Ns)) > 0 { state.gotReferral = true diff --git a/internal/resolver/iterative_test.go b/internal/resolver/iterative_test.go index cc1b30b..413cc71 100644 --- a/internal/resolver/iterative_test.go +++ b/internal/resolver/iterative_test.go @@ -41,8 +41,15 @@ func TestCollectIPs_FailedIsNoAnswer(t *testing.T) { assert.Empty(t, ips) } -// exampleCom is the zone most cases of TestUsableReply are about. -const exampleCom = "example.com." +const ( + // exampleCom is the zone most cases of TestUsableReply and + // TestNSSetFrom are about, and wwwExampleCom a name in it. + exampleCom = "example.com." + wwwExampleCom = "www.example.com." + + // exampleNS is the server the NS records nsRecord builds name. + exampleNS = "ns1.example.net." +) // nsRecord builds an NS record that names a server of zone. func nsRecord(zone string) *dns.NS { @@ -50,7 +57,7 @@ func nsRecord(zone string) *dns.NS { Hdr: dns.RR_Header{ Name: zone, Rrtype: dns.TypeNS, Class: dns.ClassINET, }, - Ns: "ns1.example.net.", + Ns: exampleNS, } } @@ -105,7 +112,7 @@ func TestUsableReply(t *testing.T) { }, { name: "com refers to example.com", resp: referralTo(exampleCom), - zone: "com.", query: "www.example.com.", want: true, + zone: "com.", query: wwwExampleCom, want: true, }, { name: "referral back to the zone", resp: referralTo(exampleCom), @@ -132,6 +139,51 @@ func TestUsableReply(t *testing.T) { } } +// TestNSSetFrom checks which NS set a reply gives for a domain; a set +// that is not empty ends the walk. The referral to example.com that +// com's servers all send alike gives its delegation, so the set is the +// same whichever of them answered, and example.com's own servers, which +// can disagree, are not asked. +func TestNSSetFrom(t *testing.T) { + t.Parallel() + + answer := new(dns.Msg) + answer.Authoritative = true + answer.Answer = []dns.RR{nsRecord(exampleCom)} + + tests := []struct { + name string + resp *dns.Msg + domain string + want []string + }{ + { + name: "com refers to example.com", resp: referralTo(exampleCom), + domain: exampleCom, want: []string{exampleNS}, + }, + { + name: "com refers on, for www.example.com", + resp: referralTo(exampleCom), domain: wwwExampleCom, + want: nil, + }, + { + name: "answer from a server that holds example.com", + resp: answer, domain: exampleCom, + want: []string{exampleNS}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + assert.ElementsMatch(t, tt.want, + resolver.NSSetFrom(tt.resp, tt.domain), + ) + }) + } +} + func TestExtractRecordValue_LetterCase(t *testing.T) { t.Parallel()