Skip to content

Commit

Permalink
Added tls configuration flags for bmo.
Browse files Browse the repository at this point in the history
  • Loading branch information
as20203 committed Jul 26, 2023
1 parent 12dd6c5 commit 70ec2ae
Show file tree
Hide file tree
Showing 4 changed files with 240 additions and 14 deletions.
7 changes: 5 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ require (
github.com/gophercloud/gophercloud v1.5.0
github.com/metal3-io/baremetal-operator/apis v0.2.0
github.com/metal3-io/baremetal-operator/pkg/hardwareutils v0.2.0
github.com/onsi/gomega v1.27.7
github.com/pkg/errors v0.9.1
github.com/prometheus/client_golang v1.16.0
github.com/stretchr/testify v1.8.4
Expand All @@ -16,6 +17,8 @@ require (
k8s.io/api v0.27.2
k8s.io/apimachinery v0.27.2
k8s.io/client-go v0.27.2
k8s.io/component-base v0.27.2
k8s.io/klog/v2 v2.90.1
k8s.io/utils v0.0.0-20230209194617-a36077c30491
sigs.k8s.io/controller-runtime v0.15.0
sigs.k8s.io/yaml v1.3.0
Expand All @@ -41,6 +44,7 @@ require (
github.com/google/gofuzz v1.1.0 // indirect
github.com/google/uuid v1.3.0 // indirect
github.com/imdario/mergo v0.3.12 // indirect
github.com/inconshreveable/mousetrap v1.0.1 // indirect
github.com/josharian/intern v1.0.0 // indirect
github.com/json-iterator/go v1.1.12 // indirect
github.com/mailru/easyjson v0.7.7 // indirect
Expand All @@ -52,6 +56,7 @@ require (
github.com/prometheus/client_model v0.4.0 // indirect
github.com/prometheus/common v0.42.0 // indirect
github.com/prometheus/procfs v0.10.1 // indirect
github.com/spf13/cobra v1.6.0 // indirect
github.com/spf13/pflag v1.0.5 // indirect
go.uber.org/atomic v1.7.0 // indirect
go.uber.org/multierr v1.6.0 // indirect
Expand All @@ -69,8 +74,6 @@ require (
gopkg.in/yaml.v2 v2.4.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
k8s.io/apiextensions-apiserver v0.27.2 // indirect
k8s.io/component-base v0.27.2 // indirect
k8s.io/klog/v2 v2.90.1 // indirect
k8s.io/kube-openapi v0.0.0-20230501164219-8b0f38b5fd1f // indirect
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect
Expand Down
7 changes: 7 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA
github.com/cespare/xxhash/v2 v2.2.0 h1:DC2CZ1Ep5Y4k3ZQ899DldepgrayRUGE6BBZ/cd9Cj44=
github.com/cespare/xxhash/v2 v2.2.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs=
github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw=
github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o=
github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
Expand Down Expand Up @@ -75,6 +76,8 @@ github.com/gophercloud/gophercloud v1.5.0 h1:cDN6XFCLKiiqvYpjQLq9AiM7RDRbIC9450W
github.com/gophercloud/gophercloud v1.5.0/go.mod h1:aAVqcocTSXh2vYFZ1JTvx4EQmfgzxRcNupUfxZbBNDM=
github.com/imdario/mergo v0.3.12 h1:b6R2BslTbIEToALKP7LxUvijTsNI9TAe80pLWN2g/HU=
github.com/imdario/mergo v0.3.12/go.mod h1:jmQim1M+e3UYxmgPu/WyfjB3N3VflVyUjjjwH0dnCYA=
github.com/inconshreveable/mousetrap v1.0.1 h1:U3uMjPSQEBMNp1lFxmllqCPM6P5u/Xq7Pgzkat/bFNc=
github.com/inconshreveable/mousetrap v1.0.1/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw=
github.com/jessevdk/go-flags v1.4.0/go.mod h1:4FA24M0QyGHXBuZZK/XkWh8h0e1EYbRYJSGM75WSRxI=
github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY=
github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y=
Expand Down Expand Up @@ -103,6 +106,7 @@ github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ=
github.com/onsi/ginkgo/v2 v2.9.5 h1:+6Hr4uxzP4XIUyAkg61dWBw8lb/gc4/X5luuxN/EC+Q=
github.com/onsi/gomega v1.27.7 h1:fVih9JD6ogIiHUN6ePK7HJidyEDpWGVB5mzM7cWNXoU=
github.com/onsi/gomega v1.27.7/go.mod h1:1p8OOlwo2iUUDsHnOrjE5UKYJ+e3W8eQ3qSlRahPmr4=
github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
Expand All @@ -118,6 +122,9 @@ github.com/prometheus/common v0.42.0/go.mod h1:xBwqVerjNdUDjgODMpudtOMwlOwf2SaTr
github.com/prometheus/procfs v0.10.1 h1:kYK1Va/YMlutzCGazswoHKo//tZVlFpKYh+PymziUAg=
github.com/prometheus/procfs v0.10.1/go.mod h1:nwNm2aOCAYw8uTR/9bWRREkZFxAUcWzPHWJq+XBB/FM=
github.com/rogpeppe/go-internal v1.10.0 h1:TMyTOH3F/DB16zRVcYyreMH6GnZZrwQVAoYjRBZyWFQ=
github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
github.com/spf13/cobra v1.6.0 h1:42a0n6jwCot1pUmomAp4T7DeMD+20LFv4Q54pxLf2LI=
github.com/spf13/cobra v1.6.0/go.mod h1:IOw/AERYS7UzyrGinqmz6HLUo219MORXGxhbaJUqzrY=
github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA=
github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
github.com/stoewer/go-strcase v1.2.0/go.mod h1:IBiWB2sKIp3wVVQ3Y035++gc+knqhUQag1KpM8ahLw8=
Expand Down
129 changes: 117 additions & 12 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,13 @@ import (
"fmt"
"os"
"runtime"
"strings"

"go.uber.org/zap/zapcore"
k8sruntime "k8s.io/apimachinery/pkg/runtime"
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"
cliflag "k8s.io/component-base/cli/flag"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/cache"
"sigs.k8s.io/controller-runtime/pkg/healthz"
Expand All @@ -44,10 +46,26 @@ import (
// +kubebuilder:scaffold:imports
)

type TLSVersion string

// Constants for TLS versions.
const (
TLSVersion12 TLSVersion = "TLS12"
TLSVersion13 TLSVersion = "TLS13"
)

type TLSOptions struct {
TLSMaxVersion string
TLSMinVersion string
TLSCipherSuites string
}

var (
scheme = k8sruntime.NewScheme()
setupLog = ctrl.Log.WithName("setup")
healthAddr string
scheme = k8sruntime.NewScheme()
setupLog = ctrl.Log.WithName("setup")
healthAddr string
tlsOptions = TLSOptions{}
tlsSupportedVersions = []string{"TLS12", "TLS13"}
)

const leaderElectionID = "baremetal-operator"
Expand Down Expand Up @@ -132,6 +150,22 @@ func main() {
"Maximum queries per second from the controller client to the Kubernetes API server. Default 20")
flag.IntVar(&restConfigBurst, "kube-api-burst", 30,
"Maximum number of queries that should be allowed in one burst from the controller client to the Kubernetes API server. Default 30")
flag.StringVar(&tlsOptions.TLSMinVersion, "tls-min-version", "TLS12",
"The minimum TLS version in use by the webhook server.\n"+
fmt.Sprintf("Possible values are %s.", strings.Join(tlsSupportedVersions, ", ")),
)
flag.StringVar(&tlsOptions.TLSMaxVersion, "tls-max-version", "TLS13",
"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()
flag.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, ", ")+".")
flag.Parse()

logOpts := zap.Options{}
Expand All @@ -149,7 +183,11 @@ func main() {
if leaderElectionNamespace == "" {
leaderElectionNamespace = watchNamespace
}

tlsOptionOverrides, err := GetTLSOptionOverrideFuncs(tlsOptions)
if err != nil {
setupLog.Error(err, "unable to add TLS settings to the webhook server")
os.Exit(1)
}
restConfig := ctrl.GetConfigOrDie()
restConfig.QPS = float32(restConfigQPS)
restConfig.Burst = restConfigBurst
Expand All @@ -158,14 +196,8 @@ func main() {
Scheme: scheme,
MetricsBindAddress: metricsBindAddr,
WebhookServer: webhook.NewServer(webhook.Options{
Port: webhookPort,
TLSOpts: []func(*tls.Config){
func(c *tls.Config) {
if c.MinVersion < tls.VersionTLS12 {
c.MinVersion = tls.VersionTLS12
}
},
},
Port: webhookPort,
TLSOpts: tlsOptionOverrides,
}),
LeaderElection: enableLeaderElection,
LeaderElectionID: leaderElectionID,
Expand Down Expand Up @@ -254,3 +286,76 @@ func main() {
os.Exit(1)
}
}

// 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 options.TLSMinVersion == string(TLSVersion13) &&
options.TLSMaxVersion == string(TLSVersion13) &&
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 string(TLSVersion12):
v = tls.VersionTLS12
case string(TLSVersion13):
v = tls.VersionTLS13
default:
return 0, fmt.Errorf("unexpected TLS version %q (must be one of: TLS12, TLS13)", version)
}
return v, nil
}
111 changes: 111 additions & 0 deletions main_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
/*
Copyright 2023 The Metal3 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())
})
}

0 comments on commit 70ec2ae

Please sign in to comment.