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

Best Practice / Docs clarification on token_reviewer_jwt #106

Closed
gavinelder opened this issue Mar 17, 2021 · 4 comments
Closed

Best Practice / Docs clarification on token_reviewer_jwt #106

gavinelder opened this issue Mar 17, 2021 · 4 comments

Comments

@gavinelder
Copy link

gavinelder commented Mar 17, 2021

👋 First of all, thank you for this amazing plugin.

The following is a clarification / potential best practice docs contribution around the token_reviewer_jwt which I am happy to do but I just wanted to flesh out the thoughts and views of this community before raising any PRs.

I also see a number of issues that indicate some confusion around authentication methods #95 (comment)

Current Mental Model of Auth methods for this plugin.

The following is my current mental model of these plugins authentication methods and approaches please correct me if anything is incorrect or there are additional risks I have not considered in relation to the delegation of system:auth-delegator

For example, I have not fully considered the privilege of a Service Account or the risks associated with account enumeration or other lateral movement paths.

Background

In late 2017 #6 was raised to allow the use of an optional dedicated vault account to be used by vault to authenticate this service account would have permission to use the TokenReview API

The TokenReview API is an API call to see if a JWT Token received as part of the authentication request to the Vault instance is still valid and has not been revoked before issuing a vault session token.

Possible approaches

When the use of a vault agent in a remote cluster is removed I see three valid approaches one can take to administering and granting Authorization against a remote vault instance.

Using a shared service account

A shared service account can be used to validate the JWT token is indeed valid at the time of the request, this service account can be used

apiVersion: rbac.authorization.k8s.io/v1beta1
kind: ClusterRoleBinding
metadata:
  name: role-tokenreview-binding
  namespace: default
roleRef:
  apiGroup: rbac.authorization.k8s.io
  kind: ClusterRole
  name: system:auth-delegator
subjects:
  - kind: ServiceAccount
    name: vault-auth
    namespace: default
  • Pro
    • This reduces the need to grant TokenReview API permissions to Kubernetes services
  • Con

CRD to grant all service accounts access to the TokenReview API

apiVersion: rbac.authorization.k8s.io/v1beta1
kind: ClusterRoleBinding
metadata:
  name: role-tokenreview-binding
  namespace: default
roleRef:
  apiGroup: rbac.authorization.k8s.io
  kind: ClusterRole
  name: system:auth-delegator
subjects:
- kind: Group
  name: system:serviceaccounts
  apiGroup: rbac.authorization.k8s.io
  • Pro
    • This reduces the need to grant TokenReview API permissions
  • Con
    • This results in a higher privilege to all Kubernetes

CRD to grant named service accounts access to the TokenReview API

apiVersion: rbac.authorization.k8s.io/v1beta1
kind: ClusterRoleBinding
metadata:
  name: role-tokenreview-binding
  namespace: default
roleRef:
  apiGroup: rbac.authorization.k8s.io
  kind: ClusterRole
  name: system:auth-delegator
subjects:
  - kind: ServiceAccount
    name: $SERVICE_NAME
    namespace: $NAMESPACE
  • Pro
    • Implements the least privilege per service.
  • Con
    • Administrative overhead.
    • Additional complexity.
@tomhjp
Copy link
Contributor

tomhjp commented Jul 13, 2021

Hi @gavinelder! 👋 Thanks for opening this. I think you lay out good pros and cons for each option, and the issues you link to would certainly help address some of the cons. One complementary option I'll point out as well is that if operators are looking for absolute least privilege, the system:auth-delegator ClusterRole actually slightly over-provisions permissions, as it's only the tokenreviews endpoint that's actually required:

apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
  annotations:
    rbac.authorization.kubernetes.io/autoupdate: "true"
  creationTimestamp: "2021-07-13T13:10:35Z"
  labels:
    kubernetes.io/bootstrapping: rbac-defaults
  name: system:auth-delegator
  resourceVersion: "99"
  uid: c0f4a4ca-4cac-47f6-9715-da97d6f0cc9f
rules:
- apiGroups:
  - authentication.k8s.io
  resources:
  - tokenreviews
  verbs:
  - create
- apiGroups:
  - authorization.k8s.io
  resources:
  - subjectaccessreviews
  verbs:
  - create

If we implemented the suggestions in #68 and #95, I think option 1 would be the clear winner right?

@gavinelder
Copy link
Author

gavinelder commented Jul 13, 2021

@tomhjp thanks for getting back to me on this one.

With regards to the RBAC policy provided this is a perfect example of least privilege and something which should be highlighted in the Vault product documentation over the existing example.

With regards to #68, I feel this is a really good suggestion and should be implemented to support improved behavior with the Terraform-Vault-Provider and resolve known pitfalls with the vault_kubernetes_auth_backend_config where an import of this resource was completed however the state has no knowledge of whether or not the token_reviewer_jwt is set and there is a potential that a subsequent application may delete this option resulting in a cluster being unable to authenticate.

With regards to #95 I think this is nice but unsure of the actual tangible benefits of doing this given that this plugin already uses the service account token where a shared service account is not configured. (I feel this leads to more implementation confusion vs using the existing implementation with clear guidance around the product threat model)

I consider the above issue less about vault-plugin functionality being required but existing documentation not being clear enough to understand that the existing approaches are sufficient if understood.

If we implemented the suggestions in #68 and #95, I think option 1 would be the clear winner right?

Personally, I don't see a clear outright winner, just ensuring users are informed about approaches and letting them threat model their use case appropriately

For me, I would like to avoid Vault impersonation using a service account given it can be avoided and it makes life easy by saying raise a PR against the existing terraform to allow the CI pipeline to trust your cluster without having to provide a secret, but that is in my risk appetite other users may not be.

@tomhjp
Copy link
Contributor

tomhjp commented Nov 29, 2021

There's an in-progress PR here which hopefully clears up the options we have today. Feel free to leave some comments if you have any!

@tomhjp
Copy link
Contributor

tomhjp commented Jan 5, 2022

Going to close this out now the mentioned PR is merged and the website updated here and here. Once #122 is merged and released, I think that's going to be the recommended option when Vault is on Kubernetes, so there will be a couple more updates to the docs at that point. Thanks for raising this for discussion!

@tomhjp tomhjp closed this as completed Jan 5, 2022
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

No branches or pull requests

2 participants