client: set TLS certificate usage for k8s/app/db certs (#6824)

* client: set TLS certificate usage for k8s/app/db certs

--- TLS usage field

The certificate usage field prevents a certificate from being used for
other purposes. For example, a k8s-specific certificate will not be
accepted by a database service endpoint.

Server-side enforcement logic was already in place for a long time, but
we stopped setting the correct Usage in UserCertRequest during keystore
refactoring in 5.0 (with introduction of k8s certs).

--- TLS certificate overwrite

As part of this, client.ReissueUserCerts will no longer write
usage-restricted certificates into the top-level TLS certificate used
for Teleport API authentication.

For example, when generating a k8s-specific certificate, we used to
overwrite both:
- `~/.tsh/keys/$proxy/$user-x509.pem`
- `~/.tsh/keys/$proxy/$user-kube/$cluster/$kubeCluster-x509.pem`
This PR stops overwriting `~/.tsh/keys/$proxy/$user-x509.pem`.
This is not a breaking change.

--- Selected k8s cluster

Prior to this PR, `tsh status` printed the selected k8s cluster based on
the top-level TLS certificate. Since we no longer overwrite that
certificate, it will not contain a k8s cluster name.

Instead, we extract it from the kubeconfig, which is actually more
accurate since a user could switch to a different context out-of-band.

* Document UserCertRequest CertUsage enum values

api/client/proto/authservice.proto

@@ -149,10 +149,21 @@ message UserCertsRequest {
     string NodeName = 9 [ (gogoproto.jsontag) = "node_name,omitempty" ];
 
     enum CertUsage {
+        // All means a request for both SSH and TLS certificates for the
+        // overall user session. These certificates are not specific to any SSH
+        // node, Kubernetes cluster, database or web app.
         All = 0;
+        // SSH means a request for an SSH certificate for access to a specific
+        // SSH node, as specified by NodeName.
         SSH = 1;
+        // Kubernetes means a request for a TLS certificate for access to a
+        // specific Kubernetes cluster, as specified by KubernetesCluster.
         Kubernetes = 2;
+        // Database means a request for a TLS certificate for access to a
+        // specific database, as specified by RouteToDatabase.
         Database = 3;
+        // App means a request for a TLS certificate for access to a specific
+        // web app, as specified by RouteToApp.
         App = 4;
     }
     // CertUsage limits the resulting user certificate to a single protocol.

lib/client/api.go

@@ -362,9 +362,6 @@ type ProfileStatus struct {
 	// kubernetes cluster.
 	KubeEnabled bool
 
-	// KubeCluster is the name of the kubernetes cluster used by this profile.
-	KubeCluster string
-
 	// KubeUsers are the kubernetes users used by this profile.
 	KubeUsers []string
 
@@ -625,7 +622,6 @@ func readProfile(profileDir string, profileName string) (*ProfileStatus, error)
 		Traits:         traits,
 		ActiveRequests: activeRequests,
 		KubeEnabled:    profile.KubeProxyAddr != "",
-		KubeCluster:    tlsID.KubernetesCluster,
 		KubeUsers:      tlsID.KubernetesUsers,
 		KubeGroups:     tlsID.KubernetesGroups,
 		Databases:      databases,

lib/client/client.go

@@ -153,14 +153,25 @@ type ReissueParams struct {
 func (p ReissueParams) usage() proto.UserCertsRequest_CertUsage {
 	switch {
 	case p.NodeName != "":
+		// SSH means a request for an SSH certificate for access to a specific
+		// SSH node, as specified by NodeName.
 		return proto.UserCertsRequest_SSH
 	case p.KubernetesCluster != "":
+		// Kubernetes means a request for a TLS certificate for access to a
+		// specific Kubernetes cluster, as specified by KubernetesCluster.
 		return proto.UserCertsRequest_Kubernetes
 	case p.RouteToDatabase.ServiceName != "":
+		// Database means a request for a TLS certificate for access to a
+		// specific database, as specified by RouteToDatabase.
 		return proto.UserCertsRequest_Database
 	case p.RouteToApp.Name != "":
+		// App means a request for a TLS certificate for access to a specific
+		// web app, as specified by RouteToApp.
 		return proto.UserCertsRequest_App
 	default:
+		// All means a request for both SSH and TLS certificates for the
+		// overall user session. These certificates are not specific to any SSH
+		// node, Kubernetes cluster, database or web app.
 		return proto.UserCertsRequest_All
 	}
 }
@@ -235,7 +246,7 @@ func (proxy *ProxyClient) reissueUserCerts(ctx context.Context, cachePolicy Cert
 		}
 	}
 
-	req, err := proxy.prepareUserCertsRequest(params, key, false)
+	req, err := proxy.prepareUserCertsRequest(params, key)
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
@@ -250,18 +261,27 @@ func (proxy *ProxyClient) reissueUserCerts(ctx context.Context, cachePolicy Cert
 		return nil, trace.Wrap(err)
 	}
 
-	key.Cert = certs.SSH
-	key.TLSCert = certs.TLS
-	if params.KubernetesCluster != "" {
-		key.KubeTLSCerts[params.KubernetesCluster] = certs.TLS
-	}
-	if params.RouteToDatabase.ServiceName != "" {
-		key.DBTLSCerts[params.RouteToDatabase.ServiceName] = certs.TLS
-	}
-	if params.RouteToApp.Name != "" {
+	key.ClusterName = params.RouteToCluster
+
+	// Only update the parts of key that match the usage. See the docs on
+	// proto.UserCertsRequest_CertUsage for which certificates match which
+	// usage.
+	//
+	// This prevents us from overwriting the top-level key.TLSCert with
+	// usage-restricted certificates.
+	switch params.usage() {
+	case proto.UserCertsRequest_All:
+		key.Cert = certs.SSH
+		key.TLSCert = certs.TLS
+	case proto.UserCertsRequest_SSH:
+		key.Cert = certs.SSH
+	case proto.UserCertsRequest_App:
 		key.AppTLSCerts[params.RouteToApp.Name] = certs.TLS
+	case proto.UserCertsRequest_Database:
+		key.DBTLSCerts[params.RouteToDatabase.ServiceName] = certs.TLS
+	case proto.UserCertsRequest_Kubernetes:
+		key.KubeTLSCerts[params.KubernetesCluster] = certs.TLS
 	}
-	key.ClusterName = params.RouteToCluster
 	return key, nil
 }
 
@@ -349,7 +369,7 @@ func (proxy *ProxyClient) IssueUserCertsWithMFA(ctx context.Context, params Reis
 	}
 	defer stream.CloseSend()
 
-	initReq, err := proxy.prepareUserCertsRequest(params, key, true)
+	initReq, err := proxy.prepareUserCertsRequest(params, key)
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
@@ -405,21 +425,12 @@ func (proxy *ProxyClient) IssueUserCertsWithMFA(ctx context.Context, params Reis
 	return key, nil
 }
 
-func (proxy *ProxyClient) prepareUserCertsRequest(params ReissueParams, key *Key, withUsage bool) (*proto.UserCertsRequest, error) {
+func (proxy *ProxyClient) prepareUserCertsRequest(params ReissueParams, key *Key) (*proto.UserCertsRequest, error) {
 	tlsCert, err := key.TeleportTLSCertificate()
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
 
-	// withUsage is used by MFA cert issue requests.
-	// TODO: After the primary TLS certificate isn't rewritten with every
-	// reissue, all cert requests should include proper usage information.
-	// See https://github.com/gravitational/teleport/issues/6161
-	usage := proto.UserCertsRequest_All
-	if withUsage {
-		usage = params.usage()
-	}
-
 	if len(params.AccessRequests) == 0 {
 		// Get the active access requests to include in the cert.
 		activeRequests, err := key.ActiveRequests()
@@ -442,7 +453,7 @@ func (proxy *ProxyClient) prepareUserCertsRequest(params ReissueParams, key *Key
 		RouteToDatabase:   params.RouteToDatabase,
 		RouteToApp:        params.RouteToApp,
 		NodeName:          params.NodeName,
-		Usage:             usage,
+		Usage:             params.usage(),
 		Format:            proxy.teleportClient.CertificateFormat,
 	}, nil
 }

tool/tsh/kube.go

@@ -22,12 +22,13 @@ import (
 	"time"
 
 	"github.com/gravitational/kingpin"
+	"github.com/gravitational/trace"
+
 	"github.com/gravitational/teleport/lib/asciitable"
 	"github.com/gravitational/teleport/lib/client"
 	"github.com/gravitational/teleport/lib/kube/kubeconfig"
 	kubeutils "github.com/gravitational/teleport/lib/kube/utils"
 	"github.com/gravitational/teleport/lib/utils"
-	"github.com/gravitational/trace"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -164,12 +165,7 @@ func (c *kubeLSCommand) run(cf *CLIConf) error {
 		return trace.Wrap(err)
 	}
 
-	var selectedCluster string
-	if kc, err := kubeconfig.Load(""); err != nil {
-		log.WithError(err).Warning("Failed parsing existing kubeconfig")
-	} else {
-		selectedCluster = kubeconfig.KubeClusterFromContext(kc.CurrentContext, currentTeleportCluster)
-	}
+	selectedCluster := selectedKubeCluster(currentTeleportCluster)
 
 	var t asciitable.Table
 	if cf.Quiet {
@@ -189,6 +185,15 @@ func (c *kubeLSCommand) run(cf *CLIConf) error {
 	return nil
 }
 
+func selectedKubeCluster(currentTeleportCluster string) string {
+	kc, err := kubeconfig.Load("")
+	if err != nil {
+		log.WithError(err).Warning("Failed parsing existing kubeconfig")
+		return ""
+	}
+	return kubeconfig.KubeClusterFromContext(kc.CurrentContext, currentTeleportCluster)
+}
+
 type kubeLoginCommand struct {
 	*kingpin.CmdClause
 	kubeCluster string

tool/tsh/tsh.go

@@ -1893,8 +1893,8 @@ func printStatus(debug bool, p *client.ProfileStatus, isActive bool) {
 	fmt.Printf("  Logins:             %v\n", strings.Join(p.Logins, ", "))
 	if p.KubeEnabled {
 		fmt.Printf("  Kubernetes:         enabled\n")
-		if p.KubeCluster != "" {
-			fmt.Printf("  Kubernetes cluster: %q\n", p.KubeCluster)
+		if kubeCluster := selectedKubeCluster(p.Cluster); kubeCluster != "" {
+			fmt.Printf("  Kubernetes cluster: %q\n", kubeCluster)
 		}
 		if len(p.KubeUsers) > 0 {
 			fmt.Printf("  Kubernetes users:   %v\n", strings.Join(p.KubeUsers, ", "))