Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.Sign up
kubeadm: enhanced TLS validation for token-based discovery in `kubeadm join` #49520
What this PR does / why we need it:
The gist of this enhancement is to support public key pinning in the style of RFC7469. When bootstrapping,
These public key hashes are short enough that the entire
This change adds two new command line flags (and associated config parameters):
This is fully backwards compatible and client side (kubeadm) only. It will be a breaking change when the flag becomes required in v1.9.
This validation is done after and in addition to the existing bootstrap token signing/MAC mechanism.
Hi @mattmoyer. Thanks for your PR.
I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with
I understand the commands that are listed here.
High level opinion that I brought up in the design doc, but wanted to reiterate here.
The value of
This won't work for anyone who provisions the masters and the workers at the same time. For example, Kubernetes Anywhere would have to use the
I am for this fix, but I think a majority of auto-scaling setups will have to use the unsafe flag.
@mattmoyer Are you sure why this happened? Was the token actually not there or is it an error in this code?
@ericchiang The user still has the power to pre-provision the CA, so they can know what the CA hash will be in beforehand
I think this was just an artifact of my local vagrant environment. It's not reproducible locally, so I think the server just wasn't up quite yet in this case.
Jul 26, 2017
I think the latest change addresses all the reviews so far, including ripping out the post-hoc validation in favor of just making two requests.
I'm leaving the original commits in https://github.com/mattmoyer/kubernetes/commits/8d9feb79c1619d8a80350f8ce1aa6dbd47350e89 and will squash/rebase this PR against current
Thanks for all the feedback.
@mikedanese : Thanks for the review. I agree that the names weren't perfect. I just updated with the names you suggested, added an explicit type on the hash value (so it's
I'll update the PR body with the new flags as well.
[APPROVALNOTIFIER] This PR is APPROVED
Associated issue: 130
The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing