From fc7340e11aa9ca6326909aedfd36bb2c5b53d2a8 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Tue, 17 Mar 2020 21:00:45 -0600 Subject: httpcaddyfile: Many tls-related improvements including on-demand support Holy heck this was complicated --- modules/caddytls/automation.go | 51 +++++++++++++++++++++++++++++------------- modules/caddytls/connpolicy.go | 2 +- modules/caddytls/tls.go | 12 ++++++++-- 3 files changed, 46 insertions(+), 19 deletions(-) (limited to 'modules/caddytls') diff --git a/modules/caddytls/automation.go b/modules/caddytls/automation.go index d10a4c6..e91811d 100644 --- a/modules/caddytls/automation.go +++ b/modules/caddytls/automation.go @@ -23,6 +23,7 @@ import ( "github.com/caddyserver/caddy/v2" "github.com/caddyserver/certmagic" "github.com/go-acme/lego/v3/challenge" + "go.uber.org/zap" ) // AutomationConfig designates configuration for the @@ -131,31 +132,49 @@ func (ap *AutomationPolicy) provision(tlsApp *TLS) error { var ond *certmagic.OnDemandConfig if ap.OnDemand { - var onDemand *OnDemandConfig - if tlsApp.Automation != nil { - onDemand = tlsApp.Automation.OnDemand - } - ond = &certmagic.OnDemandConfig{ DecisionFunc: func(name string) error { - if onDemand != nil { - if onDemand.Ask != "" { - err := onDemandAskRequest(onDemand.Ask, name) - if err != nil { - return err - } - } - // check the rate limiter last because - // doing so makes a reservation - if !onDemandRateLimiter.Allow() { - return fmt.Errorf("on-demand rate limit exceeded") + // if an "ask" endpoint was defined, consult it first + if tlsApp.Automation != nil && + tlsApp.Automation.OnDemand != nil && + tlsApp.Automation.OnDemand.Ask != "" { + err := onDemandAskRequest(tlsApp.Automation.OnDemand.Ask, name) + if err != nil { + return err } } + // check the rate limiter last because + // doing so makes a reservation + if !onDemandRateLimiter.Allow() { + return fmt.Errorf("on-demand rate limit exceeded") + } return nil }, } } + // if this automation policy has no Issuer defined, and + // none the subjects do not qualify for a public certificate, + // set the issuer to internal so that these names can all + // get certificates; critically, we can only do this if an + // issuer is not explictly configured AND if the list of + // subjects is non-empty + if ap.IssuerRaw == nil && len(ap.Subjects) > 0 { + var anyPublic bool + for _, s := range ap.Subjects { + if certmagic.SubjectQualifiesForPublicCert(s) { + anyPublic = true + break + } + } + if !anyPublic { + tlsApp.logger.Info("setting internal issuer for automation policy that has only internal subjects but no issuer configured", + zap.Strings("subjects", ap.Subjects)) + ap.IssuerRaw = json.RawMessage(`{"module":"internal"}`) + } + } + + // load and provision the issuer module if ap.IssuerRaw != nil { val, err := tlsApp.ctx.LoadModule(ap, "IssuerRaw") if err != nil { diff --git a/modules/caddytls/connpolicy.go b/modules/caddytls/connpolicy.go index 7618db4..395c55a 100644 --- a/modules/caddytls/connpolicy.go +++ b/modules/caddytls/connpolicy.go @@ -173,7 +173,7 @@ func (p *ConnectionPolicy) buildStandardTLSConfig(ctx caddy.Context) error { // TODO: I don't love how this works: we pre-build certmagic configs // so that handshakes are faster. Unfortunately, certmagic configs are // comprised of settings from both a TLS connection policy and a TLS - // automation policy. The only two fields (as of March 2020; v2 beta 16) + // automation policy. The only two fields (as of March 2020; v2 beta 17) // of a certmagic config that come from the TLS connection policy are // CertSelection and DefaultServerName, so an automation policy is what // builds the base certmagic config. Since the pre-built config is diff --git a/modules/caddytls/tls.go b/modules/caddytls/tls.go index c927ce2..4fc0850 100644 --- a/modules/caddytls/tls.go +++ b/modules/caddytls/tls.go @@ -179,9 +179,17 @@ func (t *TLS) Validate() error { if t.Automation != nil { // ensure that host aren't repeated; since only the first // automation policy is used, repeating a host in the lists - // isn't useful and is probably a mistake + // isn't useful and is probably a mistake; same for two + // catch-all/default policies + var hasDefault bool hostSet := make(map[string]int) for i, ap := range t.Automation.Policies { + 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 { 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) @@ -301,7 +309,7 @@ func (t *TLS) AddAutomationPolicy(ap *AutomationPolicy) error { // fewer names) exists, prioritize this new policy if len(other.Subjects) < len(ap.Subjects) { t.Automation.Policies = append(t.Automation.Policies[:i], - append([]*AutomationPolicy{ap}, t.Automation.Policies[i+1:]...)...) + append([]*AutomationPolicy{ap}, t.Automation.Policies[i:]...)...) return nil } } -- cgit v1.2.3