caddyhttp: Fix default SNI for default conn policy (#3141)

* add integration tests

* removed SNI test

* remove integration test condition

* minor edit

* fix sni when using static certificates

Co-authored-by: Matt Holt <mholt@users.noreply.github.com>

7 files changed, 458 insertions, 2 deletions

diff --git a/azure-pipelines.yml b/azure-pipelines.yml
index 21be1f1..1829cde 100644
--- a/azure-pipelines.yml
+++ b/azure-pipelines.yml
@@ -131,6 +131,15 @@ jobs:
     displayName: Run tests
 
   - script: |
+      set -e
+      cmd/caddy/caddy start
+      go test -v -count=1 ./caddytest/...
+      cmd/caddy/caddy stop
+    workingDirectory: '$(modulePath)'
+    continueOnError: false
+    displayName: Run Integration tests
+
+  - script: |
       mkdir coverage
       gocov convert cover-profile.out > coverage/coverage.json
       # Because Windows doesn't work with input redirection like *nix, but output redirection works.
diff --git a/caddyconfig/httpcaddyfile/httptype.go b/caddyconfig/httpcaddyfile/httptype.go
index 96f2bb0..a8df28c 100644
--- a/caddyconfig/httpcaddyfile/httptype.go
+++ b/caddyconfig/httpcaddyfile/httptype.go
@@ -450,6 +450,8 @@ func (st *ServerType) serversFromPairings(
 ) (map[string]*caddyhttp.Server, error) {
 	servers := make(map[string]*caddyhttp.Server)
 
+	defaultSNI := tryString(options["default_sni"], warnings)
+
 	for i, p := range pairings {
 		srv := &caddyhttp.Server{
 			Listen: p.addresses,
@@ -499,7 +501,6 @@ func (st *ServerType) serversFromPairings(
 			}
 
 			// tls: connection policies and toggle auto HTTPS
-			defaultSNI := tryString(options["default_sni"], warnings)
 			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
@@ -614,7 +615,7 @@ func (st *ServerType) serversFromPairings(
 		// important that it goes at the end) - see issue #3004:
 		// https://github.com/caddyserver/caddy/issues/3004
 		if len(srv.TLSConnPolicies) > 0 && !hasCatchAllTLSConnPolicy {
-			srv.TLSConnPolicies = append(srv.TLSConnPolicies, new(caddytls.ConnectionPolicy))
+			srv.TLSConnPolicies = append(srv.TLSConnPolicies, &caddytls.ConnectionPolicy{DefaultSNI: defaultSNI})
 		}
 
 		srv.Routes = consolidateRoutes(srv.Routes)
diff --git a/caddytest/caddy.localhost.crt b/caddytest/caddy.localhost.crt
new file mode 100644
index 0000000..a3a2f4c
--- /dev/null
+++ b/caddytest/caddy.localhost.crt
@@ -0,0 +1,23 @@
+-----BEGIN CERTIFICATE-----
+MIID5zCCAs8CFFmAAFKV79uhzxc5qXbUw3oBNsYXMA0GCSqGSIb3DQEBCwUAMIGv
+MQswCQYDVQQGEwJVUzELMAkGA1UECAwCTlkxGzAZBgNVBAoMEkxvY2FsIERldmVs
+b3BlbWVudDEbMBkGA1UEBwwSTG9jYWwgRGV2ZWxvcGVtZW50MRowGAYDVQQDDBEq
+LmNhZGR5LmxvY2FsaG9zdDEbMBkGA1UECwwSTG9jYWwgRGV2ZWxvcGVtZW50MSAw
+HgYJKoZIhvcNAQkBFhFhZG1pbkBjYWRkeS5sb2NhbDAeFw0yMDAzMDIwODAxMTZa
+Fw0zMDAyMjgwODAxMTZaMIGvMQswCQYDVQQGEwJVUzELMAkGA1UECAwCTlkxGzAZ
+BgNVBAoMEkxvY2FsIERldmVsb3BlbWVudDEbMBkGA1UEBwwSTG9jYWwgRGV2ZWxv
+cGVtZW50MRowGAYDVQQDDBEqLmNhZGR5LmxvY2FsaG9zdDEbMBkGA1UECwwSTG9j
+YWwgRGV2ZWxvcGVtZW50MSAwHgYJKoZIhvcNAQkBFhFhZG1pbkBjYWRkeS5sb2Nh
+bDCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAJngfeirQkWaU8ihgIC5
+SKpRQX/3koRjljDK/oCbhLs+wg592kIwVv06l7+mn7NSaNBloabjuA1GqyLRsNLL
+ptrv0HvXa5qLx28+icsb2Ny3dJnQaj9w9PwjxQ1qZqEJfWRH1D8Vz9AmB+QSV/Gu
+8e8alGFewlYZVfH1kbxoTT6QorF37TeA3bh1fgKFtzsGYKswcaZNdDBBHzLunCKZ
+HU6U6L45hm+yLADj3mmDLafUeiVOt6MRLLoSD1eLRVSXGrNo+brJ87zkZntI9+W1
+JxOBoXtZCwka7k2DlAtLihsrmBZA2ZC9yVeu/SQy3qb3iCNnTFTCyAnWeTCr6Tcq
+6w8CAwEAATANBgkqhkiG9w0BAQsFAAOCAQEAOWfXqpAmD4C3wGiMeZAeaaS4hDAR
++JmN+avPDA6F6Bq7DB4NJuIwVUlaDL2s07w5VJJtW52aZVKoBlgHR5yG/XUli6J7
+YUJRmdQJvHUSu26cmKvyoOaTrEYbmvtGICWtZc8uTlMf9wQZbJA4KyxTgEQJDXsZ
+B2XFe+wVdhAgEpobYDROi+l/p8TL5z3U24LpwVTcJy5sEZVv7Wfs886IyxU8ORt8
+VZNcDiH6V53OIGeiufIhia/mPe6jbLntfGZfIFxtCcow4IA/lTy1ned7K5fmvNNb
+ZilxOQUk+wVK8genjdrZVAnAxsYLHJIb5yf9O7rr6fWciVMF3a0k5uNK1w==
+-----END CERTIFICATE-----
diff --git a/caddytest/caddy.localhost.key b/caddytest/caddy.localhost.key
new file mode 100644
index 0000000..d85cd40
--- /dev/null
+++ b/caddytest/caddy.localhost.key
@@ -0,0 +1,27 @@
+-----BEGIN RSA PRIVATE KEY-----
+MIIEogIBAAKCAQEAmeB96KtCRZpTyKGAgLlIqlFBf/eShGOWMMr+gJuEuz7CDn3a
+QjBW/TqXv6afs1Jo0GWhpuO4DUarItGw0sum2u/Qe9drmovHbz6JyxvY3Ld0mdBq
+P3D0/CPFDWpmoQl9ZEfUPxXP0CYH5BJX8a7x7xqUYV7CVhlV8fWRvGhNPpCisXft
+N4DduHV+AoW3OwZgqzBxpk10MEEfMu6cIpkdTpTovjmGb7IsAOPeaYMtp9R6JU63
+oxEsuhIPV4tFVJcas2j5usnzvORme0j35bUnE4Ghe1kLCRruTYOUC0uKGyuYFkDZ
+kL3JV679JDLepveII2dMVMLICdZ5MKvpNyrrDwIDAQABAoIBAFcPK01zb6hfm12c
++k5aBiHOnUdgc/YRPg1XHEz5MEycQkDetZjTLrRQ7UBSbnKPgpu9lIsOtbhVLkgh
+6XAqJroiCou2oruqr+hhsqZGmBiwdvj7cNF6ADGTr05az7v22YneFdinZ481pStF
+sZocx+bm2+KHMV5zMSwXKyA0xtdJLxs2yklniDBxSZRppgppq1pDPprP5DkgKPfe
+3ekUmbQd5bHmivhW8ItbJLuf82XSsMBZ9ZhKiKIlWlbKAgiSV3SqnUQb5fi7l8hG
+yYZxbuCUIGFwKmEpUBBt/nyxrOlMiNtDh9JhrPmijTV3slq70pCLwLL/Ai2aeear
+EVA5VhkCgYEAyAmxfPqc2P7BsDAp67/sA7OEPso9qM4WyuWiVdlX2gb9TLNLYbPX
+Kk/UmpAIVzpoTAGY5Zp3wkvdD/ou8uUQsE8ioNn4S1a4G9XURH1wVhcEbUiAKI1S
+QVBH9B/Pj3eIp5OTKwob0Wj7DNdxoH7ed/Eok0EaTWzOA8pCWADKv/MCgYEAxOzY
+YsX7Nl+eyZr2+9unKyeAK/D1DCT/o99UUAHx72/xaBVP/06cfzpvKBNcF9iYc+fq
+R1yIUIrDRoSmYKBq+Kb3+nOg1nrqih/NBTokbTiI4Q+/30OQt0Al1e7y9iNKqV8H
+jYZItzluGNrWKedZbATwBwbVCY2jnNl6RMDnS3UCgYBxj3cwQUHLuoyQjjcuO80r
+qLzZvIxWiXDNDKIk5HcIMlGYOmz/8U2kGp/SgxQJGQJeq8V2C0QTjGfaCyieAcaA
+oNxCvptDgd6RBsoze5bLeNOtiqwe2WOp6n5+q5R0mOJ+Z7vzghCayGNFPgWmnH+F
+TeW/+wSIkc0+v5L8TK7NWwKBgBrlWlyLO9deUfqpHqihhICBYaEexOlGuF+yZfqT
+eW7BdFBJ8OYm33sFCR+JHV/oZlIWT8o1Wizd9vPPtEWoQ1P4wg/D8Si6GwSIeWEI
+YudD/HX4x7T/rmlI6qIAg9CYW18sqoRq3c2gm2fro6qPfYgiWIItLbWjUcBfd7Ki
+QjTtAoGARKdRv3jMWL84rlEx1nBRgL3pe9Dt+Uxzde2xT3ZeF+5Hp9NfU01qE6M6
+1I6H64smqpetlsXmCEVKwBemP3pJa6avLKgIYiQvHAD/v4rs9mqgy1RTqtYyGNhR
+1A/6dKkbiZ6wzePLLPasXVZxSKEviXf5gJooqumQVSVhCswyCZ0=
+-----END RSA PRIVATE KEY-----
diff --git a/caddytest/caddytest.go b/caddytest/caddytest.go
new file mode 100644
index 0000000..04b65ba
--- /dev/null
+++ b/caddytest/caddytest.go
@@ -0,0 +1,272 @@
+package caddytest
+
+import (
+	"bytes"
+	"context"
+	"crypto/tls"
+	"encoding/json"
+	"errors"
+	"fmt"
+	"io/ioutil"
+	"log"
+	"net"
+	"net/http"
+	"os"
+	"path"
+	"regexp"
+	"runtime"
+	"strings"
+	"testing"
+	"time"
+)
+
+// Defaults store any configuration required to make the tests run
+type Defaults struct {
+	// Port we expect caddy to listening on
+	AdminPort int
+	// Certificates we expect to be loaded before attempting to run the tests
+	Certifcates []string
+}
+
+// Default testing values
+var Default = Defaults{
+	AdminPort:   2019,
+	Certifcates: []string{"/caddy.localhost.crt", "/caddy.localhost.key"},
+}
+
+var (
+	matchKey  = regexp.MustCompile(`(/[\w\d\.]+\.key)`)
+	matchCert = regexp.MustCompile(`(/[\w\d\.]+\.crt)`)
+)
+
+type configLoadError struct {
+	Response string
+}
+
+func (e configLoadError) Error() string { return e.Response }
+
+// InitServer this will configure the server with a configurion of a specific
+// type. The configType must be either "json" or the adapter type.
+func InitServer(t *testing.T, rawConfig string, configType string) {
+	if err := initServer(t, rawConfig, configType); errors.Is(err, &configLoadError{}) {
+		t.Logf("failed to load config: %s", err)
+		t.Fail()
+	}
+}
+
+// InitServer this will configure the server with a configurion of a specific
+// type. The configType must be either "json" or the adapter type.
+func initServer(t *testing.T, rawConfig string, configType string) error {
+
+	err := validateTestPrerequisites()
+	if err != nil {
+		t.Skipf("skipping tests as failed integration prerequisites. %s", err)
+		return nil
+	}
+
+	t.Cleanup(func() {
+		if t.Failed() {
+			res, err := http.Get(fmt.Sprintf("http://localhost:%d/config/", Default.AdminPort))
+			if err != nil {
+				t.Log("unable to read the current config")
+			}
+			defer res.Body.Close()
+			body, err := ioutil.ReadAll(res.Body)
+
+			var out bytes.Buffer
+			json.Indent(&out, body, "", "  ")
+			t.Logf("----------- failed with config -----------\n%s", out.String())
+		}
+	})
+
+	rawConfig = prependCaddyFilePath(rawConfig)
+	client := &http.Client{
+		Timeout: time.Second * 2,
+	}
+	req, err := http.NewRequest("POST", fmt.Sprintf("http://localhost:%d/load", Default.AdminPort), strings.NewReader(rawConfig))
+	if err != nil {
+		t.Errorf("failed to create request. %s", err)
+		return err
+	}
+
+	if configType == "json" {
+		req.Header.Add("Content-Type", "application/json")
+	} else {
+		req.Header.Add("Content-Type", "text/"+configType)
+	}
+
+	res, err := client.Do(req)
+	if err != nil {
+		t.Errorf("unable to contact caddy server. %s", err)
+		return err
+	}
+	defer res.Body.Close()
+	body, err := ioutil.ReadAll(res.Body)
+	if err != nil {
+		t.Errorf("unable to read response. %s", err)
+		return err
+	}
+
+	if res.StatusCode != 200 {
+		return configLoadError{Response: string(body)}
+	}
+
+	return nil
+}
+
+var hasValidated bool
+var arePrerequisitesValid bool
+
+func validateTestPrerequisites() error {
+
+	if hasValidated {
+		if !arePrerequisitesValid {
+			return errors.New("caddy integration prerequisites failed. see first error")
+		}
+		return nil
+	}
+
+	hasValidated = true
+	arePrerequisitesValid = false
+
+	// check certificates are found
+	for _, certName := range Default.Certifcates {
+		if _, err := os.Stat(getIntegrationDir() + certName); os.IsNotExist(err) {
+			return fmt.Errorf("caddy integration test certificates (%s) not found", certName)
+		}
+	}
+
+	// assert that caddy is running
+	client := &http.Client{
+		Timeout: time.Second * 2,
+	}
+	_, err := client.Get(fmt.Sprintf("http://localhost:%d/config/", Default.AdminPort))
+	if err != nil {
+		return errors.New("caddy integration test caddy server not running. Expected to be listening on localhost:2019")
+	}
+
+	arePrerequisitesValid = true
+	return nil
+}
+
+func getIntegrationDir() string {
+
+	_, filename, _, ok := runtime.Caller(1)
+	if !ok {
+		panic("unable to determine the current file path")
+	}
+
+	return path.Dir(filename)
+}
+
+// use the convention to replace /[certificatename].[crt|key] with the full path
+// this helps reduce the noise in test configurations and also allow this
+// to run in any path
+func prependCaddyFilePath(rawConfig string) string {
+	r := matchKey.ReplaceAllString(rawConfig, getIntegrationDir()+"$1")
+	r = matchCert.ReplaceAllString(r, getIntegrationDir()+"$1")
+	return r
+}
+
+// creates a testing transport that forces call dialing connections to happen locally
+func createTestingTransport() *http.Transport {
+
+	dialer := net.Dialer{
+		Timeout:   5 * time.Second,
+		KeepAlive: 5 * time.Second,
+		DualStack: true,
+	}
+
+	dialContext := func(ctx context.Context, network, addr string) (net.Conn, error) {
+		parts := strings.Split(addr, ":")
+		destAddr := fmt.Sprintf("127.0.0.1:%s", parts[1])
+		log.Printf("caddytest: redirecting the dialer from %s to %s", addr, destAddr)
+		return dialer.DialContext(ctx, network, destAddr)
+	}
+
+	return &http.Transport{
+		Proxy:                 http.ProxyFromEnvironment,
+		DialContext:           dialContext,
+		ForceAttemptHTTP2:     true,
+		MaxIdleConns:          100,
+		IdleConnTimeout:       90 * time.Second,
+		TLSHandshakeTimeout:   5 * time.Second,
+		ExpectContinueTimeout: 1 * time.Second,
+		TLSClientConfig:       &tls.Config{InsecureSkipVerify: true},
+	}
+}
+
+// AssertLoadError will load a config and expect an error
+func AssertLoadError(t *testing.T, rawConfig string, configType string, expectedError string) {
+	err := initServer(t, rawConfig, configType)
+	if !strings.Contains(err.Error(), expectedError) {
+		t.Errorf("expected error \"%s\" but got \"%s\"", expectedError, err.Error())
+	}
+}
+
+// AssertGetResponse request a URI and assert the status code and the body contains a string
+func AssertGetResponse(t *testing.T, requestURI string, statusCode int, expectedBody string) (*http.Response, string) {
+	resp, body := AssertGetResponseBody(t, requestURI, statusCode)
+	if !strings.Contains(body, expectedBody) {
+		t.Errorf("expected response body \"%s\" but got \"%s\"", expectedBody, body)
+	}
+	return resp, string(body)
+}
+
+// AssertGetResponseBody request a URI and assert the status code matches
+func AssertGetResponseBody(t *testing.T, requestURI string, expectedStatusCode int) (*http.Response, string) {
+
+	client := &http.Client{
+		Transport: createTestingTransport(),
+	}
+
+	resp, err := client.Get(requestURI)
+	if err != nil {
+		t.Errorf("failed to call server %s", err)
+		return nil, ""
+	}
+
+	defer resp.Body.Close()
+
+	if expectedStatusCode != resp.StatusCode {
+		t.Errorf("expected status code: %d but got %d", expectedStatusCode, resp.StatusCode)
+	}
+
+	body, err := ioutil.ReadAll(resp.Body)
+	if err != nil {
+		t.Errorf("unable to read the response body %s", err)
+		return nil, ""
+	}
+
+	return resp, string(body)
+}
+
+// AssertRedirect makes a request and asserts the redirection happens
+func AssertRedirect(t *testing.T, requestURI string, expectedToLocation string, expectedStatusCode int) *http.Response {
+
+	redirectPolicyFunc := func(req *http.Request, via []*http.Request) error {
+		return http.ErrUseLastResponse
+	}
+
+	client := &http.Client{
+		CheckRedirect: redirectPolicyFunc,
+		Transport:     createTestingTransport(),
+	}
+
+	resp, err := client.Get(requestURI)
+	if err != nil {
+		t.Errorf("failed to call server %s", err)
+		return nil
+	}
+
+	if expectedStatusCode != resp.StatusCode {
+		t.Errorf("expected status code: %d but got %d", expectedStatusCode, resp.StatusCode)
+	}
+
+	loc, err := resp.Location()
+	if expectedToLocation != loc.String() {
+		t.Errorf("expected location: \"%s\" but got \"%s\"", expectedToLocation, loc.String())
+	}
+
+	return resp
+}
diff --git a/caddytest/caddytest_test.go b/caddytest/caddytest_test.go
new file mode 100644
index 0000000..a46867c
--- /dev/null
+++ b/caddytest/caddytest_test.go
@@ -0,0 +1,33 @@
+package caddytest
+
+import (
+	"strings"
+	"testing"
+)
+
+func TestReplaceCertificatePaths(t *testing.T) {
+	rawConfig := `a.caddy.localhost:9443 {
+		tls /caddy.localhost.crt /caddy.localhost.key {
+		}
+
+		redir / https://b.caddy.localhost:9443/version 301
+    
+		respond /version 200 {
+		  body "hello from a.caddy.localhost"
+		}	
+	  }`
+
+	r := prependCaddyFilePath(rawConfig)
+
+	if !strings.Contains(r, getIntegrationDir()+"/caddy.localhost.crt") {
+		t.Error("expected the /caddy.localhost.crt to be expanded to include the full path")
+	}
+
+	if !strings.Contains(r, getIntegrationDir()+"/caddy.localhost.key") {
+		t.Error("expected the /caddy.localhost.crt to be expanded to include the full path")
+	}
+
+	if !strings.Contains(r, "https://b.caddy.localhost:9443/version") {
+		t.Error("expected redirect uri to be unchanged")
+	}
+}
diff --git a/caddytest/integration/caddyfile_test.go b/caddytest/integration/caddyfile_test.go
new file mode 100644
index 0000000..33b4b2d
--- /dev/null
+++ b/caddytest/integration/caddyfile_test.go
@@ -0,0 +1,91 @@
+package integration
+
+import (
+	"testing"
+
+	"github.com/caddyserver/caddy/v2/caddytest"
+)
+
+func TestRespond(t *testing.T) {
+
+	// arrange
+	caddytest.InitServer(t, ` 
+  {
+    http_port     9080
+    https_port    9443
+  }
+  
+  localhost:9080 {
+    respond /version 200 {
+      body "hello from a.caddy.localhost"
+    }	
+    }
+  `, "caddyfile")
+
+	// act and assert
+	caddytest.AssertGetResponse(t, "http://localhost:9080/version", 200, "hello from a.caddy.localhost")
+}
+
+func TestRedirect(t *testing.T) {
+
+	// arrange
+	caddytest.InitServer(t, `
+  {
+    http_port     9080
+    https_port    9443
+  }
+  
+  localhost:9080 {
+    
+    redir / http://localhost:9080/hello 301
+    
+    respond /hello 200 {
+      body "hello from b"
+    }	
+    }
+  `, "caddyfile")
+
+	// act and assert
+	caddytest.AssertRedirect(t, "http://localhost:9080/", "http://localhost:9080/hello", 301)
+
+	// follow redirect
+	caddytest.AssertGetResponse(t, "http://localhost:9080/", 200, "hello from b")
+}
+
+func TestDuplicateHosts(t *testing.T) {
+
+	// act and assert
+	caddytest.AssertLoadError(t,
+		`
+    localhost:9080 {
+    }
+  
+    localhost:9080 { 
+    }
+    `,
+		"caddyfile",
+		"duplicate site address not allowed")
+}
+
+func TestDefaultSNI(t *testing.T) {
+
+	// arrange
+	caddytest.InitServer(t, ` 
+  {
+    http_port     9080
+    https_port    9443
+    default_sni   *.caddy.localhost
+  }
+  
+  127.0.0.1:9443 {
+    tls /caddy.localhost.crt /caddy.localhost.key {
+    }
+    respond /version 200 {
+      body "hello from a.caddy.localhost"
+    }	
+  }
+  `, "caddyfile")
+
+	// act and assert
+	caddytest.AssertGetResponse(t, "https://127.0.0.1:9443/version", 200, "hello from a.caddy.localhost")
+}