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-25742: Ingress-nginx custom snippets allows retrieval of ingress-nginx serviceaccount token and secrets across all namespaces #7837

Closed
cjcullen opened this issue Oct 21, 2021 · 60 comments
Assignees
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 Oct 21, 2021

Issue Details

A security issue was discovered in ingress-nginx where a user that can create or update ingress objects can use the custom snippets feature to obtain 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-25742.

Affected Components and Configurations

This bug affects ingress-nginx.

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

Affected Versions with no mitigation

  • v1.0.0
  • <= v0.49.0

Versions allowing mitigation

This issue cannot be fixed solely by upgrading ingress-nginx. It can be mitigated in the following versions:

  • v1.0.1
  • v0.49.1

Mitigation

To mitigate this vulnerability:

  1. Upgrade to a version that allows mitigation, (>= v0.49.1 or >= v1.0.1)

  2. Set allow-snippet-annotations to false in your ingress-nginx ConfigMap based on how you deploy ingress-nginx:

    Static Deploy Files
    Edit the ConfigMap for ingress-nginx after deployment:

    kubectl edit configmap -n ingress-nginx ingress-nginx-controller
    

    Add directive:

    data:
      allow-snippet-annotations: “false”
    

    More information on the ConfigMap here

    Deploying Via Helm
    Set controller.allowSnippetAnnotations to false in the Values.yaml or add the directive to the helm deploy:

    helm install [RELEASE_NAME] --set controller.allowSnippetAnnotations=false ingress-nginx/ingress-nginx
    

    https://github.com/kubernetes/ingress-nginx/blob/controller-v1.0.1/charts/ingress-nginx/values.yaml#L76

Detection

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

Acknowledgements

This vulnerability was reported by Mitch Hulscher.

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 21, 2021
@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 21, 2021
@k8s-ci-robot
Copy link
Contributor

@cjcullen: 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 HOLD CVE-2021-25742: Ingress-nginx custom snippets allows retrieval of ingress-nginx serviceaccount token and secrets across all namespaces Oct 21, 2021
@rikatz
Copy link
Contributor

rikatz commented Oct 21, 2021

/assign

@rikatz
Copy link
Contributor

rikatz commented Oct 21, 2021

Fixed in 0.X branch -> #7666
Fixed in v1.X branch -> #7670

@rikatz rikatz added 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. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Oct 21, 2021
@paolomainardi
Copy link

paolomainardi commented Oct 21, 2021

I don't know if it related but our ingresses stopped to work 2 hours ago (more or less) with:

Error: UPGRADE FAILED: cannot patch "develop-website-ingress" with kind Ingress: Internal error occurred: failed calling webhook "validate.nginx.ingress.kubernetes.io": Post "https://ingress-nginx-controller-admission.ingress-nginx.svc:443/networking/v1beta1/ingresses?timeout=10s": service "ingress-nginx-controller-admission" not found

And the service was not in place at all, just the ingress-nginx-admission was present, i had to manually delete it, the fun fact is that the ingress-nginx installation had the admission webhook disabled, still trying to face what happened here.

GKE - 1.19.13-gke.1200

@iamNoah1
Copy link
Contributor

@paolomainardi I don't think so. The CVE says, that non admin users who create or update ingress nginx instances, could possibly all secrets of the cluster. Sound more like a bug TBH. Feel free to raise an issue :)

@paolomainardi
Copy link

@iamNoah1 yes i guess so, but you know the timing was so close that i thought it was worth sharing, thanks :)

@Typositoire
Copy link

Might be obvious to others but there's an actually FIX coming right ? The final fix isn't removing a feature ?

@dezmodue
Copy link

Hi, I would appreciate some clarification.

Is the ability to retrieve "secrets across all namespaces" a consequence of "Ingress-nginx custom snippets allows retrieval of ingress-nginx serviceaccount token"?

In other words, the vulnerability allows a user to access the serviceaccount token and as a result of that the service token can then be used to access any resource that the token is allowed?

@raesene
Copy link

raesene commented Oct 22, 2021

@dezmodue I had a look at this issue today, and there's definitely an attack path that gets the ingress-nginx service account token, which has list rights on secrets at a cluster level (so allowing for all secret values to be retrieved). There may be other attack paths as well, but that is one of them.

@longwuyuan
Copy link
Contributor

longwuyuan commented Oct 22, 2021 via email

@dezmodue
Copy link

Hi @raesene, thanks for your reply.

In some setups the ingress controller is "namespaced" and the service account is only allowed to get secrets that belong to the same namespace. The users with access to the namespace are the ones managing the ingress controller and have access to the token already so this seems to be a non issue in this scenario.
Is my understanding correct?

@dwertent
Copy link

Well Kubescape already came out with a control that will detect whether your cluster has this vulnerability

image

@raesene
Copy link

raesene commented Oct 22, 2021

@dezmodue So for the case I found, I think so. Basically the snippets facility seems to provide effectively full control over the nginx config and access to the ingress-controller container, so if those are effectively not issues (i.e. all the resources owned by the ingress controller are already owned by the individual teams) then it doesn't seem like there'd be a big impact (AFAIK and I'm not one of the devs, just a curious security person :) )

@roy-work
Copy link

What is the actual vuln.?

See ingress-nginx Issue #7837 for more details.

That link is to this issue, i.e., it links to itself "for more details".

@rikatz
Copy link
Contributor

rikatz commented Oct 22, 2021

Hi folks, I'm gonna try to answer all your questions here, and we are also scheduling an office hours to explain about this vulnerability and why you should take care of it.:

@Typositoire: Might be obvious to others but there's an actually FIX coming right ? The final fix isn't removing a feature ?
We didn't discussed actually how to fix this, and it may be hard. The main problem is that the custom snippet annotations allows users to add code that might be executed by Nginx (via Lua) so we should think in a way to map and drop those actions, or create maybe a "safe/non safe" directives that may be dropped when used in annotations.

@dezmodue Is the ability to retrieve "secrets across all namespaces" a consequence of "Ingress-nginx custom snippets allows retrieval of ingress-nginx serviceaccount token"?
Yes. Ingress nginx demands a full secret access on your cluster, as the TLS certificates that you may point in your ingress object are secrets. So using ingress-nginx service account allows you to query any secret in your cluster (and other stuff, take a look into the RBAC file)

@raesene So for the case I found, I think so. Basically the snippets facility seems to provide effectively full control over the nginx config and access to the ingress-controller container, so if those are effectively not issues (i.e. all the resources owned by the ingress controller are already owned by the individual teams) then it doesn't seem like there'd be a big impact (AFAIK and I'm not one of the devs, just a curious security person :) )

Correct. If you trust users with access to the ingress object there is no big deal. The problem here is, if you share a cluster, and some user uses the custom annotation to add random code that can be used to exfiltrate secrets (or do other stuff).

@roy-work What is the actual vuln.?

This is the actual vuln. You can use custom snippets to run arbitrary code and exfiltrate secrets from the container running ingress nginx.

Got a question in Slack if disabling snippets will disable all of the directives (modsecurity, etc).
The answer for this is in here but explaining:

@roy-work
Copy link

@roy-work What is the actual vuln.?

This is the actual vuln. You can use custom snippets to run arbitrary code and exfiltrate secrets from the container running ingress nginx.

@rikatz I should have been more precise, I guess. I surmised that it was probably the case that some sort of code execution was occurring (others are guessing that that is used to access the service account credentials, and from there, "it's obvious" suffices). My understanding of snippets is that they're nginx configurations; how is it that I can run code from an nginx configuration? (Does nginx have a directive that amounts to an exec(2)?)

@Typositoire
Copy link

Typositoire commented Oct 22, 2021

@roy-work

The main problem is that the custom snippet annotations allows users to add code that might be executed by Nginx (via Lua) so we should think in a way to map and drop those actions, or create maybe a "safe/non safe" directives that may be dropped when used in annotations.

With

Yes. Ingress nginx demands a full secret access on your cluster, as the TLS certificates that you may point in your ingress object are secrets. So using ingress-nginx service account allows you to query any secret in your cluster (and other stuff, take a look into the RBAC file)

It's not split from the original questions, I guess this was his answer :p

@rikatz
Copy link
Contributor

rikatz commented Oct 22, 2021

@roy-work I prefer in this case not enter in details, just bare in mind that NGINX does not have this directive, but we use some other modules that may allow the vulnerability written here :)

@Sh1ftry
Copy link

Sh1ftry commented Nov 22, 2021

On each sync the blocklist is logged with error level. Shouldn't it have info level and a bit clearer message like Checking snippet annotations for blocklist: ...?

klog.Errorf("Blocklist: %v", s.backendConfig.AnnotationValueWordBlocklist)

@raesene
Copy link

raesene commented Dec 4, 2021

As this is now publicly available, seems like the technical details in the HackerOne report for this issue could be useful for people looking to design/test mitigations https://hackerone.com/reports/1249583

@sdickhoven
Copy link

sdickhoven commented Dec 13, 2021

rather than opening a new issue, i'm going to use this one to mention an ambiguity in the documentation.

this pr adds documentation for "suggested" strings that should be blocked in *-snippet annotations: #7942

_**suggested:**_ `"load_module,lua_package,_by_lua,location,root,proxy_pass,serviceaccount,{,},',\"`

it is not clear whether the last character that should be blocked is a \ or a " since the above string is not really valid in any interpreter. since ' is excluded, maybe " should be excluded too? or is \ the troublesome character?

maybe the documentation intends to list all the troublesome strings in which case it may be better to do:

_**suggested:**_ "`load_module,lua_package,_by_lua,location,root,proxy_pass,serviceaccount,{,},',\`"

- OR -

_**suggested:**_ "`load_module,lua_package,_by_lua,location,root,proxy_pass,serviceaccount,{,},',"`"

or

_**suggested:**_ `"load_module,lua_package,_by_lua,location,root,proxy_pass,serviceaccount,{,},',\\"`

- OR -

_**suggested:**_ `"load_module,lua_package,_by_lua,location,root,proxy_pass,serviceaccount,{,},',\""`

or

_**suggested:**_ `load_module,lua_package,_by_lua,location,root,proxy_pass,serviceaccount,{,},',\`

- OR -

_**suggested:**_ `load_module,lua_package,_by_lua,location,root,proxy_pass,serviceaccount,{,},',"`

with the double quotes included in the code comment, it seems like this should be a copy-paste-able string for my yaml config. but, of course, the yaml parser will bawk at the unterminated string: " ... \". a valid string would be " ... \"" or " ... \\".

@zohebs341
Copy link

zohebs341 commented Jan 12, 2022

Hi All,

Do we need to update configmap, even if nginx version is 1.19.4

As I’m expecting, by default below parameter will be false in nginx 1.19.4

allow-snippet-annotations:“false”

appreciate your inputs

@sathieu
Copy link
Contributor

sathieu commented Apr 8, 2022

rather than opening a new issue, i'm going to use this one to mention an ambiguity in the documentation.

this pr adds documentation for "suggested" strings that should be blocked in *-snippet annotations: #7942

Proposed a PR #8446.

@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?

@rikatz
Copy link
Contributor

rikatz commented May 1, 2022

@foxylion unfortunately not. You can still block the usage of *snippets, but there;s another vulnerability in main ingress object that should be dealt with. What version are you?

I'm closing this issue as it's already fixed in the latest version, and we are planning to deprecate the legacy version so I highly recommend migrating to ingress v1.X

Thanks!
/close

@k8s-ci-robot
Copy link
Contributor

@rikatz: Closing this issue.

In response to this:

@foxylion unfortunately not. You can still block the usage of *snippets, but there;s another vulnerability in main ingress object that should be dealt with. What version are you?

I'm closing this issue as it's already fixed in the latest version, and we are planning to deprecate the legacy version so I highly recommend migrating to ingress v1.X

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

@immanuelfodor
Copy link

Why is that I do not have an --enable-snippets CLI flag on the ingress controller? Is this behind some other feature flag that I need to enable first?

image

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