Update the token discovery proposal #628

luxas · 2017-05-17T12:36:55Z

@jbeda @mikedanese @justinsb @timothysc @roberthbailey @lukemarsden @liggitt

A first draft of the v1.7 updates for hopefully bringing Bootstrap Tokens to beta.
Minor changes but important

liggitt · 2017-05-17T12:42:28Z

contributors/design-proposals/bootstrap-discovery.md

@@ -220,16 +219,27 @@ Only one of `--discovery-url`, `--discovery-file` or `--discovery-token` can be

 ## Implementation Details

-Our documentations (and output from `kubeadm`) should stress to users that when the token is configured for authenitication and used for TLS bootstrap (using `--insecure-experimental-approve-all-kubelet-csrs-for-group`) it is essentially a root password on the cluster and should be protected as such.  Users should set a TTL to limit this risk.  Or, after the cluster is up and running, users should delete the token using `kubeadm token delete`.
+Our documentations (and output from `kubeadm`) should stress to users that when the token is configured for authentication and used for TLS bootstrap is a pretty powerful credential due to that any person with access to it can claim to be a node.
+The highest risk regarding being able to claim a credential in the `system:nodes` group is that it can read all Secrets in the cluster, which basically compromises the cluster, until the [Node Authorizer](https://github.com/kubernetes/community/blob/master/contributors/design-proposals/kubelet-authorizer.md) is fully implemented.


even once the node authorizer is in place, any credential that can be used to obtain credentials for any node (which the bootstrapper can) can still access secrets used by any pod

Yeah, you need to know the Node name, but sure. Can you think of an easy path to lock that down?

luxas · 2017-05-18T18:36:55Z

even once the node authorizer is in place, any credential that can be used to obtain credentials for any node (which the bootstrapper can) can still access secrets used by any pod

@mikedanese @liggitt @ericchiang @jbeda @jcbsmpsn What do you think of the proposal to make the group approver not auto-approve CSRs with a node name (CN=system:node:<name>) that has already have been approved under the condition the user is a bootstrap token?

Note that certificate rotation wouldn't be touched by this change -- there the kubelet already uses its own credential(s).

That way we could remove the possibility for a not trusted person being able to get client credentials for a node in the cluster that already exists and has been bootstrapped once before. By doing that, an untrusted person with a hijacked token wouldn't be able to get client creds for the master and that way get to read the super-valuable Secrets attached to it.

What do you think?

Came to think about this now... a validation of the idea would be cool ;)

luxas · 2017-05-30T19:31:58Z

contributors/design-proposals/bootstrap-discovery.md

 * **description**. An optional free form description field for denoting the purpose of the token.  If users have especially complex token management neads, they are encouraged to use labels and annotations instead of packing machined readable data in to this field.
+* **auth-groups**. A comma-separated list of which groups the token should be authenticated as. All groups must have the `system:bootstrappers:` prefix.


I'm still considering whether this is worth it or not. The rationale would be to to not give any Bootstrap Token CSR Post access (the system:bootstrappers group) by default, but instead have a specific group (system:bootstrappers:<foo>) that would be authorized to make CSR requests.

But I still haven't decided whether that makes sense

It seems like it makes sense if the usage of these token's is going to expand past simple CSR use cases. I'd be in favor of ~~putting~~ punting on this for now. It doesn't seem like it'd be necessary for beta.

I'd be in favor of putting on this for now. It doesn't seem like it'd be necessary for beta.

putting off?

blah, "punting on this for now"

@ericchiang With some proposals in flight etc., this might actually be a requirement to graduate to beta.
This would make it possible to use Bootstrap Tokens for more than the kubelet TLS Bootstrap, now the kubeadm implementation ties them unconditionally together with RBAC rules (binding the system:bootstrappers Group to CSR create access (ok) and auto-approving all kubelet CSRs (not great default))

Specifically thinking about enabling API Server TLS Bootstrapping, there we want to be able to scope down tokens to have different privs.

Yeah, adding this feature works for me.

ericchiang · 2017-05-30T20:00:51Z

contributors/design-proposals/bootstrap-discovery.md

@@ -193,12 +193,11 @@ We extend kubeadm with a set of flags and helper commands for managing and using
 ### `kubeadm join` flags

 * `--token` This sets the token for both discovery and bootstrap auth.
-* `--discovery-url` If set this will grab the cluster-info data (a kubeconfig) from a URL. Due to the sensitive nature of this data, we will only support https URLs.  This also supports `username:password@host` syntax for doing HTTP auth.
-* `--discovery-file` If set, this will load the cluster-info from a file.
+* `--discovery-file` If set, this will load the cluster-info from a file on disk or from a HTTPS URL (the HTTPS requirement due to the sensitive nature of the data)
 * `--discovery-token` If set, (or set via `--token`) then we will be using the token scheme described above.
 * `--tls-bootstrap-token` (not officially part of this spec) This sets the token used to temporarily authenticate to the API server in order to submit a CSR for signing.  If `--insecure-experimental-approve-all-kubelet-csrs-for-group` is set to `system:bootstrappers` then these CSRs will be approved automatically for a hands off joining flow.


This flag is going away. Worth updating this proposal too?

I will, thanks

What is this token being replaced with?

A ClusterRoleBinding between a ClusterRole (like these: kubernetes/kubernetes#49284) and a group that the token got authenticated into.

roberthbailey · 2017-05-31T06:47:04Z

contributors/design-proposals/bootstrap-discovery.md

@@ -72,16 +72,16 @@ After loading the ClusterInfo from a file, the client MAY look for updated infor
 If the ClusterInfo information is hosted in a trusted place via HTTPS you can just request it that way.  This will use the root certificates that are installed on the system.  It may or may not be appropriate based on the user's constraints.  This method MUST use HTTPS.  Also, even though the payload for this URL is the `kubeconfig` format, it MUST NOT contain client credentials.

 ```
-kubeadm join --discovery-url="https://example/mycluster.yaml"
+kubeadm join --discovery-file="https://example/mycluster.yaml"


why is this flag called -file when it's clearly able to load URLs?

Since it's utilizing the option to bring a file. Whether you read it from the internet using HTTPS or from disk doesn't really matter. There were quite a long debate on this on the last edit to the proposal and on Slack I think, and we ended up with this option but the proposal wasn't reflected it seems.

Kubectl does the same thing too.

kubectl create --filename=https://foo.com/manifest.yaml

it's weird but i guess it's consistent.

roberthbailey · 2017-05-31T06:48:18Z

contributors/design-proposals/bootstrap-discovery.md

@@ -101,7 +101,7 @@ An interesting aspect here is that this information is often easily obtained bef
 The user experience for joining a cluster would be something like:

 ```
-kubeadm join --token=ae23dc.faddc87f5a5ab458 <address>
+kubeadm join --token=ae23dc.faddc87f5a5ab458 <address>:<port>


is port optional? is address a fully formed URL (which will imply a port) or a raw IP address / DNS name?

No actually it isn't optional, since it's hard for the community to agree on a default port.
kubeadm uses 6443, GKE 443 IIRC, minikube maybe 8443, etc.

roberthbailey · 2017-05-31T06:49:28Z

contributors/design-proposals/bootstrap-discovery.md

@@ -112,14 +112,13 @@ kubeadm join --token=ae23dc.faddc87f5a5ab458 <address>

 * `kubeadm` connects to the API server address specified over TLS.  As we don't yet have a root certificate to trust, this is an insecure connection and the server certificate is not validated.  `kubeadm` provides no authentication credentials at all.
  * Implementation note: the API server doesn't have to expose a new and special insecure HTTP endpoint.
+  * (D)DoS concern: Before this flow is secure to use/enable publicly (when not bootstrapping), the API Server must support rate-limiting. There are a couple of ways rate-limiting can be implemented to work for this use-case, but defining the rate-limiting flow in detail here is out of scope.


@jbeda also mentioned during the sig meeting today that an intermediate case, which should largely protect against DDoS is to only accept connections from RFC 1918 IP ranges.

cc @kubernetes/sig-auth-feature-requests

Would limiting unauthenticated users to only use the API server's cache be a more reasonable step forward? Or letting users register non-resource URLs with static information? A full blown rate limiting solution built into the API server seems like a can of worms.

Yeah -- honestly, having an option on the anonymous authorizer to only work with specified IP address ranges (defaulting to the RFC1918 ranges) would be a big first step.

@jbeda Can you open a kubernetes/kubernetes issue with more details about that and assign someone?

ping @jbeda

roberthbailey · 2017-05-31T06:51:49Z

contributors/design-proposals/bootstrap-discovery.md

@@ -165,7 +164,8 @@ The JWS specification [describes how to encode](https://tools.ietf.org/html/rfc7

 #### NEW: `kube-public` namespace

-Kubernetes ConfigMaps are per-namespace and are generally only visible to principals that have read access on that namespace.  To create a config map that *everyone* can see, we introduce a new `kube-public` namespace.  This namespace, by convention, is readable by all users (including those not authenticated).
+Kubernetes ConfigMaps are per-namespace and are generally only visible to principals that have read access on that namespace.  To create a config map that *everyone* can see, we introduce a new `kube-public` namespace.  This namespace, by convention, is readable by all users (including those not authenticated). Note that is a convention


Do we expect the entire namespace to be readable w/o authentication as the convention, or only this single config map (which is what kubeadm does)?

I think it's preferable to keep everything put there really public as the convention, but I don't think we should default to having a "read-all" Role bound to anonymous users. Instead, I expect implementors to create fine-grained rules for their use-case to open things up.

Should I clarify that in the text here?

As a note, I think other component already use this namespace (simple grep shows it refer to by kube-aggregator, for example).

@ericchiang Yes, it's used by aggregated API servers as well

As a note, I think other component already use this namespace (simple grep shows it refer to by kube-aggregator, for example).

Yes, it's used by aggregated API servers as well

I'm not aware of any expected uses of this namespace by the aggregator

Gah, was indeed thinking about the apiserver-extension configmap in the kube-system namespace. Recalled incorrectly

luxas · 2017-06-19T11:46:24Z

@mikedanese @liggitt @ericchiang @jbeda @jcbsmpsn What do you think of the proposal to make the group approver not auto-approve CSRs with a node name (CN=system:node:) that has already have been approved under the condition the user is a bootstrap token?

Note that certificate rotation wouldn't be touched by this change -- there the kubelet already uses its own credential(s).

That way we could remove the possibility for a not trusted person being able to get client credentials for a node in the cluster that already exists and has been bootstrapped once before. By doing that, an untrusted person with a hijacked token wouldn't be able to get client creds for the master and that way get to read the super-valuable Secrets attached to it.

ping @liggitt @ericchiang @jbeda @roberthbailey on that idea.

I'm not sure how good it would be to make the certificate controller able of listing nodes, but are we willing to trade that so that only CSRs for new nodes are auto-approved?

I'll update this soon...

liggitt · 2017-06-19T13:18:25Z

That seems like it is trying to allow you to survive compromise of a powerful shared secret bootstrap credential... if that happened, you wouldn't easily be able to trust any node identities obtained with that credential. I'd rather see the shared secret expired/removed/revoked after bootstrapping is done than try to add on a "first-request-per-node-wins" strategy.

luxas · 2017-06-19T14:26:46Z

That seems like it is trying to allow you to survive compromise of a powerful shared secret bootstrap credential... if that happened, you wouldn't easily be able to trust any node identities obtained with that credential.

So I was thinking about the worst-case scenario where you don't even know it has leaked...
But I now realized that it wouldn't actually help much as the unauthorized person may be able to directly create a "master node" by using the right labels, taints, etc., and get Pod bindings to the relevant Secrets via the control plane DaemonSets.

I'd rather see the shared secret expired/removed/revoked after bootstrapping is done than try to add on a "first-request-per-node-wins" strategy.

There are auto-expiration mechanisms already in place, and you can easily revoke the token at any time using kubeadm token delete <token-id> for example.

ericchiang · 2017-06-19T16:56:52Z

I'm not sure how good it would be to make the certificate controller able of listing nodes, but are we willing to trade that so that only CSRs for new nodes are auto-approved?

If I remember @gtank's original motivation for the certificates API, the bootstrapping token was only supposed to be use as DoS prevention, not actual credentials that identified the node. The primary issue we were trying to solve was the auto-scaling group problem, where workers come up with identical user-data. At the point where you can pass each node a unique secret, why not just give them credentials?

Isn't auto-approval kind of avoiding the issue of node attestation, for example @mikedanese's proposal for ACME style challenges[1]? I always saw that as the end goal instead of creating richer management features for bootstrapping tokens.

[1] https://docs.google.com/document/d/1l85qgbH-CoyCst7Ksuc12VyOr7rvDmhPJ5kmB1fDnS4/edit?ts=590a3fa5#heading=h.lhk2tnjcns8x

jbeda

Looks good to me. Some questions/suggestions but nothing to hold up this update on. Can we can always tweak it more after this goes in.

jbeda · 2017-06-19T16:51:10Z

contributors/design-proposals/bootstrap-discovery.md

@@ -112,14 +112,13 @@ kubeadm join --token=ae23dc.faddc87f5a5ab458 <address>

 * `kubeadm` connects to the API server address specified over TLS.  As we don't yet have a root certificate to trust, this is an insecure connection and the server certificate is not validated.  `kubeadm` provides no authentication credentials at all.
  * Implementation note: the API server doesn't have to expose a new and special insecure HTTP endpoint.
+  * (D)DoS concern: Before this flow is secure to use/enable publicly (when not bootstrapping), the API Server must support rate-limiting. There are a couple of ways rate-limiting can be implemented to work for this use-case, but defining the rate-limiting flow in detail here is out of scope.


Yeah -- honestly, having an option on the anonymous authorizer to only work with specified IP address ranges (defaulting to the RFC1918 ranges) would be a big first step.

jbeda · 2017-06-19T17:00:02Z

contributors/design-proposals/bootstrap-discovery.md

@@ -193,12 +193,11 @@ We extend kubeadm with a set of flags and helper commands for managing and using
 ### `kubeadm join` flags

 * `--token` This sets the token for both discovery and bootstrap auth.
-* `--discovery-url` If set this will grab the cluster-info data (a kubeconfig) from a URL. Due to the sensitive nature of this data, we will only support https URLs.  This also supports `username:password@host` syntax for doing HTTP auth.
-* `--discovery-file` If set, this will load the cluster-info from a file.
+* `--discovery-file` If set, this will load the cluster-info from a file on disk or from a HTTPS URL (the HTTPS requirement due to the sensitive nature of the data)
 * `--discovery-token` If set, (or set via `--token`) then we will be using the token scheme described above.
 * `--tls-bootstrap-token` (not officially part of this spec) This sets the token used to temporarily authenticate to the API server in order to submit a CSR for signing.  If `--insecure-experimental-approve-all-kubelet-csrs-for-group` is set to `system:bootstrappers` then these CSRs will be approved automatically for a hands off joining flow.


What is this token being replaced with?

jbeda · 2017-06-19T17:03:29Z

contributors/design-proposals/bootstrap-discovery.md

+Our documentations (and output from `kubeadm`) should stress to users that when the token is configured for authentication and used for TLS bootstrap is a pretty powerful credential due to that any person with access to it can claim to be a node.
+The highest risk regarding being able to claim a credential in the `system:nodes` group is that it can read all Secrets in the cluster, which basically compromises the cluster, until the [Node Authorizer](https://github.com/kubernetes/community/blob/master/contributors/design-proposals/kubelet-authorizer.md) is fully implemented.
+
+Users should set a TTL on the token to limit the above mentioned risk. Or, after the cluster is up and running, users should delete the token using `kubeadm token delete`.


Do we want to consider setting a default TTL for kubeadm tokens? It would be a breaking change but would be much more secure. We could default it to something like 6 hours?

We've now done this 👍

jbeda · 2017-06-19T17:04:44Z

contributors/design-proposals/bootstrap-discovery.md


-This proposal assumes RBAC to lock things down in a couple of ways.  First, it will open up `cluster-info` ConfigMap in `kube-public` so that it is readable by unauthenticated users.  Next, it will make it so that the identities in the `system:bootstrappers` group can only be used with the certs API to submit CSRs.  After a TLS certificate is created, that identity should be used instead of the bootstrap token.
+The end goal is to make it possible to delegate the client TLS Bootstrapping part to the kubelet, so `kubeadm join`'s function will solely be to verify the validity of the token and fetch the CA bundle.


But are we passing the bootstrap token to the kubelet so the kubelet can then do the cert dance? Or are we getting the first cert and then let the kubelet drive renews?

We should think through the case where people aren't using the built in CA and make sure we can do that with configs/phases.

In what case wouldn't this work with the own-CA thing? The kubelet will request its identity from the CSR API in exactly the same manner as kubeadm join would.

But are we passing the bootstrap token to the kubelet so the kubelet can then do the cert dance? Or are we getting the first cert and then let the kubelet drive renews?

I don't have a strong opinion here. WDYT?

jbeda · 2017-06-19T17:57:55Z

As for "why not give nodes credentials?" -- there is a usability issue around that. If you can copy arbitrary data around then you are good to go. If you are copy/pasting command lines around, we need it to be short and somewhat secure. Ideally we could use the token along with other mechanisms.

(Like, we could make the token good for n number of CSR signatures. Or we could include the cloud identity document, etc.)

luxas · 2017-07-20T16:01:58Z

@jbeda @ericchiang @liggitt @roberthbailey Updated.
More details on how to get this to beta can also be found here: kubernetes/enhancements#130 (comment)

cc @mattmoyer

jbeda · 2017-08-02T04:03:17Z

/lgtm

luxas · 2017-08-02T08:42:49Z

Thanks Joe! I'm just gonna merge this now so @mattmoyer can update it for the new cert pinning thing he has been working on.

Update the token discovery proposal

there was a joke about 1.1 1 release mgr, 1.2 2 release mgrs and 1.3 3 release mgrs :)

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 17, 2017

liggitt reviewed May 17, 2017

View reviewed changes

luxas mentioned this pull request May 30, 2017

Write e2e tests for the BootstrapSigner, TokenCleaner and BootstrapTokenAuthenticator kubernetes/kubernetes#46634

Closed

luxas commented May 30, 2017

View reviewed changes

ericchiang reviewed May 30, 2017

View reviewed changes

roberthbailey reviewed May 31, 2017

View reviewed changes

jbeda approved these changes Jun 19, 2017

View reviewed changes

Update the token discovery proposal to reflect the v1.8 release

44e558a

luxas force-pushed the update_discovery_proposal branch from af915c4 to 44e558a Compare July 20, 2017 16:00

jbeda mentioned this pull request Jul 26, 2017

Integrate Cluster Bootstrap/Discovery with Kubenetes Core kubernetes/enhancements#130

Closed

k8s-ci-robot assigned jbeda Aug 2, 2017

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 2, 2017

luxas merged commit cae33d3 into kubernetes:master Aug 2, 2017

MadhavJivrajani pushed a commit to MadhavJivrajani/community that referenced this pull request Nov 30, 2021

Merge pull request kubernetes#628 from luxas/update_discovery_proposal

5cf522b

Update the token discovery proposal

danehans pushed a commit to danehans/community that referenced this pull request Jul 18, 2023

Add myself to 1.2 release mgr (kubernetes#628)

d641858

there was a joke about 1.1 1 release mgr, 1.2 2 release mgrs and 1.3 3 release mgrs :)

		* description. An optional free form description field for denoting the purpose of the token. If users have especially complex token management neads, they are encouraged to use labels and annotations instead of packing machined readable data in to this field.
		* auth-groups. A comma-separated list of which groups the token should be authenticated as. All groups must have the `system:bootstrappers:` prefix.


		This proposal assumes RBAC to lock things down in a couple of ways. First, it will open up `cluster-info` ConfigMap in `kube-public` so that it is readable by unauthenticated users. Next, it will make it so that the identities in the `system:bootstrappers` group can only be used with the certs API to submit CSRs. After a TLS certificate is created, that identity should be used instead of the bootstrap token.
		The end goal is to make it possible to delegate the client TLS Bootstrapping part to the kubelet, so `kubeadm join`'s function will solely be to verify the validity of the token and fetch the CA bundle.

Update the token discovery proposal #628

Update the token discovery proposal #628

Conversation

luxas commented May 17, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luxas commented May 18, 2017

Choose a reason for hiding this comment

ericchiang May 30, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luxas commented Jun 19, 2017

liggitt commented Jun 19, 2017

luxas commented Jun 19, 2017

ericchiang commented Jun 19, 2017 • edited Loading

jbeda left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbeda commented Jun 19, 2017

luxas commented Jul 20, 2017

jbeda commented Aug 2, 2017

luxas commented Aug 2, 2017

ericchiang May 30, 2017 •

edited

Loading

ericchiang commented Jun 19, 2017 •

edited

Loading