tls/http: Fix auto-HTTPS logic w/rt default issuers (fixes #3164)

The comments in the code should explain the new logic thoroughly.
The basic problem for the issue was that we were overriding a catch-all
automation policy's explicitly-configured issuer with our own, for names
that we thought looked like public names. In other words, one could
configure an internal issuer for all names, but then our auto HTTPS
would create a new policy for public-looking names that uses the
default ACME issuer, because we assume public<==>ACME and
nonpublic<==>Internal, but that is not always the case. The new logic
still assumes nonpublic<==>Internal (on catch-all policies only), but
no longer assumes that public-looking names always use an ACME issuer.

Also fix a bug where HTTPPort and HTTPSPort from the HTTP app weren't
being carried through to ACME issuers properly. It required a bit of
refactoring.

2 files changed, 8 insertions, 8 deletions

diff --git a/modules/caddytls/automation.go b/modules/caddytls/automation.go
index e91811d..9476445 100644
--- a/modules/caddytls/automation.go
+++ b/modules/caddytls/automation.go
@@ -115,8 +115,8 @@ type AutomationPolicy struct {
 	storage certmagic.Storage
 }
 
-// provision converts ap into a CertMagic config.
-func (ap *AutomationPolicy) provision(tlsApp *TLS) error {
+// Provision sets up ap and builds its underlying CertMagic config.
+func (ap *AutomationPolicy) Provision(tlsApp *TLS) error {
 	// policy-specific storage implementation
 	if ap.StorageRaw != nil {
 		val, err := tlsApp.ctx.LoadModule(ap, "StorageRaw")
@@ -157,8 +157,8 @@ func (ap *AutomationPolicy) provision(tlsApp *TLS) error {
 	// 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
+	// issuer is not explictly configured (IssuerRaw, vs. just
+	// Issuer) 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 {
@@ -174,7 +174,7 @@ func (ap *AutomationPolicy) provision(tlsApp *TLS) error {
 		}
 	}
 
-	// load and provision the issuer module
+	// load and provision any explicitly-configured issuer module
 	if ap.IssuerRaw != nil {
 		val, err := tlsApp.ctx.LoadModule(ap, "IssuerRaw")
 		if err != nil {
diff --git a/modules/caddytls/tls.go b/modules/caddytls/tls.go
index 4fc0850..076e017 100644
--- a/modules/caddytls/tls.go
+++ b/modules/caddytls/tls.go
@@ -94,12 +94,12 @@ func (t *TLS) Provision(ctx caddy.Context) error {
 		t.Automation = new(AutomationConfig)
 	}
 	t.Automation.defaultAutomationPolicy = new(AutomationPolicy)
-	err := t.Automation.defaultAutomationPolicy.provision(t)
+	err := t.Automation.defaultAutomationPolicy.Provision(t)
 	if err != nil {
 		return fmt.Errorf("provisioning default automation policy: %v", err)
 	}
 	for i, ap := range t.Automation.Policies {
-		err := ap.provision(t)
+		err := ap.Provision(t)
 		if err != nil {
 			return fmt.Errorf("provisioning automation policy %d: %v", i, err)
 		}
@@ -300,7 +300,7 @@ func (t *TLS) AddAutomationPolicy(ap *AutomationPolicy) error {
 	if t.Automation == nil {
 		t.Automation = new(AutomationConfig)
 	}
-	err := ap.provision(t)
+	err := ap.Provision(t)
 	if err != nil {
 		return err
 	}