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

CVE-2021-25745: Ingress-nginx path can be pointed to service account token file #8502

Closed
cjcullen opened this issue Apr 22, 2022 · 11 comments
Closed
Labels
kind/bug Categorizes issue or PR as related to a bug. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@cjcullen
Copy link
Member

cjcullen commented Apr 22, 2022

Issue Details

A security issue was discovered in ingress-nginx where a user that can create or update ingress objects can use the spec.rules[].http.paths[].path field of an Ingress object (in the networking.k8s.io or extensions API group) to obtain the credentials of the ingress-nginx controller. In the default configuration, that credential has access to all secrets in the cluster.

This issue has been rated High (CVSS:3.1/AV:N/AC:L/PR:L/UI:N/S:U/C:H/I:L/A:L), and assigned CVE-2021-25745.

Affected Components and Configurations

This bug affects ingress-nginx. If you do not have ingress-nginx installed on your cluster, you are not affected. You can check this by running kubectl get po -n ingress-nginx.

Multitenant environments where non-admin users have permissions to create Ingress objects are most affected by this issue.

Affected Versions

  • <v1.2.0

Fixed Versions

  • v1.2.0-beta.0
  • v1.2.0

Mitigation

If you are unable to roll out the fix, this vulnerability can be mitigated by implementing an admission policy that restricts the spec.rules[].http.paths[].path field on the networking.k8s.io/Ingress resource to known safe characters (see the newly added rules, or the suggested value for annotation-value-word-blocklist).

Detection

If you find evidence that this vulnerability has been exploited, please contact security@kubernetes.io
Additional Details
See ingress-nginx Issue #8502 for more details.

Acknowledgements

This vulnerability was reported by Gafnit Amiga.

Thank You,
CJ Cullen on behalf of the Kubernetes Security Response Committee

@cjcullen cjcullen added the kind/bug Categorizes issue or PR as related to a bug. label Apr 22, 2022
@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 Apr 22, 2022
@cjcullen cjcullen changed the title Placeholder CVE-2021-25745: Ingress-nginx path can be pointed to service account token file Apr 22, 2022
@fhke
Copy link

fhke commented Apr 22, 2022

Should the invalidSecretsDir regex be updated to /run/secrets? In the default ingress-nginx image, /var/run is symlinked to /run, so the service account token can be accessed through /run/secrets:

bash-5.1$ ls -l /var/run
lrwxrwxrwx    1 root     root             4 Apr  4 16:07 /var/run -> /run
bash-5.1$ ls -l /run/secrets/kubernetes.io/serviceaccount/
total 0
lrwxrwxrwx    1 root     root            13 Apr 22 17:44 ca.crt -> ..data/ca.crt
lrwxrwxrwx    1 root     root            16 Apr 22 17:44 namespace -> ..data/namespace
lrwxrwxrwx    1 root     root            12 Apr 22 17:44 token -> ..data/token
bash-5.1$ 

Updating the regex to /run/secrets would block both paths.

@strongjz
Copy link
Member

/triage accepted
/priority critical-urgent

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority labels Apr 22, 2022
@longwuyuan
Copy link
Contributor

@cjcullen , I am trying to experience this exploit in a instance of the ingress-nginx-controller, with chroot enabled on the nginx process.

Can you help me gt a example value for the spec ingress.spec.rules[].http.paths[].path that will demonstrate this vulnerability, on a chrooted nginx process of the controller.

@foxylion
Copy link
Contributor

We were recently notified about this CVE. I see this seems to be fixed in v1.2.0.
Is there anything except from updating to the latest version required to mitgate this type of exploit?

@chipzoller
Copy link

@foxylion yes, we are providing a Kyverno policy in the above linked PR (kyverno/policies#302)

@xcq1
Copy link

xcq1 commented Apr 27, 2022

@chipzoller This is only an alternative, if I can't update, correct?
From the CVE it sounds as if the whole fix is included in ingress-nginx v1.2.0.

@bmv126
Copy link

bmv126 commented Apr 27, 2022

As the rules for deep inspect are hard-coded in the code.
Also as mentioned above by @fhke some rules are missed.
So it doesn't look like fixed in 1.2.0 just by upgrading.
May be just like bad annotation list these rules also should be exposed to user.

@bmv126
Copy link

bmv126 commented Apr 27, 2022

@strongjz @rikatz any comments on this being completely fixed in 1.2.0

@longwuyuan
Copy link
Contributor

@bmv126 this was discussed in the last community meeting. Maintainers were aware of this and are working on it from even before the issue was created or maybe even before the CVE was published.

The controller v1.20 introduces capability to jail/charoot the nginx process so it has been tested that the obvious use case of getting a shell, by breaking out of the nginx process, will land the actor in a jailed/chrooted shell, if at all. That is one layer of protection.

If you want to discuss further, it was reported that users can use this link https://github.com/kubernetes/ingress-nginx/blob/main/SECURITY.md and be discreet about putting sensitive info in public places. You could also reach out to maintainers without including groups in such communication, so as to reduce the radius for info spread.

@rikatz
Copy link
Contributor

rikatz commented May 10, 2022

@bmv126 can you please open a PR adding the new rules?

As the main code to fix this is shipped, I'm going to close this issue but I have some additional rules myself I want to add as well :)

/close

@k8s-ci-robot
Copy link
Contributor

@rikatz: Closing this issue.

In response to this:

@bmv126 can you please open a PR adding the new rules?

As the main code to fix this is shipped, I'm going to close this issue but I have some additional rules myself I want to add as well :)

/close

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

No branches or pull requests

10 participants