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

feat(2631): Encrypt all message contents and reduce size of encryption #2749

Merged
merged 4 commits into from
Feb 19, 2025

Conversation

islathehut
Copy link
Collaborator

@islathehut islathehut commented Feb 14, 2025

Decrypted Message

export interface ChannelMessage {
  id: string
  type: number
  message: string
  createdAt: number
  channelId: string
  signature: string
  encSignature?: EncryptionSignature
  pubKey: string
  media?: FileMetadata
}

Encrypted Message

export interface EncryptedMessage {
  id: string
  contents: EncryptedPayload
  createdAt: number
  channelId: string
  encSignature: TruncatedSignedEnvelope
}

Fields From Message that are Encrypted

export interface EncryptableMessageComponents {
  type: number
  message: string
  signature: string
  pubKey: string
  media?: FileMetadata
}

Pull Request Checklist

  • I have linked this PR to a related GitHub issue.
  • I have added a description of the change (and Github issue number, if any) to the root CHANGELOG.md.

(Optional) Mobile checklist

Please ensure you completed the following checks if you did any changes to the mobile package:

  • I have run e2e tests for mobile
  • I have updated base screenshots for visual regression tests

@islathehut islathehut linked an issue Feb 14, 2025 that may be closed by this pull request
@islathehut islathehut requested a review from adrastaea February 14, 2025 20:29
@islathehut islathehut linked an issue Feb 14, 2025 that may be closed by this pull request
@@ -30,3 +31,8 @@ export type DecryptedPayload<T> = {
contents: T
isValid: boolean
}

export type TruncatedSignedEnvelope = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

TruncatedSignedEnvelope is sort of a mouthful, and not really descriptive to what this object is. We might as well just call this object Signature, but also, I don't think I really agree with this object existing independently at all. For one, just having the SignedEnvelope type makes more sense because it ensures the signature and the contents it is signing stay together reducing the chance of somehow later creating a mismatched pair of signature and contents. Also, these types are all sort of just recreating the LFA native types with new types. Why not just use the types that the library provides/returns?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed name to Signature.

context: LocalUserContext,
failOnInvalid = true
): DecryptedPayload<T> {
const isValid = this.verifyMessage(signature)
const fullSig: SignedEnvelope = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

See, here we're just recomposing the type we broke apart earlier in order to fit the verifyMessage typing. We could just maintain the shape throughout.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I updated this because using the types that LFA uses means we have the encrypted contents written twice to orbitdb. It's fine when we're just passing it around in memory but since the encrypted contents are usually the largest part of the object it's a pretty big increase in storage space/network bandwidth.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

A possible alternative is to combine the two types so we end up with

{
  "contents": <encrypted data array>,
  "recipient": <info on which key data was encrypted with>,
  "signature": <signature string>,
  "author": <info on which user's key was used to sign>
}

We probably wouldn't even need to decompose into the original LFA types since this would have the requisite fields for both the signature and the encrypted payload.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually, thinking more about this, we should be signing the raw message instead of the encrypted which changes things a bit.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Alright, I kept the same decomposed type but now its the raw message that's signed.

@adrastaea adrastaea self-requested a review February 18, 2025 17:40
@islathehut islathehut merged commit 87fdb68 into develop Feb 19, 2025
26 of 30 checks passed
@islathehut islathehut deleted the feat/encrypt-whole-message branch February 19, 2025 14:36
islathehut added a commit that referenced this pull request Feb 19, 2025
#2749)

* Encrypt all message contents and reduce size of encryption

* Sign raw message contents

* We don't need verifyMessage anymore
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.

Decrypt messages with team key Encrypt messages with team key
2 participants