httpcaddyfile: Many tls-related improvements including on-demand support

Holy heck this was complicated

10 files changed, 599 insertions, 241 deletions

diff --git a/caddyconfig/httpcaddyfile/addresses.go b/caddyconfig/httpcaddyfile/addresses.go
index 2d17833..4dad166 100644
--- a/caddyconfig/httpcaddyfile/addresses.go
+++ b/caddyconfig/httpcaddyfile/addresses.go
@@ -274,8 +274,6 @@ func ParseAddress(str string) (Address, error) {
 	return a, nil
 }
 
-// TODO: which of the methods on Address are even used?
-
 // String returns a human-readable form of a. It will
 // be a cleaned-up and filled-out URL string.
 func (a Address) String() string {
@@ -312,7 +310,7 @@ func (a Address) Normalize() Address {
 	path := a.Path
 
 	// ensure host is normalized if it's an IP address
-	host := a.Host
+	host := strings.TrimSpace(a.Host)
 	if ip := net.ParseIP(host); ip != nil {
 		host = ip.String()
 	}
diff --git a/caddyconfig/httpcaddyfile/builtins.go b/caddyconfig/httpcaddyfile/builtins.go
index 91c1c0a..26abf3c 100644
--- a/caddyconfig/httpcaddyfile/builtins.go
+++ b/caddyconfig/httpcaddyfile/builtins.go
@@ -103,14 +103,16 @@ func parseRoot(h Helper) ([]ConfigValue, error) {
 //         load      <paths...>
 //         ca        <acme_ca_endpoint>
 //         dns       <provider_name>
+//         on_demand
 //     }
 //
 func parseTLS(h Helper) ([]ConfigValue, error) {
-	var cp *caddytls.ConnectionPolicy
+	cp := new(caddytls.ConnectionPolicy)
 	var fileLoader caddytls.FileLoader
 	var folderLoader caddytls.FolderLoader
 	var acmeIssuer *caddytls.ACMEIssuer
 	var internalIssuer *caddytls.InternalIssuer
+	var onDemand bool
 
 	for h.Next() {
 		// file certificate loader
@@ -173,10 +175,6 @@ func parseTLS(h Helper) ([]ConfigValue, error) {
 				tlsCertTags[certFilename] = tag
 			}
 			certSelector := caddytls.CustomCertSelectionPolicy{Tag: tag}
-			if cp == nil {
-				cp = new(caddytls.ConnectionPolicy)
-			}
-
 			cp.CertSelection = caddyconfig.JSONModuleObject(certSelector, "policy", "custom", h.warnings)
 		default:
 			return nil, h.ArgErr()
@@ -187,7 +185,6 @@ func parseTLS(h Helper) ([]ConfigValue, error) {
 			hasBlock = true
 
 			switch h.Val() {
-			// connection policy
 			case "protocols":
 				args := h.RemainingArgs()
 				if len(args) == 0 {
@@ -197,55 +194,41 @@ func parseTLS(h Helper) ([]ConfigValue, error) {
 					if _, ok := caddytls.SupportedProtocols[args[0]]; !ok {
 						return nil, h.Errf("Wrong protocol name or protocol not supported: '%s'", args[0])
 					}
-					if cp == nil {
-						cp = new(caddytls.ConnectionPolicy)
-					}
 					cp.ProtocolMin = args[0]
 				}
 				if len(args) > 1 {
 					if _, ok := caddytls.SupportedProtocols[args[1]]; !ok {
 						return nil, h.Errf("Wrong protocol name or protocol not supported: '%s'", args[1])
 					}
-					if cp == nil {
-						cp = new(caddytls.ConnectionPolicy)
-					}
 					cp.ProtocolMax = args[1]
 				}
+
 			case "ciphers":
 				for h.NextArg() {
 					if _, ok := caddytls.SupportedCipherSuites[h.Val()]; !ok {
 						return nil, h.Errf("Wrong cipher suite name or cipher suite not supported: '%s'", h.Val())
 					}
-					if cp == nil {
-						cp = new(caddytls.ConnectionPolicy)
-					}
 					cp.CipherSuites = append(cp.CipherSuites, h.Val())
 				}
+
 			case "curves":
 				for h.NextArg() {
 					if _, ok := caddytls.SupportedCurves[h.Val()]; !ok {
 						return nil, h.Errf("Wrong curve name or curve not supported: '%s'", h.Val())
 					}
-					if cp == nil {
-						cp = new(caddytls.ConnectionPolicy)
-					}
 					cp.Curves = append(cp.Curves, h.Val())
 				}
+
 			case "alpn":
 				args := h.RemainingArgs()
 				if len(args) == 0 {
 					return nil, h.ArgErr()
 				}
-				if cp == nil {
-					cp = new(caddytls.ConnectionPolicy)
-				}
 				cp.ALPN = args
 
-			// certificate folder loader
 			case "load":
 				folderLoader = append(folderLoader, h.RemainingArgs()...)
 
-			// automation policy
 			case "ca":
 				arg := h.RemainingArgs()
 				if len(arg) != 1 {
@@ -256,7 +239,6 @@ func parseTLS(h Helper) ([]ConfigValue, error) {
 				}
 				acmeIssuer.CA = arg[0]
 
-			// DNS provider for ACME DNS challenge
 			case "dns":
 				if !h.Next() {
 					return nil, h.ArgErr()
@@ -284,6 +266,12 @@ func parseTLS(h Helper) ([]ConfigValue, error) {
 				}
 				acmeIssuer.TrustedRootsPEMFiles = append(acmeIssuer.TrustedRootsPEMFiles, arg[0])
 
+			case "on_demand":
+				if h.NextArg() {
+					return nil, h.ArgErr()
+				}
+				onDemand = true
+
 			default:
 				return nil, h.Errf("unknown subdirective: %s", h.Val())
 			}
@@ -304,31 +292,15 @@ func parseTLS(h Helper) ([]ConfigValue, error) {
 			Class: "tls.certificate_loader",
 			Value: fileLoader,
 		})
-		// ensure server uses HTTPS by setting non-nil conn policy
-		if cp == nil {
-			cp = new(caddytls.ConnectionPolicy)
-		}
 	}
 	if len(folderLoader) > 0 {
 		configVals = append(configVals, ConfigValue{
 			Class: "tls.certificate_loader",
 			Value: folderLoader,
 		})
-		// ensure server uses HTTPS by setting non-nil conn policy
-		if cp == nil {
-			cp = new(caddytls.ConnectionPolicy)
-		}
-	}
-
-	// connection policy
-	if cp != nil {
-		configVals = append(configVals, ConfigValue{
-			Class: "tls.connection_policy",
-			Value: cp,
-		})
 	}
 
-	// automation policy
+	// issuer
 	if acmeIssuer != nil && internalIssuer != nil {
 		// the logic to support this would be complex
 		return nil, h.Err("cannot use both ACME and internal issuers in same server block")
@@ -356,6 +328,24 @@ func parseTLS(h Helper) ([]ConfigValue, error) {
 		})
 	}
 
+	// on-demand TLS
+	if onDemand {
+		configVals = append(configVals, ConfigValue{
+			Class: "tls.on_demand",
+			Value: true,
+		})
+	}
+
+	// connection policy -- always add one, to ensure that TLS
+	// is enabled, because this directive was used (this is
+	// needed, for instance, when a site block has a key of
+	// just ":5000" - i.e. no hostname, and only on-demand TLS
+	// is enabled)
+	configVals = append(configVals, ConfigValue{
+		Class: "tls.connection_policy",
+		Value: cp,
+	})
+
 	return configVals, nil
 }
 
diff --git a/caddyconfig/httpcaddyfile/httptype.go b/caddyconfig/httpcaddyfile/httptype.go
index 15cfe1e..18dd0a0 100644
--- a/caddyconfig/httpcaddyfile/httptype.go
+++ b/caddyconfig/httpcaddyfile/httptype.go
@@ -26,7 +26,6 @@ import (
 	"github.com/caddyserver/caddy/v2/caddyconfig/caddyfile"
 	"github.com/caddyserver/caddy/v2/modules/caddyhttp"
 	"github.com/caddyserver/caddy/v2/modules/caddytls"
-	"github.com/caddyserver/certmagic"
 )
 
 func init() {
@@ -177,105 +176,10 @@ func (st ServerType) Setup(originalServerBlocks []caddyfile.ServerBlock,
 		Servers:   servers,
 	}
 
-	// now for the TLS app! (TODO: refactor into own func)
-	tlsApp := caddytls.TLS{CertificatesRaw: make(caddy.ModuleMap)}
-	var certLoaders []caddytls.CertificateLoader
-	for _, p := range pairings {
-		for i, sblock := range p.serverBlocks {
-			// tls automation policies
-			if issuerVals, ok := sblock.pile["tls.cert_issuer"]; ok {
-				for _, issuerVal := range issuerVals {
-					issuer := issuerVal.Value.(certmagic.Issuer)
-					sblockHosts, err := st.hostsFromServerBlockKeys(sblock.block)
-					if err != nil {
-						return nil, warnings, err
-					}
-					if len(sblockHosts) > 0 {
-						if tlsApp.Automation == nil {
-							tlsApp.Automation = new(caddytls.AutomationConfig)
-						}
-						tlsApp.Automation.Policies = append(tlsApp.Automation.Policies, &caddytls.AutomationPolicy{
-							Subjects:  sblockHosts,
-							IssuerRaw: caddyconfig.JSONModuleObject(issuer, "module", issuer.(caddy.Module).CaddyModule().ID.Name(), &warnings),
-						})
-					} else {
-						warnings = append(warnings, caddyconfig.Warning{
-							Message: fmt.Sprintf("Server block %d %v has no names that qualify for automatic HTTPS, so no TLS automation policy will be added.", i, sblock.block.Keys),
-						})
-					}
-				}
-			}
-			// tls certificate loaders
-			if clVals, ok := sblock.pile["tls.certificate_loader"]; ok {
-				for _, clVal := range clVals {
-					certLoaders = append(certLoaders, clVal.Value.(caddytls.CertificateLoader))
-				}
-			}
-		}
-	}
-	// group certificate loaders by module name, then add to config
-	if len(certLoaders) > 0 {
-		loadersByName := make(map[string]caddytls.CertificateLoader)
-		for _, cl := range certLoaders {
-			name := caddy.GetModuleName(cl)
-			// ugh... technically, we may have multiple FileLoader and FolderLoader
-			// modules (because the tls directive returns one per occurrence), but
-			// the config structure expects only one instance of each kind of loader
-			// module, so we have to combine them... instead of enumerating each
-			// possible cert loader module in a type switch, we can use reflection,
-			// which works on any cert loaders that are slice types
-			if reflect.TypeOf(cl).Kind() == reflect.Slice {
-				combined := reflect.ValueOf(loadersByName[name])
-				if !combined.IsValid() {
-					combined = reflect.New(reflect.TypeOf(cl)).Elem()
-				}
-				clVal := reflect.ValueOf(cl)
-				for i := 0; i < clVal.Len(); i++ {
-					combined = reflect.Append(reflect.Value(combined), clVal.Index(i))
-				}
-				loadersByName[name] = combined.Interface().(caddytls.CertificateLoader)
-			}
-		}
-		for certLoaderName, loaders := range loadersByName {
-			tlsApp.CertificatesRaw[certLoaderName] = caddyconfig.JSON(loaders, &warnings)
-		}
-	}
-	// if global ACME CA, DNS, or email were set, append a catch-all automation
-	// policy that ensures they will be used if no tls directive was used
-	acmeCA, hasACMECA := options["acme_ca"]
-	acmeDNS, hasACMEDNS := options["acme_dns"]
-	email, hasEmail := options["email"]
-	if hasACMECA || hasACMEDNS || hasEmail {
-		if tlsApp.Automation == nil {
-			tlsApp.Automation = new(caddytls.AutomationConfig)
-		}
-		if !hasACMECA {
-			acmeCA = ""
-		}
-		if !hasEmail {
-			email = ""
-		}
-		mgr := caddytls.ACMEIssuer{
-			CA:    acmeCA.(string),
-			Email: email.(string),
-		}
-		if hasACMEDNS {
-			provName := acmeDNS.(string)
-			dnsProvModule, err := caddy.GetModule("tls.dns." + provName)
-			if err != nil {
-				return nil, warnings, fmt.Errorf("getting DNS provider module named '%s': %v", provName, err)
-			}
-			mgr.Challenges = &caddytls.ChallengesConfig{
-				DNSRaw: caddyconfig.JSONModuleObject(dnsProvModule.New(), "provider", provName, &warnings),
-			}
-		}
-		tlsApp.Automation.Policies = append(tlsApp.Automation.Policies, &caddytls.AutomationPolicy{
-			IssuerRaw: caddyconfig.JSONModuleObject(mgr, "module", "acme", &warnings),
-		})
-	}
-	if tlsApp.Automation != nil {
-		// consolidate automation policies that are the exact same
-		tlsApp.Automation.Policies = consolidateAutomationPolicies(tlsApp.Automation.Policies)
+	// then make the TLS app
+	tlsApp, warnings, err := st.buildTLSApp(pairings, options, warnings)
+	if err != nil {
+		return nil, warnings, err
 	}
 
 	// if experimental HTTP/3 is enabled, enable it on each server
@@ -316,10 +220,10 @@ func (st ServerType) Setup(originalServerBlocks []caddyfile.ServerBlock,
 
 	// annnd the top-level config, then we're done!
 	cfg := &caddy.Config{AppsRaw: make(caddy.ModuleMap)}
-	if !reflect.DeepEqual(httpApp, caddyhttp.App{}) {
+	if len(httpApp.Servers) > 0 {
 		cfg.AppsRaw["http"] = caddyconfig.JSON(httpApp, &warnings)
 	}
-	if !reflect.DeepEqual(tlsApp, caddytls.TLS{CertificatesRaw: make(caddy.ModuleMap)}) {
+	if !reflect.DeepEqual(tlsApp, &caddytls.TLS{CertificatesRaw: make(caddy.ModuleMap)}) {
 		cfg.AppsRaw["tls"] = caddyconfig.JSON(tlsApp, &warnings)
 	}
 	if storageCvtr, ok := options["storage"].(caddy.StorageConverter); ok {
@@ -377,7 +281,6 @@ func (ServerType) evaluateGlobalOptionsBlock(serverBlocks []serverBlock, options
 		var val interface{}
 		var err error
 		disp := caddyfile.NewDispenser(segment)
-		// TODO: make this switch into a map
 		switch dir {
 		case "http_port":
 			val, err = parseOptHTTPPort(disp)
@@ -399,6 +302,10 @@ func (ServerType) evaluateGlobalOptionsBlock(serverBlocks []serverBlock, options
 			val, err = parseOptAdmin(disp)
 		case "debug":
 			options["debug"] = true
+		case "on_demand_tls":
+			val, err = parseOptOnDemand(disp)
+		case "local_certs":
+			val = true
 		default:
 			return nil, fmt.Errorf("unrecognized parameter name: %s", dir)
 		}
@@ -411,8 +318,10 @@ func (ServerType) evaluateGlobalOptionsBlock(serverBlocks []serverBlock, options
 	return serverBlocks[1:], nil
 }
 
-// hostsFromServerBlockKeys returns a list of all the
-// hostnames found in the keys of the server block sb.
+// hostsFromServerBlockKeys returns a list of all the non-empty hostnames
+// found in the keys of the server block sb. If sb has a key that omits
+// the hostname (i.e. is a catch-all/empty host), then the returned list
+// is empty, because the server block effectively matches ALL hosts.
 // The list may not be in a consistent order.
 func (st *ServerType) hostsFromServerBlockKeys(sb caddyfile.ServerBlock) ([]string, error) {
 	// first get each unique hostname
@@ -424,7 +333,9 @@ func (st *ServerType) hostsFromServerBlockKeys(sb caddyfile.ServerBlock) ([]stri
 		}
 		addr = addr.Normalize()
 		if addr.Host == "" {
-			continue
+			// server block contains a key like ":443", i.e. the host portion
+			// is empty / catch-all, which means to match all hosts
+			return []string{}, nil
 		}
 		hostMap[addr.Host] = struct{}{}
 	}
@@ -497,25 +408,18 @@ func (st *ServerType) serversFromPairings(
 				return nil, fmt.Errorf("server block %v: compiling matcher sets: %v", sblock.block.Keys, err)
 			}
 
-			// tls: connection policies and toggle auto HTTPS
-			if _, ok := sblock.pile["tls.off"]; ok {
-				// TODO: right now, no directives yield any tls.off value...
-				// tls off: disable TLS (and automatic HTTPS) for server block's names
-				if srv.AutoHTTPS == nil {
-					srv.AutoHTTPS = new(caddyhttp.AutoHTTPSConfig)
-				}
-				srv.AutoHTTPS.Disabled = true
-			} else if cpVals, ok := sblock.pile["tls.connection_policy"]; ok {
-				// tls connection policies
+			hosts, err := st.hostsFromServerBlockKeys(sblock.block)
+			if err != nil {
+				return nil, err
+			}
 
+			// tls: connection policies
+			if cpVals, ok := sblock.pile["tls.connection_policy"]; ok {
+				// tls connection policies
 				for _, cpVal := range cpVals {
 					cp := cpVal.Value.(*caddytls.ConnectionPolicy)
 
 					// make sure the policy covers all hostnames from the block
-					hosts, err := st.hostsFromServerBlockKeys(sblock.block)
-					if err != nil {
-						return nil, err
-					}
 					for _, h := range hosts {
 						if h == defaultSNI {
 							hosts = append(hosts, "")
@@ -524,7 +428,6 @@ func (st *ServerType) serversFromPairings(
 						}
 					}
 
-					// TODO: are matchers needed if every hostname of the resulting config is matched?
 					if len(hosts) > 0 {
 						cp.MatchersRaw = caddy.ModuleMap{
 							"sni": caddyconfig.JSON(hosts, warnings), // make sure to match all hosts, not just auto-HTTPS-qualified ones
@@ -536,7 +439,6 @@ func (st *ServerType) serversFromPairings(
 
 					srv.TLSConnPolicies = append(srv.TLSConnPolicies, cp)
 				}
-				// TODO: consolidate equal conn policies?
 			}
 
 			// exclude any hosts that were defined explicitly with
@@ -547,7 +449,7 @@ func (st *ServerType) serversFromPairings(
 					return nil, err
 				}
 				addr = addr.Normalize()
-				if addr.Scheme == "http" {
+				if addr.Scheme == "http" && addr.Host != "" {
 					if srv.AutoHTTPS == nil {
 						srv.AutoHTTPS = new(caddyhttp.AutoHTTPSConfig)
 					}
@@ -607,10 +509,15 @@ func (st *ServerType) serversFromPairings(
 		// catch-all/default policy if there isn't one already (it's
 		// important that it goes at the end) - see issue #3004:
 		// https://github.com/caddyserver/caddy/issues/3004
+		// TODO: maybe a smarter way to handle this might be to just make the
+		// auto-HTTPS logic at provision-time detect if there is any connection
+		// policy missing for any HTTPS-enabled hosts, if so, add it... maybe?
 		if !hasCatchAllTLSConnPolicy && (len(srv.TLSConnPolicies) > 0 || defaultSNI != "") {
 			srv.TLSConnPolicies = append(srv.TLSConnPolicies, &caddytls.ConnectionPolicy{DefaultSNI: defaultSNI})
 		}
 
+		// tidy things up a bit
+		srv.TLSConnPolicies = consolidateConnPolicies(srv.TLSConnPolicies)
 		srv.Routes = consolidateRoutes(srv.Routes)
 
 		servers[fmt.Sprintf("srv%d", i)] = srv
@@ -619,6 +526,26 @@ func (st *ServerType) serversFromPairings(
 	return servers, nil
 }
 
+// consolidateConnPolicies combines TLS connection policies that are the same,
+// for a cleaner overall output.
+func consolidateConnPolicies(cps caddytls.ConnectionPolicies) caddytls.ConnectionPolicies {
+	for i := 0; i < len(cps); i++ {
+		for j := 0; j < len(cps); j++ {
+			if j == i {
+				continue
+			}
+
+			// if they're exactly equal in every way, just keep one of them
+			if reflect.DeepEqual(cps[i], cps[j]) {
+				cps = append(cps[:j], cps[j+1:]...)
+				i--
+				break
+			}
+		}
+	}
+	return cps
+}
+
 // appendSubrouteToRouteList appends the routes in subroute
 // to the routeList, optionally qualified by matchers.
 func appendSubrouteToRouteList(routeList caddyhttp.RouteList,
@@ -750,52 +677,6 @@ func consolidateRoutes(routes caddyhttp.RouteList) caddyhttp.RouteList {
 	return routes
 }
 
-// consolidateAutomationPolicies combines automation policies that are the same,
-// for a cleaner overall output.
-func consolidateAutomationPolicies(aps []*caddytls.AutomationPolicy) []*caddytls.AutomationPolicy {
-	for i := 0; i < len(aps); i++ {
-		for j := 0; j < len(aps); j++ {
-			if j == i {
-				continue
-			}
-
-			// if they're exactly equal in every way, just keep one of them
-			if reflect.DeepEqual(aps[i], aps[j]) {
-				aps = append(aps[:j], aps[j+1:]...)
-				i--
-				break
-			}
-
-			// if the policy is the same, we can keep just one, but we have
-			// to be careful which one we keep; if only one has any hostnames
-			// defined, then we need to keep the one without any hostnames,
-			// otherwise the one without any subjects (a catch-all) would be
-			// eaten up by the one with subjects; and if both have subjects, we
-			// need to combine their lists
-			if reflect.DeepEqual(aps[i].IssuerRaw, aps[j].IssuerRaw) &&
-				aps[i].ManageSync == aps[j].ManageSync {
-				if len(aps[i].Subjects) == 0 && len(aps[j].Subjects) > 0 {
-					aps = append(aps[:j], aps[j+1:]...)
-				} else if len(aps[i].Subjects) > 0 && len(aps[j].Subjects) == 0 {
-					aps = append(aps[:i], aps[i+1:]...)
-				} else {
-					aps[i].Subjects = append(aps[i].Subjects, aps[j].Subjects...)
-					aps = append(aps[:j], aps[j+1:]...)
-				}
-				i--
-				break
-			}
-		}
-	}
-
-	// ensure any catch-all policies go last
-	sort.SliceStable(aps, func(i, j int) bool {
-		return len(aps[i].Subjects) > len(aps[j].Subjects)
-	})
-
-	return aps
-}
-
 func matcherSetFromMatcherToken(
 	tkn caddyfile.Token,
 	matcherDefs map[string]caddy.ModuleMap,
@@ -831,6 +712,7 @@ func (st *ServerType) compileEncodedMatcherSets(sblock caddyfile.ServerBlock) ([
 	// keep routes with common host and path matchers together
 	var matcherPairs []*hostPathPair
 
+	var catchAllHosts bool
 	for _, key := range sblock.Keys {
 		addr, err := ParseAddress(key)
 		if err != nil {
@@ -856,6 +738,17 @@ func (st *ServerType) compileEncodedMatcherSets(sblock caddyfile.ServerBlock) ([
 			matcherPairs = append(matcherPairs, chosenMatcherPair)
 		}
 
+		// if one of the keys has no host (i.e. is a catch-all for
+		// any hostname), then we need to null out the host matcher
+		// entirely so that it matches all hosts
+		if addr.Host == "" && !catchAllHosts {
+			chosenMatcherPair.hostm = nil
+			catchAllHosts = true
+		}
+		if catchAllHosts {
+			continue
+		}
+
 		// add this server block's keys to the matcher
 		// pair if it doesn't already exist
 		if addr.Host != "" {
diff --git a/caddyconfig/httpcaddyfile/options.go b/caddyconfig/httpcaddyfile/options.go
index 7dc7bdb..072d8f4 100644
--- a/caddyconfig/httpcaddyfile/options.go
+++ b/caddyconfig/httpcaddyfile/options.go
@@ -15,11 +15,12 @@
 package httpcaddyfile
 
 import (
-	"fmt"
 	"strconv"
+	"time"
 
 	"github.com/caddyserver/caddy/v2"
 	"github.com/caddyserver/caddy/v2/caddyconfig/caddyfile"
+	"github.com/caddyserver/caddy/v2/modules/caddytls"
 )
 
 func parseOptHTTPPort(d *caddyfile.Dispenser) (int, error) {
@@ -68,7 +69,7 @@ func parseOptOrder(d *caddyfile.Dispenser) ([]string, error) {
 		}
 		dirName := d.Val()
 		if _, ok := registeredDirectives[dirName]; !ok {
-			return nil, fmt.Errorf("%s is not a registered directive", dirName)
+			return nil, d.Errf("%s is not a registered directive", dirName)
 		}
 
 		// get positional token
@@ -104,7 +105,7 @@ func parseOptOrder(d *caddyfile.Dispenser) ([]string, error) {
 		case "before":
 		case "after":
 		default:
-			return nil, fmt.Errorf("unknown positional '%s'", pos)
+			return nil, d.Errf("unknown positional '%s'", pos)
 		}
 
 		// get name of other directive
@@ -145,11 +146,11 @@ func parseOptStorage(d *caddyfile.Dispenser) (caddy.StorageConverter, error) {
 	modName := args[0]
 	mod, err := caddy.GetModule("caddy.storage." + modName)
 	if err != nil {
-		return nil, fmt.Errorf("getting storage module '%s': %v", modName, err)
+		return nil, d.Errf("getting storage module '%s': %v", modName, err)
 	}
 	unm, ok := mod.New().(caddyfile.Unmarshaler)
 	if !ok {
-		return nil, fmt.Errorf("storage module '%s' is not a Caddyfile unmarshaler", mod.ID)
+		return nil, d.Errf("storage module '%s' is not a Caddyfile unmarshaler", mod.ID)
 	}
 	err = unm.UnmarshalCaddyfile(d.NewFromNextSegment())
 	if err != nil {
@@ -157,7 +158,7 @@ func parseOptStorage(d *caddyfile.Dispenser) (caddy.StorageConverter, error) {
 	}
 	storage, ok := unm.(caddy.StorageConverter)
 	if !ok {
-		return nil, fmt.Errorf("module %s is not a StorageConverter", mod.ID)
+		return nil, d.Errf("module %s is not a StorageConverter", mod.ID)
 	}
 	return storage, nil
 }
@@ -187,3 +188,63 @@ func parseOptAdmin(d *caddyfile.Dispenser) (string, error) {
 	}
 	return "", nil
 }
+
+func parseOptOnDemand(d *caddyfile.Dispenser) (*caddytls.OnDemandConfig, error) {
+	var ond *caddytls.OnDemandConfig
+	for d.Next() {
+		if d.NextArg() {
+			return nil, d.ArgErr()
+		}
+		for nesting := d.Nesting(); d.NextBlock(nesting); {
+			switch d.Val() {
+			case "ask":
+				if !d.NextArg() {
+					return nil, d.ArgErr()
+				}
+				if ond == nil {
+					ond = new(caddytls.OnDemandConfig)
+				}
+				ond.Ask = d.Val()
+
+			case "interval":
+				if !d.NextArg() {
+					return nil, d.ArgErr()
+				}
+				dur, err := time.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)
+
+			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
+
+			default:
+				return nil, d.Errf("unrecognized parameter '%s'", d.Val())
+			}
+		}
+	}
+	if ond == nil {
+		return nil, d.Err("expected at least one config parameter for on_demand_tls")
+	}
+	return ond, nil
+}
diff --git a/caddyconfig/httpcaddyfile/tlsapp.go b/caddyconfig/httpcaddyfile/tlsapp.go
new file mode 100644
index 0000000..4f72a4a
--- /dev/null
+++ b/caddyconfig/httpcaddyfile/tlsapp.go
@@ -0,0 +1,386 @@
+// 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 httpcaddyfile
+
+import (
+	"bytes"
+	"fmt"
+	"reflect"
+	"sort"
+
+	"github.com/caddyserver/caddy/v2"
+	"github.com/caddyserver/caddy/v2/caddyconfig"
+	"github.com/caddyserver/caddy/v2/modules/caddytls"
+	"github.com/caddyserver/certmagic"
+)
+
+func (st ServerType) buildTLSApp(
+	pairings []sbAddrAssociation,
+	options map[string]interface{},
+	warnings []caddyconfig.Warning,
+) (*caddytls.TLS, []caddyconfig.Warning, error) {
+
+	tlsApp := &caddytls.TLS{CertificatesRaw: make(caddy.ModuleMap)}
+	var certLoaders []caddytls.CertificateLoader
+
+	// count how many server blocks have a key with no host,
+	// and find all hosts that share a server block with a
+	// hostless key, so that they don't get forgotten/omitted
+	// by auto-HTTPS (since they won't appear in route matchers)
+	var serverBlocksWithHostlessKey int
+	hostsSharedWithHostlessKey := make(map[string]struct{})
+	for _, pair := range pairings {
+		for _, sb := range pair.serverBlocks {
+			for _, key := range sb.block.Keys {
+				addr, err := ParseAddress(key)
+				if err != nil {
+					return nil, warnings, err
+				}
+				addr = addr.Normalize()
+				if addr.Host == "" {
+					serverBlocksWithHostlessKey++
+					// this server block has a hostless key, now
+					// go through and add all the hosts to the set
+					for _, otherKey := range sb.block.Keys {
+						if otherKey == key {
+							continue
+						}
+						addr, err := ParseAddress(otherKey)
+						if err != nil {
+							return nil, warnings, err
+						}
+						addr = addr.Normalize()
+						if addr.Host != "" {
+							hostsSharedWithHostlessKey[addr.Host] = struct{}{}
+						}
+					}
+					break
+				}
+			}
+		}
+	}
+
+	catchAllAP, err := newBaseAutomationPolicy(options, warnings, false)
+	if err != nil {
+		return nil, warnings, err
+	}
+
+	for _, p := range pairings {
+		for _, sblock := range p.serverBlocks {
+			// get values that populate an automation policy for this block
+			var ap *caddytls.AutomationPolicy
+
+			sblockHosts, err := st.hostsFromServerBlockKeys(sblock.block)
+			if err != nil {
+				return nil, warnings, err
+			}
+			if len(sblockHosts) == 0 {
+				ap = catchAllAP
+			}
+
+			// on-demand tls
+			if _, ok := sblock.pile["tls.on_demand"]; ok {
+				if ap == nil {
+					var err error
+					ap, err = newBaseAutomationPolicy(options, warnings, true)
+					if err != nil {
+						return nil, warnings, err
+					}
+				}
+				ap.OnDemand = true
+			}
+
+			// certificate issuers
+			if issuerVals, ok := sblock.pile["tls.cert_issuer"]; ok {
+				for _, issuerVal := range issuerVals {
+					issuer := issuerVal.Value.(certmagic.Issuer)
+					if ap == nil {
+						var err error
+						ap, err = newBaseAutomationPolicy(options, warnings, true)
+						if err != nil {
+							return nil, warnings, err
+						}
+					}
+					encoded := caddyconfig.JSONModuleObject(issuer, "module", issuer.(caddy.Module).CaddyModule().ID.Name(), &warnings)
+					if ap == catchAllAP && ap.IssuerRaw != nil && !bytes.Equal(ap.IssuerRaw, encoded) {
+						return nil, warnings, fmt.Errorf("conflicting issuer configuration: %s != %s", ap.IssuerRaw, encoded)
+					}
+					ap.IssuerRaw = encoded
+				}
+			}
+
+			if ap != nil {
+				// first make sure this block is allowed to create an automation policy;
+				// doing so is forbidden if it has a key with no host (i.e. ":443")
+				// and if there is a different server block that also has a key with no
+				// host -- since a key with no host matches any host, we need its
+				// associated automation policy to have an empty Subjects list, i.e. no
+				// host filter, which is indistinguishable between the two server blocks
+				// because automation is not done in the context of a particular server...
+				// this is an example of a poor mapping from Caddyfile to JSON but that's
+				// the least-leaky abstraction I could figure out
+				if len(sblockHosts) == 0 {
+					if serverBlocksWithHostlessKey > 1 {
+						// this server block and at least one other has a key with no host,
+						// making the two indistinguishable; it is misleading to define such
+						// a policy within one server block since it actually will apply to
+						// others as well
+						return nil, warnings, fmt.Errorf("cannot make a TLS automation policy from a server block that has a host-less address when there are other server block addresses lacking a host")
+					}
+					if catchAllAP == nil {
+						// this server block has a key with no hosts, but there is not yet
+						// a catch-all automation policy (probably because no global options
+						// were set), so this one becomes it
+						catchAllAP = ap
+					}
+				}
+
+				// associate our new automation policy with this server block's hosts,
+				// unless, of course, the server block has a key with no hosts, in which
+				// case its automation policy becomes or blends with the default/global
+				// automation policy because, of necessity, it applies to all hostnames
+				// (i.e. it has no Subjects filter) -- in that case, we'll append it last
+				if ap != catchAllAP {
+					ap.Subjects = sblockHosts
+
+					// if a combination of public and internal names were given
+					// for this same server block and no issuer was specified, we
+					// need to separate them out in the automation policies so
+					// that the internal names can use the internal issuer and
+					// the other names can use the default/public/ACME issuer
+					var ap2 *caddytls.AutomationPolicy
+					if ap.Issuer == nil {
+						var internal, external []string
+						for _, s := range ap.Subjects {
+							if certmagic.SubjectQualifiesForPublicCert(s) {
+								external = append(external, s)
+							} else {
+								internal = append(internal, s)
+							}
+						}
+						if len(external) > 0 && len(internal) > 0 {
+							ap.Subjects = external
+							apCopy := *ap
+							ap2 = &apCopy
+							ap2.Subjects = internal
+							ap2.IssuerRaw = caddyconfig.JSONModuleObject(caddytls.InternalIssuer{}, "module", "internal", &warnings)
+						}
+					}
+					if tlsApp.Automation == nil {
+						tlsApp.Automation = new(caddytls.AutomationConfig)
+					}
+					tlsApp.Automation.Policies = append(tlsApp.Automation.Policies, ap)
+					if ap2 != nil {
+						tlsApp.Automation.Policies = append(tlsApp.Automation.Policies, ap2)
+					}
+				}
+			}
+
+			// certificate loaders
+			if clVals, ok := sblock.pile["tls.certificate_loader"]; ok {
+				for _, clVal := range clVals {
+					certLoaders = append(certLoaders, clVal.Value.(caddytls.CertificateLoader))
+				}
+			}
+		}
+	}
+
+	// group certificate loaders by module name, then add to config
+	if len(certLoaders) > 0 {
+		loadersByName := make(map[string]caddytls.CertificateLoader)
+		for _, cl := range certLoaders {
+			name := caddy.GetModuleName(cl)
+			// ugh... technically, we may have multiple FileLoader and FolderLoader
+			// modules (because the tls directive returns one per occurrence), but
+			// the config structure expects only one instance of each kind of loader
+			// module, so we have to combine them... instead of enumerating each
+			// possible cert loader module in a type switch, we can use reflection,
+			// which works on any cert loaders that are slice types
+			if reflect.TypeOf(cl).Kind() == reflect.Slice {
+				combined := reflect.ValueOf(loadersByName[name])
+				if !combined.IsValid() {
+					combined = reflect.New(reflect.TypeOf(cl)).Elem()
+				}
+				clVal := reflect.ValueOf(cl)
+				for i := 0; i < clVal.Len(); i++ {
+					combined = reflect.Append(reflect.Value(combined), clVal.Index(i))
+				}
+				loadersByName[name] = combined.Interface().(caddytls.CertificateLoader)
+			}
+		}
+		for certLoaderName, loaders := range loadersByName {
+			tlsApp.CertificatesRaw[certLoaderName] = caddyconfig.JSON(loaders, &warnings)
+		}
+	}
+
+	// set any of the on-demand options, for if/when on-demand TLS is enabled
+	if onDemand, ok := options["on_demand_tls"].(*caddytls.OnDemandConfig); ok {
+		if tlsApp.Automation == nil {
+			tlsApp.Automation = new(caddytls.AutomationConfig)
+		}
+		tlsApp.Automation.OnDemand = onDemand
+	}
+
+	// if there is a global/catch-all automation policy, ensure it goes last
+	if catchAllAP != nil {
+		if tlsApp.Automation == nil {
+			tlsApp.Automation = new(caddytls.AutomationConfig)
+		}
+		tlsApp.Automation.Policies = append(tlsApp.Automation.Policies, catchAllAP)
+	}
+
+	// if any hostnames appear on the same server block as a key with
+	// no host, they will not be used with route matchers because the
+	// hostless key matches all hosts, therefore, it wouldn't be
+	// considered for auto-HTTPS, so we need to make sure those hosts
+	// are manually considered for managed certificates
+	var al caddytls.AutomateLoader
+	for h := range hostsSharedWithHostlessKey {
+		al = append(al, h)
+	}
+	if len(al) > 0 {
+		tlsApp.CertificatesRaw["automate"] = caddyconfig.JSON(al, &warnings)
+	}
+
+	// do a little verification & cleanup
+	if tlsApp.Automation != nil {
+		// ensure automation policies don't overlap subjects (this should be
+		// an error at provision-time as well, but catch it in the adapt phase
+		// for convenience)
+		automationHostSet := make(map[string]struct{})
+		for _, ap := range tlsApp.Automation.Policies {
+			for _, s := range ap.Subjects {
+				if _, ok := automationHostSet[s]; ok {
+					return nil, warnings, fmt.Errorf("hostname appears in more than one automation policy, making certificate management ambiguous: %s", s)
+				}
+				automationHostSet[s] = struct{}{}
+			}
+		}
+
+		// consolidate automation policies that are the exact same
+		tlsApp.Automation.Policies = consolidateAutomationPolicies(tlsApp.Automation.Policies)
+	}
+
+	return tlsApp, warnings, nil
+}
+
+// newBaseAutomationPolicy returns a new TLS automation policy that gets
+// its values from the global options map. It should be used as the base
+// for any other automation policies. A nil policy (and no error) will be
+// returned if there are no default/global options. However, if always is
+// true, a non-nil value will always be returned (unless there is an error).
+func newBaseAutomationPolicy(options map[string]interface{}, warnings []caddyconfig.Warning, always bool) (*caddytls.AutomationPolicy, error) {
+	acmeCA, hasACMECA := options["acme_ca"]
+	acmeDNS, hasACMEDNS := options["acme_dns"]
+	acmeCARoot, hasACMECARoot := options["acme_ca_root"]
+	email, hasEmail := options["email"]
+	localCerts, hasLocalCerts := options["local_certs"]
+
+	hasGlobalAutomationOpts := hasACMECA || hasACMEDNS || hasACMECARoot || hasEmail || hasLocalCerts
+
+	// if there are no global options related to automation policies
+	// set, then we can just return right away
+	if !hasGlobalAutomationOpts {
+		if always {
+			return new(caddytls.AutomationPolicy), nil
+		}
+		return nil, nil
+	}
+
+	ap := new(caddytls.AutomationPolicy)
+
+	if localCerts != nil {
+		// internal issuer enabled trumps any ACME configurations; useful in testing
+		ap.IssuerRaw = caddyconfig.JSONModuleObject(caddytls.InternalIssuer{}, "module", "internal", &warnings)
+	} else {
+		if acmeCA == nil {
+			acmeCA = ""
+		}
+		if email == nil {
+			email = ""
+		}
+		mgr := caddytls.ACMEIssuer{
+			CA:    acmeCA.(string),
+			Email: email.(string),
+		}
+		if acmeDNS != nil {
+			provName := acmeDNS.(string)
+			dnsProvModule, err := caddy.GetModule("tls.dns." + provName)
+			if err != nil {
+				return nil, fmt.Errorf("getting DNS provider module named '%s': %v", provName, err)
+			}
+			mgr.Challenges = &caddytls.ChallengesConfig{
+				DNSRaw: caddyconfig.JSONModuleObject(dnsProvModule.New(), "provider", provName, &warnings),
+			}
+		}
+		if acmeCARoot != nil {
+			mgr.TrustedRootsPEMFiles = []string{acmeCARoot.(string)}
+		}
+		ap.IssuerRaw = caddyconfig.JSONModuleObject(mgr, "module", "acme", &warnings)
+	}
+
+	return ap, nil
+}
+
+// consolidateAutomationPolicies combines automation policies that are the same,
+// for a cleaner overall output.
+func consolidateAutomationPolicies(aps []*caddytls.AutomationPolicy) []*caddytls.AutomationPolicy {
+	for i := 0; i < len(aps); i++ {
+		for j := 0; j < len(aps); j++ {
+			if j == i {
+				continue
+			}
+
+			// if they're exactly equal in every way, just keep one of them
+			if reflect.DeepEqual(aps[i], aps[j]) {
+				aps = append(aps[:j], aps[j+1:]...)
+				i--
+				break
+			}
+
+			// if the policy is the same, we can keep just one, but we have
+			// to be careful which one we keep; if only one has any hostnames
+			// defined, then we need to keep the one without any hostnames,
+			// otherwise the one without any subjects (a catch-all) would be
+			// eaten up by the one with subjects; and if both have subjects, we
+			// need to combine their lists
+			if bytes.Equal(aps[i].IssuerRaw, aps[j].IssuerRaw) &&
+				bytes.Equal(aps[i].StorageRaw, aps[j].StorageRaw) &&
+				aps[i].MustStaple == aps[j].MustStaple &&
+				aps[i].KeyType == aps[j].KeyType &&
+				aps[i].OnDemand == aps[j].OnDemand &&
+				aps[i].RenewalWindowRatio == aps[j].RenewalWindowRatio &&
+				aps[i].ManageSync == aps[j].ManageSync {
+				if len(aps[i].Subjects) == 0 && len(aps[j].Subjects) > 0 {
+					aps = append(aps[:j], aps[j+1:]...)
+				} else if len(aps[i].Subjects) > 0 && len(aps[j].Subjects) == 0 {
+					aps = append(aps[:i], aps[i+1:]...)
+				} else {
+					aps[i].Subjects = append(aps[i].Subjects, aps[j].Subjects...)
+					aps = append(aps[:j], aps[j+1:]...)
+				}
+				i--
+				break
+			}
+		}
+	}
+
+	// ensure any catch-all policies go last
+	sort.SliceStable(aps, func(i, j int) bool {
+		return len(aps[i].Subjects) > len(aps[j].Subjects)
+	})
+
+	return aps
+}
diff --git a/modules/caddyhttp/autohttps.go b/modules/caddyhttp/autohttps.go
index dfd659f..1239abb 100644
--- a/modules/caddyhttp/autohttps.go
+++ b/modules/caddyhttp/autohttps.go
@@ -232,7 +232,7 @@ uniqueDomainsLoop:
 		// some names we've found might already have automation policies
 		// explicitly specified for them; we should exclude those from
 		// our hidden/implicit policy, since applying a name to more than
-		// one automation policy  would be confusing and an error
+		// one automation policy would be confusing and an error
 		if app.tlsApp.Automation != nil {
 			for _, ap := range app.tlsApp.Automation.Policies {
 				for _, apHost := range ap.Subjects {
diff --git a/modules/caddytls/automation.go b/modules/caddytls/automation.go
index d10a4c6..e91811d 100644
--- a/modules/caddytls/automation.go
+++ b/modules/caddytls/automation.go
@@ -23,6 +23,7 @@ import (
 	"github.com/caddyserver/caddy/v2"
 	"github.com/caddyserver/certmagic"
 	"github.com/go-acme/lego/v3/challenge"
+	"go.uber.org/zap"
 )
 
 // AutomationConfig designates configuration for the
@@ -131,31 +132,49 @@ func (ap *AutomationPolicy) provision(tlsApp *TLS) error {
 
 	var ond *certmagic.OnDemandConfig
 	if ap.OnDemand {
-		var onDemand *OnDemandConfig
-		if tlsApp.Automation != nil {
-			onDemand = tlsApp.Automation.OnDemand
-		}
-
 		ond = &certmagic.OnDemandConfig{
 			DecisionFunc: func(name string) error {
-				if onDemand != nil {
-					if onDemand.Ask != "" {
-						err := onDemandAskRequest(onDemand.Ask, name)
-						if err != nil {
-							return err
-						}
-					}
-					// check the rate limiter last because
-					// doing so makes a reservation
-					if !onDemandRateLimiter.Allow() {
-						return fmt.Errorf("on-demand rate limit exceeded")
+				// if an "ask" endpoint was defined, consult it first
+				if tlsApp.Automation != nil &&
+					tlsApp.Automation.OnDemand != nil &&
+					tlsApp.Automation.OnDemand.Ask != "" {
+					err := onDemandAskRequest(tlsApp.Automation.OnDemand.Ask, name)
+					if err != nil {
+						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
 			},
 		}
 	}
 
+	// if this automation policy has no Issuer defined, and
+	// none the subjects do not qualify for a public certificate,
+	// set the issuer to internal so that these names can all
+	// get certificates; critically, we can only do this if an
+	// issuer is not explictly configured AND if the list of
+	// subjects is non-empty
+	if ap.IssuerRaw == nil && len(ap.Subjects) > 0 {
+		var anyPublic bool
+		for _, s := range ap.Subjects {
+			if certmagic.SubjectQualifiesForPublicCert(s) {
+				anyPublic = true
+				break
+			}
+		}
+		if !anyPublic {
+			tlsApp.logger.Info("setting internal issuer for automation policy that has only internal subjects but no issuer configured",
+				zap.Strings("subjects", ap.Subjects))
+			ap.IssuerRaw = json.RawMessage(`{"module":"internal"}`)
+		}
+	}
+
+	// load and provision the issuer module
 	if ap.IssuerRaw != nil {
 		val, err := tlsApp.ctx.LoadModule(ap, "IssuerRaw")
 		if err != nil {
diff --git a/modules/caddytls/connpolicy.go b/modules/caddytls/connpolicy.go
index 7618db4..395c55a 100644
--- a/modules/caddytls/connpolicy.go
+++ b/modules/caddytls/connpolicy.go
@@ -173,7 +173,7 @@ func (p *ConnectionPolicy) buildStandardTLSConfig(ctx caddy.Context) error {
 			// TODO: I don't love how this works: we pre-build certmagic configs
 			// so that handshakes are faster. Unfortunately, certmagic configs are
 			// comprised of settings from both a TLS connection policy and a TLS
-			// automation policy. The only two fields (as of March 2020; v2 beta 16)
+			// automation policy. The only two fields (as of March 2020; v2 beta 17)
 			// of a certmagic config that come from the TLS connection policy are
 			// CertSelection and DefaultServerName, so an automation policy is what
 			// builds the base certmagic config. Since the pre-built config is
diff --git a/modules/caddytls/tls.go b/modules/caddytls/tls.go
index c927ce2..4fc0850 100644
--- a/modules/caddytls/tls.go
+++ b/modules/caddytls/tls.go
@@ -179,9 +179,17 @@ func (t *TLS) Validate() error {
 	if t.Automation != nil {
 		// ensure that host aren't repeated; since only the first
 		// automation policy is used, repeating a host in the lists
-		// isn't useful and is probably a mistake
+		// isn't useful and is probably a mistake; same for two
+		// catch-all/default policies
+		var hasDefault bool
 		hostSet := make(map[string]int)
 		for i, ap := range t.Automation.Policies {
+			if len(ap.Subjects) == 0 {
+				if hasDefault {
+					return fmt.Errorf("automation policy %d is the second policy that acts as default/catch-all, but will never be used", i)
+				}
+				hasDefault = true
+			}
 			for _, h := range ap.Subjects {
 				if first, ok := hostSet[h]; ok {
 					return fmt.Errorf("automation policy %d: cannot apply more than one automation policy to host: %s (first match in policy %d)", i, h, first)
@@ -301,7 +309,7 @@ func (t *TLS) AddAutomationPolicy(ap *AutomationPolicy) error {
 		// fewer names) exists, prioritize this new policy
 		if len(other.Subjects) < len(ap.Subjects) {
 			t.Automation.Policies = append(t.Automation.Policies[:i],
-				append([]*AutomationPolicy{ap}, t.Automation.Policies[i+1:]...)...)
+				append([]*AutomationPolicy{ap}, t.Automation.Policies[i:]...)...)
 			return nil
 		}
 	}
diff --git a/modules/filestorage/filestorage.go b/modules/filestorage/filestorage.go
index 0b2d79a..3552609 100644
--- a/modules/filestorage/filestorage.go
+++ b/modules/filestorage/filestorage.go
@@ -59,5 +59,8 @@ func (s *FileStorage) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 	return nil
 }
 
-// Interface guard
-var _ caddy.StorageConverter = (*FileStorage)(nil)
+// Interface guards
+var (
+	_ caddy.StorageConverter = (*FileStorage)(nil)
+	_ caddyfile.Unmarshaler  = (*FileStorage)(nil)
+)