Stub forward/proxy tests (#108)

Test the handling of EDNS0 payloads and forwarding to stubzones
servers.

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()
 

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)

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()
 

middleware/etcd/stub.go

 package etcd
 
 import (
+	"log"
 	"net"
 	"strconv"
 	"strings"
@@ -23,14 +24,15 @@ func (e Etcd) UpdateStubZones() {
 // Look in .../dns/stub/<zone>/xx for msg.Services. Loop through them
 // extract <zone> 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() {
-	stubmap := make(map[string]proxy.Proxy)
-	for _, zone := range e.Zones {
+// 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 {
-			continue
+		return
 	}
 
+	stubmap := make(map[string]proxy.Proxy)
 	// track the nameservers on a per domain basis, but allow a list on the domain.
 	nameservers := map[string][]string{}
 
@@ -40,19 +42,21 @@ func (e Etcd) updateStubZones() {
 		}
 		ip := net.ParseIP(serv.Host)
 		if ip == nil {
+			log.Printf("[WARNING] Non IP address stub nameserver: %s", serv.Host)
 			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)], "."))
+			// 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
 			}
 			nameservers[domain] = append(nameservers[domain], net.JoinHostPort(serv.Host, strconv.Itoa(serv.Port)))
@@ -61,8 +65,6 @@ func (e Etcd) updateStubZones() {
 	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

middleware/etcd/stub_handler.go

 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

middleware/etcd/stub_test.go

@@ -2,145 +2,89 @@
 
 package etcd
 
-import "testing"
+import (
+	"sort"
+	"testing"
 
-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)
+	"github.com/miekg/coredns/middleware"
+	"github.com/miekg/coredns/middleware/etcd/msg"
+	"github.com/miekg/coredns/middleware/test"
+	"github.com/miekg/dns"
+)
 
-	stubEx := &msg.Service{
-		// IP address of a.iana-servers.net.
-		Host: "199.43.132.53", Key: "a.example.com.stub.dns.skydns.test.",
-	}
-	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.",
+func TestStubLookup(t *testing.T) {
+	for _, serv := range servicesStub {
+		set(t, etc, serv.Key, 0, serv)
+		defer delete(t, etc, serv.Key)
 	}
-	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)
+		rec := middleware.NewResponseRecorder(&test.ResponseWriter{})
+		_, err := etc.ServeDNS(ctx, rec, m)
 		if err != nil {
-		// try twice
-		resp, _, err = c.Exchange(m, "127.0.0.1:"+StrPort)
-		if err != nil {
-			t.Fatal(err)
-		}
-	}
-	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)
+			if tc.Rcode != dns.RcodeServerFailure {
+				t.Errorf("expected no error, got %v\n", 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)
+			// This is OK, we expect this backend to *not* work.
+			continue
 		}
+		resp := rec.Msg()
 
-	// Now start another SkyDNS instance on a different port,
-	// add a stubservice for it and check if the forwarding is
-	// actually working.
-	oldStrPort := StrPort
+		sort.Sort(test.RRSet(resp.Answer))
+		sort.Sort(test.RRSet(resp.Ns))
+		sort.Sort(test.RRSet(resp.Extra))
 
-	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.",
+		if !test.Header(t, tc, resp) {
+			t.Logf("%v\n", resp)
+			continue
 		}
-	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.",
+		if !test.Section(t, tc, test.Answer, resp.Answer) {
+			t.Logf("%v\n", resp)
 		}
-	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 !test.Section(t, tc, test.Ns, resp.Ns) {
+			t.Logf("%v\n", resp)
 		}
-	if resp.Answer[0].(*dns.A).A.String() != "127.1.1.1" {
-		t.Fatalf("failed to get correct reply")
+		if !test.Section(t, tc, test.Extra, resp.Extra) {
+			t.Logf("%v\n", resp)
 		}
-
-	// 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)
+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."},
+}
 
-	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 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)},
+	},
 }
-*/

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
 			}
 

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
 }

middleware/test/helpers.go

@@ -108,9 +108,12 @@ 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 {
+			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)
 			return false
@@ -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
 			}
 		}