Skip to content

Commit 771614f

Browse files
rikatzrikatz
committed
Enable security features by default
1 parent eee2760 commit 771614f

28 files changed

+103
-262
lines changed

‎.github/workflows/ci.yaml

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ jobs:
235235
236236
strategy:
237237
matrix:
238-
k8s: [v1.26.15, v1.27.13, v1.28.9, v1.29.4, v1.30.0]
238+
k8s: [v1.28.13, v1.29.8, v1.30.4, v1.31.0]
239239

240240
steps:
241241
- name: Checkout
@@ -286,26 +286,11 @@ jobs:
286286
(needs.changes.outputs.go == 'true') || (needs.changes.outputs.baseimage == 'true') || ${{ github.event.workflow_dispatch.run_e2e == 'true' }}
287287
strategy:
288288
matrix:
289-
k8s: [v1.26.15, v1.27.13, v1.28.9, v1.29.4, v1.30.0]
289+
k8s: [v1.28.13, v1.29.8, v1.30.4, v1.31.0]
290290
uses: ./.github/workflows/zz-tmpl-k8s-e2e.yaml
291291
with:
292292
k8s-version: ${{ matrix.k8s }}
293293

294-
kubernetes-validations:
295-
name: Kubernetes with Validations
296-
needs:
297-
- changes
298-
- build
299-
if: |
300-
(needs.changes.outputs.go == 'true') || (needs.changes.outputs.baseimage == 'true') || ${{ github.event.workflow_dispatch.run_e2e == 'true' }}
301-
strategy:
302-
matrix:
303-
k8s: [v1.26.15, v1.27.13, v1.28.9, v1.29.4, v1.30.0]
304-
uses: ./.github/workflows/zz-tmpl-k8s-e2e.yaml
305-
with:
306-
k8s-version: ${{ matrix.k8s }}
307-
variation: "VALIDATIONS"
308-
309294
kubernetes-chroot:
310295
name: Kubernetes chroot
311296
needs:
@@ -315,7 +300,7 @@ jobs:
315300
(needs.changes.outputs.go == 'true') || (needs.changes.outputs.baseimage == 'true') || ${{ github.event.workflow_dispatch.run_e2e == 'true' }}
316301
strategy:
317302
matrix:
318-
k8s: [v1.26.15, v1.27.13, v1.28.9, v1.29.4, v1.30.0]
303+
k8s: [v1.28.13, v1.29.8, v1.30.4, v1.31.0]
319304
uses: ./.github/workflows/zz-tmpl-k8s-e2e.yaml
320305
with:
321306
k8s-version: ${{ matrix.k8s }}

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ jobs:
4343
SKIP_CLUSTER_CREATION: true
4444
SKIP_INGRESS_IMAGE_CREATION: true
4545
SKIP_E2E_IMAGE_CREATION: true
46-
ENABLE_VALIDATIONS: ${{ inputs.variation == 'VALIDATIONS' }}
4746
IS_CHROOT: ${{ inputs.variation == 'CHROOT' }}
4847
run: |
4948
kind get kubeconfig > $HOME/.kube/kind-config-kind

‎charts/ingress-nginx/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,7 @@ As of version `1.26.0` of this chart, by simply not providing any clusterIP valu
304304
| 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. |
305305
| controller.electionID | string | `""` | Election ID to use for status update, by default it uses the controller name combined with a suffix of 'leader' |
306306
| controller.electionTTL | string | `""` | Duration a leader election is valid before it's getting re-elected, e.g. `15s`, `10m` or `1h`. (Default: 30s) |
307-
| controller.enableAnnotationValidations | bool | `false` | |
307+
| controller.enableAnnotationValidations | bool | `true` | |
308308
| controller.enableMimalloc | bool | `true` | Enable mimalloc as a drop-in replacement for malloc. # ref: https://github.com/microsoft/mimalloc # |
309309
| 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 |
310310
| controller.existingPsp | string | `""` | Use an existing PSP instead of creating one |

‎charts/ingress-nginx/values.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ commonLabels: {}
1717

1818
controller:
1919
name: controller
20-
enableAnnotationValidations: false
20+
enableAnnotationValidations: true
2121
image:
2222
## Keep false as default for now!
2323
chroot: false

‎docs/user-guide/cli-arguments.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ They are set in the container spec of the `ingress-nginx-controller` Deployment
1515
| `--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. |
1616
| `--default-server-port` | Port to use for exposing the default server (catch-all). (default 8181) |
1717
| `--default-ssl-certificate` | Secret containing a SSL certificate to be used by the default HTTPS server (catch-all). Takes the form "namespace/name". |
18-
| `--enable-annotation-validation` | If true, will enable the annotation validation feature. This value will be defaulted to true on a future release. |
18+
| `--enable-annotation-validation` | If true, will enable the annotation validation feature. Defaults to true |
1919
| `--disable-catch-all` | Disable support for catch-all Ingresses. (default false) |
2020
| `--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). |
2121
| `--disable-svc-external-name` | Disable support for Services of type ExternalName. (default false) |

‎internal/ingress/controller/config/config.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -781,10 +781,10 @@ func NewDefault() Configuration {
781781

782782
cfg := Configuration{
783783
AllowSnippetAnnotations: false,
784-
AllowCrossNamespaceResources: true,
784+
AllowCrossNamespaceResources: false,
785785
AllowBackendServerHeader: false,
786786
AnnotationValueWordBlocklist: "",
787-
AnnotationsRiskLevel: "Critical",
787+
AnnotationsRiskLevel: "High",
788788
AccessLogPath: "/var/log/nginx/access.log",
789789
AccessLogParams: "",
790790
EnableAccessLogForDefaultBackend: false,
@@ -929,7 +929,7 @@ func NewDefault() Configuration {
929929
GlobalRateLimitMemcachedPoolSize: 50,
930930
GlobalRateLimitStatusCode: 429,
931931
DebugConnections: []string{},
932-
StrictValidatePathType: false, // TODO: This will be true in future releases
932+
StrictValidatePathType: true,
933933
GRPCBufferSizeKb: 0,
934934
}
935935

‎pkg/flags/flags.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ Requires the update-status parameter.`)
158158
annotationsPrefix = flags.String("annotations-prefix", parser.DefaultAnnotationsPrefix,
159159
`Prefix of the Ingress annotations specific to the NGINX controller.`)
160160

161-
enableAnnotationValidation = flags.Bool("enable-annotation-validation", false,
161+
enableAnnotationValidation = flags.Bool("enable-annotation-validation", true,
162162
`If true, will enable the annotation validation feature. This value will be defaulted to true on a future release`)
163163

164164
enableSSLChainCompletion = flags.Bool("enable-ssl-chain-completion", false,

‎test/e2e/admission/admission.go

Lines changed: 6 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -127,14 +127,8 @@ var _ = framework.IngressNginxDescribeSerial("[Admission] admission controller",
127127
})
128128

129129
ginkgo.It("should return an error if there is an error validating the ingress definition", func() {
130-
f.SetNginxConfigMapData(map[string]string{
131-
"allow-snippet-annotations": "true",
132-
})
133-
defer func() {
134-
f.SetNginxConfigMapData(map[string]string{
135-
"allow-snippet-annotations": "false",
136-
})
137-
}()
130+
disableSnippet := f.AllowSnippetConfiguration()
131+
defer disableSnippet()
138132

139133
host := admissionTestHost
140134

@@ -241,14 +235,8 @@ var _ = framework.IngressNginxDescribeSerial("[Admission] admission controller",
241235
})
242236

243237
ginkgo.It("should return an error if the Ingress V1 definition contains invalid annotations", func() {
244-
f.SetNginxConfigMapData(map[string]string{
245-
"allow-snippet-annotations": "true",
246-
})
247-
defer func() {
248-
f.SetNginxConfigMapData(map[string]string{
249-
"allow-snippet-annotations": "false",
250-
})
251-
}()
238+
disableSnippet := f.AllowSnippetConfiguration()
239+
defer disableSnippet()
252240

253241
out, err := createIngress(f.Namespace, invalidV1Ingress)
254242
assert.Empty(ginkgo.GinkgoT(), out)
@@ -261,14 +249,8 @@ var _ = framework.IngressNginxDescribeSerial("[Admission] admission controller",
261249
})
262250

263251
ginkgo.It("should not return an error for an invalid Ingress when it has unknown class", func() {
264-
f.SetNginxConfigMapData(map[string]string{
265-
"allow-snippet-annotations": "true",
266-
})
267-
defer func() {
268-
f.SetNginxConfigMapData(map[string]string{
269-
"allow-snippet-annotations": "false",
270-
})
271-
}()
252+
disableSnippet := f.AllowSnippetConfiguration()
253+
defer disableSnippet()
272254
out, err := createIngress(f.Namespace, invalidV1IngressWithOtherClass)
273255
assert.Equal(ginkgo.GinkgoT(), "ingress.networking.k8s.io/extensions-invalid-other created\n", out)
274256
assert.Nil(ginkgo.GinkgoT(), err, "creating an invalid ingress with unknown class using kubectl")

‎test/e2e/annotations/auth.go

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -277,14 +277,8 @@ var _ = framework.DescribeAnnotation("auth-*", func() {
277277
"nginx.ingress.kubernetes.io/auth-snippet": `
278278
proxy_set_header My-Custom-Header 42;`,
279279
}
280-
f.SetNginxConfigMapData(map[string]string{
281-
"allow-snippet-annotations": "true",
282-
})
283-
defer func() {
284-
f.SetNginxConfigMapData(map[string]string{
285-
"allow-snippet-annotations": "false",
286-
})
287-
}()
280+
disableSnippet := f.AllowSnippetConfiguration()
281+
defer disableSnippet()
288282

289283
ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, annotations)
290284
f.EnsureIngress(ing)
@@ -297,15 +291,8 @@ var _ = framework.DescribeAnnotation("auth-*", func() {
297291

298292
ginkgo.It(`should not set snippet "proxy_set_header My-Custom-Header 42;" when external auth is not configured`, func() {
299293
host := authHost
300-
301-
f.SetNginxConfigMapData(map[string]string{
302-
"allow-snippet-annotations": "true",
303-
})
304-
defer func() {
305-
f.SetNginxConfigMapData(map[string]string{
306-
"allow-snippet-annotations": "false",
307-
})
308-
}()
294+
disableSnippet := f.AllowSnippetConfiguration()
295+
defer disableSnippet()
309296

310297
annotations := map[string]string{
311298
"nginx.ingress.kubernetes.io/auth-snippet": `

‎test/e2e/annotations/fromtowwwredirect.go

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -62,14 +62,8 @@ var _ = framework.DescribeAnnotation("from-to-www-redirect", func() {
6262
})
6363

6464
ginkgo.It("should redirect from www HTTPS to HTTPS", func() {
65-
f.SetNginxConfigMapData(map[string]string{
66-
"allow-snippet-annotations": "true",
67-
})
68-
defer func() {
69-
f.SetNginxConfigMapData(map[string]string{
70-
"allow-snippet-annotations": "false",
71-
})
72-
}()
65+
disableSnippet := f.AllowSnippetConfiguration()
66+
defer disableSnippet()
7367

7468
ginkgo.By("setting up server for redirect from www")
7569

‎test/e2e/annotations/grpc.go

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -193,14 +193,8 @@ var _ = framework.DescribeAnnotation("backend-protocol - GRPC", func() {
193193
ginkgo.It("should return OK for service with backend protocol GRPCS", func() {
194194
f.NewGRPCBinDeployment()
195195

196-
f.SetNginxConfigMapData(map[string]string{
197-
"allow-snippet-annotations": "true",
198-
})
199-
defer func() {
200-
f.SetNginxConfigMapData(map[string]string{
201-
"allow-snippet-annotations": "false",
202-
})
203-
}()
196+
disableSnippet := f.AllowSnippetConfiguration()
197+
defer disableSnippet()
204198

205199
host := echoHost
206200

‎test/e2e/annotations/modsecurity/modsecurity.go

Lines changed: 22 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -100,14 +100,8 @@ var _ = framework.DescribeAnnotation("modsecurity owasp", func() {
100100
})
101101

102102
ginkgo.It("should enable modsecurity with snippet", func() {
103-
f.SetNginxConfigMapData(map[string]string{
104-
"allow-snippet-annotations": "true",
105-
})
106-
defer func() {
107-
f.SetNginxConfigMapData(map[string]string{
108-
"allow-snippet-annotations": "false",
109-
})
110-
}()
103+
disableSnippet := f.AllowSnippetConfiguration()
104+
defer disableSnippet()
111105

112106
host := modSecurityFooHost
113107
nameSpace := f.Namespace
@@ -173,14 +167,8 @@ var _ = framework.DescribeAnnotation("modsecurity owasp", func() {
173167
})
174168

175169
ginkgo.It("should enable modsecurity with snippet and block requests", func() {
176-
f.SetNginxConfigMapData(map[string]string{
177-
"allow-snippet-annotations": "true",
178-
})
179-
defer func() {
180-
f.SetNginxConfigMapData(map[string]string{
181-
"allow-snippet-annotations": "false",
182-
})
183-
}()
170+
disableSnippet := f.AllowSnippetConfiguration()
171+
defer disableSnippet()
184172

185173
host := modSecurityFooHost
186174
nameSpace := f.Namespace
@@ -212,14 +200,8 @@ var _ = framework.DescribeAnnotation("modsecurity owasp", func() {
212200
})
213201

214202
ginkgo.It("should enable modsecurity globally and with modsecurity-snippet block requests", func() {
215-
f.SetNginxConfigMapData(map[string]string{
216-
"allow-snippet-annotations": "true",
217-
})
218-
defer func() {
219-
f.SetNginxConfigMapData(map[string]string{
220-
"allow-snippet-annotations": "false",
221-
})
222-
}()
203+
disableSnippet := f.AllowSnippetConfiguration()
204+
defer disableSnippet()
223205

224206
host := modSecurityFooHost
225207
nameSpace := f.Namespace
@@ -251,16 +233,11 @@ var _ = framework.DescribeAnnotation("modsecurity owasp", func() {
251233
})
252234

253235
ginkgo.It("should enable modsecurity when enable-owasp-modsecurity-crs is set to true", func() {
254-
f.SetNginxConfigMapData(map[string]string{
255-
"allow-snippet-annotations": "true",
256-
"enable-modsecurity": "true",
257-
"enable-owasp-modsecurity-crs": "true",
258-
})
259-
defer func() {
260-
f.SetNginxConfigMapData(map[string]string{
261-
"allow-snippet-annotations": "false",
262-
})
263-
}()
236+
disableSnippet := f.AllowSnippetConfiguration()
237+
defer disableSnippet()
238+
239+
f.UpdateNginxConfigMapData("enable-modsecurity", "true")
240+
f.UpdateNginxConfigMapData("enable-owasp-modsecurity-crs", "true")
264241

265242
host := modSecurityFooHost
266243
nameSpace := f.Namespace
@@ -290,6 +267,8 @@ var _ = framework.DescribeAnnotation("modsecurity owasp", func() {
290267
})
291268

292269
ginkgo.It("should enable modsecurity through the config map", func() {
270+
disableSnippet := f.AllowSnippetConfiguration()
271+
defer disableSnippet()
293272
host := modSecurityFooHost
294273
nameSpace := f.Namespace
295274

@@ -310,17 +289,9 @@ var _ = framework.DescribeAnnotation("modsecurity owasp", func() {
310289
f.EnsureIngress(ing)
311290

312291
expectedComment := "SecRuleEngine On"
313-
f.SetNginxConfigMapData(map[string]string{
314-
"allow-snippet-annotations": "true",
315-
"enable-modsecurity": "true",
316-
"enable-owasp-modsecurity-crs": "true",
317-
"modsecurity-snippet": expectedComment,
318-
})
319-
defer func() {
320-
f.SetNginxConfigMapData(map[string]string{
321-
"allow-snippet-annotations": "false",
322-
})
323-
}()
292+
f.UpdateNginxConfigMapData("enable-modsecurity", "true")
293+
f.UpdateNginxConfigMapData("enable-owasp-modsecurity-crs", "true")
294+
f.UpdateNginxConfigMapData("modsecurity-snippet", expectedComment)
324295

325296
f.WaitForNginxServer(host,
326297
func(server string) bool {
@@ -339,6 +310,9 @@ var _ = framework.DescribeAnnotation("modsecurity owasp", func() {
339310
host := modSecurityFooHost
340311
nameSpace := f.Namespace
341312

313+
f.UpdateNginxConfigMapData("annotations-risk-level", "Critical") // To enable snippet configurations
314+
defer f.UpdateNginxConfigMapData("annotations-risk-level", "High")
315+
342316
snippet := `SecRequestBodyAccess On
343317
SecAuditEngine RelevantOnly
344318
SecAuditLogParts ABIJDEFHZ
@@ -378,14 +352,9 @@ var _ = framework.DescribeAnnotation("modsecurity owasp", func() {
378352
})
379353

380354
ginkgo.It("should disable default modsecurity conf setting when modsecurity-snippet is specified", func() {
381-
f.SetNginxConfigMapData(map[string]string{
382-
"allow-snippet-annotations": "true",
383-
})
384-
defer func() {
385-
f.SetNginxConfigMapData(map[string]string{
386-
"allow-snippet-annotations": "false",
387-
})
388-
}()
355+
disableSnippet := f.AllowSnippetConfiguration()
356+
defer disableSnippet()
357+
389358
host := modSecurityFooHost
390359
nameSpace := f.Namespace
391360

0 commit comments

Comments
 (0)