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

Kubelet TLS Bootstrap #43

Closed
philips opened this issue Jul 22, 2016 · 78 comments
Closed

Kubelet TLS Bootstrap #43

philips opened this issue Jul 22, 2016 · 78 comments
Assignees
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. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/node Categorizes an issue or PR as relevant to SIG Node. stage/stable Denotes an issue tracking an enhancement targeted for Stable/GA status
Milestone

Comments

@philips
Copy link
Contributor

philips commented Jul 22, 2016

Feature Description

  • One-line feature description (can be used as a release note): kubelet generates a private key and a CSR for submission to a cluster-level certificate signing process.
  • Primary contact (assignee): @mikedanese
  • Responsible SIGs: sig-auth
  • Design proposal link (community repo): https://github.com/kubernetes/community/blob/master/contributors/design-proposals/cluster-lifecycle/kubelet-tls-bootstrap.md
  • Link to e2e and/or unit tests:
  • Reviewer(s) - (for LGTM) recommend having 2+ reviewers (at least one from code-area OWNERS file) agreed to review. Reviewers from multiple companies preferred: @liggitt
  • Approver (likely from SIG/area to which feature belongs): @liggitt
  • Feature target (which target equals to which milestone):
    • Alpha release target (1.4)
    • Beta release target (1.6)
    • Stable release target (1.12)
@philips
Copy link
Contributor Author

philips commented Jul 22, 2016

cc @kubernetes/sig-node FYI about this feature for Kubelet TLS bootstrap

@mikedanese
Copy link
Member

I think we should consider this pre-alpha until we complete:

  • kubectl integration
  • kubelet integration
  • e2e testing

Is anyone currently working on this?

@philips
Copy link
Contributor Author

philips commented Jul 22, 2016

@mikedanese @gtank should be working on all of those things to get the feature done done.

@philips philips added this to the v1.4 milestone Jul 23, 2016
@gtank
Copy link

gtank commented Jul 26, 2016

Current status of what needs to be done:

  1. kubelet needs to use the Certificates API. This will involve one new flag, provisionally --bootstrap-auth-token which will take a long random string. The kubelet uses this token to authenticate its requests to the Certificates API. Cluster-level access control should ensure that it can only be used for certificates requests. When this token exists, if the kubelet can't find TLS assets locally it will generate a fresh keypair and a Certificate Signing Request (CSR) to submit to the API. It will then watch the CSR object for the appearance of an issued certificate before proceeding with registration. It will use the certificate for client cert auth in subsequent API requests.
  2. kubectl needs to fully support CSR objects. This is currently blocked on a problem with the swagger generation and base64-encoded []byte fields. We also need to document the best way of handling the manual approval flow.
  3. Still need to write tests and update all the docs.

@philips
Copy link
Contributor Author

philips commented Aug 5, 2016

Status update:

PR for kubelet TLS bootstrap is up: kubernetes/kubernetes#30094

@mikedanese
Copy link
Member

mikedanese commented Aug 5, 2016

Kubectl support tracked in kubernetes/kubernetes#30163 with some basic support prs out

@gtank
Copy link

gtank commented Aug 8, 2016

Status update for TLS work:

In flight:

Issues:

Merged:

@liggitt
Copy link
Member

liggitt commented Aug 9, 2016

It will use the certificate for client cert auth in subsequent API requests.

The scope of kubernetes/kubernetes#20439 was around obtaining TLS serving certs. The kubelet's use of this API should probably be limited to obtaining serving certs initially. Since the kubelet already needs API credentials to submit the initial CSR, obtaining client credentials this way doesn't buy us a whole lot from a bootstrapping perspective.

@mikedanese
Copy link
Member

It allows us to bootstrap individual kubelet identities from a single cluster wide shared secret (bearer token for a kubelet-bootstrap user authorized to submit CSRs).

@liggitt
Copy link
Member

liggitt commented Aug 9, 2016

If the goal is to identify kubelets individually so we can partition node permissions (which I am in favor of), multiple identities obtained from a single shared credential aren't meaningful from an auth perspective.

If we just want to identify requests from different nodes for debugging purposes, we can do that with user-agent strings (the same way controllers set up their own client user-agent string)

@mikedanese
Copy link
Member

multiple identities obtained from a single shared credential

I'm not convinced of this. In fact I think this would work equally well if the CSR API was open (didn't require auth[n/z]). The shared secret is a mechanism that prevents a specific dos attack where a user could spam the CSR API with CSRs that would not be approved. @gtank can you weigh in on this?

@bgrant0607
Copy link
Member

cc @kubernetes/sig-cluster-lifecycle

@philips philips added the sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. label Aug 11, 2016
@philips
Copy link
Contributor Author

philips commented Aug 11, 2016

@mikedanese @liggitt can we please move the technical discussion about the design of the system to the https://groups.google.com/forum/#!forum/kubernetes-sig-node mailing list.

@gtank
Copy link

gtank commented Aug 11, 2016

@liggitt @mikedanese The nodes derive individual identities from the shared token. The token only exists to 1) filter requests meant for other clusters and 2) allow us restrict the nodes (via groups or ABAC) from accessing the rest of the API before they have client certs.

@pwittrock
Copy link
Member

@philips Is this on target for the 1.4 feature freeze this Friday (Aug 19)?

@mike-saparov
Copy link

@pwittrock: based on kubernetes/kubernetes#30094 this feature suddenly started being redesigned in flight after 5 months of work... Any suggestions on how to avoid the major redesign three days before feature freeze are welcome

@smarterclayton
Copy link
Contributor

Occasionally we will realize gaps in designed features as they are implemented. We definitely need a process that handles that - rushing features to delivery without all the proper technical due diligence being done is going to cause more issues than occasionally features missing a release. I think everyone involved is trying to find the best and most secure option for the platform here.

@mike-saparov
Copy link

@smarterclayton based on PR discussions it would be great if @liggitt and @deads2k could help with parts of the feature, wdyt? any coding / review help is appreciated to address raised concerns.

@smarterclayton
Copy link
Contributor

I believe both of them had been representing sig-auth on the security aspects here - I had not yet seen an update on the remaining issues though. That's probably appropriate on the linked issue.

@justaugustus justaugustus removed the tracked/yes Denotes an enhancement issue is actively being tracked by the Release Team label Jun 4, 2018
@justaugustus justaugustus modified the milestones: next-milestone, v1.12 Jul 2, 2018
@justaugustus
Copy link
Member

@mikedanese @kubernetes/sig-auth-feature-requests @kubernetes/sig-cluster-lifecycle-feature-requests @kubernetes/sig-node-feature-requests --

This feature was removed from the previous milestone, so we'd like to check in and see if there are any plans for this in Kubernetes 1.12.

If so, please ensure that this issue is up-to-date with ALL of the following information:

  • One-line feature description (can be used as a release note):
  • Primary contact (assignee):
  • Responsible SIGs:
  • Design proposal link (community repo):
  • Link to e2e and/or unit tests:
  • Reviewer(s) - (for LGTM) recommend having 2+ reviewers (at least one from code-area OWNERS file) agreed to review. Reviewers from multiple companies preferred:
  • Approver (likely from SIG/area to which feature belongs):
  • Feature target (which target equals to which milestone):
    • Alpha release target (x.y)
    • Beta release target (x.y)
    • Stable release target (x.y)

Set the following:

  • Description
  • Assignee(s)
  • Labels:
    • stage/{alpha,beta,stable}
    • sig/*
    • kind/feature

Please note that the Features Freeze is July 31st, after which any incomplete Feature issues will require an Exception request to be accepted into the milestone.

In addition, please be aware of the following relevant deadlines:

  • Docs deadline (open placeholder PRs): 8/21
  • Test case freeze: 8/28

Please make sure all PRs for features have relevant release notes included as well.

Happy shipping!

/cc @justaugustus @kacole2 @robertsandoval @rajendar38

@luxas
Copy link
Member

luxas commented Jul 30, 2018

@kubernetes/sig-auth-feature-requests can we graduate this to stable in v1.12?

@justaugustus
Copy link
Member

@mikedanese @kubernetes/sig-auth-feature-requests @kubernetes/sig-auth-misc --
Feature Freeze is today. Are we planning on graduating this feature in Kubernetes 1.12?
If so, can you make sure everything is up-to-date, so I can include it on the 1.12 Feature tracking spreadsheet?

@mikedanese
Copy link
Member

I'm ok with this going to GA as long as the certificates API remains beta.

@mikedanese mikedanese added stage/stable Denotes an issue tracking an enhancement targeted for Stable/GA status and removed stage/beta Denotes an issue tracking an enhancement targeted for Beta status labels Jul 31, 2018
@liggitt
Copy link
Member

liggitt commented Aug 1, 2018

Can a GA feature depend on a beta API? What does that mean, support-wise?

@justaugustus
Copy link
Member

I've added this to the 1.12 sheet as stable, but let me know if we need to walk that back.
cc: @kacole2 @wadadli @robertsandoval @rajendar38

@justaugustus justaugustus added the tracked/yes Denotes an enhancement issue is actively being tracked by the Release Team label Aug 4, 2018
@luxas
Copy link
Member

luxas commented Aug 4, 2018

I'm ok with this going to GA as long as the certificates API remains beta.

Why not graduate the as-is API to GA and do a v2 in case we need additional features or whatever?

@liggitt
Copy link
Member

liggitt commented Aug 10, 2018

We discussed this in the sig-auth meeting on 8/8, and agreed the bootstrap feature can be promoted to stable, but not the CSR API yet. That means the externally-facing portions of the bootstrap mechanism (bootstrap kubeconfig, etc) will continue to be supported.

@zparnold
Copy link
Member

Hey there! @philips I'm the wrangler for the Docs this release. Is there any chance I could have you open up a docs PR against the release-1.12 branch as a placeholder? That gives us more confidence in the feature shipping in this release and gives me something to work with when we start doing reviews/edits. Thanks! If this feature does not require docs, could you please update the features tracking spreadsheet to reflect it?

@philips
Copy link
Contributor Author

philips commented Aug 20, 2018 via email

@zparnold
Copy link
Member

zparnold commented Aug 20, 2018 via email

@mikedanese
Copy link
Member

@zparnold please use the assignee of the feature issue rather than the original creator (which is immutable) of the issue for this kind of stuff. I can do this.

@zparnold
Copy link
Member

zparnold commented Aug 20, 2018 via email

@zparnold
Copy link
Member

@mikedanese Will you be able to handle docs?

@justaugustus
Copy link
Member

@mikedanese --
Any update on docs status for this feature? Are we still planning to land it for 1.12?
At this point, code freeze is upon us, and docs are due on 9/7 (2 days).
If we don't here anything back regarding this feature ASAP, we'll need to remove it from the milestone.

cc: @zparnold @jimangel @tfogo

@mikedanese
Copy link
Member

GA docs PR here:

kubernetes/website#10232

@kacole2
Copy link
Contributor

kacole2 commented Sep 28, 2018

It looks like this has graduated to GA so I'm going to clean up and close the issue. nice job everyone!

/close

@k8s-ci-robot
Copy link
Contributor

@kacole2: Closing this issue.

In response to this:

It looks like this has graduated to GA so I'm going to clean up and close the issue. nice job everyone!

/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.

@kacole2 kacole2 removed tracked/yes Denotes an enhancement issue is actively being tracked by the Release Team labels Oct 15, 2018
ingvagabund pushed a commit to ingvagabund/enhancements that referenced this issue Apr 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/node Categorizes an issue or PR as relevant to SIG Node. stage/stable Denotes an issue tracking an enhancement targeted for Stable/GA status
Projects
None yet
Development

No branches or pull requests