diff --git a/middleware/etcd/group_test.go b/middleware/etcd/group_test.go index 11d88e3e6..0e50e69da 100644 --- a/middleware/etcd/group_test.go +++ b/middleware/etcd/group_test.go @@ -29,7 +29,7 @@ func TestGroupLookup(t *testing.T) { _, err := etc.ServeDNS(ctx, rec, m) if err != nil { t.Errorf("expected no error, got %v\n", err) - return + continue } resp := rec.Msg() diff --git a/middleware/etcd/handler.go b/middleware/etcd/handler.go index 6488e7549..91cd40e61 100644 --- a/middleware/etcd/handler.go +++ b/middleware/etcd/handler.go @@ -22,6 +22,7 @@ func (e Etcd) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (i name := state.Name() if e.Stubmap != nil && len(*e.Stubmap) > 0 { for zone, _ := range *e.Stubmap { + // TODO(miek): use the Match function. if strings.HasSuffix(name, zone) { stub := Stub{Etcd: e, Zone: zone} return stub.ServeDNS(ctx, w, r) diff --git a/middleware/etcd/other_test.go b/middleware/etcd/other_test.go index cdf304d92..d814e2102 100644 --- a/middleware/etcd/other_test.go +++ b/middleware/etcd/other_test.go @@ -31,7 +31,7 @@ func TestOtherLookup(t *testing.T) { _, err := etc.ServeDNS(ctx, rec, m) if err != nil { t.Errorf("expected no error, got %v\n", err) - return + continue } resp := rec.Msg() diff --git a/middleware/etcd/stub.go b/middleware/etcd/stub.go index 849bafa5b..83611b896 100644 --- a/middleware/etcd/stub.go +++ b/middleware/etcd/stub.go @@ -1,6 +1,7 @@ package etcd import ( + "log" "net" "strconv" "strings" @@ -23,46 +24,47 @@ func (e Etcd) UpdateStubZones() { // Look in .../dns/stub//xx for msg.Services. Loop through them // extract and add them as forwarders (ip:port-combos) for // the stub zones. Only numeric (i.e. IP address) hosts are used. -func (e Etcd) updateStubZones() { +// Only the first zone configured on e is used for the lookup. +func (e *Etcd) updateStubZones() { + zone := e.Zones[0] + services, err := e.Records(stubDomain+"."+zone, false) + if err != nil { + return + } + stubmap := make(map[string]proxy.Proxy) - for _, zone := range e.Zones { - services, err := e.Records(stubDomain+"."+zone, false) - if err != nil { + // track the nameservers on a per domain basis, but allow a list on the domain. + nameservers := map[string][]string{} + + for _, serv := range services { + if serv.Port == 0 { + serv.Port = 53 + } + ip := net.ParseIP(serv.Host) + if ip == nil { + log.Printf("[WARNING] Non IP address stub nameserver: %s", serv.Host) continue } - // track the nameservers on a per domain basis, but allow a list on the domain. - nameservers := map[string][]string{} + domain := e.Domain(serv.Key) + labels := dns.SplitDomainName(domain) - for _, serv := range services { - if serv.Port == 0 { - serv.Port = 53 - } - ip := net.ParseIP(serv.Host) - if ip == nil { + // If the remaining name equals any of the zones we have, we ignore it. + for _, z := range e.Zones { + // Chop of left most label, because that is used as the nameserver place holder + // and drop the right most labels that belong to zone. + // We must *also* chop of dns.stub. which means cutting two more labels. + domain = dns.Fqdn(strings.Join(labels[1:len(labels)-dns.CountLabel(z)-2], ".")) + if domain == z { + log.Printf("[WARNING] Skipping nameserver for domain we are authoritative for: %s", domain) continue } - - domain := e.Domain(serv.Key) - labels := dns.SplitDomainName(domain) - // nameserver need to be tracked by domain and *then* added - - // If the remaining name equals any of the zones we have, we ignore it. - for _, z := range e.Zones { - // Chop of left most label, because that is used as the nameserver place holder - // and drop the right most labels that belong to zone. - domain = dns.Fqdn(strings.Join(labels[1:len(labels)-dns.CountLabel(z)], ".")) - if domain == z { - continue - } - nameservers[domain] = append(nameservers[domain], net.JoinHostPort(serv.Host, strconv.Itoa(serv.Port))) - } - } - for domain, nss := range nameservers { - stubmap[domain] = proxy.New(nss) + nameservers[domain] = append(nameservers[domain], net.JoinHostPort(serv.Host, strconv.Itoa(serv.Port))) } } - + for domain, nss := range nameservers { + stubmap[domain] = proxy.New(nss) + } // atomic swap (at least that's what we hope it is) if len(stubmap) > 0 { e.Stubmap = &stubmap diff --git a/middleware/etcd/stub_handler.go b/middleware/etcd/stub_handler.go index 44a2f9ce8..4c00e82ee 100644 --- a/middleware/etcd/stub_handler.go +++ b/middleware/etcd/stub_handler.go @@ -1,7 +1,10 @@ package etcd import ( + "log" + "github.com/miekg/coredns/middleware" + "github.com/miekg/dns" "golang.org/x/net/context" ) @@ -14,8 +17,8 @@ type Stub struct { func (s Stub) ServeDNS(ctx context.Context, w dns.ResponseWriter, req *dns.Msg) (int, error) { if hasStubEdns0(req) { - // TODO(miek): actual error here - return dns.RcodeServerFailure, nil + log.Printf("[WARNING] Forwarding cycle detected, refusing msg: %s", req.Question[0].Name) + return dns.RcodeRefused, nil } req = addStubEdns0(req) proxy, ok := (*s.Etcd.Stubmap)[s.Zone] @@ -55,9 +58,10 @@ func addStubEdns0(m *dns.Msg) *dns.Msg { // Add a custom EDNS0 option to the packet, so we can detect loops when 2 stubs are forwarding to each other. if option != nil { option.Option = append(option.Option, &dns.EDNS0_LOCAL{ednsStubCode, []byte{1}}) - } else { - m.Extra = append(m.Extra, ednsStub) + return m } + + m.Extra = append(m.Extra, ednsStub) return m } @@ -70,6 +74,7 @@ var ednsStub = func() *dns.OPT { o := new(dns.OPT) o.Hdr.Name = "." o.Hdr.Rrtype = dns.TypeOPT + o.SetUDPSize(4096) e := new(dns.EDNS0_LOCAL) e.Code = ednsStubCode diff --git a/middleware/etcd/stub_test.go b/middleware/etcd/stub_test.go index 37bd840a7..d1eee5b61 100644 --- a/middleware/etcd/stub_test.go +++ b/middleware/etcd/stub_test.go @@ -2,145 +2,89 @@ package etcd -import "testing" +import ( + "sort" + "testing" + + "github.com/miekg/coredns/middleware" + "github.com/miekg/coredns/middleware/etcd/msg" + "github.com/miekg/coredns/middleware/test" + "github.com/miekg/dns" +) func TestStubLookup(t *testing.T) { - // MOVE THIS TO etcd_Test.go in the main directory - // e.updateStubZones() -} - -/* -func TestDNSStubForward(t *testing.T) { - s := newTestServer(t, false) - defer s.Stop() - - c := new(dns.Client) - m := new(dns.Msg) - - stubEx := &msg.Service{ - // IP address of a.iana-servers.net. - Host: "199.43.132.53", Key: "a.example.com.stub.dns.skydns.test.", + for _, serv := range servicesStub { + set(t, etc, serv.Key, 0, serv) + defer delete(t, etc, serv.Key) } - stubBroken := &msg.Service{ - Host: "127.0.0.1", Port: 5454, Key: "b.example.org.stub.dns.skydns.test.", - } - stubLoop := &msg.Service{ - Host: "127.0.0.1", Port: Port, Key: "b.example.net.stub.dns.skydns.test.", - } - addService(t, s, stubEx.Key, 0, stubEx) - defer delService(t, s, stubEx.Key) - addService(t, s, stubBroken.Key, 0, stubBroken) - defer delService(t, s, stubBroken.Key) - addService(t, s, stubLoop.Key, 0, stubLoop) - defer delService(t, s, stubLoop.Key) + etc.updateStubZones() - s.UpdateStubZones() + for _, tc := range dnsTestCasesStub { + m := tc.Msg() - m.SetQuestion("www.example.com.", dns.TypeA) - resp, _, err := c.Exchange(m, "127.0.0.1:"+StrPort) - if err != nil { - // try twice - resp, _, err = c.Exchange(m, "127.0.0.1:"+StrPort) + rec := middleware.NewResponseRecorder(&test.ResponseWriter{}) + _, err := etc.ServeDNS(ctx, rec, m) if err != nil { - t.Fatal(err) + if tc.Rcode != dns.RcodeServerFailure { + t.Errorf("expected no error, got %v\n", err) + } + // This is OK, we expect this backend to *not* work. + continue + } + 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) } } - if len(resp.Answer) == 0 || resp.Rcode != dns.RcodeSuccess { - t.Fatal("answer expected to have A records or rcode not equal to RcodeSuccess") - } - // The main diff. here is that we expect the AA bit to be set, because we directly - // queried the authoritative servers. - if resp.Authoritative != true { - t.Fatal("answer expected to have AA bit set") - } - - // This should fail. - m.SetQuestion("www.example.org.", dns.TypeA) - resp, _, err = c.Exchange(m, "127.0.0.1:"+StrPort) - if len(resp.Answer) != 0 || resp.Rcode != dns.RcodeServerFailure { - t.Fatal("answer expected to fail for example.org") - } - - // This should really fail with a timeout. - m.SetQuestion("www.example.net.", dns.TypeA) - resp, _, err = c.Exchange(m, "127.0.0.1:"+StrPort) - if err == nil { - t.Fatal("answer expected to fail for example.net") - } else { - t.Logf("succesfully failing %s", err) - } - - // Packet with EDNS0 - m.SetEdns0(4096, true) - resp, _, err = c.Exchange(m, "127.0.0.1:"+StrPort) - if err == nil { - t.Fatal("answer expected to fail for example.net") - } else { - t.Logf("succesfully failing %s", err) - } - - // Now start another SkyDNS instance on a different port, - // add a stubservice for it and check if the forwarding is - // actually working. - oldStrPort := StrPort - - s1 := newTestServer(t, false) - defer s1.Stop() - s1.config.Domain = "skydns.com." - - // Add forwarding IP for internal.skydns.com. Use Port to point to server s. - stubForward := &msg.Service{ - Host: "127.0.0.1", Port: Port, Key: "b.internal.skydns.com.stub.dns.skydns.test.", - } - addService(t, s, stubForward.Key, 0, stubForward) - defer delService(t, s, stubForward.Key) - s.UpdateStubZones() - - // Add an answer for this in our "new" server. - stubReply := &msg.Service{ - Host: "127.1.1.1", Key: "www.internal.skydns.com.", - } - addService(t, s1, stubReply.Key, 0, stubReply) - defer delService(t, s1, stubReply.Key) - - m = new(dns.Msg) - m.SetQuestion("www.internal.skydns.com.", dns.TypeA) - resp, _, err = c.Exchange(m, "127.0.0.1:"+oldStrPort) - if err != nil { - t.Fatalf("failed to forward %s", err) - } - if resp.Answer[0].(*dns.A).A.String() != "127.1.1.1" { - t.Fatalf("failed to get correct reply") - } - - // Adding an in baliwick internal domain forward. - s2 := newTestServer(t, false) - defer s2.Stop() - s2.config.Domain = "internal.skydns.net." - - // Add forwarding IP for internal.skydns.net. Use Port to point to server s. - stubForward1 := &msg.Service{ - Host: "127.0.0.1", Port: Port, Key: "b.internal.skydns.net.stub.dns.skydns.test.", - } - addService(t, s, stubForward1.Key, 0, stubForward1) - defer delService(t, s, stubForward1.Key) - s.UpdateStubZones() - - // Add an answer for this in our "new" server. - stubReply1 := &msg.Service{ - Host: "127.10.10.10", Key: "www.internal.skydns.net.", - } - addService(t, s2, stubReply1.Key, 0, stubReply1) - defer delService(t, s2, stubReply1.Key) - - m = new(dns.Msg) - m.SetQuestion("www.internal.skydns.net.", dns.TypeA) - resp, _, err = c.Exchange(m, "127.0.0.1:"+oldStrPort) - if err != nil { - t.Fatalf("failed to forward %s", err) - } - if resp.Answer[0].(*dns.A).A.String() != "127.10.10.10" { - t.Fatalf("failed to get correct reply") - } } -*/ + +var servicesStub = []*msg.Service{ + // Two tests, ask a question that should return servfail because remote it no accessible + // and one with edns0 option added, that should return refused. + {Host: "127.0.0.1", Port: 666, Key: "b.example.org.stub.dns.skydns.test."}, + // Actual test that goes out to the internet. + {Host: "199.43.132.53", Key: "a.example.net.stub.dns.skydns.test."}, +} + +var dnsTestCasesStub = []test.Case{ + { + Qname: "example.org.", Qtype: dns.TypeA, Rcode: dns.RcodeServerFailure, + }, + { + Qname: "example.net.", Qtype: dns.TypeA, + Answer: []dns.RR{test.A("example.net. 86400 IN A 93.184.216.34")}, + Ns: []dns.RR{ + test.NS("example.net. 86400 IN NS a.iana-servers.net."), + test.NS("example.net. 86400 IN NS b.iana-servers.net."), + }, + Extra: []dns.RR{test.OPT(4096, false)}, // This will have an EDNS0 section, because *we* added our local stub forward to detect loops. + }, + { + Qname: "example.net.", Qtype: dns.TypeA, Do: true, + Answer: []dns.RR{ + test.A("example.net. 86400 IN A 93.184.216.34"), + test.RRSIG("example.net. 86400 IN RRSIG A 8 2 86400 20160428060557 20160406182909 40948 example.net. Vm+rH5KN"), + }, + Ns: []dns.RR{ + test.NS("example.net. 86400 IN NS a.iana-servers.net."), + test.NS("example.net. 86400 IN NS b.iana-servers.net."), + test.RRSIG("example.net. 86400 IN RRSIG NS 8 2 86400 20160428110538 20160407002909 40948 example.net. z74YR2"), + }, + Extra: []dns.RR{test.OPT(4096, true)}, + }, +} diff --git a/middleware/proxy/lookup.go b/middleware/proxy/lookup.go index a401705f2..92ce9ae2c 100644 --- a/middleware/proxy/lookup.go +++ b/middleware/proxy/lookup.go @@ -12,6 +12,7 @@ import ( "github.com/miekg/dns" ) +// New create a new proxy with the hosts in host and a Random policy. func New(hosts []string) Proxy { p := Proxy{Next: nil, Client: Clients()} @@ -31,7 +32,7 @@ func New(hosts []string) Proxy { Fails: 0, FailTimeout: upstream.FailTimeout, Unhealthy: false, - ExtraHeaders: upstream.proxyHeaders, + ExtraHeaders: upstream.proxyHeaders, // TODO(miek): fixer the fix CheckDown: func(upstream *staticUpstream) UpstreamHostDownFunc { return func(uh *UpstreamHost) bool { if uh.Unhealthy { @@ -80,6 +81,7 @@ func (p Proxy) lookup(state middleware.State, r *dns.Msg) (*dns.Msg, error) { for time.Now().Sub(start) < tryDuration { host := upstream.Select() if host == nil { + // TODO(miek): if all HC fail, spray the targets. return nil, errUnreachable } diff --git a/middleware/state.go b/middleware/state.go index 9a368390b..ec2b618a6 100644 --- a/middleware/state.go +++ b/middleware/state.go @@ -125,24 +125,25 @@ func (s *State) Size() int { } // SizeAndDo adds an OPT record that the reflects the intent from state. -// The returned bool indicated if an record was added. +// The returned bool indicated if an record was found and normalised. func (s *State) SizeAndDo(m *dns.Msg) bool { o := s.Req.IsEdns0() // TODO(miek): speed this up if o == nil { return false } - - size := s.Size() - Do := s.Do() - o.Hdr.Name = "." o.Hdr.Rrtype = dns.TypeOPT o.SetVersion(0) - o.SetUDPSize(uint16(size)) - if Do { - o.SetDo() + if mo := m.IsEdns0(); mo != nil { + mo.Hdr.Name = "." + mo.Hdr.Rrtype = dns.TypeOPT + mo.SetVersion(0) + mo.SetUDPSize(o.UDPSize()) + if o.Do() { + mo.SetDo() + } + return true } - // TODO(miek): test how this works with stub forwarding in etcd middleware. m.Extra = append(m.Extra, o) return true } diff --git a/middleware/test/helpers.go b/middleware/test/helpers.go index 7caa3fd4a..3096f126b 100644 --- a/middleware/test/helpers.go +++ b/middleware/test/helpers.go @@ -108,8 +108,11 @@ func Section(t *testing.T, tc Case, sect Sect, rr []dns.RR) bool { } // 303 signals: don't care what the ttl is. if section[i].Header().Ttl != 303 && a.Header().Ttl != section[i].Header().Ttl { - t.Errorf("rr %d should have a Header TTL of %d, but has %d", i, section[i].Header().Ttl, a.Header().Ttl) - return false + if _, ok := section[i].(*dns.OPT); !ok { + // we check edns0 bufize on this one + t.Errorf("rr %d should have a Header TTL of %d, but has %d", i, section[i].Header().Ttl, a.Header().Ttl) + return false + } } if a.Header().Rrtype != section[i].Header().Rrtype { t.Errorf("rr %d should have a header rr type of %d, but has %d", i, section[i].Header().Rrtype, a.Header().Rrtype) @@ -206,12 +209,12 @@ func Section(t *testing.T, tc Case, sect Sect, rr []dns.RR) bool { } case *dns.OPT: tt := section[i].(*dns.OPT) - if x.Do() != tt.Do() { - t.Errorf("OPT DO should be %q, but is %q", x.Do(), tt.Do()) + if x.UDPSize() != tt.UDPSize() { + t.Errorf("OPT UDPSize should be %d, but is %d", tt.UDPSize(), x.UDPSize()) return false } - if x.UDPSize() != tt.UDPSize() { - t.Errorf("OPT UDPSize should be %q, but is %q", x.UDPSize(), tt.UDPSize()) + if x.Do() != tt.Do() { + t.Errorf("OPT DO should be %t, but is %t", tt.Do(), x.Do()) return false } }