Add patch to remove root and alias directives

[rikatz]

Adds a patch to remove root and alias directives.

This directives are not required by Ingress and has been used to explore some reading of sensitive files

[k8s-ci-robot]

@rikatz: This issue is currently awaiting triage.

If Ingress contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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/test-infra repository.

[tao12345666333]

/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rikatz, tao12345666333

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:

• images/OWNERS [rikatz,tao12345666333]

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

[nhinze23]

Hey @rikatz,
we extend the ingress nginx image with custom pages (e.g. error page in /var/www/html) and used this root directive to point to those files.
Now this feature is a breaking change for us and we don't know a alternative way for doing such things. Can you give us a hint?
Is this maybe the complete wrong way we are doing?

With best regards

[rikatz]

Can you point me (in Slack) how you've been doing it? Iirc custom errors has its own directives @longwuyuan may know

[longwuyuan]

@nhinze23 , thanks for your update. As @rikatz said, I too feel it will help a lot to see precise details of your extensions. As suggested, you could copy/paste start a thread at https://kubernetes.slack.com in the ingress-nginx-dev channel. Thank you

[MatthewKennedy]

Trying to follow the Nginx guide for service static assets and getting this error when trying to set the root:

`-------------------------------------------------------------------------------
Error: exit status 1
2024/04/30 19:06:40 [emerg] 4403#4403: unknown directive "root" in /tmp/nginx/nginx-cfg4123983271:708
nginx: [emerg] unknown directive "root" in /tmp/nginx/nginx-cfg4123983271:708
nginx: configuration file /tmp/nginx/nginx-cfg4123983271 test failed

-------------------------------------------------------------------------------`

• https://docs.nginx.com/nginx/admin-guide/web-server/serving-static-content/
• https://www.nginx.com/resources/wiki/start/topics/tutorials/config_pitfalls/#root-inside-location-block

Can anyone point me in the right direction to add that root as suggested by Nginx?

Many thanks.

[rikatz]

We don't support root directive on ingress-nginx :) you are not supposed to serve static files from there.

Instead you should create a backend containing it, and do ingress point to that backend

[clywm520]

How to instead resolve root or alias

[rikatz]

you don't. These directives assume you want ingress nginx to provide files that are stored locally, which is not the case. ingress-nginx is a proxy and not a webserver on your Kubernetes cluster :)

[lots0logs]

you don't. These directives assume you want ingress nginx to provide files that are stored locally, which is not the case. ingress-nginx is a proxy and not a webserver on your Kubernetes cluster :)

We actually do serve some files locally from the controller on our cluster. Our cluster consists of just three worker nodes. It has worked well for our use case. Now, we can't update the controller anymore due to this breaking change.

[longwuyuan]

We actually do serve some files locally from the controller on our cluster. Our cluster consists of just three worker nodes. It has worked well for our use case. Now, we can't update the controller anymore due to this breaking change.

@lots0logs unfortunate that you landed in this situation. Ack that it has worked well for you. Hope you can get the full spectrum view of what really is going on. What you expect is helpful but a security flaw. The project is required to ship a controller that is secure out of the box. The choice that the project needs to make is between a relatively secure & stable controller out-of-the-box or a useful-but-insecure-unstable controller.

Hope this clarifies. Regards,

[lots0logs]

Whether or not it's a security flaw depends on the use case really. The directive didn't have to be completely removed in order to address the security concern. It could have just been disallowed in configuration snippets. That way anyone using a custom nginx config template for the controller could still make use of the directive if they needed.

[rikatz]

Let me try to make it really clear:

Ingress-NGINX is not supposed to be used as a webserver. It is intended to be used as a proxy server. While disabling a directive seems extreme, parsing every configuration snippet to check if a directive is being used or not, and if it is being correctly used or not is expensive, from a perspective of development, time testing, etc.

We cannot afford spending time on every use case where the controller should not be used as it is because it was possible before.

I understand the pain of deprecating such kind of thing, but have in mind that while we are suffering for constant CVEs because of different misusage of ingress controller, we need to focus on the main goal of the project, that is to provide a simple to use (and yet feature complete) Ingress/Proxy for your workloads.

People are more than welcome to fork and build their own ingress-nginx not removing these directives, but this is not going to make us go back and re-add a directive that has no role on proxying connections.

[Falven]

Whether or not it's a security flaw depends on the use case really. The directive didn't have to be completely removed in order to address the security concern. It could have just been disallowed in configuration snippets. That way anyone using a custom nginx config template for the controller could still make use of the directive if they needed.