From b97c76fb4789b8da0b80f5a2c1c1c5bebba163b5 Mon Sep 17 00:00:00 2001
From: Matthew Holt <mholt@users.noreply.github.com>
Date: Tue, 14 Mar 2023 10:02:44 -0600
Subject: [PATCH] caddytls: Require 'ask' endpoint for on-demand TLS

---
 modules/caddytls/automation.go | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/modules/caddytls/automation.go b/modules/caddytls/automation.go
index 7f216d5e3..526aef5e3 100644
--- a/modules/caddytls/automation.go
+++ b/modules/caddytls/automation.go
@@ -168,22 +168,26 @@ 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 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{
 			DecisionFunc: func(name string) error {
-				// if an "ask" endpoint was defined, consult it first
-				if tlsApp.Automation != nil &&
-					tlsApp.Automation.OnDemand != nil &&
-					tlsApp.Automation.OnDemand.Ask != "" {
-					if err := onDemandAskRequest(tlsApp.logger, tlsApp.Automation.OnDemand.Ask, name); err != nil {
-						// distinguish true errors from denials, because it's important to log actual errors
-						if !errors.Is(err, errAskDenied) {
-							tlsApp.logger.Error("request to 'ask' endpoint failed",
-								zap.Error(err),
-								zap.String("endpoint", tlsApp.Automation.OnDemand.Ask),
-								zap.String("domain", name))
-						}
-						return err
+				if err := onDemandAskRequest(tlsApp.logger, tlsApp.Automation.OnDemand.Ask, name); err != nil {
+					// distinguish true errors from denials, because it's important to elevate actual errors
+					if errors.Is(err, errAskDenied) {
+						tlsApp.logger.Debug("certificate issuance denied",
+							zap.String("ask_endpoint", tlsApp.Automation.OnDemand.Ask),
+							zap.String("domain", name),
+							zap.Error(err))
+					} else {
+						tlsApp.logger.Error("request to 'ask' endpoint failed",
+							zap.String("ask_endpoint", tlsApp.Automation.OnDemand.Ask),
+							zap.String("domain", name),
+							zap.Error(err))
 					}
+					return err
 				}
 				// check the rate limiter last because
 				// doing so makes a reservation
@@ -404,7 +408,7 @@ type OnDemandConfig struct {
 	// issuance of certificates from handshakes.
 	RateLimit *RateLimit `json:"rate_limit,omitempty"`
 
-	// If Caddy needs to obtain or renew a certificate
+	// REQUIRED. If Caddy needs to obtain/renew a certificate
 	// during a TLS handshake, it will perform a quick
 	// HTTP request to this URL to check if it should be
 	// allowed to try to get a certificate for the name