Handle broken TLS conf better

integration/fixtures/https/https_invalid_tls_options.toml

@@ -0,0 +1,60 @@
+[global]
+  checkNewVersion = false
+  sendAnonymousUsage = false
+
+[log]
+  level = "DEBUG"
+
+[entryPoints.websecure]
+  address = ":4443"
+
+[api]
+  insecure = true
+
+[providers.file]
+  filename = "{{ .SelfFilename }}"
+
+## dynamic configuration ##
+
+[http.routers]
+
+  [http.routers.router1]
+    entryPoints = ["websecure"]
+    service = "service1"
+    rule = "Host(`snitest.com`)"
+    [http.routers.router1.tls]
+      options = "invalidTLSOptions"
+
+  [http.routers.router2]
+    entryPoints = ["websecure"]
+    service = "service1"
+    rule = "Host(`snitest.org`)"
+    [http.routers.router2.tls]
+
+  # fallback router
+  [http.routers.router3]
+    entryPoints = ["websecure"]
+    service = "service1"
+    rule = "Path(`/`)"
+    [http.routers.router3.tls]
+
+[[http.services.service1.loadBalancer.servers]]
+  url = "http://127.0.0.1:9010"
+
+[[tls.certificates]]
+  certFile = "fixtures/https/snitest.com.cert"
+  keyFile = "fixtures/https/snitest.com.key"
+
+[[tls.certificates]]
+  certFile = "fixtures/https/snitest.org.cert"
+  keyFile = "fixtures/https/snitest.org.key"
+
+[tls.options]
+
+  [tls.options.default.clientAuth]
+    # Missing caFile to have an invalid mTLS configuration.
+    clientAuthType = "RequireAndVerifyClientCert"
+
+  [tls.options.invalidTLSOptions.clientAuth]
+    # Missing caFile to have an invalid mTLS configuration.
+    clientAuthType = "RequireAndVerifyClientCert"

integration/fixtures/tcp/multi-tls-options.toml

@@ -33,6 +33,13 @@
       [tcp.routers.to-whoami-sni-strict.tls]
         options = "bar"
 
+    [tcp.routers.to-whoami-invalid-tls]
+      rule = "HostSNI(`whoami-i.test`)"
+      service = "whoami-no-cert"
+      entryPoints = [ "tcp" ]
+      [tcp.routers.to-whoami-invalid-tls.tls]
+        options = "invalid"
+
     [tcp.services.whoami-no-cert]
       [tcp.services.whoami-no-cert.loadBalancer]
         [[tcp.services.whoami-no-cert.loadBalancer.servers]]
@@ -45,3 +52,7 @@
 
   [tls.options.bar]
     minVersion = "VersionTLS13"
+
+  [tls.options.invalid.clientAuth]
+    # Missing CA files to have an invalid mTLS configuration.
+    clientAuthType = "RequireAndVerifyClientCert"

integration/https_test.go

@@ -1226,3 +1226,53 @@ func (s *HTTPSSuite) TestWithDomainFronting(c *check.C) {
 		c.Assert(err, checker.IsNil)
 	}
 }
+
+// TestWithInvalidTLSOption verifies the behavior when using an invalid tlsOption configuration.
+func (s *HTTPSSuite) TestWithInvalidTLSOption(c *check.C) {
+	backend := startTestServer("9010", http.StatusOK, "server1")
+	defer backend.Close()
+
+	file := s.adaptFile(c, "fixtures/https/https_invalid_tls_options.toml", struct{}{})
+	defer os.Remove(file)
+	cmd, display := s.traefikCmd(withConfigFile(file))
+	defer display(c)
+	err := cmd.Start()
+	c.Assert(err, checker.IsNil)
+	defer s.killCmd(cmd)
+
+	// wait for Traefik
+	err = try.GetRequest("http://127.0.0.1:8080/api/rawdata", 500*time.Millisecond, try.BodyContains("Host(`snitest.com`)"))
+	c.Assert(err, checker.IsNil)
+
+	testCases := []struct {
+		desc       string
+		serverName string
+	}{
+		{
+			desc:       "With invalid TLS Options specified",
+			serverName: "snitest.com",
+		},
+		{
+			desc:       "With invalid Default TLS Options",
+			serverName: "snitest.org",
+		},
+		{
+			desc: "With TLS Options without servername (fallback to default)",
+		},
+	}
+
+	for _, test := range testCases {
+		test := test
+
+		tlsConfig := &tls.Config{
+			InsecureSkipVerify: true,
+		}
+		if test.serverName != "" {
+			tlsConfig.ServerName = test.serverName
+		}
+
+		conn, err := tls.Dial("tcp", "127.0.0.1:4443", tlsConfig)
+		c.Assert(err, checker.NotNil, check.Commentf("connected to server successfully"))
+		c.Assert(conn, checker.IsNil)
+	}
+}

integration/tcp_test.go

@@ -116,6 +116,14 @@ func (s *TCPSuite) TestTLSOptions(c *check.C) {
 	_, err = guessWhoTLSMaxVersion("127.0.0.1:8093", "whoami-d.test", true, tls.VersionTLS12)
 	c.Assert(err, checker.NotNil)
 	c.Assert(err.Error(), checker.Contains, "protocol version not supported")
+
+	// Check that we can't reach a route with an invalid mTLS configuration.
+	conn, err := tls.Dial("tcp", "127.0.0.1:8093", &tls.Config{
+		ServerName:         "whoami-i.test",
+		InsecureSkipVerify: true,
+	})
+	c.Assert(conn, checker.IsNil)
+	c.Assert(err, checker.NotNil)
 }
 
 func (s *TCPSuite) TestNonTLSFallback(c *check.C) {

pkg/server/router/router.go

@@ -3,6 +3,7 @@ package router
 import (
 	"context"
 	"errors"
+	"fmt"
 	"net/http"
 
 	"github.com/containous/alice"
@@ -16,6 +17,7 @@ import (
 	httpmuxer "github.com/traefik/traefik/v2/pkg/muxer/http"
 	"github.com/traefik/traefik/v2/pkg/server/middleware"
 	"github.com/traefik/traefik/v2/pkg/server/provider"
+	"github.com/traefik/traefik/v2/pkg/tls"
 )
 
 type middlewareBuilder interface {
@@ -35,17 +37,19 @@ type Manager struct {
 	middlewaresBuilder middlewareBuilder
 	chainBuilder       *middleware.ChainBuilder
 	conf               *runtime.Configuration
+	tlsManager         *tls.Manager
 }
 
-// NewManager Creates a new Manager.
-func NewManager(conf *runtime.Configuration, serviceManager serviceManager, middlewaresBuilder middlewareBuilder, chainBuilder *middleware.ChainBuilder, metricsRegistry metrics.Registry) *Manager {
+// NewManager creates a new Manager.
+func NewManager(conf *runtime.Configuration, serviceManager serviceManager, middlewaresBuilder middlewareBuilder, chainBuilder *middleware.ChainBuilder, metricsRegistry metrics.Registry, tlsManager *tls.Manager) *Manager {
 	return &Manager{
 		routerHandlers:     make(map[string]http.Handler),
 		serviceManager:     serviceManager,
 		metricsRegistry:    metricsRegistry,
 		middlewaresBuilder: middlewaresBuilder,
 		chainBuilder:       chainBuilder,
 		conf:               conf,
+		tlsManager:         tlsManager,
 	}
 }
 
@@ -141,6 +145,17 @@ func (m *Manager) buildRouterHandler(ctx context.Context, routerName string, rou
 		return handler, nil
 	}
 
+	if routerConfig.TLS != nil {
+		// Don't build the router if the TLSOptions configuration is invalid.
+		tlsOptionsName := tls.DefaultTLSConfigName
+		if len(routerConfig.TLS.Options) > 0 && routerConfig.TLS.Options != tls.DefaultTLSConfigName {
+			tlsOptionsName = provider.GetQualifiedName(ctx, routerConfig.TLS.Options)
+		}
+		if _, err := m.tlsManager.Get(tls.DefaultTLSStoreName, tlsOptionsName); err != nil {
+			return nil, fmt.Errorf("building router handler: %w", err)
+		}
+	}
+
 	handler, err := m.buildHTTPHandler(ctx, routerConfig, routerName)
 	if err != nil {
 		return nil, err

pkg/server/router/router_test.go

@@ -20,6 +20,7 @@ import (
 	"github.com/traefik/traefik/v2/pkg/server/middleware"
 	"github.com/traefik/traefik/v2/pkg/server/service"
 	"github.com/traefik/traefik/v2/pkg/testhelpers"
+	"github.com/traefik/traefik/v2/pkg/tls"
 	"github.com/traefik/traefik/v2/pkg/types"
 )
 
@@ -317,8 +318,9 @@ func TestRouterManager_Get(t *testing.T) {
 			serviceManager := service.NewManager(rtConf.Services, nil, nil, roundTripperManager)
 			middlewaresBuilder := middleware.NewBuilder(rtConf.Middlewares, serviceManager, nil)
 			chainBuilder := middleware.NewChainBuilder(nil, nil, nil)
+			tlsManager := tls.NewManager()
 
-			routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, chainBuilder, metrics.NewVoidRegistry())
+			routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, chainBuilder, metrics.NewVoidRegistry(), tlsManager)
 
 			handlers := routerManager.BuildHandlers(context.Background(), test.entryPoints, false)
 
@@ -423,8 +425,9 @@ func TestAccessLog(t *testing.T) {
 			serviceManager := service.NewManager(rtConf.Services, nil, nil, roundTripperManager)
 			middlewaresBuilder := middleware.NewBuilder(rtConf.Middlewares, serviceManager, nil)
 			chainBuilder := middleware.NewChainBuilder(nil, nil, nil)
+			tlsManager := tls.NewManager()
 
-			routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, chainBuilder, metrics.NewVoidRegistry())
+			routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, chainBuilder, metrics.NewVoidRegistry(), tlsManager)
 
 			handlers := routerManager.BuildHandlers(context.Background(), test.entryPoints, false)
 
@@ -462,6 +465,7 @@ func TestRuntimeConfiguration(t *testing.T) {
 		serviceConfig    map[string]*dynamic.Service
 		routerConfig     map[string]*dynamic.Router
 		middlewareConfig map[string]*dynamic.Middleware
+		tlsOptions       map[string]tls.Options
 		expectedError    int
 	}{
 		{
@@ -665,7 +669,6 @@ func TestRuntimeConfiguration(t *testing.T) {
 			},
 			expectedError: 1,
 		},
-
 		{
 			desc: "Router with broken middleware",
 			serviceConfig: map[string]*dynamic.Service{
@@ -696,8 +699,71 @@ func TestRuntimeConfiguration(t *testing.T) {
 			},
 			expectedError: 2,
 		},
+		{
+			desc: "Router with broken tlsOption",
+			serviceConfig: map[string]*dynamic.Service{
+				"foo-service": {
+					LoadBalancer: &dynamic.ServersLoadBalancer{
+						Servers: []dynamic.Server{
+							{
+								URL: "http://127.0.0.1",
+							},
+						},
+					},
+				},
+			},
+			middlewareConfig: map[string]*dynamic.Middleware{},
+			routerConfig: map[string]*dynamic.Router{
+				"bar": {
+					EntryPoints: []string{"web"},
+					Service:     "foo-service",
+					Rule:        "Host(`foo.bar`)",
+					TLS: &dynamic.RouterTLSConfig{
+						Options: "broken-tlsOption",
+					},
+				},
+			},
+			tlsOptions: map[string]tls.Options{
+				"broken-tlsOption": {
+					ClientAuth: tls.ClientAuth{
+						ClientAuthType: "foobar",
+					},
+				},
+			},
+			expectedError: 1,
+		},
+		{
+			desc: "Router with broken default tlsOption",
+			serviceConfig: map[string]*dynamic.Service{
+				"foo-service": {
+					LoadBalancer: &dynamic.ServersLoadBalancer{
+						Servers: []dynamic.Server{
+							{
+								URL: "http://127.0.0.1",
+							},
+						},
+					},
+				},
+			},
+			middlewareConfig: map[string]*dynamic.Middleware{},
+			routerConfig: map[string]*dynamic.Router{
+				"bar": {
+					EntryPoints: []string{"web"},
+					Service:     "foo-service",
+					Rule:        "Host(`foo.bar`)",
+					TLS:         &dynamic.RouterTLSConfig{},
+				},
+			},
+			tlsOptions: map[string]tls.Options{
+				"default": {
+					ClientAuth: tls.ClientAuth{
+						ClientAuthType: "foobar",
+					},
+				},
+			},
+			expectedError: 1,
+		},
 	}
-
 	for _, test := range testCases {
 		test := test
 		t.Run(test.desc, func(t *testing.T) {
@@ -711,17 +777,23 @@ func TestRuntimeConfiguration(t *testing.T) {
 					Routers:     test.routerConfig,
 					Middlewares: test.middlewareConfig,
 				},
+				TLS: &dynamic.TLSConfiguration{
+					Options: test.tlsOptions,
+				},
 			})
 
 			roundTripperManager := service.NewRoundTripperManager()
 			roundTripperManager.Update(map[string]*dynamic.ServersTransport{"default@internal": {}})
 			serviceManager := service.NewManager(rtConf.Services, nil, nil, roundTripperManager)
 			middlewaresBuilder := middleware.NewBuilder(rtConf.Middlewares, serviceManager, nil)
 			chainBuilder := middleware.NewChainBuilder(nil, nil, nil)
+			tlsManager := tls.NewManager()
+			tlsManager.UpdateConfigs(context.Background(), nil, test.tlsOptions, nil)
 
-			routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, chainBuilder, metrics.NewVoidRegistry())
+			routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, chainBuilder, metrics.NewVoidRegistry(), tlsManager)
 
 			_ = routerManager.BuildHandlers(context.Background(), entryPoints, false)
+			_ = routerManager.BuildHandlers(context.Background(), entryPoints, true)
 
 			// even though rtConf was passed by argument to the manager builders above,
 			// it's ok to use it as the result we check, because everything worth checking
@@ -793,8 +865,9 @@ func TestProviderOnMiddlewares(t *testing.T) {
 	serviceManager := service.NewManager(rtConf.Services, nil, nil, roundTripperManager)
 	middlewaresBuilder := middleware.NewBuilder(rtConf.Middlewares, serviceManager, nil)
 	chainBuilder := middleware.NewChainBuilder(nil, nil, nil)
+	tlsManager := tls.NewManager()
 
-	routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, chainBuilder, metrics.NewVoidRegistry())
+	routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, chainBuilder, metrics.NewVoidRegistry(), tlsManager)
 
 	_ = routerManager.BuildHandlers(context.Background(), entryPoints, false)
 
@@ -861,8 +934,9 @@ func BenchmarkRouterServe(b *testing.B) {
 	serviceManager := service.NewManager(rtConf.Services, nil, nil, staticRoundTripperGetter{res})
 	middlewaresBuilder := middleware.NewBuilder(rtConf.Middlewares, serviceManager, nil)
 	chainBuilder := middleware.NewChainBuilder(nil, nil, nil)
+	tlsManager := tls.NewManager()
 
-	routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, chainBuilder, metrics.NewVoidRegistry())
+	routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, chainBuilder, metrics.NewVoidRegistry(), tlsManager)
 
 	handlers := routerManager.BuildHandlers(context.Background(), entryPoints, false)