NGINX: Upgrade ModSecurity to v4.4.0.

[jessebot]

What this PR does / why we need it:

Upgrades ModSecurity's OWASP Core Rule Set from 3.3.5 to 4.4.0, which addresses #11510

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• CVE Report (Scanner found CVE and adding report)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation only

Which issue/s this PR fixes

fixes #11510

How Has This Been Tested?

I'm not sure how best to test this, but I'm open to feedback

Checklist:

• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I've read the CONTRIBUTION guide
• I have added unit and/or e2e tests to cover my changes.
• All new and existing tests passed.

[k8s-ci-robot]

Hi @jessebot. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[netlify]

✅ Deploy Preview for kubernetes-ingress-nginx canceled.

Name	Link
🔨 Latest commit	d67a5b8
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-ingress-nginx/deploys/667fa1fec2f9740008a603b9

[longwuyuan]

@jessebot thank you very much for this.

Can you change the ref for the other image called nginx-1.25, as well. please.

[jessebot]

@longwuyuan sure, no problem. Let me know if there's any tests for mod security that you recommend I run to ensure this is safe to merge. Happy to do it!

[longwuyuan]

Best way to run tests is local tests instead of CI tests as documented here https://kubernetes.github.io/ingress-nginx/developer-guide/getting-started/#local-build (use the FOCUS env-var).

But in this particular case (and all cases that make change in the $HOME/images/nginx* directory), there is a unique complexity.

The change you made is on what we call the BASE_IMAGE . The relevance is that the build of the controller, requires a pre-existing BASE_IMAGE. So first this PR needs to get merged and that will trigger a new build of the baseimage in the infra. That image takes birth in the staging registry. We need to promote that baseimage from staging to prod which is a second PR in the github.com/kubernetes/k8s.io repo. After there a 3rd PR is needed to change this project's code to use the tag + sha of the new baseimage.

And only then modsecurity realted (or any other tests depending on baseimage change) can be run.

Please double check if any other change is needed in the /images/nginx* directories, to be sure. You can even attempt a local build of the baseimage on your laptop etc. if needed,

Then we can make an attempt to ask for lgtm/approve here asap.

cc @tao12345666333

[longwuyuan]

cc @tao12345666333 #11510 (comment)

[jessebot]

@longwuyuan thanks so much for your explanation! I will try that test locally. I see there was also a failure in the ci here:
https://github.com/kubernetes/ingress-nginx/actions/runs/9694235651/job/26751971460?pr=11511#step:6:31

But I'm not sure why... still testing locally...

Also, when I ran make dev-env it tried to make these changes:

diff --git a/go.work.sum b/go.work.sum
index e047983dd..47e10bce6 100644
--- a/go.work.sum
+++ b/go.work.sum
@@ -676,6 +676,7 @@ github.com/go-playground/universal-translator v0.18.1/go.mod h1:xekY+UJKNuX9WP91
 github.com/go-playground/validator/v10 v10.14.0 h1:vgvQWe3XCz3gIeFDm/HnTIbj6UGmg/+t63MyGU2n5js=
 github.com/go-playground/validator/v10 v10.14.0/go.mod h1:9iXMNT7sEkjXb0I+enO7QXmzG6QCsPWY4zveKFVRSyU=
 github.com/go-stack/stack v1.8.0 h1:5SgMzNM5HxrEjV0ww2lTmX6E2Izsfxas4+YHWRs3Lsk=
+github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572/go.mod h1:9Pwr4B2jHnOSGXyyzV8ROjYa2ojvAY6HCGYYfMoC3Ls=
 github.com/gobwas/httphead v0.1.0 h1:exrUm0f4YX0L7EBwZHuCF4GDp8aJfVeBrlLQrs6NqWU=
 github.com/gobwas/httphead v0.1.0/go.mod h1:O/RXo79gxV8G+RqlR/otEwx4Q36zl9rqC5u12GKvMCM=
 github.com/gobwas/pool v0.2.1 h1:xfeeEhW7pwmX8nuLVlqbzVc7udMDrwetjEv+TZIz1og=
@@ -897,6 +898,7 @@ golang.org/x/arch v0.3.0 h1:02VY4/ZcO/gBOH6PUaoiptASxtXU10jazRCP865E97k=
 golang.org/x/arch v0.3.0/go.mod h1:5om86z9Hs0C8fWVUuoMHwpExlXzs5Tkyp9hOrfG7pp8=
 golang.org/x/crypto v0.0.0-20220722155217-630584e8d5aa/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
 golang.org/x/crypto v0.12.0/go.mod h1:NF0Gs7EO5K4qLn+Ylc+fih8BSTeIjAP05siRnAh98yw=
+golang.org/x/crypto v0.23.0/go.mod h1:CKFgDieR+mRhux2Lsu27y0fO304Db0wZe70UKqHu0v8=
 golang.org/x/image v0.0.0-20220302094943-723b81ca9867 h1:TcHcE0vrmgzNH1v3ppjcMGbhG5+9fMuvOmUYwNEF4q4=
 golang.org/x/lint v0.0.0-20210508222113-6edffad5e616 h1:VLliZ0d+/avPrXXH+OakdXhpJuEoBZuwh1m2j7U6Iug=
 golang.org/x/mobile v0.0.0-20190719004257-d2bd2a29d028 h1:4+4C/Iv2U4fMZBiMCc98MG1In4gJY5YRhtpDNeDeHWs=
@@ -905,6 +907,7 @@ golang.org/x/mod v0.14.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c=
 golang.org/x/net v0.14.0/go.mod h1:PpSgVXXLK0OxS0F31C1/tv6XNguvCrnXIDrFMspZIUI=
 golang.org/x/net v0.16.0/go.mod h1:NxSsAGuq816PNPmqtQdLE42eU2Fs7NoRIZrHJAlaCOE=
 golang.org/x/net v0.18.0/go.mod h1:/czyP5RqHAH4odGYxBJ1qz0+CE5WZ+2j1YgoEo8F2jQ=
+golang.org/x/net v0.23.0/go.mod h1:JKghWKKOSdJwpW2GEx0Ja7fmaKnMsbu+MWVZTokSYmg=
 golang.org/x/net v0.24.0/go.mod h1:2Q7sJY5mzlzWjKtYUEXSlBWCdyaioyXzRB2RtU8KVE8=
 golang.org/x/oauth2 v0.10.0/go.mod h1:kTpgurOux7LqtuxjuyZa4Gj2gdezIt/jQtGnNFfypQI=
 golang.org/x/oauth2 v0.11.0/go.mod h1:LdF7O/8bLR/qWK9DrpXmbHLTouvRHK0SgJl0GmDBchk=
@@ -924,6 +927,7 @@ golang.org/x/tools v0.4.0/go.mod h1:UE5sM2OK9E/d67R0ANs2xJizIymRP5gJU295PvKXxjQ=
 golang.org/x/tools v0.12.0/go.mod h1:Sc0INKfu04TlqNoRA1hgpFZbhYXHPr4V5DzpSBTPqQM=
 golang.org/x/tools v0.13.0/go.mod h1:HvlwmtVNQAhOuCjW7xxvovg8wbNq7LwfXh/k7wXUl58=
 golang.org/x/tools v0.16.1/go.mod h1:kYVVN6I1mBNoB1OX+noeBjbRk4IUEPa7JJ+TJMEooJ0=
+golang.org/x/tools v0.18.0/go.mod h1:GL7B4CwcLLeo59yx/9UWWuNOW1n3VZ4f5axWfML7Lcg=
 golang.org/x/tools v0.20.0/go.mod h1:WvitBU7JJf6A4jOdg4S1tviW9bhUxkgeCui/0JHctQg=
 golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 h1:H2TDz8ibqkAF6YGhCdN3jS9O0/s90v0rJh3X/OLHEUk=
 golang.org/x/xerrors v0.0.0-20231012003039-104605ab7028/go.mod h1:NDW/Ps6MPRej6fsCIbMTohpP40sJ/P/vI1MoTEGwX90=

Should I check in those changes?

[longwuyuan]

ok, catch me on slack if you think it will help.

But, in short, ignore running make dev-env in this specific PR because that still requires a pre-existing BASE_IMAGE. Look at the scripts under /build.

Also ignore the CI failures as there will be flakes and very likely something else is already a mess in CI.

Currently, your only scope is generating an equivalent of the BASEIMAGE locally. If you use mac its a no-no as you need to test X86_64

[jessebot]

I used FOCUS="modsecurity owasp" make kind-e2e-test locally, based on this e2e tests doc that was mentioned in the testing doc, and it seems like the tests all passed:

...
Starting the e2e test pod
If you don't see a command prompt, try pressing enter.
SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS
•SSSSSSSSSSSSSSS•SSSSSSSSSSSSSSSSSSSS•S•SSSSSSSSSSSS•SSSSSSSSSS•SSSSSSSSSSSSSSSSSS••
••••SSSSSSSSSSSSSSSS SUCCESS! 1m28.581898833s

Ginkgo ran 1 suite in 1m28.646878957s
Test Suite Passed
configmap/report-e2e-test-suite.xml.gz created
configmap/report-e2e-test-suite.xml.gz labeled
pod "e2e" deleted
Getting the report file out now..
done getting the report file out..
Deleting cluster "ingress-nginx-dev" ...
Deleted nodes: ["ingress-nginx-dev-control-plane" "ingress-nginx-dev-worker2" "ingress-nginx-dev-worker"]

I will look more at the ci test to see what happened 🤔

[jessebot]

But, in short, ignore running make dev-env in this specific PR because that still requires a pre-existing BASE_IMAGE. Look at the scripts under /build.

okie dokie

Also ignore the CI failures as there will be flakes and very likely something else is already a mess in CI.

Can do!

Currently, your only scope is generating an equivalent of the BASEIMAGE locally. If you use mac its a no-no as you need to test X86_64

Oh! I will test on x86_64 🤦 should have read that sooner.

[longwuyuan]

current local tests will NOT use a BASEIMAGE that has core-ruleset v4.x in it.

The image with core-ruleset is only available AFTER, this PR merges and 2 more PRs merge as per this previous note from me #11511 (comment)

[longwuyuan]

gentle request, please squash commits.

there is a github workflow bot that does the squash AFAIK, but why even go there needlessly. thanks again for this contribution.

[tao12345666333]

Sure, the /label tide/merge-method-squash can squash automatically

[jessebot]

Sure, I've removed the nginx base image change in favor of only doing the nginx-1.25 image, and I've made this all one commit. :) Let me know if you need anything else!

[longwuyuan]

/riage accepted

@tao12345666333 @rikatz @strongjz @Gacko @cpanato for review/lgtm/approve

[Gacko]

/triage accepted
/kind feature
/priority backlog
/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Gacko, jessebot, longwuyuan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [Gacko]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[Gacko]

/retitle NGINX: Upgrade ModSecurity to v4.4.0.

[Gacko]

@strongjz @tao12345666333

If I got our little conversation / rubberducking right yesterday, that change being merged to a branch (e.g. main) will end up in the NGINX base image as soon as we bump the TAG on that branch (e.g. main). Right now we are bumping and building the NGINX base image from main and promote that TAG to the registry.

So as soon as that TAG makes its way to the NGINX_BASE of a release-x.y branch, that release branch also uses the code introduced in this PR. Which means: We can also just cherry-pick it to the release branch even if it's a minor change, right?

[Gacko]

/cherry-pick release-1.10

[k8s-infra-cherrypick-robot]

@Gacko: new pull request created: #11548

In response to this:

/cherry-pick release-1.10

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.