From 0cc49c053f77bf6efa8107fa50d2e256a91d0ff8 Mon Sep 17 00:00:00 2001
From: Matt Holt <mholt@users.noreply.github.com>
Date: Mon, 20 Mar 2023 12:06:00 -0600
Subject: [PATCH] caddytls: Zero out throttle window first (#5443)

* caddytls: Zero out throttle window first

* Don't error for on-demand

Fixes https://github.com/caddyserver/caddy/commit/b97c76fb4789b8da0b80f5a2c1c1c5bebba163b5

---------

Co-authored-by: Francis Lavoie <lavofr@gmail.com>
---
 modules/caddytls/automation.go | 23 +++++++++++++++++++++--
 modules/caddytls/tls.go        | 15 ++-------------
 2 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/modules/caddytls/automation.go b/modules/caddytls/automation.go
index fffc0a36..1cfb28c3 100644
--- a/modules/caddytls/automation.go
+++ b/modules/caddytls/automation.go
@@ -19,6 +19,7 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
+	"strings"
 	"time"
 
 	"github.com/caddyserver/caddy/v2"
@@ -224,8 +225,10 @@ func (ap *AutomationPolicy) Provision(tlsApp *TLS) error {
 	// on-demand TLS
 	var ond *certmagic.OnDemandConfig
 	if ap.OnDemand {
-		// ask endpoint is now required after a number of negligence cases causing abuse
-		if !ap.onlyInternalIssuer() && (tlsApp.Automation == nil || tlsApp.Automation.OnDemand == nil || tlsApp.Automation.OnDemand.Ask == "") {
+		// ask endpoint is now required after a number of negligence cases causing abuse;
+		// but is still allowed for explicit subjects (non-wildcard, non-unbounded),
+		// and for the internal issuer since it doesn't cause ACME issuer pressure
+		if ap.isWildcardOrDefault() && !ap.onlyInternalIssuer() && (tlsApp.Automation == nil || tlsApp.Automation.OnDemand == nil || tlsApp.Automation.OnDemand.Ask == "") {
 			return fmt.Errorf("on-demand TLS cannot be enabled without an 'ask' endpoint to prevent abuse; please refer to documentation for details")
 		}
 		ond = &certmagic.OnDemandConfig{
@@ -294,6 +297,22 @@ func (ap *AutomationPolicy) onlyInternalIssuer() bool {
 	return ok
 }
 
+// isWildcardOrDefault determines if the subjects include any wildcard domains,
+// or is the "default" policy (i.e. no subjects) which is unbounded.
+func (ap *AutomationPolicy) isWildcardOrDefault() bool {
+	isWildcardOrDefault := false
+	if len(ap.Subjects) == 0 {
+		isWildcardOrDefault = true
+	}
+	for _, sub := range ap.Subjects {
+		if strings.HasPrefix(sub, "*") {
+			isWildcardOrDefault = true
+			break
+		}
+	}
+	return isWildcardOrDefault
+}
+
 // DefaultIssuers returns empty Issuers (not provisioned) to be used as defaults.
 // This function is experimental and has no compatibility promises.
 func DefaultIssuers() []certmagic.Issuer {
diff --git a/modules/caddytls/tls.go b/modules/caddytls/tls.go
index 92004b82..486a58cb 100644
--- a/modules/caddytls/tls.go
+++ b/modules/caddytls/tls.go
@@ -22,7 +22,6 @@ import (
 	"log"
 	"net/http"
 	"runtime/debug"
-	"strings"
 	"sync"
 	"time"
 
@@ -182,8 +181,8 @@ func (t *TLS) Provision(ctx caddy.Context) error {
 		onDemandRateLimiter.SetWindow(time.Duration(t.Automation.OnDemand.RateLimit.Interval))
 	} else {
 		// remove any existing rate limiter
-		onDemandRateLimiter.SetMaxEvents(0)
 		onDemandRateLimiter.SetWindow(0)
+		onDemandRateLimiter.SetMaxEvents(0)
 	}
 
 	// run replacer on ask URL (for environment variables) -- return errors to prevent surprises (#5036)
@@ -260,17 +259,7 @@ func (t *TLS) Start() error {
 	if t.Automation.OnDemand == nil ||
 		(t.Automation.OnDemand.Ask == "" && t.Automation.OnDemand.RateLimit == nil) {
 		for _, ap := range t.Automation.Policies {
-			isWildcardOrDefault := false
-			if len(ap.Subjects) == 0 {
-				isWildcardOrDefault = true
-			}
-			for _, sub := range ap.Subjects {
-				if strings.HasPrefix(sub, "*") {
-					isWildcardOrDefault = true
-					break
-				}
-			}
-			if ap.OnDemand && isWildcardOrDefault {
+			if ap.OnDemand && ap.isWildcardOrDefault() {
 				t.logger.Warn("YOUR SERVER MAY BE VULNERABLE TO ABUSE: on-demand TLS is enabled, but no protections are in place",
 					zap.String("docs", "https://caddyserver.com/docs/automatic-https#on-demand-tls"))
 				break