-
Notifications
You must be signed in to change notification settings - Fork 41.5k
Add proposal for kubelet TLS bootstrap #20439
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
Add proposal for kubelet TLS bootstrap #20439
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
Can one of the admins verify that this patch is reasonable to test? (reply "ok to test", or if you trust the user, reply "add to whitelist") If this message is too spammy, please complain to ixdy. |
1 similar comment
Can one of the admins verify that this patch is reasonable to test? (reply "ok to test", or if you trust the user, reply "add to whitelist") If this message is too spammy, please complain to ixdy. |
ec72389
to
5340ec8
Compare
CLAs look good, thanks! |
What defense does this API need against abuse? |
The abuse concerns I've considered are 1) DoS by making excessive requests and 2) unauthorized certificate issuance. We mitigate the first by restricting the endpoint itself with token auth (on the assumption that nodes you have provisioned are unlikely to DoS you) and the second by requiring manual intervention to approve CSRs. In the future, we'd like to implement more robust approval policies (e.g. involving ACME handshakes or TPM attestation) on top of this mechanism. |
Can one of the admins verify that this patch is reasonable to test? (reply "ok to test", or if you trust the user, reply "add to whitelist") If this message is too spammy, please complain to ixdy. |
1 similar comment
Can one of the admins verify that this patch is reasonable to test? (reply "ok to test", or if you trust the user, reply "add to whitelist") If this message is too spammy, please complain to ixdy. |
These should not change often and are thus simple to include in a static | ||
provisioning script. | ||
|
||
## API Changes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@erictune what would be a good API group to add this too? Will we have an authentication api group eventually to house user and group objects? That seems like a reasonable place to put these types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is more pki/ca-related than auth
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have it under extensions
for now. Will move it to a more appropriate location when we can figure out where that is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have it under extensions for now. Will move it to a more appropriate location when we can figure out where that is.
No more under extensions please. We can argue over the exact name in the pull or here, but I don't want any more "temporary" resources under extensions.
@erictune I assigned to you to do an initial review since it's auth related. |
This is a little left field, but have we considered perhaps using/adapting the ACME protocol used by Let's Encrypt? https://tools.ietf.org/html/draft-ietf-acme-acme-01 It is very similar to the flow in the doc but with some important differences:
A lot of the block and tackle for LetsEncrypt is written in go already -- https://github.com/letsencrypt/boulder. While we probably won't want to use this stuff verbatim, it may be possible to break out parts of the implementation and share (etcd raft-like). My motivation: I'd love to see an extensible identity system that is "dialtone" in clusters not just for the cluster system itself but for applications. Basically, we'll have the problem of "microservice A calls microservice B. How does B know it is actually A that is calling". This is obviously out of scope for k8s but it would be great to be compatible and share code. One big disadvantage here: This doesn't reuse the API patterns used through the rest of Kubernetes. I think this is probably a viable trade off as we wouldn't be rolling our own security related APIs. |
administrator to approve or reject. The apiserver should watch for updates the | ||
Status field of any CertificateSigningRequest. When a CSR is approved | ||
(signified by Status changing from Unknown to True) the apiserver should | ||
generate and sign the certificate, then update the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd recommend adding a /approve subresource for this-- we don't ordinarily do a bunch of computation when you write to an object.
A /reject subresource isn't necessary for this, since in that case the apiserver is just filling in fields, as in an ordinary update.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sgtm. I'm not terribly familiar with the API conventions yet and this still struck me as a rough edge.
I think it's in scope for the ecosystem to build up something capable
of allowing that. Agree it may not be Kube core, but it's pretty
fundamental.
|
@smarterclayton -- Wrt app->app authn, I'm thinking that something as part of the CNCF might make sense. I'm going to write a design doc over the next couple of weeks and share it around. Stay tuned. |
@mikedanese How about doing the conditions like this? |
// human readable message with details about the request state | ||
Message string `json:"message,omitempty"` | ||
// If request was approved, the controller will place the issued certificate here. | ||
Certificate []byte `json:"certificate,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this filed should be moved to the body of status.
The api needs refining but this is going to be an alpha api for 1.3 so there will be significant api reviews later. LGTM'ing to unblock progress on an implementation. |
GCE e2e build/test passed for commit 5a65bb0. |
Automatic merge from submit-queue |
The ability to post CSRs to the signing endpoint should be controlled. As a | ||
simple solution we propose that each node be provisioned with an auth token | ||
(possibly static across the cluster) that is scoped via ABAC to only allow | ||
access to the CSR endpoint. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't see a discussion (or missed it in a previous version) but can I watch the list and steal someone else's certificate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neither the csr nor the cert contains the private key of the node. A cert just holds the public key half, and isn't a secret (it's presented by the node in every TLS handshake)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, right.
On Tue, May 3, 2016 at 11:19 PM, Jordan Liggitt notifications@github.com
wrote:
In docs/proposals/kubelet-tls-bootstrap.md
#20439 (comment)
:+1. Approved if the controller should issue the cert
+2. Denied if the controller should not issue the cert
+
+The suggested command for listing iskubectl get csrs
. The approve/deny
+interactions can be accomplished with normal updates, but would be more
+conveniently accessed by direct subresource updates. We leave this for future
+updates to kubectl.
+
+## Security Considerations
+
+### Endpoint Access Control
+
+The ability to post CSRs to the signing endpoint should be controlled. As a
+simple solution we propose that each node be provisioned with an auth token
+(possibly static across the cluster) that is scoped via ABAC to only allow
+access to the CSR endpoint.Neither the csr nor the cert contains the private key of the node. A cert
just holds the public key half, and isn't a secret (it's presented by the
node in every TLS handshake)—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/kubernetes/kubernetes/pull/20439/files/5a65bb0044d9ac3fc3e7585d09cb6bb33e9c3b2c#r61986759
Automatic merge from submit-queue TLS bootstrap API group (alpha) This PR only covers the new types and related client/storage code- the vast majority of the line count is codegen. The implementation differs slightly from the current proposal document based on discussions in design thread (#20439). The controller logic and kubelet support mentioned in the proposal are forthcoming in separate requests. I submit that #18762 ("Creating a new API group is really hard") is, if anything, understating it. I've tried to structure the commits to illustrate the process. @mikedanese @erictune @smarterclayton @deads2k ```release-note-experimental An alpha implementation of the the TLS bootstrap API described in docs/proposals/kubelet-tls-bootstrap.md. ``` []()
@gtank Are there any plans to extend the CSR api proposed here, to support service->service mutual tls auth? something like |
We've had a fair number of discussions about how client certs /
serving certs might be generated. I personally prefer approaches
where the infrastructure can sign for services to have certs injected,
but that does not directly solve the client problem. I do like the
ideas endorsed in your link, of being able to request signing.
|
Automatic merge from submit-queue Add proposal for kubelet TLS bootstrap A proposal based on the discussion of issue kubernetes#18112, to implement a process by which kubelets can obtain TLS certificates in a streamlined manner.
UPSTREAM: 66397: Fix upper limit on m5/c5 instance typesn Origin-commit: 2846f0cb52d65645bb3e5d277f4f0af44aa32156
UPSTREAM: 66397: Fix upper limit on m5/c5 instance typesn Origin-commit: 2846f0cb52d65645bb3e5d277f4f0af44aa32156
A proposal based on the discussion of issue #18112, to implement a process by which kubelets can obtain TLS certificates in a streamlined manner.