From 588b98d08589864a7e0f37cbef9864d154ba1dd3 Mon Sep 17 00:00:00 2001 From: Tero Saarni Date: Fri, 20 Jan 2023 23:41:36 +0200 Subject: [PATCH] Add TLSv1.3 support. Added optional TLS min/max protocol version and command line switches to set versions for the etcd server. If max version is not explicitly set by the user, let Go select the max version which is currently TLSv1.3. Previously max version was set to TLSv1.2. Signed-off-by: Tero Saarni --- client/pkg/tlsutil/versions.go | 47 +++++++++++++++++ client/pkg/tlsutil/versions_test.go | 63 +++++++++++++++++++++++ client/pkg/transport/listener.go | 29 +++++++---- server/embed/config.go | 36 +++++++++++++ server/embed/config_test.go | 78 +++++++++++++++++++++++++++++ server/embed/etcd.go | 2 + server/etcdmain/config.go | 3 ++ server/etcdmain/help.go | 4 ++ tests/e2e/etcd_config_test.go | 30 +++++++++++ tests/integration/v3_tls_test.go | 70 ++++++++++++++++++++++++++ 10 files changed, 351 insertions(+), 11 deletions(-) create mode 100644 client/pkg/tlsutil/versions.go create mode 100644 client/pkg/tlsutil/versions_test.go diff --git a/client/pkg/tlsutil/versions.go b/client/pkg/tlsutil/versions.go new file mode 100644 index 000000000..ffcecd8c6 --- /dev/null +++ b/client/pkg/tlsutil/versions.go @@ -0,0 +1,47 @@ +// Copyright 2023 The etcd 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 tlsutil + +import ( + "crypto/tls" + "fmt" +) + +type TLSVersion string + +// Constants for TLS versions. +const ( + TLSVersionDefault TLSVersion = "" + TLSVersion12 TLSVersion = "TLS1.2" + TLSVersion13 TLSVersion = "TLS1.3" +) + +// GetTLSVersion returns the corresponding tls.Version or error. +func GetTLSVersion(version string) (uint16, error) { + var v uint16 + + switch version { + case string(TLSVersionDefault): + v = 0 // 0 means let Go decide. + 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: TLS1.2, TLS1.3)", version) + } + + return v, nil +} diff --git a/client/pkg/tlsutil/versions_test.go b/client/pkg/tlsutil/versions_test.go new file mode 100644 index 000000000..89c7c3f64 --- /dev/null +++ b/client/pkg/tlsutil/versions_test.go @@ -0,0 +1,63 @@ +// Copyright 2023 The etcd 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 tlsutil + +import ( + "crypto/tls" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestGetVersion(t *testing.T) { + tests := []struct { + name string + version string + want uint16 + expectError bool + }{ + { + name: "TLS1.2", + version: "TLS1.2", + want: tls.VersionTLS12, + }, + { + name: "TLS1.3", + version: "TLS1.3", + want: tls.VersionTLS13, + }, + { + name: "Empty version", + version: "", + want: 0, + }, + { + name: "Converting invalid version string to TLS version", + version: "not_existing", + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := GetTLSVersion(tt.version) + if err != nil { + assert.True(t, tt.expectError, "GetTLSVersion() returned error while expecting success: %v", err) + return + } + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/client/pkg/transport/listener.go b/client/pkg/transport/listener.go index 398cbc559..5e0e13e25 100644 --- a/client/pkg/transport/listener.go +++ b/client/pkg/transport/listener.go @@ -166,6 +166,14 @@ type TLSInfo struct { // Note that cipher suites are prioritized in the given order. CipherSuites []uint16 + // MinVersion is the minimum TLS version that is acceptable. + // If not set, the minimum version is TLS 1.2. + MinVersion uint16 + + // MaxVersion is the maximum TLS version that is acceptable. + // If not set, the default used by Go is selected (see tls.Config.MaxVersion). + MaxVersion uint16 + selfCert bool // parseFunc exists to simplify testing. Typically, parseFunc @@ -380,8 +388,17 @@ func (info TLSInfo) baseConfig() (*tls.Config, error) { } } + var minVersion uint16 + if info.MinVersion != 0 { + minVersion = info.MinVersion + } else { + // Default minimum version is TLS 1.2, previous versions are insecure and deprecated. + minVersion = tls.VersionTLS12 + } + cfg := &tls.Config{ - MinVersion: tls.VersionTLS12, + MinVersion: minVersion, + MaxVersion: info.MaxVersion, ServerName: info.ServerName, } @@ -512,11 +529,6 @@ func (info TLSInfo) ServerConfig() (*tls.Config, error) { // "h2" NextProtos is necessary for enabling HTTP2 for go's HTTP server cfg.NextProtos = []string{"h2"} - // go1.13 enables TLS 1.3 by default - // and in TLS 1.3, cipher suites are not configurable - // setting Max TLS version to TLS 1.2 for go 1.13 - cfg.MaxVersion = tls.VersionTLS12 - return cfg, nil } @@ -571,11 +583,6 @@ func (info TLSInfo) ClientConfig() (*tls.Config, error) { } } - // go1.13 enables TLS 1.3 by default - // and in TLS 1.3, cipher suites are not configurable - // setting Max TLS version to TLS 1.2 for go 1.13 - cfg.MaxVersion = tls.VersionTLS12 - return cfg, nil } diff --git a/server/embed/config.go b/server/embed/config.go index 2c5637742..75bcbc341 100644 --- a/server/embed/config.go +++ b/server/embed/config.go @@ -15,6 +15,7 @@ package embed import ( + "crypto/tls" "errors" "fmt" "math" @@ -226,6 +227,11 @@ type Config struct { // Note that cipher suites are prioritized in the given order. CipherSuites []string `json:"cipher-suites"` + // TlsMinVersion is the minimum accepted TLS version between client/server and peers. + TlsMinVersion string `json:"tls-min-version"` + // TlsMaxVersion is the maximum accepted TLS version between client/server and peers. + TlsMaxVersion string `json:"tls-max-version"` + ClusterState string `json:"initial-cluster-state"` DNSCluster string `json:"discovery-srv"` DNSClusterServiceName string `json:"discovery-srv-name"` @@ -660,6 +666,17 @@ func updateCipherSuites(tls *transport.TLSInfo, ss []string) error { return nil } +func updateMinMaxVersions(info *transport.TLSInfo, min, max string) { + // Validate() has been called to check the user input, so it should never fail. + var err error + if info.MinVersion, err = tlsutil.GetTLSVersion(min); err != nil { + panic(err) + } + if info.MaxVersion, err = tlsutil.GetTLSVersion(max); err != nil { + panic(err) + } +} + // Validate ensures that '*embed.Config' fields are properly configured. func (cfg *Config) Validate() error { if err := cfg.setupLogging(); err != nil { @@ -776,6 +793,25 @@ func (cfg *Config) Validate() error { zap.String("name", cfg.Name)) } + minVersion, err := tlsutil.GetTLSVersion(cfg.TlsMinVersion) + if err != nil { + return err + } + maxVersion, err := tlsutil.GetTLSVersion(cfg.TlsMaxVersion) + if err != nil { + return err + } + + // maxVersion == 0 means that Go selects the highest available version. + if maxVersion != 0 && minVersion > maxVersion { + return fmt.Errorf("min version (%s) is greater than max version (%s)", cfg.TlsMinVersion, cfg.TlsMaxVersion) + } + + // Check if user attempted to configure ciphers for TLS1.3 only: Go does not support that currently. + if minVersion == tls.VersionTLS13 && len(cfg.CipherSuites) > 0 { + return fmt.Errorf("cipher suites cannot be configured when only TLS1.3 is enabled") + } + return nil } diff --git a/server/embed/config_test.go b/server/embed/config_test.go index 06e465c5f..726f3395e 100644 --- a/server/embed/config_test.go +++ b/server/embed/config_test.go @@ -15,6 +15,7 @@ package embed import ( + "crypto/tls" "errors" "fmt" "net" @@ -429,3 +430,80 @@ func TestLogRotation(t *testing.T) { }) } } + +func TestTLSVersionMinMax(t *testing.T) { + tests := []struct { + name string + givenTLSMinVersion string + givenTLSMaxVersion string + givenCipherSuites []string + expectError bool + expectedMinTLSVersion uint16 + expectedMaxTLSVersion uint16 + }{ + { + name: "Minimum TLS version is set", + givenTLSMinVersion: "TLS1.3", + expectedMinTLSVersion: tls.VersionTLS13, + expectedMaxTLSVersion: 0, + }, + { + name: "Maximum TLS version is set", + givenTLSMaxVersion: "TLS1.2", + expectedMinTLSVersion: 0, + expectedMaxTLSVersion: tls.VersionTLS12, + }, + { + name: "Minimum and Maximum TLS versions are set", + givenTLSMinVersion: "TLS1.3", + givenTLSMaxVersion: "TLS1.3", + expectedMinTLSVersion: tls.VersionTLS13, + expectedMaxTLSVersion: tls.VersionTLS13, + }, + { + name: "Minimum and Maximum TLS versions are set in reverse order", + givenTLSMinVersion: "TLS1.3", + givenTLSMaxVersion: "TLS1.2", + expectError: true, + }, + { + name: "Invalid minimum TLS version", + givenTLSMinVersion: "invalid version", + expectError: true, + }, + { + name: "Invalid maximum TLS version", + givenTLSMaxVersion: "invalid version", + expectError: true, + }, + { + name: "Cipher suites configured for TLS 1.3", + givenTLSMinVersion: "TLS1.3", + givenCipherSuites: []string{"TLS_AES_128_GCM_SHA256"}, + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := NewConfig() + cfg.TlsMinVersion = tt.givenTLSMinVersion + cfg.TlsMaxVersion = tt.givenTLSMaxVersion + cfg.CipherSuites = tt.givenCipherSuites + + err := cfg.Validate() + if err != nil { + assert.True(t, tt.expectError, "Validate() returned error while expecting success: %v", err) + return + } + + updateMinMaxVersions(&cfg.PeerTLSInfo, cfg.TlsMinVersion, cfg.TlsMaxVersion) + updateMinMaxVersions(&cfg.ClientTLSInfo, cfg.TlsMinVersion, cfg.TlsMaxVersion) + + assert.Equal(t, tt.expectedMinTLSVersion, cfg.PeerTLSInfo.MinVersion) + assert.Equal(t, tt.expectedMaxTLSVersion, cfg.PeerTLSInfo.MaxVersion) + assert.Equal(t, tt.expectedMinTLSVersion, cfg.ClientTLSInfo.MinVersion) + assert.Equal(t, tt.expectedMaxTLSVersion, cfg.ClientTLSInfo.MaxVersion) + }) + } +} diff --git a/server/embed/etcd.go b/server/embed/etcd.go index e3863ee62..a03b4f1c9 100644 --- a/server/embed/etcd.go +++ b/server/embed/etcd.go @@ -499,6 +499,7 @@ func configurePeerListeners(cfg *Config) (peers []*peerListener, err error) { if err = cfg.PeerSelfCert(); err != nil { cfg.logger.Fatal("failed to get peer self-signed certs", zap.Error(err)) } + updateMinMaxVersions(&cfg.PeerTLSInfo, cfg.TlsMinVersion, cfg.TlsMaxVersion) if !cfg.PeerTLSInfo.Empty() { cfg.logger.Info( "starting with peer TLS", @@ -611,6 +612,7 @@ func configureClientListeners(cfg *Config) (sctxs map[string]*serveCtx, err erro if err = cfg.ClientSelfCert(); err != nil { cfg.logger.Fatal("failed to get client self-signed certs", zap.Error(err)) } + updateMinMaxVersions(&cfg.ClientTLSInfo, cfg.TlsMinVersion, cfg.TlsMaxVersion) if cfg.EnablePprof { cfg.logger.Info("pprof is enabled", zap.String("path", debugutil.HTTPPrefixPProf)) } diff --git a/server/etcdmain/config.go b/server/etcdmain/config.go index 9ad7b1b23..84763dd9a 100644 --- a/server/etcdmain/config.go +++ b/server/etcdmain/config.go @@ -26,6 +26,7 @@ import ( "go.etcd.io/etcd/api/v3/version" "go.etcd.io/etcd/client/pkg/v3/logutil" + "go.etcd.io/etcd/client/pkg/v3/tlsutil" "go.etcd.io/etcd/pkg/v3/flags" cconfig "go.etcd.io/etcd/server/v3/config" "go.etcd.io/etcd/server/v3/embed" @@ -215,6 +216,8 @@ func newConfig() *config { fs.StringVar(&cfg.ec.PeerTLSInfo.AllowedHostname, "peer-cert-allowed-hostname", "", "Allowed TLS hostname for inter peer authentication.") fs.Var(flags.NewStringsValue(""), "cipher-suites", "Comma-separated list of supported TLS cipher suites between client/server and peers (empty will be auto-populated by Go).") fs.BoolVar(&cfg.ec.PeerTLSInfo.SkipClientSANVerify, "experimental-peer-skip-client-san-verification", false, "Skip verification of SAN field in client certificate for peer connections.") + fs.StringVar(&cfg.ec.TlsMinVersion, "tls-min-version", string(tlsutil.TLSVersion12), "Minimum TLS version supported by etcd. Possible values: TLS1.2, TLS1.3.") + fs.StringVar(&cfg.ec.TlsMaxVersion, "tls-max-version", string(tlsutil.TLSVersionDefault), "Maximum TLS version supported by etcd. Possible values: TLS1.2, TLS1.3 (empty defers to Go).") fs.Var( flags.NewUniqueURLsWithExceptions("*", "*"), diff --git a/server/etcdmain/help.go b/server/etcdmain/help.go index 61e8d278c..bc444208d 100644 --- a/server/etcdmain/help.go +++ b/server/etcdmain/help.go @@ -199,6 +199,10 @@ Security: Comma-separated whitelist of origins for CORS, or cross-origin resource sharing, (empty or * means allow all). --host-whitelist '*' Acceptable hostnames from HTTP client requests, if server is not secure (empty or * means allow all). + --tls-min-version 'TLS1.2' + Minimum TLS version supported by etcd. Possible values: TLS1.2, TLS1.3. + --tls-max-version '' + Maximum TLS version supported by etcd. Possible values: TLS1.2, TLS1.3 (empty will be auto-populated by Go). Auth: --auth-token 'simple' diff --git a/tests/e2e/etcd_config_test.go b/tests/e2e/etcd_config_test.go index c76d819e3..0f130a5f5 100644 --- a/tests/e2e/etcd_config_test.go +++ b/tests/e2e/etcd_config_test.go @@ -22,6 +22,7 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/sync/errgroup" @@ -424,3 +425,32 @@ func TestEtcdHealthyWithTinySnapshotCatchupEntries(t *testing.T) { } require.NoError(t, g.Wait()) } + +func TestEtcdTLSVersion(t *testing.T) { + e2e.SkipInShortMode(t) + + d := t.TempDir() + proc, err := e2e.SpawnCmd( + []string{ + e2e.BinPath.Etcd, + "--data-dir", d, + "--name", "e1", + "--listen-client-urls", "https://0.0.0.0:0", + "--advertise-client-urls", "https://0.0.0.0:0", + "--listen-peer-urls", fmt.Sprintf("https://127.0.0.1:%d", e2e.EtcdProcessBasePort), + "--initial-advertise-peer-urls", fmt.Sprintf("https://127.0.0.1:%d", e2e.EtcdProcessBasePort), + "--initial-cluster", fmt.Sprintf("e1=https://127.0.0.1:%d", e2e.EtcdProcessBasePort), + "--peer-cert-file", e2e.CertPath, + "--peer-key-file", e2e.PrivateKeyPath, + "--cert-file", e2e.CertPath2, + "--key-file", e2e.PrivateKeyPath2, + + "--tls-min-version", "TLS1.2", + "--tls-max-version", "TLS1.3", + }, nil, + ) + assert.NoError(t, err) + assert.NoError(t, e2e.WaitReadyExpectProc(context.TODO(), proc, e2e.EtcdServerReadyLines), "did not receive expected output from etcd process") + assert.NoError(t, proc.Stop()) + +} diff --git a/tests/integration/v3_tls_test.go b/tests/integration/v3_tls_test.go index 318466d0c..ec7bcbf3f 100644 --- a/tests/integration/v3_tls_test.go +++ b/tests/integration/v3_tls_test.go @@ -20,6 +20,7 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" "google.golang.org/grpc" clientv3 "go.etcd.io/etcd/client/v3" @@ -49,6 +50,12 @@ func testTLSCipherSuites(t *testing.T, valid bool) { srvTLS.CipherSuites, cliTLS.CipherSuites = cipherSuites[:2], cipherSuites[2:] } + // go1.13 enables TLS 1.3 by default + // and in TLS 1.3, cipher suites are not configurable, + // so setting Max TLS version to TLS 1.2 to test cipher config. + srvTLS.MaxVersion = tls.VersionTLS12 + cliTLS.MaxVersion = tls.VersionTLS12 + clus := integration.NewCluster(t, &integration.ClusterConfig{Size: 1, ClientTLS: &srvTLS}) defer clus.Terminate(t) @@ -72,3 +79,66 @@ func testTLSCipherSuites(t *testing.T, valid bool) { t.Fatalf("expected TLS handshake success, got %v", cerr) } } + +func TestTLSMinMaxVersion(t *testing.T) { + integration.BeforeTest(t) + + tests := []struct { + name string + minVersion uint16 + maxVersion uint16 + expectError bool + }{ + { + name: "Connect with default TLS version should succeed", + minVersion: 0, + maxVersion: 0, + }, + { + name: "Connect with TLS 1.2 only should fail", + minVersion: tls.VersionTLS12, + maxVersion: tls.VersionTLS12, + expectError: true, + }, + { + name: "Connect with TLS 1.2 and 1.3 should succeed", + minVersion: tls.VersionTLS12, + maxVersion: tls.VersionTLS13, + }, + { + name: "Connect with TLS 1.3 only should succeed", + minVersion: tls.VersionTLS13, + maxVersion: tls.VersionTLS13, + }, + } + + // Configure server to support TLS 1.3 only. + srvTLS := integration.TestTLSInfo + srvTLS.MinVersion = tls.VersionTLS13 + srvTLS.MaxVersion = tls.VersionTLS13 + clus := integration.NewCluster(t, &integration.ClusterConfig{Size: 1, ClientTLS: &srvTLS}) + defer clus.Terminate(t) + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cc, err := integration.TestTLSInfo.ClientConfig() + assert.NoError(t, err) + + cc.MinVersion = tt.minVersion + cc.MaxVersion = tt.maxVersion + cli, cerr := integration.NewClient(t, clientv3.Config{ + Endpoints: []string{clus.Members[0].GRPCURL()}, + DialTimeout: time.Second, + DialOptions: []grpc.DialOption{grpc.WithBlock()}, + TLS: cc, + }) + if cerr != nil { + assert.True(t, tt.expectError, "got TLS handshake error while expecting success: %v", cerr) + assert.Equal(t, context.DeadlineExceeded, cerr, "expected %v with TLS handshake failure, got %v", context.DeadlineExceeded, cerr) + return + } + + cli.Close() + }) + } +}