Remove CLI-based config options from Teleport Connect MFA prompts #47875
Conversation
Joerger
changed the title
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
Joerger
added
the
no-changelog
Joerger
force-pushed
the
joerger/cli-prompt-refactor
branch
from
a85d056 to
c8d3827
Compare
Joerger
force-pushed
the
joerger/fix-otp-teleport-connect
branch
from
e32130b to
31b86bc
Compare
Joerger
force-pushed
the
joerger/fix-otp-teleport-connect
branch
from
31b86bc to
33bf6b7
Compare
There was a problem hiding this comment.
I was about to forward #46820 to you and that's how I found this PR.
I don't think this is going to affect #46820 that much nor bring it closer to completion. I assume #46820 is purely about SSH as we show the regular MFA prompt for other resources.
When you SSH to a node, the Electron app executes tsh ssh underneath, without going through the tsh daemon. That tsh ssh process has no knowledge about the Electron app. Making it Electron-aware would likely require too much effort.
I think this could be solved in two ways. tsh could show some kind of prompt letting you choose which auth method you want to use. Historically, it seems that we eschewed that in favor of supplying a specific flag if someone wants to use OTP.
The other way to solve this would be to add a new config option to Connect (ssh.mfaMode?) that adds an appropriate flag to all tsh ssh invocations coming from Connect. The user wouldn't be able to set this flag per-resource or even per profile, but at least it'd unblock users who are not able to use OTP at the moment.
There was a problem hiding this comment.
What do we think about replacing the terminal-based mfa prompt for SSH with the custom connect MFA prompt? It should be possible to inject the custom prompt in there, but I need to check.
There was a problem hiding this comment.
I tried to explain why that's not hard to do in the third paragraph. ;) The MFA prompt for every other resource kind goes through tsh daemon, e.g., the Electron app tells tshd to create a local proxy, tshd attempts to generate a cert, lib/client code prompts for an MFA check and tshd forwards this to the Electron app.
SSH connections do not go through the tsh daemon, the Electron app just executes tsh ssh.
If there's some other way of doing it without forcing tsh ssh to be Electron-aware then that's something we can explore.
There was a problem hiding this comment.
tsh could show some kind of prompt letting you choose which auth method you want to use.
IIUC this is not something we want to do, as it requires us to set AllowStdinHijack=true so that tsh can accept input. We'd need to ask Alan for more details on what issues that can cause.
If there's some other way of doing it without forcing tsh ssh to be Electron-aware then that's something we can explore.
Regardless I think we want this to start with an Electron dialog window if possible, it's cleaner than trying to add a config option or prompt for stdin.
The first idea to come to mind is to add a new tshd request to check if MFA is required before running the tsh ssh command. If MFA is required, we could see what options are available to them (Webauthn, totp, sso) and present them with a diagog to choose one, e.g. by clicking one of 3 buttons. Then depending on what they click, we could run tsh ssh --mfa-mode=sso/otp/webauthn and let the cli prompt continue as normal. WDYT?
There was a problem hiding this comment.
This would be fine UX-wise, but has the unfortunate effect of degrading performance for people not using per-session MFA, which is something we've strived to avoid. I remember this being the case when Michael was working on MFA in file transfers in the Web UI, for example.
There was a problem hiding this comment.
Fair point. In that case we should let tsh ssh do it's thing, racing an MFA and non-MFA session and only prompting when needed.
Outside of making a whole new ssh for Connect implementation that utilizes tshd <-> Electron communication, we could experiment with other ways of having tshd/Electron communicate with tsh ssh.
For example, similar to our Teleport re-exec logic, we could pass a file descriptor to tsh ssh to handle back and forth communication with the Electron App. Over this file descriptor, tsh ssh could send a generic MFA prompt request when needed, just like to the request from tshd.
Or if a file descriptor isn't a viable solution (I don't know javascript limitations well), we could use a unix/tcp socket instead. Or maybe we could plug into the tshd socket from tsh ssh. e.g. tsh ssh --tshd=/path/to/socket.
Remove CLI-based config options from the Teleport Connect MFA prompts. This was previously preventing OTP in cases where it could be allowed.
Related: #46820 - if we replace the terminal prompt with the custom teleport connect prompt, totp would work now.
Depends on #47874