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

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

Merged
merged 2 commits into from
Apr 11, 2022

Conversation

codingllama
Copy link
Contributor

Fixes a potential stdin hijacking bug by making relogin attempts default to a
single MFA method (the strongest available).

The problematic scenario is as follows:

  1. User has both OTP and security keys registered
  2. "Relogin" is triggered via a tsh command (say,
    tsh logout; tsh ssh --proxy=example.com llama@myserver)
  3. User is prompted to pick either OTP or security key ("Tap any security key or
    enter a code from a OTP device")
  4. An stdin read is fired in the background to read the OTP code (via
    prompt.Stdin)
  5. User picks the security method, thus the stdin read is "abandoned"

In most cases this is fine, as the program ends right after. The issue is when a
relogin is triggered by a long living tsh invocation (again, tsh ssh ...): in
this case the stdin hijack causes input to be swallowed.

Forcing a single MFA option avoids the potential stdin hijack, fixing the
problem for all relogin invocations. tsh login behavior remains the same.

Note that we have to default to cluster's most secure method without checking
the user devices. The user is not logged in yet, thus the backend cannot reveal
any information about that user.

Issue #11709.

@codingllama
Copy link
Contributor Author

Similar to #11847 - doesn't backport cleanly, so changes are in the second commit.

Fixes a potential stdin hijacking bug by making relogin attempts default to a
single MFA method (the strongest available).

The problematic scenario is as follows:

1. User has both OTP and security keys registered
2. "Relogin" is triggered via a tsh command (say,
   `tsh logout; tsh ssh --proxy=example.com llama@myserver`)
3. User is prompted to pick either OTP or security key ("Tap any security key or
   enter a code from a OTP device")
4. An stdin read is fired in the background to read the OTP code (via
   prompt.Stdin)
5. User picks the security method, thus the stdin read is "abandoned"

In most cases this is fine, as the program ends right after. The issue is when a
relogin is triggered by a long living tsh invocation (again, `tsh ssh ...`): in
this case the stdin hijack causes input to be swallowed.

Forcing a single MFA option avoids the potential stdin hijack, fixing the
problem for all relogin invocations. `tsh login` behavior remains the same.

Note that we have to default to cluster's most secure method _without_ checking
the user devices. The user is not logged in yet, thus the backend cannot reveal
any information about that user.

Issue #11709.
@codingllama codingllama force-pushed the codingllama/v8-hungry-hungry-tsh branch from 71fa523 to c0b115b Compare April 11, 2022 14:40
@codingllama codingllama enabled auto-merge (squash) April 11, 2022 14:41
@codingllama codingllama merged commit 387d19f into branch/v8 Apr 11, 2022
@codingllama codingllama deleted the codingllama/v8-hungry-hungry-tsh branch April 11, 2022 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants