plugin/dnssec: fix blacklies for NXDOMAIN (#1399)

* plugin/dnssec: filter bitmap also for NXDOMAIN responses

We change nxdomain to nodata, so at the point when we receive the
reply it can be nxdomain or nodata. In both cases we should filter the
nsec bitmap.

Change the code and add explicit tests for this.

* More tests

plugin/dnssec/black_lies.go

@@ -38,10 +38,9 @@ var (
 	apexBitmap = [...]uint16{dns.TypeA, dns.TypeNS, dns.TypeSOA, dns.TypeHINFO, dns.TypeMX, dns.TypeTXT, dns.TypeAAAA, dns.TypeLOC, dns.TypeSRV, dns.TypeCERT, dns.TypeSSHFP, dns.TypeRRSIG, dns.TypeNSEC, dns.TypeDNSKEY, dns.TypeTLSA, dns.TypeHIP, dns.TypeOPENPGPKEY, dns.TypeSPF}
 )
 
-// filter14 filters out t from bitmap (if it exists). If mt is not an NODATA response, just
-// return the entire bitmap.
+// filter14 filters out t from bitmap (if it exists). If mt is not an NODATA response, just return the entire bitmap.
 func filter14(t uint16, bitmap [14]uint16, mt response.Type) []uint16 {
-	if mt != response.NoData {
+	if mt != response.NoData && mt != response.NameError {
 		return zoneBitmap[:]
 	}
 	for i := range bitmap {
@@ -53,7 +52,7 @@ func filter14(t uint16, bitmap [14]uint16, mt response.Type) []uint16 {
 }
 
 func filter18(t uint16, bitmap [18]uint16, mt response.Type) []uint16 {
-	if mt != response.NoData {
+	if mt != response.NoData && mt != response.NameError {
 		return apexBitmap[:]
 	}
 	for i := range bitmap {

plugin/dnssec/black_lies_bitmap_test.go

@@ -0,0 +1,62 @@
+package dnssec
+
+import (
+	"testing"
+	"time"
+
+	"github.com/coredns/coredns/plugin/test"
+	"github.com/coredns/coredns/request"
+
+	"github.com/miekg/dns"
+)
+
+func TestBlackLiesBitmapNoData(t *testing.T) {
+	d, rm1, rm2 := newDnssec(t, []string{"example.org."})
+	defer rm1()
+	defer rm2()
+
+	m := testTLSAMsg()
+	state := request.Request{Req: m, Zone: "example.org."}
+	m = d.Sign(state, time.Now().UTC())
+
+	var nsec *dns.NSEC
+	for _, r := range m.Ns {
+		if r.Header().Rrtype == dns.TypeNSEC {
+			nsec = r.(*dns.NSEC)
+		}
+	}
+	for _, b := range nsec.TypeBitMap {
+		if uint16(b) == dns.TypeTLSA {
+			t.Errorf("Type TLSA should not be present in the type bitmap: %v", nsec.TypeBitMap)
+		}
+	}
+}
+func TestBlackLiesBitmapNameError(t *testing.T) {
+	d, rm1, rm2 := newDnssec(t, []string{"example.org."})
+	defer rm1()
+	defer rm2()
+
+	m := testTLSAMsg()
+	m.Rcode = dns.RcodeNameError // change to name error
+	state := request.Request{Req: m, Zone: "example.org."}
+	m = d.Sign(state, time.Now().UTC())
+
+	var nsec *dns.NSEC
+	for _, r := range m.Ns {
+		if r.Header().Rrtype == dns.TypeNSEC {
+			nsec = r.(*dns.NSEC)
+		}
+	}
+	for _, b := range nsec.TypeBitMap {
+		if uint16(b) == dns.TypeTLSA {
+			t.Errorf("Type TLSA should not be present in the type bitmap: %v", nsec.TypeBitMap)
+		}
+	}
+}
+
+func testTLSAMsg() *dns.Msg {
+	return &dns.Msg{MsgHdr: dns.MsgHdr{Rcode: dns.RcodeSuccess},
+		Question: []dns.Question{{Name: "25._tcp.example.org.", Qclass: dns.ClassINET, Qtype: dns.TypeTLSA}},
+		Ns: []dns.RR{test.SOA("example.org.	1800	IN	SOA	linode.example.org. miek.example.org. 1461471181 14400 3600 604800 14400")},
+	}
+}

plugin/dnssec/black_lies_test.go

@@ -41,9 +41,46 @@ func TestZoneSigningBlackLies(t *testing.T) {
 	}
 }
 
+func TestBlackLiesNoError(t *testing.T) {
+	d, rm1, rm2 := newDnssec(t, []string{"miek.nl."})
+	defer rm1()
+	defer rm2()
+
+	m := testSuccessMsg()
+	state := request.Request{Req: m, Zone: "miek.nl."}
+	m = d.Sign(state, time.Now().UTC())
+
+	if m.Rcode != dns.RcodeSuccess {
+		t.Errorf("expected rcode %d, got %d", dns.RcodeSuccess, m.Rcode)
+	}
+
+	if len(m.Answer) != 2 {
+		t.Errorf("answer section should have 2 RRs")
+	}
+	sig, txt := false, false
+	for _, rr := range m.Answer {
+		if _, ok := rr.(*dns.RRSIG); ok {
+			sig = true
+		}
+		if _, ok := rr.(*dns.TXT); ok {
+			txt = true
+		}
+	}
+	if !sig || !txt {
+		t.Errorf("expected RRSIG and TXT in answer section")
+	}
+}
+
 func testNxdomainMsg() *dns.Msg {
 	return &dns.Msg{MsgHdr: dns.MsgHdr{Rcode: dns.RcodeNameError},
 		Question: []dns.Question{{Name: "ww.miek.nl.", Qclass: dns.ClassINET, Qtype: dns.TypeTXT}},
 		Ns: []dns.RR{test.SOA("miek.nl.	1800	IN	SOA	linode.atoom.net. miek.miek.nl. 1461471181 14400 3600 604800 14400")},
 	}
 }
+
+func testSuccessMsg() *dns.Msg {
+	return &dns.Msg{MsgHdr: dns.MsgHdr{Rcode: dns.RcodeSuccess},
+		Question: []dns.Question{{Name: "www.miek.nl.", Qclass: dns.ClassINET, Qtype: dns.TypeTXT}},
+		Answer: []dns.RR{test.TXT(`www.miek.nl.	1800	IN	TXT	"response"`)},
+	}
+}