From 269b1e9aa34b2b02911f8746e7b6a162cd8222cf Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Thu, 20 Jun 2019 20:36:29 -0600 Subject: tls: Improve (and fix) on-demand configuration --- go.mod | 3 +- go.sum | 4 +++ modules/caddytls/acmemanager.go | 66 +++++++++++++++++++++++++++++++++++++---- modules/caddytls/connpolicy.go | 3 ++ modules/caddytls/tls.go | 39 +++++++++++++++++++++--- 5 files changed, 104 insertions(+), 11 deletions(-) diff --git a/go.mod b/go.mod index d401471..edf5e6c 100644 --- a/go.mod +++ b/go.mod @@ -16,12 +16,13 @@ require ( github.com/imdario/mergo v0.3.7 // indirect github.com/klauspost/compress v1.7.1-0.20190613161414-0b31f265a57b github.com/klauspost/cpuid v1.2.1 - github.com/mholt/certmagic v0.6.0 + github.com/mholt/certmagic v0.6.2-0.20190621004807-be4f86a2eb60 github.com/rs/cors v1.6.0 github.com/shurcooL/sanitized_anchor_name v1.0.0 // indirect github.com/starlight-go/starlight v0.0.0-20181207205707-b06f321544f3 go.starlark.net v0.0.0-20190604130855-6ddc71c0ba77 golang.org/x/net v0.0.0-20190603091049-60506f45cf65 + golang.org/x/time v0.0.0-20190308202827-9d24e82272b4 // indirect gopkg.in/russross/blackfriday.v2 v2.0.1 ) diff --git a/go.sum b/go.sum index 83f9419..d540fef 100644 --- a/go.sum +++ b/go.sum @@ -36,6 +36,8 @@ github.com/mholt/certmagic v0.5.2-0.20190605043235-e49d0d405641 h1:wNqOQ0DFxcZDN github.com/mholt/certmagic v0.5.2-0.20190605043235-e49d0d405641/go.mod h1:g4cOPxcjV0oFq3qwpjSA30LReKD8AoIfwAY9VvG35NY= github.com/mholt/certmagic v0.6.0 h1:gZwuBuONw2v8/fZh2nd39kvjFNjnSF2uIR4GzKaaryw= github.com/mholt/certmagic v0.6.0/go.mod h1:g4cOPxcjV0oFq3qwpjSA30LReKD8AoIfwAY9VvG35NY= +github.com/mholt/certmagic v0.6.2-0.20190621004807-be4f86a2eb60 h1:SALetD3LrWGNvna2JIwY1dG4W6rBKtoBehQtHjEKTpo= +github.com/mholt/certmagic v0.6.2-0.20190621004807-be4f86a2eb60/go.mod h1:g4cOPxcjV0oFq3qwpjSA30LReKD8AoIfwAY9VvG35NY= github.com/miekg/dns v1.1.3 h1:1g0r1IvskvgL8rR+AcHzUA+oFmGcQlaIm4IqakufeMM= github.com/miekg/dns v1.1.3/go.mod h1:W1PPwlIAgtquWBMBEV9nkV9Cazfe8ScdGz/Lj7v3Nrg= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= @@ -66,5 +68,7 @@ golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a h1:1BGLXjeY4akVXGgbC9HugT3Jv golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/text v0.3.0 h1:g61tztE5qeGQ89tm6NTjjM9VPIm088od1l6aSorWRWg= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= +golang.org/x/time v0.0.0-20190308202827-9d24e82272b4 h1:SvFZT6jyqRaOeXpc5h/JSfZenJ2O330aBsf7JfSUXmQ= +golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= gopkg.in/square/go-jose.v2 v2.2.2 h1:orlkJ3myw8CN1nVQHBFfloD+L3egixIa4FvUP6RosSA= gopkg.in/square/go-jose.v2 v2.2.2/go.mod h1:M9dMgbHiYLoDGQrXy7OpJDJWiKiU//h+vD76mk0e1AI= diff --git a/modules/caddytls/acmemanager.go b/modules/caddytls/acmemanager.go index 50f8648..871b2bf 100644 --- a/modules/caddytls/acmemanager.go +++ b/modules/caddytls/acmemanager.go @@ -3,6 +3,7 @@ package caddytls import ( "encoding/json" "fmt" + "net/url" "time" "github.com/go-acme/lego/certcrypto" @@ -30,12 +31,12 @@ func init() { type ACMEManagerMaker struct { CA string `json:"ca,omitempty"` Email string `json:"email,omitempty"` - RenewAhead caddy.Duration `json:"renew_ahead,omitempty"` + RenewAhead caddy.Duration `json:"renew_ahead,omitempty"` KeyType string `json:"key_type,omitempty"` - ACMETimeout caddy.Duration `json:"acme_timeout,omitempty"` + ACMETimeout caddy.Duration `json:"acme_timeout,omitempty"` MustStaple bool `json:"must_staple,omitempty"` - Challenges ChallengesConfig `json:"challenges"` - OnDemand *OnDemandConfig `json:"on_demand,omitempty"` + Challenges ChallengesConfig `json:"challenges,omitempty"` + OnDemand bool `json:"on_demand,omitempty"` Storage json.RawMessage `json:"storage,omitempty"` storage certmagic.Storage @@ -109,9 +110,34 @@ func (m *ACMEManagerMaker) makeCertMagicConfig(ctx caddy.Context) certmagic.Conf } var ond *certmagic.OnDemandConfig - if m.OnDemand != nil { + if m.OnDemand { + var onDemand *OnDemandConfig + appVal, err := ctx.App("tls") + if err == nil { + onDemand = appVal.(*TLS).Automation.OnDemand + } + ond = &certmagic.OnDemandConfig{ - // TODO: fill this out + 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, since + // even checking consumes a token; so + // don't even bother checking if the + // other regulations fail anyway + if onDemand.RateLimit != nil { + if !onDemandRateLimiter.Allow() { + return fmt.Errorf("on-demand rate limit exceeded") + } + } + } + return nil + }, } } @@ -134,6 +160,34 @@ func (m *ACMEManagerMaker) makeCertMagicConfig(ctx caddy.Context) certmagic.Conf } } +// onDemandAskRequest makes a request to the ask URL +// to see if a certificate can be obtained for name. +// The certificate request should be denied if this +// returns an error. +func onDemandAskRequest(ask string, name string) error { + askURL, err := url.Parse(ask) + if err != nil { + return fmt.Errorf("parsing ask URL: %v", err) + } + qs := askURL.Query() + qs.Set("domain", name) + askURL.RawQuery = qs.Encode() + + resp, err := onDemandAskClient.Get(askURL.String()) + if err != nil { + return fmt.Errorf("error checking %v to deterine if certificate for hostname '%s' should be allowed: %v", + ask, name, err) + } + resp.Body.Close() + + if resp.StatusCode < 200 || resp.StatusCode > 299 { + return fmt.Errorf("certificate for hostname '%s' not allowed; non-2xx status code %d returned from %v", + name, resp.StatusCode, ask) + } + + return nil +} + // supportedCertKeyTypes is all the key types that are supported // for certificates that are obtained through ACME. var supportedCertKeyTypes = map[string]certcrypto.KeyType{ diff --git a/modules/caddytls/connpolicy.go b/modules/caddytls/connpolicy.go index 54cad7c..a278326 100644 --- a/modules/caddytls/connpolicy.go +++ b/modules/caddytls/connpolicy.go @@ -86,6 +86,7 @@ func (cp ConnectionPolicies) TLSConfig(ctx caddy.Context) (*tls.Config, error) { } return pol.stdTLSConfig, nil } + return nil, fmt.Errorf("no server TLS configuration available for ClientHello: %+v", hello) }, }, nil @@ -148,6 +149,8 @@ func (p *ConnectionPolicy) buildStandardTLSConfig(ctx caddy.Context) error { tlsApp.SessionTickets.unregister(cfg) }) + // TODO: Clean up active locks if app (or process) is being closed! + // add all the cipher suites in order, without duplicates cipherSuitesAdded := make(map[uint16]struct{}) for _, csName := range p.CipherSuites { diff --git a/modules/caddytls/tls.go b/modules/caddytls/tls.go index 0614d24..c84118a 100644 --- a/modules/caddytls/tls.go +++ b/modules/caddytls/tls.go @@ -5,10 +5,12 @@ import ( "encoding/json" "fmt" "net/http" + "time" "github.com/caddyserver/caddy" "github.com/go-acme/lego/challenge" "github.com/mholt/certmagic" + "golang.org/x/time/rate" ) func init() { @@ -71,6 +73,16 @@ func (t *TLS) Provision(ctx caddy.Context) error { return fmt.Errorf("provisioning session tickets configuration: %v", err) } + // on-demand rate limiting + if t.Automation.OnDemand != nil && t.Automation.OnDemand.RateLimit != nil { + limit := rate.Every(time.Duration(t.Automation.OnDemand.RateLimit.Interval)) + // TODO: Burst size is not updated, see https://github.com/golang/go/issues/23575 + onDemandRateLimiter.SetLimit(limit) + } else { + // if no rate limit is specified, be sure to remove any existing limit + onDemandRateLimiter.SetLimit(0) + } + return nil } @@ -178,6 +190,7 @@ type CertificateLoader interface { // construction and use of ACME clients. type AutomationConfig struct { Policies []AutomationPolicy `json:"policies,omitempty"` + OnDemand *OnDemandConfig `json:"on_demand,omitempty"` } // AutomationPolicy designates the policy for automating the @@ -189,6 +202,9 @@ type AutomationPolicy struct { Management managerMaker `json:"-"` } +// makeCertMagicConfig converts ap into a CertMagic config. Passing onDemand +// is necessary because the automation policy does not have convenient access +// to the TLS app's global on-demand policies; func (ap AutomationPolicy) makeCertMagicConfig(ctx caddy.Context) certmagic.Config { // default manager (ACME) is a special case because of how CertMagic is designed // TODO: refactor certmagic so that ACME manager is not a special case by extracting @@ -226,10 +242,14 @@ type TLSALPNChallengeConfig struct { // OnDemandConfig configures on-demand TLS, for obtaining // needed certificates at handshake-time. type OnDemandConfig struct { - // TODO: MaxCertificates state might not endure reloads... - // MaxCertificates int `json:"max_certificates,omitempty"` - AskURL string `json:"ask_url,omitempty"` - AskStarlark string `json:"ask_starlark,omitempty"` + RateLimit *RateLimit `json:"rate_limit,omitempty"` + Ask string `json:"ask,omitempty"` +} + +// RateLimit specifies an interval with optional burst size. +type RateLimit struct { + Interval caddy.Duration `json:"interval,omitempty"` + Burst int `json:"burst,omitempty"` } // managerMaker makes a certificate manager. @@ -237,4 +257,15 @@ type managerMaker interface { newManager(interactive bool) (certmagic.Manager, error) } +// These perpetual values are used for on-demand TLS. +var ( + onDemandRateLimiter = rate.NewLimiter(0, 1) + onDemandAskClient = &http.Client{ + Timeout: 10 * time.Second, + CheckRedirect: func(req *http.Request, via []*http.Request) error { + return fmt.Errorf("following http redirects is not allowed") + }, + } +) + const automateKey = "automate" -- cgit v1.2.3