allow user to specify tls version for backward compatibility (#7654)

pureiboi · web-flow · commit 62038110b1bc · 2024-10-29T14:58:30.000+02:00
* optional tls version logic

Signed-off-by: pureiboi &lt;17396188+pureiboi@users.noreply.github.com&gt;

* update cmd description and match doc

Signed-off-by: pureiboi &lt;17396188+pureiboi@users.noreply.github.com&gt;

* feat: update doc with make docs

Signed-off-by: pureiboi &lt;17396188+pureiboi@users.noreply.github.com&gt;

* fix indentation by linter

Signed-off-by: pureiboi &lt;17396188+pureiboi@users.noreply.github.com&gt;

---------

Signed-off-by: pureiboi &lt;17396188+pureiboi@users.noreply.github.com&gt;
Signed-off-by: pureiboi &lt;17396188+pureiboi@users.noreply.github.com
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -34,6 +34,7 @@ We use *breaking :warning:* to mark changes that are not backward compatible (re
 - [#7659](https://github.com/thanos-io/thanos/pull/7659) Receive: Add support for replication using [Cap'n Proto](https://capnproto.org/). This protocol has a lower CPU and memory footprint, which leads to a reduction in resource usage in Receivers. Before enabling it, make sure that all receivers are updated to a version which supports this replication method.
 - [#7853](https://github.com/thanos-io/thanos/pull/7853) UI: Add support for selecting graph time range with mouse drag.
 - [#7855](https://github.com/thanos-io/thanos/pull/7855) Compcat/Query: Add support for comma separated replica labels.
+- [#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 supported minimum version for gRPC server. If no version is specified, it'll default to 1.3. Allowed 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
@@ -793,7 +793,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
@@ -149,7 +149,7 @@ func runReceive(
 		}
 	}
 
-	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
 	}
@@ -331,7 +331,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")
 		}
@@ -818,17 +818,18 @@ type receiveConfig struct {
 
 	grpcConfig grpcConfig
 
-	replicationAddr    string
-	rwAddress          string
-	rwServerCert       string
-	rwServerKey        string
-	rwServerClientCA   string
-	rwClientCert       string
-	rwClientKey        string
-	rwClientSecure     bool
-	rwClientServerCA   string
-	rwClientServerName string
-	rwClientSkipVerify bool
+	replicationAddr       string
+	rwAddress             string
+	rwServerCert          string
+	rwServerKey           string
+	rwServerClientCA      string
+	rwClientCert          string
+	rwClientKey           string
+	rwClientSecure        bool
+	rwClientServerCA      string
+	rwClientServerName    string
+	rwClientSkipVerify    bool
+	rwServerTlsMinVersion string
 
 	dataDir   string
 	labelStrs []string
@@ -901,6 +902,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
@@ -730,7 +730,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,11 @@ 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 supported minimum version for gRPC server.
+                                 If no version is specified, it'll default to
+                                 1.3. Allowed 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
@@ -380,6 +380,11 @@ 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 supported minimum version for gRPC server.
+                                 If no version is specified, it'll default to
+                                 1.3. Allowed 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
@@ -508,6 +513,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,11 @@ 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 supported minimum version for gRPC server.
+                                 If no version is specified, it'll default to
+                                 1.3. Allowed 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
@@ -118,6 +118,11 @@ 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 supported minimum version for gRPC server.
+                                 If no version is specified, it'll default to
+                                 1.3. Allowed 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
@@ -111,6 +111,11 @@ 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 supported minimum version for gRPC server.
+                                 If no version is specified, it'll default to
+                                 1.3. Allowed 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
`@@ -793,7 +793,7 @@ func runQuery(`
`793`	`793`	`}`
`794`	`794`	`// Start query (proxy) gRPC StoreAPI.`
`795`	`795`	`{`
`796`		`- tlsCfg, err := tls.NewServerConfig(log.With(logger, "protocol", "gRPC"), grpcServerConfig.tlsSrvCert, grpcServerConfig.tlsSrvKey, grpcServerConfig.tlsSrvClientCA)`
	`796`	`+ tlsCfg, err := tls.NewServerConfig(log.With(logger, "protocol", "gRPC"), grpcServerConfig.tlsSrvCert, grpcServerConfig.tlsSrvKey, grpcServerConfig.tlsSrvClientCA, grpcServerConfig.tlsMinVersion)`
`797`	`797`	`if err != nil {`
`798`	`798`	`return errors.Wrap(err, "setup gRPC server")`
`799`	`799`	`}`
Original file line number	Diff line number	Diff line change
`@@ -730,7 +730,7 @@ func runRule(`
`730`	`730`	`)`
`731`	`731`
`732`	`732`	`// Start gRPC server.`
`733`		`- tlsCfg, err := tls.NewServerConfig(log.With(logger, "protocol", "gRPC"), conf.grpc.tlsSrvCert, conf.grpc.tlsSrvKey, conf.grpc.tlsSrvClientCA)`
	`733`	`+ tlsCfg, err := tls.NewServerConfig(log.With(logger, "protocol", "gRPC"), conf.grpc.tlsSrvCert, conf.grpc.tlsSrvKey, conf.grpc.tlsSrvClientCA, conf.grpc.tlsMinVersion)`
`734`	`734`	`if err != nil {`
`735`	`735`	`return errors.Wrap(err, "setup gRPC server")`
`736`	`736`	`}`
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`	`}`