Skip to content

Switching recording encryption to unwrap keys using direct keystore RSA decryption#56776

Merged
eriktate merged 2 commits intomasterfrom
eriktate/rsa-recording-encryption
Jul 23, 2025
Merged

Switching recording encryption to unwrap keys using direct keystore RSA decryption#56776
eriktate merged 2 commits intomasterfrom
eriktate/rsa-recording-encryption

Conversation

@eriktate
Copy link
Copy Markdown
Contributor

@eriktate eriktate commented Jul 14, 2025

This PR removes the intermediate software X25519 keys present in the current encrypted recording implementation in favor of using hardware/keystore RSA keys directly. In order to make this work without adding new complexity around key exchange, encrypted recordings now assume that auth servers in the same cluster should have access to the same keys. This also means the vast majority of the cooperative key management has been replaced with simpler checks that a key is both provisioned and accessible.

There will be a follow-up PR implementing the manual key management flow now defined in the RFD.

@eriktate eriktate force-pushed the eriktate/rsa-recording-encryption branch 3 times, most recently from 4a27048 to 4a16d38 Compare July 15, 2025 21:06
@eriktate eriktate changed the title Switched recording encryption to unwrap keys using direct keystore RSA decryption Switching recording encryption to unwrap keys using direct keystore RSA decryption Jul 15, 2025
@eriktate eriktate force-pushed the eriktate/rsa-recording-encryption branch from 4a16d38 to 65d37b1 Compare July 15, 2025 21:20
@eriktate eriktate marked this pull request as ready for review July 15, 2025 21:21
@github-actions github-actions Bot requested review from camscale and nklaassen July 15, 2025 21:21
Copy link
Copy Markdown
Contributor

@nklaassen nklaassen left a comment

Choose a reason for hiding this comment

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

LGTM

Comment thread api/constants/constants.go Outdated
Comment thread lib/auth/keystore/software.go Outdated

decrypter := key
if alg == cryptosuites.RSA2048 {
if alg == cryptosuites.RSA4096 {
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.

Should this check be for RSA2048 and RSA4096? Is RSA2048 never used anymore?

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.

For decryption it's only RSA4096 but the GenerateDecrypterWithAlgorithm function will actually error if it's any other algo so this check is totally unnecessary.

Comment thread lib/auth/recordingencryption/manager_test.go Outdated
Comment thread lib/cryptosuites/internal/rsa/rsa.go
@eriktate eriktate force-pushed the eriktate/rsa-recording-encryption branch 2 times, most recently from 4c6f773 to cd87b3e Compare July 21, 2025 22:11
@eriktate eriktate added the no-changelog Indicates that a PR does not require a changelog entry label Jul 21, 2025
@eriktate eriktate force-pushed the eriktate/rsa-recording-encryption branch from cd87b3e to 6044e3f Compare July 22, 2025 19:59
@eriktate eriktate force-pushed the eriktate/rsa-recording-encryption branch from 17e0aab to ced2f23 Compare July 23, 2025 13:00
@public-teleport-github-review-bot public-teleport-github-review-bot Bot removed the request for review from camscale July 23, 2025 13:45
@eriktate eriktate added this pull request to the merge queue Jul 23, 2025
Merged via the queue into master with commit f56d076 Jul 23, 2025
40 checks passed
@eriktate eriktate deleted the eriktate/rsa-recording-encryption branch July 23, 2025 15:43
eriktate added a commit that referenced this pull request Aug 15, 2025
eriktate added a commit that referenced this pull request Aug 27, 2025
eriktate added a commit that referenced this pull request Sep 2, 2025
eriktate added a commit that referenced this pull request Sep 2, 2025
eriktate added a commit that referenced this pull request Sep 2, 2025
eriktate added a commit that referenced this pull request Sep 3, 2025
github-merge-queue Bot pushed a commit that referenced this pull request Sep 3, 2025
* adding support for encryption/decryption keys to keystore manager (#54428, #55652)

* adds new protos for resources related to encrypted session recordings and updates the existing SessionRecordingConfig protos to include a Status (#54780)

* adding local service implementation for recording encryption resources (#54816)

* adding Manager for RecordingEncryption resources that handles shared ops more complex than CRUD (#55078)

* Adding session recording plugin for `age` (#55120)

* adding Manager for RecordingEncryption resources that handles shared ops more complex than CRUD

* adding age plugin wrapping default X25519 Identity/Recipient implementation with hooks to more efficiently lookup private keys given their respective public key

* Adding recording encryption and playback for `sync` modes (#54901)

* adding cache for RecordingEncryption (#55857)

* adding recording_encryption service protos (#55121)

* adding async recording encryption with gRPC multipart uploader (#55859)

* adding file configuration for encrypted session recording (#56200)

* Switching recording encryption to unwrap keys using direct keystore RSA decryption (#56776)

* adding manual key management config (#56920)

* updating protos for recording encryption (#57055)

* Add missing handling for recording encryption configs and keys (#57279)

* updating protos for recording encryption

* changing labels for encryption keys to prevent automatic cleanup, adjusting pkcs11 host UUID check to allow for key sharing of encryption keys, preventing cloud tenants from enabling manual key management, preventing use of recording encryption in FIPS mode

* adding new protos for rotated keys and the local service for interacting (#57576)

with them

* Switching encryption keys from PEM to ASN.1 DER encoding (#58137)

* using pregenerated RSA4096 key for keystore tests because generation is too slow (#58138)

* extending precomputed RSA keys to support 4096-bit keys (#58251)

* adding rotation process to Manager and exposing with new RPCs and (#57577)

* adding rotation sub commands for recording encryption keys and fixing (#57780)

broken session_recording_config when using fileconf

* using more reliable method of validating key bit length
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/sm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants