caddytls: Make on-demand 'ask' permission modular (#6055)

* caddytls: Make on-demand 'ask' permission modular

This makes the 'ask' endpoint a module, which means that developers can
write custom plugins for granting permission for on-demand certificates.

Kicking myself that we didn't do it this way at the beginning, but who coulda known...

* Lint

* Error on conflicting config

* Fix bad merge

---------

Co-authored-by: Francis Lavoie <[email protected]>

8 files changed, 267 insertions, 137 deletions

diff --git a/caddyconfig/httpcaddyfile/options.go b/caddyconfig/httpcaddyfile/options.go
index fa447f8dc..9ff62d07e 100644
--- a/caddyconfig/httpcaddyfile/options.go
+++ b/caddyconfig/httpcaddyfile/options.go
@@ -335,7 +335,8 @@ func parseOptOnDemand(d *caddyfile.Dispenser, _ any) (any, error) {
 	}
 
 	var ond *caddytls.OnDemandConfig
-	for d.NextBlock(0) {
+
+	for nesting := d.Nesting(); d.NextBlock(nesting); {
 		switch d.Val() {
 		case "ask":
 			if !d.NextArg() {
@@ -344,7 +345,8 @@ func parseOptOnDemand(d *caddyfile.Dispenser, _ any) (any, error) {
 			if ond == nil {
 				ond = new(caddytls.OnDemandConfig)
 			}
-			ond.Ask = d.Val()
+			perm := caddytls.PermissionByHTTP{Endpoint: d.Val()}
+			ond.PermissionRaw = caddyconfig.JSONModuleObject(perm, "module", "http", nil)
 
 		case "interval":
 			if !d.NextArg() {
diff --git a/caddytest/integration/caddyfile_adapt/global_options.txt b/caddytest/integration/caddyfile_adapt/global_options.txt
index 603209802..88729c512 100644
--- a/caddytest/integration/caddyfile_adapt/global_options.txt
+++ b/caddytest/integration/caddyfile_adapt/global_options.txt
@@ -69,7 +69,10 @@
 					}
 				],
 				"on_demand": {
-					"ask": "https://example.com",
+					"permission": {
+						"endpoint": "https://example.com",
+						"module": "http"
+					},
 					"rate_limit": {
 						"interval": 30000000000,
 						"burst": 20
diff --git a/caddytest/integration/caddyfile_adapt/global_options_acme.txt b/caddytest/integration/caddyfile_adapt/global_options_acme.txt
index 03aee2cec..f51779253 100644
--- a/caddytest/integration/caddyfile_adapt/global_options_acme.txt
+++ b/caddytest/integration/caddyfile_adapt/global_options_acme.txt
@@ -78,7 +78,10 @@
 					}
 				],
 				"on_demand": {
-					"ask": "https://example.com",
+					"permission": {
+						"endpoint": "https://example.com",
+						"module": "http"
+					},
 					"rate_limit": {
 						"interval": 30000000000,
 						"burst": 20
diff --git a/caddytest/integration/caddyfile_adapt/global_options_admin.txt b/caddytest/integration/caddyfile_adapt/global_options_admin.txt
index 2b90d6de7..cfc578826 100644
--- a/caddytest/integration/caddyfile_adapt/global_options_admin.txt
+++ b/caddytest/integration/caddyfile_adapt/global_options_admin.txt
@@ -71,7 +71,10 @@
 					}
 				],
 				"on_demand": {
-					"ask": "https://example.com",
+					"permission": {
+						"endpoint": "https://example.com",
+						"module": "http"
+					},
 					"rate_limit": {
 						"interval": 30000000000,
 						"burst": 20
diff --git a/modules/caddytls/acmeissuer.go b/modules/caddytls/acmeissuer.go
index 036e79b1b..8a7f8b499 100644
--- a/modules/caddytls/acmeissuer.go
+++ b/modules/caddytls/acmeissuer.go
@@ -16,12 +16,8 @@ package caddytls
 
 import (
 	"context"
-	"crypto/tls"
 	"crypto/x509"
-	"errors"
 	"fmt"
-	"net"
-	"net/url"
 	"os"
 	"strconv"
 	"time"
@@ -495,49 +491,6 @@ func (iss *ACMEIssuer) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 	return nil
 }
 
-// onDemandAskRequest makes a request to the ask URL
-// to see if a certificate can be obtained for name.
-// The certificate request should be denied if this
-// returns an error.
-func onDemandAskRequest(ctx context.Context, logger *zap.Logger, ask string, name string) error {
-	askURL, err := url.Parse(ask)
-	if err != nil {
-		return fmt.Errorf("parsing ask URL: %v", err)
-	}
-	qs := askURL.Query()
-	qs.Set("domain", name)
-	askURL.RawQuery = qs.Encode()
-
-	askURLString := askURL.String()
-	resp, err := onDemandAskClient.Get(askURLString)
-	if err != nil {
-		return fmt.Errorf("error checking %v to determine if certificate for hostname '%s' should be allowed: %v",
-			ask, name, err)
-	}
-	resp.Body.Close()
-
-	// logging out the client IP can be useful for servers that want to count
-	// attempts from clients to detect patterns of abuse
-	var clientIP string
-	if hello, ok := ctx.Value(certmagic.ClientHelloInfoCtxKey).(*tls.ClientHelloInfo); ok && hello != nil {
-		if remote := hello.Conn.RemoteAddr(); remote != nil {
-			clientIP, _, _ = net.SplitHostPort(remote.String())
-		}
-	}
-
-	logger.Debug("response from ask endpoint",
-		zap.String("client_ip", clientIP),
-		zap.String("domain", name),
-		zap.String("url", askURLString),
-		zap.Int("status", resp.StatusCode))
-
-	if resp.StatusCode < 200 || resp.StatusCode > 299 {
-		return fmt.Errorf("%s: %w %s - non-2xx status code %d", name, errAskDenied, ask, resp.StatusCode)
-	}
-
-	return nil
-}
-
 func ParseCaddyfilePreferredChainsOptions(d *caddyfile.Dispenser) (*ChainPreference, error) {
 	chainPref := new(ChainPreference)
 	if d.NextArg() {
@@ -605,11 +558,6 @@ type ChainPreference struct {
 	AnyCommonName []string `json:"any_common_name,omitempty"`
 }
 
-// errAskDenied is an error that should be wrapped or returned when the
-// configured "ask" endpoint does not allow a certificate to be issued,
-// to distinguish that from other errors such as connection failure.
-var errAskDenied = errors.New("certificate not allowed by ask endpoint")
-
 // Interface guards
 var (
 	_ certmagic.PreChecker  = (*ACMEIssuer)(nil)
diff --git a/modules/caddytls/automation.go b/modules/caddytls/automation.go
index 72eeae76c..a90e5ded8 100644
--- a/modules/caddytls/automation.go
+++ b/modules/caddytls/automation.go
@@ -16,12 +16,12 @@ package caddytls
 
 import (
 	"context"
+	"crypto/tls"
 	"encoding/json"
 	"errors"
 	"fmt"
-	"net/http"
+	"net"
 	"strings"
-	"time"
 
 	"github.com/caddyserver/certmagic"
 	"github.com/mholt/acmez"
@@ -254,37 +254,52 @@ func (ap *AutomationPolicy) Provision(tlsApp *TLS) error {
 	// on-demand TLS
 	var ond *certmagic.OnDemandConfig
 	if ap.OnDemand || len(ap.Managers) > 0 {
-		// ask endpoint is now required after a number of negligence cases causing abuse;
-		// but is still allowed for explicit subjects (non-wildcard, non-unbounded),
-		// 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")
+		// permission module is now required after a number of negligence cases that allowed abuse;
+		// but it may still be optional for explicit subjects (bounded, non-wildcard), for the
+		// internal issuer since it doesn't cause public PKI pressure on ACME servers
+		if ap.isWildcardOrDefault() && !ap.onlyInternalIssuer() && (tlsApp.Automation == nil || tlsApp.Automation.OnDemand == nil || tlsApp.Automation.OnDemand.permission == nil) {
+			return fmt.Errorf("on-demand TLS cannot be enabled without a permission module to prevent abuse; please refer to documentation for details")
 		}
 		ond = &certmagic.OnDemandConfig{
 			DecisionFunc: func(ctx context.Context, name string) error {
 				if tlsApp.Automation == nil || tlsApp.Automation.OnDemand == nil {
 					return nil
 				}
-				if err := onDemandAskRequest(ctx, tlsApp.logger, tlsApp.Automation.OnDemand.Ask, name); err != nil {
+
+				// logging the remote IP can be useful for servers that want to count
+				// attempts from clients to detect patterns of abuse -- it should NOT be
+				// used solely for decision making, however
+				var remoteIP string
+				if hello, ok := ctx.Value(certmagic.ClientHelloInfoCtxKey).(*tls.ClientHelloInfo); ok && hello != nil {
+					if remote := hello.Conn.RemoteAddr(); remote != nil {
+						remoteIP, _, _ = net.SplitHostPort(remote.String())
+					}
+				}
+				tlsApp.logger.Debug("asking for permission for on-demand certificate",
+					zap.String("remote_ip", remoteIP),
+					zap.String("domain", name))
+
+				// ask the permission module if this cert is allowed
+				if err := tlsApp.Automation.OnDemand.permission.CertificateAllowed(ctx, 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),
+					if errors.Is(err, ErrPermissionDenied) {
+						tlsApp.logger.Debug("on-demand certificate issuance denied",
 							zap.String("domain", name),
 							zap.Error(err))
 					} else {
-						tlsApp.logger.Error("request to 'ask' endpoint failed",
-							zap.String("ask_endpoint", tlsApp.Automation.OnDemand.Ask),
+						tlsApp.logger.Error("failed to get permission for on-demand certificate",
 							zap.String("domain", name),
 							zap.Error(err))
 					}
 					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,
@@ -464,42 +479,6 @@ type DNSChallengeConfig struct {
 	solver acmez.Solver
 }
 
-// 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.
-type OnDemandConfig struct {
-	// REQUIRED. If Caddy needs to load a certificate from
-	// storage or 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 in the "domain" query
-	// string parameter, like so: `?domain=example.com`.
-	// The endpoint must return a 200 OK status if a certificate
-	// is allowed; anything else will cause it to be denied.
-	// Redirects are not followed.
-	Ask string `json:"ask,omitempty"`
-
-	// 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. RateLimit specifies an interval with optional burst size.
-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"`
-}
-
 // ConfigSetter is implemented by certmagic.Issuers that
 // need access to a parent certmagic.Config as part of
 // their provisioning phase. For example, the ACMEIssuer
@@ -508,14 +487,3 @@ type RateLimit struct {
 type ConfigSetter interface {
 	SetConfig(cfg *certmagic.Config)
 }
-
-// These perpetual values are used for on-demand TLS.
-var (
-	onDemandRateLimiter = certmagic.NewRateLimiter(0, 0)
-	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/ondemand.go b/modules/caddytls/ondemand.go
new file mode 100644
index 000000000..31f6ef2dc
--- /dev/null
+++ b/modules/caddytls/ondemand.go
@@ -0,0 +1,192 @@
+// Copyright 2015 Matthew Holt and The Caddy Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package caddytls
+
+import (
+	"context"
+	"crypto/tls"
+	"encoding/json"
+	"errors"
+	"fmt"
+	"net/http"
+	"net/url"
+	"time"
+
+	"github.com/caddyserver/certmagic"
+	"go.uber.org/zap"
+
+	"github.com/caddyserver/caddy/v2"
+)
+
+func init() {
+	caddy.RegisterModule(PermissionByHTTP{})
+}
+
+// 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.
+type OnDemandConfig struct {
+	// DEPRECATED. WILL BE REMOVED SOON. Use 'permission' instead.
+	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
+// whether a certificate should be allowed to be obtained or
+// loaded from storage on-demand.
+// EXPERIMENTAL: This API is experimental and subject to change.
+type OnDemandPermission interface {
+	// CertificateAllowed returns nil if a certificate for the given
+	// name is allowed to be either obtained from an issuer or loaded
+	// from storage on-demand.
+	//
+	// The context passed in has the associated *tls.ClientHelloInfo
+	// value available at the certmagic.ClientHelloInfoCtxKey key.
+	//
+	// In the worst case, this function may be called as frequently
+	// as every TLS handshake, so it should return as quick as possible
+	// to reduce latency. In the normal case, this function is only
+	// called when a certificate is needed that is not already loaded
+	// into memory ready to serve.
+	CertificateAllowed(ctx context.Context, name string) error
+}
+
+// PermissionByHTTP determines permission for a TLS certificate by
+// making a request to an HTTP endpoint.
+type PermissionByHTTP struct {
+	// The endpoint to access. It should be a full URL.
+	// A query string parameter "domain" will be added to it,
+	// containing the domain (or IP) for the desired certificate,
+	// like so: `?domain=example.com`. Generally, this endpoint
+	// is not exposed publicly to avoid a minor information leak
+	// (which domains are serviced by your application).
+	//
+	// The endpoint must return a 200 OK status if a certificate
+	// is allowed; anything else will cause it to be denied.
+	// Redirects are not followed.
+	Endpoint string `json:"endpoint"`
+
+	logger   *zap.Logger
+	replacer *caddy.Replacer
+}
+
+// CaddyModule returns the Caddy module information.
+func (PermissionByHTTP) CaddyModule() caddy.ModuleInfo {
+	return caddy.ModuleInfo{
+		ID:  "tls.permission.http",
+		New: func() caddy.Module { return new(PermissionByHTTP) },
+	}
+}
+
+func (p *PermissionByHTTP) Provision(ctx caddy.Context) error {
+	p.logger = ctx.Logger()
+	p.replacer = caddy.NewReplacer()
+	return nil
+}
+
+func (p PermissionByHTTP) CertificateAllowed(ctx context.Context, name string) error {
+	// run replacer on endpoint URL (for environment variables) -- return errors to prevent surprises (#5036)
+	askEndpoint, err := p.replacer.ReplaceOrErr(p.Endpoint, true, true)
+	if err != nil {
+		return fmt.Errorf("preparing 'ask' endpoint: %v", err)
+	}
+
+	askURL, err := url.Parse(askEndpoint)
+	if err != nil {
+		return fmt.Errorf("parsing ask URL: %v", err)
+	}
+	qs := askURL.Query()
+	qs.Set("domain", name)
+	askURL.RawQuery = qs.Encode()
+	askURLString := askURL.String()
+
+	var remote string
+	if chi, ok := ctx.Value(certmagic.ClientHelloInfoCtxKey).(*tls.ClientHelloInfo); ok && chi != nil {
+		remote = chi.Conn.RemoteAddr().String()
+	}
+
+	p.logger.Debug("asking permission endpoint",
+		zap.String("remote", remote),
+		zap.String("domain", name),
+		zap.String("url", askURLString))
+
+	resp, err := onDemandAskClient.Get(askURLString)
+	if err != nil {
+		return fmt.Errorf("checking %v to determine if certificate for hostname '%s' should be allowed: %v",
+			askEndpoint, name, err)
+	}
+	resp.Body.Close()
+
+	p.logger.Debug("response from permission endpoint",
+		zap.String("remote", remote),
+		zap.String("domain", name),
+		zap.String("url", askURLString),
+		zap.Int("status", resp.StatusCode))
+
+	if resp.StatusCode < 200 || resp.StatusCode > 299 {
+		return fmt.Errorf("%s: %w %s - non-2xx status code %d", name, ErrPermissionDenied, askEndpoint, resp.StatusCode)
+	}
+
+	return nil
+}
+
+// ErrPermissionDenied is an error that should be wrapped or returned when the
+// configured permission module does not allow a certificate to be issued,
+// to distinguish that from other errors such as connection failure.
+var ErrPermissionDenied = errors.New("certificate not allowed by permission module")
+
+// These perpetual values are used for on-demand TLS.
+var (
+	onDemandRateLimiter = certmagic.NewRateLimiter(0, 0)
+	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")
+		},
+	}
+)
+
+// Interface guards
+var (
+	_ OnDemandPermission = (*PermissionByHTTP)(nil)
+	_ caddy.Provisioner  = (*PermissionByHTTP)(nil)
+)
diff --git a/modules/caddytls/tls.go b/modules/caddytls/tls.go
index b66b09c4d..2ec7bd8fb 100644
--- a/modules/caddytls/tls.go
+++ b/modules/caddytls/tls.go
@@ -164,6 +164,36 @@ func (t *TLS) Provision(ctx caddy.Context) error {
 		t.certificateLoaders = append(t.certificateLoaders, modIface.(CertificateLoader))
 	}
 
+	// on-demand permission module
+	if t.Automation != nil && t.Automation.OnDemand != nil && t.Automation.OnDemand.PermissionRaw != nil {
+		if t.Automation.OnDemand.Ask != "" {
+			return fmt.Errorf("on-demand TLS config conflict: both 'ask' endpoint and a 'permission' module are specified; 'ask' is deprecated, so use only the permission module")
+		}
+		val, err := ctx.LoadModule(t.Automation.OnDemand, "PermissionRaw")
+		if err != nil {
+			return fmt.Errorf("loading on-demand TLS permission module: %v", err)
+		}
+		t.Automation.OnDemand.permission = val.(OnDemandPermission)
+	}
+
+	// on-demand rate limiting
+	if t.Automation != nil && t.Automation.OnDemand != nil && t.Automation.OnDemand.RateLimit != nil {
+		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)
+		if err != nil {
+			return fmt.Errorf("preparing 'ask' endpoint: %v", err)
+		}
+	}
+
 	// automation/management policies
 	if t.Automation == nil {
 		t.Automation = new(AutomationConfig)
@@ -204,24 +234,6 @@ func (t *TLS) Provision(ctx caddy.Context) error {
 		}
 	}
 
-	// on-demand rate limiting
-	if t.Automation != nil && t.Automation.OnDemand != nil && t.Automation.OnDemand.RateLimit != nil {
-		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)
-		if err != nil {
-			return fmt.Errorf("preparing 'ask' endpoint: %v", err)
-		}
-	}
-
 	// load manual/static (unmanaged) certificates - we do this in
 	// provision so that other apps (such as http) can know which
 	// certificates have been manually loaded, and also so that
@@ -288,8 +300,7 @@ func (t *TLS) Validate() error {
 // Start activates the TLS module.
 func (t *TLS) Start() error {
 	// warn if on-demand TLS is enabled but no restrictions are in place
-	if t.Automation.OnDemand == nil ||
-		(t.Automation.OnDemand.Ask == "" && t.Automation.OnDemand.RateLimit == nil) {
+	if t.Automation.OnDemand == nil || (t.Automation.OnDemand.Ask == "" && t.Automation.OnDemand.permission == nil) {
 		for _, ap := range t.Automation.Policies {
 			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",