From c8ecd7544d2247843edb138fc31cfead0d0631ad Mon Sep 17 00:00:00 2001 From: Will Thames Date: Mon, 7 Jun 2021 15:04:52 +1000 Subject: [PATCH 1/2] Allow TLS minimum version to be configured Some environments have automated security scans that trigger on TLS versions or insecure cipher suites. Setting TLS to 1.3 would solve both problems (setting to 1.2 only solves the former as the default 1.2 cipher suites are insecure). Default TLS minimum version of 1.0 remains. --- pkg/webhook/server.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/pkg/webhook/server.go b/pkg/webhook/server.go index cdd34c9660..8ba056b04e 100644 --- a/pkg/webhook/server.go +++ b/pkg/webhook/server.go @@ -70,6 +70,10 @@ type Server struct { // Defaults to "", which means server does not verify client's certificate. ClientCAName string + // TLSVersion is the minimum version of TLS supported. Accepts + // "1.1", "1.2" and "1.3" - anything else will result in "1.0" + TLSMinVersion string + // WebhookMux is the multiplexer that handles different webhooks. WebhookMux *http.ServeMux @@ -77,6 +81,10 @@ type Server struct { // and to provide better panic messages on duplicate webhook registration. webhooks map[string]http.Handler + // tlsMinVersion is the result of the conversion from human-readable TLS version (for example "1.1") + // to the values accepted by tls.Config (for example 0x301) + tlsMinVersion uint16 + // setFields allows injecting dependencies from an external source setFields inject.Func @@ -109,6 +117,17 @@ func (s *Server) setDefaults() { if len(s.KeyName) == 0 { s.KeyName = "tls.key" } + + switch s.TLSMinVersion { + case "1.1": + s.tlsMinVersion = tls.VersionTLS11 + case "1.2": + s.tlsMinVersion = tls.VersionTLS12 + case "1.3": + s.tlsMinVersion = tls.VersionTLS13 + default: + s.tlsMinVersion = tls.VersionTLS10 + } } // NeedLeaderElection implements the LeaderElectionRunnable interface, which indicates @@ -200,6 +219,7 @@ func (s *Server) Start(ctx context.Context) error { cfg := &tls.Config{ NextProtos: []string{"h2"}, GetCertificate: certWatcher.GetCertificate, + MinVersion: s.tlsMinVersion, } // load CA to verify client certificate From acdafe14569cb4c46d2f4cec5341d9aa547fe010 Mon Sep 17 00:00:00 2001 From: Will Thames Date: Wed, 9 Jun 2021 11:03:26 +1000 Subject: [PATCH 2/2] Add error handling to tls version conversion --- pkg/webhook/server.go | 44 ++++++++++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/pkg/webhook/server.go b/pkg/webhook/server.go index 8ba056b04e..dfca94e99a 100644 --- a/pkg/webhook/server.go +++ b/pkg/webhook/server.go @@ -71,7 +71,7 @@ type Server struct { ClientCAName string // TLSVersion is the minimum version of TLS supported. Accepts - // "1.1", "1.2" and "1.3" - anything else will result in "1.0" + // "", "1.0", "1.1", "1.2" and "1.3" only ("" is equivalent to "1.0" for backwards compatibility) TLSMinVersion string // WebhookMux is the multiplexer that handles different webhooks. @@ -81,10 +81,6 @@ type Server struct { // and to provide better panic messages on duplicate webhook registration. webhooks map[string]http.Handler - // tlsMinVersion is the result of the conversion from human-readable TLS version (for example "1.1") - // to the values accepted by tls.Config (for example 0x301) - tlsMinVersion uint16 - // setFields allows injecting dependencies from an external source setFields inject.Func @@ -117,17 +113,6 @@ func (s *Server) setDefaults() { if len(s.KeyName) == 0 { s.KeyName = "tls.key" } - - switch s.TLSMinVersion { - case "1.1": - s.tlsMinVersion = tls.VersionTLS11 - case "1.2": - s.tlsMinVersion = tls.VersionTLS12 - case "1.3": - s.tlsMinVersion = tls.VersionTLS13 - default: - s.tlsMinVersion = tls.VersionTLS10 - } } // NeedLeaderElection implements the LeaderElectionRunnable interface, which indicates @@ -194,6 +179,26 @@ func (s *Server) StartStandalone(ctx context.Context, scheme *runtime.Scheme) er return s.Start(ctx) } +// tlsVersion converts from human-readable TLS version (for example "1.1") +// to the values accepted by tls.Config (for example 0x301) +func tlsVersion(version string) (uint16, error) { + switch version { + // default is previous behaviour + case "": + return tls.VersionTLS10, nil + case "1.0": + return tls.VersionTLS10, nil + case "1.1": + return tls.VersionTLS11, nil + case "1.2": + return tls.VersionTLS12, nil + case "1.3": + return tls.VersionTLS13, nil + default: + return 0, fmt.Errorf("Invalid TLSMinVersion %v: expects 1.0, 1.1, 1.2, 1.3 or empty", version) + } +} + // Start runs the server. // It will install the webhook related resources depend on the server configuration. func (s *Server) Start(ctx context.Context) error { @@ -216,10 +221,15 @@ func (s *Server) Start(ctx context.Context) error { } }() + tlsMinVersion, err := tlsVersion(s.TLSMinVersion) + if err != nil { + return err + } + cfg := &tls.Config{ NextProtos: []string{"h2"}, GetCertificate: certWatcher.GetCertificate, - MinVersion: s.tlsMinVersion, + MinVersion: tlsMinVersion, } // load CA to verify client certificate