OCPBUGS-66064: use max TLS version only when defined

This commit ensures that the TLS max version is set only when explicitly
configured from the command-line (or environment variable). In the
previous version, the binary always defaulted to TLS Version 1.2 and it
created an issue with the "modern" TLS profile which defines 1.3 as the
minimum TLS (e.g. min version > max version).

Signed-off-by: Simon Pasquier <[email protected]>

Author: simonpasquier

cmd/plugin-backend.go

@@ -23,7 +23,7 @@ var (
 	logLevelArg         = flag.String("log-level", logrus.InfoLevel.String(), "verbosity of logs\noptions: ['panic', 'fatal', 'error', 'warn', 'info', 'debug', 'trace']\n'trace' level will log all incoming requests\n(default 'error')")
 	alertmanagerUrlArg  = flag.String("alertmanager", "", "alertmanager url to proxy to for acm mode")
 	thanosQuerierUrlArg = flag.String("thanos-querier", "", "thanos querier url to proxy to for acm mode")
-	tlsMinVersionArg    = flag.String("tls-min-version", "", "minimum TLS version\noptions: ['VersionTLS10', 'VersionTLS11', 'VersionTLS12', 'VersionTLS13']\n(default 'VersionTLS12')")
+	tlsMinVersionArg    = flag.String("tls-min-version", "VersionTLS12", "minimum TLS version\noptions: ['VersionTLS10', 'VersionTLS11', 'VersionTLS12', 'VersionTLS13']")
 	tlsMaxVersionArg    = flag.String("tls-max-version", "", "maximum TLS version\noptions: ['VersionTLS10', 'VersionTLS11', 'VersionTLS12', 'VersionTLS13']\n(default is the highest supported by Go)")
 	tlsCipherSuitesArg  = flag.String("tls-cipher-suites", "", "comma-separated list of cipher suites for the server\nvalues are from tls package constants (https://golang.org/pkg/crypto/tls/#pkg-constants)")
 	log                 = logrus.WithField("module", "main")
@@ -62,10 +62,17 @@ func main() {
 
 	log.Infof("enabled features: %+q\n", featuresList)
 
-	// Parse TLS configuration
+	// Parse the TLS configuration.
 	tlsMinVer := parseTLSVersion(tlsMinVersion)
+	log.Infof("Min TLS version: %q", tls.VersionName(tlsMinVer))
 	tlsMaxVer := parseTLSVersion(tlsMaxVersion)
+	if tlsMaxVer != 0 {
+		log.Infof("Max TLS version: %q", tls.VersionName(tlsMaxVer))
+	}
 	tlsCiphers := parseCipherSuites(tlsCipherSuites)
+	if tlsCipherSuites != "" {
+		log.Infof("TLS ciphers: %q", tlsCipherSuites)
+	}
 
 	srv, err := server.CreateServer(context.Background(), &server.Config{
 		Port:             port,
@@ -141,11 +148,10 @@ func getTLSVersionsMap() map[string]uint16 {
 
 func parseTLSVersion(version string) uint16 {
 	if version == "" {
-		return tls.VersionTLS12
+		return 0
 	}
 
 	tlsVersions := getTLSVersionsMap()
-
 	if v, ok := tlsVersions[version]; ok {
 		return v
 	}

pkg/server.go

@@ -145,14 +145,20 @@ func createHTTPServer(ctx context.Context, cfg *Config) (*http.Server, error) {
 	tlsEnabled := cfg.IsTLSEnabled()
 	if tlsEnabled {
 		// Set MinVersion - default to TLS 1.2 if not specified
+		tlsConfig.MinVersion = tls.VersionTLS12
 		if cfg.TLSMinVersion != 0 {
 			tlsConfig.MinVersion = cfg.TLSMinVersion
-		} else {
-			tlsConfig.MinVersion = tls.VersionTLS12
 		}
 
 		if cfg.TLSMaxVersion != 0 {
 			tlsConfig.MaxVersion = cfg.TLSMaxVersion
+			if tlsConfig.MaxVersion < tlsConfig.MinVersion {
+				return nil, fmt.Errorf(
+					"min TLS version %q greater than max TLS version %q",
+					tls.VersionName(tlsConfig.MinVersion),
+					tls.VersionName(tlsConfig.MaxVersion),
+				)
+			}
 		}
 
 		if len(cfg.TLSCipherSuites) > 0 {

pkg/server_test.go

@@ -34,6 +34,43 @@ const (
 	testHostname = "127.0.0.1"
 )
 
+func TestCreateHTTPServer(t *testing.T) {
+	for _, tc := range []struct {
+		cfg *Config
+		err bool
+	}{
+		{
+			// The minimum TLS version is 1.2 by default.
+			cfg: &Config{
+				TLSMaxVersion:  tls.VersionTLS11,
+				CertFile:       "/etc/tls/server.crt",
+				PrivateKeyFile: "/etc/tls/server.key",
+			},
+			err: true,
+		},
+		{
+			cfg: &Config{
+				TLSMinVersion:  tls.VersionTLS13,
+				TLSMaxVersion:  tls.VersionTLS12,
+				CertFile:       "/etc/tls/server.crt",
+				PrivateKeyFile: "/etc/tls/server.key",
+			},
+			err: true,
+		},
+	} {
+		t.Run("", func(t *testing.T) {
+			_, err := createHTTPServer(context.Background(), tc.cfg)
+			if tc.err {
+				require.Error(t, err)
+				return
+			}
+
+			require.NoError(t, err)
+		})
+	}
+
+}
+
 // startTestServer is a helper that starts a server for testing and returns
 // a cleanup function that should be deferred by the caller.
 func startTestServer(t *testing.T, conf *Config) (*PluginServer, func()) {