From d89ad2fd5ba8de6dee0ff420458b634431da9b89 Mon Sep 17 00:00:00 2001
From: Matt Holt <mholt@users.noreply.github.com>
Date: Thu, 9 Apr 2020 12:39:05 -0600
Subject: caddytls: Fix for TLS conn policy being applied to HTTP-only servers
 (#3243)

* httpcaddyfile: Don't add TLS policy to HTTP-only server (#3193, #3223)

* Account for HTTP port

* Add integration test written by @sarge
---
 caddyconfig/httpcaddyfile/httptype.go | 14 ++++++++++--
 caddytest/integration/sni_test.go     | 43 +++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/caddyconfig/httpcaddyfile/httptype.go b/caddyconfig/httpcaddyfile/httptype.go
index b7d16d8..2d8accc 100644
--- a/caddyconfig/httpcaddyfile/httptype.go
+++ b/caddyconfig/httpcaddyfile/httptype.go
@@ -332,6 +332,11 @@ func (st *ServerType) serversFromPairings(
 	servers := make(map[string]*caddyhttp.Server)
 	defaultSNI := tryString(options["default_sni"], warnings)
 
+	httpPort := strconv.Itoa(caddyhttp.DefaultHTTPPort)
+	if hp, ok := options["http_port"].(int); ok {
+		httpPort = strconv.Itoa(hp)
+	}
+
 	for i, p := range pairings {
 		srv := &caddyhttp.Server{
 			Listen: p.addresses,
@@ -369,7 +374,7 @@ func (st *ServerType) serversFromPairings(
 			return specificity(iLongestHost) > specificity(jLongestHost)
 		})
 
-		var hasCatchAllTLSConnPolicy bool
+		var hasCatchAllTLSConnPolicy, usesTLS bool
 
 		// create a subroute for each site in the server block
 		for _, sblock := range p.serverBlocks {
@@ -419,6 +424,9 @@ func (st *ServerType) serversFromPairings(
 						srv.AutoHTTPS.Skip = append(srv.AutoHTTPS.Skip, addr.Host)
 					}
 				}
+				if addr.Scheme != "http" && addr.Host != "" && addr.Port != httpPort {
+					usesTLS = true
+				}
 			}
 
 			// set up each handler directive, making sure to honor directive order
@@ -481,7 +489,9 @@ func (st *ServerType) serversFromPairings(
 		// 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 != "") {
+		if usesTLS &&
+			!hasCatchAllTLSConnPolicy &&
+			(len(srv.TLSConnPolicies) > 0 || defaultSNI != "") {
 			srv.TLSConnPolicies = append(srv.TLSConnPolicies, &caddytls.ConnectionPolicy{DefaultSNI: defaultSNI})
 		}
 
diff --git a/caddytest/integration/sni_test.go b/caddytest/integration/sni_test.go
index e48346d..f26131e 100644
--- a/caddytest/integration/sni_test.go
+++ b/caddytest/integration/sni_test.go
@@ -272,3 +272,46 @@ func TestDefaultSNIWithPortMappingOnly(t *testing.T) {
 	// makes a request with no sni
 	caddytest.AssertGetResponse(t, "https://127.0.0.1:9443/version", 200, "hello from a")
 }
+
+func TestHttpOnlyOnDomainWithSNI(t *testing.T) {
+	caddytest.AssertAdapt(t, `
+	{
+		default_sni a.caddy.localhost
+	}
+	:80 {
+		respond /version 200 {
+			body "hello from localhost"
+		}
+	}
+	`, "caddyfile", `{
+	"apps": {
+		"http": {
+			"servers": {
+				"srv0": {
+					"listen": [
+						":80"
+					],
+					"routes": [
+						{
+							"match": [
+								{
+									"path": [
+										"/version"
+									]
+								}
+							],
+							"handle": [
+								{
+									"body": "hello from localhost",
+									"handler": "static_response",
+									"status_code": 200
+								}
+							]
+						}
+					]
+				}
+			}
+		}
+	}
+}`)
+}
-- 
cgit v1.2.3