Merge pull request #143 from middelink/issue-140-multi-zone-certs

Issue/140: Using RFC2136 dns challenge does not allow me to create multi domain certs.

README.md

@@ -97,7 +97,7 @@ GLOBAL OPTIONS:
 									digitalocean: DO_AUTH_TOKEN
 									dnsimple: DNSIMPLE_EMAIL, DNSIMPLE_API_KEY
 									route53: AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, AWS_REGION
-									rfc2136: RFC2136_TSIG_KEY, RFC2136_TSIG_SECRET, RFC2136_TSIG_ALGORITHM, RFC2136_NAMESERVER, RFC2136_ZONE
+									rfc2136: RFC2136_TSIG_KEY, RFC2136_TSIG_SECRET, RFC2136_TSIG_ALGORITHM, RFC2136_NAMESERVER
 									manual: none
    --help, -h								show help
    --version, -v							print the version

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
+	fqdnToZone                  = 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 := fqdnToZone[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 authoritatively")
+				}
+				fqdnToZone[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 authoritatively")
+			}
+			fqdnToZone[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.
@@ -198,6 +252,11 @@ func unFqdn(name string) string {
 	return name
 }
 
+// clearFqdnCache clears the cache of fqdn to zone mappings. Primarily used in testing.
+func clearFqdnCache() {
+	fqdnToZone = map[string]string{}
+}
+
 // waitFor polls the given function 'f', once every 'interval' seconds, up to 'timeout' seconds.
 func waitFor(timeout, interval int, f func() (bool, error)) error {
 	var lastErr string

acme/dns_challenge_rfc2136.go

@@ -2,6 +2,7 @@ package acme
 
 import (
 	"fmt"
+	"net"
 	"strings"
 	"time"
 
@@ -12,26 +13,25 @@ import (
 // uses dynamic DNS updates (RFC 2136) to create TXT records on a nameserver.
 type DNSProviderRFC2136 struct {
 	nameserver    string
-	zone          string
 	tsigAlgorithm string
 	tsigKey       string
 	tsigSecret    string
-	records       map[string]string
 }
 
 // NewDNSProviderRFC2136 returns a new DNSProviderRFC2136 instance.
 // To disable TSIG authentication 'tsigAlgorithm, 'tsigKey' and 'tsigSecret' must be set to the empty string.
-// 'nameserver' must be a network address in the the form "host" or "host:port". 'zone' must be the fully
+// 'nameserver' must be a network address in the the form "host" or "host:port".
-// qualified name of the zone.
+func NewDNSProviderRFC2136(nameserver, tsigAlgorithm, tsigKey, tsigSecret string) (*DNSProviderRFC2136, error) {
-func NewDNSProviderRFC2136(nameserver, zone, 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,
-		zone:       zone,
-		records:    make(map[string]string),
 	}
 	if tsigAlgorithm == "" {
 		tsigAlgorithm = dns.HmacMD5
@@ -48,30 +48,35 @@ func NewDNSProviderRFC2136(nameserver, zone, tsigAlgorithm, tsigKey, tsigSecret
 // Present creates a TXT record using the specified parameters
 func (r *DNSProviderRFC2136) Present(domain, token, keyAuth string) error {
 	fqdn, value, ttl := DNS01Record(domain, keyAuth)
-	r.records[fqdn] = value
 	return r.changeRecord("INSERT", fqdn, value, ttl)
 }
 
 // CleanUp removes the TXT record matching the specified parameters
 func (r *DNSProviderRFC2136) CleanUp(domain, token, keyAuth string) error {
-	fqdn, _, ttl := DNS01Record(domain, keyAuth)
+	fqdn, value, ttl := DNS01Record(domain, keyAuth)
-	value := r.records[fqdn]
 	return r.changeRecord("REMOVE", fqdn, value, ttl)
 }
 
 func (r *DNSProviderRFC2136) changeRecord(action, fqdn, value string, ttl int) error {
+	// Find the zone for the given fqdn
+	zone, err := findZoneByFqdn(fqdn, r.nameserver)
+	if err != nil {
+		return err
+	}
+
 	// Create RR
 	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)
-	m.SetUpdate(dns.Fqdn(r.zone))
+	m.SetUpdate(zone)
 	switch action {
 	case "INSERT":
+		// Always remove old challenge left over from who knows what.
+		m.RemoveRRset(rrs)
 		m.Insert(rrs)
 	case "REMOVE":
 		m.Remove(rrs)

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) {
+	clearFqdnCache()
 	dns.HandleFunc("example.com.", serverHandlerHello)
 	defer dns.HandleRemove("example.com.")
 
@@ -48,6 +50,7 @@ func TestRFC2136CanaryLocalTestServer(t *testing.T) {
 }
 
 func TestRFC2136ServerSuccess(t *testing.T) {
+	clearFqdnCache()
 	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) {
+	clearFqdnCache()
 	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) {
+	clearFqdnCache()
 	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) {
+	clearFqdnCache()
 	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,15 @@ var lookupNameserversTestsErr = []struct {
 }{
 	// invalid tld
 	{"_null.n0n0.",
-		"Could not determine authoritative nameservers",
+		"Could not determine zone authoritatively",
 	},
 	// invalid domain
 	{"_null.com.",
-		"Could not determine authoritative nameservers",
+		"Could not determine zone authoritatively",
+	},
+	// invalid domain
+	{"in-valid.co.uk.",
+		"Could not determine zone authoritatively",
 	},
 }
 

cli.go

@@ -134,7 +134,7 @@ func main() {
 				"\n\tdigitalocean: DO_AUTH_TOKEN" +
 				"\n\tdnsimple: DNSIMPLE_EMAIL, DNSIMPLE_API_KEY" +
 				"\n\troute53: AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, AWS_REGION" +
-				"\n\trfc2136: RFC2136_TSIG_KEY, RFC2136_TSIG_SECRET, RFC2136_TSIG_ALGORITHM, RFC2136_NAMESERVER, RFC2136_ZONE" +
+				"\n\trfc2136: RFC2136_TSIG_KEY, RFC2136_TSIG_SECRET, RFC2136_TSIG_ALGORITHM, RFC2136_NAMESERVER" +
 				"\n\tmanual: none",
 		},
 	}

cli_handlers.go

@@ -79,12 +79,11 @@ func setup(c *cli.Context) (*Configuration, *Account, *acme.Client) {
 			provider, err = acme.NewDNSProviderRoute53("", "", awsRegion)
 		case "rfc2136":
 			nameserver := os.Getenv("RFC2136_NAMESERVER")
-			zone := os.Getenv("RFC2136_ZONE")
 			tsigAlgorithm := os.Getenv("RFC2136_TSIG_ALGORITHM")
 			tsigKey := os.Getenv("RFC2136_TSIG_KEY")
 			tsigSecret := os.Getenv("RFC2136_TSIG_SECRET")
 
-			provider, err = acme.NewDNSProviderRFC2136(nameserver, zone, tsigAlgorithm, tsigKey, tsigSecret)
+			provider, err = acme.NewDNSProviderRFC2136(nameserver, tsigAlgorithm, tsigKey, tsigSecret)
 		case "manual":
 			provider, err = acme.NewDNSProviderManual()
 		}