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

[v12] Attempt ssh connections with and without mfa at the same time #24371

Merged
merged 1 commit into from Apr 13, 2023

Conversation

rosstimothy
Copy link
Contributor

@rosstimothy rosstimothy commented Apr 11, 2023

Backport #23865 to branch/v12

Also backports #24400 and part of #24195 to prevent races during tests caused by the simultaneous connection attempts.

@rosstimothy rosstimothy force-pushed the tross/backport-23865/v12 branch 3 times, most recently from db5543a to 60966cc Compare April 12, 2023 17:13
`tsh ssh` would fallback to doing the mfa ceremony if connecting
to the node with the already provisioned certificates failed with
an access denied error. This incurs the cost of a round trip to
the target host when per session mfa is required. To combat the
additional latency when per session mfa is required we can
attempt both the connection with the certs on hand AND start the
per session mfa flow at the same time. If per session mfa is not
required the client won't attempt the mfa ceremony which adds no
impact there. If per session mfa is required the initial connection
to the host is going to fail so the mfa ceremony will need to be
performed any how.

For this to work we need to ensure that users are not prompted for
mfa if completing the mfa ceremony will not actually help the user
gain access to the host. If users just flat out do not have access
to the host we don't want to confuse them by prompting them to
touch a hardware key. Since `tsh` first calls
`proto.AuthService/IsMFARequired` before initiating the mfa ceremony
we are guaranteed not to initiate the mfa ceremony when not required.
@rosstimothy rosstimothy marked this pull request as ready for review April 12, 2023 20:57
@public-teleport-github-review-bot public-teleport-github-review-bot bot removed the request for review from greedy52 April 13, 2023 09:21
@rosstimothy rosstimothy added this pull request to the merge queue Apr 13, 2023
Merged via the queue into branch/v12 with commit 4707f27 Apr 13, 2023
18 checks passed
@rosstimothy rosstimothy deleted the tross/backport-23865/v12 branch April 13, 2023 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants