Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update proto messages #103

Closed
wants to merge 7 commits into from
Closed

Update proto messages #103

wants to merge 7 commits into from

Conversation

neekolas
Copy link
Collaborator

Summary

Updates the Protobuf message types in V3.

The goals of these changes are:

  1. Ensure backwards/forwards compatibility for outermost message envelopes. We only have a single inbound topic in V3, so we want to avoid protobuf decoding failures for any valid message sent to that topic.
  2. Add support for message types that live outside of a conversation. For example, MessageBackupRequest and MessageBackupResponse messages are always sent from another installation and don't fit into the typical conversation model. They don't need to be wrapped in EncodedContent
  3. Add support for SealedSender on headers
  4. Remove any mention of Padlock

Notes

I copied the EncodedContent message format in to the V3 folder verbatim. This is mostly just so that V3 never has to reference V2 types, which allows us to import more narrowly and will make the eventual deprecation of V2 types easier.

@neekolas neekolas marked this pull request as ready for review October 13, 2023 21:34
@neekolas neekolas requested a review from a team as a code owner October 13, 2023 21:34
Copy link
Contributor

@richardhuaaa richardhuaaa left a comment

Choose a reason for hiding this comment

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

Like these changes overall! Thanks for the continuous cleanup you've been doing

// SealedSender wrapper for arbitrary bytes
message SealedSender {
bytes ephemeral_public_key = 1;
bytes ephemeral_static_key = 2;
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the ephemeral_static_key?

Comment on lines +25 to +31
message Metadata {
string sender_user_address = 1;
string sender_installation_id = 2;
string recipient_user_address = 3;
string recipient_installation_id = 4;
bool is_prekey_message = 5;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we feeding this whole blob into AES as the equivalent of the senderIdentityPublic input in https://signal.org/blog/sealed-sender/, or as the equivalent of the sender_certificate?

string convo_id = 3;
bytes content_bytes = 4; // EncodedContent
message MessagePayloadV1 {
EdDsaSignature header_signature = 1; // Signature of the header_bytes
Copy link
Contributor

Choose a reason for hiding this comment

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

Depending on the details of how we are implementing sealedsender, I'm wondering if we still need to sign this

@neekolas
Copy link
Collaborator Author

Closing this to focus on MLS work

@neekolas neekolas closed this Oct 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants