From 39aae4167e0b488ae56ad0d3ec1e1d6812ad7dcc Mon Sep 17 00:00:00 2001 From: mpl Date: Wed, 3 Jul 2019 19:22:05 +0200 Subject: [PATCH] TLSOptions: handle conflict: same host name, different TLS options Co-authored-by: Julien Salleyron --- docs/content/routing/routers/index.md | 42 +++++++++++- docs/content/user-guides/crd-acme/01-crd.yml | 23 +++++++ .../fixtures/https/https_tls_options.toml | 18 +++++ integration/https_test.go | 66 +++++++++++++++++++ pkg/server/router/tcp/router.go | 51 ++++++++++++-- pkg/tcp/router.go | 1 - 6 files changed, 193 insertions(+), 8 deletions(-) diff --git a/docs/content/routing/routers/index.md b/docs/content/routing/routers/index.md index 811783f04..5a3d09987 100644 --- a/docs/content/routing/routers/index.md +++ b/docs/content/routing/routers/index.md @@ -327,9 +327,15 @@ Traefik will terminate the SSL connections (meaning that it will send decrypted #### `Options` -The `Options` field enables fine-grained control of the TLS parameters. +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 diff --git a/docs/content/user-guides/crd-acme/01-crd.yml b/docs/content/user-guides/crd-acme/01-crd.yml index 8914ccac1..9aef075b4 100644 --- a/docs/content/user-guides/crd-acme/01-crd.yml +++ b/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 diff --git a/integration/fixtures/https/https_tls_options.toml b/integration/fixtures/https/https_tls_options.toml index 50191c4c3..10bf6cadf 100644 --- a/integration/fixtures/https/https_tls_options.toml +++ b/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" diff --git a/integration/https_test.go b/integration/https_test.go index cdbc58489..a226c54ce 100644 --- a/integration/https_test.go +++ b/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. diff --git a/pkg/server/router/tcp/router.go b/pkg/server/router/tcp/router.go index 9ea4995f7..26ed3d096 100644 --- a/pkg/server/router/tcp/router.go +++ b/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,12 +130,44 @@ func (m *Manager) buildEntryPointHandler(ctx context.Context, configs map[string logger.Debug(err) continue } - - router.AddRouteHTTPTLS(domain, tlsConf) + if tlsOptionsForHostSNI[domain] == nil { + 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) + } + } + for routerName, routerConfig := range configs { ctxRouter := log.With(internal.AddProviderInContext(ctx, routerName), log.Str(log.RouterName, routerName)) logger := log.FromContext(ctxRouter) diff --git a/pkg/tcp/router.go b/pkg/tcp/router.go index aeb98171e..dd7eb5020 100644 --- a/pkg/tcp/router.go +++ b/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 }