caddytls: Support multiple issuers (#3862)

* caddytls: Support multiple issuers

Defaults are Let's Encrypt and ZeroSSL.

There are probably bugs.

* Commit updated integration tests, d'oh

* Update go.mod

2 files changed, 140 insertions, 115 deletions

diff --git a/caddyconfig/httpcaddyfile/builtins.go b/caddyconfig/httpcaddyfile/builtins.go
index 5ba35aa..f8b5e2c 100644
--- a/caddyconfig/httpcaddyfile/builtins.go
+++ b/caddyconfig/httpcaddyfile/builtins.go
@@ -88,7 +88,7 @@ func parseTLS(h Helper) ([]ConfigValue, error) {
 	var certSelector caddytls.CustomCertSelectionPolicy
 	var acmeIssuer *caddytls.ACMEIssuer
 	var internalIssuer *caddytls.InternalIssuer
-	var issuer certmagic.Issuer
+	var issuers []certmagic.Issuer
 	var onDemand bool
 
 	for h.Next() {
@@ -297,10 +297,11 @@ func parseTLS(h Helper) ([]ConfigValue, error) {
 				if err != nil {
 					return nil, err
 				}
-				issuer, ok = unm.(certmagic.Issuer)
+				issuer, ok := unm.(certmagic.Issuer)
 				if !ok {
 					return nil, h.Errf("module %s is not a certmagic.Issuer", mod.ID)
 				}
+				issuers = append(issuers, issuer)
 
 			case "dns":
 				if !h.NextArg() {
@@ -371,42 +372,28 @@ func parseTLS(h Helper) ([]ConfigValue, error) {
 		})
 	}
 
-	// issuer
-	if acmeIssuer != nil && internalIssuer != nil {
-		// the logic to support this would be complex
-		return nil, h.Err("cannot use both ACME and internal issuers in same server block")
+	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)")
 	}
-	if issuer != nil && (acmeIssuer != nil || internalIssuer != nil) {
-		// similarly, the logic to support this would be complex
-		return nil, h.Err("when defining an issuer, all its config must be in its block, rather than from separate tls subdirectives")
-	}
-	switch {
-	case issuer != nil:
+	for _, issuer := range issuers {
 		configVals = append(configVals, ConfigValue{
 			Class: "tls.cert_issuer",
 			Value: issuer,
 		})
-
-	case internalIssuer != nil:
+	}
+	if acmeIssuer != nil {
 		configVals = append(configVals, ConfigValue{
 			Class: "tls.cert_issuer",
-			Value: internalIssuer,
+			Value: disambiguateACMEIssuer(acmeIssuer),
 		})
-
-	case acmeIssuer != nil:
-		// fill in global defaults, if configured
-		if email := h.Option("email"); email != nil && acmeIssuer.Email == "" {
-			acmeIssuer.Email = email.(string)
-		}
-		if acmeCA := h.Option("acme_ca"); acmeCA != nil && acmeIssuer.CA == "" {
-			acmeIssuer.CA = acmeCA.(string)
-		}
-		if caPemFile := h.Option("acme_ca_root"); caPemFile != nil {
-			acmeIssuer.TrustedRootsPEMFiles = append(acmeIssuer.TrustedRootsPEMFiles, caPemFile.(string))
-		}
+	}
+	if internalIssuer != nil {
 		configVals = append(configVals, ConfigValue{
 			Class: "tls.cert_issuer",
-			Value: disambiguateACMEIssuer(acmeIssuer),
+			Value: internalIssuer,
 		})
 	}
 
diff --git a/caddyconfig/httpcaddyfile/tlsapp.go b/caddyconfig/httpcaddyfile/tlsapp.go
index 0ac862e..fe4c1b1 100644
--- a/caddyconfig/httpcaddyfile/tlsapp.go
+++ b/caddyconfig/httpcaddyfile/tlsapp.go
@@ -110,47 +110,43 @@ func (st ServerType) buildTLSApp(
 
 			// certificate issuers
 			if issuerVals, ok := sblock.pile["tls.cert_issuer"]; ok {
+				var issuers []certmagic.Issuer
 				for _, issuerVal := range issuerVals {
-					issuer := issuerVal.Value.(certmagic.Issuer)
-					if ap == catchAllAP && !reflect.DeepEqual(ap.Issuer, issuer) {
-						return nil, warnings, fmt.Errorf("automation policy from site block is also default/catch-all policy because of key without hostname, and the two are in conflict: %#v != %#v", ap.Issuer, issuer)
-					}
-					ap.Issuer = issuer
+					ap.Issuers = append(ap.Issuers, issuerVal.Value.(certmagic.Issuer))
+				}
+				if ap == catchAllAP && !reflect.DeepEqual(ap.Issuers, issuers) {
+					return nil, warnings, fmt.Errorf("automation policy from site block is also default/catch-all policy because of key without hostname, and the two are in conflict: %#v != %#v", ap.Issuers, issuers)
 				}
 			}
 
 			// custom bind host
 			for _, cfgVal := range sblock.pile["bind"] {
-				// if an issuer was already configured and it is NOT an ACME
-				// issuer, skip, since we intend to adjust only ACME issuers
-				var acmeIssuer *caddytls.ACMEIssuer
-				if ap.Issuer != nil {
-					// ensure we include any issuer that embeds/wraps an underlying ACME issuer
-					type acmeCapable interface{ GetACMEIssuer() *caddytls.ACMEIssuer }
-					if acmeWrapper, ok := ap.Issuer.(acmeCapable); ok {
+				for _, iss := range ap.Issuers {
+					// if an issuer was already configured and it is NOT an ACME issuer,
+					// skip, since we intend to adjust only ACME issuers; ensure we
+					// include any issuer that embeds/wraps an underlying ACME issuer
+					var acmeIssuer *caddytls.ACMEIssuer
+					if acmeWrapper, ok := iss.(acmeCapable); ok {
 						acmeIssuer = acmeWrapper.GetACMEIssuer()
-					} else {
-						break
 					}
-				}
+					if acmeIssuer == nil {
+						continue
+					}
 
-				// proceed to configure the ACME issuer's bind host, without
-				// overwriting any existing settings
-				if acmeIssuer == nil {
-					acmeIssuer = new(caddytls.ACMEIssuer)
-				}
-				if acmeIssuer.Challenges == nil {
-					acmeIssuer.Challenges = new(caddytls.ChallengesConfig)
-				}
-				if acmeIssuer.Challenges.BindHost == "" {
-					// only binding to one host is supported
-					var bindHost string
-					if bindHosts, ok := cfgVal.Value.([]string); ok && len(bindHosts) > 0 {
-						bindHost = bindHosts[0]
+					// proceed to configure the ACME issuer's bind host, without
+					// overwriting any existing settings
+					if acmeIssuer.Challenges == nil {
+						acmeIssuer.Challenges = new(caddytls.ChallengesConfig)
+					}
+					if acmeIssuer.Challenges.BindHost == "" {
+						// only binding to one host is supported
+						var bindHost string
+						if bindHosts, ok := cfgVal.Value.([]string); ok && len(bindHosts) > 0 {
+							bindHost = bindHosts[0]
+						}
+						acmeIssuer.Challenges.BindHost = bindHost
 					}
-					acmeIssuer.Challenges.BindHost = bindHost
 				}
-				ap.Issuer = acmeIssuer // we'll encode it later
 			}
 
 			// first make sure this block is allowed to create an automation policy;
@@ -188,7 +184,7 @@ func (st ServerType) buildTLSApp(
 			// that the internal names can use the internal issuer and
 			// the other names can use the default/public/ACME issuer
 			var ap2 *caddytls.AutomationPolicy
-			if ap.Issuer == nil {
+			if len(ap.Issuers) == 0 {
 				var internal, external []string
 				for _, s := range ap.Subjects {
 					if !certmagic.SubjectQualifiesForCert(s) {
@@ -212,7 +208,7 @@ func (st ServerType) buildTLSApp(
 					apCopy := *ap
 					ap2 = &apCopy
 					ap2.Subjects = internal
-					ap2.IssuerRaw = caddyconfig.JSONModuleObject(caddytls.InternalIssuer{}, "module", "internal", &warnings)
+					ap2.IssuersRaw = []json.RawMessage{caddyconfig.JSONModuleObject(caddytls.InternalIssuer{}, "module", "internal", &warnings)}
 				}
 			}
 			if tlsApp.Automation == nil {
@@ -277,7 +273,7 @@ func (st ServerType) buildTLSApp(
 	// get internal certificates by default rather than ACME
 	var al caddytls.AutomateLoader
 	internalAP := &caddytls.AutomationPolicy{
-		IssuerRaw: json.RawMessage(`{"module":"internal"}`),
+		IssuersRaw: []json.RawMessage{json.RawMessage(`{"module":"internal"}`)},
 	}
 	for h := range hostsSharedWithHostlessKey {
 		al = append(al, h)
@@ -295,14 +291,48 @@ func (st ServerType) buildTLSApp(
 		tlsApp.Automation.Policies = append(tlsApp.Automation.Policies, internalAP)
 	}
 
+	// if there are any global options set for issuers (ACME ones in particular), make sure they
+	// take effect in every automation policy that does not have any issuers
+	if tlsApp.Automation != nil {
+		globalEmail := options["email"]
+		globalACMECA := options["acme_ca"]
+		globalACMECARoot := options["acme_ca_root"]
+		globalACMEDNS := options["acme_dns"]
+		globalACMEEAB := options["acme_eab"]
+		hasGlobalACMEDefaults := globalEmail != nil || globalACMECA != nil || globalACMECARoot != nil || globalACMEDNS != nil || globalACMEEAB != nil
+		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}
+					}
+				}
+			}
+		}
+	}
+
 	// finalize and verify policies; do cleanup
 	if tlsApp.Automation != nil {
-		// encode any issuer values we created, so they will be rendered in the output
-		for _, ap := range tlsApp.Automation.Policies {
-			if ap.Issuer != nil && ap.IssuerRaw == nil {
-				// encode issuer now that it's all set up
-				issuerName := ap.Issuer.(caddy.Module).CaddyModule().ID.Name()
-				ap.IssuerRaw = caddyconfig.JSONModuleObject(ap.Issuer, "module", issuerName, &warnings)
+		for i, ap := range tlsApp.Automation.Policies {
+			// ensure all issuers have global defaults filled in
+			for j, issuer := range ap.Issuers {
+				err := fillInGlobalACMEDefaults(issuer, options)
+				if err != nil {
+					return nil, warnings, fmt.Errorf("filling in global issuer defaults for AP %d, issuer %d: %v", i, j, err)
+				}
+			}
+
+			// encode all issuer values we created, so they will be rendered in the output
+			if len(ap.Issuers) > 0 && ap.IssuersRaw == nil {
+				for _, iss := range ap.Issuers {
+					issuerName := iss.(caddy.Module).CaddyModule().ID.Name()
+					ap.IssuersRaw = append(ap.IssuersRaw, caddyconfig.JSONModuleObject(iss, "module", issuerName, &warnings))
+				}
 			}
 		}
 
@@ -334,6 +364,51 @@ func (st ServerType) buildTLSApp(
 	return tlsApp, warnings, nil
 }
 
+type acmeCapable interface{ GetACMEIssuer() *caddytls.ACMEIssuer }
+
+func fillInGlobalACMEDefaults(issuer certmagic.Issuer, options map[string]interface{}) error {
+	acmeWrapper, ok := issuer.(acmeCapable)
+	if !ok {
+		return nil
+	}
+	acmeIssuer := acmeWrapper.GetACMEIssuer()
+	if acmeIssuer == nil {
+		return nil
+	}
+
+	globalEmail := options["email"]
+	globalACMECA := options["acme_ca"]
+	globalACMECARoot := options["acme_ca_root"]
+	globalACMEDNS := options["acme_dns"]
+	globalACMEEAB := options["acme_eab"]
+
+	if globalEmail != nil && acmeIssuer.Email == "" {
+		acmeIssuer.Email = globalEmail.(string)
+	}
+	if globalACMECA != nil && acmeIssuer.CA == "" {
+		acmeIssuer.CA = globalACMECA.(string)
+	}
+	if globalACMECARoot != nil && !sliceContains(acmeIssuer.TrustedRootsPEMFiles, globalACMECARoot.(string)) {
+		acmeIssuer.TrustedRootsPEMFiles = append(acmeIssuer.TrustedRootsPEMFiles, globalACMECARoot.(string))
+	}
+	if globalACMEDNS != nil && (acmeIssuer.Challenges == nil || acmeIssuer.Challenges.DNS == nil) {
+		provName := globalACMEDNS.(string)
+		dnsProvModule, err := caddy.GetModule("dns.providers." + provName)
+		if err != nil {
+			return fmt.Errorf("getting DNS provider module named '%s': %v", provName, err)
+		}
+		acmeIssuer.Challenges = &caddytls.ChallengesConfig{
+			DNS: &caddytls.DNSChallengeConfig{
+				ProviderRaw: caddyconfig.JSONModuleObject(dnsProvModule.New(), "name", provName, nil),
+			},
+		}
+	}
+	if globalACMEEAB != nil && acmeIssuer.ExternalAccount == nil {
+		acmeIssuer.ExternalAccount = globalACMEEAB.(*acme.EAB)
+	}
+	return nil
+}
+
 // newBaseAutomationPolicy returns a new TLS automation policy that gets
 // its values from the global options map. It should be used as the base
 // for any other automation policies. A nil policy (and no error) will be
@@ -341,17 +416,10 @@ func (st ServerType) buildTLSApp(
 // true, a non-nil value will always be returned (unless there is an error).
 func newBaseAutomationPolicy(options map[string]interface{}, warnings []caddyconfig.Warning, always bool) (*caddytls.AutomationPolicy, error) {
 	issuer, hasIssuer := options["cert_issuer"]
-
-	acmeCA, hasACMECA := options["acme_ca"]
-	acmeCARoot, hasACMECARoot := options["acme_ca_root"]
-	acmeDNS, hasACMEDNS := options["acme_dns"]
-	acmeEAB, hasACMEEAB := options["acme_eab"]
-
-	email, hasEmail := options["email"]
-	localCerts, hasLocalCerts := options["local_certs"]
+	_, hasLocalCerts := options["local_certs"]
 	keyType, hasKeyType := options["key_type"]
 
-	hasGlobalAutomationOpts := hasIssuer || hasACMECA || hasACMECARoot || hasACMEDNS || hasACMEEAB || hasEmail || hasLocalCerts || hasKeyType
+	hasGlobalAutomationOpts := hasIssuer || hasLocalCerts || hasKeyType
 
 	// if there are no global options related to automation policies
 	// set, then we can just return right away
@@ -363,48 +431,18 @@ func newBaseAutomationPolicy(options map[string]interface{}, warnings []caddycon
 	}
 
 	ap := new(caddytls.AutomationPolicy)
-	if keyType != nil {
+	if hasKeyType {
 		ap.KeyType = keyType.(string)
 	}
 
+	if hasIssuer && hasLocalCerts {
+		return nil, fmt.Errorf("global options are ambiguous: local_certs is confusing when combined with cert_issuer, because local_certs is also a specific kind of issuer")
+	}
+
 	if hasIssuer {
-		if hasACMECA || hasACMEDNS || hasACMEEAB || hasEmail || hasLocalCerts {
-			return nil, fmt.Errorf("global options are ambiguous: cert_issuer is confusing when combined with acme_*, email, or local_certs options")
-		}
-		ap.Issuer = issuer.(certmagic.Issuer)
-	} else if localCerts != nil {
-		// internal issuer enabled trumps any ACME configurations; useful in testing
-		ap.Issuer = new(caddytls.InternalIssuer) // we'll encode it later
-	} else {
-		if acmeCA == nil {
-			acmeCA = ""
-		}
-		if email == nil {
-			email = ""
-		}
-		mgr := &caddytls.ACMEIssuer{
-			CA:    acmeCA.(string),
-			Email: email.(string),
-		}
-		if acmeDNS != nil {
-			provName := acmeDNS.(string)
-			dnsProvModule, err := caddy.GetModule("dns.providers." + provName)
-			if err != nil {
-				return nil, fmt.Errorf("getting DNS provider module named '%s': %v", provName, err)
-			}
-			mgr.Challenges = &caddytls.ChallengesConfig{
-				DNS: &caddytls.DNSChallengeConfig{
-					ProviderRaw: caddyconfig.JSONModuleObject(dnsProvModule.New(), "name", provName, &warnings),
-				},
-			}
-		}
-		if acmeCARoot != nil {
-			mgr.TrustedRootsPEMFiles = []string{acmeCARoot.(string)}
-		}
-		if acmeEAB != nil {
-			mgr.ExternalAccount = acmeEAB.(*acme.EAB)
-		}
-		ap.Issuer = disambiguateACMEIssuer(mgr) // we'll encode it later
+		ap.Issuers = []certmagic.Issuer{issuer.(certmagic.Issuer)}
+	} else if hasLocalCerts {
+		ap.Issuers = []certmagic.Issuer{new(caddytls.InternalIssuer)}
 	}
 
 	return ap, nil
@@ -463,7 +501,7 @@ func consolidateAutomationPolicies(aps []*caddytls.AutomationPolicy) []*caddytls
 			// otherwise the one without any subjects (a catch-all) would be
 			// eaten up by the one with subjects; and if both have subjects, we
 			// need to combine their lists
-			if bytes.Equal(aps[i].IssuerRaw, aps[j].IssuerRaw) &&
+			if reflect.DeepEqual(aps[i].IssuersRaw, aps[j].IssuersRaw) &&
 				bytes.Equal(aps[i].StorageRaw, aps[j].StorageRaw) &&
 				aps[i].MustStaple == aps[j].MustStaple &&
 				aps[i].KeyType == aps[j].KeyType &&