From 76a5902efa99eff4158dad921e079c6808459494 Mon Sep 17 00:00:00 2001 From: Sam Batschelet Date: Tue, 21 Sep 2021 15:21:44 -0400 Subject: [PATCH 1/2] server/etcdmain: add configurable cipher list to gRPC proxy listener Signed-off-by: Allen Ray --- CHANGELOG/CHANGELOG-3.6.md | 4 +++ client/pkg/tlsutil/cipher_suites.go | 19 ++++++++++++- server/embed/config.go | 10 ++----- server/etcdmain/grpc_proxy.go | 44 ++++++++++++++++++----------- tests/e2e/etcd_config_test.go | 40 ++++++++++++++++++++++++++ 5 files changed, 92 insertions(+), 25 deletions(-) diff --git a/CHANGELOG/CHANGELOG-3.6.md b/CHANGELOG/CHANGELOG-3.6.md index 1f785a52c..064203bf8 100644 --- a/CHANGELOG/CHANGELOG-3.6.md +++ b/CHANGELOG/CHANGELOG-3.6.md @@ -76,6 +76,10 @@ See [code changes](https://github.com/etcd-io/etcd/compare/v3.5.0...v3.6.0). - Fix [Restrict the max size of each WAL entry to the remaining size of the WAL file](https://github.com/etcd-io/etcd/pull/14122). - Fix [memberID equals zero in corruption alarm](https://github.com/etcd-io/etcd/pull/14272) +### gRPC Proxy + +- Add [listen-cipher-suites](https://github.com/etcd-io/etcd/pull/14308) flag. + ### tools/benchmark - [Add etcd client autoSync flag](https://github.com/etcd-io/etcd/pull/13416) diff --git a/client/pkg/tlsutil/cipher_suites.go b/client/pkg/tlsutil/cipher_suites.go index f278a61f8..e1f21755d 100644 --- a/client/pkg/tlsutil/cipher_suites.go +++ b/client/pkg/tlsutil/cipher_suites.go @@ -14,7 +14,10 @@ package tlsutil -import "crypto/tls" +import ( + "crypto/tls" + "fmt" +) // GetCipherSuite returns the corresponding cipher suite, // and boolean value if it is supported. @@ -37,3 +40,17 @@ func GetCipherSuite(s string) (uint16, bool) { } return 0, false } + +// GetCipherSuites returns list of corresponding cipher suite IDs. +func GetCipherSuites(ss []string) ([]uint16, error) { + cs := make([]uint16, len(ss)) + for i, s := range ss { + var ok bool + cs[i], ok = GetCipherSuite(s) + if !ok { + return nil, fmt.Errorf("unexpected TLS cipher suite %q", s) + } + } + + return cs, nil +} diff --git a/server/embed/config.go b/server/embed/config.go index af4e25241..b0fec532a 100644 --- a/server/embed/config.go +++ b/server/embed/config.go @@ -652,13 +652,9 @@ func updateCipherSuites(tls *transport.TLSInfo, ss []string) error { return fmt.Errorf("TLSInfo.CipherSuites is already specified (given %v)", ss) } if len(ss) > 0 { - cs := make([]uint16, len(ss)) - for i, s := range ss { - var ok bool - cs[i], ok = tlsutil.GetCipherSuite(s) - if !ok { - return fmt.Errorf("unexpected TLS cipher suite %q", s) - } + cs, err := tlsutil.GetCipherSuites(ss) + if err != nil { + return err } tls.CipherSuites = cs } diff --git a/server/etcdmain/grpc_proxy.go b/server/etcdmain/grpc_proxy.go index b13520695..3d258c535 100644 --- a/server/etcdmain/grpc_proxy.go +++ b/server/etcdmain/grpc_proxy.go @@ -31,6 +31,7 @@ import ( pb "go.etcd.io/etcd/api/v3/etcdserverpb" "go.etcd.io/etcd/client/pkg/v3/logutil" + "go.etcd.io/etcd/client/pkg/v3/tlsutil" "go.etcd.io/etcd/client/pkg/v3/transport" clientv3 "go.etcd.io/etcd/client/v3" "go.etcd.io/etcd/client/v3/leasing" @@ -41,12 +42,12 @@ import ( "go.etcd.io/etcd/server/v3/etcdserver/api/v3election/v3electionpb" "go.etcd.io/etcd/server/v3/etcdserver/api/v3lock/v3lockpb" "go.etcd.io/etcd/server/v3/proxy/grpcproxy" - "go.uber.org/zap/zapgrpc" grpc_prometheus "github.com/grpc-ecosystem/go-grpc-prometheus" "github.com/soheilhy/cmux" "github.com/spf13/cobra" "go.uber.org/zap" + "go.uber.org/zap/zapgrpc" "golang.org/x/net/http2" "google.golang.org/grpc" "google.golang.org/grpc/grpclog" @@ -73,12 +74,13 @@ var ( // tls for clients connecting to proxy - grpcProxyListenCA string - grpcProxyListenCert string - grpcProxyListenKey string - grpcProxyListenAutoTLS bool - grpcProxyListenCRL string - selfSignedCertValidity uint + grpcProxyListenCA string + grpcProxyListenCert string + grpcProxyListenKey string + grpcProxyListenCipherSuites []string + grpcProxyListenAutoTLS bool + grpcProxyListenCRL string + selfSignedCertValidity uint grpcProxyAdvertiseClientURL string grpcProxyResolverPrefix string @@ -152,6 +154,7 @@ func newGRPCProxyStartCommand() *cobra.Command { cmd.Flags().StringVar(&grpcProxyListenCert, "cert-file", "", "identify secure connections to the proxy using this TLS certificate file") cmd.Flags().StringVar(&grpcProxyListenKey, "key-file", "", "identify secure connections to the proxy using this TLS key file") cmd.Flags().StringVar(&grpcProxyListenCA, "trusted-ca-file", "", "verify certificates of TLS-enabled secure proxy using this CA bundle") + cmd.Flags().StringSliceVar(&grpcProxyListenCipherSuites, "listen-cipher-suites", grpcProxyListenCipherSuites, "Comma-separated list of supported TLS cipher suites between client/proxy (empty will be auto-populated by Go).") cmd.Flags().BoolVar(&grpcProxyListenAutoTLS, "auto-tls", false, "proxy TLS using generated certificates") cmd.Flags().StringVar(&grpcProxyListenCRL, "client-crl-file", "", "proxy client certificate revocation list file.") cmd.Flags().UintVar(&selfSignedCertValidity, "self-signed-cert-validity", 1, "The validity period of the proxy certificates, unit is year") @@ -185,21 +188,28 @@ func startGRPCProxy(cmd *cobra.Command, args []string) { // The proxy itself (ListenCert) can have not-empty CN. // The empty CN is required for grpcProxyCert. // Please see https://github.com/etcd-io/etcd/issues/11970#issuecomment-687875315 for more context. - tlsinfo := newTLS(grpcProxyListenCA, grpcProxyListenCert, grpcProxyListenKey, false) - - if tlsinfo == nil && grpcProxyListenAutoTLS { + tlsInfo := newTLS(grpcProxyListenCA, grpcProxyListenCert, grpcProxyListenKey, false) + if len(grpcProxyListenCipherSuites) > 0 { + cs, err := tlsutil.GetCipherSuites(grpcProxyListenCipherSuites) + if err != nil { + log.Fatal(err) + } + tlsInfo.CipherSuites = cs + } + if tlsInfo == nil && grpcProxyListenAutoTLS { host := []string{"https://" + grpcProxyListenAddr} dir := filepath.Join(grpcProxyDataDir, "fixtures", "proxy") autoTLS, err := transport.SelfCert(lg, dir, host, selfSignedCertValidity) if err != nil { log.Fatal(err) } - tlsinfo = &autoTLS + tlsInfo = &autoTLS } - if tlsinfo != nil { - lg.Info("gRPC proxy server TLS", zap.String("tls-info", fmt.Sprintf("%+v", tlsinfo))) + + if tlsInfo != nil { + lg.Info("gRPC proxy server TLS", zap.String("tls-info", fmt.Sprintf("%+v", tlsInfo))) } - m := mustListenCMux(lg, tlsinfo) + m := mustListenCMux(lg, tlsInfo) grpcl := m.Match(cmux.HTTP2()) defer func() { grpcl.Close() @@ -212,11 +222,11 @@ func startGRPCProxy(cmd *cobra.Command, args []string) { // TODO: The mechanism should be refactored to use internal connection. var proxyClient *clientv3.Client if grpcProxyAdvertiseClientURL != "" { - proxyClient = mustNewProxyClient(lg, tlsinfo) + proxyClient = mustNewProxyClient(lg, tlsInfo) } httpClient := mustNewHTTPClient(lg) - srvhttp, httpl := mustHTTPListener(lg, m, tlsinfo, client, proxyClient) + srvhttp, httpl := mustHTTPListener(lg, m, tlsInfo, client, proxyClient) if err := http2.ConfigureServer(srvhttp, &http2.Server{ MaxConcurrentStreams: maxConcurrentStreams, @@ -229,7 +239,7 @@ func startGRPCProxy(cmd *cobra.Command, args []string) { go func() { errc <- srvhttp.Serve(httpl) }() go func() { errc <- m.Serve() }() if len(grpcProxyMetricsListenAddr) > 0 { - mhttpl := mustMetricsListener(lg, tlsinfo) + mhttpl := mustMetricsListener(lg, tlsInfo) go func() { mux := http.NewServeMux() grpcproxy.HandleMetrics(mux, httpClient, client.Endpoints()) diff --git a/tests/e2e/etcd_config_test.go b/tests/e2e/etcd_config_test.go index e00421024..546d2d3ff 100644 --- a/tests/e2e/etcd_config_test.go +++ b/tests/e2e/etcd_config_test.go @@ -302,6 +302,46 @@ func TestGrpcproxyAndCommonName(t *testing.T) { } } +func TestGrpcproxyAndListenCipherSuite(t *testing.T) { + e2e.SkipInShortMode(t) + + cases := []struct { + name string + args []string + }{ + { + name: "ArgsWithCipherSuites", + args: []string{ + e2e.BinDir + "/etcd", + "grpc-proxy", + "start", + "--listen-cipher-suites", "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256,TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256", + }, + }, + { + name: "ArgsWithoutCipherSuites", + args: []string{ + e2e.BinDir + "/etcd", + "grpc-proxy", + "start", + "--listen-cipher-suites", "", + }, + }, + } + + for _, test := range cases { + t.Run(test.name, func(t *testing.T) { + pw, err := e2e.SpawnCmd(test.args, nil) + if err != nil { + t.Fatal(err) + } + if err = pw.Stop(); err != nil { + t.Fatal(err) + } + }) + } +} + func TestBootstrapDefragFlag(t *testing.T) { e2e.SkipInShortMode(t) From 959ef2062c5e85630cc46f6e18bb47ff2064b038 Mon Sep 17 00:00:00 2001 From: Allen Ray Date: Mon, 12 Sep 2022 13:56:14 -0400 Subject: [PATCH 2/2] Update CHANGELOG/CHANGELOG-3.6.md Co-authored-by: Benjamin Wang Signed-off-by: Allen Ray --- CHANGELOG/CHANGELOG-3.6.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG/CHANGELOG-3.6.md b/CHANGELOG/CHANGELOG-3.6.md index a588e1ee2..45b89fd86 100644 --- a/CHANGELOG/CHANGELOG-3.6.md +++ b/CHANGELOG/CHANGELOG-3.6.md @@ -81,7 +81,7 @@ See [code changes](https://github.com/etcd-io/etcd/compare/v3.5.0...v3.6.0). ### etcd grpc-proxy - Add [`etcd grpc-proxy start --endpoints-auto-sync-interval`](https://github.com/etcd-io/etcd/pull/14354) flag to enable and configure interval of auto sync of endpoints with server. -- Add [listen-cipher-suites](https://github.com/etcd-io/etcd/pull/14308) flag. +- Add [`etcd grpc-proxy start --listen-cipher-suites`](https://github.com/etcd-io/etcd/pull/14308) flag to support adding configurable cipher list. ### tools/benchmark