Fixing incorrect default TLS min and TLS max versions (#5480)

* Fixing incorrect TLS min and TLS max Versions by default

* Added test cases for TLS min and TLS max versions

* Added entry to changelog

* Added Invalid TLS test cases

* TLS MinVersion and TLS MaxVersion defaults to be handled by crypto/tls

* Removed description that can go out of sync

CHANGELOG.md

@@ -85,6 +85,7 @@
 
 - Fixes the "service.version" label value for internal metrics, always was "latest" in core/contrib distros. (#5449).
 - Send correct batch stats when SendBatchMaxSize is set (#5385)
+- TLS `MinVersion` and `MaxVersion` defaults will be handled by `crypto/tls` (#5480)
 
 ## v0.52.0 Beta
 

config/configtls/README.md

@@ -36,10 +36,11 @@ won't use TLS at all.
 
 Minimum and maximum TLS version can be set:
 
-- `min_version` (default = "1.2"): Minimum acceptable TLS version.
-It's recommended to use at least 1.2 as the minimum version.
+- `min_version` (default = "" handled by [crypto/tls](https://github.com/golang/go/blob/master/src/crypto/tls/common.go#L694)): Minimum acceptable TLS version.
+  - options: ["1.0", "1.1", "1.2", "1.3"]
 
-- `max_version` (default = "1.3"): Maximum acceptable TLS version.
+- `max_version` (default = "" handled by [crypto/tls](https://github.com/golang/go/blob/master/src/crypto/tls/common.go#L700)): Maximum acceptable TLS version.
+  - options: ["1.0", "1.1", "1.2", "1.3"]
 
 Additionally certifaces may be reloaded by setting the below configuration.
 

config/configtls/configtls.go

@@ -41,11 +41,11 @@ type TLSSetting struct {
 	KeyFile string `mapstructure:"key_file"`
 
 	// MinVersion sets the minimum TLS version that is acceptable.
-	// If not set, TLS 1.0 is used. (optional)
+	// If not set, refer to crypto/tls for defaults. (optional)
 	MinVersion string `mapstructure:"min_version"`
 
 	// MaxVersion sets the maximum TLS version that is acceptable.
-	// If not set, TLS 1.3 is used. (optional)
+	// If not set, refer to crypto/tls for defaults. (optional)
 	MaxVersion string `mapstructure:"max_version"`
 
 	// ReloadInterval specifies the duration after which the certificate will be reloaded
@@ -240,8 +240,9 @@ func (c TLSServerSetting) LoadTLSConfig() (*tls.Config, error) {
 }
 
 func convertVersion(v string) (uint16, error) {
+	// Defaults will be handled by go/crypto/tls
 	if v == "" {
-		return tls.VersionTLS12, nil // default
+		return 0, nil
 	}
 	val, ok := tlsVersions[v]
 	if !ok {

config/configtls/configtls_test.go

@@ -351,3 +351,44 @@ func TestCertificateReload(t *testing.T) {
 		})
 	}
 }
+
+func TestMinMaxTLSVersions(t *testing.T) {
+	tests := []struct {
+		name          string
+		minVersion    string
+		maxVersion    string
+		outMinVersion uint16
+		outMaxVersion uint16
+		errorTxt      string
+	}{
+		{name: `TLS Config ["", ""] to give [0, 0]`, minVersion: "", maxVersion: "", outMinVersion: 0, outMaxVersion: 0},
+		{name: `TLS Config ["", "1.3"] to give [0, TLS1.3]`, minVersion: "", maxVersion: "1.3", outMinVersion: 0, outMaxVersion: tls.VersionTLS13},
+		{name: `TLS Config ["1.2", ""] to give [TLS1.2, 0]`, minVersion: "1.2", maxVersion: "", outMinVersion: tls.VersionTLS12, outMaxVersion: 0},
+		{name: `TLS Config ["1.3", "1.3"] to give [TLS1.3, TLS1.3]`, minVersion: "1.3", maxVersion: "1.3", outMinVersion: tls.VersionTLS13, outMaxVersion: tls.VersionTLS13},
+		{name: `TLS Config ["1.0", "1.1"] to give [TLS1.0, TLS1.1]`, minVersion: "1.0", maxVersion: "1.1", outMinVersion: tls.VersionTLS10, outMaxVersion: tls.VersionTLS11},
+		{name: `TLS Config ["asd", ""] to give [Error]`, minVersion: "asd", maxVersion: "", errorTxt: `invalid TLS min_version: unsupported TLS version: "asd"`},
+		{name: `TLS Config ["", "asd"] to give [Error]`, minVersion: "", maxVersion: "asd", errorTxt: `invalid TLS max_version: unsupported TLS version: "asd"`},
+		{name: `TLS Config ["0.4", ""] to give [Error]`, minVersion: "0.4", maxVersion: "", errorTxt: `invalid TLS min_version: unsupported TLS version: "0.4"`},
+
+		// Allowing this, however, expecting downstream TLS handshake will throw an error
+		{name: `TLS Config ["1.2", "1.1"] to give [TLS1.2, TLS1.1]`, minVersion: "1.2", maxVersion: "1.1", outMinVersion: tls.VersionTLS12, outMaxVersion: tls.VersionTLS11},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			setting := TLSSetting{
+				MinVersion: test.minVersion,
+				MaxVersion: test.maxVersion,
+			}
+
+			config, err := setting.loadTLSConfig()
+
+			if test.errorTxt == "" {
+				assert.Equal(t, config.MinVersion, test.outMinVersion)
+				assert.Equal(t, config.MaxVersion, test.outMaxVersion)
+			} else {
+				assert.EqualError(t, err, test.errorTxt)
+			}
+		})
+	}
+}