From 1bc745619d91b690c8985bbc16097e9fe804d2d2 Mon Sep 17 00:00:00 2001
From: kbweave <106193439+kbweave@users.noreply.github.com>
Date: Fri, 5 Jan 2024 07:50:34 -0700
Subject: [PATCH] Include SECLEVEL and STRENGTH as part of ssl-cipher list
 validation (#10754)

---
 .../ingress/annotations/sslcipher/main.go     |  3 ++-
 .../annotations/sslcipher/main_test.go        |  4 ++++
 test/e2e/annotations/sslciphers.go            | 23 +++++++++++++++++++
 3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/internal/ingress/annotations/sslcipher/main.go b/internal/ingress/annotations/sslcipher/main.go
index 685ef90bf3..7a282d0a0c 100644
--- a/internal/ingress/annotations/sslcipher/main.go
+++ b/internal/ingress/annotations/sslcipher/main.go
@@ -32,7 +32,8 @@ const (
 )
 
 // Should cover something like "ALL:!aNULL:!EXPORT56:RC4+RSA:+HIGH:+MEDIUM:+LOW:+SSLv2:+EXP"
-var regexValidSSLCipher = regexp.MustCompile(`^[A-Za-z0-9!:+\-]*$`)
+// (?:@STRENGTH) is included twice so it can appear before or after @SECLEVEL=n
+var regexValidSSLCipher = regexp.MustCompile(`^(?:(?:[A-Za-z0-9!:+\-])*(?:@STRENGTH)*(?:@SECLEVEL=[0-5])*(?:@STRENGTH)*)*$`)
 
 var sslCipherAnnotations = parser.Annotation{
 	Group: "backend",
diff --git a/internal/ingress/annotations/sslcipher/main_test.go b/internal/ingress/annotations/sslcipher/main_test.go
index 20cb4d7af1..f1675d4c77 100644
--- a/internal/ingress/annotations/sslcipher/main_test.go
+++ b/internal/ingress/annotations/sslcipher/main_test.go
@@ -42,6 +42,10 @@ func TestParse(t *testing.T) {
 		expectErr   bool
 	}{
 		{map[string]string{annotationSSLCiphers: "ALL:!aNULL:!EXPORT56:RC4+RSA:+HIGH:+MEDIUM:+LOW:+SSLv2:+EXP"}, Config{"ALL:!aNULL:!EXPORT56:RC4+RSA:+HIGH:+MEDIUM:+LOW:+SSLv2:+EXP", ""}, false},
+		{map[string]string{annotationSSLCiphers: "ALL:!aNULL:!EXPORT56@SECLEVEL=2:RC4+RSA:+HIGH:+MEDIUM:+LOW:+SSLv2:+EXP"}, Config{"ALL:!aNULL:!EXPORT56@SECLEVEL=2:RC4+RSA:+HIGH:+MEDIUM:+LOW:+SSLv2:+EXP", ""}, false},
+		{map[string]string{annotationSSLCiphers: "ALL:!aNULL:!EXPORT56:RC4+RSA:+HIGH:+MEDIUM:+LOW:+SSLv2:+EXP@STRENGTH"}, Config{"ALL:!aNULL:!EXPORT56:RC4+RSA:+HIGH:+MEDIUM:+LOW:+SSLv2:+EXP@STRENGTH", ""}, false},
+		{map[string]string{annotationSSLCiphers: "ALL:!aNULL:!EXPORT56:RC4+RSA:+HIGH:+MEDIUM:+LOW:+SSLv2:+EXP@STRENGTH@SECLEVEL=3"}, Config{"ALL:!aNULL:!EXPORT56:RC4+RSA:+HIGH:+MEDIUM:+LOW:+SSLv2:+EXP@STRENGTH@SECLEVEL=3", ""}, false},
+		{map[string]string{annotationSSLCiphers: "ALL:!aNULL:!EXPORT56:RC4+RSA@STRENGTH:+HIGH@SECLEVEL=5:+MEDIUM:+LOW:+SSLv2:+EXP"}, Config{"ALL:!aNULL:!EXPORT56:RC4+RSA@STRENGTH:+HIGH@SECLEVEL=5:+MEDIUM:+LOW:+SSLv2:+EXP", ""}, false},
 		{
 			map[string]string{annotationSSLCiphers: "ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-SHA384:ECDHE-RSA-AES256-SHA384:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256"},
 			Config{"ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-SHA384:ECDHE-RSA-AES256-SHA384:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256", ""},
diff --git a/test/e2e/annotations/sslciphers.go b/test/e2e/annotations/sslciphers.go
index 58010421df..aece7fc41e 100644
--- a/test/e2e/annotations/sslciphers.go
+++ b/test/e2e/annotations/sslciphers.go
@@ -54,4 +54,27 @@ var _ = framework.DescribeAnnotation("ssl-ciphers", func() {
 			Expect().
 			Status(http.StatusOK)
 	})
+
+	ginkgo.It("should keep ssl ciphers", func() {
+		host := "ciphers.foo.com"
+		annotations := map[string]string{
+			"nginx.ingress.kubernetes.io/ssl-ciphers":               "ALL:!aNULL:!EXPORT56:RC4+RSA@STRENGTH:+HIGH@SECLEVEL=0:+MEDIUM:+LOW:+SSLv2:+EXP",
+			"nginx.ingress.kubernetes.io/ssl-prefer-server-ciphers": "true",
+		}
+
+		ing := framework.NewSingleIngress(host, "/something", host, f.Namespace, framework.EchoService, 80, annotations)
+		f.EnsureIngress(ing)
+
+		f.WaitForNginxServer(host,
+			func(server string) bool {
+				return strings.Contains(server, "ssl_ciphers ALL:!aNULL:!EXPORT56:RC4+RSA@STRENGTH:+HIGH@SECLEVEL=0:+MEDIUM:+LOW:+SSLv2:+EXP;") &&
+					strings.Contains(server, "ssl_prefer_server_ciphers on;")
+			})
+		f.HTTPTestClient().
+			GET("/something").
+			WithURL(f.GetURL(framework.HTTPS)).
+			WithHeader("Host", host).
+			Expect().
+			Status(http.StatusOK)
+	})
 })