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

3 files changed, 102 insertions, 4 deletions

diff --git a/plugin/dnssec/black_lies.go b/plugin/dnssec/black_lies.go
index d5541da79..fd7aa3de1 100644
--- a/plugin/dnssec/black_lies.go
+++ b/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 {
diff --git a/plugin/dnssec/black_lies_bitmap_test.go b/plugin/dnssec/black_lies_bitmap_test.go
new file mode 100644
index 000000000..d02aaa9a4
--- /dev/null
+++ b/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")},
+	}
+}
diff --git a/plugin/dnssec/black_lies_test.go b/plugin/dnssec/black_lies_test.go
index 851ead483..eb89d90e3 100644
--- a/plugin/dnssec/black_lies_test.go
+++ b/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"`)},
+	}
+}