From e16a886814d8cd43d545de38a4d6b98313fb31cb Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Mon, 27 Mar 2023 17:16:22 -0400 Subject: caddytls: Eval replacer on automation policy subjects (#5459) Also renamed the field to SubjectsRaw, which can be considered a breaking change but I don't expect this to affect much. --- caddyconfig/httpcaddyfile/tlsapp.go | 38 +++++++++++++++---------------- caddyconfig/httpcaddyfile/tlsapp_test.go | 4 ++-- modules/caddyhttp/autohttps.go | 6 ++--- modules/caddyhttp/reverseproxy/command.go | 4 ++-- modules/caddytls/automation.go | 24 +++++++++++++++---- modules/caddytls/tls.go | 21 ++++++++++------- 6 files changed, 58 insertions(+), 39 deletions(-) diff --git a/caddyconfig/httpcaddyfile/tlsapp.go b/caddyconfig/httpcaddyfile/tlsapp.go index 3d32b4f..2021970 100644 --- a/caddyconfig/httpcaddyfile/tlsapp.go +++ b/caddyconfig/httpcaddyfile/tlsapp.go @@ -206,8 +206,8 @@ func (st ServerType) buildTLSApp( } // associate our new automation policy with this server block's hosts - ap.Subjects = sblock.hostsFromKeysNotHTTP(httpPort) - sort.Strings(ap.Subjects) // solely for deterministic test results + ap.SubjectsRaw = sblock.hostsFromKeysNotHTTP(httpPort) + sort.Strings(ap.SubjectsRaw) // solely for deterministic test results // if a combination of public and internal names were given // for this same server block and no issuer was specified, we @@ -217,7 +217,7 @@ func (st ServerType) buildTLSApp( var ap2 *caddytls.AutomationPolicy if len(ap.Issuers) == 0 { var internal, external []string - for _, s := range ap.Subjects { + for _, s := range ap.SubjectsRaw { if !certmagic.SubjectQualifiesForCert(s) { return nil, warnings, fmt.Errorf("subject does not qualify for certificate: '%s'", s) } @@ -235,10 +235,10 @@ func (st ServerType) buildTLSApp( } } if len(external) > 0 && len(internal) > 0 { - ap.Subjects = external + ap.SubjectsRaw = external apCopy := *ap ap2 = &apCopy - ap2.Subjects = internal + ap2.SubjectsRaw = internal ap2.IssuersRaw = []json.RawMessage{caddyconfig.JSONModuleObject(caddytls.InternalIssuer{}, "module", "internal", &warnings)} } } @@ -339,14 +339,14 @@ func (st ServerType) buildTLSApp( for h := range httpsHostsSharedWithHostlessKey { al = append(al, h) if !certmagic.SubjectQualifiesForPublicCert(h) { - internalAP.Subjects = append(internalAP.Subjects, h) + internalAP.SubjectsRaw = append(internalAP.SubjectsRaw, h) } } } if len(al) > 0 { tlsApp.CertificatesRaw["automate"] = caddyconfig.JSON(al, &warnings) } - if len(internalAP.Subjects) > 0 { + if len(internalAP.SubjectsRaw) > 0 { if tlsApp.Automation == nil { tlsApp.Automation = new(caddytls.AutomationConfig) } @@ -412,7 +412,7 @@ func (st ServerType) buildTLSApp( // for convenience) automationHostSet := make(map[string]struct{}) for _, ap := range tlsApp.Automation.Policies { - for _, s := range ap.Subjects { + for _, s := range ap.SubjectsRaw { if _, ok := automationHostSet[s]; ok { return nil, warnings, fmt.Errorf("hostname appears in more than one automation policy, making certificate management ambiguous: %s", s) } @@ -533,7 +533,7 @@ func consolidateAutomationPolicies(aps []*caddytls.AutomationPolicy) []*caddytls if automationPolicyIsSubset(aps[j], aps[i]) { return false } - return len(aps[i].Subjects) > len(aps[j].Subjects) + return len(aps[i].SubjectsRaw) > len(aps[j].SubjectsRaw) }) emptyAPCount := 0 @@ -541,7 +541,7 @@ func consolidateAutomationPolicies(aps []*caddytls.AutomationPolicy) []*caddytls // compute the number of empty policies (disregarding subjects) - see #4128 emptyAP := new(caddytls.AutomationPolicy) for i := 0; i < len(aps); i++ { - emptyAP.Subjects = aps[i].Subjects + emptyAP.SubjectsRaw = aps[i].SubjectsRaw if reflect.DeepEqual(aps[i], emptyAP) { emptyAPCount++ if !automationPolicyHasAllPublicNames(aps[i]) { @@ -583,7 +583,7 @@ outer: aps[i].KeyType == aps[j].KeyType && aps[i].OnDemand == aps[j].OnDemand && aps[i].RenewalWindowRatio == aps[j].RenewalWindowRatio { - if len(aps[i].Subjects) > 0 && len(aps[j].Subjects) == 0 { + if len(aps[i].SubjectsRaw) > 0 && len(aps[j].SubjectsRaw) == 0 { // later policy (at j) has no subjects ("catch-all"), so we can // remove the identical-but-more-specific policy that comes first // AS LONG AS it is not shadowed by another policy before it; e.g. @@ -598,9 +598,9 @@ outer: } } else { // avoid repeated subjects - for _, subj := range aps[j].Subjects { - if !sliceContains(aps[i].Subjects, subj) { - aps[i].Subjects = append(aps[i].Subjects, subj) + for _, subj := range aps[j].SubjectsRaw { + if !sliceContains(aps[i].SubjectsRaw, subj) { + aps[i].SubjectsRaw = append(aps[i].SubjectsRaw, subj) } } aps = append(aps[:j], aps[j+1:]...) @@ -616,15 +616,15 @@ outer: // automationPolicyIsSubset returns true if a's subjects are a subset // of b's subjects. func automationPolicyIsSubset(a, b *caddytls.AutomationPolicy) bool { - if len(b.Subjects) == 0 { + if len(b.SubjectsRaw) == 0 { return true } - if len(a.Subjects) == 0 { + if len(a.SubjectsRaw) == 0 { return false } - for _, aSubj := range a.Subjects { + for _, aSubj := range a.SubjectsRaw { var inSuperset bool - for _, bSubj := range b.Subjects { + for _, bSubj := range b.SubjectsRaw { if certmagic.MatchWildcard(aSubj, bSubj) { inSuperset = true break @@ -662,7 +662,7 @@ func subjectQualifiesForPublicCert(ap *caddytls.AutomationPolicy, subj string) b } func automationPolicyHasAllPublicNames(ap *caddytls.AutomationPolicy) bool { - for _, subj := range ap.Subjects { + for _, subj := range ap.SubjectsRaw { if !subjectQualifiesForPublicCert(ap, subj) { return false } diff --git a/caddyconfig/httpcaddyfile/tlsapp_test.go b/caddyconfig/httpcaddyfile/tlsapp_test.go index 1925e02..d8edbdf 100644 --- a/caddyconfig/httpcaddyfile/tlsapp_test.go +++ b/caddyconfig/httpcaddyfile/tlsapp_test.go @@ -47,8 +47,8 @@ func TestAutomationPolicyIsSubset(t *testing.T) { expect: false, }, } { - apA := &caddytls.AutomationPolicy{Subjects: test.a} - apB := &caddytls.AutomationPolicy{Subjects: test.b} + apA := &caddytls.AutomationPolicy{SubjectsRaw: test.a} + apB := &caddytls.AutomationPolicy{SubjectsRaw: test.b} if actual := automationPolicyIsSubset(apA, apB); actual != test.expect { t.Errorf("Test %d: Expected %t but got %t (A: %v B: %v)", i, test.expect, actual, test.a, test.b) } diff --git a/modules/caddyhttp/autohttps.go b/modules/caddyhttp/autohttps.go index be229ea..86b34d3 100644 --- a/modules/caddyhttp/autohttps.go +++ b/modules/caddyhttp/autohttps.go @@ -285,7 +285,7 @@ uniqueDomainsLoop: // one automation policy would be confusing and an error if app.tlsApp.Automation != nil { for _, ap := range app.tlsApp.Automation.Policies { - for _, apHost := range ap.Subjects { + for _, apHost := range ap.Subjects() { if apHost == d { continue uniqueDomainsLoop } @@ -518,7 +518,7 @@ func (app *App) createAutomationPolicies(ctx caddy.Context, internalNames []stri } // while we're here, is this the catch-all/base policy? - if !foundBasePolicy && len(ap.Subjects) == 0 { + if !foundBasePolicy && len(ap.SubjectsRaw) == 0 { basePolicy = ap foundBasePolicy = true } @@ -634,7 +634,7 @@ func (app *App) createAutomationPolicies(ctx caddy.Context, internalNames []stri // rather they just want to change the CA for the set // of names that would normally use the production API; // anyway, that gets into the weeds a bit... - newPolicy.Subjects = internalNames + newPolicy.SubjectsRaw = internalNames newPolicy.Issuers = []certmagic.Issuer{internalIssuer} err := app.tlsApp.AddAutomationPolicy(newPolicy) if err != nil { diff --git a/modules/caddyhttp/reverseproxy/command.go b/modules/caddyhttp/reverseproxy/command.go index 02c921c..5e8beb1 100644 --- a/modules/caddyhttp/reverseproxy/command.go +++ b/modules/caddyhttp/reverseproxy/command.go @@ -259,8 +259,8 @@ func cmdReverseProxy(fs caddycmd.Flags) (int, error) { tlsApp := caddytls.TLS{ Automation: &caddytls.AutomationConfig{ Policies: []*caddytls.AutomationPolicy{{ - Subjects: []string{fromAddr.Host}, - IssuersRaw: []json.RawMessage{json.RawMessage(`{"module":"internal"}`)}, + SubjectsRaw: []string{fromAddr.Host}, + IssuersRaw: []json.RawMessage{json.RawMessage(`{"module":"internal"}`)}, }}, }, } diff --git a/modules/caddytls/automation.go b/modules/caddytls/automation.go index 1cfb28c..58ffe4c 100644 --- a/modules/caddytls/automation.go +++ b/modules/caddytls/automation.go @@ -85,7 +85,7 @@ type AutomationConfig struct { // TLS app to properly provision a new policy. type AutomationPolicy struct { // Which subjects (hostnames or IP addresses) this policy applies to. - Subjects []string `json:"subjects,omitempty"` + SubjectsRaw []string `json:"subjects,omitempty"` // The modules that may issue certificates. Default: internal if all // subjects do not qualify for public certificates; othewise acme and @@ -147,12 +147,21 @@ type AutomationPolicy struct { Issuers []certmagic.Issuer `json:"-"` Managers []certmagic.Manager `json:"-"` - magic *certmagic.Config - storage certmagic.Storage + subjects []string + magic *certmagic.Config + storage certmagic.Storage } // Provision sets up ap and builds its underlying CertMagic config. func (ap *AutomationPolicy) Provision(tlsApp *TLS) error { + // replace placeholders in subjects to allow environment variables + repl := caddy.NewReplacer() + subjects := make([]string, len(ap.SubjectsRaw)) + for i, sub := range ap.SubjectsRaw { + subjects[i] = repl.ReplaceAll(sub, "") + } + ap.subjects = subjects + // policy-specific storage implementation if ap.StorageRaw != nil { val, err := tlsApp.ctx.LoadModule(ap, "StorageRaw") @@ -289,6 +298,11 @@ func (ap *AutomationPolicy) Provision(tlsApp *TLS) error { return nil } +// Subjects returns the list of subjects with all placeholders replaced. +func (ap *AutomationPolicy) Subjects() []string { + return ap.subjects +} + func (ap *AutomationPolicy) onlyInternalIssuer() bool { if len(ap.Issuers) != 1 { return false @@ -301,10 +315,10 @@ func (ap *AutomationPolicy) onlyInternalIssuer() bool { // or is the "default" policy (i.e. no subjects) which is unbounded. func (ap *AutomationPolicy) isWildcardOrDefault() bool { isWildcardOrDefault := false - if len(ap.Subjects) == 0 { + if len(ap.subjects) == 0 { isWildcardOrDefault = true } - for _, sub := range ap.Subjects { + for _, sub := range ap.subjects { if strings.HasPrefix(sub, "*") { isWildcardOrDefault = true break diff --git a/modules/caddytls/tls.go b/modules/caddytls/tls.go index 486a58c..9b5b552 100644 --- a/modules/caddytls/tls.go +++ b/modules/caddytls/tls.go @@ -126,7 +126,12 @@ func (t *TLS) Provision(ctx caddy.Context) error { // special case; these will be loaded in later using our automation facilities, // which we want to avoid doing during provisioning if automateNames, ok := modIface.(*AutomateLoader); ok && automateNames != nil { - t.automateNames = []string(*automateNames) + repl := caddy.NewReplacer() + subjects := make([]string, len(*automateNames)) + for i, sub := range *automateNames { + subjects[i] = repl.ReplaceAll(sub, "") + } + t.automateNames = subjects } else { return fmt.Errorf("loading certificates with 'automate' requires array of strings, got: %T", modIface) } @@ -231,13 +236,13 @@ func (t *TLS) Validate() error { var hasDefault bool hostSet := make(map[string]int) for i, ap := range t.Automation.Policies { - if len(ap.Subjects) == 0 { + if len(ap.subjects) == 0 { if hasDefault { return fmt.Errorf("automation policy %d is the second policy that acts as default/catch-all, but will never be used", i) } hasDefault = true } - for _, h := range ap.Subjects { + for _, h := range ap.subjects { if first, ok := hostSet[h]; ok { return fmt.Errorf("automation policy %d: cannot apply more than one automation policy to host: %s (first match in policy %d)", i, h, first) } @@ -388,8 +393,8 @@ func (t *TLS) AddAutomationPolicy(ap *AutomationPolicy) error { // first see if existing is superset of ap for all names var otherIsSuperset bool outer: - for _, thisSubj := range ap.Subjects { - for _, otherSubj := range existing.Subjects { + for _, thisSubj := range ap.subjects { + for _, otherSubj := range existing.subjects { if certmagic.MatchWildcard(thisSubj, otherSubj) { otherIsSuperset = true break outer @@ -398,7 +403,7 @@ func (t *TLS) AddAutomationPolicy(ap *AutomationPolicy) error { } // if existing AP is a superset or if it contains fewer names (i.e. is // more general), then new AP is more specific, so insert before it - if otherIsSuperset || len(existing.Subjects) < len(ap.Subjects) { + if otherIsSuperset || len(existing.SubjectsRaw) < len(ap.SubjectsRaw) { t.Automation.Policies = append(t.Automation.Policies[:i], append([]*AutomationPolicy{ap}, t.Automation.Policies[i:]...)...) return nil @@ -420,10 +425,10 @@ func (t *TLS) getConfigForName(name string) *certmagic.Config { // public certificate or not. func (t *TLS) getAutomationPolicyForName(name string) *AutomationPolicy { for _, ap := range t.Automation.Policies { - if len(ap.Subjects) == 0 { + if len(ap.subjects) == 0 { return ap // no host filter is an automatic match } - for _, h := range ap.Subjects { + for _, h := range ap.subjects { if certmagic.MatchWildcard(name, h) { return ap } -- cgit v1.2.3