remove windows desktop certs from client keyring

lib/client/client.go

@@ -118,10 +118,9 @@ type ReissueParams struct {
 	KubernetesCluster string
 	AccessRequests    []string
 	// See [proto.UserCertsRequest.DropAccessRequests].
-	DropAccessRequests    []string
-	RouteToDatabase       proto.RouteToDatabase
-	RouteToApp            proto.RouteToApp
-	RouteToWindowsDesktop proto.RouteToWindowsDesktop
+	DropAccessRequests []string
+	RouteToDatabase    proto.RouteToDatabase
+	RouteToApp         proto.RouteToApp
 
 	// ExistingCreds is a gross hack for lib/web/terminal.go to pass in
 	// existing user credentials. The TeleportClient in lib/web/terminal.go
@@ -165,8 +164,6 @@ func (p ReissueParams) usage() proto.UserCertsRequest_CertUsage {
 		// App means a request for a TLS certificate for access to a specific
 		// web app, as specified by RouteToApp.
 		return proto.UserCertsRequest_App
-	case p.RouteToWindowsDesktop.WindowsDesktop != "":
-		return proto.UserCertsRequest_WindowsDesktop
 	default:
 		// All means a request for both SSH and TLS certificates for the
 		// overall user session. These certificates are not specific to any SSH
@@ -175,7 +172,7 @@ func (p ReissueParams) usage() proto.UserCertsRequest_CertUsage {
 	}
 }
 
-func (p ReissueParams) isMFARequiredRequest(sshLogin string) *proto.IsMFARequiredRequest {
+func (p ReissueParams) isMFARequiredRequest(sshLogin string) (*proto.IsMFARequiredRequest, error) {
 	req := new(proto.IsMFARequiredRequest)
 	switch {
 	case p.NodeName != "":
@@ -184,12 +181,12 @@ func (p ReissueParams) isMFARequiredRequest(sshLogin string) *proto.IsMFARequire
 		req.Target = &proto.IsMFARequiredRequest_KubernetesCluster{KubernetesCluster: p.KubernetesCluster}
 	case p.RouteToDatabase.ServiceName != "":
 		req.Target = &proto.IsMFARequiredRequest_Database{Database: &p.RouteToDatabase}
-	case p.RouteToWindowsDesktop.WindowsDesktop != "":
-		req.Target = &proto.IsMFARequiredRequest_WindowsDesktop{WindowsDesktop: &p.RouteToWindowsDesktop}
 	case p.RouteToApp.Name != "":
 		req.Target = &proto.IsMFARequiredRequest_App{App: &p.RouteToApp}
+	default:
+		return nil, trace.BadParameter("reissue params have no valid MFA target")
 	}
-	return req
+	return req, nil
 }
 
 // CertCachePolicy describes what should happen to the certificate cache when a

lib/client/cluster_client.go

@@ -246,8 +246,6 @@ func (c *ClusterClient) generateUserCerts(ctx context.Context, cachePolicy CertC
 			PrivateKey: privKey,
 			Cert:       certs.TLS,
 		}
-	case proto.UserCertsRequest_WindowsDesktop:
-		keyRing.WindowsDesktopCerts[params.RouteToWindowsDesktop.WindowsDesktop] = certs.TLS
 	}
 
 	return keyRing, nil
@@ -391,23 +389,22 @@ func (c *ClusterClient) prepareUserCertsRequest(ctx context.Context, params Reis
 	}
 
 	return privateKey, &proto.UserCertsRequest{
-		SSHPublicKey:          sshPublicKey,
-		TLSPublicKey:          tlsPublicKey,
-		Username:              tlsCert.Subject.CommonName,
-		Expires:               expires,
-		RouteToCluster:        params.RouteToCluster,
-		KubernetesCluster:     params.KubernetesCluster,
-		AccessRequests:        params.AccessRequests,
-		DropAccessRequests:    params.DropAccessRequests,
-		RouteToDatabase:       params.RouteToDatabase,
-		RouteToWindowsDesktop: params.RouteToWindowsDesktop,
-		RouteToApp:            params.RouteToApp,
-		NodeName:              params.NodeName,
-		Usage:                 params.usage(),
-		Format:                c.tc.CertificateFormat,
-		RequesterName:         params.RequesterName,
-		SSHLogin:              c.tc.HostLogin,
-		AttestationStatement:  keyRing.PrivateKey.GetAttestationStatement().ToProto(),
+		SSHPublicKey:         sshPublicKey,
+		TLSPublicKey:         tlsPublicKey,
+		Username:             tlsCert.Subject.CommonName,
+		Expires:              expires,
+		RouteToCluster:       params.RouteToCluster,
+		KubernetesCluster:    params.KubernetesCluster,
+		AccessRequests:       params.AccessRequests,
+		DropAccessRequests:   params.DropAccessRequests,
+		RouteToDatabase:      params.RouteToDatabase,
+		RouteToApp:           params.RouteToApp,
+		NodeName:             params.NodeName,
+		Usage:                params.usage(),
+		Format:               c.tc.CertificateFormat,
+		RequesterName:        params.RequesterName,
+		SSHLogin:             c.tc.HostLogin,
+		AttestationStatement: keyRing.PrivateKey.GetAttestationStatement().ToProto(),
 	}, nil
 }
 
@@ -419,12 +416,16 @@ func (c *ClusterClient) performMFACeremony(ctx context.Context, rootClient *Clus
 		return nil, trace.Wrap(err)
 	}
 
+	mfaRequiredReq, err := params.isMFARequiredRequest(c.tc.HostLogin)
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
 	keyRing, _, err = PerformMFACeremony(ctx, PerformMFACeremonyParams{
 		CurrentAuthClient: c.AuthClient,
 		RootAuthClient:    rootClient.AuthClient,
 		MFAPrompt:         mfaPrompt,
 		MFAAgainstRoot:    c.cluster == rootClient.cluster,
-		MFARequiredReq:    params.isMFARequiredRequest(c.tc.HostLogin),
+		MFARequiredReq:    mfaRequiredReq,
 		ChallengeExtensions: mfav1.ChallengeExtensions{
 			Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_USER_SESSION,
 		},
@@ -473,7 +474,11 @@ func (c *ClusterClient) IssueUserCertsWithMFA(ctx context.Context, params Reissu
 			}
 		}
 
-		resp, err := authClient.IsMFARequired(ctx, params.isMFARequiredRequest(c.tc.HostLogin))
+		mfaRequiredReq, err := params.isMFARequiredRequest(c.tc.HostLogin)
+		if err != nil {
+			return nil, proto.MFARequired_MFA_REQUIRED_UNSPECIFIED, trace.Wrap(err)
+		}
+		resp, err := authClient.IsMFARequired(ctx, mfaRequiredReq)
 		if err != nil {
 			return nil, proto.MFARequired_MFA_REQUIRED_UNSPECIFIED, trace.Wrap(err)
 		}
@@ -686,7 +691,6 @@ func PerformMFACeremony(ctx context.Context, params PerformMFACeremonyParams) (*
 				Cert:       newCerts.TLS,
 				PrivateKey: params.PrivateKey,
 			}
-
 		case proto.UserCertsRequest_Database:
 			dbCert, err := makeDatabaseClientPEM(certsReq.RouteToDatabase.Protocol, newCerts.TLS, params.PrivateKey)
 			if err != nil {
@@ -699,13 +703,6 @@ func PerformMFACeremony(ctx context.Context, params PerformMFACeremonyParams) (*
 				Cert:       dbCert,
 				PrivateKey: params.PrivateKey,
 			}
-
-		case proto.UserCertsRequest_WindowsDesktop:
-			if keyRing.WindowsDesktopCerts == nil {
-				keyRing.WindowsDesktopCerts = make(map[string][]byte)
-			}
-			keyRing.WindowsDesktopCerts[certsReq.RouteToWindowsDesktop.WindowsDesktop] = newCerts.TLS
-
 		case proto.UserCertsRequest_App:
 			if keyRing.AppTLSCredentials == nil {
 				keyRing.AppTLSCredentials = make(map[string]TLSCredential)

lib/client/cluster_client_test.go

@@ -272,7 +272,6 @@ func TestIssueUserCertsWithMFA(t *testing.T) {
 			assertion: func(t *testing.T, keyRing *KeyRing, mfaRequired proto.MFARequired, err error) {
 				require.Error(t, err)
 				require.Nil(t, keyRing)
-				require.Equal(t, proto.MFARequired_MFA_REQUIRED_YES, mfaRequired)
 			},
 		},
 		{

lib/client/identityfile/identity.go

@@ -182,6 +182,8 @@ type WriteConfig struct {
 	OutputPath string
 	// KeyRing contains the credentials to write to the identity file.
 	KeyRing *client.KeyRing
+	// WindowsDesktopCerts contains windows desktop certs to write.
+	WindowsDesktopCerts map[string][]byte
 	// Format is the output format for the identity file.
 	Format Format
 	// KubeProxyAddr is the public address of the proxy with its kubernetes
@@ -283,7 +285,7 @@ func Write(ctx context.Context, cfg WriteConfig) (filesWritten []string, err err
 		}
 
 	case FormatWindows:
-		for k, cert := range cfg.KeyRing.WindowsDesktopCerts {
+		for k, cert := range cfg.WindowsDesktopCerts {
 			certPath := cfg.OutputPath + "." + k + ".der"
 			filesWritten = append(filesWritten, certPath)
 			if err := checkOverwrite(ctx, writer, cfg.OverwriteDestination, certPath); err != nil {

lib/client/identityfile/identity_test.go

@@ -44,6 +44,7 @@ import (
 	"github.com/gravitational/teleport/lib/auth/testauthority"
 	"github.com/gravitational/teleport/lib/client"
 	"github.com/gravitational/teleport/lib/defaults"
+	"github.com/gravitational/teleport/lib/fixtures"
 	"github.com/gravitational/teleport/lib/kube/kubeconfig"
 	"github.com/gravitational/teleport/lib/services"
 	"github.com/gravitational/teleport/lib/sshutils"
@@ -74,7 +75,7 @@ func newSelfSignedCA(priv crypto.Signer) (*tlsca.CertAuthority, authclient.Trust
 }
 
 func newClientKeyRing(t *testing.T, modifiers ...func(*tlsca.Identity)) *client.KeyRing {
-	privateKey, err := testauthority.New().GeneratePrivateKey()
+	privateKey, err := keys.ParsePrivateKey(fixtures.PEMBytes["rsa"])
 	require.NoError(t, err)
 
 	ff, tc, err := newSelfSignedCA(privateKey)
@@ -101,8 +102,7 @@ func newClientKeyRing(t *testing.T, modifiers ...func(*tlsca.Identity)) *client.
 	})
 	require.NoError(t, err)
 
-	ta := testauthority.New()
-	signer, err := ta.GeneratePrivateKey()
+	signer, err := keys.ParsePrivateKey([]byte(fixtures.SSHCAPrivateKey))
 	require.NoError(t, err)
 	caSigner, err := ssh.NewSignerFromKey(signer)
 	require.NoError(t, err)
@@ -228,14 +228,11 @@ func TestWriteAllFormats(t *testing.T) {
 		t.Run(string(format), func(t *testing.T) {
 			keyRing := newClientKeyRing(t)
 
-			keyRing.WindowsDesktopCerts = map[string][]byte{
-				"windows-user": []byte("cert data"),
-			}
-
 			cfg := WriteConfig{
-				OutputPath: path.Join(t.TempDir(), "identity"),
-				KeyRing:    keyRing,
-				Format:     format,
+				OutputPath:          path.Join(t.TempDir(), "identity"),
+				KeyRing:             keyRing,
+				WindowsDesktopCerts: map[string][]byte{"windows-user": []byte("cert data")},
+				Format:              format,
 			}
 
 			// extra fields for kubernetes

lib/client/interfaces.go

@@ -121,9 +121,6 @@ type KeyRing struct {
 	// AppTLSCredetials are TLS credentials for application access.
 	// Map key is the application name.
 	AppTLSCredentials map[string]TLSCredential
-	// WindowsDesktopCerts are TLS certificates for Windows Desktop access.
-	// Map key is the desktop server name.
-	WindowsDesktopCerts map[string][]byte `json:"WindowsDesktopCerts,omitempty"`
 	// TrustedCerts is a list of trusted certificate authorities
 	TrustedCerts []authclient.TrustedCerts
 }
@@ -185,11 +182,10 @@ func GenerateRSAKeyRing() (*KeyRing, error) {
 // NewKeyRing creates a new KeyRing for the given private key.
 func NewKeyRing(priv *keys.PrivateKey) *KeyRing {
 	return &KeyRing{
-		PrivateKey:          priv,
-		KubeTLSCredentials:  make(map[string]TLSCredential),
-		DBTLSCredentials:    make(map[string]TLSCredential),
-		AppTLSCredentials:   make(map[string]TLSCredential),
-		WindowsDesktopCerts: make(map[string][]byte),
+		PrivateKey:         priv,
+		KubeTLSCredentials: make(map[string]TLSCredential),
+		DBTLSCredentials:   make(map[string]TLSCredential),
+		AppTLSCredentials:  make(map[string]TLSCredential),
 	}
 }
 

lib/fixtures/fixtures.go

@@ -312,7 +312,8 @@ LdSp0EPWZQ6sMmAOCbpwBjNj2fonL7C5bMF2bnpJzCJPW9w7NZcfivr68qnp8yzy
 fE2RAoGBALWvlHVH/29KOVmM52sOk49tcyc3czjs/YANvbokiItxOB8VPY6QQQnS
 /CBsCZxUuWegYmkUnstHDmY1LYqjxW4goOqizIksaReivPmsTuQ1qd+aqXTfg2pt
 uy6c6X17xkP5q2Lq4i90ikyWm3Oc25aUEw48pRyK/6rABRUzpDLB
------END RSA PRIVATE KEY-----`
+-----END RSA PRIVATE KEY-----
+`
 	SSHCAPublicKey = `ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQC8kYdyZA1ZSNjZ4pqybDXvWplHQHkU6fPL+cAYHUkAT5CiQV4GOjwaSTcvZNK5U2fQ0jm6jknCnsZi1t9JujCjXUT3bYHCnSwWhXN55QzIu530Q/MeXz5W8TxYRrWULgPhqqtq8B9N554+s40higG21fmhhdDtpmQzw3vJLspY05mnL1+fW+RIKkM4rb150sdZXKINxfNQvERteE8WX0vL2yG4RuqJzYtGCDEGeHd+HLne7xfmqPxun7bUYaxAlplhm1z2J41hqaj8pBwDSEV9SBOZXvh6FjS9nvJCT7Z1bbZwWrAO/7E2ac0eV+5iEc0J+TyufO3F9uod+J+AICtB`
 )
 

lib/fixtures/keys.go

@@ -55,7 +55,8 @@ MHeDg2Bs7/XZsIrn6vo7kXmQSoQKA8O2E7rYSigUayBKa/+5thbnjKlEP+slBzmp
 1zVRrQKBgHmGNSOpSuQiHRn9YuzZ/h5dX8jCLf+wHJzymCC1wVur8IxJjhhSuOIp
 7JPquig/B6L2pNoxPa41VDGawQjJY5m4l3ap/oJj61HBB+Auf29BWXqg7V7B7XMB
 NFJgTFxC2o3mVBkQ/s6FeDl62hpMheCuO6jRYbZjsM2tUeAKORws
------END RSA PRIVATE KEY-----`),
+-----END RSA PRIVATE KEY-----
+`),
 	"rsa-db-client": []byte(`-----BEGIN RSA PRIVATE KEY-----
 MIIEpAIBAAKCAQEA3scV81B4bpa0qkpBsJDUf3UOIs6A4+WZnf5eXJcJg7zDi5/J
 2vtBuk8CTvp8eQhu4Pq2G0RhHZzrYiBMWLf/ORzBSKrN+bQi9pRKNzN/hJ7SOq+T

tool/tctl/common/auth_command.go

@@ -363,13 +363,8 @@ func (a *AuthCommand) generateWindowsCert(ctx context.Context, clusterAPI certif
 	}
 
 	_, err = identityfile.Write(ctx, identityfile.WriteConfig{
-		OutputPath: a.output,
-		KeyRing: &client.KeyRing{
-			// the godocs say the map key is the desktop server name,
-			// but in this case we're just generating a cert that's not
-			// specific to a particular desktop
-			WindowsDesktopCerts: map[string][]byte{a.windowsUser: certDER},
-		},
+		OutputPath:           a.output,
+		WindowsDesktopCerts:  map[string][]byte{a.windowsUser: certDER},
 		Format:               a.outputFormat,
 		OverwriteDestination: a.signOverwrite,
 		Writer:               a.identityWriter,