Be stricter validating custom TLSSecurityProfiles (#2554)

Explicitly fail with a clear error message also when: - minTLSVersion is invalid - custom ciphers are selected with minTLSVersion==VersionTLS13 Signed-off-by: Simone Tiraboschi <[email protected]>
kubevirt · Oct 24, 2023 · 5a5bbc6 · 5a5bbc6
1 parent 2ad0642
commit 5a5bbc6
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 8 deletions.
diff --git a/pkg/webhooks/validator/validator.go b/pkg/webhooks/validator/validator.go
@@ -376,8 +376,14 @@ func (wh *WebhookHandler) validateTLSSecurityProfiles(hc *v1beta1.HyperConverged
 		return nil
 	}
 
+	if !isValidTLSProtocolVersion(tlsSP.Custom.MinTLSVersion) {
+		return fmt.Errorf("invalid value for spec.tlsSecurityProfile.custom.minTLSVersion")
+	}
+
 	if tlsSP.Custom.MinTLSVersion < openshiftconfigv1.VersionTLS13 && !hasRequiredHTTP2Ciphers(tlsSP.Custom.Ciphers) {
 		return fmt.Errorf("http2: TLSConfig.CipherSuites is missing an HTTP/2-required AES_128_GCM_SHA256 cipher (need at least one of ECDHE-RSA-AES128-GCM-SHA256 or ECDHE-ECDSA-AES128-GCM-SHA256)")
+	} else if tlsSP.Custom.MinTLSVersion == openshiftconfigv1.VersionTLS13 && len(tlsSP.Custom.Ciphers) > 0 {
+		return fmt.Errorf("custom ciphers cannot be selected when minTLSVersion is VersionTLS13")
 	}
 
 	return nil
@@ -439,3 +445,15 @@ func SelectCipherSuitesAndMinTLSVersion() ([]string, openshiftconfigv1.TLSProtoc
 
 	return openshiftconfigv1.TLSProfiles[profile.Type].Ciphers, openshiftconfigv1.TLSProfiles[profile.Type].MinTLSVersion
 }
+
+func isValidTLSProtocolVersion(pv openshiftconfigv1.TLSProtocolVersion) bool {
+	switch pv {
+	case
+		openshiftconfigv1.VersionTLS10,
+		openshiftconfigv1.VersionTLS11,
+		openshiftconfigv1.VersionTLS12,
+		openshiftconfigv1.VersionTLS13:
+		return true
+	}
+	return false
+}
diff --git a/pkg/webhooks/validator/validator_test.go b/pkg/webhooks/validator/validator_test.go
@@ -446,16 +446,28 @@ var _ = Describe("webhooks validator", func() {
 			)
 
 			It("should fail if does not have any of the HTTP/2-required ciphers", func() {
-				Expect(
-					updateTLSSecurityProfile(openshiftconfigv1.VersionTLS12, []string{"DHE-RSA-AES256-GCM-SHA384", "DHE-RSA-CHACHA20-POLY1305"}),
-				).ToNot(Succeed())
+				err := updateTLSSecurityProfile(openshiftconfigv1.VersionTLS12, []string{"DHE-RSA-AES256-GCM-SHA384", "DHE-RSA-CHACHA20-POLY1305"})
+				Expect(err).To(HaveOccurred())
+				Expect(err.Error()).Should(ContainSubstring("http2: TLSConfig.CipherSuites is missing an HTTP/2-required AES_128_GCM_SHA256 cipher (need at least one of ECDHE-RSA-AES128-GCM-SHA256 or ECDHE-ECDSA-AES128-GCM-SHA256)"))
 			})
 
 			It("should succeed if does not have any of the HTTP/2-required ciphers but TLS version >= 1.3", func() {
 				Expect(
-					updateTLSSecurityProfile(openshiftconfigv1.VersionTLS13, []string{"DHE-RSA-AES256-GCM-SHA384", "DHE-RSA-CHACHA20-POLY1305"}),
+					updateTLSSecurityProfile(openshiftconfigv1.VersionTLS13, []string{}),
 				).To(Succeed())
 			})
+
+			It("should fail if does have custom ciphers with TLS version >= 1.3", func() {
+				err := updateTLSSecurityProfile(openshiftconfigv1.VersionTLS13, []string{"TLS_AES_128_GCM_SHA256", "TLS_CHACHA20_POLY1305_SHA256"})
+				Expect(err).To(HaveOccurred())
+				Expect(err.Error()).Should(ContainSubstring("custom ciphers cannot be selected when minTLSVersion is VersionTLS13"))
+			})
+
+			It("should fail when minTLSVersion is invalid", func() {
+				err := updateTLSSecurityProfile("invalidProtocolVersion", []string{"TLS_AES_128_GCM_SHA256", "TLS_CHACHA20_POLY1305_SHA256"})
+				Expect(err).To(HaveOccurred())
+				Expect(err.Error()).Should(ContainSubstring("invalid value for spec.tlsSecurityProfile.custom.minTLSVersion"))
+			})
 		})
 
 		Context("validate feature gates", func() {
@@ -1080,16 +1092,28 @@ var _ = Describe("webhooks validator", func() {
 			)
 
 			It("should fail if does not have any of the HTTP/2-required ciphers", func() {
-				Expect(
-					updateTLSSecurityProfile(openshiftconfigv1.VersionTLS12, []string{"DHE-RSA-AES256-GCM-SHA384", "DHE-RSA-CHACHA20-POLY1305"}),
-				).ToNot(Succeed())
+				err := updateTLSSecurityProfile(openshiftconfigv1.VersionTLS12, []string{"DHE-RSA-AES256-GCM-SHA384", "DHE-RSA-CHACHA20-POLY1305"})
+				Expect(err).To(HaveOccurred())
+				Expect(err.Error()).Should(ContainSubstring("http2: TLSConfig.CipherSuites is missing an HTTP/2-required AES_128_GCM_SHA256 cipher (need at least one of ECDHE-RSA-AES128-GCM-SHA256 or ECDHE-ECDSA-AES128-GCM-SHA256)"))
 			})
 
 			It("should succeed if does not have any of the HTTP/2-required ciphers but TLS version >= 1.3", func() {
 				Expect(
-					updateTLSSecurityProfile(openshiftconfigv1.VersionTLS13, []string{"DHE-RSA-AES256-GCM-SHA384", "DHE-RSA-CHACHA20-POLY1305"}),
+					updateTLSSecurityProfile(openshiftconfigv1.VersionTLS13, []string{}),
 				).To(Succeed())
 			})
+
+			It("should fail if does have custom ciphers with TLS version >= 1.3", func() {
+				err := updateTLSSecurityProfile(openshiftconfigv1.VersionTLS13, []string{"TLS_AES_128_GCM_SHA256", "TLS_CHACHA20_POLY1305_SHA256"})
+				Expect(err).To(HaveOccurred())
+				Expect(err.Error()).Should(ContainSubstring("custom ciphers cannot be selected when minTLSVersion is VersionTLS13"))
+			})
+
+			It("should fail when minTLSVersion is invalid", func() {
+				err := updateTLSSecurityProfile("invalidProtocolVersion", []string{"TLS_AES_128_GCM_SHA256", "TLS_CHACHA20_POLY1305_SHA256"})
+				Expect(err).To(HaveOccurred())
+				Expect(err.Error()).Should(ContainSubstring("invalid value for spec.tlsSecurityProfile.custom.minTLSVersion"))
+			})
 		})
 
 		Context("validate feature gates", func() {