- Moved findZone from rfc2136 to dns_challenge.go and renamed to findZoneByFqdn

- Reworked the code in dns_challenge.go to not assume nameserver is
   port-less or defaults to 53. (messes up testing)
- Updated nameserver test to clear the fqdn2zone cache and return a dummy
  SOA RR to make initial findZoneByFqdn call happy.
- Used publicsuffix library to determine if the "authorative" zone we found
  is a public registry, in that case error out. (Also used by boulder btw)

acme/dns_challenge.go

@@ -11,13 +11,17 @@ import (
 	"time"
 
 	"github.com/miekg/dns"
+	"golang.org/x/net/publicsuffix"
 )
 
 type preCheckDNSFunc func(fqdn, value string) (bool, error)
 
-var preCheckDNS preCheckDNSFunc = checkDNSPropagation
+var (
+	preCheckDNS preCheckDNSFunc = checkDNSPropagation
+	fqdn2zone                   = map[string]string{}
+)
 
-var recursiveNameserver = "google-public-dns-a.google.com"
+var recursiveNameserver = "google-public-dns-a.google.com:53"
 
 // DNS01Record returns a DNS record which will fulfill the `dns-01` challenge
 func DNS01Record(domain, keyAuth string) (fqdn string, value string, ttl int) {
@@ -105,7 +109,7 @@ func checkDNSPropagation(fqdn, value string) (bool, error) {
 // checkAuthoritativeNss queries each of the given nameservers for the expected TXT record.
 func checkAuthoritativeNss(fqdn, value string, nameservers []string) (bool, error) {
 	for _, ns := range nameservers {
-		r, err := dnsQuery(fqdn, dns.TypeTXT, ns, false)
+		r, err := dnsQuery(fqdn, dns.TypeTXT, net.JoinHostPort(ns, "53"), false)
 		if err != nil {
 			return false, err
 		}
@@ -133,6 +137,7 @@ func checkAuthoritativeNss(fqdn, value string, nameservers []string) (bool, erro
 }
 
 // dnsQuery sends a DNS query to the given nameserver.
+// The nameserver should include a port, to facilitate testing where we talk to a mock dns server.
 func dnsQuery(fqdn string, rtype uint16, nameserver string, recursive bool) (in *dns.Msg, err error) {
 	m := new(dns.Msg)
 	m.SetQuestion(fqdn, rtype)
@@ -142,7 +147,7 @@ func dnsQuery(fqdn string, rtype uint16, nameserver string, recursive bool) (in
 		m.RecursionDesired = false
 	}
 
-	in, err = dns.Exchange(m, net.JoinHostPort(nameserver, "53"))
+	in, err = dns.Exchange(m, nameserver)
 	if err == dns.ErrTruncated {
 		tcp := &dns.Client{Net: "tcp"}
 		in, _, err = tcp.Exchange(m, nameserver)
@@ -155,7 +160,12 @@ func dnsQuery(fqdn string, rtype uint16, nameserver string, recursive bool) (in
 func lookupNameservers(fqdn string) ([]string, error) {
 	var authoritativeNss []string
 
-	r, err := dnsQuery(fqdn, dns.TypeNS, recursiveNameserver, true)
+	zone, err := findZoneByFqdn(fqdn, recursiveNameserver)
+	if err != nil {
+		return nil, err
+	}
+
+	r, err := dnsQuery(zone, dns.TypeNS, recursiveNameserver, true)
 	if err != nil {
 		return nil, err
 	}
@@ -169,15 +179,59 @@ func lookupNameservers(fqdn string) ([]string, error) {
 	if len(authoritativeNss) > 0 {
 		return authoritativeNss, nil
 	}
-
-	// Strip of the left most label to get the parent domain.
-	offset, _ := dns.NextLabel(fqdn, 0)
-	next := fqdn[offset:]
-	if dns.CountLabel(next) < 2 {
 	return nil, fmt.Errorf("Could not determine authoritative nameservers")
 }
 
-	return lookupNameservers(next)
+// findZoneByFqdn determines the zone of the given fqdn
+func findZoneByFqdn(fqdn, nameserver string) (string, error) {
+	// Do we have it cached?
+	if zone, ok := fqdn2zone[fqdn]; ok {
+		return zone, nil
+	}
+
+	// Query the authorative nameserver for a hopefully non-existing SOA record,
+	// in the authority section of the reply it will have the SOA of the
+	// containing zone. rfc2308 has this to say on the subject:
+	//   Name servers authoritative for a zone MUST include the SOA record of
+	//   the zone in the authority section of the response when reporting an
+	//   NXDOMAIN or indicating that no data (NODATA) of the requested type exists
+	in, err := dnsQuery(fqdn, dns.TypeSOA, nameserver, true)
+	if err != nil {
+		return "", err
+	}
+	if in.Rcode != dns.RcodeNameError {
+		if in.Rcode != dns.RcodeSuccess {
+			return "", fmt.Errorf("NS %s returned %s for %s", nameserver, dns.RcodeToString[in.Rcode], fqdn)
+		}
+		// We have a success, so one of the answers has to be a SOA RR
+		for _, ans := range in.Answer {
+			if soa, ok := ans.(*dns.SOA); ok {
+				zone := soa.Hdr.Name
+				// If we ended up on one of the TLDs, it means the domain did not exist.
+				publicsuffix, _ := publicsuffix.PublicSuffix(unFqdn(zone))
+				if publicsuffix == unFqdn(zone) {
+					return "", fmt.Errorf("Could not determine zone authoritively")
+				}
+				fqdn2zone[fqdn] = zone
+				return zone, nil
+			}
+		}
+		// Or it is NODATA, fall through to NXDOMAIN
+	}
+	// Search the authority section for our precious SOA RR
+	for _, ns := range in.Ns {
+		if soa, ok := ns.(*dns.SOA); ok {
+			zone := soa.Hdr.Name
+			// If we ended up on one of the TLDs, it means the domain did not exist.
+			publicsuffix, _ := publicsuffix.PublicSuffix(unFqdn(zone))
+			if publicsuffix == unFqdn(zone) {
+				return "", fmt.Errorf("Could not determine zone authoritively")
+			}
+			fqdn2zone[fqdn] = zone
+			return zone, nil
+		}
+	}
+	return "", fmt.Errorf("NS %s did not return the expected SOA record in the authority section", nameserver)
 }
 
 // toFqdn converts the name into a fqdn appending a trailing dot.

acme/dns_challenge_rfc2136.go

@@ -2,6 +2,7 @@ package acme
 
 import (
 	"fmt"
+	"net"
 	"strings"
 	"time"
 
@@ -15,7 +16,6 @@ type DNSProviderRFC2136 struct {
 	tsigAlgorithm string
 	tsigKey       string
 	tsigSecret    string
-	domain2zone   map[string]string
 }
 
 // NewDNSProviderRFC2136 returns a new DNSProviderRFC2136 instance.
@@ -23,12 +23,15 @@ type DNSProviderRFC2136 struct {
 // 'nameserver' must be a network address in the the form "host" or "host:port".
 func NewDNSProviderRFC2136(nameserver, tsigAlgorithm, tsigKey, tsigSecret string) (*DNSProviderRFC2136, error) {
 	// Append the default DNS port if none is specified.
-	if !strings.Contains(nameserver, ":") {
+	if _, _, err := net.SplitHostPort(nameserver); err != nil {
-		nameserver += ":53"
+		if strings.Contains(err.Error(), "missing port") {
+			nameserver = net.JoinHostPort(nameserver, "53")
+		} else {
+			return nil, err
+		}
 	}
 	d := &DNSProviderRFC2136{
 		nameserver: nameserver,
-		domain2zone: make(map[string]string),
 	}
 	if tsigAlgorithm == "" {
 		tsigAlgorithm = dns.HmacMD5
@@ -55,7 +58,8 @@ func (r *DNSProviderRFC2136) CleanUp(domain, token, keyAuth string) error {
 }
 
 func (r *DNSProviderRFC2136) changeRecord(action, fqdn, value string, ttl int) error {
-	zone, err := r.findZone(fqdn)
+	// Find the zone for the given fqdn
+	zone, err := findZoneByFqdn(fqdn, r.nameserver)
 	if err != nil {
 		return err
 	}
@@ -64,8 +68,7 @@ func (r *DNSProviderRFC2136) changeRecord(action, fqdn, value string, ttl int) e
 	rr := new(dns.TXT)
 	rr.Hdr = dns.RR_Header{Name: fqdn, Rrtype: dns.TypeTXT, Class: dns.ClassINET, Ttl: uint32(ttl)}
 	rr.Txt = []string{value}
-	rrs := make([]dns.RR, 1)
+	rrs := []dns.RR{rr}
-	rrs[0] = rr
 
 	// Create dynamic update packet
 	m := new(dns.Msg)
@@ -101,55 +104,3 @@ func (r *DNSProviderRFC2136) changeRecord(action, fqdn, value string, ttl int) e
 
 	return nil
 }
-
-// findZone determines the zone of a qualifying cert DNSname
-func (r *DNSProviderRFC2136) findZone(fqdn string) (string, error) {
-	// Do we have it cached?
-	if val, ok := r.domain2zone[fqdn]; ok {
-		return val, nil
-	}
-
-	// Query the authorative nameserver for a hopefully non-existing SOA record,
-	// in the authority section of the reply it will have the SOA of the
-	// containing zone. rfc2308 has this to say on the subject:
-	//   Name servers authoritative for a zone MUST include the SOA record of
-	//   the zone in the authority section of the response when reporting an
-	//   NXDOMAIN or indicating that no data (NODATA) of the requested type exists
-	m := new(dns.Msg)
-	m.SetQuestion(fqdn, dns.TypeSOA)
-	m.SetEdns0(4096, false)
-	m.RecursionDesired = true
-	m.Authoritative = true
-
-	in, err := dns.Exchange(m, r.nameserver)
-	if err == dns.ErrTruncated {
-		tcp := &dns.Client{Net: "tcp"}
-		in, _, err = tcp.Exchange(m, r.nameserver)
-	}
-	if err != nil {
-		return "", err
-	}
-	if in.Rcode != dns.RcodeNameError {
-		if in.Rcode != dns.RcodeSuccess {
-			return "", fmt.Errorf("DNS Query for zone %q failed", fqdn)
-		}
-		// We have a success, so one of the answers has to be a SOA RR
-		for _, ans := range in.Answer {
-			if ans.Header().Rrtype == dns.TypeSOA {
-				zone := ans.Header().Name
-				r.domain2zone[fqdn] = zone
-				return zone, nil
-			}
-		}
-		// Or it is NODATA, fall through to NXDOMAIN
-	}
-	// Search the authority section for our precious SOA RR
-	for _, ns := range in.Ns {
-		if ns.Header().Rrtype == dns.TypeSOA {
-			zone := ns.Header().Name
-			r.domain2zone[fqdn] = zone
-			return zone, nil
-		}
-	}
-	return "", fmt.Errorf("Expected a SOA record in the authority section")
-}

acme/dns_challenge_rfc2136_test.go

@@ -2,6 +2,7 @@ package acme
 
 import (
 	"bytes"
+	"fmt"
 	"net"
 	"strings"
 	"sync"
@@ -25,6 +26,7 @@ var (
 var reqChan = make(chan *dns.Msg, 10)
 
 func TestRFC2136CanaryLocalTestServer(t *testing.T) {
+	fqdn2zone = map[string]string{}
 	dns.HandleFunc("example.com.", serverHandlerHello)
 	defer dns.HandleRemove("example.com.")
 
@@ -48,6 +50,7 @@ func TestRFC2136CanaryLocalTestServer(t *testing.T) {
 }
 
 func TestRFC2136ServerSuccess(t *testing.T) {
+	fqdn2zone = map[string]string{}
 	dns.HandleFunc(rfc2136TestZone, serverHandlerReturnSuccess)
 	defer dns.HandleRemove(rfc2136TestZone)
 
@@ -57,7 +60,7 @@ func TestRFC2136ServerSuccess(t *testing.T) {
 	}
 	defer server.Shutdown()
 
-	provider, err := NewDNSProviderRFC2136(addrstr, rfc2136TestZone, "", "", "")
+	provider, err := NewDNSProviderRFC2136(addrstr, "", "", "")
 	if err != nil {
 		t.Fatalf("Expected NewDNSProviderRFC2136() to return no error but the error was -> %v", err)
 	}
@@ -67,6 +70,7 @@ func TestRFC2136ServerSuccess(t *testing.T) {
 }
 
 func TestRFC2136ServerError(t *testing.T) {
+	fqdn2zone = map[string]string{}
 	dns.HandleFunc(rfc2136TestZone, serverHandlerReturnErr)
 	defer dns.HandleRemove(rfc2136TestZone)
 
@@ -76,7 +80,7 @@ func TestRFC2136ServerError(t *testing.T) {
 	}
 	defer server.Shutdown()
 
-	provider, err := NewDNSProviderRFC2136(addrstr, rfc2136TestZone, "", "", "")
+	provider, err := NewDNSProviderRFC2136(addrstr, "", "", "")
 	if err != nil {
 		t.Fatalf("Expected NewDNSProviderRFC2136() to return no error but the error was -> %v", err)
 	}
@@ -88,6 +92,7 @@ func TestRFC2136ServerError(t *testing.T) {
 }
 
 func TestRFC2136TsigClient(t *testing.T) {
+	fqdn2zone = map[string]string{}
 	dns.HandleFunc(rfc2136TestZone, serverHandlerReturnSuccess)
 	defer dns.HandleRemove(rfc2136TestZone)
 
@@ -97,7 +102,7 @@ func TestRFC2136TsigClient(t *testing.T) {
 	}
 	defer server.Shutdown()
 
-	provider, err := NewDNSProviderRFC2136(addrstr, rfc2136TestZone, "", rfc2136TestTsigKey, rfc2136TestTsigSecret)
+	provider, err := NewDNSProviderRFC2136(addrstr, "", rfc2136TestTsigKey, rfc2136TestTsigSecret)
 	if err != nil {
 		t.Fatalf("Expected NewDNSProviderRFC2136() to return no error but the error was -> %v", err)
 	}
@@ -107,6 +112,7 @@ func TestRFC2136TsigClient(t *testing.T) {
 }
 
 func TestRFC2136ValidUpdatePacket(t *testing.T) {
+	fqdn2zone = map[string]string{}
 	dns.HandleFunc(rfc2136TestZone, serverHandlerPassBackRequest)
 	defer dns.HandleRemove(rfc2136TestZone)
 
@@ -116,18 +122,11 @@ func TestRFC2136ValidUpdatePacket(t *testing.T) {
 	}
 	defer server.Shutdown()
 
-	rr := new(dns.TXT)
+	txtRR, _ := dns.NewRR(fmt.Sprintf("%s %d IN TXT %s", rfc2136TestFqdn, rfc2136TestTTL, rfc2136TestValue))
-	rr.Hdr = dns.RR_Header{
+	rrs := []dns.RR{txtRR}
-		Name:   rfc2136TestFqdn,
-		Rrtype: dns.TypeTXT,
-		Class:  dns.ClassINET,
-		Ttl:    uint32(rfc2136TestTTL),
-	}
-	rr.Txt = []string{rfc2136TestValue}
-	rrs := make([]dns.RR, 1)
-	rrs[0] = rr
 	m := new(dns.Msg)
-	m.SetUpdate(dns.Fqdn(rfc2136TestZone))
+	m.SetUpdate(rfc2136TestZone)
+	m.RemoveRRset(rrs)
 	m.Insert(rrs)
 	expectstr := m.String()
 	expect, err := m.Pack()
@@ -135,7 +134,7 @@ func TestRFC2136ValidUpdatePacket(t *testing.T) {
 		t.Fatalf("Error packing expect msg: %v", err)
 	}
 
-	provider, err := NewDNSProviderRFC2136(addrstr, rfc2136TestZone, "", "", "")
+	provider, err := NewDNSProviderRFC2136(addrstr, "", "", "")
 	if err != nil {
 		t.Fatalf("Expected NewDNSProviderRFC2136() to return no error but the error was -> %v", err)
 	}
@@ -198,6 +197,11 @@ func serverHandlerHello(w dns.ResponseWriter, req *dns.Msg) {
 func serverHandlerReturnSuccess(w dns.ResponseWriter, req *dns.Msg) {
 	m := new(dns.Msg)
 	m.SetReply(req)
+	if req.Opcode == dns.OpcodeQuery && req.Question[0].Qtype == dns.TypeSOA && req.Question[0].Qclass == dns.ClassINET {
+		// Return SOA to appease findZoneByFqdn()
+		soaRR, _ := dns.NewRR(fmt.Sprintf("%s %d IN SOA ns1.%s admin.%s 2016022801 28800 7200 2419200 1200", rfc2136TestZone, rfc2136TestTTL, rfc2136TestZone, rfc2136TestZone))
+		m.Answer = []dns.RR{soaRR}
+	}
 
 	if t := req.IsTsig(); t != nil {
 		if w.TsigStatus() == nil {
@@ -218,6 +222,11 @@ func serverHandlerReturnErr(w dns.ResponseWriter, req *dns.Msg) {
 func serverHandlerPassBackRequest(w dns.ResponseWriter, req *dns.Msg) {
 	m := new(dns.Msg)
 	m.SetReply(req)
+	if req.Opcode == dns.OpcodeQuery && req.Question[0].Qtype == dns.TypeSOA && req.Question[0].Qclass == dns.ClassINET {
+		// Return SOA to appease findZoneByFqdn()
+		soaRR, _ := dns.NewRR(fmt.Sprintf("%s %d IN SOA ns1.%s admin.%s 2016022801 28800 7200 2419200 1200", rfc2136TestZone, rfc2136TestTTL, rfc2136TestZone, rfc2136TestZone))
+		m.Answer = []dns.RR{soaRR}
+	}
 
 	if t := req.IsTsig(); t != nil {
 		if w.TsigStatus() == nil {
@@ -227,5 +236,8 @@ func serverHandlerPassBackRequest(w dns.ResponseWriter, req *dns.Msg) {
 	}
 
 	w.WriteMsg(m)
+	if req.Opcode != dns.OpcodeQuery || req.Question[0].Qtype != dns.TypeSOA || req.Question[0].Qclass != dns.ClassINET {
+		// Only talk back when it is not the SOA RR.
 		reqChan <- req
 	}
+}

acme/dns_challenge_test.go

@@ -35,11 +35,11 @@ var lookupNameserversTestsErr = []struct {
 }{
 	// invalid tld
 	{"_null.n0n0.",
-		"Could not determine authoritative nameservers",
+		"Could not determine zone authoritively",
 	},
 	// invalid domain
 	{"_null.com.",
-		"Could not determine authoritative nameservers",
+		"Could not determine zone authoritively",
 	},
 }