Support min/max TLS version in TLS config (#3567)

* Add minimum tls version support in tls config Signed-off-by: Ashmita152 <ashmita.bohara@logz.io> * Feedbacks Signed-off-by: Ashmita152 <ashmita.bohara@logz.io> * Add maximum tls version support in tls config Signed-off-by: Ashmita152 <ashmita.bohara@logz.io> * Feedbacks Signed-off-by: Ashmita152 <ashmita.bohara@logz.io>
jaegertracing · Mar 7, 2022 · 46bced6 · 46bced6
1 parent df44af3
commit 46bced6
Show file tree

Hide file tree

Showing 6 changed files with 109 additions and 4 deletions.
diff --git a/pkg/config/tlscfg/ciphersuites.go b/pkg/config/tlscfg/ciphersuites.go
@@ -19,6 +19,14 @@ import (
 	"fmt"
 )
 
+// https://pkg.go.dev/crypto/tls#pkg-constants
+var versions = map[string]uint16{
+	"1.0": tls.VersionTLS10,
+	"1.1": tls.VersionTLS11,
+	"1.2": tls.VersionTLS12,
+	"1.3": tls.VersionTLS13,
+}
+
 func allCiphers() map[string]uint16 {
 	acceptedCiphers := make(map[string]uint16)
 	for _, suite := range tls.CipherSuites() {
@@ -40,3 +48,11 @@ func CipherSuiteNamesToIDs(cipherNames []string) ([]uint16, error) {
 	}
 	return ciphersIDs, nil
 }
+
+// VersionNameToID returns the version ID from version name
+func VersionNameToID(versionName string) (uint16, error) {
+	if version, ok := versions[versionName]; ok {
+		return version, nil
+	}
+	return 0, fmt.Errorf("unknown tls version %q", versionName)
+}
diff --git a/pkg/config/tlscfg/ciphersuites_test.go b/pkg/config/tlscfg/ciphersuites_test.go
@@ -20,7 +20,7 @@ import (
 	"testing"
 )
 
-func TestTLSCipherSuites(t *testing.T) {
+func TestCipherSuiteNamesToIDs(t *testing.T) {
 	tests := []struct {
 		flag          []string
 		expected      []uint16
@@ -68,3 +68,34 @@ func TestTLSCipherSuites(t *testing.T) {
 		}
 	}
 }
+
+func TestVersionNameToID(t *testing.T) {
+	tests := []struct {
+		flag          string
+		expected      uint16
+		expectedError bool
+	}{
+		{
+			// Happy case
+			flag:          "1.1",
+			expected:      tls.VersionTLS11,
+			expectedError: false,
+		},
+		{
+			// Invalid flag
+			flag:          "Invalid",
+			expected:      0,
+			expectedError: true,
+		},
+	}
+
+	for i, test := range tests {
+		uIntFlag, err := VersionNameToID(test.flag)
+		if !reflect.DeepEqual(uIntFlag, test.expected) {
+			t.Errorf("%d: expected %+v, got %+v", i, test.expected, uIntFlag)
+		}
+		if test.expectedError && err == nil {
+			t.Errorf("%d: expecting error, got %+v", i, err)
+		}
+	}
+}
diff --git a/pkg/config/tlscfg/flags.go b/pkg/config/tlscfg/flags.go
@@ -31,6 +31,8 @@ const (
 	tlsClientCA       = tlsPrefix + ".client-ca"
 	tlsSkipHostVerify = tlsPrefix + ".skip-host-verify"
 	tlsCipherSuites   = tlsPrefix + ".cipher-suites"
+	tlsMinVersion     = tlsPrefix + ".min-version"
+	tlsMaxVersion     = tlsPrefix + ".max-version"
 )
 
 // ClientFlagsConfig describes which CLI flags for TLS client should be generated.
@@ -60,6 +62,8 @@ func (c ServerFlagsConfig) AddFlags(flags *flag.FlagSet) {
 	flags.String(c.Prefix+tlsKey, "", "Path to a TLS Private Key file, used to identify this server to clients")
 	flags.String(c.Prefix+tlsClientCA, "", "Path to a TLS CA (Certification Authority) file used to verify certificates presented by clients (if unset, all clients are permitted)")
 	flags.String(c.Prefix+tlsCipherSuites, "", "Comma-separated list of cipher suites for the server, values are from tls package constants (https://golang.org/pkg/crypto/tls/#pkg-constants).")
+	flags.String(c.Prefix+tlsMinVersion, "", "Minimum TLS version supported (Possible values: 1.0, 1.1, 1.2, 1.3)")
+	flags.String(c.Prefix+tlsMaxVersion, "", "Maximum TLS version supported (Possible values: 1.0, 1.1, 1.2, 1.3)")
 }
 
 // InitFromViper creates tls.Config populated with values retrieved from Viper.
@@ -82,6 +86,8 @@ func (c ServerFlagsConfig) InitFromViper(v *viper.Viper) Options {
 	p.KeyPath = v.GetString(c.Prefix + tlsKey)
 	p.ClientCAPath = v.GetString(c.Prefix + tlsClientCA)
 	p.CipherSuites = strings.Split(stripWhiteSpace(v.GetString(c.Prefix+tlsCipherSuites)), ",")
+	p.MinVersion = v.GetString(c.Prefix + tlsMinVersion)
+	p.MaxVersion = v.GetString(c.Prefix + tlsMaxVersion)
 	return p
 }
 

diff --git a/pkg/config/tlscfg/flags_test.go b/pkg/config/tlscfg/flags_test.go
@@ -75,6 +75,8 @@ func TestServerFlags(t *testing.T) {
 		"--prefix.tls.cert=cert-file",
 		"--prefix.tls.key=key-file",
 		"--prefix.tls.cipher-suites=TLS_AES_256_GCM_SHA384,TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA",
+		"--prefix.tls.min-version=1.2",
+		"--prefix.tls.max-version=1.3",
 	}
 
 	tests := []struct {
@@ -109,6 +111,8 @@ func TestServerFlags(t *testing.T) {
 				KeyPath:      "key-file",
 				ClientCAPath: test.file,
 				CipherSuites: []string{"TLS_AES_256_GCM_SHA384", "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA"},
+				MinVersion:   "1.2",
+				MaxVersion:   "1.3",
 			}, tlsOpts)
 		})
 	}

diff --git a/pkg/config/tlscfg/options.go b/pkg/config/tlscfg/options.go
@@ -34,6 +34,8 @@ type Options struct {
 	ServerName     string       `mapstructure:"server_name"` // only for client-side TLS config
 	ClientCAPath   string       `mapstructure:"client_ca"`   // only for server-side TLS config for client auth
 	CipherSuites   []string     `mapstructure:"cipher_suites"`
+	MinVersion     string       `mapstructure:"min_version"`
+	MaxVersion     string       `mapstructure:"max_version"`
 	SkipHostVerify bool         `mapstructure:"skip_host_verify"`
 	certWatcher    *certWatcher `mapstructure:"-"`
 }
@@ -42,6 +44,8 @@ var systemCertPool = x509.SystemCertPool // to allow overriding in unit test
 
 // Config loads TLS certificates and returns a TLS Config.
 func (p *Options) Config(logger *zap.Logger) (*tls.Config, error) {
+	var minVersionId, maxVersionId uint16
+
 	certPool, err := p.loadCertPool()
 	if err != nil {
 		return nil, fmt.Errorf("failed to load CA CertPool: %w", err)
@@ -52,12 +56,34 @@ func (p *Options) Config(logger *zap.Logger) (*tls.Config, error) {
 		return nil, fmt.Errorf("failed to get cipher suite ids from cipher suite names: %w", err)
 	}
 
+	if p.MinVersion != "" {
+		minVersionId, err = VersionNameToID(p.MinVersion)
+		if err != nil {
+			return nil, fmt.Errorf("failed to get minimum tls version: %w", err)
+		}
+	}
+
+	if p.MaxVersion != "" {
+		maxVersionId, err = VersionNameToID(p.MaxVersion)
+		if err != nil {
+			return nil, fmt.Errorf("failed to get maximum tls version: %w", err)
+		}
+	}
+
+	if p.MinVersion != "" && p.MaxVersion != "" {
+		if minVersionId > maxVersionId {
+			return nil, fmt.Errorf("minimum tls version can't be greater than maximum tls version")
+		}
+	}
+
 	// #nosec G402
 	tlsCfg := &tls.Config{
 		RootCAs:            certPool,
 		ServerName:         p.ServerName,
 		InsecureSkipVerify: p.SkipHostVerify,
 		CipherSuites:       cipherSuiteIds,
+		MinVersion:         minVersionId,
+		MaxVersion:         maxVersionId,
 	}
 
 	if p.ClientCAPath != "" {

diff --git a/pkg/config/tlscfg/options_test.go b/pkg/config/tlscfg/options_test.go
@@ -109,25 +109,47 @@ func TestOptionsToConfig(t *testing.T) {
 			expectError: "failed to load CA",
 		},
 		{
-			name: "should fail with invalid Client CA pool",
+			name: "should fail with invalid TLS Client CA pool",
 			options: Options{
 				ClientCAPath: testCertKeyLocation + "/bad-CA-cert.txt",
 			},
 			expectError: "failed to parse CA",
 		},
 		{
-			name: "should pass with valid Client CA pool",
+			name: "should pass with valid TLS Client CA pool",
 			options: Options{
 				ClientCAPath: testCertKeyLocation + "/example-CA-cert.pem",
 			},
 		},
 		{
-			name: "should fail with invalid Cipher Suite",
+			name: "should fail with invalid TLS Cipher Suite",
 			options: Options{
 				CipherSuites: []string{"TLS_INVALID_CIPHER_SUITE"},
 			},
 			expectError: "failed to get cipher suite ids from cipher suite names: cipher suite TLS_INVALID_CIPHER_SUITE not supported or doesn't exist",
 		},
+		{
+			name: "should fail with invalid TLS Min Version",
+			options: Options{
+				MinVersion: "Invalid",
+			},
+			expectError: "failed to get minimum tls version",
+		},
+		{
+			name: "should fail with invalid TLS Max Version",
+			options: Options{
+				MaxVersion: "Invalid",
+			},
+			expectError: "failed to get maximum tls version",
+		},
+		{
+			name: "should fail with TLS Min Version greater than TLS Max Version error",
+			options: Options{
+				MinVersion: "1.2",
+				MaxVersion: "1.1",
+			},
+			expectError: "minimum tls version can't be greater than maximum tls version",
+		},
 	}
 
 	for _, test := range tests {