From 4f36e63a0578679daa44ace280afab66b44eef5a Mon Sep 17 00:00:00 2001 From: Miek Gieben Date: Wed, 19 Oct 2016 17:46:03 +0100 Subject: [PATCH] middleware/file: fix DS handling (#344) The DS record is handled specially in the server ServeDNS mux, but there was no code that actually called the correct middleware handler chain when encountering a DS. This PR fixes that behavoir, additonal bugs has been files to look into how we are handling delegation (secure and non-secure ones). --- core/dnsserver/server.go | 18 ++++++++ middleware/file/ds_test.go | 92 ++++++++++++++++++++++++++++++++++++++ test/ds_file_test.go | 86 +++++++++++++++++++++++++++++++++++ test/miek_test.go | 31 +++++++++++++ 4 files changed, 227 insertions(+) create mode 100644 middleware/file/ds_test.go create mode 100644 test/ds_file_test.go create mode 100644 test/miek_test.go diff --git a/core/dnsserver/server.go b/core/dnsserver/server.go index b4e4bc13e..aa94dcd8c 100644 --- a/core/dnsserver/server.go +++ b/core/dnsserver/server.go @@ -177,6 +177,8 @@ func (s *Server) ServeDNS(w dns.ResponseWriter, r *dns.Msg) { off, end := 0, false ctx := context.Background() + var dshandler *Config + for { l := len(q[off:]) for i := 0; i < l; i++ { @@ -195,12 +197,28 @@ func (s *Server) ServeDNS(w dns.ResponseWriter, r *dns.Msg) { } return } + // The type is DS, keep the handler, but keep on searching as maybe we are serving + // the parent as well and the DS should be routed to it - this will probably *misroute* DS + // queries to a possibly grand parent, but there is no way for us to know at this point + // if there is an actually delegation from grandparent -> parent -> zone. + // In all fairness: direct DS queries should not be needed. + dshandler = h } off, end = dns.NextLabel(q, off) if end { break } } + + if dshandler != nil { + // DS request, and we found a zone, use the handler for the query + rcode, _ := dshandler.middlewareChain.ServeDNS(ctx, w, r) + if rcodeNoClientWrite(rcode) { + DefaultErrorFunc(w, r, rcode) + } + return + } + // Wildcard match, if we have found nothing try the root zone as a last resort. if h, ok := s.zones["."]; ok { rcode, _ := h.middlewareChain.ServeDNS(ctx, w, r) diff --git a/middleware/file/ds_test.go b/middleware/file/ds_test.go new file mode 100644 index 000000000..a7ba06263 --- /dev/null +++ b/middleware/file/ds_test.go @@ -0,0 +1,92 @@ +package file + +import ( + "sort" + "strings" + "testing" + + "github.com/miekg/coredns/middleware/pkg/dnsrecorder" + "github.com/miekg/coredns/middleware/test" + + "github.com/miekg/dns" + "golang.org/x/net/context" +) + +var dsTestCases = []test.Case{ + { + Qname: "a.delegated.miek.nl.", Qtype: dns.TypeDS, + Ns: []dns.RR{ + test.NS("delegated.miek.nl. 1800 IN NS a.delegated.miek.nl."), + test.NS("delegated.miek.nl. 1800 IN NS ns-ext.nlnetlabs.nl."), + }, + Extra: []dns.RR{ + test.A("a.delegated.miek.nl. 1800 IN A 139.162.196.78"), + test.AAAA("a.delegated.miek.nl. 1800 IN AAAA 2a01:7e00::f03c:91ff:fef1:6735"), + }, + }, + { + Qname: "_udp.delegated.miek.nl.", Qtype: dns.TypeDS, + Ns: []dns.RR{ + test.NS("delegated.miek.nl. 1800 IN NS a.delegated.miek.nl."), + test.NS("delegated.miek.nl. 1800 IN NS ns-ext.nlnetlabs.nl."), + }, + Extra: []dns.RR{ + test.A("a.delegated.miek.nl. 1800 IN A 139.162.196.78"), + test.AAAA("a.delegated.miek.nl. 1800 IN AAAA 2a01:7e00::f03c:91ff:fef1:6735"), + }, + }, + { + // This works *here* because we skip the server routing for DS in core/dnsserver/server.go + Qname: "_udp.miek.nl.", Qtype: dns.TypeDS, + Rcode: dns.RcodeNameError, + Ns: []dns.RR{ + test.SOA("miek.nl. 1800 IN SOA linode.atoom.net. miek.miek.nl. 1282630057 14400 3600 604800 14400"), + }, + }, + { + Qname: "miek.nl.", Qtype: dns.TypeDS, + Ns: []dns.RR{ + test.SOA("miek.nl. 1800 IN SOA linode.atoom.net. miek.miek.nl. 1282630057 14400 3600 604800 14400"), + }, + }, +} + +func TestLookupDS(t *testing.T) { + zone, err := Parse(strings.NewReader(dbMiekNLDelegation), testzone, "stdin") + if err != nil { + t.Fatalf("expect no error when reading zone, got %q", err) + } + + fm := File{Next: test.ErrorHandler(), Zones: Zones{Z: map[string]*Zone{testzone: zone}, Names: []string{testzone}}} + ctx := context.TODO() + + for _, tc := range dsTestCases { + m := tc.Msg() + + rec := dnsrecorder.New(&test.ResponseWriter{}) + _, err := fm.ServeDNS(ctx, rec, m) + if err != nil { + t.Errorf("expected no error, got %v\n", err) + return + } + + resp := rec.Msg + sort.Sort(test.RRSet(resp.Answer)) + sort.Sort(test.RRSet(resp.Ns)) + sort.Sort(test.RRSet(resp.Extra)) + + if !test.Header(t, tc, resp) { + t.Logf("%v\n", resp) + continue + } + if !test.Section(t, tc, test.Answer, resp.Answer) { + t.Logf("%v\n", resp) + } + if !test.Section(t, tc, test.Ns, resp.Ns) { + t.Logf("%v\n", resp) + } + if !test.Section(t, tc, test.Extra, resp.Extra) { + t.Logf("%v\n", resp) + } + } +} diff --git a/test/ds_file_test.go b/test/ds_file_test.go new file mode 100644 index 000000000..407e04b42 --- /dev/null +++ b/test/ds_file_test.go @@ -0,0 +1,86 @@ +package test + +import ( + "io/ioutil" + "log" + "sort" + "testing" + + "github.com/miekg/coredns/middleware/proxy" + mtest "github.com/miekg/coredns/middleware/test" + "github.com/miekg/coredns/request" + + "github.com/miekg/dns" +) + +// Using miek.nl here because this is the easiest zone to get access to and it's masters +// run both NSD and BIND9, making checks like "what should we actually return" super easy. +var dsTestCases = []mtest.Case{ + { + Qname: "_udp.miek.nl.", Qtype: dns.TypeDS, + Rcode: dns.RcodeNameError, + Ns: []dns.RR{ + mtest.SOA("miek.nl. 1800 IN SOA linode.atoom.net. miek.miek.nl. 1282630057 14400 3600 604800 14400"), + }, + }, + { + Qname: "miek.nl.", Qtype: dns.TypeDS, + Ns: []dns.RR{ + mtest.SOA("miek.nl. 1800 IN SOA linode.atoom.net. miek.miek.nl. 1282630057 14400 3600 604800 14400"), + }, + }, +} + +func TestLookupDS(t *testing.T) { + name, rm, err := TempFile(".", miekNL) + if err != nil { + t.Fatalf("failed to created zone: %s", err) + } + defer rm() + + corefile := `miek.nl:0 { + file ` + name + ` +} +` + + i, err := CoreDNSServer(corefile) + if err != nil { + t.Fatalf("Could not get CoreDNS serving instance: %s", err) + } + + udp, _ := CoreDNSServerPorts(i, 0) + if udp == "" { + t.Fatalf("Could not get UDP listening port") + } + defer i.Stop() + + log.SetOutput(ioutil.Discard) + + p := proxy.New([]string{udp}) + state := request.Request{W: &mtest.ResponseWriter{}, Req: new(dns.Msg)} + + for _, tc := range dsTestCases { + resp, err := p.Lookup(state, tc.Qname, tc.Qtype) + if err != nil || resp == nil { + t.Fatalf("Expected to receive reply, but didn't for %s %d", tc.Qname, tc.Qtype) + } + + sort.Sort(mtest.RRSet(resp.Answer)) + sort.Sort(mtest.RRSet(resp.Ns)) + sort.Sort(mtest.RRSet(resp.Extra)) + + if !mtest.Header(t, tc, resp) { + t.Logf("%v\n", resp) + continue + } + if !mtest.Section(t, tc, mtest.Answer, resp.Answer) { + t.Logf("%v\n", resp) + } + if !mtest.Section(t, tc, mtest.Ns, resp.Ns) { + t.Logf("%v\n", resp) + } + if !mtest.Section(t, tc, mtest.Extra, resp.Extra) { + t.Logf("%v\n", resp) + } + } +} diff --git a/test/miek_test.go b/test/miek_test.go new file mode 100644 index 000000000..2778817a3 --- /dev/null +++ b/test/miek_test.go @@ -0,0 +1,31 @@ +package test + +const miekNL = `; miek.nl test zone +$TTL 30M +$ORIGIN miek.nl. +@ IN SOA linode.atoom.net. miek.miek.nl. ( + 1282630059 ; Serial + 4H ; Refresh + 1H ; Retry + 7D ; Expire + 4H ) ; Negative Cache TTL + IN NS linode.atoom.net. + IN NS ns-ext.nlnetlabs.nl. + IN NS omval.tednet.nl. + IN NS ext.ns.whyscream.net. + + IN MX 1 aspmx.l.google.com. + IN MX 5 alt1.aspmx.l.google.com. + IN MX 5 alt2.aspmx.l.google.com. + IN MX 10 aspmx2.googlemail.com. + IN MX 10 aspmx3.googlemail.com. + + IN A 176.58.119.54 + IN AAAA 2a01:7e00::f03c:91ff:fe79:234c + IN HINFO "Please stop asking for ANY" "See draft-ietf-dnsop-refuse-any" + +a IN A 176.58.119.54 + IN AAAA 2a01:7e00::f03c:91ff:fe79:234c +www IN CNAME a +archive IN CNAME a +`