Enable security features by default (#11819)

.github/workflows/ci.yaml

@@ -258,7 +258,7 @@ jobs:
 
     strategy:
       matrix:
-        k8s: [v1.26.15, v1.27.13, v1.28.9, v1.29.4, v1.30.0]
+        k8s: [v1.28.13, v1.29.8, v1.30.4, v1.31.0]
 
     steps:
       - name: Checkout
@@ -309,26 +309,11 @@ jobs:
       (needs.changes.outputs.go == 'true') || (needs.changes.outputs.baseimage == 'true') || ${{ github.event.workflow_dispatch.run_e2e == 'true' }}
     strategy:
       matrix:
-        k8s: [v1.26.15, v1.27.13, v1.28.9, v1.29.4, v1.30.0]
+        k8s: [v1.28.13, v1.29.8, v1.30.4, v1.31.0]
     uses: ./.github/workflows/zz-tmpl-k8s-e2e.yaml
     with:
       k8s-version: ${{ matrix.k8s }}
 
-  kubernetes-validations:
-    name: Kubernetes with Validations
-    needs:
-      - changes
-      - build
-    if: |
-      (needs.changes.outputs.go == 'true') || (needs.changes.outputs.baseimage == 'true') || ${{ github.event.workflow_dispatch.run_e2e == 'true' }}
-    strategy:
-      matrix:
-        k8s: [v1.26.15, v1.27.13, v1.28.9, v1.29.4, v1.30.0]
-    uses: ./.github/workflows/zz-tmpl-k8s-e2e.yaml
-    with:
-      k8s-version: ${{ matrix.k8s }}
-      variation: "VALIDATIONS"
-
   kubernetes-chroot:
     name: Kubernetes chroot
     needs:
@@ -338,7 +323,7 @@ jobs:
       (needs.changes.outputs.go == 'true') || (needs.changes.outputs.baseimage == 'true') || ${{ github.event.workflow_dispatch.run_e2e == 'true' }}
     strategy:
       matrix:
-        k8s: [v1.26.15, v1.27.13, v1.28.9, v1.29.4, v1.30.0]
+        k8s: [v1.28.13, v1.29.8, v1.30.4, v1.31.0]
     uses: ./.github/workflows/zz-tmpl-k8s-e2e.yaml
     with:
       k8s-version: ${{ matrix.k8s }}

.github/workflows/zz-tmpl-k8s-e2e.yaml

@@ -43,7 +43,6 @@ jobs:
           SKIP_CLUSTER_CREATION: true
           SKIP_INGRESS_IMAGE_CREATION: true
           SKIP_E2E_IMAGE_CREATION: true
-          ENABLE_VALIDATIONS: ${{ inputs.variation == 'VALIDATIONS' }}
           IS_CHROOT: ${{ inputs.variation == 'CHROOT' }}
         run: |
           kind get kubeconfig > $HOME/.kube/kind-config-kind

charts/ingress-nginx/README.md

@@ -304,7 +304,7 @@ As of version `1.26.0` of this chart, by simply not providing any clusterIP valu
 | controller.dnsPolicy | string | `"ClusterFirst"` | Optionally change this to ClusterFirstWithHostNet in case you have 'hostNetwork: true'. By default, while using host network, name resolution uses the host's DNS. If you wish nginx-controller to keep resolving names inside the k8s network, use ClusterFirstWithHostNet. |
 | controller.electionID | string | `""` | Election ID to use for status update, by default it uses the controller name combined with a suffix of 'leader' |
 | controller.electionTTL | string | `""` | Duration a leader election is valid before it's getting re-elected, e.g. `15s`, `10m` or `1h`. (Default: 30s) |
-| controller.enableAnnotationValidations | bool | `false` |  |
+| controller.enableAnnotationValidations | bool | `true` |  |
 | controller.enableMimalloc | bool | `true` | Enable mimalloc as a drop-in replacement for malloc. # ref: https://github.com/microsoft/mimalloc # |
 | controller.enableTopologyAwareRouting | bool | `false` | This configuration enables Topology Aware Routing feature, used together with service annotation service.kubernetes.io/topology-mode="auto" Defaults to false |
 | controller.existingPsp | string | `""` | Use an existing PSP instead of creating one |

charts/ingress-nginx/values.yaml

@@ -17,7 +17,7 @@ commonLabels: {}
 
 controller:
   name: controller
-  enableAnnotationValidations: false
+  enableAnnotationValidations: true
   image:
     ## Keep false as default for now!
     chroot: false

docs/user-guide/cli-arguments.md

@@ -15,7 +15,7 @@ They are set in the container spec of the `ingress-nginx-controller` Deployment
 | `--default-backend-service`        | Service used to serve HTTP requests not matching any known server name (catch-all). Takes the form "namespace/name". The controller configures NGINX to forward requests to the first port of this Service. |
 | `--default-server-port`            | Port to use for exposing the default server (catch-all). (default 8181) |
 | `--default-ssl-certificate`        | Secret containing a SSL certificate to be used by the default HTTPS server (catch-all). Takes the form "namespace/name". |
-| `--enable-annotation-validation`  | If true, will enable the annotation validation feature. This value will be defaulted to true on a future release. |
+| `--enable-annotation-validation`  | If true, will enable the annotation validation feature. Defaults to true |
 | `--disable-catch-all`              | Disable support for catch-all Ingresses. (default false) |
 | `--disable-full-test` | Disable full test of all merged ingresses at the admission stage and tests the template of the ingress being created or updated  (full test of all ingresses is enabled by default). |
 | `--disable-svc-external-name` | Disable support for Services of type ExternalName. (default false) |

internal/ingress/controller/config/config.go

@@ -776,10 +776,10 @@ func NewDefault() Configuration {
 
 	cfg := Configuration{
 		AllowSnippetAnnotations:          false,
-		AllowCrossNamespaceResources:     true,
+		AllowCrossNamespaceResources:     false,
 		AllowBackendServerHeader:         false,
 		AnnotationValueWordBlocklist:     "",
-		AnnotationsRiskLevel:             "Critical",
+		AnnotationsRiskLevel:             "High",
 		AccessLogPath:                    "/var/log/nginx/access.log",
 		AccessLogParams:                  "",
 		EnableAccessLogForDefaultBackend: false,
@@ -924,7 +924,7 @@ func NewDefault() Configuration {
 		GlobalRateLimitMemcachedPoolSize:       50,
 		GlobalRateLimitStatusCode:              429,
 		DebugConnections:                       []string{},
-		StrictValidatePathType:                 false, // TODO: This will be true in future releases
+		StrictValidatePathType:                 true,
 		GRPCBufferSizeKb:                       0,
 	}
 

pkg/flags/flags.go

@@ -158,7 +158,7 @@ Requires the update-status parameter.`)
 		annotationsPrefix = flags.String("annotations-prefix", parser.DefaultAnnotationsPrefix,
 			`Prefix of the Ingress annotations specific to the NGINX controller.`)
 
-		enableAnnotationValidation = flags.Bool("enable-annotation-validation", false,
+		enableAnnotationValidation = flags.Bool("enable-annotation-validation", true,
 			`If true, will enable the annotation validation feature. This value will be defaulted to true on a future release`)
 
 		enableSSLChainCompletion = flags.Bool("enable-ssl-chain-completion", false,

test/e2e/admission/admission.go

@@ -127,14 +127,8 @@ var _ = framework.IngressNginxDescribeSerial("[Admission] admission controller",
 	})
 
 	ginkgo.It("should return an error if there is an error validating the ingress definition", func() {
-		f.SetNginxConfigMapData(map[string]string{
-			"allow-snippet-annotations": "true",
-		})
-		defer func() {
-			f.SetNginxConfigMapData(map[string]string{
-				"allow-snippet-annotations": "false",
-			})
-		}()
+		disableSnippet := f.AllowSnippetConfiguration()
+		defer disableSnippet()
 
 		host := admissionTestHost
 
@@ -241,14 +235,8 @@ var _ = framework.IngressNginxDescribeSerial("[Admission] admission controller",
 	})
 
 	ginkgo.It("should return an error if the Ingress V1 definition contains invalid annotations", func() {
-		f.SetNginxConfigMapData(map[string]string{
-			"allow-snippet-annotations": "true",
-		})
-		defer func() {
-			f.SetNginxConfigMapData(map[string]string{
-				"allow-snippet-annotations": "false",
-			})
-		}()
+		disableSnippet := f.AllowSnippetConfiguration()
+		defer disableSnippet()
 
 		out, err := createIngress(f.Namespace, invalidV1Ingress)
 		assert.Empty(ginkgo.GinkgoT(), out)
@@ -261,14 +249,8 @@ var _ = framework.IngressNginxDescribeSerial("[Admission] admission controller",
 	})
 
 	ginkgo.It("should not return an error for an invalid Ingress when it has unknown class", func() {
-		f.SetNginxConfigMapData(map[string]string{
-			"allow-snippet-annotations": "true",
-		})
-		defer func() {
-			f.SetNginxConfigMapData(map[string]string{
-				"allow-snippet-annotations": "false",
-			})
-		}()
+		disableSnippet := f.AllowSnippetConfiguration()
+		defer disableSnippet()
 		out, err := createIngress(f.Namespace, invalidV1IngressWithOtherClass)
 		assert.Equal(ginkgo.GinkgoT(), "ingress.networking.k8s.io/extensions-invalid-other created\n", out)
 		assert.Nil(ginkgo.GinkgoT(), err, "creating an invalid ingress with unknown class using kubectl")

test/e2e/annotations/auth.go

@@ -277,14 +277,8 @@ var _ = framework.DescribeAnnotation("auth-*", func() {
 			"nginx.ingress.kubernetes.io/auth-snippet": `
 				proxy_set_header My-Custom-Header 42;`,
 		}
-		f.SetNginxConfigMapData(map[string]string{
-			"allow-snippet-annotations": "true",
-		})
-		defer func() {
-			f.SetNginxConfigMapData(map[string]string{
-				"allow-snippet-annotations": "false",
-			})
-		}()
+		disableSnippet := f.AllowSnippetConfiguration()
+		defer disableSnippet()
 
 		ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, annotations)
 		f.EnsureIngress(ing)
@@ -297,15 +291,8 @@ var _ = framework.DescribeAnnotation("auth-*", func() {
 
 	ginkgo.It(`should not set snippet "proxy_set_header My-Custom-Header 42;" when external auth is not configured`, func() {
 		host := authHost
-
-		f.SetNginxConfigMapData(map[string]string{
-			"allow-snippet-annotations": "true",
-		})
-		defer func() {
-			f.SetNginxConfigMapData(map[string]string{
-				"allow-snippet-annotations": "false",
-			})
-		}()
+		disableSnippet := f.AllowSnippetConfiguration()
+		defer disableSnippet()
 
 		annotations := map[string]string{
 			"nginx.ingress.kubernetes.io/auth-snippet": `

test/e2e/annotations/fromtowwwredirect.go

@@ -62,14 +62,8 @@ var _ = framework.DescribeAnnotation("from-to-www-redirect", func() {
 	})
 
 	ginkgo.It("should redirect from www HTTPS to HTTPS", func() {
-		f.SetNginxConfigMapData(map[string]string{
-			"allow-snippet-annotations": "true",
-		})
-		defer func() {
-			f.SetNginxConfigMapData(map[string]string{
-				"allow-snippet-annotations": "false",
-			})
-		}()
+		disableSnippet := f.AllowSnippetConfiguration()
+		defer disableSnippet()
 
 		ginkgo.By("setting up server for redirect from www")
 

test/e2e/annotations/grpc.go

@@ -193,14 +193,8 @@ var _ = framework.DescribeAnnotation("backend-protocol - GRPC", func() {
 	ginkgo.It("should return OK for service with backend protocol GRPCS", func() {
 		f.NewGRPCBinDeployment()
 
-		f.SetNginxConfigMapData(map[string]string{
-			"allow-snippet-annotations": "true",
-		})
-		defer func() {
-			f.SetNginxConfigMapData(map[string]string{
-				"allow-snippet-annotations": "false",
-			})
-		}()
+		disableSnippet := f.AllowSnippetConfiguration()
+		defer disableSnippet()
 
 		host := echoHost
 

test/e2e/annotations/modsecurity/modsecurity.go

@@ -100,14 +100,8 @@ var _ = framework.DescribeAnnotation("modsecurity owasp", func() {
 	})
 
 	ginkgo.It("should enable modsecurity with snippet", func() {
-		f.SetNginxConfigMapData(map[string]string{
-			"allow-snippet-annotations": "true",
-		})
-		defer func() {
-			f.SetNginxConfigMapData(map[string]string{
-				"allow-snippet-annotations": "false",
-			})
-		}()
+		disableSnippet := f.AllowSnippetConfiguration()
+		defer disableSnippet()
 
 		host := modSecurityFooHost
 		nameSpace := f.Namespace
@@ -173,14 +167,8 @@ var _ = framework.DescribeAnnotation("modsecurity owasp", func() {
 	})
 
 	ginkgo.It("should enable modsecurity with snippet and block requests", func() {
-		f.SetNginxConfigMapData(map[string]string{
-			"allow-snippet-annotations": "true",
-		})
-		defer func() {
-			f.SetNginxConfigMapData(map[string]string{
-				"allow-snippet-annotations": "false",
-			})
-		}()
+		disableSnippet := f.AllowSnippetConfiguration()
+		defer disableSnippet()
 
 		host := modSecurityFooHost
 		nameSpace := f.Namespace
@@ -212,14 +200,8 @@ var _ = framework.DescribeAnnotation("modsecurity owasp", func() {
 	})
 
 	ginkgo.It("should enable modsecurity globally and with modsecurity-snippet block requests", func() {
-		f.SetNginxConfigMapData(map[string]string{
-			"allow-snippet-annotations": "true",
-		})
-		defer func() {
-			f.SetNginxConfigMapData(map[string]string{
-				"allow-snippet-annotations": "false",
-			})
-		}()
+		disableSnippet := f.AllowSnippetConfiguration()
+		defer disableSnippet()
 
 		host := modSecurityFooHost
 		nameSpace := f.Namespace
@@ -251,16 +233,11 @@ var _ = framework.DescribeAnnotation("modsecurity owasp", func() {
 	})
 
 	ginkgo.It("should enable modsecurity when enable-owasp-modsecurity-crs is set to true", func() {
-		f.SetNginxConfigMapData(map[string]string{
-			"allow-snippet-annotations":    "true",
-			"enable-modsecurity":           "true",
-			"enable-owasp-modsecurity-crs": "true",
-		})
-		defer func() {
-			f.SetNginxConfigMapData(map[string]string{
-				"allow-snippet-annotations": "false",
-			})
-		}()
+		disableSnippet := f.AllowSnippetConfiguration()
+		defer disableSnippet()
+
+		f.UpdateNginxConfigMapData("enable-modsecurity", "true")
+		f.UpdateNginxConfigMapData("enable-owasp-modsecurity-crs", "true")
 
 		host := modSecurityFooHost
 		nameSpace := f.Namespace
@@ -290,6 +267,8 @@ var _ = framework.DescribeAnnotation("modsecurity owasp", func() {
 	})
 
 	ginkgo.It("should enable modsecurity through the config map", func() {
+		disableSnippet := f.AllowSnippetConfiguration()
+		defer disableSnippet()
 		host := modSecurityFooHost
 		nameSpace := f.Namespace
 
@@ -310,17 +289,9 @@ var _ = framework.DescribeAnnotation("modsecurity owasp", func() {
 		f.EnsureIngress(ing)
 
 		expectedComment := "SecRuleEngine On"
-		f.SetNginxConfigMapData(map[string]string{
-			"allow-snippet-annotations":    "true",
-			"enable-modsecurity":           "true",
-			"enable-owasp-modsecurity-crs": "true",
-			"modsecurity-snippet":          expectedComment,
-		})
-		defer func() {
-			f.SetNginxConfigMapData(map[string]string{
-				"allow-snippet-annotations": "false",
-			})
-		}()
+		f.UpdateNginxConfigMapData("enable-modsecurity", "true")
+		f.UpdateNginxConfigMapData("enable-owasp-modsecurity-crs", "true")
+		f.UpdateNginxConfigMapData("modsecurity-snippet", expectedComment)
 
 		f.WaitForNginxServer(host,
 			func(server string) bool {
@@ -339,6 +310,9 @@ var _ = framework.DescribeAnnotation("modsecurity owasp", func() {
 		host := modSecurityFooHost
 		nameSpace := f.Namespace
 
+		f.UpdateNginxConfigMapData("annotations-risk-level", "Critical") // To enable snippet configurations
+		defer f.UpdateNginxConfigMapData("annotations-risk-level", "High")
+
 		snippet := `SecRequestBodyAccess On
 		SecAuditEngine RelevantOnly
 		SecAuditLogParts ABIJDEFHZ
@@ -378,14 +352,9 @@ var _ = framework.DescribeAnnotation("modsecurity owasp", func() {
 	})
 
 	ginkgo.It("should disable default modsecurity conf setting when modsecurity-snippet is specified", func() {
-		f.SetNginxConfigMapData(map[string]string{
-			"allow-snippet-annotations": "true",
-		})
-		defer func() {
-			f.SetNginxConfigMapData(map[string]string{
-				"allow-snippet-annotations": "false",
-			})
-		}()
+		disableSnippet := f.AllowSnippetConfiguration()
+		defer disableSnippet()
+
 		host := modSecurityFooHost
 		nameSpace := f.Namespace