tsh ssh when authenticating eats the first two keypresses
#11709
Assignees
Comments
zmb3
added
the
tsh
|
@codingllama I wonder if any of your recent changes to the OTP/MFA prompts resolve this? |
|
@zmb3, it's probably worth a shot, but this looks like a lingering symptom of the stdin hijacking, which still has to happen. My guess is that the ContextReader thread/goroutine lingers a bit and "eats" a couple key presses before the system shuts it down. Relevant PRs: #11436 and #11346. I didn't backport them to v9. #11436 would be a clean cherry pick, but #11346 not so much. |
|
Worth noting that this doesn't seem to be a time-based race condition -- it doesn't matter if I wait seconds or minutes after the prompt appears. Let me know if there's anything else I can do to help debug! The output from |
This is interesting info, thanks for sharing. I'll try to get a quick repro just to see if the commits mentioned above have an effect on it - if they do, then the path is clear, otherwise we may need to get someone to take a closer look. |
|
I can confirm repros for Teleport 8 and 9. I haven't gone further down, but any version that has U2F and ContextReader/prompt.Stdin has this issue. The issue is the stdin hijack from PromptMFAChallenge, which only happens if the user has both OTP and U2F/WebAuthn devices registered. It's also made worse by the PRs above, because of the hijack via term.ReadPassword (I lose about every other keystroke forever). A simple mitigation, like asking the user to press enter, works for master/v10 because we can then "reclaim" the lost read (it has to be explicitly coded in, it won't just work otherwise). This works on master because ContextReader doesn't loop eagerly anymore. It's not clear to me why earlier versions only eat two chars, but it is an interesting detail. Broadly, I'd say we are looking at two possible "buckets" of fixes:
In terms of UX changes, a reasonable change would be to ask the user to choose between OTP or MFA beforehand. This makes it easy to ensure we won't fire a read that may be ignored. A possible optimization for v10/master is to default to MFA if we detect a device capable of doing the ceremony right away (this could be nice regardless, but doesn't eliminate the issue). As for clever hacks, I haven't found anything good yet. A possibility is running login in a separate process, but that feels like a big bump in complexity for tsh. |
I generally use the U2F key, but if I use the OTP entry, it only eats one keypress. Probably unrelated, but I notice that if I use the OTP, the light on the Yubikey keeps blinking. And with |
|
Yes, the blinking is something else. Using master they stop blinking after a short while for me, even when compiled with the legacy U2F code. The "hid: not permitted" errors are red herrings, they just happen sometimes (but ideally not after you authenticate). I may take a look at those, but I'll focus on the main issue first. |
codingllama
added a commit
that referenced
this issue
Apr 8, 2022
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.
Fixes #11709.
* Add UseStrongestAuth flag to PromptMFAChallenge
* Add TeleportClient.UseStrongestAuth and set it true for relogin
* Proper testing
* Address review comments
codingllama
added a commit
that referenced
this issue
Apr 8, 2022
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.
Fixes #11709.
* Add UseStrongestAuth flag to PromptMFAChallenge
* Add TeleportClient.UseStrongestAuth and set it true for relogin
* Proper testing
* Address review comments
codingllama
added a commit
that referenced
this issue
Apr 8, 2022
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
added a commit
that referenced
this issue
Apr 11, 2022
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
added a commit
that referenced
this issue
Apr 11, 2022
…1848) 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. * Make relogin attempts use the strongest auth method (#11781) * Fix conflicts for v8
codingllama
added a commit
that referenced
this issue
Apr 11, 2022
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.
Fixes #11709.
* Add UseStrongestAuth flag to PromptMFAChallenge
* Add TeleportClient.UseStrongestAuth and set it true for relogin
* Proper testing
* Address review comments
codingllama
added a commit
that referenced
this issue
Apr 11, 2022
…1847) 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. * Make relogin attempts use the strongest auth method (#11781) * Fix conflicts for v9
|
Hey @alexmv, this is now fixed on master and backported to v8 and v9 - whenever we get a new release it should include the fix. The fix is essentially a UX change: we avoid the bug on |
|
Thanks for the update, and the quick fix! That seems like a totally fine compromise to me -- looking forward to 9.0.5. |
This was referenced May 16, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Description
What happened:
When not currently authenticated, either via explicit
tsh logout, or auth having expired, the firsttsh sshconnection successfully re-auths, but consumes the first two keypresses of input to the host. Here's me connecting, pasting in my password, pressing my Yubikey, and then typing "12345" once I see the remote host's prompt appear:It's not just visible characters; if I wait until the
username@hostname:~$prompt appears, I have to press Ctrl-D three times before it closes the connection.What you expected to happen: Once the remote prompt appears, all input should get sent to the remote server.
Reproduction Steps
As minimally and precisely as possible, describe step-by-step how to reproduce the problem.
tsh logouttsh ssh --proxy teleport.example.net --user alexmv -l username --debug hostname.example.netServer Details
teleport version):Teleport v9.0.2 git:v9.0.2-0-g354b8c037 go1.17.7/etc/os-release):Ubuntu 20.04.4 LTS (Focal Fossa)Client Details
tsh version):Teleport v9.0.3 git: go1.18The text was updated successfully, but these errors were encountered: