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

Users Can't Delegate CSR Approval/Signing Permissions Within A Domain #122154

Open
stevekuznetsov opened this issue Dec 1, 2023 · 16 comments · May be fixed by kubernetes/website#49275
Open

Users Can't Delegate CSR Approval/Signing Permissions Within A Domain #122154

stevekuznetsov opened this issue Dec 1, 2023 · 16 comments · May be fixed by kubernetes/website#49275
Labels
needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. sig/auth Categorizes an issue or PR as relevant to SIG Auth.

Comments

@stevekuznetsov
Copy link
Contributor

What happened?

If a user has permission to approve or sign CSRs for a whole domain, they cannot delegate some part of that domain to another role.

What did you expect to happen?

The escalation check on RBAC creation should not erroneously tell you that you can't escalate, when you are not escalating.

How can we reproduce it (as minimally and precisely as possible)?

$ cat /tmp/setup.yaml
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
  name: uber-csr-approver
rules:
- apiGroups:
  - rbac.authorization.k8s.io
  resources:
  - clusterroles
  verbs:
  - create
- apiGroups:
  - certificates.k8s.io
  resourceNames:
  - example.com/*
  resources:
  - signers
  verbs:
  - approve
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
  name: delegator-uber-csr-approver
roleRef:
  apiGroup: rbac.authorization.k8s.io
  kind: ClusterRole
  name: uber-csr-approver
subjects:
- kind: ServiceAccount
  name: delegator
  namespace: csr-test
---
apiVersion: v1
kind: Namespace
metadata:
  name: csr-test
---
apiVersion: v1
kind: ServiceAccount
metadata:
  name: delegator
  namespace: csr-test

$ kubectl apply -f /tmp/setup.yaml
clusterrole.rbac.authorization.k8s.io/uber-csr-approver created
clusterrolebinding.rbac.authorization.k8s.io/delegator-uber-csr-approver created
namespace/csr-test created
serviceaccount/delegator created

$ cat /tmp/delegate.yaml
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
  name: specific-csr-approver
rules:
- apiGroups:
  - certificates.k8s.io
  resourceNames:
  - example.com/specific
  resources:
  - signers
  verbs:
  - approve

$ kubectl --as system:serviceaccount:csr-test:delegator apply -f /tmp/delegate.yaml
Error from server (Forbidden): error when creating "/tmp/csr.yaml": clusterroles.rbac.authorization.k8s.io "specific-csr-approver" is forbidden: user "system:serviceaccount:csr-test:delegator" (groups=["system:serviceaccounts" "system:serviceaccounts:csr-test" "system:authenticated"]) is attempting to grant RBAC permissions not currently held:
{APIGroups:["certificates.k8s.io"], Resources:["signers"], ResourceNames:["example.com/specific"], Verbs:["approve"]}

Anything else we need to know?

No response

Kubernetes version

$ kubectl version
WARNING: This version information is deprecated and will be replaced with the output from kubectl version --short.  Use --output=yaml|json to get the full version.
Client Version: version.Info{Major:"1", Minor:"26", GitVersion:"v1.26.1", GitCommit:"17b7accf8", GitTreeState:"clean", BuildDate:"2023-08-11T12:23:21Z", GoVersion:"go1.19.10", Compiler:"gc", Platform:"linux/amd64"}
Kustomize Version: v4.5.7
Server Version: version.Info{Major:"1", Minor:"26", GitVersion:"v1.26.9+aa37255", GitCommit:"e782f8ba0e57d260867ea108b671c94844780ef2", GitTreeState:"clean", BuildDate:"2023-11-21T19:15:07Z", GoVersion:"go1.19.13 X:strictfipsruntime", Compiler:"gc", Platform:"linux/amd64"}

Cloud provider

N/A

OS version

N/A

Install tools

N/A

Container runtime (CRI) and version (if applicable)

N/A

Related plugins (CNI, CSI, ...) and versions (if applicable)

N/A
@stevekuznetsov stevekuznetsov added the kind/bug Categorizes issue or PR as related to a bug. label Dec 1, 2023
@k8s-ci-robot k8s-ci-robot added needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Dec 1, 2023
@stevekuznetsov stevekuznetsov changed the title Users Can't Delegate CSR Approval/Signing Permissions Users Can't Delegate CSR Approval/Signing Permissions Within A Domain Dec 1, 2023
@enj
Copy link
Member

enj commented Dec 1, 2023

/triage accepted
/sig auth
cc @liggitt @deads2k @munnerz

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. sig/auth Categorizes an issue or PR as relevant to SIG Auth. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Dec 1, 2023
@k8s-ci-robot k8s-ci-robot removed the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Dec 1, 2023
@github-project-automation github-project-automation bot moved this to Needs Triage in SIG Auth Dec 1, 2023
@enj enj moved this from Needs Triage to Backlog in SIG Auth Dec 1, 2023
@liggitt
Copy link
Member

liggitt commented Dec 1, 2023

resourceNames don't support partial globs, so example.com/* is a different permission than example.com/foo, and is considered escalating

@liggitt
Copy link
Member

liggitt commented Dec 1, 2023

/remove-kind bug

@k8s-ci-robot k8s-ci-robot added needs-kind Indicates a PR lacks a `kind/foo` label and requires one. and removed kind/bug Categorizes issue or PR as related to a bug. labels Dec 1, 2023
@stevekuznetsov
Copy link
Contributor Author

I understand that, but since the authorizer for CSRs allows example.foo/* to approve for all signers under that domain, as a user I expect that to work...

@stevekuznetsov
Copy link
Contributor Author

As in, if the system:serviceaccount:csr-test:delegator service account were to attempt approving the CSR for example.foo/specific, it would be allowed - it is authorized. That's why it's a bug that the escalation check thinks that it's not authorized.

@liggitt
Copy link
Member

liggitt commented Dec 1, 2023

The CSR admission plugin does two subject access review checks to check for the wildcard permission and the specific permission.

example.com/* covering example.com/specific is a semantic specific to the CSR admission plugin, it is not part of the authorization API or RBAC.

@deads2k
Copy link
Contributor

deads2k commented Dec 1, 2023

Explaining for people wondering why, "can't you just make * glob for name?". Because our authorizer doesn't interpret names or require referenced resources to exist, it's entirely possible that some entity is actually using "" for "something/" for a meaning we don't control. The only fully safe way to make a change is introduce a new field.

@stevekuznetsov
Copy link
Contributor Author

I think the point is less that the generic RBAC implementation of escalation detection should be different - or that it should honor any generic admission plugin or any custom verb, etc, and that if we choose to ship Kubernetes with this custom logic around CSRs, it might be beneficial if that logic were more fully supported / more self-consistent and had fewer rough edges.

@stevekuznetsov
Copy link
Contributor Author

However, I know better than to try convince you two that "just this one time" we need a one-off codepath to support something ;)

Consider the issue a documentation of the limitation.

@stevekuznetsov
Copy link
Contributor Author

After some more conversation @deads2k showed me every way that an actor with no signer name restriction on CSR approval could abuse the system to get CSR creation rights (given that the actor has other privileges), since those are not super locked down, and then they'd be able to create and self-approve a CSR for system:master credentials. @liggitt maybe you'd be open to a feature request for something in this space? Seems like we should be able to do better here.

@enj
Copy link
Member

enj commented Dec 2, 2023

While we try to figure out a better solution in this space, you could try restricting what such an actor could do via a CEL admission policy. It is not that different from letting users create pods but then preventing them from creating privileged pods.

@sftim
Copy link
Contributor

sftim commented Dec 7, 2023

How we might solve this: CEL authz where one rule definition covers both authz and the admission stage. Not for the faint hearted, I think.

@k8s-triage-robot
Copy link

This issue has not been updated in over 1 year, and should be re-triaged.

You can:

  • Confirm that this issue is still relevant with /triage accepted (org members only)
  • Close this issue with /close

For more details on the triage process, see https://www.kubernetes.dev/docs/guide/issue-triage/

/remove-triage accepted

@k8s-ci-robot k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. and removed triage/accepted Indicates an issue or PR is ready to be actively worked on. labels Dec 6, 2024
@sftim
Copy link
Contributor

sftim commented Dec 15, 2024

If you want to either:

  • document how to do this (eg ValidatingAdmissionPolicy?)
  • file an issue asking that somebody document how to do this (eg ValidatingAdmissionPolicy?)
  • propose a blog article about how to delegate specific CSR approval / signing permissions

then any of those would be champion, @stevekuznetsov

@stevekuznetsov
Copy link
Contributor Author

@sftim would you want to see something more broadly scoped than a blog that answers the question of "how do you give some identity the ability to delegate CSR approval and signing using VAP"? I can certainly write something like that.

@sftim
Copy link
Contributor

sftim commented Dec 17, 2024

@stevekuznetsov that sounds ideal

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. sig/auth Categorizes an issue or PR as relevant to SIG Auth.
Projects
Status: Backlog
Development

Successfully merging a pull request may close this issue.

7 participants