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

Holy heck this was complicated

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 {

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 {

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

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

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