Merge pull request #660 from simonpasquier/OCPBUGS-66064

OCPBUGS-66064: configure max TLS version only when specified

Author: openshift-merge-bot[bot]

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()) {