k3s-io / k3s

Lightweight Kubernetes
https://k3s.io
Apache License 2.0
27.53k stars 2.31k forks source link

remove KeyEncipherment from KeyUsage for ECDSA certs #7800

Open networkhermit opened 1 year ago

networkhermit commented 1 year ago

Environmental Info: K3s Version:

k3s version v1.27.2+k3s1 (213d7ad4)
go version go1.20.4

Node(s) CPU architecture, OS, and Version:

Linux kali 6.1.0-kali9-amd64 #1 SMP PREEMPT_DYNAMIC Debian 6.1.27-1kali1 (2023-05-12) x86_64 GNU/Linux

Cluster Configuration: 1 server

Describe the bug: Actually it's not a bug, but it seems that both the kubernetes and letsencrypt community have adopted the recommanded practice. It's good for the k3s to align with the community.

Steps To Reproduce:

Expected behavior: KeyUsage doesn't contain KeyEncipherment for ECDSA certs

Actual behavior: KeyEncipherment in KeyUsage

Additional context / logs:

brandond commented 1 year ago

Hmm, it looks like this is only allowed by 1.27 and newer? On older releases this will cause errors. I think we should probably continue to use the current behavior at least through 1.28 or 1.29, just for the purposes of two-minor interoperability with older agents.

networkhermit commented 1 year ago

https://github.com/kubernetes/kubernetes/issues/117506#issuecomment-1517028932

Does k3s maintain a separate approver / signer logic? I got the impression that k3s is very close to the upstream. So I think the changes made by upstream is reflected in k3s as well, and it shouldn't introduce extra issues.

I don't know how that specific change would affact the k3s compatibility between versions. But I think if interoperability is primary concern then it's better to come up with a safe transition path, rather than suddenly change the k3s behavior in a unknown future release.

P.S. In fact I'm a little surprised that k3s use ECDSA certs by default, because upstream kubernetes uses RSA by default. Is there any config flag for k3s to use RSA certs?

brandond commented 1 year ago

No, we exclusively use ecdsa for everything we generate internally, using our own logic (not the Kubernetes cert signing API). There is no option to do otherwise.

The server nodes generate certificates for agents. Because we need to support agents up to two minors back, we will need to continue to generate certs with the current usages until it is no longer necessary to support 1.26 clients that will balk if the correct usages are missing - assuming I'm reading the scope of this change correctly.

Note that the bit we're talking about here only impacts node bootstrap, analogous to upstream kubeadm logic. The Kubernetes cert signing API stuff behaves the same as it does in vanilla Kubernetes.

networkhermit commented 1 year ago

Note that the bit we're talking about here only impacts node bootstrap, analogous to upstream kubeadm logic. The Kubernetes cert signing API stuff behaves the same as it does in vanilla Kubernetes.

Thanks for the clarification. It's clear to me now.

brandond commented 9 months ago

FWIW I think this may need to be changed in dynamiclistener.