optional tls version logic

pureiboi · pureiboi · commit f4b319a2fc62 · 2024-08-22T00:47:41.000+08:00
Signed-off-by: pureiboi &lt;17396188+pureiboi@users.noreply.github.com&gt;
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -22,6 +22,7 @@ We use *breaking :warning:* to mark changes that are not backward compatible (re
 - [#7609](https://github.com/thanos-io/thanos/pull/7609) API: Add limit param to metadata APIs (series, label names, label values).
 - [#7429](https://github.com/thanos-io/thanos/pull/7429): Reloader: introduce `TolerateEnvVarExpansionErrors` to allow suppressing errors when expanding environment variables in the configuration file. When set, this will ensure that the reloader won't consider the operation to fail when an unset environment variable is encountered. Note that all unset environment variables are left as is, whereas all set environment variables are expanded as usual.
 - [#7560](https://github.com/thanos-io/thanos/pull/7560) Query: Added the possibility of filtering rules by rule_name, rule_group or file to HTTP api.
+- [#7654](https://github.com/thanos-io/thanos/pull/7654) *: Add '--grpc-server-tls-min-version' flag to allow user to specify TLS version, otherwise default to TLS 1.3
 
 ### Changed
 
diff --git a/cmd/thanos/config.go b/cmd/thanos/config.go
@@ -28,6 +28,7 @@ type grpcConfig struct {
 	tlsSrvCert       string
 	tlsSrvKey        string
 	tlsSrvClientCA   string
+	tlsMinVersion    string
 	gracePeriod      time.Duration
 	maxConnectionAge time.Duration
 }
@@ -45,6 +46,9 @@ func (gc *grpcConfig) registerFlag(cmd extkingpin.FlagClause) *grpcConfig {
 	cmd.Flag("grpc-server-tls-client-ca",
 		"TLS CA to verify clients against. If no client CA is specified, there is no client verification on server side. (tls.NoClientCert)").
 		Default("").StringVar(&gc.tlsSrvClientCA)
+	cmd.Flag("grpc-server-tls-min-version",
+		"TLS minimum version to gRPC server, unset will default to tls 1.3, allow values: [\"1.0\", \"1.1\", \"1.2\", \"1.3\"]").
+		Default("1.3").StringVar(&gc.tlsMinVersion)
 	cmd.Flag("grpc-server-max-connection-age", "The grpc server max connection age. This controls how often to re-establish connections and redo TLS handshakes.").
 		Default("60m").DurationVar(&gc.maxConnectionAge)
 	cmd.Flag("grpc-grace-period",
diff --git a/cmd/thanos/query.go b/cmd/thanos/query.go
@@ -785,7 +785,7 @@ func runQuery(
 	}
 	// Start query (proxy) gRPC StoreAPI.
 	{
-		tlsCfg, err := tls.NewServerConfig(log.With(logger, "protocol", "gRPC"), grpcServerConfig.tlsSrvCert, grpcServerConfig.tlsSrvKey, grpcServerConfig.tlsSrvClientCA)
+		tlsCfg, err := tls.NewServerConfig(log.With(logger, "protocol", "gRPC"), grpcServerConfig.tlsSrvCert, grpcServerConfig.tlsSrvKey, grpcServerConfig.tlsSrvClientCA, grpcServerConfig.tlsMinVersion)
 		if err != nil {
 			return errors.Wrap(err, "setup gRPC server")
 		}
diff --git a/cmd/thanos/receive.go b/cmd/thanos/receive.go
@@ -136,7 +136,7 @@ func runReceive(
 
 	level.Info(logger).Log("mode", receiveMode, "msg", "running receive")
 
-	rwTLSConfig, err := tls.NewServerConfig(log.With(logger, "protocol", "HTTP"), conf.rwServerCert, conf.rwServerKey, conf.rwServerClientCA)
+	rwTLSConfig, err := tls.NewServerConfig(log.With(logger, "protocol", "HTTP"), conf.rwServerCert, conf.rwServerKey, conf.rwServerClientCA, conf.rwServerTlsMinVersion)
 	if err != nil {
 		return err
 	}
@@ -316,7 +316,7 @@ func runReceive(
 
 	level.Debug(logger).Log("msg", "setting up gRPC server")
 	{
-		tlsCfg, err := tls.NewServerConfig(log.With(logger, "protocol", "gRPC"), conf.grpcConfig.tlsSrvCert, conf.grpcConfig.tlsSrvKey, conf.grpcConfig.tlsSrvClientCA)
+		tlsCfg, err := tls.NewServerConfig(log.With(logger, "protocol", "gRPC"), conf.grpcConfig.tlsSrvCert, conf.grpcConfig.tlsSrvKey, conf.grpcConfig.tlsSrvClientCA, conf.grpcConfig.tlsMinVersion)
 		if err != nil {
 			return errors.Wrap(err, "setup gRPC server")
 		}
@@ -782,16 +782,17 @@ type receiveConfig struct {
 
 	grpcConfig grpcConfig
 
-	rwAddress          string
-	rwServerCert       string
-	rwServerKey        string
-	rwServerClientCA   string
-	rwClientCert       string
-	rwClientKey        string
-	rwClientSecure     bool
-	rwClientServerCA   string
-	rwClientServerName string
-	rwClientSkipVerify bool
+	rwAddress             string
+	rwServerCert          string
+	rwServerKey           string
+	rwServerClientCA      string
+	rwServerTlsMinVersion string
+	rwClientCert          string
+	rwClientKey           string
+	rwClientSecure        bool
+	rwClientServerCA      string
+	rwClientServerName    string
+	rwClientSkipVerify    bool
 
 	dataDir   string
 	labelStrs []string
@@ -861,6 +862,8 @@ func (rc *receiveConfig) registerFlag(cmd extkingpin.FlagClause) {
 
 	cmd.Flag("remote-write.server-tls-client-ca", "TLS CA to verify clients against. If no client CA is specified, there is no client verification on server side. (tls.NoClientCert)").Default("").StringVar(&rc.rwServerClientCA)
 
+	cmd.Flag("remote-write.server-tls-min-version", "TLS version for the gRPC server, leave blank to default to TLS 1.3, allow values: [\"1.0\", \"1.1\", \"1.2\", \"1.3\"]").Default("1.3").StringVar(&rc.rwServerTlsMinVersion)
+
 	cmd.Flag("remote-write.client-tls-cert", "TLS Certificates to use to identify this client to the server.").Default("").StringVar(&rc.rwClientCert)
 
 	cmd.Flag("remote-write.client-tls-key", "TLS Key for the client's certificate.").Default("").StringVar(&rc.rwClientKey)
diff --git a/cmd/thanos/rule.go b/cmd/thanos/rule.go
@@ -722,7 +722,7 @@ func runRule(
 	)
 
 	// Start gRPC server.
-	tlsCfg, err := tls.NewServerConfig(log.With(logger, "protocol", "gRPC"), conf.grpc.tlsSrvCert, conf.grpc.tlsSrvKey, conf.grpc.tlsSrvClientCA)
+	tlsCfg, err := tls.NewServerConfig(log.With(logger, "protocol", "gRPC"), conf.grpc.tlsSrvCert, conf.grpc.tlsSrvKey, conf.grpc.tlsSrvClientCA, conf.grpc.tlsMinVersion)
 	if err != nil {
 		return errors.Wrap(err, "setup gRPC server")
 	}
diff --git a/cmd/thanos/sidecar.go b/cmd/thanos/sidecar.go
@@ -303,7 +303,7 @@ func runSidecar(
 		}
 
 		tlsCfg, err := tls.NewServerConfig(log.With(logger, "protocol", "gRPC"),
-			conf.grpc.tlsSrvCert, conf.grpc.tlsSrvKey, conf.grpc.tlsSrvClientCA)
+			conf.grpc.tlsSrvCert, conf.grpc.tlsSrvKey, conf.grpc.tlsSrvClientCA, conf.grpc.tlsMinVersion)
 		if err != nil {
 			return errors.Wrap(err, "setup gRPC server")
 		}
diff --git a/cmd/thanos/store.go b/cmd/thanos/store.go
@@ -516,7 +516,7 @@ func runStore(
 
 	// Start query (proxy) gRPC StoreAPI.
 	{
-		tlsCfg, err := tls.NewServerConfig(log.With(logger, "protocol", "gRPC"), conf.grpcConfig.tlsSrvCert, conf.grpcConfig.tlsSrvKey, conf.grpcConfig.tlsSrvClientCA)
+		tlsCfg, err := tls.NewServerConfig(log.With(logger, "protocol", "gRPC"), conf.grpcConfig.tlsSrvCert, conf.grpcConfig.tlsSrvKey, conf.grpcConfig.tlsSrvClientCA, conf.grpcConfig.tlsMinVersion)
 		if err != nil {
 			return errors.Wrap(err, "setup gRPC server")
 		}
diff --git a/docs/components/query.md b/docs/components/query.md
@@ -353,6 +353,10 @@ Flags:
                                  verification on server side. (tls.NoClientCert)
       --grpc-server-tls-key=""   TLS Key for the gRPC server, leave blank to
                                  disable TLS
+      --grpc-server-tls-min-version="1.3"
+                                 TLS minimum version to gRPC server, unset will
+                                 default to tls 1.3, allow values: ["1.0",
+                                 "1.1", "1.2", "1.3"]
   -h, --help                     Show context-sensitive help (also try
                                  --help-long and --help-man).
       --http-address="0.0.0.0:10902"
diff --git a/docs/components/receive.md b/docs/components/receive.md
@@ -338,6 +338,10 @@ Flags:
                                  verification on server side. (tls.NoClientCert)
       --grpc-server-tls-key=""   TLS Key for the gRPC server, leave blank to
                                  disable TLS
+      --grpc-server-tls-min-version="1.3"
+                                 TLS minimum version to gRPC server, unset will
+                                 default to tls 1.3, allow values: ["1.0",
+                                 "1.1", "1.2", "1.3"]
       --hash-func=               Specify which hash function to use when
                                  calculating the hashes of produced files.
                                  If no function has been specified, it does not
@@ -460,6 +464,10 @@ Flags:
       --remote-write.server-tls-key=""
                                  TLS Key for the HTTP server, leave blank to
                                  disable TLS.
+      --remote-write.server-tls-min-version="1.3"
+                                 TLS version for the gRPC server, leave blank
+                                 to default to TLS 1.3, allow values: ["1.0",
+                                 "1.1", "1.2", "1.3"]
       --request.logging-config=<content>
                                  Alternative to 'request.logging-config-file'
                                  flag (mutually exclusive). Content
diff --git a/docs/components/rule.md b/docs/components/rule.md
@@ -352,6 +352,10 @@ Flags:
                                  verification on server side. (tls.NoClientCert)
       --grpc-server-tls-key=""   TLS Key for the gRPC server, leave blank to
                                  disable TLS
+      --grpc-server-tls-min-version="1.3"
+                                 TLS minimum version to gRPC server, unset will
+                                 default to tls 1.3, allow values: ["1.0",
+                                 "1.1", "1.2", "1.3"]
       --hash-func=               Specify which hash function to use when
                                  calculating the hashes of produced files.
                                  If no function has been specified, it does not
diff --git a/docs/components/sidecar.md b/docs/components/sidecar.md
@@ -117,6 +117,10 @@ Flags:
                                  verification on server side. (tls.NoClientCert)
       --grpc-server-tls-key=""   TLS Key for the gRPC server, leave blank to
                                  disable TLS
+      --grpc-server-tls-min-version="1.3"
+                                 TLS minimum version to gRPC server, unset will
+                                 default to tls 1.3, allow values: ["1.0",
+                                 "1.1", "1.2", "1.3"]
       --hash-func=               Specify which hash function to use when
                                  calculating the hashes of produced files.
                                  If no function has been specified, it does not
diff --git a/docs/components/store.md b/docs/components/store.md
@@ -110,6 +110,10 @@ Flags:
                                  verification on server side. (tls.NoClientCert)
       --grpc-server-tls-key=""   TLS Key for the gRPC server, leave blank to
                                  disable TLS
+      --grpc-server-tls-min-version="1.3"
+                                 TLS minimum version to gRPC server, unset will
+                                 default to tls 1.3, allow values: ["1.0",
+                                 "1.1", "1.2", "1.3"]
   -h, --help                     Show context-sensitive help (also try
                                  --help-long and --help-man).
       --http-address="0.0.0.0:10902"
diff --git a/pkg/tls/options.go b/pkg/tls/options.go
@@ -6,8 +6,11 @@ package tls
 import (
 	"crypto/tls"
 	"crypto/x509"
+	"fmt"
 	"os"
 	"path/filepath"
+	"sort"
+	"strings"
 	"sync"
 	"time"
 
@@ -17,7 +20,7 @@ import (
 )
 
 // NewServerConfig provides new server TLS configuration.
-func NewServerConfig(logger log.Logger, certPath, keyPath, clientCA string) (*tls.Config, error) {
+func NewServerConfig(logger log.Logger, certPath, keyPath, clientCA, tlsMinVersion string) (*tls.Config, error) {
 	if keyPath == "" && certPath == "" {
 		if clientCA != "" {
 			return nil, errors.New("when a client CA is used a server key and certificate must also be provided")
@@ -33,8 +36,13 @@ func NewServerConfig(logger log.Logger, certPath, keyPath, clientCA string) (*tl
 		return nil, errors.New("both server key and certificate must be provided")
 	}
 
+	minTlsVersion, err := getTlsVersion(tlsMinVersion)
+	if err != nil {
+		return nil, err
+	}
+
 	tlsCfg := &tls.Config{
-		MinVersion: tls.VersionTLS13,
+		MinVersion: minTlsVersion,
 	}
 	// Certificate is loaded during server startup to check for any errors.
 	certificate, err := tls.LoadX509KeyPair(certPath, keyPath)
@@ -190,3 +198,35 @@ func (m *clientTLSManager) getClientCertificate(*tls.CertificateRequestInfo) (*t
 
 	return m.cert, nil
 }
+
+type validOption struct {
+	tlsOption map[string]uint16
+}
+
+func (validOption validOption) joinString() string {
+	var keys []string
+
+	for key := range validOption.tlsOption {
+		keys = append(keys, key)
+	}
+	sort.Strings(keys)
+	return strings.Join(keys, ", ")
+}
+
+func getTlsVersion(tlsMinVersion string) (uint16, error) {
+
+	validOption := validOption{
+		tlsOption: map[string]uint16{
+			"1.0": tls.VersionTLS10,
+			"1.1": tls.VersionTLS11,
+			"1.2": tls.VersionTLS12,
+			"1.3": tls.VersionTLS13,
+		},
+	}
+
+	if _, ok := validOption.tlsOption[tlsMinVersion]; !ok {
+		return 0, errors.New(fmt.Sprintf("invalid TLS version: %s, valid values are %s", tlsMinVersion, validOption.joinString()))
+	}
+
+	return validOption.tlsOption[tlsMinVersion], nil
+}
diff --git a/pkg/tls/options_test.go b/pkg/tls/options_test.go
@@ -0,0 +1,58 @@
+// Copyright (c) The Thanos Authors.
+// Licensed under the Apache License 2.0.
+
+package tls
+
+import (
+	"crypto/tls"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestTlsOptions(t *testing.T) {
+	var tests = []struct {
+		input  string
+		fail   bool
+		result uint16
+	}{
+		{
+			input: "",
+			fail:  true,
+		}, {
+			input: "ab",
+			fail:  true,
+		}, {
+			input: "1",
+			fail:  true,
+		}, {
+			input:  "1.0",
+			result: tls.VersionTLS10,
+		},
+		{
+			input:  "1.1",
+			result: tls.VersionTLS11,
+		},
+		{
+			input:  "1.2",
+			result: tls.VersionTLS12,
+		},
+		{
+			input:  "1.3",
+			result: tls.VersionTLS13,
+		},
+	}
+
+	for _, test := range tests {
+		minTlsVersion, err := getTlsVersion(test.input)
+
+		if test.fail {
+			require.Error(t, err)
+			continue
+		}
+
+		require.NoError(t, err)
+		assert.Equal(t, test.result, minTlsVersion)
+	}
+}
diff --git a/test/e2e/tls_test.go b/test/e2e/tls_test.go
@@ -49,11 +49,12 @@ func TestGRPCServerCertAutoRotate(t *testing.T) {
 	caSrv := filepath.Join(tmpDirSrv, "ca")
 	certSrv := filepath.Join(tmpDirSrv, "cert")
 	keySrv := filepath.Join(tmpDirSrv, "key")
+	tlsMinVersion := "1.3"
 
 	genCerts(t, certSrv, keySrv, caClt)
 	genCerts(t, certClt, keyClt, caSrv)
 
-	configSrv, err := thTLS.NewServerConfig(logger, certSrv, keySrv, caSrv)
+	configSrv, err := thTLS.NewServerConfig(logger, certSrv, keySrv, caSrv, tlsMinVersion)
 	testutil.Ok(t, err)
 
 	srv := grpc.NewServer(grpc.KeepaliveParams(keepalive.ServerParameters{MaxConnectionAge: 1 * time.Millisecond}), grpc.Creds(credentials.NewTLS(configSrv)))
@@ -187,7 +188,8 @@ func TestInvalidCertAndKey(t *testing.T) {
 	caSrv := filepath.Join(tmpDirSrv, "ca")
 	certSrv := filepath.Join(tmpDirSrv, "cert")
 	keySrv := filepath.Join(tmpDirSrv, "key")
+	tlsMinVersion := "1.3"
 	// Certificate and key are not present in the above path
-	_, err := thTLS.NewServerConfig(logger, certSrv, keySrv, caSrv)
+	_, err := thTLS.NewServerConfig(logger, certSrv, keySrv, caSrv, tlsMinVersion)
 	testutil.NotOk(t, err)
 }

Original file line number	Diff line number	Diff line change
`@@ -785,7 +785,7 @@ func runQuery(`
`785`	`785`	`}`
`786`	`786`	`// Start query (proxy) gRPC StoreAPI.`
`787`	`787`	`{`
`788`		`- tlsCfg, err := tls.NewServerConfig(log.With(logger, "protocol", "gRPC"), grpcServerConfig.tlsSrvCert, grpcServerConfig.tlsSrvKey, grpcServerConfig.tlsSrvClientCA)`
	`788`	`+ tlsCfg, err := tls.NewServerConfig(log.With(logger, "protocol", "gRPC"), grpcServerConfig.tlsSrvCert, grpcServerConfig.tlsSrvKey, grpcServerConfig.tlsSrvClientCA, grpcServerConfig.tlsMinVersion)`
`789`	`789`	`if err != nil {`
`790`	`790`	`return errors.Wrap(err, "setup gRPC server")`
`791`	`791`	`}`
Original file line number	Diff line number	Diff line change
`@@ -722,7 +722,7 @@ func runRule(`
`722`	`722`	`)`
`723`	`723`
`724`	`724`	`// Start gRPC server.`
`725`		`- tlsCfg, err := tls.NewServerConfig(log.With(logger, "protocol", "gRPC"), conf.grpc.tlsSrvCert, conf.grpc.tlsSrvKey, conf.grpc.tlsSrvClientCA)`
	`725`	`+ tlsCfg, err := tls.NewServerConfig(log.With(logger, "protocol", "gRPC"), conf.grpc.tlsSrvCert, conf.grpc.tlsSrvKey, conf.grpc.tlsSrvClientCA, conf.grpc.tlsMinVersion)`
`726`	`726`	`if err != nil {`
`727`	`727`	`return errors.Wrap(err, "setup gRPC server")`
`728`	`728`	`}`
Original file line number	Diff line number	Diff line change
`@@ -303,7 +303,7 @@ func runSidecar(`
`303`	`303`	`}`
`304`	`304`
`305`	`305`	`tlsCfg, err := tls.NewServerConfig(log.With(logger, "protocol", "gRPC"),`
`306`		`- conf.grpc.tlsSrvCert, conf.grpc.tlsSrvKey, conf.grpc.tlsSrvClientCA)`
	`306`	`+ conf.grpc.tlsSrvCert, conf.grpc.tlsSrvKey, conf.grpc.tlsSrvClientCA, conf.grpc.tlsMinVersion)`
`307`	`307`	`if err != nil {`
`308`	`308`	`return errors.Wrap(err, "setup gRPC server")`
`309`	`309`	`}`
Original file line number	Diff line number	Diff line change
`@@ -516,7 +516,7 @@ func runStore(`
`516`	`516`
`517`	`517`	`// Start query (proxy) gRPC StoreAPI.`
`518`	`518`	`{`
`519`		`- tlsCfg, err := tls.NewServerConfig(log.With(logger, "protocol", "gRPC"), conf.grpcConfig.tlsSrvCert, conf.grpcConfig.tlsSrvKey, conf.grpcConfig.tlsSrvClientCA)`
	`519`	`+ tlsCfg, err := tls.NewServerConfig(log.With(logger, "protocol", "gRPC"), conf.grpcConfig.tlsSrvCert, conf.grpcConfig.tlsSrvKey, conf.grpcConfig.tlsSrvClientCA, conf.grpcConfig.tlsMinVersion)`
`520`	`520`	`if err != nil {`
`521`	`521`	`return errors.Wrap(err, "setup gRPC server")`
`522`	`522`	`}`