From d68cff8eb6211be10fc79d3e8d469562420b78cd Mon Sep 17 00:00:00 2001
From: Matthew Holt <mholt@users.noreply.github.com>
Date: Tue, 19 Jan 2021 14:16:06 -0700
Subject: [PATCH] httpcaddyfile: Skip TLS APs for HTTP-only hosts (fix #3977)

This is probably an invasive change, but existing tests continue to pass.
It seems to make sense this way. There is likely an edge case I haven't
considered.
---
 caddyconfig/httpcaddyfile/httptype.go         | 44 +++++++++++++-----
 caddyconfig/httpcaddyfile/tlsapp.go           |  9 ++++
 .../caddyfile_adapt/http_only_hostnames.txt   | 45 +++++++++++++++++++
 3 files changed, 88 insertions(+), 10 deletions(-)
 create mode 100644 caddytest/integration/caddyfile_adapt/http_only_hostnames.txt

diff --git a/caddyconfig/httpcaddyfile/httptype.go b/caddyconfig/httpcaddyfile/httptype.go
index f16a0dfb..c6225df2 100644
--- a/caddyconfig/httpcaddyfile/httptype.go
+++ b/caddyconfig/httpcaddyfile/httptype.go
@@ -409,7 +409,7 @@ func (st *ServerType) serversFromPairings(
 			var iLongestHost, jLongestHost string
 			var iWildcardHost, jWildcardHost bool
 			for _, addr := range p.serverBlocks[i].keys {
-				if strings.Contains(addr.Host, "*.") {
+				if strings.Contains(addr.Host, "*") {
 					iWildcardHost = true
 				}
 				if specificity(addr.Host) > specificity(iLongestHost) {
@@ -420,7 +420,7 @@ func (st *ServerType) serversFromPairings(
 				}
 			}
 			for _, addr := range p.serverBlocks[j].keys {
-				if strings.Contains(addr.Host, "*.") {
+				if strings.Contains(addr.Host, "*") {
 					jWildcardHost = true
 				}
 				if specificity(addr.Host) > specificity(jLongestHost) {
@@ -500,16 +500,20 @@ func (st *ServerType) serversFromPairings(
 			}
 
 			for _, addr := range sblock.keys {
-				// exclude any hosts that were defined explicitly with "http://"
-				// in the key from automated cert management (issue #2998)
-				if addr.Scheme == "http" && addr.Host != "" {
-					if srv.AutoHTTPS == nil {
-						srv.AutoHTTPS = new(caddyhttp.AutoHTTPSConfig)
-					}
-					if !sliceContains(srv.AutoHTTPS.Skip, addr.Host) {
-						srv.AutoHTTPS.Skip = append(srv.AutoHTTPS.Skip, addr.Host)
+				// if server only uses HTTPS port, auto-HTTPS will not apply
+				if listenersUseAnyPortOtherThan(srv.Listen, httpPort) {
+					// exclude any hosts that were defined explicitly with "http://"
+					// in the key from automated cert management (issue #2998)
+					if addr.Scheme == "http" && addr.Host != "" {
+						if srv.AutoHTTPS == nil {
+							srv.AutoHTTPS = new(caddyhttp.AutoHTTPSConfig)
+						}
+						if !sliceContains(srv.AutoHTTPS.Skip, addr.Host) {
+							srv.AutoHTTPS.Skip = append(srv.AutoHTTPS.Skip, addr.Host)
+						}
 					}
 				}
+
 				// we'll need to remember if the address qualifies for auto-HTTPS, so we
 				// can add a TLS conn policy if necessary
 				if addr.Scheme == "https" ||
@@ -1183,6 +1187,26 @@ func sliceContains(haystack []string, needle string) bool {
 	return false
 }
 
+// listenersUseAnyPortOtherThan returns true if there are any
+// listeners in addresses that use a port which is not otherPort.
+// Mostly borrowed from unexported method in caddyhttp package.
+func listenersUseAnyPortOtherThan(addresses []string, otherPort string) bool {
+	otherPortInt, err := strconv.Atoi(otherPort)
+	if err != nil {
+		return false
+	}
+	for _, lnAddr := range addresses {
+		laddrs, err := caddy.ParseNetworkAddress(lnAddr)
+		if err != nil {
+			continue
+		}
+		if uint(otherPortInt) > laddrs.EndPort || uint(otherPortInt) < laddrs.StartPort {
+			return true
+		}
+	}
+	return false
+}
+
 // specificity returns len(s) minus any wildcards (*) and
 // placeholders ({...}). Basically, it's a length count
 // that penalizes the use of wildcards and placeholders.
diff --git a/caddyconfig/httpcaddyfile/tlsapp.go b/caddyconfig/httpcaddyfile/tlsapp.go
index 10b5e7d5..dbf3cc71 100644
--- a/caddyconfig/httpcaddyfile/tlsapp.go
+++ b/caddyconfig/httpcaddyfile/tlsapp.go
@@ -40,6 +40,10 @@ func (st ServerType) buildTLSApp(
 	tlsApp := &caddytls.TLS{CertificatesRaw: make(caddy.ModuleMap)}
 	var certLoaders []caddytls.CertificateLoader
 
+	httpPort := strconv.Itoa(caddyhttp.DefaultHTTPPort)
+	if hp, ok := options["http_port"].(int); ok {
+		httpPort = strconv.Itoa(hp)
+	}
 	httpsPort := strconv.Itoa(caddyhttp.DefaultHTTPSPort)
 	if hsp, ok := options["https_port"].(int); ok {
 		httpsPort = strconv.Itoa(hsp)
@@ -91,6 +95,11 @@ func (st ServerType) buildTLSApp(
 	}
 
 	for _, p := range pairings {
+		// avoid setting up TLS automation policies for a server that is HTTP-only
+		if !listenersUseAnyPortOtherThan(p.addresses, httpPort) {
+			continue
+		}
+
 		for _, sblock := range p.serverBlocks {
 			// get values that populate an automation policy for this block
 			ap, err := newBaseAutomationPolicy(options, warnings, true)
diff --git a/caddytest/integration/caddyfile_adapt/http_only_hostnames.txt b/caddytest/integration/caddyfile_adapt/http_only_hostnames.txt
new file mode 100644
index 00000000..d867e166
--- /dev/null
+++ b/caddytest/integration/caddyfile_adapt/http_only_hostnames.txt
@@ -0,0 +1,45 @@
+# https://github.com/caddyserver/caddy/issues/3977
+http://* {
+	respond "Hello, world!"
+}
+----------
+{
+	"apps": {
+		"http": {
+			"servers": {
+				"srv0": {
+					"listen": [
+						":80"
+					],
+					"routes": [
+						{
+							"match": [
+								{
+									"host": [
+										"*"
+									]
+								}
+							],
+							"handle": [
+								{
+									"handler": "subroute",
+									"routes": [
+										{
+											"handle": [
+												{
+													"body": "Hello, world!",
+													"handler": "static_response"
+												}
+											]
+										}
+									]
+								}
+							],
+							"terminal": true
+						}
+					]
+				}
+			}
+		}
+	}
+}
\ No newline at end of file