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-2023-5043: Ingress nginx annotation injection causes arbitrary command execution #10571

Closed
cjcullen opened this issue Oct 25, 2023 · 25 comments
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.

Comments

@cjcullen
Copy link
Member

cjcullen commented Oct 25, 2023

Issue Details

A security issue was identified in ingress-nginx where the nginx.ingress.kubernetes.io/configuration-snippet annotation on an Ingress object (in the networking.k8s.io or extensions API group) can be used to inject arbitrary commands, and 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-2023-5043.

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.

If you are running the “chrooted” ingress-nginx controller introduced in v1.2.0 (gcr.io/k8s-staging-ingress-nginx/controller-chroot), command execution is possible but credential extraction is not, so the High severity does not apply.

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

Affected Versions

  • <v1.9.0

Versions allowing mitigation

  • v1.9.0

Mitigation

Ingress Administrators should set the --enable-annotation-validation flag to enforce restrictions on the contents of ingress-nginx annotation fields.

Detection

If you find evidence that this vulnerability has been exploited, please contact security@kubernetes.io

Additional Details

See ingress-nginx Issue #10571 for more details.

Acknowledgements

This vulnerability was reported by suanve

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 Oct 25, 2023
@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Oct 25, 2023
@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/test-infra repository.

@cjcullen cjcullen changed the title Placeholder CVE-2023-5043: Ingress nginx annotation injection causes arbitrary command execution Oct 25, 2023
@danragnar
Copy link

When setting --enable-annotation-validation, is the validation done in the validating admission controller or later as well?

@phidlipus
Copy link
Contributor

phidlipus commented Oct 31, 2023

Is there any way how to mitigate this for versions <1.9.0? We are using v1.8.X version and according to compatility matrix we can't upgrade to later versions, because we are using Kubernetes <1.25.
Do you have any recommandations? Or is possible just ignore the compatibility matrix and update to the latest version?
Same question is also valid for CVE-2023-5044.

@thiDucTran
Copy link

thiDucTran commented Oct 31, 2023

sorry..to confirm we need to add --enable-annotation-validation to kind: Deployment of the ingress-controller right (https://github.com/kubernetes/ingress-nginx/blob/main/deploy/static/provider/cloud/deploy.yaml#L424) ?

@EishanR
Copy link

EishanR commented Oct 31, 2023

@thiDucTran I suppose you are supposed to add --enable-annotation-validation as a custom value before installing the official helm chart , this setting can be found here and for customizing the chart before installing : here

@helbros86
Copy link

helbros86 commented Oct 31, 2023

sorry..to confirm we need to add --enable-annotation-validation to kind: Deployment of the ingress-controller right (https://github.com/kubernetes/ingress-nginx/blob/main/deploy/static/provider/cloud/deploy.yaml#L424) ?

@thiDucTran,
You will need to update your local values.yaml file with this toggle set to true

enableAnnotationValidations: false

@tao12345666333
Copy link
Member

@phidlipus You can upgrade, there are no v1.25+ specific features in the new version of Ingress NGINX.

But please also make sure to test before upgrading.

@thiDucTran
Copy link

thanks all..fyi..if you are not using helm...the equivalent is to add --enable-annotation-validation=true as an argument in https://github.com/kubernetes/ingress-nginx/blob/main/deploy/static/provider/cloud/deploy.yaml#L424

@ElliotAlderson1312
Copy link

i tried to set nginx.ingress.kubernetes.io/enable-annotation-validations: "true" as an annotation or --enable-annotation-validation as an argument in my nginx controller deployment but im still able to set invalid values in my annotations.
Am i setting the flag the wrong way or misunderstanding the mitigation process.

@maeri
Copy link

maeri commented Nov 1, 2023

i tried to set nginx.ingress.kubernetes.io/enable-annotation-validations: "true" as an annotation or --enable-annotation-validation as an argument in my nginx controller deployment but im still able to set invalid values in my annotations. Am i setting the flag the wrong way or misunderstanding the mitigation process.

From k9s was able to add last line to ingress-controller

template:
metadata:
creationTimestamp: null
labels:
app.kubernetes.io/component: controller
app.kubernetes.io/instance: ingress-nginx
app.kubernetes.io/name: ingress-nginx
app.kubernetes.io/part-of: ingress-nginx
app.kubernetes.io/version: 1.9.4
spec:
containers:
- args:
- /nginx-ingress-controller
- --publish-service=$(POD_NAMESPACE)/ingress-nginx-controller
- --election-id=ingress-nginx-leader
- --controller-class=k8s.io/ingress-nginx
- --ingress-class=nginx
- --configmap=$(POD_NAMESPACE)/ingress-nginx-controller
- --validating-webhook=:8443
- --validating-webhook-certificate=/usr/local/certificates/cert
- --validating-webhook-key=/usr/local/certificates/key
- --enable-annotation-validation=true

@stromvirvel
Copy link

If the ingress controller is configured withallowSnippetAnnotations: false, is this vulnerability mitigated?

@sumit22nov
Copy link

If the ingress controller is configured withallowSnippetAnnotations: false, is this vulnerability mitigated?

+1

@ElliotAlderson1312
Copy link

ElliotAlderson1312 commented Nov 2, 2023

i tried to set nginx.ingress.kubernetes.io/enable-annotation-validations: "true" as an annotation or --enable-annotation-validation as an argument in my nginx controller deployment but im still able to set invalid values in my annotations. Am i setting the flag the wrong way or misunderstanding the mitigation process.

From k9s was able to add last line to ingress-controller

template: metadata: creationTimestamp: null labels: app.kubernetes.io/component: controller app.kubernetes.io/instance: ingress-nginx app.kubernetes.io/name: ingress-nginx app.kubernetes.io/part-of: ingress-nginx app.kubernetes.io/version: 1.9.4 spec: containers: - args: - /nginx-ingress-controller - --publish-service=$(POD_NAMESPACE)/ingress-nginx-controller - --election-id=ingress-nginx-leader - --controller-class=k8s.io/ingress-nginx - --ingress-class=nginx - --configmap=$(POD_NAMESPACE)/ingress-nginx-controller - --validating-webhook=:8443 - --validating-webhook-certificate=/usr/local/certificates/cert - --validating-webhook-key=/usr/local/certificates/key - --enable-annotation-validation=true

done this but still able to create ingress objects with this kind of annotations :
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
annotations:
kubernetes.io/ingress.class: nginx
invalid.annotation: "some malicious code"
nginx.ingress.kubernetes.io/configuration-snippet: invalid-configuration-snippet
kubernetes.io/tls-acme: "true"

@bmv126
Copy link

bmv126 commented Nov 3, 2023

Invalid annotation will not be understood by controller, so will be ignored.
To not allow snippets based input, you need to set allowSnippet to false in values.yaml

@rikatz
Copy link
Contributor

rikatz commented Nov 3, 2023

If the ingress controller is configured withallowSnippetAnnotations: false, is this vulnerability mitigated?

@stromvirvel unfortunatelly not

@rikatz
Copy link
Contributor

rikatz commented Nov 3, 2023

/assign @cpanato @rikatz @strongjz @tao12345666333

@rikatz
Copy link
Contributor

rikatz commented Nov 3, 2023

Hey folks,

As this CVE has been opened for 1 week now, I'm closing the issue.

The description of the issue contains all the required mitigations, and we plan in future releases to turn the validation on by default and also implement more safety measures.

Thank you all for using the project, and for your continuous support for us.

/close

@k8s-ci-robot
Copy link
Contributor

@rikatz: Closing this issue.

In response to this:

Hey folks,

As this CVE has been opened for 1 week now, I'm closing the issue.

The description of the issue contains all the required mitigations, and we plan in future releases to turn the validation on by default and also implement more safety measures.

Thank you all for using the project, and for your continuous support for us.

/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.

@stromvirvel
Copy link

@rikatz Thanks for the reply. Can you please explain how the vulnerability can be exploited via configuration-snippet annotation if we have disallowed this usage at all?

@reinildes
Copy link

Can someone suggest an invalid value for nginx.ingress.kubernetes.io/configuration-snippet one can use in order to validate that the remediation works and do a before and after test ?

@pna-nca
Copy link

pna-nca commented Nov 16, 2023

Can someone suggest an invalid value for nginx.ingress.kubernetes.io/configuration-snippet one can use in order to validate that the remediation works and do a before and after test ?

For all three these issues the exploitation is similar to what is described on this page: https://raesene.github.io/blog/2023/10/29/exploiting-CVE-2023-5044/
Only the approach to escape from the current nginx config context differs.
The existing regex-based validations do not work or can be bypassed.
In case of "configuration-snippet" what works as remediation: disabling this annotation.

@reinildes
Copy link

@pna-nca thanks for answering and providing that link as well.

Are you saying that the remediation suggested of --enable-annotation-validation doesn't work for configuration-snippet and disabling the annotation with controller.allowSnippetAnnotations: false should be used instead ?

Many people are using the remediation suggested by @cjcullen in their k8s cluster. If it is not effective, can we work and update the description to mention controller.allowSnippetAnnotations: false instead ?

@pna-nca
Copy link

pna-nca commented Nov 16, 2023

Are you saying that the remediation suggested of --enable-annotation-validation doesn't work for configuration-snippet and disabling the annotation with controller.allowSnippetAnnotations: false should be used instead ?

enable-annotation-validation does enable additional validations. Unfortunately, they are just regular expressions working in blacklist mode. This never gives 100% assurance that mitigation works. https://github.com/kubernetes/ingress-nginx/blob/main/internal/ingress/annotations/parser/validators.go#L62

There are some checks here: https://github.com/kubernetes/ingress-nginx/blob/main/internal/ingress/inspector/rules.go#L25 but they are easily bypassable.

As you can see here, configuration-snippet is not validated: https://github.com/kubernetes/ingress-nginx/blob/main/internal/ingress/annotations/snippet/main.go#L36 anyway.

Many people are using the remediation suggested by @cjcullen in their k8s cluster. If it is not effective, can we work and update the description to mention controller.allowSnippetAnnotations: false instead ?

I have a lot of questions why these stories were closed at all, as the product is still affected and suggested mitigations are "mitigations" at best (even if they would work), but not fixes.

On the other hand, configuration snippet annotation is indeed now disabled by default, which is indeed the best way to tackle this particular finding. :)

@strongjz
Copy link
Member

strongjz commented Nov 16, 2023

We take the same path here as other projects, ingress-nginx, nginx, even kubernetes can be deployed in an insecure way. We are trying to provide security and user flexibility. The annotations validation is a small step in that direction. Disabling snippets is a way for an administrator to mitigate the issue altogether as you pointed out. Any time you allow users to run untrusted code it could result in security flaws. We are actively trying to brake out the security model for data plane and the control plane to help increase the security of the controller. If you have other suggesti9ns @pna-nca please join our community meetings and we can discuss them. If there are serious security concerns still please disclose them by emailing secuirty@kubernetes.io

@pna-nca
Copy link

pna-nca commented Nov 16, 2023

We are actively trying to brake out the security model for data plane and the control plane to help increase the security of the controller.

Good to hear!
My suggestion is to avoid closing security vulnerabilities without investing sufficient time into validating the provided mitigations. I understand it is usually not trivial and time consuming. Luckily, the community provided quite some doubts already to reconsider the previously made decision.

I would suggest pinpointing that for this particular finding the mitigations are the following:

  • Keeping the (now default) setting allowSnippetAnnotations: false
  • If the above is not possible, limiting and auditing the number of users which are allowed to use configuration-snippet annotation. OPA Gatekeeper or Kyverno policies can do their job for this.

For vulnerability CVE-2023-5044 (permanent-redirect) the provided mitigation (enable-annotation-validation) will work, as basically only valid URL is expected.

Vulnerability CVE-2022-4886 (ImplementationSpecific) path is a really complicated beast. By definition user is allowed to use regular expressions there, which means "weird" symbols. However, as controller just inserts the provided setting into nginx config file, this might lead to config context escape. In order to avoid that one may just forbid ImplementationSpecific types via OPA Gatekeeper or Kyverno. However, this feature is way more used than even configuration-snippet. The provided mitigation in the closed finding is vague, enable-annotation-validation is not complete in this case as well, because dangerous symbols are allowed. I would adjust validation on controller' side and allow less characters in regex.

Any time you allow users to run untrusted code it could result in security flaws.

That is why we are relying on containerization. :) In this case, the controller provides functionality to adjust web server's configuration directly, without doing context-aware escape. It is similar to granting users a freedom of editing web page they are viewing: everything is fine, until there is an adversary (compromised user) which injects JavaScript to this page and benefit from others.

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. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.
Projects
Archived in project
Development

No branches or pull requests