From d7564d632fbed209e81978c5c2c529a7bf1836f7 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Mon, 7 Oct 2024 17:39:47 -0400 Subject: [PATCH] caddytls: Drop `rate_limit` and `burst`, has been deprecated (#6611) --- caddyconfig/httpcaddyfile/options.go | 30 ++---------------- .../global_options.caddyfiletest | 6 ---- .../global_options_acme.caddyfiletest | 6 ---- .../global_options_admin.caddyfiletest | 6 ---- modules/caddytls/automation.go | 6 ---- modules/caddytls/ondemand.go | 31 +++---------------- modules/caddytls/tls.go | 11 ------- 7 files changed, 7 insertions(+), 89 deletions(-) diff --git a/caddyconfig/httpcaddyfile/options.go b/caddyconfig/httpcaddyfile/options.go index abbe8f4185e..9bae760c01a 100644 --- a/caddyconfig/httpcaddyfile/options.go +++ b/caddyconfig/httpcaddyfile/options.go @@ -394,36 +394,10 @@ func parseOptOnDemand(d *caddyfile.Dispenser, _ any) (any, error) { ond.PermissionRaw = caddyconfig.JSONModuleObject(perm, "module", modName, nil) case "interval": - if !d.NextArg() { - return nil, d.ArgErr() - } - dur, err := caddy.ParseDuration(d.Val()) - if err != nil { - return nil, err - } - if ond == nil { - ond = new(caddytls.OnDemandConfig) - } - if ond.RateLimit == nil { - ond.RateLimit = new(caddytls.RateLimit) - } - ond.RateLimit.Interval = caddy.Duration(dur) + return nil, d.Errf("the on_demand_tls 'interval' option is no longer supported, remove it from your config") case "burst": - if !d.NextArg() { - return nil, d.ArgErr() - } - burst, err := strconv.Atoi(d.Val()) - if err != nil { - return nil, err - } - if ond == nil { - ond = new(caddytls.OnDemandConfig) - } - if ond.RateLimit == nil { - ond.RateLimit = new(caddytls.RateLimit) - } - ond.RateLimit.Burst = burst + return nil, d.Errf("the on_demand_tls 'burst' option is no longer supported, remove it from your config") default: return nil, d.Errf("unrecognized parameter '%s'", d.Val()) diff --git a/caddytest/integration/caddyfile_adapt/global_options.caddyfiletest b/caddytest/integration/caddyfile_adapt/global_options.caddyfiletest index 88729c51287..af301615b41 100644 --- a/caddytest/integration/caddyfile_adapt/global_options.caddyfiletest +++ b/caddytest/integration/caddyfile_adapt/global_options.caddyfiletest @@ -17,8 +17,6 @@ admin off on_demand_tls { ask https://example.com - interval 30s - burst 20 } local_certs key_type ed25519 @@ -72,10 +70,6 @@ "permission": { "endpoint": "https://example.com", "module": "http" - }, - "rate_limit": { - "interval": 30000000000, - "burst": 20 } } }, diff --git a/caddytest/integration/caddyfile_adapt/global_options_acme.caddyfiletest b/caddytest/integration/caddyfile_adapt/global_options_acme.caddyfiletest index bc4b6dcaf91..004a3a32ece 100644 --- a/caddytest/integration/caddyfile_adapt/global_options_acme.caddyfiletest +++ b/caddytest/integration/caddyfile_adapt/global_options_acme.caddyfiletest @@ -17,8 +17,6 @@ admin off on_demand_tls { ask https://example.com - interval 30s - burst 20 } storage_clean_interval 7d renew_interval 1d @@ -89,10 +87,6 @@ "permission": { "endpoint": "https://example.com", "module": "http" - }, - "rate_limit": { - "interval": 30000000000, - "burst": 20 } }, "ocsp_interval": 172800000000000, diff --git a/caddytest/integration/caddyfile_adapt/global_options_admin.caddyfiletest b/caddytest/integration/caddyfile_adapt/global_options_admin.caddyfiletest index cfc5788267f..be309eaa304 100644 --- a/caddytest/integration/caddyfile_adapt/global_options_admin.caddyfiletest +++ b/caddytest/integration/caddyfile_adapt/global_options_admin.caddyfiletest @@ -16,8 +16,6 @@ } on_demand_tls { ask https://example.com - interval 30s - burst 20 } local_certs key_type ed25519 @@ -74,10 +72,6 @@ "permission": { "endpoint": "https://example.com", "module": "http" - }, - "rate_limit": { - "interval": 30000000000, - "burst": 20 } } } diff --git a/modules/caddytls/automation.go b/modules/caddytls/automation.go index 1f1042ba0da..f6a5350778d 100644 --- a/modules/caddytls/automation.go +++ b/modules/caddytls/automation.go @@ -322,12 +322,6 @@ func (ap *AutomationPolicy) Provision(tlsApp *TLS) error { 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 }, Managers: ap.Managers, diff --git a/modules/caddytls/ondemand.go b/modules/caddytls/ondemand.go index 89abfe03f41..066473cd939 100644 --- a/modules/caddytls/ondemand.go +++ b/modules/caddytls/ondemand.go @@ -38,12 +38,11 @@ func init() { // OnDemandConfig configures on-demand TLS, for obtaining // needed certificates at handshake-time. Because this -// feature can easily be abused, you should use this to -// establish rate limits and/or an internal endpoint that -// Caddy can "ask" if it should be allowed to manage -// certificates for a given hostname. +// feature can easily be abused, Caddy must ask permission +// to your application whether a particular domain is allowed +// to have a certificate issued for it. type OnDemandConfig struct { - // DEPRECATED. WILL BE REMOVED SOON. Use 'permission' instead. + // DEPRECATED. WILL BE REMOVED SOON. Use 'permission' instead with the `http` module. Ask string `json:"ask,omitempty"` // REQUIRED. A module that will determine whether a @@ -51,25 +50,6 @@ type OnDemandConfig struct { // or obtained from an issuer on demand. PermissionRaw json.RawMessage `json:"permission,omitempty" caddy:"namespace=tls.permission inline_key=module"` permission OnDemandPermission - - // DEPRECATED. An optional rate limit to throttle - // the checking of storage and the issuance of - // certificates from handshakes if not already in - // storage. WILL BE REMOVED IN A FUTURE RELEASE. - RateLimit *RateLimit `json:"rate_limit,omitempty"` -} - -// DEPRECATED. WILL LIKELY BE REMOVED SOON. -// Instead of using this rate limiter, use a proper tool such as a -// level 3 or 4 firewall and/or a permission module to apply rate limits. -type RateLimit struct { - // A duration value. Storage may be checked and a certificate may be - // obtained 'burst' times during this interval. - Interval caddy.Duration `json:"interval,omitempty"` - - // How many times during an interval storage can be checked or a - // certificate can be obtained. - Burst int `json:"burst,omitempty"` } // OnDemandPermission is a type that can give permission for @@ -195,8 +175,7 @@ var ErrPermissionDenied = errors.New("certificate not allowed by permission modu // These perpetual values are used for on-demand TLS. var ( - onDemandRateLimiter = certmagic.NewRateLimiter(0, 0) - onDemandAskClient = &http.Client{ + 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") diff --git a/modules/caddytls/tls.go b/modules/caddytls/tls.go index f04beb2ee47..6e660dea851 100644 --- a/modules/caddytls/tls.go +++ b/modules/caddytls/tls.go @@ -188,17 +188,6 @@ func (t *TLS) Provision(ctx caddy.Context) error { t.Automation.OnDemand.permission = val.(OnDemandPermission) } - // on-demand rate limiting (TODO: deprecated, and should be removed later; rate limiting is ineffective now that permission modules are required) - if t.Automation != nil && t.Automation.OnDemand != nil && t.Automation.OnDemand.RateLimit != nil { - t.logger.Warn("DEPRECATED: on_demand.rate_limit will be removed in a future release; use permission modules or external certificate managers instead") - onDemandRateLimiter.SetMaxEvents(t.Automation.OnDemand.RateLimit.Burst) - onDemandRateLimiter.SetWindow(time.Duration(t.Automation.OnDemand.RateLimit.Interval)) - } else { - // remove any existing rate limiter - onDemandRateLimiter.SetWindow(0) - onDemandRateLimiter.SetMaxEvents(0) - } - // run replacer on ask URL (for environment variables) -- return errors to prevent surprises (#5036) if t.Automation != nil && t.Automation.OnDemand != nil && t.Automation.OnDemand.Ask != "" { t.Automation.OnDemand.Ask, err = repl.ReplaceOrErr(t.Automation.OnDemand.Ask, true, true)