Merge pull request #254 from janeczku/findzonebyfqdn-fix

Correctly determine the zone for CNAME domains pointing to another zone

acme/dns_challenge.go

@@ -186,7 +186,7 @@ func lookupNameservers(fqdn string) ([]string, error) {
 
 	zone, err := FindZoneByFqdn(fqdn, RecursiveNameservers)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("Could not determine the zone: %v", err)
 	}
 
 	r, err := dnsQuery(zone, dns.TypeNS, RecursiveNameservers, true)
@@ -206,53 +206,54 @@ func lookupNameservers(fqdn string) ([]string, error) {
 	return nil, fmt.Errorf("Could not determine authoritative nameservers")
 }
 
-// FindZoneByFqdn determines the zone of the given fqdn
+// FindZoneByFqdn determines the zone apex for the given fqdn by recursing up the
+// domain labels until the nameserver returns a SOA record in the answer section.
 func FindZoneByFqdn(fqdn string, nameservers []string) (string, error) {
 	// Do we have it cached?
 	if zone, ok := fqdnToZone[fqdn]; ok {
 		return zone, nil
 	}
 
-	// Query the authoritative nameserver for a hopefully non-existing SOA record,
+	labelIndexes := dns.Split(fqdn)
-	// in the authority section of the reply it will have the SOA of the
+	for _, index := range labelIndexes {
-	// containing zone. rfc2308 has this to say on the subject:
+		domain := fqdn[index:]
-	//   Name servers authoritative for a zone MUST include the SOA record of
+		// Give up if we have reached the TLD
-	//   the zone in the authority section of the response when reporting an
+		if isTLD(domain) {
-	//   NXDOMAIN or indicating that no data (NODATA) of the requested type exists
+			break
-	in, err := dnsQuery(fqdn, dns.TypeSOA, nameservers, true)
-	if err != nil {
-		return "", err
-	}
-	if in.Rcode != dns.RcodeNameError {
-		if in.Rcode != dns.RcodeSuccess {
-			return "", fmt.Errorf("The NS returned %s for %s", 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 {
+		in, err := dnsQuery(domain, dns.TypeSOA, nameservers, true)
-			if soa, ok := ans.(*dns.SOA); ok {
+		if err != nil {
-				return checkIfTLD(fqdn, soa)
+			return "", err
+		}
+
+		// Any response code other than NOERROR and NXDOMAIN is treated as error
+		if in.Rcode != dns.RcodeNameError && in.Rcode != dns.RcodeSuccess {
+			return "", fmt.Errorf("Unexpected response code '%s' for %s",
+				dns.RcodeToString[in.Rcode], domain)
+		}
+
+		// Check if we got a SOA RR in the answer section
+		if in.Rcode == dns.RcodeSuccess {
+			for _, ans := range in.Answer {
+				if soa, ok := ans.(*dns.SOA); ok {
+					zone := soa.Hdr.Name
+					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 {
+	return "", fmt.Errorf("Could not find the start of authority")
-		if soa, ok := ns.(*dns.SOA); ok {
-			return checkIfTLD(fqdn, soa)
-		}
-	}
-	return "", fmt.Errorf("The NS did not return the expected SOA record in the authority section")
 }
 
-func checkIfTLD(fqdn string, soa *dns.SOA) (string, error) {
+func isTLD(domain string) bool {
-	zone := soa.Hdr.Name
+	publicsuffix, _ := publicsuffix.PublicSuffix(UnFqdn(domain))
-	// If we ended up on one of the TLDs, it means the domain did not exist.
+	if publicsuffix == UnFqdn(domain) {
-	publicsuffix, _ := publicsuffix.PublicSuffix(UnFqdn(zone))
+		return true
-	if publicsuffix == UnFqdn(zone) {
-		return "", fmt.Errorf("Could not determine zone authoritatively")
 	}
-	fqdnToZone[fqdn] = zone
+	return false
-	return zone, nil
 }
 
 // ClearFqdnCache clears the cache of fqdn to zone mappings. Primarily used in testing.

acme/dns_challenge_test.go

@@ -35,18 +35,26 @@ var lookupNameserversTestsErr = []struct {
 }{
 	// invalid tld
 	{"_null.n0n0.",
-		"Could not determine zone authoritatively",
+		"Could not determine the zone",
 	},
 	// invalid domain
 	{"_null.com.",
-		"Could not determine zone authoritatively",
+		"Could not determine the zone",
 	},
 	// invalid domain
 	{"in-valid.co.uk.",
-		"Could not determine zone authoritatively",
+		"Could not determine the zone",
 	},
 }
 
+var findZoneByFqdnTests = []struct {
+	fqdn string
+	zone string
+}{
+	{"mail.google.com.", "google.com."}, // domain is a CNAME
+	{"foo.google.com.", "google.com."},  // domain is a non-existent subdomain
+}
+
 var checkAuthoritativeNssTests = []struct {
 	fqdn, value string
 	ns          []string
@@ -142,6 +150,18 @@ func TestLookupNameserversErr(t *testing.T) {
 	}
 }
 
+func TestFindZoneByFqdn(t *testing.T) {
+	for _, tt := range findZoneByFqdnTests {
+		res, err := FindZoneByFqdn(tt.fqdn, RecursiveNameservers)
+		if err != nil {
+			t.Errorf("FindZoneByFqdn failed for %s: %v", tt.fqdn, err)
+		}
+		if res != tt.zone {
+			t.Errorf("%s: got %s; want %s", tt.fqdn, res, tt.zone)
+		}
+	}
+}
+
 func TestCheckAuthoritativeNss(t *testing.T) {
 	for _, tt := range checkAuthoritativeNssTests {
 		ok, _ := checkAuthoritativeNss(tt.fqdn, tt.value, tt.ns)