diff --git a/lib/client/api.go b/lib/client/api.go index fa47d1c48d8bf..b2cdaa8c16f3c 100644 --- a/lib/client/api.go +++ b/lib/client/api.go @@ -321,6 +321,11 @@ type Config struct { // AuthConnector is the name of the authentication connector to use. AuthConnector string + // PreferOTP prefers OTP in favor of other MFA methods. + // Useful in constrained environments without access to USB or platform + // authenticators, such as remote hosts or virtual machines. + PreferOTP bool + // CheckVersions will check that client version is compatible // with auth server version when connecting. CheckVersions bool @@ -371,11 +376,11 @@ 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 + // AllowStdinHijack allows stdin hijack during MFA prompts. + // Stdin hijack provides a better login UX, but it can be difficult to reason + // about and is often a source of bugs. + // Do not set this options unless you deeply understand what you are doing. + AllowStdinHijack bool } // CachePolicy defines cache policy for local clients @@ -682,8 +687,6 @@ func (p *ProfileStatus) AppNames() (result []string) { // 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 { @@ -700,17 +703,6 @@ 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) { @@ -1584,8 +1576,8 @@ func (tc *TeleportClient) IssueUserCertsWithMFA(ctx context.Context, params Reis defer proxyClient.Close() key, err := proxyClient.IssueUserCertsWithMFA(ctx, params, - func(ctx context.Context, proxyAddr string, c *proto.MFAAuthenticateChallenge) (*proto.MFAAuthenticateResponse, error) { - return PromptMFAChallenge(ctx, c, proxyAddr, nil /* opts */) + func(ctx context.Context, _ string, c *proto.MFAAuthenticateChallenge) (*proto.MFAAuthenticateResponse, error) { + return tc.PromptMFAChallenge(ctx, c, nil /* optsOverride */) }) return key, err @@ -2762,11 +2754,6 @@ func (tc *TeleportClient) GetWebConfig(ctx context.Context) (*webclient.WebConfi // 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) { @@ -3323,7 +3310,8 @@ func (tc *TeleportClient) mfaLocalLogin(ctx context.Context, pub []byte) (*auth. }, User: tc.Username, Password: password, - UseStrongestAuth: tc.UseStrongestAuth, + AllowStdinHijack: tc.AllowStdinHijack, + PreferOTP: tc.PreferOTP, }) return response, trace.Wrap(err) diff --git a/lib/client/api_login_test.go b/lib/client/api_login_test.go index 982fdae4c2fd5..238e182f9789d 100644 --- a/lib/client/api_login_test.go +++ b/lib/client/api_login_test.go @@ -159,46 +159,61 @@ func TestTeleportClient_Login_localMFALogin(t *testing.T) { 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 + allowStdinHijack bool + preferOTP bool }{ { - name: "OK OTP device login", + name: "OK OTP device login with hijack", secondFactor: constants.SecondFactorOptional, solveOTP: solveOTP, solveU2F: func(context.Context, string, ...u2flib.AuthenticateChallenge) (*u2flib.AuthenticateChallengeResponse, error) { panic("unused") }, - solveWebauthn: promptWebauthnNoop, + solveWebauthn: promptWebauthnNoop, + allowStdinHijack: true, }, { - name: "OK Webauthn device login", + name: "OK Webauthn device login with hijack", secondFactor: constants.SecondFactorOptional, solveOTP: promptOTPNoop, solveU2F: func(context.Context, string, ...u2flib.AuthenticateChallenge) (*u2flib.AuthenticateChallengeResponse, error) { panic("unused") }, - solveWebauthn: solveWebauthn, + solveWebauthn: solveWebauthn, + allowStdinHijack: true, }, { - name: "Webauthn and UseStrongestAuth", - secondFactor: constants.SecondFactorOptional, - solveOTP: func(ctx context.Context) (string, error) { + name: "OK U2F device login with hijack", + secondFactor: constants.SecondFactorU2F, + solveOTP: promptOTPNoop, + solveU2F: solveU2F, + solveWebauthn: func(context.Context, string, *wanlib.CredentialAssertion) (*proto.MFAAuthenticateResponse, error) { panic("unused") }, + allowStdinHijack: true, + }, + { + name: "OTP preferred", + secondFactor: constants.SecondFactorOptional, + solveOTP: solveOTP, solveU2F: func(context.Context, string, ...u2flib.AuthenticateChallenge) (*u2flib.AuthenticateChallengeResponse, error) { panic("unused") }, - solveWebauthn: solveWebauthn, - useStrongestAuth: true, + solveWebauthn: func(ctx context.Context, origin string, assertion *wanlib.CredentialAssertion) (*proto.MFAAuthenticateResponse, error) { + panic("unused") + }, + preferOTP: true, }, { - name: "OK U2F device login", - secondFactor: constants.SecondFactorU2F, - solveOTP: promptOTPNoop, - solveU2F: solveU2F, - solveWebauthn: func(context.Context, string, *wanlib.CredentialAssertion) (*proto.MFAAuthenticateResponse, error) { + name: "Webauthn device login", + 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, }, } for _, test := range tests { @@ -222,7 +237,8 @@ func TestTeleportClient_Login_localMFALogin(t *testing.T) { tc, err := client.NewClient(cfg) require.NoError(t, err) - tc.UseStrongestAuth = test.useStrongestAuth + tc.AllowStdinHijack = test.allowStdinHijack + tc.PreferOTP = test.preferOTP clock.Advance(30 * time.Second) _, err = tc.Login(ctx) diff --git a/lib/client/client.go b/lib/client/client.go index 6b2d62ab11483..59c7bb8ed1793 100644 --- a/lib/client/client.go +++ b/lib/client/client.go @@ -1637,8 +1637,8 @@ func (proxy *ProxyClient) sessionSSHCertificate(ctx context.Context, nodeAddr No NodeName: nodeName(nodeAddr.Addr), RouteToCluster: nodeAddr.Cluster, }, - func(ctx context.Context, proxyAddr string, c *proto.MFAAuthenticateChallenge) (*proto.MFAAuthenticateResponse, error) { - return PromptMFAChallenge(ctx, c, proxyAddr, nil /* opts */) + func(ctx context.Context, _ string, c *proto.MFAAuthenticateChallenge) (*proto.MFAAuthenticateResponse, error) { + return proxy.teleportClient.PromptMFAChallenge(ctx, c, nil /* optsOverride */) }, ) if err != nil { diff --git a/lib/client/mfa.go b/lib/client/mfa.go index 8133ddd21e433..02293108d330f 100644 --- a/lib/client/mfa.go +++ b/lib/client/mfa.go @@ -49,19 +49,37 @@ type PromptMFAChallengeOpts struct { 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 + // AllowStdinHijack allows stdin hijack during MFA prompts. + // Stdin hijack provides a better login UX, but it can be difficult to reason + // about and is often a source of bugs. + // Do not set this options unless you deeply understand what you are doing. + // If false then only the strongest auth method is prompted. + AllowStdinHijack bool + // PreferOTP favors OTP challenges, if applicable. + // Takes precedence over AuthenticatorAttachment settings. + PreferOTP bool +} + +// PromptMFAChallenge prompts the user to complete MFA authentication +// challenges. +// See client.PromptMFAChallenge. +func (tc *TeleportClient) PromptMFAChallenge( + ctx context.Context, c *proto.MFAAuthenticateChallenge, optsOverride *PromptMFAChallengeOpts) (*proto.MFAAuthenticateResponse, error) { + opts := &PromptMFAChallengeOpts{ + AllowStdinHijack: tc.AllowStdinHijack, + PreferOTP: tc.PreferOTP, + } + if optsOverride != nil { + opts.PromptDevicePrefix = optsOverride.PromptDevicePrefix + opts.Quiet = optsOverride.Quiet + opts.PreferOTP = optsOverride.PreferOTP + opts.AllowStdinHijack = optsOverride.AllowStdinHijack + } + return PromptMFAChallenge(ctx, c, tc.WebProxyAddr, opts) } // PromptMFAChallenge prompts the user to complete MFA authentication // challenges. -// -// 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 { @@ -87,8 +105,12 @@ func PromptMFAChallenge(ctx context.Context, c *proto.MFAAuthenticateChallenge, hasNonTOTP = false } - // Prompt only for the strongest auth method available? - if opts.UseStrongestAuth && hasNonTOTP { + // Tweak enabled/disabled methods according to opts. + switch { + case hasTOTP && opts.PreferOTP: + hasNonTOTP = false + case hasNonTOTP && !opts.AllowStdinHijack: + // Use strongest auth if hijack is not allowed. hasTOTP = false } @@ -137,23 +159,25 @@ func PromptMFAChallenge(ctx context.Context, c *proto.MFAAuthenticateChallenge, } // Fire Webauthn or U2F goroutine. - origin := proxyAddr - if !strings.HasPrefix(origin, "https://") { - origin = "https://" + origin - } - switch { - case c.WebauthnChallenge != nil: - go func() { - log.Debugf("WebAuthn: prompting U2F devices with origin %q", origin) - resp, err := promptWebauthn(ctx, origin, wanlib.CredentialAssertionFromProto(c.WebauthnChallenge)) - respC <- response{kind: "WEBAUTHN", resp: resp, err: err} - }() - case len(c.U2F) > 0: - go func() { - log.Debugf("prompting U2F devices with facet %q", origin) - resp, err := promptU2FChallenges(ctx, proxyAddr, c.U2F) - respC <- response{kind: "U2F", resp: resp, err: err} - }() + if hasNonTOTP { + origin := proxyAddr + if !strings.HasPrefix(origin, "https://") { + origin = "https://" + origin + } + switch { + case c.WebauthnChallenge != nil: + go func() { + log.Debugf("WebAuthn: prompting U2F devices with origin %q", origin) + resp, err := promptWebauthn(ctx, origin, wanlib.CredentialAssertionFromProto(c.WebauthnChallenge)) + respC <- response{kind: "WEBAUTHN", resp: resp, err: err} + }() + case len(c.U2F) > 0: + go func() { + log.Debugf("prompting U2F devices with facet %q", origin) + resp, err := promptU2FChallenges(ctx, proxyAddr, c.U2F) + respC <- response{kind: "U2F", resp: resp, err: err} + }() + } } for i := 0; i < numGoroutines; i++ { diff --git a/lib/client/weblogin.go b/lib/client/weblogin.go index 9ef571440b397..92fb9523d4487 100644 --- a/lib/client/weblogin.go +++ b/lib/client/weblogin.go @@ -215,13 +215,14 @@ type SSHLoginMFA struct { SSHLogin // User is the login username. User string - // User is the login password. + // Password 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 + + // AllowStdinHijack allows stdin hijack during MFA prompts. + // Do not set this options unless you deeply understand what you are doing. + AllowStdinHijack bool + // PreferOTP prefers OTP in favor of other MFA methods. + PreferOTP bool } // initClient creates a new client to the HTTPS web proxy. @@ -416,7 +417,8 @@ func SSHAgentMFALogin(ctx context.Context, login SSHLoginMFA) (*auth.SSHLoginRes } respPB, err := PromptMFAChallenge(ctx, challengePB, login.ProxyAddr, &PromptMFAChallengeOpts{ - UseStrongestAuth: login.UseStrongestAuth, + AllowStdinHijack: login.AllowStdinHijack, + PreferOTP: login.PreferOTP, }) if err != nil { return nil, trace.Wrap(err) diff --git a/tool/tsh/tsh.go b/tool/tsh/tsh.go index 6eeb992725b9b..6e64216bb51af 100644 --- a/tool/tsh/tsh.go +++ b/tool/tsh/tsh.go @@ -74,6 +74,15 @@ var log = logrus.WithFields(logrus.Fields{ trace.Component: teleport.ComponentTSH, }) +const ( + // mfaModeAuto automatically chooses the best MFA device(s), without any + // restrictions. + // Allows both Webauthn and OTP. + mfaModeAuto = "auto" + // mfaModeOTP utilizes only OTP devices. + mfaModeOTP = "otp" +) + // CLIConf stores command line arguments and flags: type CLIConf struct { // UserHost contains "[login]@hostname" argument to SSH command @@ -199,6 +208,9 @@ type CLIConf struct { // AuthConnector is the name of the connector to use. AuthConnector string + // MFAMode is the preferred mode for MFA assertions. + MFAMode string + // SkipVersionCheck skips version checking for client and server SkipVersionCheck bool @@ -369,6 +381,7 @@ const ( addKeysToAgentEnvVar = "TELEPORT_ADD_KEYS_TO_AGENT" useLocalSSHAgentEnvVar = "TELEPORT_USE_LOCAL_SSH_AGENT" globalTshConfigEnvVar = "TELEPORT_GLOBAL_TSH_CONFIG" + mfaModeEnvVar = "TELEPORT_MFA_MODE" clusterHelp = "Specify the Teleport cluster to connect" browserHelp = "Set to 'none' to suppress browser opening on login" @@ -437,6 +450,11 @@ func Run(args []string, opts ...cliOption) error { Default("true"). BoolVar(&cf.EnableEscapeSequences) app.Flag("bind-addr", "Override host:port used when opening a browser for cluster logins").Envar(bindAddrEnvVar).StringVar(&cf.BindAddr) + modes := []string{mfaModeAuto, mfaModeOTP} + app.Flag("mfa-mode", fmt.Sprintf("Preferred mode for MFA and Passwordless assertions (%v)", strings.Join(modes, ", "))). + Default(mfaModeAuto). + Envar(mfaModeEnvVar). + EnumVar(&cf.MFAMode, modes...) app.HelpFlag.Short('h') ver := app.Command("version", "Print the version") ver.Flag("format", formatFlagDescription(defaultFormats...)).Short('f').Default(teleport.Text).EnumVar(&cf.Format, defaultFormats...) @@ -977,6 +995,7 @@ func onLogin(cf *CLIConf) error { return trace.Wrap(err) } tc.HomePath = cf.HomePath + // client is already logged in and profile is not expired if profile != nil && !profile.IsExpired(clockwork.NewRealClock()) { switch { @@ -1057,10 +1076,16 @@ func onLogin(cf *CLIConf) error { // -i flag specified? save the retrieved cert into an identity file makeIdentityFile := (cf.IdentityFileOut != "") + // stdin hijack is OK for login, since it tsh doesn't read input after the + // login ceremony is complete. + // Only allow the option during the login ceremony. + tc.AllowStdinHijack = true + key, err := tc.Login(cf.Context) if err != nil { return trace.Wrap(err) } + tc.AllowStdinHijack = false // the login operation may update the username and should be considered the more // "authoritative" source. @@ -2296,6 +2321,11 @@ func makeClient(cf *CLIConf, useProfileLogin bool) (*client.TeleportClient, erro if cf.AuthConnector != "" { c.AuthConnector = cf.AuthConnector } + mfaOpts, err := parseMFAMode(cf.MFAMode) + if err != nil { + return nil, trace.Wrap(err) + } + c.PreferOTP = mfaOpts.PreferOTP // If agent forwarding was specified on the command line enable it. c.ForwardAgent = options.ForwardAgent @@ -2374,6 +2404,22 @@ func makeClient(cf *CLIConf, useProfileLogin bool) (*client.TeleportClient, erro return tc, nil } +type mfaModeOpts struct { + PreferOTP bool +} + +func parseMFAMode(mode string) (*mfaModeOpts, error) { + opts := &mfaModeOpts{} + switch mode { + case "", mfaModeAuto: + case mfaModeOTP: + opts.PreferOTP = true + default: + return nil, fmt.Errorf("invalid MFA mode: %q", mode) + } + return opts, nil +} + // setX11Config sets X11 config using CLI and SSH option flags. func setX11Config(c *client.Config, cf *CLIConf, o Options, fn envGetter) error { // X11 forwarding can be enabled with -X, -Y, or -oForwardX11=yes