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

User's RBAC violations during volume provisioning #213

Closed
venkatsc opened this issue Jan 10, 2019 · 9 comments
Closed

User's RBAC violations during volume provisioning #213

venkatsc opened this issue Jan 10, 2019 · 9 comments
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.

Comments

@venkatsc
Copy link

venkatsc commented Jan 10, 2019

/kind bug
What happened?

Configure two users - UserA and UserB service accounts restricting both to their own namespaces and additional ClusterRole with access to StorageClasses, PersistentVoluems.

Scenario 1

  1. Create StorageClass with UserA's secret (stored in their namespace) with any CSI external provisioner plugin (tested with Quobyte CSI plugin).
  2. Create PVC in UserB's namespace using UserA's storageclass (PVC should trigger dynamic provisioning).
  3. UserB is able to access UserA's secret and can create volume.

Scenario 2

  1. Remove ClusterRole RBAC of UserB that gives access to PV and StorageClass
  2. Create PVC in UserB's namespace using UserA's storageclass (PVC should trigger dynamic provisioning).
  3. UserB is able to access UserA's secret and can create volume.

Expected:
UserB should not be able create volumes as User A

Note on mounting:
UserB may not be able to mount the volume given there is authentication during the mounting phase (as this part of CSI code is inside kubernetes and RBAC prevents the reading of UserA's secret by UserB). But the issue of volume creation by UserB using UserA's secret is a deviation from the expected behaviour of RBAC.

Why it happened?
CSI provisioner is deployed with a service account that can access all the secrets and storage classes. When a PVC is created in the Kubernetes by UserB, the RBAC only checked whether this can create PVC but neither access to the StorageClass nor the Secrets included in the StorageClass referred by PVC.

When CSI Provisioner started provisioning volume outside of Kubernetes for the triggered PVC, it does not have any user information and used its own service account with elevated privileges to get the secret of UserA.

Possible fixes:

  1. An admission plugin that verifies access to StorageClass and access to CSI secrets during PVC creation time. But it is not sufficient for RBAC revocation, as there is no check after once PVC is created.

  2. Use impersonation API and from CSI and check if the current user can access resources before accessing it with CSI's service account. But, currently, Kubernetes resources does not contain any user information once they are admitted.

@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Jan 10, 2019
@venkatsc venkatsc changed the title User's RBAC voilations during volume provisioning User's RBAC violations during volume provisioning Jan 10, 2019
@msau42
Copy link
Collaborator

msau42 commented Jan 10, 2019

Hi @venkatsc, StorageClass is a cluster-level resource and is meant to be created and managed by a cluster admin, not by a particular user/namespace. So the provision secret referenced in the StorageClass is designed to be used cluster-wide. That being said, I think the kubernets-csi documentation could be more clear in this regard. cc @saad-ali

As a workaround, you can restrict a StorageClass to specific namespaces by setting StorageClass quota.

For other CSI secrets, we do support secret namespace templating using the pvc.namespace (and other parameters), but not for provisioning. From discussing with @liggitt, the main reasoning was that during volume delete, we would not have the PVC object anymore so we could not resolve the template at that time. But for storage systems where secrets are not needed for volume delete, we could potentially relax the templating rules and allow pvc.namespace on create. If this is something that your storage system can support, then we can consider adding this as a new feature.

@venkatsc
Copy link
Author

@msau42 Thanks for looking into it.

The issue is more of namespace violation of secrets due to CSI external provisioner having different RBAC rules (kind of superuser privileges for accessing secrets and storage classes). This issue is prevented only if the user cannot create PVC which IMO severely amputees the usability.

As explained in the Scenario 2, a user without access to StorageClass can access the storage class by simply knowing the StorageClass name, exploiting the privileges of the service account used to deploy CSI-external provisioner.

Let us say, CSI external provisioner is deployed with the service account CSI-provisioner. The Scenario 2 happens as follows:

  1. UserB is given permission to create PVC in namespace ns-B (without access to StorageClass or PV resources) Scope: inside Kubernetes, user: UserB
  2. Upon PVC creation by UserB, Kubernetes checks the RBAC and sees the PVC create permission on his RBAC rules and admits the PVC without checking access to StorageClass and CSI secrets included in the StorageClass - this can be exploited to include any StorageClass (Kubernetes not associating any user information once the resource passes the RBAC checks). Scope: inside Kubernetes, user: UserB
  3. External provisioner running as CSI-provisioner sees the claim creation (currently, at this point, PVC does not have any user information associated with it) and starts provisioning volumes as CSI-provisioner (this service account has access to all secrets). Scope: outside Kubernetes, user: CSI-provisioner
  4. As the CSI's mounting is inside Kubernetes, during mount calls, Kubernetes checks the RBAC of UserB and fails to access UserA's credentias from UserA's name space ns-A. Scope: inside Kubernetes, user: UserB

The issue is due to that brief service account switching from Step 2 to Step 3 of the volume provisioning without real user's RBAC capability check.

As can be seen, this permission escalation should be part of all the external plugins that deploy their own RBACs (as these override users RBAC rules). IMO, these can be fixed by associating user information with resource and validating capabilities as explained in the Kubernetes impersonation API.

On PVC not being available during delete, Can owner references be used to prevent deletion before volume deletion completion in the storage vendor system?

@msau42
Copy link
Collaborator

msau42 commented Jan 10, 2019

Hi @venkatsc, I think there's some confusion around the scope of the provisioning secrets specified in a StorageClass. Per-user provisioning secrets are currently not supported by the StorageClass API unless you pair it with StorageClass quota restricting that specific StorageClass to a particular namespace. Secrets used by the provisioner are intended to be created by the cluster administrator with cluster-wide scope, in namespaces that may be independent from a user's namespace.

We can look into supporting per-user namespace provision secrets, but it would have the limitation that it could not be used for volume deletion.

Regarding volume deletion, the deletion of the PVC object is what triggers the volume deletion call to be made for the PV.

@venkatsc
Copy link
Author

Hi @msau42
Could you point me to any references on PVC namespace templating?

"We can look into supporting per-user namespace provision secrets, but it would have the limitation that it could not be used for volume deletion." -- that would be great.

@msau42
Copy link
Collaborator

msau42 commented Jan 15, 2019

The documentation here talks about pvc namespace templating for ControllerPublish and NodePublish secrets. Similarly, we could investigate doing something similar for Provision secrets (with some limitations)

@venkatsc
Copy link
Author

Thank you @msau42 for pointing me to the relevant documentation.

I will raise a new feature request for per-user provisioning secrets.

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 22, 2019
@msau42
Copy link
Collaborator

msau42 commented Jun 20, 2019

Per PVC secret templating was added in #274
/close

@k8s-ci-robot
Copy link
Contributor

@msau42: Closing this issue.

In response to this:

Per PVC secret templating was added in #274
/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.

kbsonlong pushed a commit to kbsonlong/external-provisioner that referenced this issue Dec 29, 2023
…tion-removal

Removed dependency on node annotation in favor of CSINode resource
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. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.
Projects
None yet
Development

No branches or pull requests

4 participants