Skip to content

Fix discrepancies between Node and Proxy recording modes.#58707

Merged
Joerger merged 8 commits intomasterfrom
joerger/proxy-rec-server-id
Sep 24, 2025
Merged

Fix discrepancies between Node and Proxy recording modes.#58707
Joerger merged 8 commits intomasterfrom
joerger/proxy-rec-server-id

Conversation

@Joerger
Copy link
Copy Markdown
Contributor

@Joerger Joerger commented Sep 3, 2025

Changes:

  • Remove the unique ID from the ephemeral forwarding server, which provided no benefit. Instead, the ID of the server will be the Node ID. Combined with the forwarded_by: <proxy_id> field in events, it will be clear that the actual component being connected to is the ephemeral forwarding server for that node session.
  • Rename the forwarding server field hostUUID to proxyUUID for clarity. HostUUID() still returns the same proxy uuid.
  • Update the forwarding server GetInfo method to return the full end server info, including labels, subkind, etc.
    • Build event metadata from the server info from GetInfo. This eliminates some of the discrepancies I found between the events and the actual server info, and between the forwarding vs. node recording events.

Adapted from #52920

Depends on #58700

Closes #4146

Diff between Node recordings before and after this PR:

{
  "addr.remote": "127.0.0.1:52234",
  "cluster_name": "root.example.com",
  "code": "T2000I",
  "ei": 0,
  "event": "session.start",
  "initial_command": [
    ""
  ],
  "login": "bjoerger",
  "namespace": "default",
  "private_key_policy": "none",
  "proto": "ssh",
  "server_hostname": "node",
  "server_id": "978e143e-6271-4ff0-8344-3d71528615b1",
  "server_labels": {
    "arch": "arm",
    "cluster": "root.example.com",
    "env": "production",
    "hostname": "Brians-MacBook-Pro-2.local"
  },
+ "server_sub_kind": "teleport",
  "server_version": "19.0.0-dev",
  "session_recording": "node",
  "sid": "7f3da66e-eab4-437f-bc96-65cf40363d18",
  "size": "167:23",
  "time": "2025-09-03T21:29:50.374Z",
  "uid": "0ae3ecae-1f57-4436-8493-60a24870a932",
  "user": "dev",
  "user_kind": 1
}

Diff between Proxy recordings before and after this PR:

{
  "addr.remote": "127.0.0.1:52863",
  "cluster_name": "root.example.com",
  "code": "T2000I",
  "ei": 0,
  "event": "session.start",
  "forwarded_by": "7dae42bb-6401-405f-b683-efa44d57e484",
  "initial_command": [
    ""
  ],
  "login": "bjoerger",
  "namespace": "default",
  "private_key_policy": "none",
  "proto": "ssh",
- "server_addr": "@local-node",
  "server_hostname": "node",
- "server_id": "7f4ce963-f184-4fe7-a0fe-13f35a7d9e42.root.example.com",
+ "server_id": "7f4ce963-f184-4fe7-a0fe-13f35a7d9e42",
+ "server_labels": {
+   "arch": "arm",
+   "cluster": "root.example.com",
+   "env": "production",
+   "hostname": "Brians-MacBook-Pro-2.local"
+ },
  "server_sub_kind": "teleport",
  "server_version": "19.0.0-dev",
  "session_recording": "proxy",
  "sid": "00e55c91-7bd3-48e7-96c4-30e17d78da2f",
  "size": "167:23",
  "time": "2025-09-03T21:47:12.505Z",
  "uid": "995f48d4-684c-4d72-bfa8-cd52ac00bab4",
  "user": "dev",
  "user_kind": 1
}

Diff between Node and Proxy recordings after this PR:

{
  "addr.remote": "127.0.0.1:52562",
  "cluster_name": "root.example.com",
  "code": "T2000I",
  "ei": 0,
  "event": "session.start",
+ "forwarded_by": "1d3f9f84-3f63-4267-be51-0fae1e2fe4b0",
  "initial_command": [
    ""
  ],
  "login": "bjoerger",
  "namespace": "default",
  "private_key_policy": "none",
  "proto": "ssh",
  "server_hostname": "node",
  "server_id": "27a558dd-2d35-407c-bfcc-3333dca34b48",
  "server_labels": {
    "arch": "arm",
    "cluster": "root.example.com",
    "env": "production",
    "hostname": "Brians-MacBook-Pro-2.local"
  },
  "server_sub_kind": "teleport",
  "server_version": "19.0.0-dev",
- "session_recording": "node",
+ "session_recording": "proxy",
  "sid": "3a1c8d5b-9719-4df2-ac6a-12191062d8eb",
  "size": "167:23",
  "time": "2025-09-03T21:34:45.056Z",
  "uid": "9dd08c52-7f68-4a03-b827-bb3230e76c43",
  "user": "dev",
  "user_kind": 1
}

Diff between Node and Proxy recordings after this PR, connecting to node w/o tunnel mode:

{
- "addr.local": "127.0.0.1:3080",
+ "addr.local": "127.0.0.1:3022",
  "addr.remote": "127.0.0.1:53139",
  "cluster_name": "root.example.com",
  "code": "T2000I",
  "ei": 0,
  "event": "session.start",
+ "forwarded_by": "7dae42bb-6401-405f-b683-efa44d57e484",
  "initial_command": [
    ""
  ],
  "login": "bjoerger",
  "namespace": "default",
  "private_key_policy": "none",
  "proto": "ssh",
- "server_addr": "[::]:3022",
+ "server_addr": "127.0.0.1:3022",
  "server_hostname": "server01",
  "server_id": "7dae42bb-6401-405f-b683-efa44d57e484",
  "server_labels": {
    "arch": "arm",
    "cluster": "root.example.com",
    "env": "staging",
    "hostname": "Brians-MacBook-Pro-2.local",
    "idp_1": "access_teleport_template",
    "oidc-client-id": "dev"
  },
  "server_sub_kind": "teleport",
  "server_version": "19.0.0-dev",
- "session_recording": "node",
+ "session_recording": "proxy",
  "sid": "bd6a3988-8f28-40a8-a4fd-464382e11a42",
  "size": "167:33",
  "time": "2025-09-03T22:03:14.344Z",
  "uid": "6dfa4428-09d3-405f-827b-794efb5ec6e0",
  "user": "dev",
  "user_kind": 1
}

Comment thread integration/integration_test.go Outdated
Comment thread integration/integration_test.go Outdated
Comment thread integration/integration_test.go
Comment thread lib/proxy/router.go Outdated
Comment thread lib/proxy/router.go Outdated
@Joerger Joerger changed the title Joerger/proxy rec server Fix discrepancies between Node and Proxy recording modes. Sep 5, 2025
@Joerger Joerger force-pushed the joerger/proxy-rec-server-id branch from 24e22ed to 47255ac Compare September 6, 2025 00:28
Comment thread lib/srv/ctx.go Outdated
Comment thread lib/srv/forward/sshserver.go
@Joerger Joerger requested review from rosstimothy and zmb3 September 9, 2025 00:34
@Joerger Joerger force-pushed the joerger/require-target-server branch from dff14c1 to 186dd57 Compare September 9, 2025 17:35
@Joerger Joerger force-pushed the joerger/proxy-rec-server-id branch 2 times, most recently from 0f698ac to 04eac02 Compare September 9, 2025 20:16
Comment thread integration/integration_test.go Outdated
Base automatically changed from joerger/require-target-server to master September 9, 2025 23:58
@Joerger Joerger force-pushed the joerger/proxy-rec-server-id branch 4 times, most recently from 73b6018 to 27e6594 Compare September 12, 2025 22:32
@Joerger
Copy link
Copy Markdown
Contributor Author

Joerger commented Sep 16, 2025

@doggydogworld Friendly ping to review

@Joerger
Copy link
Copy Markdown
Contributor Author

Joerger commented Sep 17, 2025

Reopened... this got automatically closed due to a faulty rebase (git machete) that reset the branch to master.

@Joerger Joerger force-pushed the joerger/proxy-rec-server-id branch from 06618f0 to 9dd1cec Compare September 17, 2025 21:47
Joerger and others added 5 commits September 17, 2025 15:50
Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>
Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>
@Joerger Joerger force-pushed the joerger/proxy-rec-server-id branch from e2a9318 to 565b395 Compare September 17, 2025 22:50
@Joerger
Copy link
Copy Markdown
Contributor Author

Joerger commented Sep 23, 2025

@doggydogworld @zmb3 Friendly ping to review

@Joerger Joerger added this pull request to the merge queue Sep 23, 2025
Merged via the queue into master with commit 64f6154 Sep 24, 2025
41 checks passed
@Joerger Joerger deleted the joerger/proxy-rec-server-id branch September 24, 2025 00:03
@backport-bot-workflows
Copy link
Copy Markdown
Contributor

@Joerger See the table below for backport results.

Branch Result
branch/v16 Failed
branch/v17 Failed
branch/v18 Failed

Joerger added a commit that referenced this pull request Sep 25, 2025
* Fix usage of node server ID and proxy host UUID in the forwarding server.

* Make GetInfo and TargetMetadata consistent between the forwarding and regular server.

* Rename TargetMetadata to EventMetadata and only use it for events' ServerMetadata.

* Remove unused method.

* Resolves comments on test.

* Remove unused fields.

* Update lib/srv/ctx.go

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>

* Update integration/integration_test.go

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

---------

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>
Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>
Joerger added a commit that referenced this pull request Sep 25, 2025
* Fix usage of node server ID and proxy host UUID in the forwarding server.

* Make GetInfo and TargetMetadata consistent between the forwarding and regular server.

* Rename TargetMetadata to EventMetadata and only use it for events' ServerMetadata.

* Remove unused method.

* Resolves comments on test.

* Remove unused fields.

* Update lib/srv/ctx.go

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>

* Update integration/integration_test.go

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

---------

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>
Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>
Joerger added a commit that referenced this pull request Sep 25, 2025
* Fix usage of node server ID and proxy host UUID in the forwarding server.

* Make GetInfo and TargetMetadata consistent between the forwarding and regular server.

* Rename TargetMetadata to EventMetadata and only use it for events' ServerMetadata.

* Remove unused method.

* Resolves comments on test.

* Remove unused fields.

* Update lib/srv/ctx.go

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>

* Update integration/integration_test.go

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

---------

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>
Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>
Joerger added a commit that referenced this pull request Sep 25, 2025
* Fix usage of node server ID and proxy host UUID in the forwarding server.

* Make GetInfo and TargetMetadata consistent between the forwarding and regular server.

* Rename TargetMetadata to EventMetadata and only use it for events' ServerMetadata.

* Remove unused method.

* Resolves comments on test.

* Remove unused fields.

* Update lib/srv/ctx.go

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>

* Update integration/integration_test.go

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

---------

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>
Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>
rhammonds-teleport pushed a commit that referenced this pull request Nov 6, 2025
* Fix usage of node server ID and proxy host UUID in the forwarding server.

* Make GetInfo and TargetMetadata consistent between the forwarding and regular server.

* Rename TargetMetadata to EventMetadata and only use it for events' ServerMetadata.

* Remove unused method.

* Resolves comments on test.

* Remove unused fields.

* Update lib/srv/ctx.go

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>

* Update integration/integration_test.go

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

---------

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>
Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>
@Joerger Joerger mentioned this pull request Nov 11, 2025
36 tasks
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

backport/branch/v18 no-changelog Indicates that a PR does not require a changelog entry size/sm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect Event Data, Proxy Recording Mode

3 participants