httpcaddyfile: inherit global ACME issuer settings in tls shortcuts (#7617)

Author: steadytao

caddyconfig/httpcaddyfile/builtins.go

@@ -550,26 +550,11 @@ 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(acmeIssuer.Email)
-
-		// 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}
-		}
-
+		// implicit ACME issuers (from various subdirectives) should inherit from
+		// any globally-configured ACME issuer templates, then apply the local
+		// shortcut settings as overrides.
+		defaultIssuers := implicitACMEIssuers(h, acmeIssuer)
 		for _, issuer := range defaultIssuers {
-			// 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",
 				Value: issuer,

caddyconfig/httpcaddyfile/options_test.go

@@ -3,7 +3,9 @@ package httpcaddyfile
 import (
 	"encoding/json"
 	"testing"
+	"time"
 
+	"github.com/caddyserver/caddy/v2"
 	"github.com/caddyserver/caddy/v2/caddyconfig/caddyfile"
 	"github.com/caddyserver/caddy/v2/modules/caddytls"
 	_ "github.com/caddyserver/caddy/v2/modules/logging"
@@ -166,3 +168,126 @@ func TestGlobalResolversOption(t *testing.T) {
 		})
 	}
 }
+
+func TestGlobalCertIssuerAppliesToImplicitACMEIssuer(t *testing.T) {
+	adapter := caddyfile.Adapter{
+		ServerType: ServerType{},
+	}
+
+	input := `{
+		cert_issuer acme {
+			disable_tlsalpn_challenge
+		}
+	}
+	report.company.intern {
+		tls {
+			ca https://deglacme01.company.intern/acme/acme/directory
+			ca_root /etc/certs/company_root2.crt
+		}
+		respond "ok"
+	}`
+
+	out, _, err := adapter.Adapt([]byte(input), nil)
+	if err != nil {
+		t.Fatalf("adapting caddyfile: %v", err)
+	}
+
+	var config struct {
+		Apps struct {
+			TLS *caddytls.TLS `json:"tls"`
+		} `json:"apps"`
+	}
+	if err := json.Unmarshal(out, &config); err != nil {
+		t.Fatalf("unmarshaling adapted config: %v", err)
+	}
+	if config.Apps.TLS == nil || config.Apps.TLS.Automation == nil {
+		t.Fatal("expected tls automation config")
+	}
+
+	var subjectPolicy *caddytls.AutomationPolicy
+	for _, ap := range config.Apps.TLS.Automation.Policies {
+		if len(ap.SubjectsRaw) == 1 && ap.SubjectsRaw[0] == "report.company.intern" {
+			subjectPolicy = ap
+			break
+		}
+	}
+	if subjectPolicy == nil {
+		t.Fatal("expected subject-specific automation policy")
+	}
+	if len(subjectPolicy.IssuersRaw) != 1 {
+		t.Fatalf("expected one issuer for subject-specific policy, got %d", len(subjectPolicy.IssuersRaw))
+	}
+
+	var issuer caddytls.ACMEIssuer
+	if err := json.Unmarshal(subjectPolicy.IssuersRaw[0], &issuer); err != nil {
+		t.Fatalf("unmarshaling issuer: %v", err)
+	}
+	if issuer.CA != "https://deglacme01.company.intern/acme/acme/directory" {
+		t.Fatalf("expected custom ACME CA, got %q", issuer.CA)
+	}
+	if len(issuer.TrustedRootsPEMFiles) != 1 || issuer.TrustedRootsPEMFiles[0] != "/etc/certs/company_root2.crt" {
+		t.Fatalf("expected trusted roots to include site CA root, got %v", issuer.TrustedRootsPEMFiles)
+	}
+	if issuer.Challenges == nil || issuer.Challenges.TLSALPN == nil || !issuer.Challenges.TLSALPN.Disabled {
+		t.Fatalf("expected tls-alpn challenge to be disabled, got %#v", issuer.Challenges)
+	}
+}
+
+func TestMergeACMEIssuers(t *testing.T) {
+	base := &caddytls.ACMEIssuer{
+		Email: "ops@example.com",
+		Challenges: &caddytls.ChallengesConfig{
+			HTTP: &caddytls.HTTPChallengeConfig{
+				AlternatePort: 8080,
+			},
+			TLSALPN: &caddytls.TLSALPNChallengeConfig{
+				Disabled:      true,
+				AlternatePort: 8443,
+			},
+			DNS: &caddytls.DNSChallengeConfig{
+				Resolvers:      []string{"1.1.1.1"},
+				OverrideDomain: "_acme-challenge.example.net",
+			},
+		},
+		TrustedRootsPEMFiles: []string{"global.pem"},
+	}
+	overrides := &caddytls.ACMEIssuer{
+		CA: "https://deglacme01.company.intern/acme/acme/directory",
+		Challenges: &caddytls.ChallengesConfig{
+			HTTP: &caddytls.HTTPChallengeConfig{
+				Disabled: true,
+			},
+			DNS: &caddytls.DNSChallengeConfig{
+				PropagationTimeout: caddy.Duration(time.Minute),
+			},
+		},
+		TrustedRootsPEMFiles: []string{"site.pem"},
+	}
+
+	merged := mergeACMEIssuers(base, overrides)
+	if merged.CA != overrides.CA {
+		t.Fatalf("expected merged CA %q, got %q", overrides.CA, merged.CA)
+	}
+	if merged.Email != base.Email {
+		t.Fatalf("expected merged email %q, got %q", base.Email, merged.Email)
+	}
+	if len(merged.TrustedRootsPEMFiles) != 2 || merged.TrustedRootsPEMFiles[0] != "global.pem" || merged.TrustedRootsPEMFiles[1] != "site.pem" {
+		t.Fatalf("expected merged roots [global.pem site.pem], got %v", merged.TrustedRootsPEMFiles)
+	}
+	if merged.Challenges == nil || merged.Challenges.HTTP == nil || !merged.Challenges.HTTP.Disabled || merged.Challenges.HTTP.AlternatePort != 8080 {
+		t.Fatalf("expected merged HTTP challenge config to preserve alternate port and apply disable flag, got %#v", merged.Challenges)
+	}
+	if merged.Challenges.TLSALPN == nil || !merged.Challenges.TLSALPN.Disabled || merged.Challenges.TLSALPN.AlternatePort != 8443 {
+		t.Fatalf("expected merged TLS-ALPN challenge config to preserve global settings, got %#v", merged.Challenges)
+	}
+	if merged.Challenges.DNS == nil || merged.Challenges.DNS.PropagationTimeout != caddy.Duration(time.Minute) || len(merged.Challenges.DNS.Resolvers) != 1 || merged.Challenges.DNS.Resolvers[0] != "1.1.1.1" || merged.Challenges.DNS.OverrideDomain != "_acme-challenge.example.net" {
+		t.Fatalf("expected merged DNS challenge config to preserve global values and apply overrides, got %#v", merged.Challenges)
+	}
+
+	if base.CA != "" {
+		t.Fatalf("expected base issuer to remain unchanged, got CA %q", base.CA)
+	}
+	if len(base.TrustedRootsPEMFiles) != 1 || base.TrustedRootsPEMFiles[0] != "global.pem" {
+		t.Fatalf("expected base roots to remain unchanged, got %v", base.TrustedRootsPEMFiles)
+	}
+}