diff options
| -rw-r--r-- | admin.go | 2 | ||||
| -rw-r--r-- | caddy.go | 5 | ||||
| -rw-r--r-- | context.go | 26 | ||||
| -rw-r--r-- | go.mod | 3 | ||||
| -rw-r--r-- | go.sum | 11 | ||||
| -rw-r--r-- | modules/caddyhttp/autohttps.go | 3 | ||||
| -rw-r--r-- | modules/caddyhttp/reverseproxy/httptransport.go | 2 | ||||
| -rw-r--r-- | modules/caddypki/adminapi.go | 6 | ||||
| -rw-r--r-- | modules/caddytls/automation.go | 4 | ||||
| -rw-r--r-- | modules/caddytls/tls.go | 91 |
10 files changed, 115 insertions, 38 deletions
@@ -1018,7 +1018,7 @@ func handleConfigID(w http.ResponseWriter, r *http.Request) error { // map the ID to the expanded path currentCtxMu.RLock() expanded, ok := rawCfgIndex[id] - defer currentCtxMu.RUnlock() + currentCtxMu.RUnlock() if !ok { return APIError{ HTTPStatus: http.StatusNotFound, @@ -959,8 +959,9 @@ func Version() (simple, full string) { // This function is experimental and might be changed // or removed in the future. func ActiveContext() Context { - currentCtxMu.RLock() - defer currentCtxMu.RUnlock() + // TODO: This locking might still be needed; more investigation is required (deadlock during Cleanup for the caddytls.TLS module). + // currentCtxMu.RLock() + // defer currentCtxMu.RUnlock() return currentCtx } diff --git a/context.go b/context.go index 615192c15..004dee600 100644 --- a/context.go +++ b/context.go @@ -410,6 +410,11 @@ func (ctx Context) loadModuleInline(moduleNameKey, moduleScope string, raw json. // called during the Provision/Validate phase to reference a // module's own host app (since the parent app module is still // in the process of being provisioned, it is not yet ready). +// +// We return any type instead of the App type because it is NOT +// intended for the caller of this method to be the one to start +// or stop App modules. The caller is expected to assert to the +// concrete type. func (ctx Context) App(name string) (any, error) { if app, ok := ctx.cfg.apps[name]; ok { return app, nil @@ -428,18 +433,15 @@ func (ctx Context) App(name string) (any, error) { // AppIfConfigured returns an app by its name if it has been // configured. Can be called instead of App() to avoid -// instantiating an empty app when that's not desirable. -func (ctx Context) AppIfConfigured(name string) (any, error) { - app, ok := ctx.cfg.apps[name] - if !ok || app == nil { - return nil, nil - } - - appModule, err := ctx.App(name) - if err != nil { - return nil, err - } - return appModule, nil +// instantiating an empty app when that's not desirable. If +// the app has not been loaded, nil is returned. +// +// We return any type instead of the App type because it is not +// intended for the caller of this method to be the one to start +// or stop App modules. The caller is expected to assert to the +// concrete type. +func (ctx Context) AppIfConfigured(name string) any { + return ctx.cfg.apps[name] } // Storage returns the configured Caddy storage implementation. @@ -7,7 +7,7 @@ require ( github.com/Masterminds/sprig/v3 v3.2.3 github.com/alecthomas/chroma/v2 v2.7.0 github.com/aryann/difflib v0.0.0-20210328193216-ff5ff6dc229b - github.com/caddyserver/certmagic v0.18.2 + github.com/caddyserver/certmagic v0.19.0 github.com/dustin/go-humanize v1.0.1 github.com/go-chi/chi v4.1.2+incompatible github.com/google/cel-go v0.15.1 @@ -61,6 +61,7 @@ require ( github.com/quic-go/qtls-go1-20 v0.2.2 // indirect github.com/smallstep/go-attestation v0.4.4-0.20230509120429-e17291421738 // indirect github.com/x448/float16 v0.8.4 // indirect + github.com/zeebo/blake3 v0.2.3 // indirect go.opentelemetry.io/contrib/propagators/aws v1.17.0 // indirect go.opentelemetry.io/contrib/propagators/b3 v1.17.0 // indirect go.opentelemetry.io/contrib/propagators/jaeger v1.17.0 // indirect @@ -170,8 +170,8 @@ github.com/bketelsen/crypt v0.0.3-0.20200106085610-5cbc8cc4026c/go.mod h1:MKsuJm github.com/blakesmith/ar v0.0.0-20190502131153-809d4375e1fb/go.mod h1:PkYb9DJNAwrSvRx5DYA+gUcOIgTGVMNkfSCbZM8cWpI= github.com/boltdb/bolt v1.3.1/go.mod h1:clJnj/oiGkjum5o1McbSZDSLxVThjynRyGBgiAx27Ps= github.com/caarlos0/ctrlc v1.0.0/go.mod h1:CdXpj4rmq0q/1Eb44M9zi2nKB0QraNKuRGYGrrHhcQw= -github.com/caddyserver/certmagic v0.18.2 h1:Nj2+M+A2Ho9IF6n1wUSbra4mX1X6ALzWpul9HooprHA= -github.com/caddyserver/certmagic v0.18.2/go.mod h1:cLsgYXecH1iVUPjDXw15/1SKjZk/TK+aFfQk5FnugGQ= +github.com/caddyserver/certmagic v0.19.0 h1:HuJ1Yf1H1jAfmBGrSSQN1XRkafnWcpDtyIiyMV6vmpM= +github.com/caddyserver/certmagic v0.19.0/go.mod h1:fsL01NomQ6N+kE2j37ZCnig2MFosG+MIO4ztnmG/zz8= github.com/campoy/unique v0.0.0-20180121183637-88950e537e7e/go.mod h1:9IOqJGCPMSc6E5ydlp5NIonxObaeu/Iub/X03EKPVYo= github.com/casbin/casbin/v2 v2.1.2/go.mod h1:YcPU1XXisHhLzuxH9coDNf2FbKpjGlbCg3n9yuLkIJQ= github.com/cavaliercoder/go-cpio v0.0.0-20180626203310-925f9528c45e/go.mod h1:oDpT4efm8tSYHXV5tHSdRvBet/b/QzxZ+XyyPehvm3A= @@ -650,6 +650,7 @@ github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+o github.com/klauspost/compress v1.12.3/go.mod h1:8dP1Hq4DHOhN9w426knH3Rhby4rFm6D8eO+e+Dq5Gzg= github.com/klauspost/compress v1.16.6 h1:91SKEy4K37vkp255cJ8QesJhjyRO0hn9i9G0GoUwLsk= github.com/klauspost/compress v1.16.6/go.mod h1:ntbaceVETuRiXiv4DpjP66DpAtAGkEQskQzEyD//IeE= +github.com/klauspost/cpuid/v2 v2.0.12/go.mod h1:g2LTdtYhdyuGPqyWyv7qRAmj1WBqxuObKfj5c0PQa7c= github.com/klauspost/cpuid/v2 v2.2.5 h1:0E5MSMDEoAulmXNFquVs//DdoomxaoTY1kUhbc/qbZg= github.com/klauspost/cpuid/v2 v2.2.5/go.mod h1:Lcz8mBdAVJIBVzewtcLocK12l3Y+JytZYpaMropDUws= github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ= @@ -1012,6 +1013,12 @@ github.com/yuin/goldmark v1.5.4 h1:2uY/xC0roWy8IBEGLgB1ywIoEJFGmRrX21YQcvGZzjU= github.com/yuin/goldmark v1.5.4/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= github.com/yuin/goldmark-highlighting/v2 v2.0.0-20220924101305-151362477c87 h1:Py16JEzkSdKAtEFJjiaYLYBOWGXc1r/xHj/Q/5lA37k= github.com/yuin/goldmark-highlighting/v2 v2.0.0-20220924101305-151362477c87/go.mod h1:ovIvrum6DQJA4QsJSovrkC4saKHQVs7TvcaeO8AIl5I= +github.com/zeebo/assert v1.1.0 h1:hU1L1vLTHsnO8x8c9KAR5GmM5QscxHg5RNU5z5qbUWY= +github.com/zeebo/assert v1.1.0/go.mod h1:Pq9JiuJQpG8JLJdtkwrJESF0Foym2/D9XMU5ciN/wJ0= +github.com/zeebo/blake3 v0.2.3 h1:TFoLXsjeXqRNFxSbk35Dk4YtszE/MQQGK10BH4ptoTg= +github.com/zeebo/blake3 v0.2.3/go.mod h1:mjJjZpnsyIVtVgTOSpJ9vmRE4wgDeyt2HU3qXvvKCaQ= +github.com/zeebo/pcg v1.0.1 h1:lyqfGeWiv4ahac6ttHs+I5hwtH/+1mrhlCtVNQM2kHo= +github.com/zeebo/pcg v1.0.1/go.mod h1:09F0S9iiKrwn9rlI5yjLkmrug154/YRW6KnnXVDM/l4= github.com/zenazn/goji v0.9.0/go.mod h1:7S9M489iMyHBNxwZnk9/EHS098H4/F6TATF2mIxtB1Q= go.etcd.io/bbolt v1.3.2/go.mod h1:IbVyRI1SCnLcuJnV2u8VeU0CEYM7e686BmAb1XKL+uU= go.etcd.io/bbolt v1.3.3/go.mod h1:IbVyRI1SCnLcuJnV2u8VeU0CEYM7e686BmAb1XKL+uU= diff --git a/modules/caddyhttp/autohttps.go b/modules/caddyhttp/autohttps.go index 4ade3c5bf..39ec135d9 100644 --- a/modules/caddyhttp/autohttps.go +++ b/modules/caddyhttp/autohttps.go @@ -196,8 +196,7 @@ func (app *App) automaticHTTPSPhase1(ctx caddy.Context, repl *caddy.Replacer) er // if a certificate for this name is already loaded, // don't obtain another one for it, unless we are // supposed to ignore loaded certificates - if !srv.AutoHTTPS.IgnoreLoadedCerts && - len(app.tlsApp.AllMatchingCertificates(d)) > 0 { + if !srv.AutoHTTPS.IgnoreLoadedCerts && app.tlsApp.HasCertificateForSubject(d) { logger.Info("skipping automatic certificate management because one or more matching certificates are already loaded", zap.String("domain", d), zap.String("server_name", srvName), diff --git a/modules/caddyhttp/reverseproxy/httptransport.go b/modules/caddyhttp/reverseproxy/httptransport.go index 1135862a8..8334f25ad 100644 --- a/modules/caddyhttp/reverseproxy/httptransport.go +++ b/modules/caddyhttp/reverseproxy/httptransport.go @@ -525,7 +525,7 @@ func (t TLSConfig) MakeTLSClientConfig(ctx caddy.Context) (*tls.Config, error) { return nil, fmt.Errorf("managing client certificate: %v", err) } cfg.GetClientCertificate = func(cri *tls.CertificateRequestInfo) (*tls.Certificate, error) { - certs := tlsApp.AllMatchingCertificates(t.ClientCertificateAutomate) + certs := caddytls.AllMatchingCertificates(t.ClientCertificateAutomate) var err error for _, cert := range certs { err = cri.SupportsCertificate(&cert.Certificate) diff --git a/modules/caddypki/adminapi.go b/modules/caddypki/adminapi.go index cab7c7030..24371e746 100644 --- a/modules/caddypki/adminapi.go +++ b/modules/caddypki/adminapi.go @@ -50,11 +50,7 @@ func (a *adminAPI) Provision(ctx caddy.Context) error { a.log = ctx.Logger(a) // TODO: passing in 'a' is a hack until the admin API is officially extensible (see #5032) // Avoid initializing PKI if it wasn't configured - pkiApp, err := a.ctx.AppIfConfigured("pki") - if err != nil { - return err - } - if pkiApp != nil { + if pkiApp := a.ctx.AppIfConfigured("pki"); pkiApp != nil { a.pkiApp = pkiApp.(*PKI) } diff --git a/modules/caddytls/automation.go b/modules/caddytls/automation.go index de8820131..114d7aad7 100644 --- a/modules/caddytls/automation.go +++ b/modules/caddytls/automation.go @@ -294,7 +294,9 @@ func (ap *AutomationPolicy) Provision(tlsApp *TLS) error { Issuers: issuers, Logger: tlsApp.logger, } - ap.magic = certmagic.New(tlsApp.certCache, template) + certCacheMu.RLock() + ap.magic = certmagic.New(certCache, template) + certCacheMu.RUnlock() // sometimes issuers may need the parent certmagic.Config in // order to function properly (for example, ACMEIssuer needs diff --git a/modules/caddytls/tls.go b/modules/caddytls/tls.go index 52f1159b1..1456d294d 100644 --- a/modules/caddytls/tls.go +++ b/modules/caddytls/tls.go @@ -36,6 +36,11 @@ func init() { caddy.RegisterModule(AutomateLoader{}) } +var ( + certCache *certmagic.Cache + certCacheMu sync.RWMutex +) + // TLS provides TLS facilities including certificate // loading and management, client auth, and more. type TLS struct { @@ -77,12 +82,15 @@ type TLS struct { certificateLoaders []CertificateLoader automateNames []string - certCache *certmagic.Cache ctx caddy.Context storageCleanTicker *time.Ticker storageCleanStop chan struct{} logger *zap.Logger events *caddyevents.App + + // set of subjects with managed certificates, + // and hashes of manually-loaded certificates + managing, loaded map[string]struct{} } // CaddyModule returns the Caddy module information. @@ -103,6 +111,7 @@ func (t *TLS) Provision(ctx caddy.Context) error { t.ctx = ctx t.logger = ctx.Logger() repl := caddy.NewReplacer() + t.managing, t.loaded = make(map[string]struct{}), make(map[string]struct{}) // set up a new certificate cache; this (re)loads all certificates cacheOpts := certmagic.CacheOptions{ @@ -121,7 +130,14 @@ func (t *TLS) Provision(ctx caddy.Context) error { if cacheOpts.Capacity <= 0 { cacheOpts.Capacity = 10000 } - t.certCache = certmagic.NewCache(cacheOpts) + + certCacheMu.Lock() + if certCache == nil { + certCache = certmagic.NewCache(cacheOpts) + } else { + certCache.SetOptions(cacheOpts) + } + certCacheMu.Unlock() // certificate loaders val, err := ctx.LoadModule(t, "CertificatesRaw") @@ -209,7 +225,8 @@ func (t *TLS) Provision(ctx caddy.Context) error { // provision so that other apps (such as http) can know which // certificates have been manually loaded, and also so that // commands like validate can be a better test - magic := certmagic.New(t.certCache, certmagic.Config{ + certCacheMu.RLock() + magic := certmagic.New(certCache, certmagic.Config{ Storage: ctx.Storage(), Logger: t.logger, OnEvent: t.onEvent, @@ -217,16 +234,18 @@ func (t *TLS) Provision(ctx caddy.Context) error { DisableStapling: t.DisableOCSPStapling, }, }) + certCacheMu.RUnlock() for _, loader := range t.certificateLoaders { certs, err := loader.LoadCertificates() if err != nil { return fmt.Errorf("loading certificates: %v", err) } for _, cert := range certs { - err := magic.CacheUnmanagedTLSCertificate(ctx, cert.Certificate, cert.Tags) + hash, err := magic.CacheUnmanagedTLSCertificate(ctx, cert.Certificate, cert.Tags) if err != nil { return fmt.Errorf("caching unmanaged certificate: %v", err) } + t.loaded[hash] = struct{}{} } } @@ -305,16 +324,44 @@ func (t *TLS) Stop() error { // Cleanup frees up resources allocated during Provision. func (t *TLS) Cleanup() error { - // stop the certificate cache - if t.certCache != nil { - t.certCache.Stop() - } - // stop the session ticket rotation goroutine if t.SessionTickets != nil { t.SessionTickets.stop() } + // if a new TLS app was loaded, remove certificates from the cache that are no longer + // being managed or loaded by the new config; if there is no more TLS app running, + // then stop cert maintenance and let the cert cache be GC'ed + if nextTLS := caddy.ActiveContext().AppIfConfigured("tls"); nextTLS != nil { + nextTLSApp := nextTLS.(*TLS) + + // compute which certificates were managed or loaded into the cert cache by this + // app instance (which is being stopped) that are not managed or loaded by the + // new app instance (which just started), and remove them from the cache + var noLongerManaged, noLongerLoaded []string + for subj := range t.managing { + if _, ok := nextTLSApp.managing[subj]; !ok { + noLongerManaged = append(noLongerManaged, subj) + } + } + for hash := range t.loaded { + if _, ok := nextTLSApp.loaded[hash]; !ok { + noLongerLoaded = append(noLongerLoaded, hash) + } + } + + certCacheMu.RLock() + certCache.RemoveManaged(noLongerManaged) + certCache.Remove(noLongerLoaded) + certCacheMu.RUnlock() + } else { + // no more TLS app running, so delete in-memory cert cache + certCache.Stop() + certCacheMu.Lock() + certCache = nil + certCacheMu.Unlock() + } + return nil } @@ -339,6 +386,9 @@ func (t *TLS) Manage(names []string) error { if err != nil { return fmt.Errorf("automate: manage %v: %v", names, err) } + for _, name := range names { + t.managing[name] = struct{}{} + } } return nil @@ -449,8 +499,27 @@ func (t *TLS) getAutomationPolicyForName(name string) *AutomationPolicy { // AllMatchingCertificates returns the list of all certificates in // the cache which could be used to satisfy the given SAN. -func (t *TLS) AllMatchingCertificates(san string) []certmagic.Certificate { - return t.certCache.AllMatchingCertificates(san) +func AllMatchingCertificates(san string) []certmagic.Certificate { + return certCache.AllMatchingCertificates(san) +} + +func (t *TLS) HasCertificateForSubject(subject string) bool { + certCacheMu.RLock() + allMatchingCerts := certCache.AllMatchingCertificates(subject) + certCacheMu.RUnlock() + for _, cert := range allMatchingCerts { + // check if the cert is manually loaded by this config + if _, ok := t.loaded[cert.Hash()]; ok { + return true + } + // check if the cert is automatically managed by this config + for _, name := range cert.Names { + if _, ok := t.managing[name]; ok { + return true + } + } + } + return false } // keepStorageClean starts a goroutine that immediately cleans up all |