From 0ab95c3f130379f1d70b84fb1ee33a7f880923f2 Mon Sep 17 00:00:00 2001
From: mpl <mathieu.lonjaret@gmail.com>
Date: Fri, 21 Oct 2016 19:14:31 +0200
Subject: [PATCH] server/camnetdns: reply with NXDOMAIN when needed

When we get a query for a name we are authoritative about, we should
reply with NXDOMAIN when this name does not exist.

This change moves the name lookup to as early as possible to make sure
of that. This means we're now doing lookups even for cases where we
technically wouldn't have needed them, so maybe a substantial increase
in load? We'll see.

Change-Id: I5e9946dd67757856f626f484b547197c6246cccd
---
 server/camnetdns/camnetdns.go | 40 ++++++++++++++++++-----------------
 1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/server/camnetdns/camnetdns.go b/server/camnetdns/camnetdns.go
index 487288a7d..5ac6e2a9c 100644
--- a/server/camnetdns/camnetdns.go
+++ b/server/camnetdns/camnetdns.go
@@ -87,7 +87,7 @@ const (
 	domain      = "camlistore.net."
 	authorityNS = "camnetdns.camlistore.org."
 	// Increment after every change with format YYYYMMDDnn.
-	soaSerial = 2016102003
+	soaSerial = 2016102101
 )
 
 var (
@@ -151,19 +151,32 @@ func (ds *DNSServer) ServeDNS(rw dns.ResponseWriter, mes *dns.Msg) {
 		}
 		return
 	}
+	resp.SetReply(mes)
+	// TODO(mpl): Should we make sure that at least q.Name ends in
+	// "camlistore.net" before claiming we're authoritative on that response?
+	resp.Authoritative = true
+
 	for _, q := range mes.Question {
 		log.Printf("DNS request from %s: %s", rw.RemoteAddr(), &q)
+
+		answer, err := ds.HandleLookup(q.Name)
+		if err == sorted.ErrNotFound {
+			resp.SetRcode(mes, dns.RcodeNameError)
+			if err := rw.WriteMsg(resp); err != nil {
+				log.Printf("error responding to DNS query: %s", err)
+			}
+			return
+		}
+		if err != nil {
+			log.Printf("error looking up %q: %v", q.Name, err)
+			continue
+		}
+
 		if q.Qclass != dns.ClassINET {
 			log.Printf("error: got invalid DNS question class %d\n", q.Qclass)
 			continue
 		}
 
-		// TODO(mpl): according to
-		// https://community.letsencrypt.org/t/is-lets-encrypt-dns-not-liking-my-domain-name-server/21303/18
-		// we should probably always start here by doing a lookup and reply
-		// with RcodeNameError if there's no record, regardless of the
-		// query type.
-
 		switch q.Qtype {
 		// As long as we send a reply (even an empty one), we apparently
 		// look compliant. Or at least more than if we replied with
@@ -180,11 +193,6 @@ func (ds *DNSServer) ServeDNS(rw dns.ResponseWriter, mes *dns.Msg) {
 			resp.Extra = []dns.RR{additionalSection}
 
 		case dns.TypeCAA:
-			_, err := ds.HandleLookup(q.Name)
-			if err != nil {
-				log.Println(err)
-				continue
-			}
 			header := commonHeader(q)
 			rr := &dns.CAA{
 				Hdr:   header,
@@ -195,11 +203,7 @@ func (ds *DNSServer) ServeDNS(rw dns.ResponseWriter, mes *dns.Msg) {
 			resp.Answer = []dns.RR{rr}
 
 		case dns.TypeA, dns.TypeAAAA:
-			val, err := ds.HandleLookup(q.Name)
-			if err != nil {
-				log.Println(err)
-				continue
-			}
+			val := answer
 			ip := net.ParseIP(val)
 			// TODO(mpl): maybe we should have a distinct memstore for each type?
 			isIP6 := strings.Contains(ip.String(), ":")
@@ -239,8 +243,6 @@ func (ds *DNSServer) ServeDNS(rw dns.ResponseWriter, mes *dns.Msg) {
 		}
 		break
 	}
-	resp.SetReply(mes)
-	resp.Authoritative = true
 
 	if err := rw.WriteMsg(resp); err != nil {
 		log.Printf("error responding to DNS query: %s", err)