TLSOptions: handle conflict: same host name, different TLS options

Co-authored-by: Julien Salleyron <julien.salleyron@gmail.com>

docs/content/routing/routers/index.md

@@ -330,6 +330,12 @@ Traefik will terminate the SSL connections (meaning that it will send decrypted
 The `Options` field enables fine-grained control of the TLS parameters.
 It refers to a [TLS Options](../../https/tls.md#tls-options) and will be applied only if a `Host` rule is defined.
 
+!!! note "Server Name Association"
+
+    Even though one might get the impression that a TLS options reference is mapped to a router, or a router rule, one should realize that it is actually mapped only to the host name found in the `Host` part of the rule. Of course, there could also be several `Host` parts in a rule, in which case the TLS options reference would be mapped to as many host names.
+
+    Another thing to keep in mind is: the TLS option is picked from the mapping mentioned above and based on the server name provided during the TLS handshake, and it all happens before routing actually occurs.
+
 ??? example "Configuring the TLS options"
 
     ```toml tab="TOML"
@@ -369,6 +375,40 @@ It refers to a [TLS Options](../../https/tls.md#tls-options) and will be applied
           - TLS_RSA_WITH_AES_256_GCM_SHA384
     ```
 
+!!! important "Conflicting TLS Options"
+
+    Since a TLS options reference is mapped to a host name, if a configuration introduces a situation where the same host name (from a `Host` rule) gets matched with two TLS options references, a conflict occurs, such as in the example below:
+
+    ```toml tab="TOML"
+    [http.routers]
+      [http.routers.routerfoo]
+        rule = "Host(`snitest.com`) && Path(`/foo`)"
+        [http.routers.routerfoo.tls]
+          options="foo"
+
+    [http.routers]
+      [http.routers.routerbar]
+        rule = "Host(`snitest.com`) && Path(`/bar`)"
+        [http.routers.routerbar.tls]
+          options="bar"
+    ```
+
+    ```yaml tab="YAML"
+    http:
+      routers:
+        routerfoo:
+          rule: "Host(`snitest.com`) && Path(`/foo`)"
+          tls:
+            options: foo
+
+        routerbar:
+          rule: "Host(`snitest.com`) && Path(`/bar`)"
+          tls:
+            options: bar
+    ```
+
+    If that happens, both mappings are discarded, and the host name (`snitest.com` in this case) for these routers gets associated with the default TLS options instead.
+
 ## Configuring TCP Routers
 
 ### General

docs/content/user-guides/crd-acme/01-crd.yml

@@ -42,6 +42,21 @@ spec:
     singular: middleware
   scope: Namespaced
 
+---
+apiVersion: apiextensions.k8s.io/v1beta1
+kind: CustomResourceDefinition
+metadata:
+  name: tlsoptions.traefik.containo.us
+
+spec:
+  group: traefik.containo.us
+  version: v1alpha1
+  names:
+    kind: TLSOption
+    plural: tlsoptions
+    singular: tlsoption
+  scope: Namespaced
+
 ---
 kind: ClusterRole
 apiVersion: rbac.authorization.k8s.io/v1beta1
@@ -97,6 +112,14 @@ rules:
       - get
       - list
       - watch
+  - apiGroups:
+      - traefik.containo.us
+    resources:
+      - tlsoptions
+    verbs:
+      - get
+      - list
+      - watch
 
 ---
 kind: ClusterRoleBinding

integration/fixtures/https/https_tls_options.toml

@@ -35,6 +35,18 @@
     [http.routers.router3.tls]
       options = "unknown"
 
+  [http.routers.router4]
+    service = "service1"
+    rule = "Host(`snitest.net`)"
+    [http.routers.router4.tls]
+      options = "foo"
+
+  [http.routers.router5]
+    service = "service1"
+    rule = "Host(`snitest.net`)"
+    [http.routers.router5.tls]
+      options = "baz"
+
 [http.services]
   [http.services.service1]
     [http.services.service1.loadBalancer]
@@ -59,5 +71,11 @@
   [tls.options.foo]
     minversion = "VersionTLS11"
 
+  [tls.options.baz]
+    minversion = "VersionTLS11"
+
   [tls.options.bar]
     minversion = "VersionTLS12"
+
+  [tls.options.default]
+    minversion = "VersionTLS12"

integration/https_test.go

@@ -195,6 +195,72 @@ func (s *HTTPSSuite) TestWithTLSOptions(c *check.C) {
 	c.Assert(err, checker.IsNil)
 }
 
+// TestWithConflictingTLSOptions checks that routers with same SNI but different TLS options get fallbacked to the default TLS options.
+func (s *HTTPSSuite) TestWithConflictingTLSOptions(c *check.C) {
+	cmd, display := s.traefikCmd(withConfigFile("fixtures/https/https_tls_options.toml"))
+	defer display(c)
+	err := cmd.Start()
+	c.Assert(err, checker.IsNil)
+	defer cmd.Process.Kill()
+
+	// wait for Traefik
+	err = try.GetRequest("http://127.0.0.1:8080/api/rawdata", 1*time.Second, try.BodyContains("Host(`snitest.net`)"))
+	c.Assert(err, checker.IsNil)
+
+	backend1 := startTestServer("9010", http.StatusNoContent)
+	backend2 := startTestServer("9020", http.StatusResetContent)
+	defer backend1.Close()
+	defer backend2.Close()
+
+	err = try.GetRequest(backend1.URL, 1*time.Second, try.StatusCodeIs(http.StatusNoContent))
+	c.Assert(err, checker.IsNil)
+	err = try.GetRequest(backend2.URL, 1*time.Second, try.StatusCodeIs(http.StatusResetContent))
+	c.Assert(err, checker.IsNil)
+
+	tr4 := &http.Transport{
+		TLSClientConfig: &tls.Config{
+			InsecureSkipVerify: true,
+			MaxVersion:         tls.VersionTLS11,
+			ServerName:         "snitest.net",
+		},
+	}
+
+	trDefault := &http.Transport{
+		TLSClientConfig: &tls.Config{
+			InsecureSkipVerify: true,
+			MaxVersion:         tls.VersionTLS12,
+			ServerName:         "snitest.net",
+		},
+	}
+
+	// With valid TLS options and request
+	req, err := http.NewRequest(http.MethodGet, "https://127.0.0.1:4443/", nil)
+	c.Assert(err, checker.IsNil)
+	req.Host = trDefault.TLSClientConfig.ServerName
+	req.Header.Set("Host", trDefault.TLSClientConfig.ServerName)
+	req.Header.Set("Accept", "*/*")
+
+	err = try.RequestWithTransport(req, 30*time.Second, trDefault, try.StatusCodeIs(http.StatusNoContent))
+	c.Assert(err, checker.IsNil)
+
+	// With a bad TLS version
+	req, err = http.NewRequest(http.MethodGet, "https://127.0.0.1:4443/", nil)
+	c.Assert(err, checker.IsNil)
+	req.Host = tr4.TLSClientConfig.ServerName
+	req.Header.Set("Host", tr4.TLSClientConfig.ServerName)
+	req.Header.Set("Accept", "*/*")
+	client := http.Client{
+		Transport: tr4,
+	}
+	_, err = client.Do(req)
+	c.Assert(err, checker.NotNil)
+	c.Assert(err.Error(), checker.Contains, "protocol version not supported")
+
+	// with unknown tls option
+	err = try.GetRequest("http://127.0.0.1:8080/api/rawdata", 1*time.Second, try.BodyContains(fmt.Sprintf("found different TLS options for routers on the same host %v, so using the default TLS option instead", tr4.TLSClientConfig.ServerName)))
+	c.Assert(err, checker.IsNil)
+}
+
 // TestWithSNIStrictNotMatchedRequest involves a client sending a SNI hostname of
 // "snitest.org", which does not match the CN of 'snitest.com.crt'. The test
 // verifies that traefik closes the connection.

pkg/server/router/tcp/router.go

@@ -2,6 +2,7 @@ package tcp
 
 import (
 	"context"
+	"crypto/tls"
 	"fmt"
 	"net/http"
 
@@ -11,7 +12,7 @@ import (
 	"github.com/containous/traefik/pkg/server/internal"
 	tcpservice "github.com/containous/traefik/pkg/server/service/tcp"
 	"github.com/containous/traefik/pkg/tcp"
-	"github.com/containous/traefik/pkg/tls"
+	traefiktls "github.com/containous/traefik/pkg/tls"
 )
 
 // NewManager Creates a new Manager
@@ -19,7 +20,7 @@ func NewManager(conf *config.RuntimeConfiguration,
 	serviceManager *tcpservice.Manager,
 	httpHandlers map[string]http.Handler,
 	httpsHandlers map[string]http.Handler,
-	tlsManager *tls.Manager,
+	tlsManager *traefiktls.Manager,
 ) *Manager {
 	return &Manager{
 		serviceManager: serviceManager,
@@ -35,7 +36,7 @@ type Manager struct {
 	serviceManager *tcpservice.Manager
 	httpHandlers   map[string]http.Handler
 	httpsHandlers  map[string]http.Handler
-	tlsManager     *tls.Manager
+	tlsManager     *traefiktls.Manager
 	conf           *config.RuntimeConfiguration
 }
 
@@ -90,6 +91,12 @@ func (m *Manager) buildEntryPointHandler(ctx context.Context, configs map[string
 
 	router.HTTPSHandler(handlerHTTPS, defaultTLSConf)
 
+	type nameAndConfig struct {
+		routerName string // just so we have it as additional information when logging
+		TLSConfig  *tls.Config
+	}
+	// Keyed by domain, then by options reference.
+	tlsOptionsForHostSNI := map[string]map[string]nameAndConfig{}
 	for routerHTTPName, routerHTTPConfig := range configsHTTP {
 		if len(routerHTTPConfig.TLS.Options) == 0 || routerHTTPConfig.TLS.Options == defaultTLSConfigName {
 			continue
@@ -107,7 +114,7 @@ func (m *Manager) buildEntryPointHandler(ctx context.Context, configs map[string
 		}
 
 		if len(domains) == 0 {
-			logger.Warnf("The 'default' TLS options will be applied instead of %q as no domain has been found in the rule", routerHTTPConfig.TLS.Options)
+			logger.Warnf("No domain found in rule %v, the TLS options applied for this router will depend on the hostSNI of each request", routerHTTPConfig.Rule)
 		}
 
 		for _, domain := range domains {
@@ -123,9 +130,41 @@ func (m *Manager) buildEntryPointHandler(ctx context.Context, configs map[string
 					logger.Debug(err)
 					continue
 				}
-
+				if tlsOptionsForHostSNI[domain] == nil {
-				router.AddRouteHTTPTLS(domain, tlsConf)
+					tlsOptionsForHostSNI[domain] = make(map[string]nameAndConfig)
 				}
+				tlsOptionsForHostSNI[domain][routerHTTPConfig.TLS.Options] = nameAndConfig{
+					routerName: routerHTTPName,
+					TLSConfig:  tlsConf,
+				}
+			}
+		}
+	}
+
+	logger := log.FromContext(ctx)
+	for hostSNI, tlsConfigs := range tlsOptionsForHostSNI {
+		if len(tlsConfigs) == 1 {
+			var optionsName string
+			var config *tls.Config
+			for k, v := range tlsConfigs {
+				optionsName = k
+				config = v.TLSConfig
+				break
+			}
+			logger.Debugf("Adding route for %s with TLS options %s", hostSNI, optionsName)
+			router.AddRouteHTTPTLS(hostSNI, config)
+		} else {
+			routers := make([]string, 0, len(tlsConfigs))
+			for _, v := range tlsConfigs {
+				// TODO: properly deal with critical errors VS non-critical errors
+				if configsHTTP[v.routerName].Err != "" {
+					configsHTTP[v.routerName].Err += "\n"
+				}
+				configsHTTP[v.routerName].Err += fmt.Sprintf("found different TLS options for routers on the same host %v, so using the default TLS option instead", hostSNI)
+				routers = append(routers, v.routerName)
+			}
+			logger.Warnf("Found different TLS options for routers on the same host %v, so using the default TLS options instead for these routers: %#v", hostSNI, routers)
+			router.AddRouteHTTPTLS(hostSNI, defaultTLSConf)
 		}
 	}
 

pkg/tcp/router.go

@@ -90,7 +90,6 @@ func (r *Router) AddRouteHTTPTLS(sniHost string, config *tls.Config) {
 	if r.hostHTTPTLSConfig == nil {
 		r.hostHTTPTLSConfig = map[string]*tls.Config{}
 	}
-	log.Debugf("adding route %s with minversion %d", sniHost, config.MinVersion)
 	r.hostHTTPTLSConfig[sniHost] = config
 }