Closed NLRemco closed 2 years ago
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.
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 createTokenReview
resources. With the removal of this config entry, every service account that vault is authenticating will need to have access to createTokenReview
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!
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?
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 removetoken_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!
After a lot of discussions: closing the issue as an alternative has been chosen to be implemented instead.
@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
@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.
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 theTokenReview
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 thetoken_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.