plugin/kubernetes: Validate transfers are allowed (#2292)

* check allowed transfers

* add tests for parsing, and comment about refactor

plugin/kubernetes/setup_transfer_test.go

@@ -0,0 +1,47 @@
+package kubernetes
+
+import (
+	"testing"
+
+	"github.com/mholt/caddy"
+)
+
+func TestKubernetesParseTransfer(t *testing.T) {
+	tests := []struct {
+		input     string // Corefile data as string
+		expected  string
+		shouldErr bool
+	}{
+		{`kubernetes cluster.local {
+			transfer to 1.2.3.4
+		}`, "1.2.3.4:53", false},
+		{`kubernetes cluster.local {
+			transfer to 1.2.3.4:53
+		}`, "1.2.3.4:53", false},
+		{`kubernetes cluster.local {
+			transfer to *
+		}`, "*", false},
+		{`kubernetes cluster.local {
+			transfer
+		}`, "", true},
+	}
+
+	for i, tc := range tests {
+		c := caddy.NewTestController("dns", tc.input)
+		k, err := kubernetesParse(c)
+		if err != nil && !tc.shouldErr {
+			t.Fatalf("Test %d: Expected no error, got %q", i, err)
+		}
+		if err == nil && tc.shouldErr {
+			t.Fatalf("Test %d: Expected error, got none", i)
+		}
+		if err != nil && tc.shouldErr {
+			// input should error
+			continue
+		}
+
+		if k.TransferTo[0] != tc.expected {
+			t.Errorf("Test %d: Expected Transfer To to be %s, got %s", i, tc.expected, k.TransferTo[0])
+		}
+	}
+}

plugin/kubernetes/xfr.go

@@ -25,6 +25,10 @@ func (k *Kubernetes) MinTTL(state request.Request) uint32 { return 30 }
 // Transfer implements the Transferer interface.
 func (k *Kubernetes) Transfer(ctx context.Context, state request.Request) (int, error) {
 
+	if !k.transferAllowed(state) {
+		return dns.RcodeRefused, nil
+	}
+
 	// Get all services.
 	rrs := make(chan dns.RR)
 	go k.transfer(rrs, state.Zone)
@@ -71,6 +75,26 @@ func (k *Kubernetes) Transfer(ctx context.Context, state request.Request) (int,
 	return dns.RcodeSuccess, nil
 }
 
+// transferAllowed checks if incoming request for transferring the zone is allowed according to the ACLs.
+// Note: This is copied from zone.transferAllowed, but should eventually be factored into a common transfer pkg.
+func (k *Kubernetes) transferAllowed(state request.Request) bool {
+	for _, t := range k.TransferTo {
+		if t == "*" {
+			return true
+		}
+		// If remote IP matches we accept.
+		remote := state.IP()
+		to, _, err := net.SplitHostPort(t)
+		if err != nil {
+			continue
+		}
+		if to == remote {
+			return true
+		}
+	}
+	return false
+}
+
 func (k *Kubernetes) transfer(c chan dns.RR, zone string) {
 
 	defer close(c)

plugin/kubernetes/xfr_test.go

@@ -15,7 +15,7 @@ import (
 func TestKubernetesXFR(t *testing.T) {
 	k := New([]string{"cluster.local."})
 	k.APIConn = &APIConnServeTest{}
-	k.TransferTo = []string{"127.0.0.1"}
+	k.TransferTo = []string{"10.240.0.1:53"}
 	k.Namespaces = map[string]bool{"testns": true}
 
 	ctx := context.TODO()
@@ -30,7 +30,12 @@ func TestKubernetesXFR(t *testing.T) {
 
 	if len(w.Msgs) == 0 {
 		t.Logf("%+v\n", w)
-		t.Error("Did not get back a zone response")
+		t.Fatal("Did not get back a zone response")
+	}
+
+	if len(w.Msgs[0].Answer) == 0 {
+		t.Logf("%+v\n", w)
+		t.Fatal("Did not get back an answer")
 	}
 
 	// Ensure xfr starts with SOA
@@ -95,6 +100,33 @@ func TestKubernetesXFR(t *testing.T) {
 	}
 }
 
+func TestKubernetesXFRNotAllowed(t *testing.T) {
+	k := New([]string{"cluster.local."})
+	k.APIConn = &APIConnServeTest{}
+	k.TransferTo = []string{"1.2.3.4:53"}
+	k.Namespaces = map[string]bool{"testns": true}
+
+	ctx := context.TODO()
+	w := dnstest.NewMultiRecorder(&test.ResponseWriter{})
+	dnsmsg := &dns.Msg{}
+	dnsmsg.SetAxfr(k.Zones[0])
+
+	_, err := k.ServeDNS(ctx, w, dnsmsg)
+	if err != nil {
+		t.Error(err)
+	}
+
+	if len(w.Msgs) == 0 {
+		t.Logf("%+v\n", w)
+		t.Fatal("Did not get back a zone response")
+	}
+
+	if len(w.Msgs[0].Answer) != 0 {
+		t.Logf("%+v\n", w)
+		t.Fatal("Got an answer, should not have")
+	}
+}
+
 // difference shows what we're missing when comparing two RR slices
 func difference(testRRs []dns.RR, gotRRs []dns.RR) []dns.RR {
 	expectedRRs := map[string]bool{}