[v9] Make relogin attempts use the strongest auth method (#11781)

lib/client/api.go

@@ -366,6 +366,12 @@ type Config struct {
 
 	// ExtraProxyHeaders is a collection of http headers to be included in requests to the WebProxy.
 	ExtraProxyHeaders map[string]string
+
+	// UseStrongestAuth instructs TeleportClient to use the strongest
+	// authentication method supported by the cluster in Login attempts.
+	// Apart from the obvious benefits, UseStrongestAuth also avoids stdin
+	// hijacking issues from Login, as a single auth method is used.
+	UseStrongestAuth bool
 }
 
 // CachePolicy defines cache policy for local clients
@@ -532,8 +538,10 @@ func (p *ProfileStatus) AppNames() (result []string) {
 	return result
 }
 
-// RetryWithRelogin is a helper error handling method,
-// attempts to relogin and retry the function once
+// RetryWithRelogin is a helper error handling method, attempts to relogin and
+// retry the function once.
+// RetryWithRelogin automatically enables tc.UseStrongestAuth for Login attempts
+// in order to avoid stdin hijack bugs.
 func RetryWithRelogin(ctx context.Context, tc *TeleportClient, fn func() error) error {
 	err := fn()
 	if err == nil {
@@ -550,10 +558,21 @@ func RetryWithRelogin(ctx context.Context, tc *TeleportClient, fn func() error)
 	}
 	log.Debugf("Activating relogin on %v.", err)
 
+	if !tc.UseStrongestAuth {
+		defer func() {
+			tc.UseStrongestAuth = false
+		}()
+		// Avoid stdin hijack on relogin attempts.
+		// Users can pick an alternative MFA method by explicitly calling Login (or
+		// running `tsh login`).
+		tc.UseStrongestAuth = true
+		log.Debug("Enabling strongest auth for login. Use `tsh login` for alternative authentication methods.")
+	}
+
 	key, err := tc.Login(ctx)
 	if err != nil {
 		if trace.IsTrustError(err) {
-			return trace.Wrap(err, "refusing to connect to untrusted proxy %v without --insecure flag\n", tc.Config.SSHProxyAddr)
+			return trace.Wrap(err, "refusing to connect to untrusted proxy %v without --insecure flag\n", tc.SSHProxyAddr)
 		}
 		return trace.Wrap(err)
 	}
@@ -1323,7 +1342,7 @@ func (tc *TeleportClient) IssueUserCertsWithMFA(ctx context.Context, params Reis
 
 	key, err := proxyClient.IssueUserCertsWithMFA(ctx, params,
 		func(ctx context.Context, proxyAddr string, c *proto.MFAAuthenticateChallenge) (*proto.MFAAuthenticateResponse, error) {
-			return PromptMFAChallenge(ctx, proxyAddr, c, "", false)
+			return PromptMFAChallenge(ctx, c, proxyAddr, nil /* opts */)
 		})
 
 	return key, err
@@ -2491,9 +2510,13 @@ func (tc *TeleportClient) PingAndShowMOTD(ctx context.Context) (*webclient.PingR
 
 // Login logs the user into a Teleport cluster by talking to a Teleport proxy.
 //
+// Login may hijack stdin in some scenarios; it's strongly recommended for
+// callers to rely exclusively on prompt.Stdin after calling this method.
+// Alternatively, if tc.UseStrongestAuth is set, then no stdin hijacking
+// happens.
+//
 // The returned Key should typically be passed to ActivateKey in order to
 // update local agent state.
-//
 func (tc *TeleportClient) Login(ctx context.Context) (*Key, error) {
 	// Ping the endpoint to see if it's up and find the type of authentication
 	// supported, also show the message of the day if available.
@@ -3046,8 +3069,9 @@ func (tc *TeleportClient) mfaLocalLogin(ctx context.Context, pub []byte) (*auth.
 			RouteToCluster:    tc.SiteName,
 			KubernetesCluster: tc.KubernetesCluster,
 		},
-		User:     tc.Config.Username,
-		Password: password,
+		User:             tc.Username,
+		Password:         password,
+		UseStrongestAuth: tc.UseStrongestAuth,
 	})
 
 	return response, trace.Wrap(err)

lib/client/api_login_test.go

@@ -154,11 +154,12 @@ func TestTeleportClient_Login_localMFALogin(t *testing.T) {
 
 	ctx := context.Background()
 	tests := []struct {
-		name          string
-		secondFactor  constants.SecondFactorType
-		solveOTP      func(context.Context) (string, error)
-		solveU2F      func(ctx context.Context, facet string, challenges ...u2flib.AuthenticateChallenge) (*u2flib.AuthenticateChallengeResponse, error)
-		solveWebauthn func(ctx context.Context, origin string, assertion *wanlib.CredentialAssertion) (*proto.MFAAuthenticateResponse, error)
+		name             string
+		secondFactor     constants.SecondFactorType
+		solveOTP         func(context.Context) (string, error)
+		solveU2F         func(ctx context.Context, facet string, challenges ...u2flib.AuthenticateChallenge) (*u2flib.AuthenticateChallengeResponse, error)
+		solveWebauthn    func(ctx context.Context, origin string, assertion *wanlib.CredentialAssertion) (*proto.MFAAuthenticateResponse, error)
+		useStrongestAuth bool
 	}{
 		{
 			name:         "OK OTP device login",
@@ -178,6 +179,18 @@ func TestTeleportClient_Login_localMFALogin(t *testing.T) {
 			},
 			solveWebauthn: solveWebauthn,
 		},
+		{
+			name:         "Webauthn and UseStrongestAuth",
+			secondFactor: constants.SecondFactorOptional,
+			solveOTP: func(ctx context.Context) (string, error) {
+				panic("unused")
+			},
+			solveU2F: func(context.Context, string, ...u2flib.AuthenticateChallenge) (*u2flib.AuthenticateChallengeResponse, error) {
+				panic("unused")
+			},
+			solveWebauthn:    solveWebauthn,
+			useStrongestAuth: true,
+		},
 		{
 			name:         "OK U2F device login",
 			secondFactor: constants.SecondFactorU2F,
@@ -209,6 +222,7 @@ func TestTeleportClient_Login_localMFALogin(t *testing.T) {
 
 			tc, err := client.NewClient(cfg)
 			require.NoError(t, err)
+			tc.UseStrongestAuth = test.useStrongestAuth
 
 			clock.Advance(30 * time.Second)
 			_, err = tc.Login(ctx)

lib/client/client.go

@@ -1640,7 +1640,7 @@ func (proxy *ProxyClient) sessionSSHCertificate(ctx context.Context, nodeAddr No
 			RouteToCluster: nodeAddr.Cluster,
 		},
 		func(ctx context.Context, proxyAddr string, c *proto.MFAAuthenticateChallenge) (*proto.MFAAuthenticateResponse, error) {
-			return PromptMFAChallenge(ctx, proxyAddr, c, "", false)
+			return PromptMFAChallenge(ctx, c, proxyAddr, nil /* opts */)
 		},
 	)
 	if err != nil {

lib/client/mfa.go

@@ -41,17 +41,37 @@ var promptU2F = u2f.AuthenticateSignChallenge
 // promptWebauthn allows tests to override the Webauthn prompt function.
 var promptWebauthn = wancli.Login
 
+// PromptMFAChallengeOpts groups optional settings for PromptMFAChallenge.
+type PromptMFAChallengeOpts struct {
+	// PromptDevicePrefix is an optional prefix printed before "security key" or
+	// "device". It is used to emphasize between different kinds of devices, like
+	// registered vs new.
+	PromptDevicePrefix string
+	// Quiet suppresses users prompts.
+	Quiet bool
+	// UseStrongestAuth prompts the user to solve only the strongest challenge
+	// available.
+	// If set it also avoids stdin hijacking, as only one prompt is necessary.
+	UseStrongestAuth bool
+}
+
 // PromptMFAChallenge prompts the user to complete MFA authentication
 // challenges.
 //
-// If promptDevicePrefix is set, it will be printed in prompts before "security
-// key" or "device". This is used to emphasize between different kinds of
-// devices, like registered vs new.
-func PromptMFAChallenge(ctx context.Context, proxyAddr string, c *proto.MFAAuthenticateChallenge, promptDevicePrefix string, quiet bool) (*proto.MFAAuthenticateResponse, error) {
+// PromptMFAChallenge makes an attempt to read OTPs from prompt.Stdin and
+// abandons the read if the user chooses WebAuthn instead. For this reason
+// callers must use prompt.Stdin exclusively after calling this function.
+// Set opts.UseStrongestAuth to avoid stdin hijacking.
+func PromptMFAChallenge(ctx context.Context, c *proto.MFAAuthenticateChallenge, proxyAddr string, opts *PromptMFAChallengeOpts) (*proto.MFAAuthenticateResponse, error) {
 	// Is there a challenge present?
 	if c.TOTP == nil && len(c.U2F) == 0 && c.WebauthnChallenge == nil {
 		return &proto.MFAAuthenticateResponse{}, nil
 	}
+	if opts == nil {
+		opts = &PromptMFAChallengeOpts{}
+	}
+	promptDevicePrefix := opts.PromptDevicePrefix
+	quiet := opts.Quiet
 
 	// We have three maximum challenges, from which we only pick two: TOTP and
 	// either Webauthn (preferred) or U2F.
@@ -67,6 +87,11 @@ func PromptMFAChallenge(ctx context.Context, proxyAddr string, c *proto.MFAAuthe
 		hasNonTOTP = false
 	}
 
+	// Prompt only for the strongest auth method available?
+	if opts.UseStrongestAuth && hasNonTOTP {
+		hasTOTP = false
+	}
+
 	var numGoroutines int
 	if hasTOTP && hasNonTOTP {
 		numGoroutines = 2

lib/client/presence.go

@@ -86,7 +86,9 @@ func solveMFA(ctx context.Context, term io.Writer, tc *TeleportClient, challenge
 	// We don't support TOTP for live presence.
 	challenge.TOTP = nil
 
-	response, err := PromptMFAChallenge(ctx, tc.Config.WebProxyAddr, challenge, "", true)
+	response, err := PromptMFAChallenge(ctx, challenge, tc.Config.WebProxyAddr, &PromptMFAChallengeOpts{
+		Quiet: true,
+	})
 	if err != nil {
 		fmt.Fprintf(term, "\r\nTeleport > Failed to confirm presence: %v\r\n", err)
 		return nil, trace.Wrap(err)

lib/client/weblogin.go

@@ -213,6 +213,11 @@ type SSHLoginMFA struct {
 	User string
 	// User is the login password.
 	Password string
+	// UseStrongestAuth instructs the MFA prompt to use the strongest
+	// authentication method supported by the cluster.
+	// Apart from the obvious benefits, UseStrongestAuth also avoids stdin
+	// hijacking issues from MFA prompts, as a single auth method is used.
+	UseStrongestAuth bool
 }
 
 // initClient creates a new client to the HTTPS web proxy.
@@ -406,7 +411,9 @@ func SSHAgentMFALogin(ctx context.Context, login SSHLoginMFA) (*auth.SSHLoginRes
 		challengePB.WebauthnChallenge = wanlib.CredentialAssertionToProto(challenge.WebauthnChallenge)
 	}
 
-	respPB, err := PromptMFAChallenge(ctx, login.ProxyAddr, challengePB, "", false)
+	respPB, err := PromptMFAChallenge(ctx, challengePB, login.ProxyAddr, &PromptMFAChallengeOpts{
+		UseStrongestAuth: login.UseStrongestAuth,
+	})
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}

tool/tsh/mfa.go

@@ -278,7 +278,9 @@ func (c *mfaAddCommand) addDeviceRPC(ctx context.Context, tc *client.TeleportCli
 		if authChallenge == nil {
 			return trace.BadParameter("server bug: server sent %T when client expected AddMFADeviceResponse_ExistingMFAChallenge", resp.Response)
 		}
-		authResp, err := client.PromptMFAChallenge(ctx, tc.Config.WebProxyAddr, authChallenge, "*registered* ", false)
+		authResp, err := client.PromptMFAChallenge(ctx, authChallenge, tc.Config.WebProxyAddr, &client.PromptMFAChallengeOpts{
+			PromptDevicePrefix: "*registered*",
+		})
 		if err != nil {
 			return trace.Wrap(err)
 		}
@@ -498,7 +500,7 @@ func (c *mfaRemoveCommand) run(cf *CLIConf) error {
 		if authChallenge == nil {
 			return trace.BadParameter("server bug: server sent %T when client expected DeleteMFADeviceResponse_MFAChallenge", resp.Response)
 		}
-		authResp, err := client.PromptMFAChallenge(cf.Context, tc.Config.WebProxyAddr, authChallenge, "", false)
+		authResp, err := client.PromptMFAChallenge(cf.Context, authChallenge, tc.Config.WebProxyAddr, nil /* opts */)
 		if err != nil {
 			return trace.Wrap(err)
 		}