Refactor hostname resolution for SSH connections via the WebUI #35773
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rosstimothy
added
the
no-changelog
rosstimothy
force-pushed
the
tross/resolve_hostname_locally
branch
6 times, most recently
from
821af64
to
650a804
Compare
rosstimothy
force-pushed
the
tross/resolve_hostname_locally
branch
from
650a804
to
8cede18
Compare
fspmarshall
approved these changes
Jan 3, 2024
zmb3
approved these changes
Jan 3, 2024
public-teleport-github-review-bot
bot
removed the request for review
from Tener
Previously all hostname resolution was performed by sending either
the hostname or server uuid to Auth in a GetSSHTargets request.
The returned information was then sent to the UI so that the console
tab can be updated to include `user@host`. While this worked, it
required a round trip to Auth that had to iterate through the entire
set of nodes to do the resolution. During a concurrent session test
that tried to spawn several thousand web sessions it was discovered
that this cause a massive spike in CPU on Auth.
There are two reasons that Auth was used to resolve the hostname.
1) The proxy doesn't perform RBAC on behalf of the user. Instead
the proxy has an Auth client with the users identity.
2) The UI used to provide an input box that users could manually
enter the target host.
Since we no longer allow open dialing and the input box to enter
a connection string manually has been removed we no longer need
to worry about the second case. To avoid the round trip to auth
we can use the local Proxy cache to look up the hostname if we
wait until AFTER the ssh connection to the target is established.
At this point we can be sure that the user does have access to
the target since the node allowed the connection.
The testing utilities used to mimic the web ui half of the websocket made assumptions that the session metadata would be the first message provided. However, now that hostname resolution occurs after the initial SSH connection has been established it is possible for other messages to be sent first. This caused some tests to fail and others to hang forever. The duplicated terminal logic from (WebSuite) makeTerminal and (testProxy makeTerminal has now consolidated into a single test terminal utility. The new `terminal` now wraps a TerminalStream which allows test code to make use of the message processing logic already in place there instead of having to write custom logic per test. All a test needs to do is provide handlers for any messages that it desires to introspect.
rosstimothy
force-pushed
the
tross/resolve_hostname_locally
branch
from
8cede18
to
c4df6d1
Compare
|
/excludeflake * |
Envek
added a commit
to Envek/teleport
that referenced
this pull request
Jan 4, 2024
…se-anon-key * origin/master: (344 commits) Undelete CreateHostUserMode_HOST_USER_MODE_DROP (gravitational#36273) allow cwd to be changed in difftest (gravitational#35946) Auth device list component (gravitational#36235) make unified resources responsive (gravitational#35961) Support running Teleport in a "hot reload" mode (gravitational#35040) Prevent deleting enum values, allow deleting enum reservations in types.proto (gravitational#36248) Remove support for legacy (Amazon Linux 2) AMIs (gravitational#36153) Bump version(s) used for teleport-lab and teleport-quickstart (gravitational#36167) Allow Reconciler update handler to examine old value during update (gravitational#36171) Validate the user still exists during account reset (gravitational#35676) ButtonTextWithAddIcon shared component (gravitational#36103) Refactor hostname resolution for SSH connections via the WebUI (gravitational#35773) add structuredClone to jest JSDOMEnvironment (gravitational#36213) fix flaky `lib/auth` cache-enabled tests (gravitational#36216) Report resource usage counts by handling heartbeat events (gravitational#35968) Reviewer bot should use the stable version of Go (gravitational#36242) RFD 0153 Resource Guidelines (gravitational#34103) Use cmp and cmpots properly in operator tests (gravitational#36215) Relax Kubernetes CRD discovery when building cache (gravitational#36214) Add Access List messages to TAG protobuf (gravitational#36176) ...
rosstimothy
added a commit
that referenced
this pull request
Feb 16, 2024
The changes to hostname resolution introduced in #35773 were always using the root cache to lookup a matching node, which is never going to find a match if the target is in the leaf cluster. This is now resolved by using the `services.NodeWatcher` of the cluster that the target node is a member of.
rosstimothy
added a commit
that referenced
this pull request
Feb 16, 2024
The changes to hostname resolution introduced in #35773 were always using the root cache to lookup a matching node, which is never going to find a match if the target is in the leaf cluster. This is now resolved by using the `services.NodeWatcher` of the cluster that the target node is a member of.
rosstimothy
added a commit
that referenced
this pull request
Feb 16, 2024
The changes to hostname resolution introduced in #35773 were always using the root cache to lookup a matching node, which is never going to find a match if the target is in the leaf cluster. This is now resolved by using the `services.NodeWatcher` of the cluster that the target node is a member of.
github-merge-queue bot
pushed a commit
that referenced
this pull request
Feb 20, 2024
The changes to hostname resolution introduced in #35773 were always using the root cache to lookup a matching node, which is never going to find a match if the target is in the leaf cluster. This is now resolved by using the `services.NodeWatcher` of the cluster that the target node is a member of.
github-actions bot
pushed a commit
that referenced
this pull request
Feb 20, 2024
The changes to hostname resolution introduced in #35773 were always using the root cache to lookup a matching node, which is never going to find a match if the target is in the leaf cluster. This is now resolved by using the `services.NodeWatcher` of the cluster that the target node is a member of.
github-merge-queue bot
pushed a commit
that referenced
this pull request
Feb 27, 2024
The changes to hostname resolution introduced in #35773 were always using the root cache to lookup a matching node, which is never going to find a match if the target is in the leaf cluster. This is now resolved by using the `services.NodeWatcher` of the cluster that the target node is a member of.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously all hostname resolution was performed by sending either the hostname or server uuid to Auth in a GetSSHTargets request. The returned information was then sent to the UI so that the console tab can be updated to include
user@host. While this worked, it required a round trip to Auth that had to iterate through the entire set of nodes to do the resolution. During a concurrent session test that tried to spawn several thousand web sessions it was discovered that this causes a massive spike in CPU on Auth.There are two reasons that Auth was used to resolve the hostname.
Since we no longer allow open dialing and the input box to enter a connection string manually has been removed we no longer need to worry about the second case. To avoid the round trip to Auth we can use the local Proxy cache to look up the hostname if we wait until AFTER the ssh connection to the target is established. At this point we can be sure that the user does have access to the target since the node allowed the connection.