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.

Sign up for GitHub

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

Jump to bottom

Add ClusterRoles for CSR approval modes to RBAC bootstrappolicy #48191

Closed
luxas opened this issue Jun 28, 2017 · 9 comments · Fixed by #49284
Closed

Add ClusterRoles for CSR approval modes to RBAC bootstrappolicy #48191

luxas opened this issue Jun 28, 2017 · 9 comments · Fixed by #49284
Assignees
@mikedanese @luxas
Labels
kind/feature Categorizes issue or PR as related to a new feature. sig/auth Categorizes an issue or PR as relevant to SIG Auth.

Comments

@luxas
Copy link
Member

luxas commented Jun 28, 2017

/kind feature

From: kubernetes/website#4208

# A ClusterRole which instructs the CSR approver to approve a user requesting
# node client credentials.
kind: ClusterRole
apiVersion: rbac.authorization.k8s.io/v1beta1
metadata:
  name: system:csr-approver:approve-node-client-csr
rules:
- apiGroups: ["certificates.k8s.io"]
  resources: ["certificatesigningrequests/nodeclient"]
  verbs: ["create"]
---
# A ClusterRole which instructs the CSR approver to approve a node renewing its
# own client credentials.
kind: ClusterRole
apiVersion: rbac.authorization.k8s.io/v1beta1
metadata:
  name: system:csr-approver:approve-node-client-renewal-csr
rules:
- apiGroups: ["certificates.k8s.io"]
  resources: ["certificatesigningrequests/selfnodeclient"]
  verbs: ["create"]
---
# A ClusterRole which instructs the CSR approver to approve a node requesting a
# serving cert matching its client cert.
kind: ClusterRole
apiVersion: rbac.authorization.k8s.io/v1beta1
metadata:
  name: system:csr-approver:approve-node-server-renewal-csr
rules:
- apiGroups: ["certificates.k8s.io"]
  resources: ["certificatesigningrequests/selfnodeserver"]
  verbs: ["create"]

Would it make sense to add these rules to the bootstrappolicy? To me they seem generic enough, and they wouldn't bind to something. Consumers can do the binding to their subjects as they like.

I'm asking because it's already duplicated twice for two early adopters:
https://github.com/kubernetes/kubernetes/blob/master/cluster/addons/rbac/kubelet-certificate-management.yaml
https://github.com/kubernetes/kubernetes/blob/master/cmd/kubeadm/app/phases/apiconfig/clusterroles.go

I don't think those ClusterRoles are specific to kubeadm and GKE at all; but the ClusterRoleBindings will surely be, so therefore we'd create the roles but not the bindings.

@kubernetes/sig-auth-feature-requests
@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. sig/auth Categorizes an issue or PR as relevant to SIG Auth. labels Jun 28, 2017
@deads2k
Copy link
Contributor

deads2k commented Jun 28, 2017

Wouldn't we expect and encourage people using the CSR capability for their nodes to use node identities that match the node authorizer, in which case we won't need the client or server renewal CSR roles?

@liggitt
Copy link
Member

liggitt commented Jun 28, 2017

The node authorizer doesn't grant these, since otherwise you could never prevent a compromised node from renewing its credentials

@deads2k
Copy link
Contributor

deads2k commented Jun 28, 2017

The node authorizer doesn't grant these, since otherwise you could never prevent a compromised node from renewing its credentials

Realistically, with these roles you still wouldn't. People would grant them to the node group, not each individual node, but I guess that would be a tradeoff for the deployment mechanism.

@liggitt
Copy link
Member

liggitt commented Jun 28, 2017

You could individually revoke this role from the group and stop renewals without disrupting regular operation

@luxas
Copy link
Member Author

luxas commented Jun 28, 2017

Realistically, with these roles you still wouldn't. People would grant them to the node group, not each individual node, but I guess that would be a tradeoff for the deployment mechanism.

I can definitely see myself revoking a binding from system:nodes to multiple specific system:node:{node} bindings in case of a compromise.

Anyway, this issue is more "should we create these by default or not".
I've already mentioned my opinion.

@deads2k
Copy link
Contributor

deads2k commented Jun 28, 2017

Creating the roles seems reasonable. I don't think I want bindings. I think the binding should be up the particular deployer.

@luxas
Copy link
Member Author

luxas commented Jun 28, 2017

Perfect, that's exactly my proposal.

I'll go ahead and make this change...

@luxas luxas self-assigned this Jun 28, 2017
@jcbsmpsn
Copy link
Contributor

jcbsmpsn commented Jul 5, 2017

/assign @mikedanese

@luxas
Copy link
Member Author

luxas commented Jul 14, 2017

@mikedanese are you working on this or should I send a PR?

@luxas luxas mentioned this issue Jul 20, 2017
Merged
k8s-github-robot pushed a commit that referenced this issue Aug 1, 2017
Merge pull request #49284 from luxas/csr_cluster_roles
2c521d7 
Automatic merge from submit-queue (batch tested with PRs 49284, 49555, 47639, 49526, 49724)

Add clusterroles for approving CSRs easily

**What this PR does / why we need it**:

Adds ClusterRoles for CSR approving. Currently consumers like kubeadm and GKE have to create these rules by themselves, but are doing it slightly differently which leads to sprawl. Instead, the ClusterRoles are created by core, and the actual bindings created by respective deployment method.

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #

fixes #48191

**Special notes for your reviewer**:

**Release note**:

```release-note
The API Server now automatically creates RBAC ClusterRoles for CSR approving. 
Each deployment method should bind users/groups to the ClusterRoles if they are using this feature.
```
cc @kubernetes/sig-auth-pr-reviews @kubernetes/sig-cluster-lifecycle-pr-reviews
This was referenced Aug 8, 2017
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mikedanese mikedanese

@luxas luxas

Labels
kind/feature Categorizes issue or PR as related to a new feature. sig/auth Categorizes an issue or PR as relevant to SIG Auth.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add clusterroles for approving CSRs easily luxas/kubernetes
6 participants
@liggitt @mikedanese @luxas @deads2k @k8s-ci-robot @jcbsmpsn