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

Update the token discovery proposal #628

Merged
merged 1 commit into from
Aug 2, 2017
Merged

Update the token discovery proposal #628

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 35 additions & 22 deletions contributors/design-proposals/bootstrap-discovery.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ For the `kubeadm` flow, the command line might look like:
kubeadm join --discovery-file=my-cluster.yaml
```

After loading the ClusterInfo from a file, the client MAY look for updated information from the server by reading the `kube-public` `cluster-info` ConfigMap defined below. However, when retrieving this ConfigMap the client MUST validate the certificate chain when talking to the API server.
After loading the ClusterInfo from a file, the client MAY look for updated information from the server by reading the `kube-public/cluster-info` ConfigMap defined below. However, when retrieving this ConfigMap the client MUST validate the certificate chain when talking to the API server.

**Note:** TLS bootstrap (which establishes a way for a client to authenticate itself to the server) is a separate issue and has its own set of methods. This command line may have a TLS bootstrap token (or config file) on the command line also. For this reason, even thought the `--discovery-file` argument is in the form of a `kubeconfig`, it MUST NOT contain client credentials as defined above.

Expand All @@ -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"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kubectl does the same thing too.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

```

This is really a shorthand for someone doing something like (assuming we support stdin with `-`):

```
curl https://example.com/mycluster.json | kubeadm join --discovery-file=-
curl https://example.com/mycluster.yaml | kubeadm join --discovery-file=-
```

After loading the ClusterInfo from a URL, the client MAY look for updated information from the server by reading the `kube-public` `cluster-info` ConfigMap defined below. However, when retrieving this ConfigMap the client MUST validate the certificate chain when talking to the API server.
After loading the ClusterInfo from a URL, the client MAY look for updated information from the server by reading the `kube-public/cluster-info` ConfigMap defined below. However, when retrieving this ConfigMap the client MUST validate the certificate chain when talking to the API server.

**Note:** support for loading from stdin for `--discovery-file` may not be implemented immediately.

Expand All @@ -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>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

```

**Note:** This is logically a different use of the token used for authentication for TLS bootstrap. We harmonize these usages and allow the same token to play double duty.
Expand All @@ -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. One simple idea is limiting unauthenticated requests to come from clients in RFC1918 ranges.
* `kubeadm` requests a ConfigMap containing the kubeconfig file defined above.
* This ConfigMap exists at a well known URL: `https://<server>/api/v1/namespaces/kube-public/configmaps/cluster-info`
* This ConfigMap is really public. Users don't need to authenticate to read this ConfigMap. In fact, the client MUST NOT use a bearer token here as we don't trust this endpoint yet.
* The API server returns the ConfigMap with the kubeconfig contents as normal
* Extra data items on that ConfigMap contains JWS signatures. `kubeadm` finds the correct signature based on the `token-id` part of the token. (Described below).
* `kubeadm` verifies the JWS and can now trust the server. Further
communication is simpler as the CA certificate in the kubeconfig file can be
trusted.
* `kubeadm` verifies the JWS and can now trust the server. Further communication is simpler as the CA certificate in the kubeconfig file can be trusted.


#### NEW: Bootstrap Token Structure
Expand All @@ -138,17 +137,17 @@ The `token-id` must be 6 characters and the `token-secret` must be 16 characters

#### NEW: Bootstrap Token Secrets

Bootstrap tokens are stored and managed via Kubernetes secrets in the `kube-system` namespace. They have type `bootstrap.kubernetes.io/token`.
Bootstrap tokens are stored and managed via Kubernetes secrets in the `kube-system` namespace. They have the type `bootstrap.kubernetes.io/token`.

The following keys are on the secret data:
* **token-id**. As defined above.
* **token-secret**. As defined above.
* **expiration**. After this time the token should be automatically deleted. This is encoded as an absolute UTC time using RFC3339.
* **usage-bootstrap-signing**. Set to `true` to indicate this token should be used for signing bootstrap configs. If this is missing from the token secret or set to any other value, the usage is not allowed.
* **usage-bootstrap-authentication**. Set to true to indicate that this token should be used for authenticating to the API server. If this is missing from the token secret or set to any other value, the usage is not allowed. The bootstrap token authenticagtor will use this token to auth as a user that is `system:bootstrap:<token-id>` in the group `system:bootstrappers`.
* **usage-bootstrap-authentication**. Set to true to indicate that this token should be used for authenticating to the API server. If this is missing from the token secret or set to any other value, the usage is not allowed. The bootstrap token authenticator will use this token to auth as a user that is `system:bootstrap:<token-id>` in the group `system:bootstrappers`.
* **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.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@ericchiang ericchiang May 30, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

putting off?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blah, "punting on this for now"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, adding this feature works for me.


**Future**: At some point in the future we may add the ability to specify a set of groups that this token part of during authentication. This will allow users to segment off which tokens are allowed to bootstrap which nodes. However, we will restrict these groups under `system:bootstrappers:*` to discourage usage outside of bootstrapping.

These secrets MUST be named `bootstrap-token-<token-id>`. If a token doesn't adhere to this naming scheme it MUST be ignored. The secret MUST also be ignored if the `token-id` key in the secret doesn't match the name of the secret.

Expand All @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

(to expose everything in `kube-public`), not something that's done by default in Kubernetes. `kubeadm` does _solely_ expose the `cluster-info` ConfigMap, not anything else.

In the initial implementation the `kube-public` namespace (and the `cluster-info` config map) will be created by `kubeadm`. That means that these won't exist for clusters that aren't bootstrapped with `kubeadm`. As we have need for this configmap in other contexts (self describing HA clusters?) we'll make this be more generally available.

Expand All @@ -179,7 +179,7 @@ A new controller (`bootstrapsigner`) is introduced that will watch for both new/

Another controller (`tokencleaner`) is introduced that deletes tokens that are past their expiration time.

Logically these controllers could run as a component in the control plane. But, for the sake of efficiency, they are bundeled as part of the Kubernetes controller-manager.
Logically these controllers could run as a separate component in the control plane. But, for the sake of efficiency, they are bundeled as part of the Kubernetes controller-manager.

## `kubeadm` UX

Expand All @@ -188,17 +188,16 @@ We extend kubeadm with a set of flags and helper commands for managing and using
### `kubeadm init` flags

* `--token` If set, this injects the bootstrap token to use when initializing the cluster. If this is unset, then a random token is created and shown to the user. If set explicitly to the empty string then no token is generated or created. This token is used for both discovery and TLS bootstrap by having `usage-bootstrap-signing` and `usage-bootstrap-authentication` set on the token secret.
* `--token-ttl` If set, this sets the TTL for the lifetime of this token. Defaults to 0 which means "forever"
* `--token-ttl` If set, this sets the TTL for the lifetime of this token. Defaults to 0 which means "forever" in v1.6 and v1.7. Defaults to `24h` in v1.8

### `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.
* `--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 the `system:csr-approver:approve-node-client-csr` ClusterRole is bound to the group the Bootstrap Token authenticates to, the CSR will be approved automatically (by the `csrapprover` controller) for a hands off joining flow.

Only one of `--discovery-url`, `--discovery-file` or `--discovery-token` can be set. If more than one is set then an error is surfaced and `kubeadm join` exits. Setting `--token` counts as setting `--discovery-token`.
Only one of `--discovery-file` or `--discovery-token` can be set. If more than one is set then an error is surfaced and `kubeadm join` exits. Setting `--token` counts as setting `--discovery-token`.

### `kubeadm token` commands

Expand All @@ -220,15 +219,29 @@ 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 may compromise the cluster.
The [Node Authorizer](https://github.com/kubernetes/community/blob/master/contributors/design-proposals/kubelet-authorizer.md) locks this down a bit, but an untrusted person could still try to
guess a node's name, get such a credential, guess the name of the Secret and be able to get that.

Users should set a TTL on the token to limit the above mentioned risk. `kubeadm` sets a 24h TTL on the node bootstrap token by default in v1.8.
Or, after the cluster is up and running, users should delete the token using `kubeadm token delete`.

After some back and forth, we decided to keep the separator in the token between the ID and Secret be a `.`. During the 1.6 cycle, at one point `:` was implemented but then reverted.

See https://github.com/kubernetes/client-go/issues/114 for details on creating a shared package with common constants for this scheme.
See [kubernetes/client-go#114](https://github.com/kubernetes/client-go/issues/114) for details on creating a shared package with common constants for this scheme.

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 (user `system:anonymous`). 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, the groupapprover controller will approve the CSR and the CSR identity can 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.

The binding of the `system:bootstrappers` (or similar) group to the ability to submit CSRs is not part of the default RBAC configuration. Consumers of this feature like `kubeadm` will have to explicitly create this binding.

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.
## Revision history

The binding of `system:bootstrappers` to the ability to submit certs is not part of the default RBAC configuration. Tools like `kubeadm` will have to explicitly create this binding.
- Initial proposal ([@jbeda](https://github.com/jbeda)): [link](https://github.com/kubernetes/community/blob/cb9f198a0763e0a7540cdcc9db912a403ab1acab/contributors/design-proposals/bootstrap-discovery.md)
- v1.6 updates ([@jbeda](https://github.com/jbeda)): [link](https://github.com/kubernetes/community/blob/d8ce9e91b0099795318bb06c13f00d9dad41ac26/contributors/design-proposals/bootstrap-discovery.md)
- v1.8 updates ([@luxas](https://github.com/luxas))

<!-- BEGIN MUNGE: GENERATED_ANALYTICS -->
[![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/docs/proposals/super-simple-discovery-api.md?pixel)]()
Expand Down