Skip to content

Update app auth config session ID generation to include a prefix#64457

Draft
gabrielcorado wants to merge 2 commits intomasterfrom
gabrielcorado/push-nypwnmzzuyrw
Draft

Update app auth config session ID generation to include a prefix#64457
gabrielcorado wants to merge 2 commits intomasterfrom
gabrielcorado/push-nypwnmzzuyrw

Conversation

@gabrielcorado
Copy link
Copy Markdown
Contributor

Raised in this thread.

With the addition of app auth config sessions, we can now generate session IDs from other values (using SHA256) instead of having them generated using a "crypto-strong pseudo-random generator". This change increased the chances of conflicting session IDs. Although the chances are still small, a conflict would cause the session to be overwritten, and existing clients would access a different session.

Given this, in this PR, we add a non-hex prefix to app auth config sessions, eliminating the chance of conflicting with regular app sessions.

IMPORTANT: The regular app session IDs remain unchanged, and this affects only sessions using app auth config.

Manual Test Plan

Test Environment

The environment requires using the branch from #62975, which introduces MCP sessions using app auth config. The test environment used was the same as the other PR; you can check there for more details.

With that setup, we need to generate a few (tested with 5) sessions (by starting the MCP client).

Test Cases

For each audit storage backend:

  • PostgreSQL (pgevents)
  • Athena (athena)
  • Firestore (firestoreevents)
  • DyanamoDB (dynamoevents)
  • Filelog (filelog)

The following was tested:

  • WebUI list audit events
  • WebUI list session recordings
  • WebUI watch SSH session recording
  • tsh recordings ls
  • tsh play ssh-session-id

Although programmatically those other parts are not affected, I also tested them since touch audit events:

  • Events handler export to Fluentd (doesn't unmarshal/inspect app sessions IDs, but it is on the events).
  • tsh play -f json chunk-id (doesn't directly use the session ID, but has it on the events)

@gabrielcorado gabrielcorado added the no-changelog Indicates that a PR does not require a changelog entry label Mar 10, 2026
@github-actions github-actions bot requested a review from kshi36 March 10, 2026 02:42
@github-actions github-actions bot added audit-log Issues related to Teleports Audit Log size/sm labels Mar 10, 2026
// appAuthConfigSessionIDPrefix is a prefix added to app auth config session
// IDs. The prefix includes a non-hex character ("u") so that it will never
// conflict with regular app session ID (which are hex values).
const appAuthConfigSessionIDPrefix = "auth"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The prefix includes a non-hex character ("u")

interesting. so his allows us to keep the same length? any other alternative name than auth?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, we keep the same length. We could use anything that is non-hex, including a single character (for example, zXXXX). I've left it as auth so we have both a descriptive indicator and also avoid conflicts with regular app sessions, but I'm open to suggestions.

@gabrielcorado gabrielcorado marked this pull request as draft March 11, 2026 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

audit-log Issues related to Teleports Audit Log 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.

2 participants