Merge pull request k0sproject#2869 from twz123/backport-2863-to-relea…

…se-1.23 [Backport release-1.23] TLS server configuration hardening
RedSerenity · Mar 8, 2023 · b21dd24 · b21dd24
2 parents c29af74 + 855c304
commit b21dd24
Show file tree

Hide file tree

Showing 9 changed files with 95 additions and 27 deletions.
diff --git a/cmd/api/api.go b/cmd/api/api.go
@@ -17,6 +17,7 @@ package api
 
 import (
 	"context"
+	"crypto/tls"
 	"encoding/base64"
 	"encoding/json"
 	"fmt"
@@ -38,6 +39,7 @@ import (
 	"github.com/k0sproject/k0s/internal/pkg/templatewriter"
 	"github.com/k0sproject/k0s/pkg/apis/k0s.k0sproject.io/v1beta1"
 	"github.com/k0sproject/k0s/pkg/config"
+	"github.com/k0sproject/k0s/pkg/constant"
 	"github.com/k0sproject/k0s/pkg/etcd"
 	"github.com/k0sproject/k0s/pkg/kubernetes"
 )
@@ -103,8 +105,12 @@ func (c *CmdOpts) startAPI() error {
 	)
 
 	srv := &http.Server{
-		Handler:      router,
-		Addr:         fmt.Sprintf(":%d", c.NodeConfig.Spec.API.K0sAPIPort),
+		Handler: router,
+		Addr:    fmt.Sprintf(":%d", c.NodeConfig.Spec.API.K0sAPIPort),
+		TLSConfig: &tls.Config{
+			MinVersion:   tls.VersionTLS12,
+			CipherSuites: constant.AllowedTLS12CipherSuiteIDs,
+		},
 		WriteTimeout: 15 * time.Second,
 		ReadTimeout:  15 * time.Second,
 	}

diff --git a/go.mod b/go.mod
@@ -40,6 +40,7 @@ require (
 	go.etcd.io/etcd/etcdutl/v3 v3.5.4
 	go.uber.org/zap v1.19.1
 	golang.org/x/crypto v0.0.0-20220411220226-7b82a4e95df4
+	golang.org/x/exp v0.0.0-20220827204233-334a2380cb91
 	golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4
 	golang.org/x/sys v0.5.0
 	golang.org/x/tools v0.1.12
@@ -128,7 +129,7 @@ require (
 	github.com/golang/protobuf v1.5.2 // indirect
 	github.com/google/btree v1.0.1 // indirect
 	github.com/google/certificate-transparency-go v1.0.21 // indirect
-	github.com/google/go-cmp v0.5.5 // indirect
+	github.com/google/go-cmp v0.5.8 // indirect
 	github.com/google/gofuzz v1.1.0 // indirect
 	github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 // indirect
 	github.com/google/uuid v1.2.0 // indirect

diff --git a/go.sum b/go.sum
@@ -529,8 +529,9 @@ github.com/google/go-cmp v0.5.1/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/
 github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-cmp v0.5.3/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
-github.com/google/go-cmp v0.5.5 h1:Khx7svrCpmxxtHBq5j2mp/xVjsi8hQMfNLvJFAlrGgU=
 github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
+github.com/google/go-cmp v0.5.8 h1:e6P7q2lk1O+qJJb4BtCQXlK8vWEO8V1ZeuEdJNOqZyg=
+github.com/google/go-cmp v0.5.8/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
 github.com/google/go-github v17.0.0+incompatible/go.mod h1:zLgOLi98H3fifZn+44m+umXrS52loVEgC2AApnigrVQ=
 github.com/google/go-github/v24 v24.0.1/go.mod h1:CRqaW1Uns1TCkP0wqTpxYyRxRjxwvKU/XSS44u6X74M=
 github.com/google/go-querystring v1.0.0/go.mod h1:odCYkC5MyYFN7vkCjXpyrEuKhc/BUO6wN/zVPAxq5ck=
@@ -1163,6 +1164,8 @@ golang.org/x/exp v0.0.0-20191227195350-da58074b4299/go.mod h1:2RIsYlXP63K8oxa1u0
 golang.org/x/exp v0.0.0-20200119233911-0405dc783f0a/go.mod h1:2RIsYlXP63K8oxa1u096TMicItID8zy7Y6sNkU49FU4=
 golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EHIKF9dgMWnmCNThgcyBT1FY9mM=
 golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU=
+golang.org/x/exp v0.0.0-20220827204233-334a2380cb91 h1:tnebWN09GYg9OLPss1KXj8txwZc6X6uMr6VFdcGNbHw=
+golang.org/x/exp v0.0.0-20220827204233-334a2380cb91/go.mod h1:cyybsKvd6eL0RnXn6p/Grxp8F5bW7iYuBgsNCOHpMYE=
 golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js=
 golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0=
 golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE=
@@ -1447,7 +1450,6 @@ golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc
 golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
-golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE=
 golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 gomodules.xyz/jsonpatch/v2 v2.2.0 h1:4pT439QV83L+G9FkcCriY6EkpcK6r6bK+A5FBUMI7qY=
 gomodules.xyz/jsonpatch/v2 v2.2.0/go.mod h1:WXp+iVDkoLQqPudfQ9GBlwB2eZ5DKOnjQZCYdOS8GPY=

diff --git a/pkg/component/controller/apiserver.go b/pkg/component/controller/apiserver.go
@@ -58,7 +58,6 @@ var apiDefaultArgs = map[string]string{
 	"requestheader-username-headers":     "X-Remote-User",
 	"secure-port":                        "6443",
 	"anonymous-auth":                     "false",
-	"tls-cipher-suites":                  "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384",
 }
 
 const egressSelectorConfigTemplate = `
@@ -105,6 +104,7 @@ func (a *APIServer) Run(_ context.Context) error {
 		"requestheader-client-ca-file":     path.Join(a.K0sVars.CertRootDir, "front-proxy-ca.crt"),
 		"service-account-key-file":         path.Join(a.K0sVars.CertRootDir, "sa.pub"),
 		"service-cluster-ip-range":         a.ClusterConfig.Spec.Network.BuildServiceCIDR(a.ClusterConfig.Spec.API.Address),
+		"tls-min-version":                  "VersionTLS12",
 		"tls-cert-file":                    path.Join(a.K0sVars.CertRootDir, "server.crt"),
 		"tls-private-key-file":             path.Join(a.K0sVars.CertRootDir, "server.key"),
 		"service-account-signing-key-file": path.Join(a.K0sVars.CertRootDir, "sa.key"),
@@ -146,6 +146,10 @@ func (a *APIServer) Run(_ context.Context) error {
 			args[name] = value
 		}
 	}
+	if args["tls-cipher-suites"] == "" {
+		args["tls-cipher-suites"] = constant.AllowedTLS12CipherSuiteNames()
+	}
+
 	if a.DisableEndpointReconciler {
 		args["endpoint-reconciler-type"] = "none"
 	}

diff --git a/pkg/component/controller/etcd.go b/pkg/component/controller/etcd.go
@@ -156,15 +156,20 @@ func (e *Etcd) Run(ctx context.Context) error {
 		"--listen-peer-urls":            peerURL,
 		"--initial-advertise-peer-urls": peerURL,
 		"--name":                        name,
-		"--trusted-ca-file":             etcdCaCert,
-		"--cert-file":                   etcdServerCert,
-		"--key-file":                    etcdServerKey,
-		"--peer-trusted-ca-file":        etcdCaCert,
-		"--peer-key-file":               etcdPeerKey,
-		"--peer-cert-file":              etcdPeerCert,
-		"--log-level":                   e.LogLevel,
-		"--peer-client-cert-auth":       "true",
-		"--enable-pprof":                "false",
+		// Specifying a minimum TLS version is not yet possible in etcd,
+		// although support for it has already been merged upstream. Enable this
+		// flag once it's available in the etcd release that ships with k0s.
+		// https://github.com/etcd-io/etcd/pull/15156
+		// "--tls-min-version": "TLS1.2",
+		"--trusted-ca-file":       etcdCaCert,
+		"--cert-file":             etcdServerCert,
+		"--key-file":              etcdServerKey,
+		"--peer-trusted-ca-file":  etcdCaCert,
+		"--peer-key-file":         etcdPeerKey,
+		"--peer-cert-file":        etcdPeerCert,
+		"--log-level":             e.LogLevel,
+		"--peer-client-cert-auth": "true",
+		"--enable-pprof":          "false",
 	}
 
 	if file.Exists(filepath.Join(e.K0sVars.EtcdDataDir, "member", "snap", "db")) {
@@ -188,6 +193,14 @@ func (e *Etcd) Run(ctx context.Context) error {
 		args["--auth-token"] = auth
 	}
 
+	// The tls-min-version flag is not yet supported by etcd, but support for it
+	// has already been merged upstream. Once it becomes available, specifying a
+	// minimum version of TLS 1.3 _and_ a list of cipher suites will be rejected.
+	// https://github.com/etcd-io/etcd/pull/15156/files#diff-538c79cd00ec18cb43b5dddd5f36b979d9d050cf478a241304493284739d31bfR810-R813
+	if args["--cipher-suites"] == "" && args["--tls-min-version"] != "TLS1.3" {
+		args["--cipher-suites"] = constant.AllowedTLS12CipherSuiteNames()
+	}
+
 	logrus.Debugf("starting etcd with args: %v", args)
 
 	e.supervisor = supervisor.Supervisor{

diff --git a/pkg/component/controller/konnectivity.go b/pkg/component/controller/konnectivity.go
@@ -134,6 +134,7 @@ func (k *Konnectivity) defaultArgs() stringmap.StringMap {
 		"--delete-existing-uds-file": "true",
 		"--server-id":                serverID,
 		"--proxy-strategies":         "destHost,default",
+		"--cipher-suites":            constant.AllowedTLS12CipherSuiteNames(),
 	}
 }
 

diff --git a/pkg/component/controller/kubeletconfig.go b/pkg/component/controller/kubeletconfig.go
@@ -18,6 +18,7 @@ package controller
 import (
 	"bytes"
 	"context"
+	"crypto/tls"
 	"encoding/json"
 	"fmt"
 	"io"
@@ -227,6 +228,11 @@ func getDefaultProfile(dnsAddress string, dualStack bool) unstructuredYamlObject
 	// - it's easier to merge programatically defined structure
 	// - apart from map[string]interface there is no good way to define free-form mapping
 
+	cipherSuites := make([]string, len(constant.AllowedTLS12CipherSuiteIDs))
+	for i, cipherSuite := range constant.AllowedTLS12CipherSuiteIDs {
+		cipherSuites[i] = tls.CipherSuiteName(cipherSuite)
+	}
+
 	// for the authentication.x509.clientCAFile and volumePluginDir we want to use later binding so we put template placeholder instead of actual value there
 	profile := unstructuredYamlObject{
 		"apiVersion": "kubelet.config.k8s.io/v1beta1",
@@ -250,18 +256,9 @@ func getDefaultProfile(dnsAddress string, dualStack bool) unstructuredYamlObject
 				"cacheUnauthorizedTTL": "0s",
 			},
 		},
-		"clusterDNS":    []string{dnsAddress},
-		"clusterDomain": "cluster.local",
-		"tlsCipherSuites": []string{
-			"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256",
-			"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256",
-			"TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305",
-			"TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384",
-			"TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305",
-			"TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384",
-			"TLS_RSA_WITH_AES_256_GCM_SHA384",
-			"TLS_RSA_WITH_AES_128_GCM_SHA256",
-		},
+		"clusterDNS":           []string{dnsAddress},
+		"clusterDomain":        "cluster.local",
+		"tlsCipherSuites":      cipherSuites,
 		"volumeStatsAggPeriod": "0s",
 		"volumePluginDir":      "{{.VolumePluginDir}}", // see line 174 explanation
 		"failSwapOn":           false,

diff --git a/pkg/constant/constant_shared.go b/pkg/constant/constant_shared.go
@@ -16,9 +16,11 @@ limitations under the License.
 package constant
 
 import (
+	"crypto/tls"
 	"os"
 	"path/filepath"
 	"runtime"
+	"strings"
 )
 
 // WinDataDirDefault default data-dir for windows
@@ -120,6 +122,33 @@ const (
 	K0SNodeRoleLabel       = "node.k0sproject.io/role"
 )
 
+// The list of allowed TLS v1.2 cipher suites. Those should be used for k0s
+// itself and all embedded components. Note that TLS v1.3 ciphers are currently
+// not configurable in Go.
+//
+// https://ssl-config.mozilla.org/#server=go&config=intermediate
+var AllowedTLS12CipherSuiteIDs = []uint16{
+	tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
+	tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
+	tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256,
+	tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
+	tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,
+	tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256,
+}
+
+// A comma-separated string version of [AllowedTLS12CipherSuiteIDs], suitable to
+// be used as CLI arg for binaries.
+func AllowedTLS12CipherSuiteNames() string {
+	var cipherSuites strings.Builder
+	for i, cipherSuite := range AllowedTLS12CipherSuiteIDs {
+		if i > 0 {
+			cipherSuites.WriteRune(',')
+		}
+		cipherSuites.WriteString(tls.CipherSuiteName(cipherSuite))
+	}
+	return cipherSuites.String()
+}
+
 // CfgVars is a struct that holds all the config variables required for K0s
 type CfgVars struct {
 	AdminKubeConfigPath        string // The cluster admin kubeconfig location

diff --git a/pkg/constant/constant_shared_test.go b/pkg/constant/constant_shared_test.go
@@ -17,6 +17,7 @@ package constant
 
 import (
 	"bytes"
+	"crypto/tls"
 	"fmt"
 	"os/exec"
 	"strings"
@@ -25,6 +26,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"golang.org/x/exp/slices"
 	"golang.org/x/tools/go/packages"
 )
 
@@ -46,6 +48,19 @@ func TestConstants(t *testing.T) {
 	})
 }
 
+func TestTLSCipherSuites(t *testing.T) {
+	// Verify that the ciphers in use are still considered secure by Go.
+	cipherSuites := tls.CipherSuites()
+	for _, cipherSuite := range AllowedTLS12CipherSuiteIDs {
+		idx := slices.IndexFunc(cipherSuites, func(x *tls.CipherSuite) bool {
+			return x.ID == cipherSuite
+		})
+		if idx < 0 {
+			assert.Fail(t, "Not in tls.CipherSuites(), potentially insecure", "(0x%04x) %s", cipherSuite, tls.CipherSuiteName(cipherSuite))
+		}
+	}
+}
+
 func TestKubernetesModuleVersions(t *testing.T) {
 	kubernetesVersion := getVersion(t, "kubernetes")