caddytls: Drop rate_limit and burst, has been deprecated (#6611)

Author: francislavoie

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())

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
 					}
 				}
 			},

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,

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
 					}
 				}
 			}

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,

modules/caddytls/ondemand.go

@@ -38,38 +38,18 @@ 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
 	// certificate is allowed to be loaded from storage
 	// 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")

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)