Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Jun 3, 2022

Conversation

codingllama
Copy link
Contributor

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.

@github-actions github-actions bot requested review from espadolini and Tener June 2, 2022 21:45
@github-actions github-actions bot added the tsh tsh - Teleport's command line tool for logging into nodes running Teleport. label Jun 2, 2022
Copy link
Collaborator

@zmb3 zmb3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code changes look good - see below for a few comments.

Are there any docs that we should update along with this change?

lib/auth/webauthncli/fido2.go Show resolved Hide resolved
lib/auth/webauthncli/fido2.go Show resolved Hide resolved
lib/auth/webauthncli/fido2_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@Tener Tener left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall.

Would it make sense to make it possible to put the preferred modes into tsh config file?

// TshConfig represents configuration loaded from the tsh config file.
type TshConfig struct {
// ExtraHeaders are additional http headers to be included in
// webclient requests.
ExtraHeaders []ExtraProxyHeaders `yaml:"add_headers,omitempty"`
}

lib/auth/webauthncli/fido2_test.go Outdated Show resolved Hide resolved
@codingllama
Copy link
Contributor Author

@zmb3 I'll follow up with docs separately - both for tsh and passwordless (which I still need to write).

@Tener I'll have a look at the config file and possibly follow up in a separate PR, although I think the env var might be good enough for remote machines. Let's see.

@codingllama codingllama force-pushed the codingllama/sessionmfa-input branch from 6f51c36 to 908856d Compare June 3, 2022 14:28
@codingllama
Copy link
Contributor Author

Thanks for the reviews, everyone.

@codingllama codingllama removed the request for review from espadolini June 3, 2022 14:28
@codingllama codingllama enabled auto-merge (squash) June 3, 2022 14:29
@codingllama codingllama force-pushed the codingllama/sessionmfa-input branch 10 times, most recently from 10853fa to 6ed3d66 Compare June 3, 2022 20:50
@codingllama codingllama force-pushed the codingllama/sessionmfa-input branch from 6ed3d66 to 0ad28aa Compare June 3, 2022 21:50
@codingllama codingllama merged commit 9ca045b into master Jun 3, 2022
@codingllama codingllama deleted the codingllama/sessionmfa-input branch June 3, 2022 22:14
codingllama added a commit that referenced this pull request Jun 6, 2022
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
codingllama added a commit that referenced this pull request Jun 8, 2022
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
codingllama added a commit that referenced this pull request Jun 8, 2022
…13212)

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.

* Expand --mfa-mode and disable stdin hijack by default (#13134)
* Use TeleportClient.PromptMFAChallenge where applicable
* Adapt/backport tests and fix prefer OTP
* Appease linter
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tsh tsh - Teleport's command line tool for logging into nodes running Teleport.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UseStrongestAuth "relogin" problematic on remote environments
3 participants