httpserver: Prevent TLS client authentication bypass in 3 ways (#2099)

- Introduce StrictHostMatching mode for sites that require clientauth
- Error if QUIC is enabled whilst TLS clientauth is configured
  (Our QUIC implementation does not yet support TLS clientauth, but
  maybe it will in the future - fixes #2095)
- Error if one but not all TLS configs for the same hostname have a
  different ClientAuth CA pool

4 files changed, 49 insertions, 3 deletions

diff --git a/caddyhttp/httpserver/plugin.go b/caddyhttp/httpserver/plugin.go
index ad2d87388..1508b41ec 100644
--- a/caddyhttp/httpserver/plugin.go
+++ b/caddyhttp/httpserver/plugin.go
@@ -15,6 +15,7 @@
 package httpserver
 
 import (
+	"crypto/tls"
 	"flag"
 	"fmt"
 	"log"
@@ -207,8 +208,13 @@ func (h *httpContext) InspectServerBlocks(sourceFile string, serverBlocks []cadd
 // MakeServers uses the newly-created siteConfigs to
 // create and return a list of server instances.
 func (h *httpContext) MakeServers() ([]caddy.Server, error) {
-	// make sure TLS is disabled for explicitly-HTTP sites
-	// (necessary when HTTP address shares a block containing tls)
+	// Iterate each site configuration and make sure that:
+	// 1) TLS is disabled for explicitly-HTTP sites (necessary
+	//    when an HTTP address shares a block containing tls)
+	// 2) if QUIC is enabled, TLS ClientAuth is not, because
+	//    currently, QUIC does not support ClientAuth (TODO:
+	//    revisit this when our QUIC implementation supports it)
+	// 3) if TLS ClientAuth is used, StrictHostMatching is on
 	for _, cfg := range h.siteConfigs {
 		if !cfg.TLS.Enabled {
 			continue
@@ -230,6 +236,17 @@ func (h *httpContext) MakeServers() ([]caddy.Server, error) {
 			// instead of 443 because it doesn't know about TLS.
 			cfg.Addr.Port = HTTPSPort
 		}
+		if cfg.TLS.ClientAuth != tls.NoClientCert {
+			if QUIC {
+				return nil, fmt.Errorf("cannot enable TLS client authentication with QUIC, because QUIC does not yet support it")
+			}
+			// this must be enabled so that a client cannot connect
+			// using SNI for another site on this listener that
+			// does NOT require ClientAuth, and then send HTTP
+			// requests with the Host header of this site which DOES
+			// require client auth, thus bypassing it...
+			cfg.StrictHostMatching = true
+		}
 	}
 
 	// we must map (group) each config to a bind address
@@ -556,7 +573,6 @@ var directives = []string{
 	"minify",       // github.com/hacdias/caddy-minify
 	"ipfilter",     // github.com/pyed/ipfilter
 	"ratelimit",    // github.com/xuqingfeng/caddy-rate-limit
-	"search",       // github.com/pedronasser/caddy-search
 	"expires",      // github.com/epicagency/caddy-expires
 	"forwardproxy", // github.com/caddyserver/forwardproxy
 	"basicauth",
diff --git a/caddyhttp/httpserver/server.go b/caddyhttp/httpserver/server.go
index a98a8fdf3..8028fce85 100644
--- a/caddyhttp/httpserver/server.go
+++ b/caddyhttp/httpserver/server.go
@@ -416,6 +416,18 @@ func (s *Server) serveHTTP(w http.ResponseWriter, r *http.Request) (int, error)
 		r.URL = trimPathPrefix(r.URL, pathPrefix)
 	}
 
+	// enforce strict host matching, which ensures that the SNI
+	// value (if any), matches the Host header; essential for
+	// sites that rely on TLS ClientAuth sharing a port with
+	// sites that do not - if mismatched, close the connection
+	if vhost.StrictHostMatching && r.TLS != nil &&
+		strings.ToLower(r.TLS.ServerName) != strings.ToLower(hostname) {
+		r.Close = true
+		log.Printf("[ERROR] %s - strict host matching: SNI (%s) and HTTP Host (%s) values differ",
+			vhost.Addr, r.TLS.ServerName, hostname)
+		return http.StatusForbidden, nil
+	}
+
 	return vhost.middlewareChain.ServeHTTP(w, r)
 }
 
diff --git a/caddyhttp/httpserver/siteconfig.go b/caddyhttp/httpserver/siteconfig.go
index 1cd9a0504..3b29f911a 100644
--- a/caddyhttp/httpserver/siteconfig.go
+++ b/caddyhttp/httpserver/siteconfig.go
@@ -36,6 +36,16 @@ type SiteConfig struct {
 	// TLS configuration
 	TLS *caddytls.Config
 
+	// If true, the Host header in the HTTP request must
+	// match the SNI value in the TLS handshake (if any).
+	// This should be enabled whenever a site relies on
+	// TLS client authentication, for example; or any time
+	// you want to enforce that THIS site's TLS config
+	// is used and not the TLS config of any other site
+	// on the same listener. TODO: Check how relevant this
+	// is with TLS 1.3.
+	StrictHostMatching bool
+
 	// Uncompiled middleware stack
 	middleware []Middleware
 
diff --git a/caddytls/config.go b/caddytls/config.go
index 808ed5271..80f1633f0 100644
--- a/caddytls/config.go
+++ b/caddytls/config.go
@@ -513,6 +513,14 @@ func assertConfigsCompatible(cfg1, cfg2 *Config) error {
 	if c1.ClientAuth != c2.ClientAuth {
 		return fmt.Errorf("client authentication policy mismatch")
 	}
+	if c1.ClientAuth != tls.NoClientCert && c2.ClientAuth != tls.NoClientCert && c1.ClientCAs != c2.ClientCAs {
+		// Two hosts defined on the same listener are not compatible if they
+		// have ClientAuth enabled, because there's no guarantee beyond the
+		// hostname which config will be used (because SNI only has server name).
+		// To prevent clients from bypassing authentication, require that
+		// ClientAuth be configured in an unambiguous manner.
+		return fmt.Errorf("multiple hosts requiring client authentication ambiguously configured")
+	}
 
 	return nil
 }