self-hosted/teleport
self-hosted/teleport
1
0
Fork 0
mirror of https://github.com/gravitational/teleport synced 2024-10-21 17:53:28 +00:00
Code Issues Packages Projects Releases Wiki Activity

Expand --mfa-mode and disable stdin hijack by default (#13134)

Browse source 
Avoid "input swallowing" bugs by disabling stdin hijacking by default.

Only `tsh login` is allowed to hijack stdin, as it is expected to exit right
after authentication. Any MFA authentication attempts resulting from
non-`tsh login` invocations default to the user's strongest auth method.

Defaulting to the strongest auth method can cause problems in constrained
environments for users that have both Webauthn and OTP registered. For example,
someone using `tsh` under WSL (Windows Subsystem for Linux) or a remote machine
could be locked into Webauthn MFA, which they can't use because their
environment lacks USB access or they don't have physical access to it. In order
to solve this problem I've slightly modified the meaning of the `--mfa-mode`
flag so `otp` can be specified.

The `TELEPORT_MFA_MODE` environment variable may be set to avoid constant flag
passing.

Fixes #12675 and #13021.

* Expand --mfa-mode and disable stdin hijack by default
* Use TELEPORT_ instead of TSH_ for FIDO2 env var
* Use t.Setenv in tests
This commit is contained in:
Alan Parra 2022-06-03 19:13:50 -03:00 committed by GitHub
parent c865e7ea92
commit 9ca045bacd
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 121 additions and 85 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
lib/auth/webauthncli/fido2.go
View file

@ -77,7 +77,7 @@ var fidoNewDevice = func(path string) (FIDODevice, error) {
// IsFIDO2Available returns true if libfido2 is available in the current build.
func IsFIDO2Available() bool {
val, ok := os.LookupEnv("TSH_FIDO2")
val, ok := os.LookupEnv("TELEPORT_FIDO2")
// Default to enabled, otherwise obey the env variable.
return !ok || val == "1"
}

16
lib/auth/webauthncli/fido2_test.go
View file

@ -130,34 +130,30 @@ func (p pinCancelPrompt) PromptTouch() {
}
func TestIsFIDO2Available(t *testing.T) {
const fido2Key = "TSH_FIDO2"
defer func() {
os.Unsetenv(fido2Key)
}()
const fido2Key = "TELEPORT_FIDO2"
tests := []struct {
name string
setenv func()
want bool
}{
{
name: "TSH_FIDO2 unset",
name: "env var unset",
setenv: func() {
os.Unsetenv(fido2Key)
},
want: true,
},
{
name: "TSH_FIDO2=1",
name: "env var set to 1",
setenv: func() {
os.Setenv(fido2Key, "1")
t.Setenv(fido2Key, "1")
},
want: true,
},
{
name: "TSH_FIDO2=0",
name: "env var set to 0",
setenv: func() {
os.Setenv(fido2Key, "0")
t.Setenv(fido2Key, "0")
},
want: false,
},

36
lib/client/api.go
View file

@ -326,6 +326,11 @@ type Config struct {
// AuthenticatorAttachment is the desired authenticator attachment.
AuthenticatorAttachment wancli.AuthenticatorAttachment
// 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
@ -376,11 +381,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
@ -699,8 +704,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 {
@ -717,17 +720,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) {
@ -2775,11 +2767,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) {
@ -3015,8 +3002,9 @@ func (tc *TeleportClient) mfaLocalLogin(ctx context.Context, pub []byte) (*auth.
},
User: tc.Username,
Password: password,
UseStrongestAuth: tc.UseStrongestAuth,
AuthenticatorAttachment: tc.AuthenticatorAttachment,
PreferOTP: tc.PreferOTP,
AllowStdinHijack: tc.AllowStdinHijack,
})
return response, trace.Wrap(err)

37
lib/client/api_login_test.go
View file

@ -144,22 +144,41 @@ func TestTeleportClient_Login_local(t *testing.T) {
inputReader *prompt.FakeReader
solveWebauthn func(ctx context.Context, origin string, assertion *wanlib.CredentialAssertion, prompt wancli.LoginPrompt) (*proto.MFAAuthenticateResponse, error)
authConnector string
useStrongestAuth bool
allowStdinHijack bool
preferOTP bool
}{
{
name: "OTP device login",
name: "OTP device login with hijack",
secondFactor: constants.SecondFactorOptional,
inputReader: prompt.NewFakeReader().AddString(password).AddReply(solveOTP),
solveWebauthn: noopWebauthnFn,
allowStdinHijack: true,
},
{
name: "Webauthn device login",
name: "Webauthn device login with hijack",
secondFactor: constants.SecondFactorOptional,
inputReader: prompt.NewFakeReader().AddString(password).AddReply(waitForCancelFn),
solveWebauthn: solveWebauthn,
allowStdinHijack: true,
},
{
name: "Webauthn and UseStrongestAuth",
name: "Webauthn device with PIN and hijack", // a bit hypothetical, but _could_ happen.
secondFactor: constants.SecondFactorOptional,
inputReader: prompt.NewFakeReader().AddString(password).AddReply(waitForCancelFn).AddReply(userPINFn),
solveWebauthn: solvePIN,
allowStdinHijack: true,
},
{
name: "OTP preferred",
secondFactor: constants.SecondFactorOptional,
inputReader: prompt.NewFakeReader().AddString(password).AddReply(solveOTP),
solveWebauthn: func(ctx context.Context, origin string, assertion *wanlib.CredentialAssertion, prompt wancli.LoginPrompt) (*proto.MFAAuthenticateResponse, error) {
panic("this should not be called")
},
preferOTP: true,
},
{
name: "Webauthn device login",
secondFactor: constants.SecondFactorOptional,
inputReader: prompt.NewFakeReader().
AddString(password).
@ -167,13 +186,6 @@ func TestTeleportClient_Login_local(t *testing.T) {
panic("this should not be called")
}),
solveWebauthn: solveWebauthn,
useStrongestAuth: true,
},
{
name: "Webauthn device with PIN", // a bit hypothetical, but _could_ happen.
secondFactor: constants.SecondFactorOptional,
inputReader: prompt.NewFakeReader().AddString(password).AddReply(waitForCancelFn).AddReply(userPINFn),
solveWebauthn: solvePIN,
},
{
name: "passwordless login",
@ -207,8 +219,9 @@ func TestTeleportClient_Login_local(t *testing.T) {
tc, err := client.NewClient(cfg)
require.NoError(t, err)
tc.AllowStdinHijack = test.allowStdinHijack
tc.AuthConnector = test.authConnector
tc.UseStrongestAuth = test.useStrongestAuth
tc.PreferOTP = test.preferOTP
clock.Advance(30 * time.Second)
_, err = tc.Login(ctx)

32
lib/client/mfa.go
View file

@ -56,12 +56,17 @@ 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
// AuthenticatorAttachment specifies the desired authenticator attachment.
AuthenticatorAttachment wancli.AuthenticatorAttachment
// PreferOTP favors OTP challenges, if applicable.
// Takes precedence over AuthenticatorAttachment settings.
PreferOTP bool
}
// PromptMFAChallenge prompts the user to complete MFA authentication
@ -71,24 +76,22 @@ func (tc *TeleportClient) PromptMFAChallenge(
ctx context.Context, c *proto.MFAAuthenticateChallenge, optsOverride *PromptMFAChallengeOpts) (*proto.MFAAuthenticateResponse, error) {
opts := &PromptMFAChallengeOpts{
AuthenticatorAttachment: tc.AuthenticatorAttachment,
PreferOTP: tc.PreferOTP,
}
if optsOverride != nil {
opts.PromptDevicePrefix = optsOverride.PromptDevicePrefix
opts.Quiet = optsOverride.Quiet
opts.UseStrongestAuth = optsOverride.UseStrongestAuth
if optsOverride.AuthenticatorAttachment != wancli.AttachmentAuto {
opts.AuthenticatorAttachment = optsOverride.AuthenticatorAttachment
}
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 && c.WebauthnChallenge == nil {
@ -112,8 +115,15 @@ func PromptMFAChallenge(ctx context.Context, c *proto.MFAAuthenticateChallenge,
hasWebauthn = false
}
// Prompt only for the strongest auth method available?
if opts.UseStrongestAuth && hasWebauthn {
// Tweak enabled/disabled methods according to opts.
switch {
case hasTOTP && opts.PreferOTP:
hasWebauthn = false
case hasWebauthn && opts.AuthenticatorAttachment != wancli.AttachmentAuto:
// Prefer Webauthn if an specific attachment was requested.
hasTOTP = false
case hasWebauthn && !opts.AllowStdinHijack:
// Use strongest auth if hijack is not allowed.
hasTOTP = false
}

18
lib/client/weblogin.go
View file

@ -213,15 +213,16 @@ 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
// AuthenticatorAttachment is the desired authenticator attachment.
// AllowStdinHijack allows stdin hijack during MFA prompts.
// Do not set this options unless you deeply understand what you are doing.
AllowStdinHijack bool
// AuthenticatorAttachment is the authenticator attachment for MFA prompts.
AuthenticatorAttachment wancli.AuthenticatorAttachment
// PreferOTP prefers OTP in favor of other MFA methods.
PreferOTP bool
}
// initClient creates a new client to the HTTPS web proxy.
@ -397,8 +398,9 @@ func SSHAgentMFALogin(ctx context.Context, login SSHLoginMFA) (*auth.SSHLoginRes
}
respPB, err := PromptMFAChallenge(ctx, challengePB, login.ProxyAddr, &PromptMFAChallengeOpts{
UseStrongestAuth: login.UseStrongestAuth,
AllowStdinHijack: login.AllowStdinHijack,
AuthenticatorAttachment: login.AuthenticatorAttachment,
PreferOTP: login.PreferOTP,
})
if err != nil {
return nil, trace.Wrap(err)

47
tool/tsh/tsh.go
View file

@ -77,11 +77,19 @@ var log = logrus.WithFields(logrus.Fields{
})
const (
// mfaModeAuto, mfaModeCrossPlatform and mfaModePlatform correspond to
// wancli.AuthenticatorAttachment values.
// mfaModeAuto automatically chooses the best MFA device(s), without any
// restrictions.
// Allows both Webauthn and OTP.
mfaModeAuto = "auto"
// mfaModeCrossPlatform utilizes only cross-platform devices, such as
// pluggable hardware keys.
// Implies Webauthn.
mfaModeCrossPlatform = "cross-platform"
// mfaModePlatform utilizes only platform devices, such as Touch ID.
// Implies Webauthn.
mfaModePlatform = "platform"
// mfaModeOTP utilizes only OTP devices.
mfaModeOTP = "otp"
)
// CLIConf stores command line arguments and flags:
@ -397,6 +405,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"
@ -470,9 +479,10 @@ func Run(ctx context.Context, 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, mfaModeCrossPlatform, mfaModePlatform}
modes := []string{mfaModeAuto, mfaModeCrossPlatform, mfaModePlatform, 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')
@ -1194,6 +1204,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 {
@ -1274,10 +1285,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.
@ -2535,10 +2552,12 @@ func makeClient(cf *CLIConf, useProfileLogin bool) (*client.TeleportClient, erro
if cf.AuthConnector != "" {
c.AuthConnector = cf.AuthConnector
}
c.AuthenticatorAttachment, err = mfaModeToAttachment(cf.MFAMode)
mfaOpts, err := parseMFAMode(cf.MFAMode)
if err != nil {
return nil, trace.Wrap(err)
}
c.AuthenticatorAttachment = mfaOpts.AuthenticatorAttachment
c.PreferOTP = mfaOpts.PreferOTP
// If agent forwarding was specified on the command line enable it.
c.ForwardAgent = options.ForwardAgent
@ -2617,17 +2636,25 @@ func makeClient(cf *CLIConf, useProfileLogin bool) (*client.TeleportClient, erro
return tc, nil
}
func mfaModeToAttachment(val string) (wancli.AuthenticatorAttachment, error) {
switch val {
type mfaModeOpts struct {
AuthenticatorAttachment wancli.AuthenticatorAttachment
PreferOTP bool
}
func parseMFAMode(mode string) (*mfaModeOpts, error) {
opts := &mfaModeOpts{}
switch mode {
case "", mfaModeAuto:
return wancli.AttachmentAuto, nil
case mfaModeCrossPlatform:
return wancli.AttachmentCrossPlatform, nil
opts.AuthenticatorAttachment = wancli.AttachmentCrossPlatform
case mfaModePlatform:
return wancli.AttachmentPlatform, nil
opts.AuthenticatorAttachment = wancli.AttachmentPlatform
case mfaModeOTP:
opts.PreferOTP = true
default:
return wancli.AttachmentAuto, fmt.Errorf("invalid MFA mode: %q", val)
return nil, fmt.Errorf("invalid MFA mode: %q", mode)
}
return opts, nil
}
// setX11Config sets X11 config using CLI and SSH option flags.