Adding new protos for session recording encryption#54780
Merged
Conversation
eriktate
force-pushed
the
eriktate/encrypted-recording-protos
branch
5 times, most recently
from
02ae232 to
1148b4b
Compare
eriktate
force-pushed
the
eriktate/keystore-decryption
branch
from
c3b364d to
6a5041a
Compare
eriktate
force-pushed
the
eriktate/encrypted-recording-protos
branch
from
1148b4b to
245cb67
Compare
eriktate
force-pushed
the
eriktate/keystore-decryption
branch
from
6a5041a to
d4161b8
Compare
eriktate
force-pushed
the
eriktate/encrypted-recording-protos
branch
3 times, most recently
from
adfe856 to
32e26ff
Compare
marcoandredinis
approved these changes
May 26, 2025
| ]; | ||
| // Status is the SessionRecordingConfig status containing active encryption keys | ||
| SessionRecordingConfigStatus Status = 6 [ | ||
| (gogoproto.nullable) = false, |
Contributor
There was a problem hiding this comment.
RFD had nullable = true
| types.EncryptionKeyPair key_encryption_pair = 2; | ||
| // State represents what state a given WrappedKey is currently in. Primarily used | ||
| // to identify what phase of a rotaiton a given key is currently in. | ||
| KeyState state = 3; |
Contributor
There was a problem hiding this comment.
Using enums in proto brings some UX issues to the Teleport terraform provider.
When using those, we have to write the numerical identifier instead of ACTIVE/ROTATING/ROTATED.
I'm not sure this view is popular, but lately I've favored string representations instead of Enums.
Up to you (and whoever also reviews this), just trying to raise your attention to this.
Note: this is a terraform provider issue, but while we don't fix it, this will have a negative impact in UX.
Contributor
There was a problem hiding this comment.
Using a string instead of an enum also yields a friendly representation for yaml if this resource is obtainable via tctl get.
Contributor
There was a problem hiding this comment.
You could also consider omitting this now while rotations are still being worked out and add it in later.
Contributor
Author
There was a problem hiding this comment.
I think this field actually started as a string in the RFD but was replaced with an enum after a couple of rounds of feedback. The enum might make a bit more sense in this case since RecordingEncryption state is meant to be internally created and managed within the auth service. The KeyState values shouldn't be exposed in a way that would expect to impact the TF provider or even tctl get since the only planned RPCs are for powering the encryption key rotation sub command.
That said I might go ahead and just omit for the time being.
eriktate
force-pushed
the
eriktate/encrypted-recording-protos
branch
from
32e26ff to
673fe22
Compare
eriktate
force-pushed
the
eriktate/keystore-decryption
branch
2 times, most recently
from
027543d to
cd91ba6
Compare
eriktate
force-pushed
the
eriktate/encrypted-recording-protos
branch
2 times, most recently
from
b9b6291 to
fc7c1c3
Compare
rosstimothy
approved these changes
May 28, 2025
eriktate
force-pushed
the
eriktate/keystore-decryption
branch
from
cd91ba6 to
12ade31
Compare
eriktate
force-pushed
the
eriktate/encrypted-recording-protos
branch
from
fc7c1c3 to
6f8f538
Compare
… and updates the existing SessionRecordingConfig protos to include a Status
eriktate
force-pushed
the
eriktate/encrypted-recording-protos
branch
from
6f8f538 to
32487ed
Compare
Contributor
|
Amplify deployment status
|
eriktate
added
the
no-changelog
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
eriktate
added a commit
that referenced
this pull request
Jun 30, 2025
… and updates the existing SessionRecordingConfig protos to include a Status (#54780)
eriktate
added a commit
that referenced
this pull request
Jul 1, 2025
… and updates the existing SessionRecordingConfig protos to include a Status (#54780)
eriktate
added a commit
that referenced
this pull request
Jul 1, 2025
… and updates the existing SessionRecordingConfig protos to include a Status (#54780)
eriktate
added a commit
that referenced
this pull request
Jul 1, 2025
… and updates the existing SessionRecordingConfig protos to include a Status (#54780)
eriktate
added a commit
that referenced
this pull request
Aug 15, 2025
… and updates the existing SessionRecordingConfig protos to include a Status (#54780)
eriktate
added a commit
that referenced
this pull request
Aug 27, 2025
… and updates the existing SessionRecordingConfig protos to include a Status (#54780)
eriktate
added a commit
that referenced
this pull request
Sep 2, 2025
… and updates the existing SessionRecordingConfig protos to include a Status (#54780)
eriktate
added a commit
that referenced
this pull request
Sep 2, 2025
… and updates the existing SessionRecordingConfig protos to include a Status (#54780)
eriktate
added a commit
that referenced
this pull request
Sep 2, 2025
… and updates the existing SessionRecordingConfig protos to include a Status (#54780)
eriktate
added a commit
that referenced
this pull request
Sep 3, 2025
… and updates the existing SessionRecordingConfig protos to include a Status (#54780)
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds new protos in support of the encrypted session recording RFD #53348