Parse equal-preference cipher groups (#6461)

Signed-off-by: Tero Saarni <tero.saarni@est.tech>
projectcontour · May 24, 2024 · bfffa94 · bfffa94
1 parent 0796cd9
commit bfffa94
Show file tree

Hide file tree

Showing 6 changed files with 58 additions and 16 deletions.
diff --git a/.codespell.ignorewords b/.codespell.ignorewords
@@ -8,3 +8,4 @@ wit
 aks
 immediatedly
 te
+NotIn
diff --git a/apis/projectcontour/v1alpha1/ciphersuites.go b/apis/projectcontour/v1alpha1/ciphersuites.go
@@ -18,6 +18,9 @@ package v1alpha1
 // - most of the clients that might need to use the commented ciphers are
 // unable to connect without TLS 1.0, which contour never enables.
 //
+// Ciphers are listed in order of preference.
+// [cipher1|cipher2|...] defines an equal-preference group of ciphers.
+//
 // This list is ignored if the client and server negotiate TLS 1.3.
 //
 // The commented ciphers are left in place to simplify updating this list for future
@@ -41,18 +44,18 @@ var DefaultTLSCiphers = []string{
 // See: https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/transport_sockets/tls/v3/common.proto#extensions-transport-sockets-tls-v3-tlsparameters
 // Note: This list is a superset of what is valid for stock Envoy builds and those using BoringSSL FIPS.
 var ValidTLSCiphers = map[string]struct{}{
-	"[ECDHE-ECDSA-AES128-GCM-SHA256|ECDHE-ECDSA-CHACHA20-POLY1305]": {},
-	"[ECDHE-RSA-AES128-GCM-SHA256|ECDHE-RSA-CHACHA20-POLY1305]":     {},
-	"ECDHE-ECDSA-AES128-GCM-SHA256":                                 {},
-	"ECDHE-RSA-AES128-GCM-SHA256":                                   {},
-	"ECDHE-ECDSA-AES128-SHA":                                        {},
-	"ECDHE-RSA-AES128-SHA":                                          {},
-	"AES128-GCM-SHA256":                                             {},
-	"AES128-SHA":                                                    {},
-	"ECDHE-ECDSA-AES256-GCM-SHA384":                                 {},
-	"ECDHE-RSA-AES256-GCM-SHA384":                                   {},
-	"ECDHE-ECDSA-AES256-SHA":                                        {},
-	"ECDHE-RSA-AES256-SHA":                                          {},
-	"AES256-GCM-SHA384":                                             {},
-	"AES256-SHA":                                                    {},
+	"ECDHE-ECDSA-CHACHA20-POLY1305": {},
+	"ECDHE-RSA-CHACHA20-POLY1305":   {},
+	"ECDHE-ECDSA-AES128-GCM-SHA256": {},
+	"ECDHE-RSA-AES128-GCM-SHA256":   {},
+	"ECDHE-ECDSA-AES128-SHA":        {},
+	"ECDHE-RSA-AES128-SHA":          {},
+	"AES128-GCM-SHA256":             {},
+	"AES128-SHA":                    {},
+	"ECDHE-ECDSA-AES256-GCM-SHA384": {},
+	"ECDHE-RSA-AES256-GCM-SHA384":   {},
+	"ECDHE-ECDSA-AES256-SHA":        {},
+	"ECDHE-RSA-AES256-SHA":          {},
+	"AES256-GCM-SHA384":             {},
+	"AES256-SHA":                    {},
 }
diff --git a/apis/projectcontour/v1alpha1/contourconfig_helpers.go b/apis/projectcontour/v1alpha1/contourconfig_helpers.go
@@ -185,6 +185,27 @@ func ValidateTLSProtocolVersions(min, max string) error {
 	return nil
 }
 
+// isValidTLSCipher parses a cipher string and returns true if it is valid.
+// We do not support the full syntax defined in the BoringSSL documentation,
+// see https://commondatastorage.googleapis.com/chromium-boringssl-docs/ssl.h.html#Cipher-suite-configuration
+func isValidTLSCipher(cipherSpec string) bool {
+	// Equal-preference group: [cipher1|cipher2|...]
+	if strings.HasPrefix(cipherSpec, "[") && strings.HasSuffix(cipherSpec, "]") {
+		for _, cipher := range strings.Split(strings.Trim(cipherSpec, "[]"), "|") {
+			if _, ok := ValidTLSCiphers[cipher]; !ok {
+				return false
+			}
+		}
+		return true
+	}
+
+	if _, ok := ValidTLSCiphers[cipherSpec]; !ok {
+		return false
+	}
+
+	return true
+}
+
 // Validate ensures EnvoyTLS configuration is valid.
 func (e *EnvoyTLS) Validate() error {
 	if err := ValidateTLSProtocolVersions(e.MinimumProtocolVersion, e.MaximumProtocolVersion); err != nil {
@@ -193,7 +214,7 @@ func (e *EnvoyTLS) Validate() error {
 
 	var invalidCipherSuites []string
 	for _, c := range e.CipherSuites {
-		if _, ok := ValidTLSCiphers[c]; !ok {
+		if !isValidTLSCipher(c) {
 			invalidCipherSuites = append(invalidCipherSuites, c)
 		}
 	}

diff --git a/apis/projectcontour/v1alpha1/contourconfig_helpers_test.go b/apis/projectcontour/v1alpha1/contourconfig_helpers_test.go
@@ -141,8 +141,10 @@ func TestContourConfigurationSpecValidate(t *testing.T) {
 		c.Envoy.Listener.TLS.CipherSuites = []string{
 			"[ECDHE-ECDSA-AES128-GCM-SHA256|ECDHE-ECDSA-CHACHA20-POLY1305]",
 			"ECDHE-ECDSA-AES128-GCM-SHA256",
+			"ECDHE-ECDSA-CHACHA20-POLY1305",
 			"[ECDHE-RSA-AES128-GCM-SHA256|ECDHE-RSA-CHACHA20-POLY1305]",
 			"ECDHE-RSA-AES128-GCM-SHA256",
+			"ECDHE-RSA-CHACHA20-POLY1305",
 			"ECDHE-ECDSA-AES128-SHA",
 			"AES128-GCM-SHA256",
 			"AES128-SHA",
@@ -161,6 +163,20 @@ func TestContourConfigurationSpecValidate(t *testing.T) {
 			"AES128-GCM-SHA256",
 		}
 		require.Error(t, c.Validate())
+
+		// Equal-preference group with invalid cipher.
+		c.Envoy.Listener.TLS.CipherSuites = []string{
+			"[ECDHE-ECDSA-AES128-GCM-SHA256|NOTAVALIDCIPHER]",
+			"ECDHE-ECDSA-AES128-GCM-SHA256",
+		}
+		require.Error(t, c.Validate())
+
+		// Unmatched brackets.
+		c.Envoy.Listener.TLS.CipherSuites = []string{
+			"[ECDHE-ECDSA-AES128-GCM-SHA256|ECDHE-ECDSA-CHACHA20-POLY1305",
+			"ECDHE-ECDSA-AES128-GCM-SHA256",
+		}
+		require.Error(t, c.Validate())
 	})
 
 	t.Run("gateway validation", func(t *testing.T) {

diff --git a/changelogs/unreleased/6461-tsaarni-small.md b/changelogs/unreleased/6461-tsaarni-small.md
@@ -0,0 +1 @@
+Add support for defining equal-preference cipher groups ([cipher1|cipher2|...]) and permit `ECDHE-ECDSA-CHACHA20-POLY1305` and `ECDHE-RSA-CHACHA20-POLY1305` to be used separately.
diff --git a/internal/provisioner/objects/deployment/deployment_test.go b/internal/provisioner/objects/deployment/deployment_test.go
@@ -280,7 +280,7 @@ func TestDesiredDeploymentWhenSettingDisabledFeature(t *testing.T) {
 		disabledFeatures []contour_v1.Feature
 	}{
 		{
-			description:      "disable 2 featuers",
+			description:      "disable 2 features",
 			disabledFeatures: []contour_v1.Feature{"tlsroutes", "grpcroutes"},
 		},
 		{