Split user keys for k8s access

[nklaassen]

This PR is part of the implementation of RFD 136.

The main change here is that tsh now uses a unique private key every time it gets a new k8s cert issued. This new key will use a signature algorithm according to the cluster's currently configured signature_algorithm_suite.

This is very similar to the parent PR #44718

[tigrato]

Does this means that we generate a private key per kube cluster?
Is there any reason why we do not generate a single pk per teleport cluster given that it satisfies the cluster signature algo

seems a bit slow to generate a new rsa pk every time we switch to a different cluster

[nklaassen]

Does this means that we generate a private key per kube cluster?

Yes

Is there any reason why we do not generate a single pk per teleport cluster given that it satisfies the cluster signature algo

seems a bit slow to generate a new rsa pk every time we switch to a different cluster

Two reasons to use unique private keys:

1. better isolation in case a single protocol or connection compromises the key somehow
2. better flexibility to possibly use a different key algorithm for different clusters in the future, without another breaking change to the file layout

With ECDSA keys I'd like to keep it this way, but if you think rsa keygen will be too slow client side, I can make a change to reuse a single RSA TLS private key. We could still save that same key at this file path for each cluster, just duplicated, so that we won't need another breaking change to file paths

[nklaassen]

If you think I should reuse RSA keys I'd do it for all protocols, I can handle it in another PR

[tigrato]

I suggest we reuse RSA keys, considering how slow they are to generate.

Additionally, you need to handle a special case when multiple commands are executed in parallel. Currently, we use the private key (PK) generated during tsh login, and each time someone invokes kubectl ..., it calls tsh kube credentials.

tsh kube credentials is responsible for asking auth to sign the certificate for a given cluster, which it then stores on disk. If multiple kubectl calls happen in parallel and all generate certificates, it's fine because the PK remains the same, and only the certificate is rewritten to disk (which ends up being the same for all signing requests). Although this behavior is racy, it has no downsides.

However, with a unique PK per cluster, you might end up writing a PK and a signed certificate that aren't a cert-key pair, preventing TLS from being established. The resulting error will be difficult to understand.

Note that tsh kube login doesn't generate keys; only tsh kube credentials does, and it is invoked with each call to kubectl. If you have a script or run a tool like Kubelens or k9s, they end up invoking tsh kube credentials several times per second, almost certainly causing this racy behavior to manifest.

[nklaassen]

Good points

I'm reusing RSA keys in the latest

I also added file locking when reading and writing key/cert pairs now so that processes won't do interleaved reads/writes of non-matching keys/certs. Whenever reading or writing the 2 files, the process will grab an OS file lock on the key file.

The file locking makes LoadKeysToKubeFromStore about 30% slower due to the extra syscalls, not ideal but not sure it's avoidable. On the upside, working with ECDSA keys will be much faster than RSA in other areas. It's about 4% faster just to read the (smaller) files, and in my experience cert signing and mTLS dials will be ~10x faster

[tigrato]

Can be crazy and store the cert and key in the same file instead of separate files? That would solve the lock issue and make it faster to load

[nklaassen]

For kube we probably can if never have other clients load the key/cert. Couldn't do it for apps/dbs though

[tigrato]

I would say that for kube it's probably worth it. We had customers in the past that had more than 30 kubectl invocations per second and it was really slow

[nklaassen]

ack I'll put that in on monday

[nklaassen]

@tigrato done in the latest

LoadKeysToKubeFromStore is now ~39% faster here than it is on master

$ benchstat master.txt split.txt unified.txt
goos: darwin
goarch: arm64
pkg: github.com/gravitational/teleport/lib/client
                                                   │  master.txt  │               split.txt               │             unified.txt             │
                                                   │    sec/op    │    sec/op     vs base                 │   sec/op     vs base                │
LoadKeysToKubeFromStore/LoadKeysToKubeFromStore-12   162.34µ ± 0%   211.42µ ± 0%   +30.24% (p=0.000 n=10)   99.22µ ± 0%  -38.88% (p=0.000 n=10)
LoadKeysToKubeFromStore/GetKeyRing-12                 1.030m ± 0%    2.256m ± 0%  +119.05% (p=0.000 n=10)   1.174m ± 0%  +13.97% (p=0.000 n=10)
geomean                                               408.9µ         690.7µ        +68.91%                  341.3µ       -16.54%

GetKeyRing is still a bit slower because it reads the TLS key cert, and I added a file lock here

[tigrato]

can you update these docs?

[nklaassen]

yes, thanks for catching this, done

[tigrato]

thanks for handling it