From 3384c9f36cdbc65df6e1cf4976ec2196fdeb8faa Mon Sep 17 00:00:00 2001 From: Tuomo Tanskanen Date: Wed, 7 Feb 2024 14:26:48 +0200 Subject: [PATCH] add TLS configuration flags support Add --tls-min-version and --tls-max-versin configuration flags. Same flags can be found in k8s, CAPI, CAPM3 etc. Co-authored-by: Jawad Zaheer Signed-off-by: Tuomo Tanskanen --- main.go | 117 ++++++++++++++++++++++++++++++++++++++++++++++++++- main_test.go | 111 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 226 insertions(+), 2 deletions(-) create mode 100644 main_test.go diff --git a/main.go b/main.go index 01c6caa6a3..c2414fc674 100644 --- a/main.go +++ b/main.go @@ -17,11 +17,13 @@ package main import ( "context" + "crypto/tls" "flag" "fmt" "net/http" _ "net/http/pprof" "os" + "strings" "time" "github.com/spf13/pflag" @@ -55,9 +57,23 @@ import ( "sigs.k8s.io/cluster-api-provider-openstack/version" ) +// Constants for TLS versions. +const ( + TLSVersion12 = "TLS12" + TLSVersion13 = "TLS13" +) + +type TLSOptions struct { + TLSMaxVersion string + TLSMinVersion string + TLSCipherSuites string +} + var ( - scheme = runtime.NewScheme() - setupLog = ctrl.Log.WithName("setup") + scheme = runtime.NewScheme() + setupLog = ctrl.Log.WithName("setup") + tlsOptions = TLSOptions{} + tlsSupportedVersions = []string{TLSVersion12, TLSVersion13} // flags. diagnosticsOptions = flags.DiagnosticsOptions{} @@ -157,6 +173,24 @@ func InitFlags(fs *pflag.FlagSet) { fs.IntVar(&scopeCacheMaxSize, "scope-cache-max-size", 10, "The maximum credentials count the operator should keep in cache. Setting this value to 0 means no cache.") fs.BoolVar(&showVersion, "version", false, "Show current version and exit.") + + fs.StringVar(&tlsOptions.TLSMinVersion, "tls-min-version", TLSVersion12, + "The minimum TLS version in use by the webhook server.\n"+ + fmt.Sprintf("Possible values are %s.", strings.Join(tlsSupportedVersions, ", ")), + ) + + fs.StringVar(&tlsOptions.TLSMaxVersion, "tls-max-version", TLSVersion13, + "The maximum TLS version in use by the webhook server.\n"+ + fmt.Sprintf("Possible values are %s.", strings.Join(tlsSupportedVersions, ", ")), + ) + + tlsCipherPreferredValues := cliflag.PreferredTLSCipherNames() + tlsCipherInsecureValues := cliflag.InsecureTLSCipherNames() + fs.StringVar(&tlsOptions.TLSCipherSuites, "tls-cipher-suites", "", + "Comma-separated list of cipher suites for the webhook server. "+ + "If omitted, the default Go cipher suites will be used. \n"+ + "Preferred values: "+strings.Join(tlsCipherPreferredValues, ", ")+". \n"+ + "Insecure values: "+strings.Join(tlsCipherInsecureValues, ", ")+".") } // Add RBAC for the authorized diagnostics endpoint. @@ -189,6 +223,12 @@ func main() { }() } + tlsOptionOverrides, err := GetTLSOptionOverrideFuncs(tlsOptions) + if err != nil { + setupLog.Error(err, "unable to add TLS settings to the webhook server") + os.Exit(1) + } + cfg, err := config.GetConfigWithContext(os.Getenv("KUBECONTEXT")) if err != nil { setupLog.Error(err, "unable to get kubeconfig") @@ -238,6 +278,7 @@ func main() { webhook.Options{ Port: webhookPort, CertDir: webhookCertDir, + TLSOpts: tlsOptionOverrides, }, ), HealthProbeBindAddress: healthAddr, @@ -345,3 +386,75 @@ func setupWebhooks(mgr ctrl.Manager) { func concurrency(c int) controller.Options { return controller.Options{MaxConcurrentReconciles: c} } + +// GetTLSOptionOverrideFuncs returns a list of TLS configuration overrides to be used +// by the webhook server. +func GetTLSOptionOverrideFuncs(options TLSOptions) ([]func(*tls.Config), error) { + var tlsOptions []func(config *tls.Config) + + tlsMinVersion, err := GetTLSVersion(options.TLSMinVersion) + if err != nil { + return nil, err + } + + tlsMaxVersion, err := GetTLSVersion(options.TLSMaxVersion) + if err != nil { + return nil, err + } + + if tlsMaxVersion != 0 && tlsMinVersion > tlsMaxVersion { + return nil, fmt.Errorf("TLS version flag min version (%s) is greater than max version (%s)", + options.TLSMinVersion, options.TLSMaxVersion) + } + + tlsOptions = append(tlsOptions, func(cfg *tls.Config) { + cfg.MinVersion = tlsMinVersion + }) + + tlsOptions = append(tlsOptions, func(cfg *tls.Config) { + cfg.MaxVersion = tlsMaxVersion + }) + // Cipher suites should not be set if empty. + if tlsMinVersion >= tls.VersionTLS13 && + options.TLSCipherSuites != "" { + setupLog.Info("warning: Cipher suites should not be set for TLS version 1.3. Ignoring ciphers") + options.TLSCipherSuites = "" + } + + if options.TLSCipherSuites != "" { + tlsCipherSuites := strings.Split(options.TLSCipherSuites, ",") + suites, err := cliflag.TLSCipherSuites(tlsCipherSuites) + if err != nil { + return nil, err + } + + insecureCipherValues := cliflag.InsecureTLSCipherNames() + for _, cipher := range tlsCipherSuites { + for _, insecureCipherName := range insecureCipherValues { + if insecureCipherName == cipher { + setupLog.Info(fmt.Sprintf("warning: use of insecure cipher '%s' detected.", cipher)) + } + } + } + tlsOptions = append(tlsOptions, func(cfg *tls.Config) { + cfg.CipherSuites = suites + }) + } + + return tlsOptions, nil +} + +// GetTLSVersion returns the corresponding tls.Version or error. +func GetTLSVersion(version string) (uint16, error) { + var v uint16 + + switch version { + case TLSVersion12: + v = tls.VersionTLS12 + case TLSVersion13: + v = tls.VersionTLS13 + default: + return 0, fmt.Errorf("unexpected TLS version %q (must be one of: %s)", version, strings.Join(tlsSupportedVersions, ", ")) + } + return v, nil +} diff --git a/main_test.go b/main_test.go new file mode 100644 index 0000000000..b4139280c8 --- /dev/null +++ b/main_test.go @@ -0,0 +1,111 @@ +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "bytes" + "testing" + + . "github.com/onsi/gomega" + "k8s.io/klog/v2" + ctrl "sigs.k8s.io/controller-runtime" +) + +func TestTLSInsecureCiperSuite(t *testing.T) { + t.Run("test insecure cipher suite passed as TLS flag", func(t *testing.T) { + g := NewWithT(t) + tlsMockOptions := TLSOptions{ + TLSMaxVersion: "TLS13", + TLSMinVersion: "TLS12", + TLSCipherSuites: "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256", + } + ctrl.Log.WithName("setup") + ctrl.SetLogger(klog.Background()) + + bufWriter := bytes.NewBuffer(nil) + klog.SetOutput(bufWriter) + klog.LogToStderr(false) // this is important, because klog by default logs to stderr only + _, err := GetTLSOptionOverrideFuncs(tlsMockOptions) + g.Expect(err).Should(BeNil()) + g.Expect(bufWriter.String()).Should(ContainSubstring("use of insecure cipher 'TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256' detected.")) + }) +} + +func TestTLSMinAndMaxVersion(t *testing.T) { + t.Run("should fail if TLS min version is greater than max version.", func(t *testing.T) { + g := NewWithT(t) + tlsMockOptions := TLSOptions{ + TLSMaxVersion: "TLS12", + TLSMinVersion: "TLS13", + } + _, err := GetTLSOptionOverrideFuncs(tlsMockOptions) + g.Expect(err.Error()).To(Equal("TLS version flag min version (TLS13) is greater than max version (TLS12)")) + }) +} + +func Test13CipherSuite(t *testing.T) { + t.Run("should reset ciphersuite flag if TLS min and max version are set to 1.3", func(t *testing.T) { + g := NewWithT(t) + + // Here TLS_RSA_WITH_AES_128_CBC_SHA is a tls12 cipher suite. + tlsMockOptions := TLSOptions{ + TLSMaxVersion: "TLS13", + TLSMinVersion: "TLS13", + TLSCipherSuites: "TLS_RSA_WITH_AES_128_CBC_SHA,TLS_AES_256_GCM_SHA384", + } + + ctrl.Log.WithName("setup") + ctrl.SetLogger(klog.Background()) + + bufWriter := bytes.NewBuffer(nil) + klog.SetOutput(bufWriter) + klog.LogToStderr(false) // this is important, because klog by default logs to stderr only + _, err := GetTLSOptionOverrideFuncs(tlsMockOptions) + g.Expect(bufWriter.String()).Should(ContainSubstring("warning: Cipher suites should not be set for TLS version 1.3. Ignoring ciphers")) + g.Expect(err).Should(BeNil()) + }) +} + +func TestGetTLSVersion(t *testing.T) { + t.Run("should error out when incorrect tls version passed", func(t *testing.T) { + g := NewWithT(t) + tlsVersion := "TLS11" + _, err := GetTLSVersion(tlsVersion) + g.Expect(err.Error()).Should(Equal("unexpected TLS version \"TLS11\" (must be one of: TLS12, TLS13)")) + }) + t.Run("should pass and output correct tls version", func(t *testing.T) { + const VersionTLS12 uint16 = 771 + g := NewWithT(t) + tlsVersion := "TLS12" + version, err := GetTLSVersion(tlsVersion) + g.Expect(version).To(Equal(VersionTLS12)) + g.Expect(err).Should(BeNil()) + }) +} + +func TestTLSOptions(t *testing.T) { + t.Run("should pass with all the correct options below with no error.", func(t *testing.T) { + g := NewWithT(t) + tlsMockOptions := TLSOptions{ + TLSMinVersion: "TLS12", + TLSMaxVersion: "TLS13", + TLSCipherSuites: "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384", + } + _, err := GetTLSOptionOverrideFuncs(tlsMockOptions) + g.Expect(err).Should(BeNil()) + }) +}