From 4fbdd232833bd776b078eb1fe512b7b577be68cd Mon Sep 17 00:00:00 2001 From: Daniel Santos Date: Tue, 25 Feb 2020 16:04:59 -0700 Subject: [PATCH] tls: Add insecure_disable_sni_matching subdirective (#3075) * Disable StrictHostMatching for single server configs * Add the insecure_disable_sni_matching directive * Do not override insecure_disable_sni_matching * Remove comment --- caddyhttp/httpserver/plugin.go | 7 ------- caddyhttp/httpserver/server.go | 3 ++- caddyhttp/httpserver/siteconfig.go | 10 ---------- caddytls/config.go | 12 ++++++++++++ caddytls/setup.go | 2 ++ 5 files changed, 16 insertions(+), 18 deletions(-) diff --git a/caddyhttp/httpserver/plugin.go b/caddyhttp/httpserver/plugin.go index 378e9cb8f..45cc8d0ea 100644 --- a/caddyhttp/httpserver/plugin.go +++ b/caddyhttp/httpserver/plugin.go @@ -251,7 +251,6 @@ func (h *httpContext) MakeServers() ([]caddy.Server, error) { // 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 var atLeastOneSiteLooksLikeProduction bool for _, cfg := range h.siteConfigs { // see if all the addresses (both sites and @@ -292,12 +291,6 @@ func (h *httpContext) MakeServers() ([]caddy.Server, error) { 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 } } diff --git a/caddyhttp/httpserver/server.go b/caddyhttp/httpserver/server.go index 7a69565c6..3dc5c9d9d 100644 --- a/caddyhttp/httpserver/server.go +++ b/caddyhttp/httpserver/server.go @@ -442,11 +442,12 @@ func (s *Server) serveHTTP(w http.ResponseWriter, r *http.Request) (int, error) r.URL = trimPathPrefix(r.URL, pathPrefix) } + // if not disabled via `insecure_disable_sni_matching` // 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 && + if !vhost.TLS.InsecureDisableSNIMatching && 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", diff --git a/caddyhttp/httpserver/siteconfig.go b/caddyhttp/httpserver/siteconfig.go index f94707eef..2e3d5b118 100644 --- a/caddyhttp/httpserver/siteconfig.go +++ b/caddyhttp/httpserver/siteconfig.go @@ -36,16 +36,6 @@ 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 a4dc51626..9dcc70e05 100644 --- a/caddytls/config.go +++ b/caddytls/config.go @@ -62,6 +62,18 @@ type Config struct { // client authentication is enabled ClientCerts []string + // Allow mismatched TLS SNI and Host header when using TLS client authentication + // If false (the default), the Host header in the HTTP request must + // match the SNI value in the TLS handshake (if any). + // This should be enabled whenever the TLS SNI and Host header + // in the HTTP request can be different, for example when doing mTLS with multiple servers + // and the upstream addresses do not match the HTTP request Host header. + // If a site relies on TLS client authentication 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, set this to false. + // TODO: Check how relevant this is with TLS 1.3. + InsecureDisableSNIMatching bool + // Manual means user provides own certs and keys Manual bool diff --git a/caddytls/setup.go b/caddytls/setup.go index 67072e0f9..00f9e8251 100644 --- a/caddytls/setup.go +++ b/caddytls/setup.go @@ -222,6 +222,8 @@ func setupTLS(c *caddy.Controller) error { } config.ClientCerts = clientCertList[listStart:] + case "insecure_disable_sni_matching": + config.InsecureDisableSNIMatching = true case "load": c.Args(&loadDir) config.Manual = true