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

move group approver to use a subject access review #45030

Closed
mikedanese opened this issue Apr 27, 2017 · 12 comments
Closed

move group approver to use a subject access review #45030

mikedanese opened this issue Apr 27, 2017 · 12 comments
Assignees
Labels
area/controller-manager area/security sig/auth Categorizes an issue or PR as relevant to SIG Auth.

Comments

@mikedanese
Copy link
Member

Right now the group approver is configured with a static group name to allow provisioning of kubelet client certificates. The group approver should move to using a subject access review. We need 3 verbs to auto approve the full kubelet flow:

  1. csr/nodeclientcert which if granted indicates the group approver should approve any kubelet client shaped certificate requested.
  2. csr/selfnodeclientcert which if granted indicates the group approver should approve a kubelet client shaped certificate requested if the username of the csr creating user matches the subject of the csr.
  3. csr/selfnodeservercert which if granted indicates the group approver should approve a kubelet server shaped certificate requested if the username of the csr creating user matches the subject of the csr.

Names of the verbs are a rough draft. Existence of 1 allows for a preshared cluster wide bootstrap identity that can be rotated and invalidated. Existence of 2 allows a kubelet to rotate it's own client credentials. Existence of 2 allows a kubelet to request and rotate it's own server certificate.

I'm capturing a slack conversation with @liggitt so let me know if i got something wrong.

@mikedanese mikedanese added the sig/auth Categorizes an issue or PR as relevant to SIG Auth. label Apr 27, 2017
@mikedanese
Copy link
Member Author

@jcbsmpsn @pipejakob

@luxas
Copy link
Member

luxas commented May 10, 2017

@mikedanese This requires RBAC being enabled, and the right Roles/RoleBindings should be created, right?
In order to make the groupapprover approve csrs from an individual user, should a RoleBinding be created?

@mikedanese @jcbsmpsn Could you provide a short list of the implications/work items for kubeadm so we can make sure we'll get that into kubeadm v1.7?

@liggitt
Copy link
Member

liggitt commented May 10, 2017

It does not require RBAC, subject access reviews will use whatever authorizer is configured.

If you are using RBAC, I would expect a role for each of the individual permissions, which could be bound to bootstrap users, individual node users, or the entire node group.

@mikedanese
Copy link
Member Author

We will need to add an rbac role for rbac enabled kubeadm. The change should be small.

@luxas
Copy link
Member

luxas commented May 10, 2017

So the RBAC role would look something like this then?

apiVersion: rbac.authorization.k8s.io/v1beta1
kind: ClusterRoleBinding
metadata:
  name: approve-node-csrs-for-<token-id>
roleRef:
  apiGroup: rbac.authorization.k8s.io
  kind: ClusterRole
  name: system:csrapproval:nodeclientcert
subjects:
- kind: User
  name: system:bootstrap:<token-id>
---
apiVersion: rbac.authorization.k8s.io/v1beta1
kind: ClusterRole
metadata:
  name: system:csrapproval:nodeclientcert
rules:
- apiGroups:
  - authorization.k8s.io/v1
  resources:
  - subjectaccessreviews
  resourceName:
  - csr/nodeclientcert
  verbs:
  - create

I'm confused whether csr/nodeclientcert will be the resource, subresource or just in .spec.resourceAttributes of the SAR.

I guess we want to lock things down on a token (user)-level, to only let the generated-by-default-token do the TLS bootstrap and nobody else.

@liggitt
Copy link
Member

liggitt commented May 10, 2017

Probably rules something more like this (where the different categories manifest as subresources of the certificatesigningrequests resource):

In a role bound to the bootstrapper:

- apiGroups: ["certificates.k8s.io"]
  resources: ["certificatesigningrequests/nodeclient"]
  verbs: ["create"]

In a role to enable kubelets to rotate their own client certs (would need more thought about how to cut off a compromised node):

- apiGroups: ["certificates.k8s.io"]
  resources: ["certificatesigningrequests/selfnodeclient"]
  verbs: ["create"]

In a role to enable kubelets to bootstrap/rotate their serving certs (would need more thought about how to verify the CN/SANs in the cert belong to that node, and how to cut off a compromised node):

- apiGroups: ["certificates.k8s.io"]
  resources: ["certificatesigningrequests/selfnodeserver"]
  verbs: ["create"]

@ericchiang
Copy link
Contributor

ericchiang commented May 10, 2017

Are the last two roles bound to the bootstrapper? I'd imagine they'd be tied to the node themselves, which would give a good way to revoke follow-up access.

apiVersion: rbac.authorization.k8s.io/v1beta1
kind: ClusterRoleBinding
metadata:
  name: approve-node-client-renewal-for-<node-id>
roleRef:
  apiGroup: rbac.authorization.k8s.io
  kind: ClusterRole
  name: system:csrapproval:selfnodeclient
subjects:
- kind: User
  name: system:node:<node-id>
---
apiVersion: rbac.authorization.k8s.io/v1beta1
kind: ClusterRole
metadata:
  name: system:csrapproval:selfnodeclient
rules:
- apiGroups: ["certificates.k8s.io"]
  resources: ["certificatesigningrequests/selfnodeclient"]
  verbs: ["create"]

@luxas
Copy link
Member

luxas commented May 10, 2017

Ah that makes sense @liggitt, thanks!
Seems like I misinterpreted the usage of the SAR, but with your example I see how it fits together.

This looks really good then, I think it should be prioritized for v1.7 if possible (I really want to get rid of the
--insecure-experimental-approve-all-node-csrs-for-group flag)
@ericchiang also volunteered to pick this up in case @jcbsmpsn doesn't have time at this particular moment.

@luxas
Copy link
Member

luxas commented May 10, 2017

@ericchiang Yes, we have that option as well.
a) bind to system:nodes (hard to revoke on compromised node)
b) bind to system:node:<nodename> (who should create those RBAC bindings?)

@mikedanese mikedanese assigned mikedanese and unassigned jcbsmpsn May 10, 2017
@mikedanese
Copy link
Member Author

I've already started working on this as part of my certificate controller refactoring.

@luxas
Copy link
Member

luxas commented May 10, 2017

Thank you @mikedanese! Ping me for review please ;)

k8s-github-robot pushed a commit that referenced this issue May 31, 2017
Automatic merge from submit-queue (batch tested with PRs 46635, 45619, 46637, 45059, 46415)

Certificate rotation for kubelet server certs.

Replaces the current kubelet server side self signed certs with certs signed by
the Certificate Request Signing API on the API server. Also renews expiring
kubelet server certs as expiration approaches.

Two Points:
1. With `--feature-gates=RotateKubeletServerCertificate=true` set, the kubelet will
    request a certificate during the boot cycle and pause waiting for the request to
    be satisfied.
2. In order to have the kubelet's certificate signing request auto approved,
    `--insecure-experimental-approve-all-kubelet-csrs-for-group=` must be set on
    the cluster controller manager. There is an improved mechanism for auto
    approval [proposed](#45030).

**Release note**:
```release-note
With `--feature-gates=RotateKubeletServerCertificate=true` set, the kubelet will
request a server certificate from the API server during the boot cycle and pause
waiting for the request to be satisfied. It will continually refresh the certificate as
the certificates expiration approaches.
```
@ericchiang
Copy link
Contributor

This was done in #45619.

Can we keep this issue open until we get some docs merge?

ericchiang added a commit to ericchiang/website that referenced this issue Jun 27, 2017
This includes documenting the new CSR approver built into the
controller manager and the kubelet alpha features for certifiate
rotation.

ref:

- kubernetes/kubernetes#45030
- kubernetes/enhancements#266
- kubernetes/enhancements#267
ericchiang added a commit to ericchiang/website that referenced this issue Jun 27, 2017
This includes documenting the new CSR approver built into the
controller manager and the kubelet alpha features for certifiate
rotation.

Since the CSR approver changed over the 1.7 release cycle we need
to call out the migration steps for those using the alpha feature.
This document as a whole could probably use some updates, but the
main focus of this PR is just to get these features minimally
documented before the release.

ref:

- kubernetes/kubernetes#45030
- kubernetes/enhancements#266
- kubernetes/enhancements#267
ericchiang added a commit to ericchiang/website that referenced this issue Jun 27, 2017
This includes documenting the new CSR approver built into the
controller manager and the kubelet alpha features for certificate
rotation.

Since the CSR approver changed over the 1.7 release cycle we need
to call out the migration steps for those using the alpha feature.
This document as a whole could probably use some updates, but the
main focus of this PR is just to get these features minimally
documented before the release.

ref:

- kubernetes/kubernetes#45030
- kubernetes/enhancements#266
- kubernetes/enhancements#267
ericchiang added a commit to ericchiang/website that referenced this issue Jun 27, 2017
This includes documenting the new CSR approver built into the
controller manager and the kubelet alpha features for certificate
rotation.

Since the CSR approver changed over the 1.7 release cycle we need
to call out the migration steps for those using the alpha feature.
This document as a whole could probably use some updates, but the
main focus of this PR is just to get these features minimally
documented before the release.

ref:

- kubernetes/kubernetes#45030
- kubernetes/enhancements#266
- kubernetes/enhancements#267
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/controller-manager area/security sig/auth Categorizes an issue or PR as relevant to SIG Auth.
Projects
None yet
Development

No branches or pull requests

5 participants