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.

Sign up for GitHub

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

Jump to bottom

CVE-2022-4886: ingress-nginx path sanitization can be bypassed #126815

Closed
cjcullen opened this issue Oct 25, 2023 · 11 comments
Closed

CVE-2022-4886: ingress-nginx path sanitization can be bypassed #126815

cjcullen opened this issue Oct 25, 2023 · 11 comments
Assignees
@strongjz @tao12345666333 @cpanato @rikatz
Labels
area/security committee/security-response Denotes an issue or PR intended to be handled by the product security committee. kind/bug Categorizes issue or PR as related to a bug. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. official-cve-feed Issues or PRs related to CVEs officially announced by Security Response Committee (SRC) triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@cjcullen
Copy link
Member

cjcullen commented Oct 25, 2023 •
edited
Loading

Issue Details

A security issue was discovered in ingress-nginx where a user that can create or update ingress objects can use directives to bypass the sanitization of 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:H/A:H), and assigned CVE-2022-4886.

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

Versions allowing mitigation

  • v1.8.0

Mitigation

Ingress objects contain a field called pathType that defines the proxy behavior. It can be Exact, Prefix and ImplementationSpecific.

When pathType is configured as Exact or Prefix, there is more strict validation, allowing only paths starting with "/" and containing only alphanumeric characters and "-", "_" and additional "/".

When this option is enabled, the validation happens in the Admission Webhook, denying creation of any Ingress containing invalid characters (unless pathType is ImplementationSpecific).

https://kubernetes.github.io/ingress-nginx/user-guide/nginx-configuration/configmap/#strict-validate-path-type

Ingress Admins should enable this validation by default. If you still need to allow implementation specific paths due to the usage of features like Regex/rewrite on path, we recommend implementing countermeasures to allow just trusted users to consume this feature, as an example with OPA: https://kubernetes.github.io/ingress-nginx/examples/openpolicyagent/

Detection

If you find evidence that this vulnerability has been exploited, please contact [email protected]

Additional Details

See ingress-nginx Issue #10570 for more details.

Acknowledgements

This vulnerability was reported by Ginoah, working with the DEVCORE Internship Program.

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

@k8s-ci-robot k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Oct 25, 2023
@cjcullen cjcullen changed the title Placeholder CVE-2022-4886: Ingress-nginx path sanitization can be bypassed Oct 25, 2023
@agnis-mateuss agnis-mateuss mentioned this issue Oct 30, 2023
Merged
@lukos
Copy link

lukos commented Oct 31, 2023

Hi @cjcullen I think you mis-typed your description.

When pathType is configured as Exact or Prefix, there is more strict validation

should read as

When pathType is configured as Exact or Prefix, there should be a more strict validation

This is important because although the English is not great, it is saying that the strict validation that there "should be" is enabled by setting the strict-validate-path-type option, it is not saying that the validation is already strict and the option does something else.

@simon-wessel
Copy link

simon-wessel commented Nov 1, 2023 •
edited
Loading

Can this CVE be mitigated for type ImplementationSpecific by disallowing to set the nginx.ingress.kubernetes.io/use-regex annotation (using OPA)?

@miklezzzz miklezzzz mentioned this issue Nov 1, 2023
Merged
4 tasks
@ElliotAlderson1312
Copy link

i have changed PathType from ImplementationSpecific to Prefix in my ingriss objects , i want to demonstrate how this changement can mitigate the vulnerability.

@rikatz
Copy link
Contributor

rikatz commented Nov 3, 2023

hey folks.

As this issue is opened for a week now, and it contains all the information on it and mitigations, I am closing it.

Bear in mind that we will force the strict path validation on release v1.10, and this is not enabled by default right now because it can be a breaking change

Thank you all for using the project, and your continuous support for us on the feedbacks!

/close

@k8s-ci-robot
Copy link
Contributor

@rikatz: Closing this issue.

In response to this:

hey folks.

As this issue is opened for a week now, and it contains all the information on it and mitigations, I am closing it.

Bear in mind that we will force the strict path validation on release v1.10, and this is not enabled by default right now because it can be a breaking change

Thank you all for using the project, and your continuous support for us on the feedbacks!

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

@rikatz
Copy link
Contributor

rikatz commented Nov 3, 2023

/assign @strongjz @tao12345666333 @rikatz @cpanato

@paulohaacke paulohaacke mentioned this issue Nov 6, 2023
Closed
starlingx-github referenced this issue in starlingx/nginx-ingress-controller-armada-app Nov 16, 2023
Mitigate CVE-2022-4886 and CVE-2023-5044
462d728 
This commit adds the flags and config options recommended by the
community in:

https://github.com/kubernetes/ingress-nginx/issues/10570
https://github.com/kubernetes/ingress-nginx/issues/10572

CVE-2023-5044 is mitigated with enableAnnotationValidations
CVE-2022-4886 is mitigated with strict-validate-path-type

Test cases:

PASS: Full build, system install, bootstrap and unlock.
PASS: system application-update to this new version
PASS: Create Ingress resource with special character in path /apple$,
      Verify it's possible to curl localhost/apple$.
      Apply strict-validate-path-type override and verify creating the
      same Ingress object is not possible anymore, neither curl works.
PASS: Create Ingress resource with special characters and verify that
      it creates successfully.

      annotations:

      nginx.ingress.kubernetes.io/permanent-redirect: |
        https://www.google.com$HOST

      Apply enableAnnotationValidations override and verify creating the
      same Ingress object is not possible anymore and a validation error
      is now returned.
PASS: stx-openstack applies without error.

Closes-Bug: 2042977

Change-Id: I2f2279ebb34094d0a21d4440e48ef890f09a6133
Signed-off-by: Rei Oliveira <[email protected]>
openstack-mirroring referenced this issue in openstack/openstack-helm-infra Jan 18, 2024
Change default ingress path type to prefix
5b72041 
Due to CVE-2022-4886 the default pathType for an ingress should be
either "Exact" or "Prefix". This allows for more strict path validation by
the admission controller. This PS changes the default pathType to Prefix.
This value can be overridden.

In a separate PS I will add the pathType parameter to the ingressOpts
for all helm charts that create an ingress.

See:

https://github.com/kubernetes/ingress-nginx/issues/10570

Change-Id: I8f1df594f0c86f2de6cdd7cf2ee56637bd508565
openstack-mirroring referenced this issue in openstack/openstack Jan 18, 2024
@openstack-gerrit
Update git submodules
d6e6394 
* Update openstack-helm-infra from branch 'master'
  to d0c93b5648704b9d9999412d932f8329c80362c0
  - Merge "Change default ingress path type to prefix"
  - Change default ingress path type to prefix
    
    Due to CVE-2022-4886 the default pathType for an ingress should be
    either "Exact" or "Prefix". This allows for more strict path validation by
    the admission controller. This PS changes the default pathType to Prefix.
    This value can be overridden.
    
    In a separate PS I will add the pathType parameter to the ingressOpts
    for all helm charts that create an ingress.
    
    See:
    
    https://github.com/kubernetes/ingress-nginx/issues/10570
    
    Change-Id: I8f1df594f0c86f2de6cdd7cf2ee56637bd508565
@sharebear
Copy link

Bear in mind that we will force the strict path validation on release v1.10, and this is not enabled by default right now because it can be a breaking change

@rikatz I don't see this listed as a breaking change on the 1.10 release that came out last month. Has the default value not yet been changed, or is it missing from the changelog?

@AlbinoDrought AlbinoDrought mentioned this issue Jun 10, 2024
Open
@cji
Copy link
Member

cji commented Aug 20, 2024

/transfer kubernetes

@k8s-ci-robot k8s-ci-robot added the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Aug 20, 2024
@k8s-ci-robot k8s-ci-robot transferred this issue from kubernetes/ingress-nginx Aug 20, 2024
@cji
Copy link
Member

cji commented Aug 20, 2024

/area security
/kind bug
/committee security-response
/triage accepted
/lifecycle frozen
/label official-cve-feed

@k8s-ci-robot k8s-ci-robot added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. area/security labels Aug 20, 2024
@k8s-ci-robot k8s-ci-robot added committee/security-response Denotes an issue or PR intended to be handled by the product security committee. triage/accepted Indicates an issue or PR is ready to be actively worked on. official-cve-feed Issues or PRs related to CVEs officially announced by Security Response Committee (SRC) and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Aug 20, 2024
@sftim
Copy link
Contributor

sftim commented Aug 20, 2024

To make the CVE feed look a little nicer:
/retitle CVE-2022-4886: ingress-nginx path sanitization can be bypassed

(removing the Markdown, fixing the case)

@k8s-ci-robot k8s-ci-robot changed the title CVE-2022-4886: Ingress-nginx path sanitization can be bypassed CVE-2022-4886: ingress-nginx path sanitization can be bypassed Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@strongjz strongjz

@tao12345666333 tao12345666333

@cpanato cpanato

@rikatz rikatz

Labels
area/security committee/security-response Denotes an issue or PR intended to be handled by the product security committee. kind/bug Categorizes issue or PR as related to a bug. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. official-cve-feed Issues or PRs related to CVEs officially announced by Security Response Committee (SRC) triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
[SIG Network] Ingress NGINX
Archived in project
Milestone
No milestone
Development

No branches or pull requests