-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Disable static tokens by default as of Kubernetes 1.18 #8850
Disable static tokens by default as of Kubernetes 1.18 #8850
Conversation
Okay, so have to remove basic auth first. |
@johngmyers @rifelpet This looks more like complete removal of static tokens for 1.18+. I guess that means it's not something that should be used by people other than devs, but I am curious about the reason to remove it from Kops starting 1.18+. |
The removal would be for security hardening. Line 20 in bf24a64
states they've been deprecated since 1.7, yet they're still provisioned always. Also kops/pkg/apis/kops/componentconfig.go Line 308 in 64ffe00
incorrectly states the field is unused. |
Seems a good idea to me. My suggestion would be to disable by default in 1.18 and remove completely in 1.19, same as with basic auth. |
f6af815
to
ba094b4
Compare
The question would be whether it is worth adding a field to the API for re-enabling this in 1.18. Other possibilities would be to reuse |
ba094b4
to
68148bd
Compare
68148bd
to
b8dd96b
Compare
Continuing the discussion in office hours, my vote is for adding a new flag, instead of reusing the |
These autogenerated static tokens have been deprecated since kops 1.8. The only thing using any of them was the basic auth support, which is controlled by the field in question. Basic auth will not work without also enabling static tokens. Basic auth and static tokens have equivalent security properties—it is just a difference of protocol encoding. There is little to no benefit of disabling basic auth without also disabling the autogenerated static tokens. |
I'm sort of confused about kubernetes/kubernetes#89069 - it removes basic auth but not the seemingly equivalent token auth. This makes it hard to find a reasonable answer, I feel. AFAICT k8s isn't removing or deprecating token-auth, at least not in #89069. kops can be proactive here and lock this down ahead of removal from k8s upstream - that does seem like a good idea. One thing we could do fairly easily is to not generate auth tokens we don't ourselves use, i.e. I don't think we should still be auto-generating the tokens in GetKubernetesAuthTokens_Deprecated - with the possible exception of "kube" or "admin" which should probably be tied to basic-auth (and thus also go away in 1.19). If anyone was relying on these tokens, we should be able to figure that out in 1.18 beta. It's also not too hard to generate your own tokens (it looks like we don't have The fallback I imagine is that users could still directly set TokenAuthFile on the apiserver in their cluster config; so we would stop defaulting it, but if users set it we would honor it. To introduce this gradually, we could continue to write To summarize:
I think this meets the need - we do generate and write these tokens to disk, but I think they are harmless unless we pass the flag to kube-apiserver. |
I'll work tonight on changing the PR to create |
94427a4
to
0fb1ee6
Compare
/retest |
0fb1ee6
to
1bf9085
Compare
1bf9085
to
a3e7ca2
Compare
/lgtm |
Thanks @johngmyers (and @hakman for the discussion!) /approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: johngmyers, justinsb The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Disables by default in k8s 1.18, can be re-enabled
by enabling basic auth. Removed in k8s 1.19using instructions in security.md.