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

Remove usage of token_reviewer_jwt completely #128

Conversation

NLRemco
Copy link

@NLRemco NLRemco commented Oct 28, 2021

Overview

Removing the usage of token_reviewer_jwt token entry from the KubeConfig. This means that the Kubernetes Auth plugin no longer uses this predefined variable and that all the validation and authentication is done on the JWT token sent to Kubernetes.
This changes will affect Kubernetes clusters using RBAC for service accounts currently authenticating to vault. With the change, every single service account token attached to the entity attempting to Authenticate to Vault will require to have the system:auth-delegator RoleBinding attached to it. This is necessary to access the TokenReview API.

The original intent for this solution was found when reviewing access to Vault with the newly introduced service account token volume projection. To be more precise: it was reviewed on a setup which included an external Vault. And as a final note: when using an external Vault that you are currently only able to use a static token_reviewer_jwt rather than having it be loaded from the local file system, since this won't be possible since you are then loading the token_reviewer_jwt from the Vault pod rather than the client's pod, excluding the possibility to use the newly introduced projection or to have the possibility to have short-lived tokens in general for this use case. This only makes things more complex as the amount of possibilities to obtain a service account tokens increase, combined with the use cases for Vault such as: internal Vault, external Vault. Having a "one shoe fits all" solution sounded like a nice idea.
With this, my main concern for this issue to replace and/or remove the token_reviewer_jwt had to do with security. Having a static-based secret token with the capabilities that it has inside Vault, instead of an ephemeral token, was heavier than when changing roles in the K8s cluster; however this depends on your level of trust in your user, the impact of the role being present and the attack service. I agree that having the possibility to revoke access to the k8s API for a specific service account is very important, so this might be a good topic for discussion.

Design of Change

Removing the usage of token_reviewer_jwt token entry from the KubeConfig and while sending a TokenReview request.

Alternatives

Support various deployment models, (for external clusters) support the refreshing of the token_reviewer_jwt through f.e. TokenRequest API rather than a static secret-based service account token. Or different ways to integrate with Kubernetes outside of the TokenReview API such as: OIDC issuer validation.

Related Issues/Pull Requests

Issue #12951
PR #12980

Contributor Checklist

[x] Add relevant docs to upstream Vault repository, or sufficient reasoning why docs won’t be added yet
My Docs PR Link
Example Nr 1
Example Nr 2
Example Nr 3
[x] Add output for any tests not ran in CI to the PR description (eg, acceptance tests)
[x] Backwards compatible
Vault keeps working as before, only difference will be approach of the validation of the sent-in JWT token. However, this PR aims towards removing this functionality.
[x] Check for tests
Test outcome is the same, the validation process still returns the same valid results
[x] Add alternatives
Add alternatives to be able to possible forward the issue.

@hashicorp-cla
Copy link

hashicorp-cla commented Oct 28, 2021

CLA assistant check
All committers have signed the CLA.

@anush-cr
Copy link

anush-cr commented Nov 9, 2021

this changes nothing for any user of Vault.

I think this will affect the k8s RBAC for service accounts currently authenticating to vault. With the token_reviewer_jwt config entry, we only needed to grant this single service account access to create TokenReview resources. With the removal of this config entry, every service account that vault is authenticating will need to have access to create TokenReview resources.

@NLRemco
Copy link
Author

NLRemco commented Nov 9, 2021

this changes nothing for any user of Vault.

I think this will affect the k8s RBAC for service accounts currently authenticating to vault. With the token_reviewer_jwt config entry, we only needed to grant this single service account access to create TokenReview resources. With the removal of this config entry, every service account that vault is authenticating will need to have access to create TokenReview resources.

@anush-cr You are 100% correct, I forgot to update my PR with this detail after including this in the issue, and updated it now to include this!

@mitsutaka
Copy link

This is a breaking change for me. I don't prefer Vault authenticating to the Kubernetes API using any client's service account token. Enforcing a system:auth-delegator ClusterRole on all service accounts performing Vault logins, regardless of long-term or short-term tokens, will be a big role change on the K8s cluster. It's important to validate the pod's service account JWT, but it still can restrict logins with the k8s auth backend role. Is it possible to keep the current implementation? Or any extra reasons why you want to remove token_reviewer_jwt completely?

@NLRemco
Copy link
Author

NLRemco commented Nov 17, 2021

This is a breaking change for me. I don't prefer Vault authenticating to the Kubernetes API using any client's service account token. Enforcing a system:auth-delegator ClusterRole on all service accounts performing Vault logins, regardless of long-term or short-term tokens, will be a big role change on the K8s cluster. It's important to validate the pod's service account JWT, but it still can restrict logins with the k8s auth backend role. Is it possible to keep the current implementation? Or any extra reasons why you want to remove token_reviewer_jwt completely?

@mitsutaka It is possible to keep the current implementation, however there are some parts that feel like they are missing when it comes to maintaining/storing/fetching authentication tokens. Perhaps rather than removing this feature: replacing it or adding additional token validation methods might be a better pick since it can be used more widely. What do you think f.e. about the noted possible alternatives?

To add to the issue: the original intent for this solution was found when reviewing access to Vault with the newly introduced service account token volume projection. To be more precise: it was reviewed on a setup which included an external Vault. And as a final note: when using an external Vault that you are currently only able to use a static token_reviewer_jwt rather than having it be loaded from the local file system, since this won't be possible since you are then loading the token_reviewer_jwt from the Vault pod rather than the client's pod, excluding the possibility to use the newly introduced projection or to have the possibility to have short-lived tokens in general for this use case. This only makes things more complex as the amount of possibilities to obtain a service account tokens increase, combined with the use cases for Vault such as: internal Vault, external Vault. Having a "one shoe fits all" solution sounded like a nice idea.
With this in mind, my main concern for this issue to replace and/or remove the token_reviewer_jwt had to do with security, usability and simplicity. Having a static-based secret token with the capabilities that it has inside Vault, instead of an ephemeral token, was (in my opinion) heavier than when adding the capability to authenticate and authorise themselves or another to a Kubernetes Cluster; however this depends on your level of trust in your user, the impact of the role being present, the attack service, etc. What is your view on this?
I agree that having the possibility to revoke access to the k8s API for a specific service account is something important and is far easier than having to revoke possibly multiple service account(s), so this might be a good topic for discussion and I am looking forward to your reaction!

@NLRemco
Copy link
Author

NLRemco commented Nov 17, 2021

After a lot of discussions: closing the issue as an alternative has been chosen to be implemented instead.

@mitsutaka
Copy link

@RemcoBuddelmeijer Thank you for sharing your case! this is interesting case that I didn't know about.

I've come up with a way to verify the requested token, but I'm not sure this is the best solution.

It may be possible to pass the JWT requested by the client to jwt of the TokenReview spec and use the long-term token_reviewer_jwt to check the token. This solves at least my cases that keep system:auth-delegator to only one Service Account. however, it doesn't solve that still have to keep static token_reviewer_jwt.

On the other hand, the way to verify the requested token without applying the ClusterRole is to call SelfSubjectAccessReview (equivalent to kubectl auth can-i). We can verify that the token itself is valid, but we don't call the Kubernetes API from Vault, so we don't specify a resource. Moreover, the audience specified by the service account token volume projection cannot be validated. It may also be incompatible with the more recently implemented option to assign {namespace}/{serviceaccount name} as the Vault's Identity Alias ​​name (#110). You can limit the available audiences with the role of the Kubernetes Auth method, but if you want to validate with the Kubernetes API including the audience, there is only the TokenReview API. But you need a ClusterRole to call this.

Alternatively, if the vault is running in a pod, you may be able to completely disable token_reviewer_jwt by calling the TokenRequest API and setting a short TTL + TokenReview permission on the token created.vault-csi-provider adopts a similar method

@NLRemco
Copy link
Author

NLRemco commented Nov 24, 2021

@mitsutaka I have read your case(s) ..

I am not entirely sure what you are trying to say with your first case? This is how it currently is implemented, where the token_reviewer_jwt is used as the Authentication header for a TokenReview request populated with the JWT token passed in by the client when authenticating with Vault. The "long term token" is currently in use when having an external Auth where instead of basing your token_reviewer_jwt off of a pod's service account token, you base it of the secret based service account token linked to the service account itself.

The issue with this token_reviewer_jwt token is that currently it is not reloadable. And with the new Bound Service Account Token Volume this will be a problem as these tokens are ephemeral and expire after a certain amount of time or when the pod (that is the owner of this token) is deleted. This issue could easily be resolved by allowing to reload the token on the local file system before it's expired, however one other issue still remains.
The other issue is only applicable when you have an external Vault server. Even though this token does not expire (as mentioned before), it is in my opinion just not the safest type of maintaining a service account token and keeping up with security standards is always a huge plus.

I think when looking at the SelfSubjectAccessReview, or any SubjectAccessReview subtype, there is one huge downside. While this approach fits in the access control category: this resource is not intended to verify the token that is being sent in. This is only negligible if you very specifically define that when authenticating/authorising: your cluster ALSO has to send a webhook to the TokenReview API.
I am however not sure how this would allow Vault not to have to call the Kubernetes API from Vault?
So I don't think this would add any more value other than simply attempting to authenticate to Kubernetes with the sent-in service account token and executing any action on it.

Perhaps the only other approach. Users would then have to define a token that is used to authenticate both (and facilitate one of them) the TokenRequest and the TokenReview requests.
One huge downside that I see with this is that then that token has to have even more permissions than initially needed. It not only needs to be able to access the TokenReview API, but it also needs to be able to create tokens for service account tokens through the TokenRequest API. It would be an immense task to ensure that this token would not be capable of creating service account tokens of any service account other than the intended service account(s). So once again this wouldn't be a good solution.

What I have been looking at, and still am, is an approach where you authenticate with something other than a service account token. Preferably something Kubernetes native. Since really the only other approach would be external validation which would introduce complex setup(s) that might not fit into ones and Vault's infrastructure.
It would be a shame if the logic would have to be split between having an internal and external Vault, since this adds a lot of confusion when setting a Vault cluster up.
While typing this out, I noticed that a PR is open with instructions on how to allow Vault itself to validate the tokens when acting as an OIDC provider, more about this PR over here. At the file "website/content/docs/auth/kubernetes.mdx", line 122, talks about removing the usage of token_reviewer_jwt and in favor uses the client provided JWT: this would be in line with this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants