Adding `RecordingEncryptionService` protos by eriktate · Pull Request #55121 · gravitational/teleport

eriktate · 2025-05-23T20:59:16Z

This PR adds the proto for the RecordingEncryptionService. This will eventually have RPCs for facilitating key rotation, but for now it will only be responsible for encrypted session uploads for async recording modes.

espadolini · 2025-05-27T21:02:32Z

+service RecordingEncryptionService {
+  // UploadEncryptedRecording is used to upload encrypted .tar files containing session recording
+  // events into long term storage.
+  rpc UploadEncryptedRecording(stream UploadEncryptedRecordingRequest) returns (UploadEncryptedRecordingResponse);


Obvious question: why a client stream? Is there some optimization in the upload if contiguous parts are uploaded sequentially? How will the client understand which parts were successfully sent if the stream terminates ungracefully?

The biggest reason was just that I didn't want to expose the entire multipart upload API over RPC. Uploading as a stream has natural points to initialize and complete an upload without making those into explicit RPCs. If the stream terminates ungracefully then the entire upload would be retried and any previously uploaded parts should be removed.

@espadolini I thought about this some more and I don't think there's any good reason to do this as a stream anymore 😅 I've updated the PR with a unary implementation that just exposes the parts of the MultipartUploader interface needed to do simple uploads.

If there's a failure partway through, the intention would still be for the entire upload to be retried rather than trying to resume where we left off. This is mostly just to simplify the process and avoid checkpointing since we're batch uploading instead of making individual RPCs per event.

espadolini · 2025-06-18T13:59:36Z

+  // Upload represents the encrypted session to upload the part to.
+  Upload upload = 1;


Should this be the full Upload or should it be just the upload_id?

It's the full upload: https://github.com/gravitational/teleport/blob/master/lib/events/api.go#L1062

I changed the doc comment to refer to it as a handle to make it a bit clearer.

espadolini · 2025-06-18T14:00:23Z

+  // ETag is a part e-tag.
+  string e_tag = 2;


Is the etag available for every session storage driver?

I don't think every storage driver uses it, but it's present on the generic multipart upload interface: https://github.com/gravitational/teleport/blob/master/lib/events/api.go#L1019

espadolini · 2025-06-18T14:00:58Z

+  // PartIndex is the ordered index applied to the part.
+  int64 part_number = 1;


Is this allowed to differ from the part_number in the request? Was this called part index initially?

It was called part index. I switched it to "number" when I realized I diverged from how we refer to it elsewhere. I don't think it necessarily has to match what was given in the request as long as the final list of parts provided in CompleteUpload match what were actually uploaded.

espadolini · 2025-06-18T14:01:58Z

+  repeated UploadPartResponse parts = 3;
+}
+
+// CompleteUploadResponse


espadolini · 2025-06-18T14:02:54Z

+  // LastModified captures the timestamp of the most recent modification of this part (if available)
+  google.protobuf.Timestamp last_modified = 3;


Is this necessary for a specific driver or what? Would it be meaningfully different than the time measured when the rpc returns?

It's present in the actual MultipartUploader interface, but I don't think it's required for this specific usage. I'll remove it for now, we can always add it back if needed

espadolini

it will only be responsible for encrypted session uploads for async recording modes

Are we going to make a stream rpc for sync recording modes? That I could see being quite useful.

eriktate · 2025-06-26T19:07:54Z

it will only be responsible for encrypted session uploads for async recording modes

Are we going to make a stream rpc for sync recording modes? That I could see being quite useful.

Encryption for sync modes actually happens on the auth server, so it's just calling RecordEvent the same way unencrypted recordings would

* 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

github-actions Bot requested review from espadolini and kopiczko May 23, 2025 20:59

github-actions Bot added the size/sm label May 23, 2025

espadolini reviewed May 27, 2025

View reviewed changes

eriktate force-pushed the eriktate/sync-recording-encryption branch 2 times, most recently from 28e5480 to 1bf349f Compare May 29, 2025 03:38

eriktate force-pushed the eriktate/encrypted-recording-service-proto branch from 0b823d3 to 3d30b53 Compare May 29, 2025 18:45

eriktate force-pushed the eriktate/sync-recording-encryption branch from 01e20fb to 04c6341 Compare May 30, 2025 13:03

eriktate force-pushed the eriktate/encrypted-recording-service-proto branch from 3d30b53 to ccb603c Compare May 30, 2025 13:12

eriktate force-pushed the eriktate/sync-recording-encryption branch 3 times, most recently from 946c684 to affe736 Compare June 3, 2025 00:31

eriktate force-pushed the eriktate/encrypted-recording-service-proto branch from ccb603c to 9b2ccf7 Compare June 3, 2025 00:39

eriktate force-pushed the eriktate/sync-recording-encryption branch from affe736 to b0ab570 Compare June 10, 2025 18:53

eriktate force-pushed the eriktate/encrypted-recording-service-proto branch from 9b2ccf7 to 28ec447 Compare June 10, 2025 18:57

eriktate force-pushed the eriktate/sync-recording-encryption branch 4 times, most recently from c138f0c to 20bb5ab Compare June 16, 2025 19:53

eriktate force-pushed the eriktate/encrypted-recording-service-proto branch from 28ec447 to 8244b3c Compare June 18, 2025 00:25

eriktate changed the base branch from eriktate/sync-recording-encryption to eriktate/encrypted-recording-cache-and-events June 18, 2025 00:40

eriktate force-pushed the eriktate/encrypted-recording-service-proto branch from 8244b3c to a88ae7e Compare June 18, 2025 00:40

eriktate force-pushed the eriktate/encrypted-recording-cache-and-events branch from c477750 to 9e0b61c Compare June 18, 2025 00:45

eriktate force-pushed the eriktate/encrypted-recording-service-proto branch from a88ae7e to 6528a81 Compare June 18, 2025 00:46

eriktate force-pushed the eriktate/encrypted-recording-cache-and-events branch from 9e0b61c to 7032a39 Compare June 18, 2025 01:34

eriktate force-pushed the eriktate/encrypted-recording-service-proto branch from 6528a81 to 951fe6a Compare June 18, 2025 01:34

espadolini reviewed Jun 18, 2025

View reviewed changes

eriktate force-pushed the eriktate/encrypted-recording-cache-and-events branch from 7032a39 to 8fe244f Compare June 24, 2025 13:49

eriktate force-pushed the eriktate/encrypted-recording-service-proto branch from 951fe6a to 9951b3c Compare June 24, 2025 13:50

eriktate force-pushed the eriktate/encrypted-recording-cache-and-events branch from 8fe244f to b6c22fe Compare June 24, 2025 14:10

eriktate force-pushed the eriktate/encrypted-recording-cache-and-events branch from 9d9d44b to 9bc2d2c Compare June 25, 2025 13:30

eriktate force-pushed the eriktate/encrypted-recording-service-proto branch from 780f861 to a3a28cb Compare June 25, 2025 15:53

rosstimothy requested a review from espadolini June 25, 2025 19:02

espadolini approved these changes Jun 25, 2025

View reviewed changes

rosstimothy approved these changes Jun 25, 2025

View reviewed changes

public-teleport-github-review-bot Bot removed the request for review from kopiczko June 25, 2025 20:39

eriktate force-pushed the eriktate/encrypted-recording-cache-and-events branch from 9bc2d2c to 458ff1c Compare June 26, 2025 19:50

eriktate force-pushed the eriktate/encrypted-recording-service-proto branch from a3a28cb to d0beb68 Compare June 26, 2025 19:51

Base automatically changed from eriktate/encrypted-recording-cache-and-events to master June 26, 2025 20:44

adding recording_encryption service protos

571355f

eriktate force-pushed the eriktate/encrypted-recording-service-proto branch from d0beb68 to 571355f Compare June 26, 2025 20:49

eriktate enabled auto-merge June 26, 2025 20:49

eriktate added this pull request to the merge queue Jun 26, 2025

Merged via the queue into master with commit 2566633 Jun 26, 2025
43 checks passed

eriktate deleted the eriktate/encrypted-recording-service-proto branch June 26, 2025 21:31

eriktate added a commit that referenced this pull request Jun 30, 2025

adding recording_encryption service protos (#55121)

5a75d4e

eriktate mentioned this pull request Jun 30, 2025

[v18] Encrypted Session Recording #56262

Closed

eriktate added a commit that referenced this pull request Jul 1, 2025

adding recording_encryption service protos (#55121)

1d7ce7e

eriktate mentioned this pull request Jul 1, 2025

[v17] Encrypted Session Recording #56308

Closed

eriktate added a commit that referenced this pull request Aug 15, 2025

adding recording_encryption service protos (#55121)

b67d5a1

eriktate mentioned this pull request Aug 15, 2025

[v18] Encrypted Session Recordings #57959

Merged

eriktate added a commit that referenced this pull request Aug 27, 2025

adding recording_encryption service protos (#55121)

51984ae

eriktate added a commit that referenced this pull request Sep 2, 2025

adding recording_encryption service protos (#55121)

e60bd71

eriktate added a commit that referenced this pull request Sep 2, 2025

adding recording_encryption service protos (#55121)

c7863a1

eriktate added a commit that referenced this pull request Sep 2, 2025

adding recording_encryption service protos (#55121)

e110a1b

eriktate added a commit that referenced this pull request Sep 3, 2025

adding recording_encryption service protos (#55121)

d1c941a

		// Upload represents the encrypted session to upload the part to.
		Upload upload = 1;

		// PartIndex is the ordered index applied to the part.
		int64 part_number = 1;

		// LastModified captures the timestamp of the most recent modification of this part (if available)
		google.protobuf.Timestamp last_modified = 3;

Conversation

eriktate commented May 23, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

espadolini left a comment

Choose a reason for hiding this comment

Uh oh!

eriktate commented Jun 26, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants