Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CORS configuration from annotation fails penetration test #12230

Open
stex79 opened this issue Oct 25, 2024 · 6 comments
Open

CORS configuration from annotation fails penetration test #12230

stex79 opened this issue Oct 25, 2024 · 6 comments
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/support Categorizes issue or PR as a support question. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.

Comments

@stex79
Copy link

stex79 commented Oct 25, 2024

What happened:
When adding the CORS annotation for thee allowed origin:

nginx.ingress.kubernetes.io/cors-allow-headers: DNT,Keep-Alive,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Range,Authorization,X-Api-Key,Powered-By,Content-Encoding,Auto-Refresh
nginx.ingress.kubernetes.io/cors-allow-origin: https://mydomain.com
nginx.ingress.kubernetes.io/enable-cors: 'true'

the generated configuration looks like this:

if ($http_origin ~* ((https://mydomain\.com))$ ) { set $cors 'true'; }

This configuration limits the setting of $cors to true only for requests with origin matching https://mydomain.com and it does not enforce CORS setting for all the other requests.
For this reason penetration tests for the CORS settings fails because don't satisfy basic requirements for CORS enforcement (see https://owasp.org/www-project-web-security-testing-guide/latest/4-Web_Application_Security_Testing/11-Client-side_Testing/07-Testing_Cross_Origin_Resource_Sharing)

What you expected to happen:
You would expect that when enabled by the annotation the CORS settings are applied according to standards, and specifically the if statement before mentioned should instead be removed forcing the allowed origin to be set as specified by the annotation.

The template engine should be modified to rectify this vulnerability.

NGINX Ingress controller version (exec into the pod and run nginx-ingress-controller --version.):
registry.k8s.io/ingress-nginx/controller:v1.9.4@sha256:5b161f051d017e55d358435f295f5e9a297e66158f136321d9b04520ec6c48a3

Kubernetes version (use kubectl version):
1.27.10

Environment:

How to reproduce this issue:
Just set the following annotations to an ingress and check the resulting nginx.conf generated:

nginx.ingress.kubernetes.io/cors-allow-origin: https://mydomain.com
nginx.ingress.kubernetes.io/enable-cors: 'true

Anything else we need to know:
OWASP documentation: https://owasp.org/www-project-web-security-testing-guide/latest/4-Web_Application_Security_Testing/11-Client-side_Testing/07-Testing_Cross_Origin_Resource_Sharing
CORS configuration reference example: https://enable-cors.org/server_nginx.html

@stex79 stex79 added the kind/bug Categorizes issue or PR as related to a bug. label Oct 25, 2024
@k8s-ci-robot
Copy link
Contributor

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-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority labels Oct 25, 2024
@longwuyuan
Copy link
Contributor

longwuyuan commented Oct 26, 2024

/remove-kind bug
/kind support
/help

We need a CORS expert to comment here.

There was some work done to harden the CORS config. If you can not set more than the allowed origin, its bad for you but good for the project, as the risk is less when compared to allowing whatever origins you want to be set..

@k8s-ci-robot
Copy link
Contributor

@longwuyuan:
This request has been marked as needing help from a contributor.

Guidelines

Please ensure that the issue body includes answers to the following questions:

  • Why are we solving this issue?
  • To address this issue, are there any code changes? If there are code changes, what needs to be done in the code and what places can the assignee treat as reference points?
  • Does this issue have zero to low barrier of entry?
  • How can the assignee reach out to you for help?

For more details on the requirements of such an issue, please see here and ensure that they are met.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

/remove-kind bug
/kind support
/help

We need a CORS expert to comment here.

There was some work done to harden the CORS config. If you can not set more than the allowed origin, its bad for you for good for the project as risk is less when compared to allowing whatever origins you want to be set..

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.

@k8s-ci-robot k8s-ci-robot added kind/support Categorizes issue or PR as a support question. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. and removed kind/bug Categorizes issue or PR as related to a bug. labels Oct 26, 2024
@Gacko
Copy link
Member

Gacko commented Oct 30, 2024

Ingress NGINX Controller v1.9.4 is no longer supported. Please update to a more recent Controller version and try again. I remember some changes to CORS in the newer releases.

@stex79
Copy link
Author

stex79 commented Oct 31, 2024

As you can see from this link

originsRegex := "if ($http_origin ~* ("
the logic for building the CORS configuration with the origin wrapped inside a if statement is there even in the main branch, so there is no point to perform an update for this specific issue.

Copy link

This is stale, but we won't close it automatically, just bare in mind the maintainers may be busy with other tasks and will reach your issue ASAP. If you have any question or request to prioritize this, please reach #ingress-nginx-dev on Kubernetes Slack.

@github-actions github-actions bot added the lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. label Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/support Categorizes issue or PR as a support question. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.
Projects
Development

No branches or pull requests

4 participants