diff --git a/plugin/file/lookup.go b/plugin/file/lookup.go index 7b5753278..6bbc310b3 100644 --- a/plugin/file/lookup.go +++ b/plugin/file/lookup.go @@ -434,8 +434,11 @@ func (z *Zone) doLookup(ctx context.Context, state request.Request, target strin } // additionalProcessing checks the current answer section and retrieves A or AAAA records -// (and possible SIGs) to need to be put in the additional section. +// (and possible SIGs) to need to be put in the additional section. A target referenced by +// more than one record is only resolved once. func (z *Zone) additionalProcessing(answer []dns.RR, do bool) (extra []dns.RR) { + var lookup map[string]struct{} + for _, rr := range answer { name := "" switch x := rr.(type) { @@ -452,6 +455,20 @@ func (z *Zone) additionalProcessing(answer []dns.RR, do bool) (extra []dns.RR) { continue } + // The answer can reference one target more than once, e.g. two MX records that only + // differ in preference. Its addresses belong in the additional section once. Compare + // canonically: SRV targets are not lowercased on insert (see Zone.Insert), while the + // zone's tree matches names case-insensitively. + target := dns.CanonicalName(name) + if _, ok := lookup[target]; ok { + continue + } + if lookup == nil { + // Allocate on first use: this runs for every answer, most of which carry no target. + lookup = make(map[string]struct{}, len(answer)) + } + lookup[target] = struct{}{} + elem, _ := z.Search(name) if elem == nil { continue diff --git a/plugin/file/lookup_test.go b/plugin/file/lookup_test.go index d7796705b..0c794ce7e 100644 --- a/plugin/file/lookup_test.go +++ b/plugin/file/lookup_test.go @@ -360,3 +360,148 @@ txt IN TXT "v=spf1 a mx ~all" caa IN CAA 0 issue letsencrypt.org *.nodata IN A 139.162.196.79 ext-cname IN CNAME example.com.` + +var additionalAuth = []dns.RR{ + test.NS("example.org. 1800 IN NS ns.example.org."), +} + +var additionalTestCases = []test.Case{ + { + // Two MX records that only differ in preference share a target. Its addresses + // belong in the additional section once. + Qname: "mx.example.org.", Qtype: dns.TypeMX, + Answer: []dns.RR{ + test.MX("mx.example.org. 1800 IN MX 10 host.example.org."), + test.MX("mx.example.org. 1800 IN MX 20 host.example.org."), + }, + Ns: additionalAuth, + Extra: []dns.RR{ + test.A("host.example.org. 1800 IN A 192.0.2.10"), + test.AAAA("host.example.org. 1800 IN AAAA 2001:db8::10"), + }, + }, + { + // SRV runs through the same additional processing. + Qname: "srv.example.org.", Qtype: dns.TypeSRV, + Answer: []dns.RR{ + test.SRV("srv.example.org. 1800 IN SRV 10 50 8080 host.example.org."), + test.SRV("srv.example.org. 1800 IN SRV 20 50 8080 host.example.org."), + }, + Ns: additionalAuth, + Extra: []dns.RR{ + test.A("host.example.org. 1800 IN A 192.0.2.10"), + test.AAAA("host.example.org. 1800 IN AAAA 2001:db8::10"), + }, + }, + { + // Distinct targets must each still be resolved. + Qname: "two.example.org.", Qtype: dns.TypeMX, + Answer: []dns.RR{ + test.MX("two.example.org. 1800 IN MX 10 host.example.org."), + test.MX("two.example.org. 1800 IN MX 20 other.example.org."), + }, + Ns: additionalAuth, + Extra: []dns.RR{ + test.A("host.example.org. 1800 IN A 192.0.2.10"), + test.AAAA("host.example.org. 1800 IN AAAA 2001:db8::10"), + test.A("other.example.org. 1800 IN A 192.0.2.20"), + test.AAAA("other.example.org. 1800 IN AAAA 2001:db8::20"), + }, + }, +} + +func TestAdditionalSectionDeduplication(t *testing.T) { + zone, err := Parse(strings.NewReader(dbAdditionalExample), testAdditionalOrigin, "stdin", 0) + if err != nil { + t.Fatalf("Expected no error when reading zone, got %q", err) + } + + fm := File{Next: test.ErrorHandler(), Zones: Zones{Z: map[string]*Zone{testAdditionalOrigin: zone}, Names: []string{testAdditionalOrigin}}} + ctx := context.TODO() + + for _, tc := range additionalTestCases { + m := tc.Msg() + + rec := dnstest.NewRecorder(&test.ResponseWriter{}) + _, err := fm.ServeDNS(ctx, rec, m) + if err != nil { + t.Errorf("Expected no error for %q/%d, got %v", tc.Qname, tc.Qtype, err) + continue + } + + resp := rec.Msg + if err := test.SortAndCheck(resp, tc); err != nil { + t.Errorf("Test %q/%d: %v", tc.Qname, tc.Qtype, err) + } + } +} + +func TestAdditionalSectionDeduplicationMixedCase(t *testing.T) { + // SRV targets are not lowercased on insert, so one target can reach additional + // processing under two spellings. The zone's tree matches names case-insensitively, + // so both resolve to the same addresses and those belong in the response once. + zone, err := Parse(strings.NewReader(dbAdditionalExample), testAdditionalOrigin, "stdin", 0) + if err != nil { + t.Fatalf("Expected no error when reading zone, got %q", err) + } + + fm := File{Next: test.ErrorHandler(), Zones: Zones{Z: map[string]*Zone{testAdditionalOrigin: zone}, Names: []string{testAdditionalOrigin}}} + ctx := context.TODO() + + m := new(dns.Msg) + m.SetQuestion("mixed.example.org.", dns.TypeSRV) + + rec := dnstest.NewRecorder(&test.ResponseWriter{}) + if _, err := fm.ServeDNS(ctx, rec, m); err != nil { + t.Fatalf("Expected no error, got %v", err) + } + + resp := rec.Msg + if len(resp.Answer) != 2 { + t.Fatalf("Expected 2 answers, got %d", len(resp.Answer)) + } + if len(resp.Extra) != 2 { + t.Errorf("Expected 2 additional records (one A, one AAAA), got %d: %v", len(resp.Extra), resp.Extra) + } +} + +const testAdditionalOrigin = "example.org." + +const dbAdditionalExample = ` +$TTL 30M +$ORIGIN example.org. +@ IN SOA ns.example.org. admin.example.org. ( + 2024010100 ; serial + 14400 ; refresh (4 hours) + 3600 ; retry (1 hour) + 604800 ; expire (1 week) + 14400 ; minimum (4 hours) + ) + IN NS ns.example.org. + +ns IN A 192.0.2.1 + +; The target shared by the records below. +host IN A 192.0.2.10 + IN AAAA 2001:db8::10 + +; A second, distinct target. +other IN A 192.0.2.20 + IN AAAA 2001:db8::20 + +; Two MX records differing only in preference, pointing at one target. +mx IN MX 10 host.example.org. + IN MX 20 host.example.org. + +; Two SRV records pointing at one target. +srv IN SRV 10 50 8080 host.example.org. + IN SRV 20 50 8080 host.example.org. + +; The same target, spelled differently. SRV targets are not normalized on insert. +mixed IN SRV 10 50 8080 host.example.org. + IN SRV 20 50 8080 HOST.EXAMPLE.ORG. + +; Two MX records with distinct targets. +two IN MX 10 host.example.org. + IN MX 20 other.example.org. +`