Skip to content

Make SSH session client provide session params upfront rather than with synchronous envs@goteleport.com requests#59206

Merged
Joerger merged 21 commits intomasterfrom
joerger/ssh-session-req-data
Oct 4, 2025
Merged

Make SSH session client provide session params upfront rather than with synchronous envs@goteleport.com requests#59206
Joerger merged 21 commits intomasterfrom
joerger/ssh-session-req-data

Conversation

@Joerger
Copy link
Copy Markdown
Contributor

@Joerger Joerger commented Sep 16, 2025

Currently, Teleport SSH session clients provide session params such as the join session ID, participant mode, and invited users via synchronous env var requests. These env var requests can occur anytime between the client's session and shell requests. As a result, these session params cannot be finalized and used by the server until the shell request.

In particular, this makes it difficult to sync the session ID between the the proxy and node in proxy recording mode, resulting in duplicate events.

This change adds session params passed as extra data in the session request so that session params can be finalized upfront and used/referenced before the shell request. This will be used in a follow up PR to sync the session ID between the proxy and node in proxy recording mode to align the session recording ID with session events, tracker, etc, and avoid emitting duplicate events.

Depends on #59294, #59291

Related to #42263

@Joerger Joerger changed the title Make SSH session client provide session params upfront rather than with synchronous "envs@goteleport.com" requests Make SSH session client provide session params upfront rather than with synchronous envs@goteleport.com requests Sep 16, 2025
@Joerger Joerger force-pushed the joerger/ssh-session-req-data-base branch from 9cb1949 to 6d8ac3d Compare September 16, 2025 20:42
@Joerger Joerger force-pushed the joerger/ssh-session-req-data branch 3 times, most recently from 8ddc2e8 to 4c30199 Compare September 17, 2025 22:52
@Joerger Joerger changed the base branch from joerger/ssh-session-req-data-base to joerger/ssh-session-client September 17, 2025 22:52
@Joerger Joerger force-pushed the joerger/ssh-session-req-data branch from 646158b to 9b57f2a Compare September 19, 2025 19:14
@Joerger Joerger changed the base branch from joerger/ssh-session-client to joerger/fix-connection-session-id September 19, 2025 23:41
@Joerger Joerger changed the base branch from joerger/fix-connection-session-id to joerger/ssh-session-req-data-base September 19, 2025 23:42
@Joerger Joerger force-pushed the joerger/ssh-session-req-data branch from 9b57f2a to d7c7798 Compare September 19, 2025 23:55
@Joerger Joerger force-pushed the joerger/ssh-session-req-data-base branch from c26b846 to ce8c7fe Compare September 22, 2025 20:19
@Joerger Joerger force-pushed the joerger/ssh-session-req-data branch 2 times, most recently from 8f92ec5 to 7725fa9 Compare September 23, 2025 01:10
@Joerger Joerger changed the base branch from joerger/ssh-session-req-data-base to joerger/ssh-session-client September 23, 2025 01:16
@Joerger Joerger force-pushed the joerger/ssh-session-req-data branch from 7725fa9 to 2d22b1b Compare September 24, 2025 00:08
@Joerger Joerger force-pushed the joerger/ssh-session-client branch from b9c06c3 to 9816928 Compare September 24, 2025 00:14
@Joerger Joerger force-pushed the joerger/ssh-session-req-data branch 2 times, most recently from e55ac1f to dd9e5b0 Compare September 25, 2025 16:42
@Joerger Joerger marked this pull request as ready for review September 25, 2025 20:28
@Joerger Joerger added the no-changelog Indicates that a PR does not require a changelog entry label Sep 29, 2025
@Joerger Joerger force-pushed the joerger/ssh-session-client branch from 9816928 to b6d1e16 Compare September 29, 2025 17:21
@Joerger Joerger force-pushed the joerger/ssh-session-req-data branch 2 times, most recently from bf71f6b to 6bec451 Compare September 29, 2025 17:49
@Joerger Joerger requested a review from rosstimothy September 30, 2025 17:03
@Joerger Joerger force-pushed the joerger/ssh-session-req-data branch from ba8a678 to f7a7568 Compare September 30, 2025 18:05
@Joerger Joerger force-pushed the joerger/ssh-session-req-data branch from 3f3fcf4 to aea16c5 Compare September 30, 2025 23:27
Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>
@Joerger
Copy link
Copy Markdown
Contributor Author

Joerger commented Oct 2, 2025

@vapopov Friendly ping to review.

@Joerger Joerger force-pushed the joerger/ssh-session-req-data branch from e9c3553 to 09bd801 Compare October 2, 2025 22:47
@Joerger Joerger added this pull request to the merge queue Oct 3, 2025
Merged via the queue into master with commit 4774000 Oct 4, 2025
41 checks passed
@Joerger Joerger deleted the joerger/ssh-session-req-data branch October 4, 2025 00:05
rhammonds-teleport pushed a commit that referenced this pull request Nov 6, 2025
…th synchronous `envs@goteleport.com` requests (#59206)

* Client: add session channel request extra data to replace env vars.

* Server Changes:

- Set session params on server side from request, with env var fallback for backwards compatibility.

- Remove CreateOrJoinSession.

- Separate JoinSession from OpenSession; Remove session ID from connection context as it is now purely controlled by the session.

- Ensure participant mode is always set for joining.

* Replace NewSessionWithParams method.

* Update comments.

* Move session params to server context.

* Add ModeratedSessionID to session params to replace TELEPORT_MODERATED_SESSION_ID env var.

* Fix nil reference.

* Fix tests.

* Only send session params to Teleport servers.

* Add WebProxyAddr to session params.

* Cleanup.

* Add tests.

* Fix test.

* Address comments.

* Add NewSessionWithParams.

* Fix tests.

* Update comment.

* Update api/observability/tracing/ssh/client.go

Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>

* Fix forwarding server missing Teleport ssh version.

---------

Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>
Joerger added a commit that referenced this pull request Nov 11, 2025
…th synchronous `envs@goteleport.com` requests (#59206)
@Joerger Joerger mentioned this pull request Nov 11, 2025
36 tasks
Joerger added a commit that referenced this pull request Nov 17, 2025
…th synchronous `envs@goteleport.com` requests (#59206)
Joerger added a commit that referenced this pull request Nov 24, 2025
…th synchronous `envs@goteleport.com` requests (#59206)
github-merge-queue bot pushed a commit that referenced this pull request Dec 3, 2025
* Fix discrepancies between Node and Proxy recording modes. (#58707)

* Replace flaky test with more straightforward event metadata test. (#59610)

* Make SSH session client provide session params upfront rather than with synchronous `envs@goteleport.com` requests (#59206)

* Coordinate session ID with Node in Proxy recording mode (#59850)

* Generalize PrepareToReceiveSessionID.

* Initialize session ID in the connection context and update it from node current-session-id request.

* Add session-id-query-v2@goteleport.com request and ensure new session ID is correctly set in proxy recording mode during the channel request.

* Replace PrepareToReceiveSessionID with simpler in-place logic.

* Don't emit session events or tracker when proxy forwarding to a Teleport Node.

* Fix missing session tracker for outdated Teleport Node.

* Remove extra major version grace period.

* Update integration test.

* Cleanup current session ID handling and fix failing tests.

* Fix tests.

* Address comments.

* Restructure currentSessionID handling.

* Set newSessionID in test server context.

* Fix integration test.

* Fix AuditOn integration test.

* Address comment on channel close.

* Track session on forwarding node.

* Fix web shutdown.

* Fix nil pointer dereference in test.

* Fix test flake.

* Fix nil pointer in test.

* Fix test flake.

* Update lib/srv/ctx.go

Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>

* Forwarding Node accepts client connection after receiving preparing session ID from node. This way, the forwarder can reject client connections if there is an issue preparing the session ID (impossible join sessions).

* Remove check for session.data event which may not be emitted in time for the test.

* Address comments.

---------

Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>

---------

Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog Indicates that a PR does not require a changelog entry size/md

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants