httpcaddyfile: Fix default issuers when email provided

If `tls <email>` is used, we should apply that to all applicable default issuers, not drop them. This refactoring applies implicit ACME issuer settings from the tls directive to all default ACME issuers, like ZeroSSL.

We also consolidate some annoying logic and improve config validity checks.

Ref: https://caddy.community/t/error-obtaining-certificate-after-caddy-restart/11335/8

4 files changed, 72 insertions, 50 deletions

diff --git a/caddyconfig/httpcaddyfile/builtins.go b/caddyconfig/httpcaddyfile/builtins.go
index aa68adb..4945a81 100644
--- a/caddyconfig/httpcaddyfile/builtins.go
+++ b/caddyconfig/httpcaddyfile/builtins.go
@@ -369,31 +369,57 @@ func parseTLS(h Helper) ([]ConfigValue, error) {
 		})
 	}
 
+	// some tls subdirectives are shortcuts that implicitly configure issuers, and the
+	// user can also configure issuers explicitly using the issuer subdirective; the
+	// logic to support both would likely be complex, or at least unintuitive
 	if len(issuers) > 0 && (acmeIssuer != nil || internalIssuer != nil) {
-		// some tls subdirectives are shortcuts that implicitly configure issuers, and the
-		// user can also configure issuers explicitly using the issuer subdirective; the
-		// logic to support both would likely be complex, or at least unintuitive
 		return nil, h.Err("cannot mix issuer subdirective (explicit issuers) with other issuer-specific subdirectives (implicit issuers)")
 	}
-	for _, issuer := range issuers {
-		configVals = append(configVals, ConfigValue{
-			Class: "tls.cert_issuer",
-			Value: issuer,
-		})
-	}
-	if acmeIssuer != nil {
-		configVals = append(configVals, ConfigValue{
-			Class: "tls.cert_issuer",
-			Value: disambiguateACMEIssuer(acmeIssuer),
-		})
+	if acmeIssuer != nil && internalIssuer != nil {
+		return nil, h.Err("cannot create both ACME and internal certificate issuers")
 	}
-	if internalIssuer != nil {
+
+	// now we should either have: explicitly-created issuers, or an implicitly-created
+	// ACME or internal issuer, or no issuers at all
+	switch {
+	case len(issuers) > 0:
+		for _, issuer := range issuers {
+			configVals = append(configVals, ConfigValue{
+				Class: "tls.cert_issuer",
+				Value: issuer,
+			})
+		}
+
+	case acmeIssuer != nil:
+		// implicit ACME issuers (from various subdirectives) - use defaults; there might be more than one
+		defaultIssuers := caddytls.DefaultIssuers()
+
+		// if a CA endpoint was set, override multiple implicit issuers since it's a specific one
+		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
+			}
+			configVals = append(configVals, ConfigValue{
+				Class: "tls.cert_issuer",
+				Value: issuer,
+			})
+		}
+
+	case internalIssuer != nil:
 		configVals = append(configVals, ConfigValue{
 			Class: "tls.cert_issuer",
 			Value: internalIssuer,
 		})
 	}
 
+	// certificate key type
 	if keyType != "" {
 		configVals = append(configVals, ConfigValue{
 			Class: "tls.key_type",
diff --git a/caddyconfig/httpcaddyfile/tlsapp.go b/caddyconfig/httpcaddyfile/tlsapp.go
index dbf3cc7..d831d1b 100644
--- a/caddyconfig/httpcaddyfile/tlsapp.go
+++ b/caddyconfig/httpcaddyfile/tlsapp.go
@@ -316,13 +316,15 @@ func (st ServerType) buildTLSApp(
 		if hasGlobalACMEDefaults {
 			for _, ap := range tlsApp.Automation.Policies {
 				if len(ap.Issuers) == 0 {
-					acme, zerosslACME := new(caddytls.ACMEIssuer), new(caddytls.ACMEIssuer)
-					zerossl := &caddytls.ZeroSSLIssuer{ACMEIssuer: zerosslACME}
-					ap.Issuers = []certmagic.Issuer{acme, zerossl} // TODO: keep this in sync with Caddy's other issuer defaults elsewhere, like in caddytls/automation.go (DefaultIssuers).
-
-					// if a non-ZeroSSL endpoint is specified, we assume we can't use the ZeroSSL issuer successfully
-					if globalACMECA != nil && !strings.Contains(globalACMECA.(string), "zerossl") {
-						ap.Issuers = []certmagic.Issuer{acme}
+					ap.Issuers = caddytls.DefaultIssuers()
+
+					// 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)}
+						}
 					}
 				}
 			}
@@ -463,19 +465,6 @@ func newBaseAutomationPolicy(options map[string]interface{}, warnings []caddycon
 	return ap, nil
 }
 
-// disambiguateACMEIssuer returns an issuer based on the properties of acmeIssuer.
-// If acmeIssuer implicitly configures a certain kind of ACMEIssuer (for example,
-// ZeroSSL), the proper wrapper over acmeIssuer will be returned instead.
-func disambiguateACMEIssuer(acmeIssuer *caddytls.ACMEIssuer) certmagic.Issuer {
-	// as a special case, we integrate with ZeroSSL's ACME endpoint if it looks like an
-	// implicit ZeroSSL configuration (this requires a wrapper type over ACMEIssuer
-	// because of the EAB generation; if EAB is provided, we can use plain ACMEIssuer)
-	if strings.Contains(acmeIssuer.CA, "acme.zerossl.com") && acmeIssuer.ExternalAccount == nil {
-		return &caddytls.ZeroSSLIssuer{ACMEIssuer: acmeIssuer}
-	}
-	return acmeIssuer
-}
-
 // consolidateAutomationPolicies combines automation policies that are the same,
 // for a cleaner overall output.
 func consolidateAutomationPolicies(aps []*caddytls.AutomationPolicy) []*caddytls.AutomationPolicy {
diff --git a/modules/caddyhttp/autohttps.go b/modules/caddyhttp/autohttps.go
index c1d4c08..5c83d8f 100644
--- a/modules/caddyhttp/autohttps.go
+++ b/modules/caddyhttp/autohttps.go
@@ -444,7 +444,7 @@ func (app *App) createAutomationPolicies(ctx caddy.Context, internalNames []stri
 		// what the HTTP and HTTPS ports are)
 		if ap.Issuers == nil {
 			var err error
-			ap.Issuers, err = caddytls.DefaultIssuers(ctx)
+			ap.Issuers, err = caddytls.DefaultIssuersProvisioned(ctx)
 			if err != nil {
 				return err
 			}
@@ -499,7 +499,7 @@ func (app *App) createAutomationPolicies(ctx caddy.Context, internalNames []stri
 	// never overwrite any other issuer that might already be configured
 	if basePolicy.Issuers == nil {
 		var err error
-		basePolicy.Issuers, err = caddytls.DefaultIssuers(ctx)
+		basePolicy.Issuers, err = caddytls.DefaultIssuersProvisioned(ctx)
 		if err != nil {
 			return err
 		}
diff --git a/modules/caddytls/automation.go b/modules/caddytls/automation.go
index ed29e06..bcc0a0c 100644
--- a/modules/caddytls/automation.go
+++ b/modules/caddytls/automation.go
@@ -187,7 +187,7 @@ func (ap *AutomationPolicy) Provision(tlsApp *TLS) error {
 	issuers := ap.Issuers
 	if len(issuers) == 0 {
 		var err error
-		issuers, err = DefaultIssuers(tlsApp.ctx)
+		issuers, err = DefaultIssuersProvisioned(tlsApp.ctx)
 		if err != nil {
 			return err
 		}
@@ -242,21 +242,28 @@ func (ap *AutomationPolicy) Provision(tlsApp *TLS) error {
 	return nil
 }
 
-// DefaultIssuers returns empty but provisioned default Issuers.
+// DefaultIssuers returns empty Issuers (not provisioned) to be used as defaults.
 // This function is experimental and has no compatibility promises.
-func DefaultIssuers(ctx caddy.Context) ([]certmagic.Issuer, error) {
-	acme := new(ACMEIssuer)
-	err := acme.Provision(ctx)
-	if err != nil {
-		return nil, err
+func DefaultIssuers() []certmagic.Issuer {
+	return []certmagic.Issuer{
+		new(ACMEIssuer),
+		&ZeroSSLIssuer{ACMEIssuer: new(ACMEIssuer)},
 	}
-	zerossl := new(ZeroSSLIssuer)
-	err = zerossl.Provision(ctx)
-	if err != nil {
-		return nil, err
+}
+
+// DefaultIssuersProvisioned returns empty but provisioned default Issuers from
+// DefaultIssuers(). This function is experimental and has no compatibility promises.
+func DefaultIssuersProvisioned(ctx caddy.Context) ([]certmagic.Issuer, error) {
+	issuers := DefaultIssuers()
+	for i, iss := range issuers {
+		if prov, ok := iss.(caddy.Provisioner); ok {
+			err := prov.Provision(ctx)
+			if err != nil {
+				return nil, fmt.Errorf("provisioning default issuer %d: %T: %v", i, iss, err)
+			}
+		}
 	}
-	// TODO: eventually, insert ZeroSSL into first position in the slice -- see also httpcaddyfile/tlsapp.go for where similar defaults are configured
-	return []certmagic.Issuer{acme, zerossl}, nil
+	return issuers, nil
 }
 
 // ChallengesConfig configures the ACME challenges.