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.
This commit is contained in:
Francis Lavoie 2023-03-27 17:16:22 -04:00 committed by GitHub
parent dd86171d67
commit e16a886814
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 58 additions and 39 deletions

View file

@ -206,8 +206,8 @@ func (st ServerType) buildTLSApp(
} }
// associate our new automation policy with this server block's hosts // associate our new automation policy with this server block's hosts
ap.Subjects = sblock.hostsFromKeysNotHTTP(httpPort) ap.SubjectsRaw = sblock.hostsFromKeysNotHTTP(httpPort)
sort.Strings(ap.Subjects) // solely for deterministic test results sort.Strings(ap.SubjectsRaw) // solely for deterministic test results
// if a combination of public and internal names were given // if a combination of public and internal names were given
// for this same server block and no issuer was specified, we // for this same server block and no issuer was specified, we
@ -217,7 +217,7 @@ func (st ServerType) buildTLSApp(
var ap2 *caddytls.AutomationPolicy var ap2 *caddytls.AutomationPolicy
if len(ap.Issuers) == 0 { if len(ap.Issuers) == 0 {
var internal, external []string var internal, external []string
for _, s := range ap.Subjects { for _, s := range ap.SubjectsRaw {
if !certmagic.SubjectQualifiesForCert(s) { if !certmagic.SubjectQualifiesForCert(s) {
return nil, warnings, fmt.Errorf("subject does not qualify for certificate: '%s'", 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 { if len(external) > 0 && len(internal) > 0 {
ap.Subjects = external ap.SubjectsRaw = external
apCopy := *ap apCopy := *ap
ap2 = &apCopy ap2 = &apCopy
ap2.Subjects = internal ap2.SubjectsRaw = internal
ap2.IssuersRaw = []json.RawMessage{caddyconfig.JSONModuleObject(caddytls.InternalIssuer{}, "module", "internal", &warnings)} ap2.IssuersRaw = []json.RawMessage{caddyconfig.JSONModuleObject(caddytls.InternalIssuer{}, "module", "internal", &warnings)}
} }
} }
@ -339,14 +339,14 @@ func (st ServerType) buildTLSApp(
for h := range httpsHostsSharedWithHostlessKey { for h := range httpsHostsSharedWithHostlessKey {
al = append(al, h) al = append(al, h)
if !certmagic.SubjectQualifiesForPublicCert(h) { if !certmagic.SubjectQualifiesForPublicCert(h) {
internalAP.Subjects = append(internalAP.Subjects, h) internalAP.SubjectsRaw = append(internalAP.SubjectsRaw, h)
} }
} }
} }
if len(al) > 0 { if len(al) > 0 {
tlsApp.CertificatesRaw["automate"] = caddyconfig.JSON(al, &warnings) tlsApp.CertificatesRaw["automate"] = caddyconfig.JSON(al, &warnings)
} }
if len(internalAP.Subjects) > 0 { if len(internalAP.SubjectsRaw) > 0 {
if tlsApp.Automation == nil { if tlsApp.Automation == nil {
tlsApp.Automation = new(caddytls.AutomationConfig) tlsApp.Automation = new(caddytls.AutomationConfig)
} }
@ -412,7 +412,7 @@ func (st ServerType) buildTLSApp(
// for convenience) // for convenience)
automationHostSet := make(map[string]struct{}) automationHostSet := make(map[string]struct{})
for _, ap := range tlsApp.Automation.Policies { for _, ap := range tlsApp.Automation.Policies {
for _, s := range ap.Subjects { for _, s := range ap.SubjectsRaw {
if _, ok := automationHostSet[s]; ok { if _, ok := automationHostSet[s]; ok {
return nil, warnings, fmt.Errorf("hostname appears in more than one automation policy, making certificate management ambiguous: %s", s) 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]) { if automationPolicyIsSubset(aps[j], aps[i]) {
return false return false
} }
return len(aps[i].Subjects) > len(aps[j].Subjects) return len(aps[i].SubjectsRaw) > len(aps[j].SubjectsRaw)
}) })
emptyAPCount := 0 emptyAPCount := 0
@ -541,7 +541,7 @@ func consolidateAutomationPolicies(aps []*caddytls.AutomationPolicy) []*caddytls
// compute the number of empty policies (disregarding subjects) - see #4128 // compute the number of empty policies (disregarding subjects) - see #4128
emptyAP := new(caddytls.AutomationPolicy) emptyAP := new(caddytls.AutomationPolicy)
for i := 0; i < len(aps); i++ { for i := 0; i < len(aps); i++ {
emptyAP.Subjects = aps[i].Subjects emptyAP.SubjectsRaw = aps[i].SubjectsRaw
if reflect.DeepEqual(aps[i], emptyAP) { if reflect.DeepEqual(aps[i], emptyAP) {
emptyAPCount++ emptyAPCount++
if !automationPolicyHasAllPublicNames(aps[i]) { if !automationPolicyHasAllPublicNames(aps[i]) {
@ -583,7 +583,7 @@ outer:
aps[i].KeyType == aps[j].KeyType && aps[i].KeyType == aps[j].KeyType &&
aps[i].OnDemand == aps[j].OnDemand && aps[i].OnDemand == aps[j].OnDemand &&
aps[i].RenewalWindowRatio == aps[j].RenewalWindowRatio { 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 // later policy (at j) has no subjects ("catch-all"), so we can
// remove the identical-but-more-specific policy that comes first // remove the identical-but-more-specific policy that comes first
// AS LONG AS it is not shadowed by another policy before it; e.g. // AS LONG AS it is not shadowed by another policy before it; e.g.
@ -598,9 +598,9 @@ outer:
} }
} else { } else {
// avoid repeated subjects // avoid repeated subjects
for _, subj := range aps[j].Subjects { for _, subj := range aps[j].SubjectsRaw {
if !sliceContains(aps[i].Subjects, subj) { if !sliceContains(aps[i].SubjectsRaw, subj) {
aps[i].Subjects = append(aps[i].Subjects, subj) aps[i].SubjectsRaw = append(aps[i].SubjectsRaw, subj)
} }
} }
aps = append(aps[:j], aps[j+1:]...) aps = append(aps[:j], aps[j+1:]...)
@ -616,15 +616,15 @@ outer:
// automationPolicyIsSubset returns true if a's subjects are a subset // automationPolicyIsSubset returns true if a's subjects are a subset
// of b's subjects. // of b's subjects.
func automationPolicyIsSubset(a, b *caddytls.AutomationPolicy) bool { func automationPolicyIsSubset(a, b *caddytls.AutomationPolicy) bool {
if len(b.Subjects) == 0 { if len(b.SubjectsRaw) == 0 {
return true return true
} }
if len(a.Subjects) == 0 { if len(a.SubjectsRaw) == 0 {
return false return false
} }
for _, aSubj := range a.Subjects { for _, aSubj := range a.SubjectsRaw {
var inSuperset bool var inSuperset bool
for _, bSubj := range b.Subjects { for _, bSubj := range b.SubjectsRaw {
if certmagic.MatchWildcard(aSubj, bSubj) { if certmagic.MatchWildcard(aSubj, bSubj) {
inSuperset = true inSuperset = true
break break
@ -662,7 +662,7 @@ func subjectQualifiesForPublicCert(ap *caddytls.AutomationPolicy, subj string) b
} }
func automationPolicyHasAllPublicNames(ap *caddytls.AutomationPolicy) bool { func automationPolicyHasAllPublicNames(ap *caddytls.AutomationPolicy) bool {
for _, subj := range ap.Subjects { for _, subj := range ap.SubjectsRaw {
if !subjectQualifiesForPublicCert(ap, subj) { if !subjectQualifiesForPublicCert(ap, subj) {
return false return false
} }

View file

@ -47,8 +47,8 @@ func TestAutomationPolicyIsSubset(t *testing.T) {
expect: false, expect: false,
}, },
} { } {
apA := &caddytls.AutomationPolicy{Subjects: test.a} apA := &caddytls.AutomationPolicy{SubjectsRaw: test.a}
apB := &caddytls.AutomationPolicy{Subjects: test.b} apB := &caddytls.AutomationPolicy{SubjectsRaw: test.b}
if actual := automationPolicyIsSubset(apA, apB); actual != test.expect { 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) t.Errorf("Test %d: Expected %t but got %t (A: %v B: %v)", i, test.expect, actual, test.a, test.b)
} }

View file

@ -285,7 +285,7 @@ uniqueDomainsLoop:
// one automation policy would be confusing and an error // one automation policy would be confusing and an error
if app.tlsApp.Automation != nil { if app.tlsApp.Automation != nil {
for _, ap := range app.tlsApp.Automation.Policies { for _, ap := range app.tlsApp.Automation.Policies {
for _, apHost := range ap.Subjects { for _, apHost := range ap.Subjects() {
if apHost == d { if apHost == d {
continue uniqueDomainsLoop 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? // 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 basePolicy = ap
foundBasePolicy = true 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 // rather they just want to change the CA for the set
// of names that would normally use the production API; // of names that would normally use the production API;
// anyway, that gets into the weeds a bit... // anyway, that gets into the weeds a bit...
newPolicy.Subjects = internalNames newPolicy.SubjectsRaw = internalNames
newPolicy.Issuers = []certmagic.Issuer{internalIssuer} newPolicy.Issuers = []certmagic.Issuer{internalIssuer}
err := app.tlsApp.AddAutomationPolicy(newPolicy) err := app.tlsApp.AddAutomationPolicy(newPolicy)
if err != nil { if err != nil {

View file

@ -259,7 +259,7 @@ func cmdReverseProxy(fs caddycmd.Flags) (int, error) {
tlsApp := caddytls.TLS{ tlsApp := caddytls.TLS{
Automation: &caddytls.AutomationConfig{ Automation: &caddytls.AutomationConfig{
Policies: []*caddytls.AutomationPolicy{{ Policies: []*caddytls.AutomationPolicy{{
Subjects: []string{fromAddr.Host}, SubjectsRaw: []string{fromAddr.Host},
IssuersRaw: []json.RawMessage{json.RawMessage(`{"module":"internal"}`)}, IssuersRaw: []json.RawMessage{json.RawMessage(`{"module":"internal"}`)},
}}, }},
}, },

View file

@ -85,7 +85,7 @@ type AutomationConfig struct {
// TLS app to properly provision a new policy. // TLS app to properly provision a new policy.
type AutomationPolicy struct { type AutomationPolicy struct {
// Which subjects (hostnames or IP addresses) this policy applies to. // 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 // The modules that may issue certificates. Default: internal if all
// subjects do not qualify for public certificates; othewise acme and // subjects do not qualify for public certificates; othewise acme and
@ -147,12 +147,21 @@ type AutomationPolicy struct {
Issuers []certmagic.Issuer `json:"-"` Issuers []certmagic.Issuer `json:"-"`
Managers []certmagic.Manager `json:"-"` Managers []certmagic.Manager `json:"-"`
subjects []string
magic *certmagic.Config magic *certmagic.Config
storage certmagic.Storage storage certmagic.Storage
} }
// Provision sets up ap and builds its underlying CertMagic config. // Provision sets up ap and builds its underlying CertMagic config.
func (ap *AutomationPolicy) Provision(tlsApp *TLS) error { 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 // policy-specific storage implementation
if ap.StorageRaw != nil { if ap.StorageRaw != nil {
val, err := tlsApp.ctx.LoadModule(ap, "StorageRaw") val, err := tlsApp.ctx.LoadModule(ap, "StorageRaw")
@ -289,6 +298,11 @@ func (ap *AutomationPolicy) Provision(tlsApp *TLS) error {
return nil 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 { func (ap *AutomationPolicy) onlyInternalIssuer() bool {
if len(ap.Issuers) != 1 { if len(ap.Issuers) != 1 {
return false return false
@ -301,10 +315,10 @@ func (ap *AutomationPolicy) onlyInternalIssuer() bool {
// or is the "default" policy (i.e. no subjects) which is unbounded. // or is the "default" policy (i.e. no subjects) which is unbounded.
func (ap *AutomationPolicy) isWildcardOrDefault() bool { func (ap *AutomationPolicy) isWildcardOrDefault() bool {
isWildcardOrDefault := false isWildcardOrDefault := false
if len(ap.Subjects) == 0 { if len(ap.subjects) == 0 {
isWildcardOrDefault = true isWildcardOrDefault = true
} }
for _, sub := range ap.Subjects { for _, sub := range ap.subjects {
if strings.HasPrefix(sub, "*") { if strings.HasPrefix(sub, "*") {
isWildcardOrDefault = true isWildcardOrDefault = true
break break

View file

@ -126,7 +126,12 @@ func (t *TLS) Provision(ctx caddy.Context) error {
// special case; these will be loaded in later using our automation facilities, // special case; these will be loaded in later using our automation facilities,
// which we want to avoid doing during provisioning // which we want to avoid doing during provisioning
if automateNames, ok := modIface.(*AutomateLoader); ok && automateNames != nil { 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 { } else {
return fmt.Errorf("loading certificates with 'automate' requires array of strings, got: %T", modIface) 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 var hasDefault bool
hostSet := make(map[string]int) hostSet := make(map[string]int)
for i, ap := range t.Automation.Policies { for i, ap := range t.Automation.Policies {
if len(ap.Subjects) == 0 { if len(ap.subjects) == 0 {
if hasDefault { if hasDefault {
return fmt.Errorf("automation policy %d is the second policy that acts as default/catch-all, but will never be used", i) return fmt.Errorf("automation policy %d is the second policy that acts as default/catch-all, but will never be used", i)
} }
hasDefault = true hasDefault = true
} }
for _, h := range ap.Subjects { for _, h := range ap.subjects {
if first, ok := hostSet[h]; ok { 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) 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 // first see if existing is superset of ap for all names
var otherIsSuperset bool var otherIsSuperset bool
outer: outer:
for _, thisSubj := range ap.Subjects { for _, thisSubj := range ap.subjects {
for _, otherSubj := range existing.Subjects { for _, otherSubj := range existing.subjects {
if certmagic.MatchWildcard(thisSubj, otherSubj) { if certmagic.MatchWildcard(thisSubj, otherSubj) {
otherIsSuperset = true otherIsSuperset = true
break outer 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 // 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 // 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], t.Automation.Policies = append(t.Automation.Policies[:i],
append([]*AutomationPolicy{ap}, t.Automation.Policies[i:]...)...) append([]*AutomationPolicy{ap}, t.Automation.Policies[i:]...)...)
return nil return nil
@ -420,10 +425,10 @@ func (t *TLS) getConfigForName(name string) *certmagic.Config {
// public certificate or not. // public certificate or not.
func (t *TLS) getAutomationPolicyForName(name string) *AutomationPolicy { func (t *TLS) getAutomationPolicyForName(name string) *AutomationPolicy {
for _, ap := range t.Automation.Policies { 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 return ap // no host filter is an automatic match
} }
for _, h := range ap.Subjects { for _, h := range ap.subjects {
if certmagic.MatchWildcard(name, h) { if certmagic.MatchWildcard(name, h) {
return ap return ap
} }