Skip to content

fix: outgoing messages should not be decrypted #28

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.

Sign up for GitHub

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

Merged
kaichaosun merged 3 commits into from
Dec 1, 2025
Merged

fix: outgoing messages should not be decrypted #28

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
523dca4
fix: outgoing messages should not be decrypted
kaichaosun Nov 28, 2025
622fe0a
chore: not expose private dh key.
kaichaosun Dec 1, 2025
eca8b57
Merge branch 'main' into fix-decrypt-out-messages
kaichaosun Dec 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/chat_sdk/conversations/private_v1.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,10 @@ proc decrypt*(convo: PrivateV1, enc: EncryptedPayload): Result[seq[byte], ChatEr
)
copyMem(addr header.dhPublic[0], unsafeAddr dr.dh[0], dr.dh.len) # TODO: Avoid this copy

if convo.doubleratchet.dhSelf.public == header.dhPublic:
info "outgoing message, no need to decrypt"
return err(ChatError(code: errDecryptOutgoing, context: "Attempted to decrypt outgoing message"))

Copy link
Collaborator

@jazzz jazzz Dec 1, 2025

Choose a reason for hiding this comment

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

[Pebble] With this change, handleFrame will fail with a decryption error when an outgoing message is parsed. This doesn't seem like what we want.

Decrypting outgoing messages is a state that should never occur(ProgramError), rather than an DecryptionError.

I'd suggest that errDecryptOutgoing ought to be caught in handleFrame; which then logs and returns early. At that point frame has been successfully parsed, as there is no work to do.

Alternatively the check could be performed in handleFrame prior to decryption. This way decrypt is only ever called on ciphertexts that it should be able to handle successfully.

convo.doubleratchet.decrypt(header, dr.ciphertext, @[]).mapErr(proc(e: NaxolotlError): ChatError = ChatError(code: errWrapped, context: repr(e) ))


Expand Down
1 change: 1 addition & 0 deletions src/chat_sdk/errors.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ type
ErrorCode* = enum
errTypeError
errWrapped
errDecryptOutgoing


proc `$`*(x: ChatError): string =
Expand Down
2 changes: 1 addition & 1 deletion src/naxolotl/naxolotl.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const maxSkip = 10


type Doubleratchet* = object
dhSelf: PrivateKey
dhSelf*: PrivateKey
Copy link
Collaborator

@jazzz jazzz Dec 1, 2025

Choose a reason for hiding this comment

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

[Boulder] Exposing the PrivateKey outside of the module is risky here. Only the publicKey is needed, consider making a function to access the publicKey.

dhRemote: PublicKey

rootKey: RootKey
Expand Down