Accommodate ZeroSSL CSR requirements; fix DNS prop check

Author: mholt

config.go

@@ -562,7 +562,7 @@ func (cfg *Config) obtainCert(ctx context.Context, name string, interactive bool
 			}
 		}
 
-		csr, err := cfg.generateCSR(privKey, []string{name})
+		csr, err := cfg.generateCSR(privKey, []string{name}, false)
 		if err != nil {
 			return err
 		}
@@ -584,7 +584,19 @@ func (cfg *Config) obtainCert(ctx context.Context, name string, interactive bool
 				}
 			}
 
-			issuedCert, err = issuer.Issue(ctx, csr)
+			// TODO: ZeroSSL's API currently requires CommonName to be set, and requires it be
+			// distinct from SANs. If this was a cert it would violate the BRs, but their certs
+			// are compliant, so their CSR requirements just needlessly add friction, complexity,
+			// and inefficiency for clients. CommonName has been deprecated for 25+ years.
+			useCSR := csr
+			if _, ok := issuer.(*ZeroSSLIssuer); ok {
+				useCSR, err = cfg.generateCSR(privKey, []string{name}, true)
+				if err != nil {
+					return err
+				}
+			}
+
+			issuedCert, err = issuer.Issue(ctx, useCSR)
 			if err == nil {
 				issuerUsed = issuer
 				break
@@ -808,7 +820,7 @@ func (cfg *Config) renewCert(ctx context.Context, name string, force, interactiv
 			}
 		}
 
-		csr, err := cfg.generateCSR(privateKey, []string{name})
+		csr, err := cfg.generateCSR(privateKey, []string{name}, false)
 		if err != nil {
 			return err
 		}
@@ -818,6 +830,18 @@ func (cfg *Config) renewCert(ctx context.Context, name string, force, interactiv
 		var issuerUsed Issuer
 		var issuerKeys []string
 		for _, issuer := range cfg.Issuers {
+			// TODO: ZeroSSL's API currently requires CommonName to be set, and requires it be
+			// distinct from SANs. If this was a cert it would violate the BRs, but their certs
+			// are compliant, so their CSR requirements just needlessly add friction, complexity,
+			// and inefficiency for clients. CommonName has been deprecated for 25+ years.
+			useCSR := csr
+			if _, ok := issuer.(*ZeroSSLIssuer); ok {
+				useCSR, err = cfg.generateCSR(privateKey, []string{name}, true)
+				if err != nil {
+					return err
+				}
+			}
+
 			issuerKeys = append(issuerKeys, issuer.IssuerKey())
 			if prechecker, ok := issuer.(PreChecker); ok {
 				err = prechecker.PreCheck(ctx, []string{name}, interactive)
@@ -826,7 +850,7 @@ func (cfg *Config) renewCert(ctx context.Context, name string, force, interactiv
 				}
 			}
 
-			issuedCert, err = issuer.Issue(ctx, csr)
+			issuedCert, err = issuer.Issue(ctx, useCSR)
 			if err == nil {
 				issuerUsed = issuer
 				break
@@ -898,10 +922,16 @@ func (cfg *Config) renewCert(ctx context.Context, name string, force, interactiv
 	return err
 }
 
-func (cfg *Config) generateCSR(privateKey crypto.PrivateKey, sans []string) (*x509.CertificateRequest, error) {
+// generateCSR generates a CSR for the given SANs. If useCN is true, CommonName will get the first SAN (TODO: this is only a temporary hack for ZeroSSL API support).
+func (cfg *Config) generateCSR(privateKey crypto.PrivateKey, sans []string, useCN bool) (*x509.CertificateRequest, error) {
 	csrTemplate := new(x509.CertificateRequest)
 
 	for _, name := range sans {
+		// TODO: This is a temporary hack to support ZeroSSL API...
+		if useCN && csrTemplate.Subject.CommonName == "" && len(name) <= 64 {
+			csrTemplate.Subject.CommonName = name
+			continue
+		}
 		if ip := net.ParseIP(name); ip != nil {
 			csrTemplate.IPAddresses = append(csrTemplate.IPAddresses, ip)
 		} else if strings.Contains(name, "@") {

crypto.go

@@ -280,6 +280,11 @@ func hashCertificateChain(certChain [][]byte) string {
 
 func namesFromCSR(csr *x509.CertificateRequest) []string {
 	var nameSet []string
+	// TODO: CommonName should not be used (it has been deprecated for 25+ years,
+	// but Sectigo CA still requires it to be filled out and not overlap SANs...)
+	if csr.Subject.CommonName != "" {
+		nameSet = append(nameSet, csr.Subject.CommonName)
+	}
 	nameSet = append(nameSet, csr.DNSNames...)
 	nameSet = append(nameSet, csr.EmailAddresses...)
 	for _, v := range csr.IPAddresses {

dnsutil.go

@@ -222,7 +222,7 @@ func checkDNSPropagation(fqdn string, recType uint16, expectedValue string, chec
 	if recType != dns.TypeCNAME {
 		r, err := dnsQuery(fqdn, recType, resolvers, true)
 		if err != nil {
-			return false, err
+			return false, fmt.Errorf("CNAME dns query: %v", err)
 		}
 		if r.Rcode == dns.RcodeSuccess {
 			fqdn = updateDomainWithCName(r, fqdn)
@@ -232,7 +232,7 @@ func checkDNSPropagation(fqdn string, recType uint16, expectedValue string, chec
 	if checkAuthoritativeServers {
 		authoritativeServers, err := lookupNameservers(fqdn, resolvers)
 		if err != nil {
-			return false, err
+			return false, fmt.Errorf("looking up authoritative nameservers: %v", err)
 		}
 		populateNameserverPorts(authoritativeServers)
 		resolvers = authoritativeServers
@@ -244,9 +244,9 @@ func checkDNSPropagation(fqdn string, recType uint16, expectedValue string, chec
 // checkAuthoritativeNss queries each of the given nameservers for the expected TXT record.
 func checkAuthoritativeNss(fqdn string, recType uint16, expectedValue string, nameservers []string) (bool, error) {
 	for _, ns := range nameservers {
-		r, err := dnsQuery(fqdn, recType, []string{net.JoinHostPort(ns, "53")}, true)
+		r, err := dnsQuery(fqdn, recType, []string{ns}, true)
 		if err != nil {
-			return false, err
+			return false, fmt.Errorf("querying authoritative nameservers: %v", err)
 		}
 
 		if r.Rcode != dns.RcodeSuccess {
@@ -290,12 +290,12 @@ func lookupNameservers(fqdn string, resolvers []string) ([]string, error) {
 
 	zone, err := findZoneByFQDN(fqdn, resolvers)
 	if err != nil {
-		return nil, fmt.Errorf("could not determine the zone: %w", err)
+		return nil, fmt.Errorf("could not determine the zone for '%s': %w", fqdn, err)
 	}
 
 	r, err := dnsQuery(zone, dns.TypeNS, resolvers, true)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("querying NS resolver for zone '%s' recursively: %v", zone, err)
 	}
 
 	for _, rr := range r.Answer {

solvers.go

@@ -430,6 +430,8 @@ func (s *DNSManager) wait(ctx context.Context, zrec zoneRecord) error {
 		recType = dns.TypeCNAME
 	}
 
+	absName := libdns.AbsoluteName(zrec.record.Name, zrec.zone)
+
 	var err error
 	start := time.Now()
 	for time.Since(start) < timeout {
@@ -439,9 +441,9 @@ func (s *DNSManager) wait(ctx context.Context, zrec zoneRecord) error {
 			return ctx.Err()
 		}
 		var ready bool
-		ready, err = checkDNSPropagation(libdns.AbsoluteName(zrec.record.Name, zrec.zone), recType, zrec.record.Value, checkAuthoritativeServers, resolvers)
+		ready, err = checkDNSPropagation(absName, recType, zrec.record.Value, checkAuthoritativeServers, resolvers)
 		if err != nil {
-			return fmt.Errorf("checking DNS propagation of %q: %w", zrec.record.Name, err)
+			return fmt.Errorf("checking DNS propagation of %q (relative=%s zone=%s resolvers=%v): %w", absName, zrec.record.Name, zrec.zone, resolvers, err)
 		}
 		if ready {
 			return nil

zerosslissuer.go

@@ -42,14 +42,13 @@ func NewZeroSSLIssuer(cfg *Config, template ZeroSSLIssuer) *ZeroSSLIssuer {
 }
 
 // ZeroSSLIssuer can get certificates from ZeroSSL's API. (To use ZeroSSL's ACME
-// endpoint, use the ACMEIssuer instead.)
+// endpoint, use the ACMEIssuer instead.) Note that use of the API is restricted
+// by payment tier.
 type ZeroSSLIssuer struct {
 	// The API key (or "access key") for using the ZeroSSL API.
 	APIKey string
 
 	// How many days the certificate should be valid for.
-	// Note that customizing certificate lifetime may be
-	// a paid feature.
 	ValidityDays int
 
 	// The host to bind to when opening a listener for