caddytls: Upgrade ACMEz to v2; support ZeroSSL API; various fixes (#6229

)

* WIP: acmez v2, CertMagic, and ZeroSSL issuer upgrades

* caddytls: ZeroSSLIssuer now uses ZeroSSL API instead of ACME

* Fix go.mod

* caddytls: Fix automation related to managers (fix #6060)

* Fix typo (appease linter)

* Fix HTTP validation with ZeroSSL API

admin.go

@@ -474,7 +474,6 @@ func manageIdentity(ctx Context, cfg *Config) error {
 	// import the caddytls package -- but it works
 	if cfg.Admin.Identity.IssuersRaw == nil {
 		cfg.Admin.Identity.IssuersRaw = []json.RawMessage{
-			json.RawMessage(`{"module": "zerossl"}`),
 			json.RawMessage(`{"module": "acme"}`),
 		}
 	}

caddyconfig/httpcaddyfile/builtins.go

@@ -24,7 +24,7 @@ import (
 	"time"
 
 	"github.com/caddyserver/certmagic"
-	"github.com/mholt/acmez/acme"
+	"github.com/mholt/acmez/v2/acme"
 	"go.uber.org/zap/zapcore"
 
 	"github.com/caddyserver/caddy/v2"
@@ -107,7 +107,6 @@ func parseTLS(h Helper) ([]ConfigValue, error) {
 	var onDemand bool
 	var reusePrivateKeys bool
 
-	// file certificate loader
 	firstLine := h.RemainingArgs()
 	switch len(firstLine) {
 	case 0:
@@ -117,13 +116,13 @@ func parseTLS(h Helper) ([]ConfigValue, error) {
 		} else if !strings.Contains(firstLine[0], "@") {
 			return nil, h.Err("single argument must either be 'internal' or an email address")
 		} else {
-			if acmeIssuer == nil {
-				acmeIssuer = new(caddytls.ACMEIssuer)
+			acmeIssuer = &caddytls.ACMEIssuer{
+				Email: firstLine[0],
 			}
-			acmeIssuer.Email = firstLine[0]
 		}
 
 	case 2:
+		// file certificate loader
 		certFilename := firstLine[0]
 		keyFilename := firstLine[1]
 
@@ -488,19 +487,24 @@ func parseTLS(h Helper) ([]ConfigValue, error) {
 
 	case acmeIssuer != nil:
 		// implicit ACME issuers (from various subdirectives) - use defaults; there might be more than one
-		defaultIssuers := caddytls.DefaultIssuers()
+		defaultIssuers := caddytls.DefaultIssuers(acmeIssuer.Email)
 
-		// if a CA endpoint was set, override multiple implicit issuers since it's a specific one
+		// if an ACME CA endpoint was set, the user expects to use that specific one,
+		// not any others that may be defaults, so replace all defaults with that ACME CA
 		if acmeIssuer.CA != "" {
 			defaultIssuers = []certmagic.Issuer{acmeIssuer}
 		}
 
 		for _, issuer := range defaultIssuers {
-			switch iss := issuer.(type) {
-			case *caddytls.ACMEIssuer:
-				issuer = acmeIssuer
-			case *caddytls.ZeroSSLIssuer:
-				iss.ACMEIssuer = acmeIssuer
+			// apply settings from the implicitly-configured ACMEIssuer to any
+			// default ACMEIssuers, but preserve each default issuer's CA endpoint,
+			// because, for example, if you configure the DNS challenge, it should
+			// apply to any of the default ACMEIssuers, but you don't want to trample
+			// out their unique CA endpoints
+			if iss, ok := issuer.(*caddytls.ACMEIssuer); ok && iss != nil {
+				acmeCopy := *acmeIssuer
+				acmeCopy.CA = iss.CA
+				issuer = &acmeCopy
 			}
 			configVals = append(configVals, ConfigValue{
 				Class: "tls.cert_issuer",

caddyconfig/httpcaddyfile/options.go

@@ -18,7 +18,7 @@ import (
 	"strconv"
 
 	"github.com/caddyserver/certmagic"
-	"github.com/mholt/acmez/acme"
+	"github.com/mholt/acmez/v2/acme"
 
 	"github.com/caddyserver/caddy/v2"
 	"github.com/caddyserver/caddy/v2/caddyconfig"
@@ -212,9 +212,9 @@ func parseOptACMEDNS(d *caddyfile.Dispenser, _ any) (any, error) {
 	if err != nil {
 		return nil, err
 	}
-	prov, ok := unm.(certmagic.ACMEDNSProvider)
+	prov, ok := unm.(certmagic.DNSProvider)
 	if !ok {
-		return nil, d.Errf("module %s (%T) is not a certmagic.ACMEDNSProvider", modID, unm)
+		return nil, d.Errf("module %s (%T) is not a certmagic.DNSProvider", modID, unm)
 	}
 	return prov, nil
 }

caddyconfig/httpcaddyfile/tlsapp.go

@@ -24,7 +24,7 @@ import (
 	"strings"
 
 	"github.com/caddyserver/certmagic"
-	"github.com/mholt/acmez/acme"
+	"github.com/mholt/acmez/v2/acme"
 
 	"github.com/caddyserver/caddy/v2"
 	"github.com/caddyserver/caddy/v2/caddyconfig"
@@ -224,7 +224,7 @@ func (st ServerType) buildTLSApp(
 				var internal, external []string
 				for _, s := range ap.SubjectsRaw {
 					// do not create Issuers for Tailscale domains; they will be given a Manager instead
-					if strings.HasSuffix(strings.ToLower(s), ".ts.net") {
+					if isTailscaleDomain(s) {
 						continue
 					}
 					if !certmagic.SubjectQualifiesForCert(s) {
@@ -378,15 +378,12 @@ func (st ServerType) buildTLSApp(
 				if len(ap.Issuers) == 0 && automationPolicyHasAllPublicNames(ap) {
 					// for public names, create default issuers which will later be filled in with configured global defaults
 					// (internal names will implicitly use the internal issuer at auto-https time)
-					ap.Issuers = caddytls.DefaultIssuers()
+					emailStr, _ := globalEmail.(string)
+					ap.Issuers = caddytls.DefaultIssuers(emailStr)
 
 					// if a specific endpoint is configured, can't use multiple default issuers
 					if globalACMECA != nil {
-						if strings.Contains(globalACMECA.(string), "zerossl") {
-							ap.Issuers = []certmagic.Issuer{&caddytls.ZeroSSLIssuer{ACMEIssuer: new(caddytls.ACMEIssuer)}}
-						} else {
-							ap.Issuers = []certmagic.Issuer{new(caddytls.ACMEIssuer)}
-						}
+						ap.Issuers = []certmagic.Issuer{new(caddytls.ACMEIssuer)}
 					}
 				}
 			}
@@ -666,17 +663,33 @@ func automationPolicyShadows(i int, aps []*caddytls.AutomationPolicy) int {
 // subjectQualifiesForPublicCert is like certmagic.SubjectQualifiesForPublicCert() except
 // that this allows domains with multiple wildcard levels like '*.*.example.com' to qualify
 // if the automation policy has OnDemand enabled (i.e. this function is more lenient).
+//
+// IP subjects are considered as non-qualifying for public certs. Technically, there are
+// now public ACME CAs as well as non-ACME CAs that issue IP certificates. But this function
+// is used solely for implicit automation (defaults), where it gets really complicated to
+// keep track of which issuers support IP certificates in which circumstances. Currently,
+// issuers that support IP certificates are very few, and all require some sort of config
+// from the user anyway (such as an account credential). Since we cannot implicitly and
+// automatically get public IP certs without configuration from the user, we treat IPs as
+// not qualifying for public certificates. Users should expressly configure an issuer
+// that supports IP certs for that purpose.
 func subjectQualifiesForPublicCert(ap *caddytls.AutomationPolicy, subj string) bool {
 	return !certmagic.SubjectIsIP(subj) &&
 		!certmagic.SubjectIsInternal(subj) &&
 		(strings.Count(subj, "*.") < 2 || ap.OnDemand)
 }
 
+// automationPolicyHasAllPublicNames returns true if all the names on the policy
+// do NOT qualify for public certs OR are tailscale domains.
 func automationPolicyHasAllPublicNames(ap *caddytls.AutomationPolicy) bool {
 	for _, subj := range ap.SubjectsRaw {
-		if !subjectQualifiesForPublicCert(ap, subj) {
+		if !subjectQualifiesForPublicCert(ap, subj) || isTailscaleDomain(subj) {
 			return false
 		}
 	}
 	return true
 }
+
+func isTailscaleDomain(name string) bool {
+	return strings.HasSuffix(strings.ToLower(name), ".ts.net")
+}

caddytest/integration/acme_test.go

@@ -13,8 +13,8 @@ import (
 
 	"github.com/caddyserver/caddy/v2"
 	"github.com/caddyserver/caddy/v2/caddytest"
-	"github.com/mholt/acmez"
-	"github.com/mholt/acmez/acme"
+	"github.com/mholt/acmez/v2"
+	"github.com/mholt/acmez/v2/acme"
 	smallstepacme "github.com/smallstep/certificates/acme"
 	"go.uber.org/zap"
 )
@@ -77,7 +77,7 @@ func TestACMEServerWithDefaults(t *testing.T) {
 		return
 	}
 
-	certs, err := client.ObtainCertificate(ctx, account, certPrivateKey, []string{"localhost"})
+	certs, err := client.ObtainCertificateForSANs(ctx, account, certPrivateKey, []string{"localhost"})
 	if err != nil {
 		t.Errorf("obtaining certificate: %v", err)
 		return
@@ -146,7 +146,7 @@ func TestACMEServerWithMismatchedChallenges(t *testing.T) {
 		return
 	}
 
-	certs, err := client.ObtainCertificate(ctx, account, certPrivateKey, []string{"localhost"})
+	certs, err := client.ObtainCertificateForSANs(ctx, account, certPrivateKey, []string{"localhost"})
 	if len(certs) > 0 {
 		t.Errorf("expected '0' certificates, but received '%d'", len(certs))
 	}

caddytest/integration/acmeserver_test.go

@@ -9,8 +9,8 @@ import (
 	"testing"
 
 	"github.com/caddyserver/caddy/v2/caddytest"
-	"github.com/mholt/acmez"
-	"github.com/mholt/acmez/acme"
+	"github.com/mholt/acmez/v2"
+	"github.com/mholt/acmez/v2/acme"
 	"go.uber.org/zap"
 )
 
@@ -105,12 +105,7 @@ func TestACMEServerAllowPolicy(t *testing.T) {
 		return
 	}
 	{
-		certs, err := client.ObtainCertificate(
-			ctx,
-			account,
-			certPrivateKey,
-			[]string{"localhost"},
-		)
+		certs, err := client.ObtainCertificateForSANs(ctx, account, certPrivateKey, []string{"localhost"})
 		if err != nil {
 			t.Errorf("obtaining certificate for allowed domain: %v", err)
 			return
@@ -126,7 +121,7 @@ func TestACMEServerAllowPolicy(t *testing.T) {
 		}
 	}
 	{
-		_, err := client.ObtainCertificate(ctx, account, certPrivateKey, []string{"not-matching.localhost"})
+		_, err := client.ObtainCertificateForSANs(ctx, account, certPrivateKey, []string{"not-matching.localhost"})
 		if err == nil {
 			t.Errorf("obtaining certificate for 'not-matching.localhost' domain")
 		} else if err != nil && !strings.Contains(err.Error(), "urn:ietf:params:acme:error:rejectedIdentifier") {
@@ -199,7 +194,7 @@ func TestACMEServerDenyPolicy(t *testing.T) {
 		return
 	}
 	{
-		_, err := client.ObtainCertificate(ctx, account, certPrivateKey, []string{"deny.localhost"})
+		_, err := client.ObtainCertificateForSANs(ctx, account, certPrivateKey, []string{"deny.localhost"})
 		if err == nil {
 			t.Errorf("obtaining certificate for 'deny.localhost' domain")
 		} else if err != nil && !strings.Contains(err.Error(), "urn:ietf:params:acme:error:rejectedIdentifier") {

caddytest/integration/caddyfile_adapt/global_options_preferred_chains.caddyfiletest

@@ -40,12 +40,6 @@ example.com
 								"preferred_chains": {
 									"smallest": true
 								}
-							},
-							{
-								"module": "zerossl",
-								"preferred_chains": {
-									"smallest": true
-								}
 							}
 						]
 					}

caddytest/integration/caddyfile_adapt/tls_automation_policies_3.caddyfiletest

@@ -70,8 +70,9 @@ c.example.com {
 								"module": "acme"
 							},
 							{
+								"ca": "https://acme.zerossl.com/v2/DV90",
 								"email": "abc@example.com",
-								"module": "zerossl"
+								"module": "acme"
 							}
 						]
 					},

caddytest/integration/caddyfile_adapt/tls_automation_policies_4.caddyfiletest

@@ -131,8 +131,9 @@ abc.de {
 								"module": "acme"
 							},
 							{
+								"ca": "https://acme.zerossl.com/v2/DV90",
 								"email": "my.email@example.com",
-								"module": "zerossl"
+								"module": "acme"
 							}
 						]
 					}

caddytest/integration/caddyfile_adapt/tls_automation_policies_8.caddyfiletest

@@ -86,8 +86,9 @@ http://localhost:8081 {
 								"module": "acme"
 							},
 							{
+								"ca": "https://acme.zerossl.com/v2/DV90",
 								"email": "abc@example.com",
-								"module": "zerossl"
+								"module": "acme"
 							}
 						]
 					}

caddytest/integration/caddyfile_adapt/tls_automation_policies_global_email_localhost.caddyfiletest

@@ -54,8 +54,9 @@ example.com {
 								"module": "acme"
 							},
 							{
+								"ca": "https://acme.zerossl.com/v2/DV90",
 								"email": "foo@bar",
-								"module": "zerossl"
+								"module": "acme"
 							}
 						]
 					}

caddytest/integration/caddyfile_adapt/tls_dns_ttl.caddyfiletest

@@ -58,14 +58,6 @@ tls {
 									}
 								},
 								"module": "acme"
-							},
-							{
-								"challenges": {
-									"dns": {
-										"ttl": 310000000000
-									}
-								},
-								"module": "zerossl"
 							}
 						]
 					}

caddytest/integration/caddyfile_adapt/tls_explicit_issuer_dns_ttl.caddyfiletest

@@ -5,7 +5,7 @@ tls {
 	issuer acme {
 		dns_ttl 5m10s
 	}
-	issuer zerossl {
+	issuer zerossl api_key {
 		dns_ttl 10m20s
 	}
 }
@@ -65,10 +65,9 @@ tls {
 								"module": "acme"
 							},
 							{
-								"challenges": {
-									"dns": {
-										"ttl": 620000000000
-									}
+								"api_key": "api_key",
+								"cname_validation": {
+									"ttl": 620000000000
 								},
 								"module": "zerossl"
 							}

caddytest/integration/caddyfile_adapt/tls_explicit_issuer_propagation_options.caddyfiletest

@@ -6,7 +6,7 @@ tls {
 		propagation_delay 5m10s
 		propagation_timeout 10m20s
 	}
-	issuer zerossl {
+	issuer zerossl api_key {
 		propagation_delay 5m30s
 		propagation_timeout -1
 	}
@@ -68,11 +68,10 @@ tls {
 								"module": "acme"
 							},
 							{
-								"challenges": {
-									"dns": {
-										"propagation_delay": 330000000000,
-										"propagation_timeout": -1
-									}
+								"api_key": "api_key",
+								"cname_validation": {
+									"propagation_delay": 330000000000,
+									"propagation_timeout": -1
 								},
 								"module": "zerossl"
 							}

caddytest/integration/caddyfile_adapt/tls_propagation_options.caddyfiletest

@@ -60,15 +60,6 @@ tls {
 									}
 								},
 								"module": "acme"
-							},
-							{
-								"challenges": {
-									"dns": {
-										"propagation_delay": 310000000000,
-										"propagation_timeout": 620000000000
-									}
-								},
-								"module": "zerossl"
 							}
 						]
 					}

cmd/caddy/setcap.sh

@@ -1,13 +1,16 @@
 #!/bin/sh
 
-# USAGE: go run -exec ./setcap.sh main.go <args...>
+# USAGE:
+# 	go run -exec ./setcap.sh main.go <args...>
 #
 # (Example: `go run -exec ./setcap.sh main.go run --config caddy.json`)
 #
 # For some reason this does not work on my Arch system, so if you find that's
-# the case, you can instead do: go build && ./setcap.sh ./caddy <args...>
-# but this will leave the ./caddy binary laying around.
+# the case, you can instead do:
+#
+# 	go build && ./setcap.sh ./caddy <args...>
 #
+# but this will leave the ./caddy binary laying around.
 #
 
 sudo setcap cap_net_bind_service=+ep "$1"