Skip to content

Reduce time spent setting ssh session envs#23731

Merged
rosstimothy merged 5 commits intomasterfrom
tross/ssh_envs
Mar 30, 2023
Merged

Reduce time spent setting ssh session envs#23731
rosstimothy merged 5 commits intomasterfrom
tross/ssh_envs

Conversation

@rosstimothy
Copy link
Copy Markdown
Contributor

@rosstimothy rosstimothy commented Mar 28, 2023

tsh sets a number of environment variables when setting up the users session. Each key value pair is transmitted one at a time in a "env" ssh request, which adds a number of envs * RTT of additional latency per session.

This introduces a new envs@goteleport.com request which sets multiple environment variables in a single ssh request, which reduces the amount of time spent setting envs down to the RTT of a single ssh request. In order to ensure backward compat and interoperability with OpenSSH, if the server does not recognize the envs@goteleport.com request the ssh client will resort to sending individual "env" requests.

@rosstimothy rosstimothy force-pushed the tross/ssh_envs branch 2 times, most recently from 592b152 to c76c825 Compare March 28, 2023 21:56
@rosstimothy
Copy link
Copy Markdown
Contributor Author

traces captured from a customer
image

tsh v12.1.0
Screenshot 2023-03-29 at 9 55 13 AM

tsh from this branch
Screenshot 2023-03-29 at 9 55 47 AM

@rosstimothy rosstimothy marked this pull request as ready for review March 29, 2023 14:08
`tsh` sets a number of environment variables when setting up the
users session. Each key value pair is transmitted one at a time
in a "env" ssh request, which adds a num envs * RTT of additional
latency per session.

This introduces a new `envs@goteleport.com` request which sets
multiple environment variables in a single ssh request, which
reduces the amount of time spent setting envs down to the RTT of
a single ssh request. In order to ensure backward compat and
interoperability with OpenSSH, if the server does not recognize
the `envs@goteleport.com` request the ssh client will resort to
sending individual "env" requests.
Copy link
Copy Markdown
Contributor

@codingllama codingllama left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice optimization!

Comment thread api/observability/tracing/ssh/session.go Outdated
Comment thread api/observability/tracing/ssh/session.go Outdated
Comment thread api/observability/tracing/ssh/ssh.go
Comment thread api/observability/tracing/ssh/ssh.go Outdated
Comment thread api/observability/tracing/ssh/ssh.go Outdated
Comment thread api/observability/tracing/ssh/client_test.go Outdated
Comment thread api/observability/tracing/ssh/client_test.go
Comment thread lib/srv/forward/sshserver.go
Comment thread api/observability/tracing/ssh/client_test.go Outdated
Comment thread api/observability/tracing/ssh/client_test.go Outdated
@rosstimothy rosstimothy requested a review from codingllama March 29, 2023 20:40
Comment thread api/observability/tracing/ssh/client_test.go
Comment thread api/observability/tracing/ssh/client_test.go Outdated
Comment thread lib/srv/forward/sshserver.go
rosstimothy and others added 2 commits March 29, 2023 17:46
Co-authored-by: Alan Parra <alan.parra@goteleport.com>
Co-authored-by: Alan Parra <alan.parra@goteleport.com>
Copy link
Copy Markdown
Contributor

@marcoandredinis marcoandredinis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGMT

Just a question: does this optimization work for direct connections and for webui/connect ssh sessions?

@rosstimothy
Copy link
Copy Markdown
Contributor Author

LGMT

Just a question: does this optimization work for direct connections and for webui/connect ssh sessions?

Connect calls tsh ssh directly and the Web UI uses the client.TeleportClient to create a session, so they will benefit as long as the Teleport ssh_service understands the new envs@goteleport.com. Any older Teleport instances or OpenSSH instances will still use the standard env request to send variables one at a time.

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