Split user keys for k8s access

api/utils/keypaths/keypaths.go

@@ -34,8 +34,11 @@ const (
 	fileNameKnownHosts = "known_hosts"
 	// fileExtTLSCertLegacy is the legacy suffix/extension of a file where a TLS cert is stored.
 	fileExtTLSCertLegacy = "-x509.pem"
-	// fileExtTLSCert is the suffix/extension of a file where a TLS cert is stored.
-	fileExtTLSCert = ".crt"
+	// FileExtTLSCert is the suffix/extension of a file where a TLS cert is stored.
+	FileExtTLSCert = ".crt"
+	// FileExtKubeCred is the suffix/extension of a file where a kubernetes
+	// credential is stored (TLS key and cert combined in a single file).
+	FileExtKubeCred = ".cred"
 	// fileExtTLSKey is the suffix/extension of a file where a TLS private key is stored.
 	fileExtTLSKey = ".key"
 	// fileNameTLSCerts is a file where TLS Cert Authorities are stored.
@@ -111,13 +114,13 @@ const (
 //    │   ├── foo-kube                 --> Kubernetes certs for user "foo"
 //    │   |    ├── root                 --> Kubernetes certs for Teleport cluster "root"
 //    │   |    │   ├── kubeA-kubeconfig --> standalone kubeconfig for Kubernetes cluster "kubeA"
-//    │   |    │   ├── kubeA-x509.pem   --> TLS cert for Kubernetes cluster "kubeA"
+//    │   |    │   ├── kubeA.cred       --> TLS private key and cert for Kubernetes cluster "kubeA"
 //    │   |    │   ├── kubeB-kubeconfig --> standalone kubeconfig for Kubernetes cluster "kubeB"
-//    │   |    │   ├── kubeB-x509.pem   --> TLS cert for Kubernetes cluster "kubeB"
+//    │   |    │   ├── kubeB.cred       --> TLS private key and cert for Kubernetes cluster "kubeB"
 //    │   |    │   └── localca.pem      --> Self-signed localhost CA cert for Teleport cluster "root"
 //    │   |    └── leaf                 --> Kubernetes certs for Teleport cluster "leaf"
 //    │   |        ├── kubeC-kubeconfig --> standalone kubeconfig for Kubernetes cluster "kubeC"
-//    │   |        └── kubeC-x509.pem   --> TLS cert for Kubernetes cluster "kubeC"
+//    │   |        └── kubeC.cred       --> TLS private key and cert for Kubernetes cluster "kubeC"
 //    |   └── cas                       --> Trusted clusters certificates
 //    |        ├── root.pem             --> TLS CA for teleport cluster "root"
 //    |        ├── leaf1.pem            --> TLS CA for teleport cluster "leaf1"
@@ -250,7 +253,7 @@ func AppCredentialDir(baseDir, proxy, username, cluster string) string {
 //
 // <baseDir>/keys/<proxy>/<username>-app/<cluster>/<appname>.crt
 func AppCertPath(baseDir, proxy, username, cluster, appname string) string {
-	return filepath.Join(AppCredentialDir(baseDir, proxy, username, cluster), appname+fileExtTLSCert)
+	return filepath.Join(AppCredentialDir(baseDir, proxy, username, cluster), appname+FileExtTLSCert)
 }
 
 // AppKeyPath returns the path to the user's private key for the given proxy,
@@ -290,7 +293,7 @@ func DatabaseCredentialDir(baseDir, proxy, username, cluster string) string {
 //
 // <baseDir>/keys/<proxy>/<username>-db/<cluster>/<dbname>.crt
 func DatabaseCertPath(baseDir, proxy, username, cluster, dbname string) string {
-	return filepath.Join(DatabaseCredentialDir(baseDir, proxy, username, cluster), dbname+fileExtTLSCert)
+	return filepath.Join(DatabaseCredentialDir(baseDir, proxy, username, cluster), dbname+FileExtTLSCert)
 }
 
 // DatabaseKeyPath returns the path to the user's TLS private key
@@ -316,28 +319,28 @@ func KubeDir(baseDir, proxy, username string) string {
 	return filepath.Join(ProxyKeyDir(baseDir, proxy), username+kubeDirSuffix)
 }
 
-// KubeCertDir returns the path to the user's kube cert directory
+// KubeCredentialDir returns the path to the user's kube credential directory
 // for the given proxy and cluster.
 //
 // <baseDir>/keys/<proxy>/<username>-kube/<cluster>
-func KubeCertDir(baseDir, proxy, username, cluster string) string {
+func KubeCredentialDir(baseDir, proxy, username, cluster string) string {
 	return filepath.Join(KubeDir(baseDir, proxy, username), cluster)
 }
 
-// KubeCertPath returns the path to the user's TLS certificate
-// for the given proxy, cluster, and kube cluster.
+// KubeCredPath returns the path to the user's TLS credential for the given
+// proxy, cluster, and kube cluster.
 //
-// <baseDir>/keys/<proxy>/<username>-kube/<cluster>/<kubename>-x509.pem
-func KubeCertPath(baseDir, proxy, username, cluster, kubename string) string {
-	return filepath.Join(KubeCertDir(baseDir, proxy, username, cluster), kubename+fileExtTLSCertLegacy)
+// <baseDir>/keys/<proxy>/<username>-kube/<cluster>/<kubename>.cred
+func KubeCredPath(baseDir, proxy, username, cluster, kubename string) string {
+	return filepath.Join(KubeCredentialDir(baseDir, proxy, username, cluster), kubename+FileExtKubeCred)
 }
 
 // KubeConfigPath returns the path to the user's standalone kubeconfig
 // for the given proxy, cluster, and kube cluster.
 //
 // <baseDir>/keys/<proxy>/<username>-kube/<cluster>/<kubename>-kubeconfig
 func KubeConfigPath(baseDir, proxy, username, cluster, kubename string) string {
-	return filepath.Join(KubeCertDir(baseDir, proxy, username, cluster), kubename+kubeConfigSuffix)
+	return filepath.Join(KubeCredentialDir(baseDir, proxy, username, cluster), kubename+kubeConfigSuffix)
 }
 
 // KubeCredLockfilePath returns the kube credentials lock file for given proxy
@@ -369,7 +372,7 @@ func IdentitySSHCertPath(path string) string {
 // TrimCertPathSuffix returns the given path with any cert suffix/extension trimmed off.
 func TrimCertPathSuffix(path string) string {
 	trimmedPath := strings.TrimSuffix(path, fileExtTLSCertLegacy)
-	trimmedPath = strings.TrimSuffix(trimmedPath, fileExtTLSCert)
+	trimmedPath = strings.TrimSuffix(trimmedPath, FileExtTLSCert)
 	trimmedPath = strings.TrimSuffix(trimmedPath, fileExtSSHCert)
 	return trimmedPath
 }

api/utils/keypaths/keypaths_test.go

@@ -31,7 +31,7 @@ func TestIsProfileKubeConfigPath(t *testing.T) {
 	require.NoError(t, err)
 	require.False(t, isKubeConfig)
 
-	path = keypaths.KubeCertPath("~/tsh", "proxy", "user", "cluster", "kube")
+	path = keypaths.KubeCredPath("~/tsh", "proxy", "user", "cluster", "kube")
 	isKubeConfig, err = keypaths.IsProfileKubeConfigPath(path)
 	require.NoError(t, err)
 	require.False(t, isKubeConfig)

api/utils/keys/privatekey_test.go

@@ -30,6 +30,8 @@ import (
 	"testing"
 	"time"
 
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/stretchr/testify/require"
 )
 
@@ -87,7 +89,7 @@ func TestX509KeyPair(t *testing.T) {
 			tlsCert, err := X509KeyPair(tc.certPEM, tc.keyPEM)
 			require.NoError(t, err)
 
-			require.Equal(t, expectCert, tlsCert)
+			require.Empty(t, cmp.Diff(expectCert, tlsCert, cmpopts.IgnoreFields(tls.Certificate{}, "Leaf")))
 		})
 	}
 }

lib/benchmark/kube.go

@@ -99,9 +99,9 @@ func getKubeTLSClientConfig(ctx context.Context, tc *client.TeleportClient) (res
 		return rest.TLSClientConfig{}, trace.Wrap(err)
 	}
 
-	certPem := k.KubeTLSCerts[tc.KubernetesCluster]
+	cred := k.KubeTLSCredentials[tc.KubernetesCluster]
 
-	keyPEM, err := k.PrivateKey.SoftwarePrivateKeyPEM()
+	keyPEM, err := cred.PrivateKey.SoftwarePrivateKeyPEM()
 	if err != nil {
 		return rest.TLSClientConfig{}, trace.Wrap(err)
 	}
@@ -132,7 +132,7 @@ func getKubeTLSClientConfig(ctx context.Context, tc *client.TeleportClient) (res
 
 	return rest.TLSClientConfig{
 		CAData:     bytes.Join(clusterCAs, []byte("\n")),
-		CertData:   certPem,
+		CertData:   cred.Cert,
 		KeyData:    keyPEM,
 		ServerName: tlsServerName,
 	}, nil

lib/client/api.go

@@ -3724,7 +3724,10 @@ func (tc *TeleportClient) SSHLogin(ctx context.Context, sshLoginFunc SSHLoginFun
 	keyRing.ProxyHost = tc.WebProxyHost()
 
 	if tc.KubernetesCluster != "" {
-		keyRing.KubeTLSCerts[tc.KubernetesCluster] = response.TLSCert
+		keyRing.KubeTLSCredentials[tc.KubernetesCluster] = TLSCredential{
+			Cert:       response.TLSCert,
+			PrivateKey: priv,
+		}
 	}
 	if tc.DatabaseService != "" {
 		keyRing.DBTLSCredentials[tc.DatabaseService] = TLSCredential{

lib/client/client_store.go

@@ -21,7 +21,6 @@ package client
 import (
 	"errors"
 	"net/url"
-	"os"
 	"time"
 
 	"github.com/gravitational/trace"
@@ -258,27 +257,19 @@ func (s *Store) FullProfileStatus() (*ProfileStatus, []*ProfileStatus, error) {
 // Store transverses the entire store to find the keys. This operation takes a long time
 // when the store has a lot of keys and when we call the function multiple times in
 // parallel.
-// Although this function speeds up the process since it removes all transversals,
-// it still has to read 2 different files:
-// - $TSH_HOME/keys/$PROXY/$USER-kube/$TELEPORT_CLUSTER/$KUBE_CLUSTER-x509.pem
-// - $TSH_HOME/keys/$PROXY/$USER
-func LoadKeysToKubeFromStore(profile *profile.Profile, dirPath, teleportCluster, kubeCluster string) ([]byte, []byte, error) {
+// This function speeds up the process since it removes all transversals, and
+// only reads 1 file:
+// - $TSH_HOME/keys/$PROXY/$USER-kube/$TELEPORT_CLUSTER/$KUBE_CLUSTER.cred
+func LoadKeysToKubeFromStore(profile *profile.Profile, dirPath, teleportCluster, kubeCluster string) (keyPEM, certPEM []byte, err error) {
 	fsKeyStore := NewFSKeyStore(dirPath)
 
-	certPath := fsKeyStore.kubeCertPath(KeyRingIndex{ProxyHost: profile.SiteName, ClusterName: teleportCluster, Username: profile.Username}, kubeCluster)
-	kubeCert, err := os.ReadFile(certPath)
+	credPath := fsKeyStore.kubeCredPath(KeyRingIndex{ProxyHost: profile.SiteName, ClusterName: teleportCluster, Username: profile.Username}, kubeCluster)
+	keyPEM, certPEM, err = readKubeCredentialFile(credPath)
 	if err != nil {
 		return nil, nil, trace.Wrap(err)
 	}
-
-	privKeyPath := fsKeyStore.userKeyPath(KeyRingIndex{ProxyHost: profile.SiteName, Username: profile.Username})
-	privKey, err := os.ReadFile(privKeyPath)
-	if err != nil {
-		return nil, nil, trace.Wrap(err)
-	}
-
-	if err := keys.AssertSoftwarePrivateKey(privKey); err != nil {
+	if err := keys.AssertSoftwarePrivateKey(keyPEM); err != nil {
 		return nil, nil, trace.Wrap(err, "unsupported private key type")
 	}
-	return kubeCert, privKey, nil
+	return keyPEM, certPEM, nil
 }

lib/client/client_store_test.go

@@ -20,7 +20,13 @@ package client
 
 import (
 	"context"
+	"crypto/rand"
+	"crypto/x509"
 	"crypto/x509/pkix"
+	"encoding/pem"
+	"fmt"
+	"math/big"
+	"sync"
 	"sync/atomic"
 	"testing"
 	"time"
@@ -36,6 +42,7 @@ import (
 	apisshutils "github.com/gravitational/teleport/api/utils/sshutils"
 	"github.com/gravitational/teleport/lib/auth/authclient"
 	"github.com/gravitational/teleport/lib/auth/testauthority"
+	"github.com/gravitational/teleport/lib/cryptosuites"
 	"github.com/gravitational/teleport/lib/defaults"
 	"github.com/gravitational/teleport/lib/services"
 	"github.com/gravitational/teleport/lib/sshutils"
@@ -62,7 +69,11 @@ func newTestAuthority(t *testing.T) testAuthority {
 
 // makeSignedKeyRing helper returns a new user key ring signed by CAPriv key.
 func (s *testAuthority) makeSignedKeyRing(t *testing.T, idx KeyRingIndex, makeExpired bool) *KeyRing {
-	priv, err := s.keygen.GeneratePrivateKey()
+	signer, err := cryptosuites.GenerateKeyWithAlgorithm(cryptosuites.ECDSAP256)
+	require.NoError(t, err)
+	keyPEM, err := keys.MarshalPrivateKey(signer)
+	require.NoError(t, err)
+	priv, err := keys.NewPrivateKey(signer, keyPEM)
 	require.NoError(t, err)
 
 	allowedLogins := []string{idx.Username, "root"}
@@ -71,8 +82,8 @@ func (s *testAuthority) makeSignedKeyRing(t *testing.T, idx KeyRingIndex, makeEx
 		ttl = -ttl
 	}
 
-	// reuse the same RSA keys for SSH and TLS keys
-	// TODO(nklaassen): don't
+	// reuse the same keys for SSH and TLS keys
+	// TODO(nklaassen): don't reuse these keys.
 	clock := clockwork.NewRealClock()
 	identity := tlsca.Identity{
 		Username: idx.Username,
@@ -360,6 +371,102 @@ func TestProxySSHConfig(t *testing.T) {
 	})
 }
 
+// BenchmarkLoadKeysToKubeFromStore benchmarks the namesake function used in the
+// `tsh kube credentials` command called by kubectl. It should be reasonably
+// fast to avoid adding latency to all kubectl calls. It should tolerate being
+// called many times in parallel.
+func BenchmarkLoadKeysToKubeFromStore(b *testing.B) {
+	key, err := cryptosuites.GenerateKeyWithAlgorithm(cryptosuites.ECDSAP256)
+	require.NoError(b, err)
+
+	template := x509.Certificate{
+		Subject: pkix.Name{
+			CommonName: "k8scluster",
+		},
+		SerialNumber: big.NewInt(1),
+		NotBefore:    time.Now(),
+		NotAfter:     time.Now().Add(time.Hour),
+		KeyUsage:     x509.KeyUsageDigitalSignature,
+	}
+	certDER, err := x509.CreateCertificate(rand.Reader, &template, &template, key.Public(), key)
+	require.NoError(b, err)
+	certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certDER})
+	require.NotEmpty(b, certPEM)
+
+	keyPEM, err := keys.MarshalPrivateKey(key)
+	require.NoError(b, err)
+	privateKey, err := keys.NewPrivateKey(key, keyPEM)
+	require.NoError(b, err)
+
+	kubeCred := TLSCredential{
+		PrivateKey: privateKey,
+		Cert:       certPEM,
+	}
+
+	dir := b.TempDir()
+	fsKeyStore := NewFSKeyStore(dir)
+
+	keyRing := &KeyRing{
+		KeyRingIndex: KeyRingIndex{
+			ProxyHost:   "teleport.example.com",
+			Username:    "tester",
+			ClusterName: "teleportcluster",
+		},
+		PrivateKey:         privateKey,
+		TLSCert:            certPEM,
+		KubeTLSCredentials: make(map[string]TLSCredential, 10),
+	}
+
+	kubeClusterNames := make([]string, 0, 10)
+	for i := 0; i < 10; i++ {
+		kubeClusterName := fmt.Sprintf("kubecluster-%d", i)
+		keyRing.KubeTLSCredentials[kubeClusterName] = kubeCred
+		kubeClusterNames = append(kubeClusterNames, kubeClusterName)
+	}
+
+	err = fsKeyStore.AddKeyRing(keyRing)
+	require.NoError(b, err)
+
+	b.Run("LoadKeysToKubeFromStore", func(b *testing.B) {
+		for i := 0; i < b.N; i++ {
+			var wg sync.WaitGroup
+			wg.Add(len(kubeClusterNames))
+			for _, kubeClusterName := range kubeClusterNames {
+				go func() {
+					defer wg.Done()
+					keyPEM, certPEM, err := LoadKeysToKubeFromStore(&profile.Profile{
+						SiteName: "teleport.example.com",
+						Username: "tester",
+					}, dir, "teleportcluster", kubeClusterName)
+					require.NoError(b, err)
+					require.NotEmpty(b, certPEM)
+					require.NotEmpty(b, keyPEM)
+				}()
+			}
+			wg.Wait()
+		}
+	})
+
+	// Compare against a naive GetKeyRing call which loads the key and cert for
+	// all active kube clusters, not just the one requested.
+	b.Run("GetKeyRing", func(b *testing.B) {
+		for i := 0; i < b.N; i++ {
+			var wg sync.WaitGroup
+			wg.Add(len(kubeClusterNames))
+			for _, kubeClusterName := range kubeClusterNames {
+				go func() {
+					defer wg.Done()
+					keyRing, err := fsKeyStore.GetKeyRing(keyRing.KeyRingIndex, WithKubeCerts{})
+					require.NoError(b, err)
+					require.NotNil(b, keyRing.KubeTLSCredentials[kubeClusterName].PrivateKey)
+					require.NotEmpty(b, keyRing.KubeTLSCredentials[kubeClusterName].Cert)
+				}()
+			}
+			wg.Wait()
+		}
+	})
+}
+
 var (
 	CAPriv = []byte(`-----BEGIN RSA PRIVATE KEY-----
 MIIEowIBAAKCAQEAwBgwn+vkjCcKEr2fbX1mLN555B9amVYfD/fUZBNbXKpHaqYn

lib/client/cluster_client.go

@@ -241,7 +241,10 @@ func (c *ClusterClient) generateUserCerts(ctx context.Context, cachePolicy CertC
 			PrivateKey: privKey,
 		}
 	case proto.UserCertsRequest_Kubernetes:
-		keyRing.KubeTLSCerts[params.KubernetesCluster] = certs.TLS
+		keyRing.KubeTLSCredentials[params.KubernetesCluster] = TLSCredential{
+			PrivateKey: privKey,
+			Cert:       certs.TLS,
+		}
 	case proto.UserCertsRequest_WindowsDesktop:
 		keyRing.WindowsDesktopCerts[params.RouteToWindowsDesktop.WindowsDesktop] = certs.TLS
 	}
@@ -353,8 +356,8 @@ func (c *ClusterClient) prepareUserCertsRequest(ctx context.Context, params Reis
 	var sshPublicKey, tlsPublicKey []byte
 	var privateKey *keys.PrivateKey
 	switch params.usage() {
-	case proto.UserCertsRequest_App:
-		privateKey, err = keyRing.GenerateKey(ctx, c.tc, cryptosuites.UserTLS)
+	case proto.UserCertsRequest_App, proto.UserCertsRequest_Kubernetes:
+		privateKey, err = keyRing.generateSubjectTLSKey(ctx, c.tc, cryptosuites.UserTLS)
 		if err != nil {
 			return nil, nil, trace.Wrap(err)
 		}
@@ -363,7 +366,7 @@ func (c *ClusterClient) prepareUserCertsRequest(ctx context.Context, params Reis
 			return nil, nil, trace.Wrap(err)
 		}
 	case proto.UserCertsRequest_Database:
-		privateKey, err = keyRing.GenerateKey(ctx, c.tc, cryptosuites.UserDatabase)
+		privateKey, err = keyRing.generateSubjectTLSKey(ctx, c.tc, cryptosuites.UserDatabase)
 		if err != nil {
 			return nil, nil, trace.Wrap(err)
 		}
@@ -670,10 +673,13 @@ func PerformMFACeremony(ctx context.Context, params PerformMFACeremonyParams) (*
 	case len(newCerts.TLS) > 0:
 		switch certsReq.Usage {
 		case proto.UserCertsRequest_Kubernetes:
-			if keyRing.KubeTLSCerts == nil {
-				keyRing.KubeTLSCerts = make(map[string][]byte)
+			if keyRing.KubeTLSCredentials == nil {
+				keyRing.KubeTLSCredentials = make(map[string]TLSCredential)
+			}
+			keyRing.KubeTLSCredentials[certsReq.KubernetesCluster] = TLSCredential{
+				Cert:       newCerts.TLS,
+				PrivateKey: params.PrivateKey,
 			}
-			keyRing.KubeTLSCerts[certsReq.KubernetesCluster] = newCerts.TLS
 
 		case proto.UserCertsRequest_Database:
 			dbCert, err := makeDatabaseClientPEM(certsReq.RouteToDatabase.Protocol, newCerts.TLS, params.PrivateKey)