From dcb1cb1f3a48ef8475ec8daf8821a6ea499c401f Mon Sep 17 00:00:00 2001 From: Sagar Muchhal Date: Fri, 4 Nov 2022 10:16:22 -0700 Subject: [PATCH] Addresses review comments This patch tries two approaches: 1. Using a common util function generate the TLS Options 2. Using a local method to generate the TLS Options. Signed-off-by: Sagar Muchhal --- bootstrap/kubeadm/main.go | 32 ++++++++++++----------- controlplane/kubeadm/main.go | 32 ++++++++++++----------- main.go | 49 +++++++++++++----------------------- util/flags/tls.go | 48 +++++++++++++++++++++++++++++++++++ 4 files changed, 101 insertions(+), 60 deletions(-) create mode 100644 util/flags/tls.go diff --git a/bootstrap/kubeadm/main.go b/bootstrap/kubeadm/main.go index b1fddf59ca75..3f5e72ba9a65 100644 --- a/bootstrap/kubeadm/main.go +++ b/bootstrap/kubeadm/main.go @@ -88,7 +88,7 @@ var ( healthAddr string tokenTTL time.Duration tlsMinVersion string - tlsCipherSuites string + tlsCipherSuites []string logOptions = logs.NewOptions() ) @@ -139,15 +139,18 @@ func InitFlags(fs *pflag.FlagSet) { fs.StringVar(&healthAddr, "health-addr", ":9440", "The address the health endpoint binds to.") - flag.StringVar(&tlsMinVersion, "tls-min-version", "VersionTLS12", + flag.StringVar(&tlsMinVersion, "tls-min-version", "", "The minimum TLS version in use by the webhook server.\n"+ fmt.Sprintf("Possible values are %s.", strings.Join(cliflag.TLSPossibleVersions(), ", ")), ) - flag.StringVar(&tlsCipherSuites, "tls-cipher-suites", "", - "Comma-separated list of cipher suites for the server. If omitted, the default Go cipher suites will be used.\n"+ - fmt.Sprintf("Possible values are %s.", strings.Join(cliflag.TLSCipherPossibleValues(), ", ")), - ) + tlsCipherPreferredValues := cliflag.PreferredTLSCipherNames() + tlsCipherInsecureValues := cliflag.InsecureTLSCipherNames() + fs.StringSliceVar(&tlsCipherSuites, "tls-cipher-suites", []string{}, + "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, ", ")+".") feature.MutableGates.AddFlag(fs) } @@ -179,6 +182,7 @@ func main() { restConfig := ctrl.GetConfigOrDie() restConfig.UserAgent = remote.DefaultClusterAPIUserAgent("cluster-api-kubeadm-bootstrap-manager") + tlsOptions := calculateTLSOpts(tlsMinVersion, tlsCipherSuites) mgr, err := ctrl.NewManager(restConfig, ctrl.Options{ Scheme: scheme, MetricsBindAddress: metricsBindAddr, @@ -197,6 +201,7 @@ func main() { Port: webhookPort, HealthProbeBindAddress: healthAddr, CertDir: webhookCertDir, + TLSOpts: tlsOptions, }) if err != nil { setupLog.Error(err, "unable to start manager") @@ -207,7 +212,6 @@ func main() { ctx := ctrl.SetupSignalHandler() setupChecks(mgr) - setupWebhookTLSConfigs(mgr, tlsMinVersion, tlsCipherSuites) setupWebhooks(mgr) setupReconcilers(ctx, mgr) @@ -231,28 +235,28 @@ func setupChecks(mgr ctrl.Manager) { } } -func setupWebhookTLSConfigs(mgr ctrl.Manager, tlsMinVersion, tlsCipherSuites string) { +func calculateTLSOpts(tlsMinVersion string, tlsCipherSuites []string) []func(*tls.Config) { + var tlsOptions []func(config *tls.Config) tlsVersion, err := cliflag.TLSVersion(tlsMinVersion) if err != nil { setupLog.Error(err, "unable to set TLS min version") os.Exit(1) } - mgr.GetWebhookServer().TLSOpts = append(mgr.GetWebhookServer().TLSOpts, func(cfg *tls.Config) { + tlsOptions = append(tlsOptions, func(cfg *tls.Config) { cfg.MinVersion = tlsVersion }) - if tlsCipherSuites != "" { - cipherSuites := strings.Split(tlsCipherSuites, ",") - suites, err := cliflag.TLSCipherSuites(cipherSuites) + if len(tlsCipherSuites) != 0 { + suites, err := cliflag.TLSCipherSuites(tlsCipherSuites) if err != nil { setupLog.Error(err, "unable to set TLS Cipher suites") os.Exit(1) } - - mgr.GetWebhookServer().TLSOpts = append(mgr.GetWebhookServer().TLSOpts, func(cfg *tls.Config) { + tlsOptions = append(tlsOptions, func(cfg *tls.Config) { cfg.CipherSuites = suites }) } + return tlsOptions } func setupReconcilers(ctx context.Context, mgr ctrl.Manager) { diff --git a/controlplane/kubeadm/main.go b/controlplane/kubeadm/main.go index 77f82d1f19dc..9db8cff35aba 100644 --- a/controlplane/kubeadm/main.go +++ b/controlplane/kubeadm/main.go @@ -92,7 +92,7 @@ var ( healthAddr string etcdDialTimeout time.Duration tlsMinVersion string - tlsCipherSuites string + tlsCipherSuites []string logOptions = logs.NewOptions() ) @@ -143,15 +143,18 @@ func InitFlags(fs *pflag.FlagSet) { fs.DurationVar(&etcdDialTimeout, "etcd-dial-timeout-duration", 10*time.Second, "Duration that the etcd client waits at most to establish a connection with etcd") - flag.StringVar(&tlsMinVersion, "tls-min-version", "VersionTLS12", + flag.StringVar(&tlsMinVersion, "tls-min-version", "", "The minimum TLS version in use by the webhook server.\n"+ fmt.Sprintf("Possible values are %s.", strings.Join(cliflag.TLSPossibleVersions(), ", ")), ) - flag.StringVar(&tlsCipherSuites, "tls-cipher-suites", "", - "Comma-separated list of cipher suites for the server. If omitted, the default Go cipher suites will be used.\n"+ - fmt.Sprintf("Possible values are %s.", strings.Join(cliflag.TLSCipherPossibleValues(), ", ")), - ) + tlsCipherPreferredValues := cliflag.PreferredTLSCipherNames() + tlsCipherInsecureValues := cliflag.InsecureTLSCipherNames() + fs.StringSliceVar(&tlsCipherSuites, "tls-cipher-suites", []string{}, + "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, ", ")+".") feature.MutableGates.AddFlag(fs) } @@ -183,6 +186,7 @@ func main() { restConfig := ctrl.GetConfigOrDie() restConfig.UserAgent = remote.DefaultClusterAPIUserAgent("cluster-api-kubeadm-control-plane-manager") + tlsOptions := calculateTLSOpts(tlsMinVersion, tlsCipherSuites) mgr, err := ctrl.NewManager(restConfig, ctrl.Options{ Scheme: scheme, MetricsBindAddress: metricsBindAddr, @@ -201,6 +205,7 @@ func main() { Port: webhookPort, HealthProbeBindAddress: healthAddr, CertDir: webhookCertDir, + TLSOpts: tlsOptions, }) if err != nil { setupLog.Error(err, "unable to start manager") @@ -211,7 +216,6 @@ func main() { ctx := ctrl.SetupSignalHandler() setupChecks(mgr) - setupWebhookTLSConfigs(mgr, tlsMinVersion, tlsCipherSuites) setupReconcilers(ctx, mgr) setupWebhooks(mgr) @@ -235,28 +239,28 @@ func setupChecks(mgr ctrl.Manager) { } } -func setupWebhookTLSConfigs(mgr ctrl.Manager, tlsMinVersion, tlsCipherSuites string) { +func calculateTLSOpts(tlsMinVersion string, tlsCipherSuites []string) []func(*tls.Config) { + var tlsOptions []func(config *tls.Config) tlsVersion, err := cliflag.TLSVersion(tlsMinVersion) if err != nil { setupLog.Error(err, "unable to set TLS min version") os.Exit(1) } - mgr.GetWebhookServer().TLSOpts = append(mgr.GetWebhookServer().TLSOpts, func(cfg *tls.Config) { + tlsOptions = append(tlsOptions, func(cfg *tls.Config) { cfg.MinVersion = tlsVersion }) - if tlsCipherSuites != "" { - cipherSuites := strings.Split(tlsCipherSuites, ",") - suites, err := cliflag.TLSCipherSuites(cipherSuites) + if len(tlsCipherSuites) != 0 { + suites, err := cliflag.TLSCipherSuites(tlsCipherSuites) if err != nil { setupLog.Error(err, "unable to set TLS Cipher suites") os.Exit(1) } - - mgr.GetWebhookServer().TLSOpts = append(mgr.GetWebhookServer().TLSOpts, func(cfg *tls.Config) { + tlsOptions = append(tlsOptions, func(cfg *tls.Config) { cfg.CipherSuites = suites }) } + return tlsOptions } func setupReconcilers(ctx context.Context, mgr ctrl.Manager) { diff --git a/main.go b/main.go index c38ca6502ce1..62edd071f909 100644 --- a/main.go +++ b/main.go @@ -19,7 +19,6 @@ package main import ( "context" - "crypto/tls" "flag" "fmt" "math/rand" @@ -69,6 +68,7 @@ import ( runtimeclient "sigs.k8s.io/cluster-api/internal/runtime/client" runtimeregistry "sigs.k8s.io/cluster-api/internal/runtime/registry" runtimewebhooks "sigs.k8s.io/cluster-api/internal/webhooks/runtime" + "sigs.k8s.io/cluster-api/util/flags" "sigs.k8s.io/cluster-api/version" "sigs.k8s.io/cluster-api/webhooks" ) @@ -102,7 +102,7 @@ var ( webhookCertDir string healthAddr string tlsMinVersion string - tlsCipherSuites string + tlsCipherSuites []string logOptions = logs.NewOptions() ) @@ -202,15 +202,18 @@ func InitFlags(fs *pflag.FlagSet) { fs.StringVar(&healthAddr, "health-addr", ":9440", "The address the health endpoint binds to.") - flag.StringVar(&tlsMinVersion, "tls-min-version", "VersionTLS12", + flag.StringVar(&tlsMinVersion, "tls-min-version", "", "The minimum TLS version in use by the webhook server.\n"+ fmt.Sprintf("Possible values are %s.", strings.Join(cliflag.TLSPossibleVersions(), ", ")), ) - flag.StringVar(&tlsCipherSuites, "tls-cipher-suites", "", - "Comma-separated list of cipher suites for the server. If omitted, the default Go cipher suites will be used.\n"+ - fmt.Sprintf("Possible values are %s.", strings.Join(cliflag.TLSCipherPossibleValues(), ", ")), - ) + tlsCipherPreferredValues := cliflag.PreferredTLSCipherNames() + tlsCipherInsecureValues := cliflag.InsecureTLSCipherNames() + fs.StringSliceVar(&tlsCipherSuites, "tls-cipher-suites", []string{}, + "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, ", ")+".") feature.MutableGates.AddFlag(fs) } @@ -254,6 +257,12 @@ func main() { os.Exit(1) } + tlsOptions, err := flags.CalculateTLSOpts(tlsMinVersion, tlsCipherSuites) + if err != nil { + setupLog.Error(err, "unable to add TLS settings to the webhook server") + os.Exit(1) + } + mgr, err := ctrl.NewManager(restConfig, ctrl.Options{ Scheme: scheme, MetricsBindAddress: metricsBindAddr, @@ -272,6 +281,7 @@ func main() { Port: webhookPort, CertDir: webhookCertDir, HealthProbeBindAddress: healthAddr, + TLSOpts: tlsOptions, }) if err != nil { setupLog.Error(err, "unable to start manager") @@ -283,7 +293,6 @@ func main() { setupChecks(mgr) setupIndexes(ctx, mgr) - setupWebhookTLSConfigs(mgr, tlsMinVersion, tlsCipherSuites) setupReconcilers(ctx, mgr) setupWebhooks(mgr) @@ -314,30 +323,6 @@ func setupIndexes(ctx context.Context, mgr ctrl.Manager) { } } -func setupWebhookTLSConfigs(mgr ctrl.Manager, tlsMinVersion, tlsCipherSuites string) { - tlsVersion, err := cliflag.TLSVersion(tlsMinVersion) - if err != nil { - setupLog.Error(err, "unable to set TLS min version") - os.Exit(1) - } - mgr.GetWebhookServer().TLSOpts = append(mgr.GetWebhookServer().TLSOpts, func(cfg *tls.Config) { - cfg.MinVersion = tlsVersion - }) - - if tlsCipherSuites != "" { - cipherSuites := strings.Split(tlsCipherSuites, ",") - suites, err := cliflag.TLSCipherSuites(cipherSuites) - if err != nil { - setupLog.Error(err, "unable to set TLS Cipher suites") - os.Exit(1) - } - - mgr.GetWebhookServer().TLSOpts = append(mgr.GetWebhookServer().TLSOpts, func(cfg *tls.Config) { - cfg.CipherSuites = suites - }) - } -} - func setupReconcilers(ctx context.Context, mgr ctrl.Manager) { // Set up a ClusterCacheTracker and ClusterCacheReconciler to provide to controllers // requiring a connection to a remote cluster diff --git a/util/flags/tls.go b/util/flags/tls.go new file mode 100644 index 000000000000..f32725f45fce --- /dev/null +++ b/util/flags/tls.go @@ -0,0 +1,48 @@ +/* +Copyright 2022 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 flags implements the webhook server TLS options utilities. +package flags + +import ( + "crypto/tls" + + cliflag "k8s.io/component-base/cli/flag" +) + +// CalculateTLSOpts returns a list of TLS configuration overrides +// to be used when setting the webhook server. +func CalculateTLSOpts(tlsMinVersion string, tlsCipherSuites []string) ([]func(*tls.Config), error) { + var tlsOptions []func(config *tls.Config) + tlsVersion, err := cliflag.TLSVersion(tlsMinVersion) + if err != nil { + return tlsOptions, err + } + tlsOptions = append(tlsOptions, func(cfg *tls.Config) { + cfg.MinVersion = tlsVersion + }) + + if len(tlsCipherSuites) != 0 { + suites, err := cliflag.TLSCipherSuites(tlsCipherSuites) + if err != nil { + return tlsOptions, err + } + tlsOptions = append(tlsOptions, func(cfg *tls.Config) { + cfg.CipherSuites = suites + }) + } + return tlsOptions, nil +}