Repository navigation
fix(e2ee): decrypt data packets with the ratcheted key when retrying - #2188
Conversation
🦋 Changeset detectedLatest commit: 404ac32 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
RaphaelFakhri
left a comment
RaphaelFakhri
left a comment
There was a problem hiding this comment.
I ran this branch locally. The two ratchet tests fail on main with valid key missing for participant receiver and pass with the change. vitest run src passes (995 passed, 1 skipped) and tsc --noEmit is clean.
I also ran some edge cases in a throwaway test against the PR's DataCryptor, with the receiver one ratchet step behind:
- Five packets decrypted concurrently all succeed.
ratchetKeydeduplicates them into one derivation,KeyRatchetedfires once, and the next packet decrypts without another ratchet. - A late packet encrypted with the pre-ratchet key rejects with
valid key missing(window 2) and leaves the stored key set unchanged, so later packets still decrypt. - If
setKey()replaces the key while a retry is in flight, the retry still decrypts and doesn't overwrite the new key. - Key index 0 and
keyringSize - 1both recover.
One non-blocking note: a gap of two or more ratchet steps still fails, even with ratchetWindowSize: 4. When a sender calls ratchetKey(1) twice, the receiver throws valid key missing. Each retry calls keys.ratchetKey(keyIndex, false), which derives from the stored material, and that material stays the same until a decryption succeeds. As a result, every attempt tries the same one-step key. FrameCryptor.decryptFrame() has the same loop, so this is consistent with media and not a regression. A follow-up that ratchets from the previous attempt's material would make the window size meaningful for both.
lukasIO
left a comment
lukasIO
left a comment
There was a problem hiding this comment.
this makes sense, thank you!
Fixes #2187
When a data packet fails to decrypt and ratcheting is enabled,
DataCryptor.decrypt()derives the ratcheted key and passes it to the retry inratchetOpts.encryptionKey, but every attempt decrypted with the storedkeySet.encryptionKey, so the retry could never succeed. A receiver that only gets data from a sender (no media to ratchet it throughFrameCryptor) never decrypted that sender's packets again after the sender ratcheted.This mirrors
FrameCryptor.decryptFrame():ratchetOpts.encryptionKey ?? keySet.encryptionKeyinitialMaterial ?? keySetto the recursive call, so the "stored key unchanged" checks compare against the key the packet was first tried withTests
handles ratcheting when enabledtest ratcheted the sender withratchetKey(1, false), which doesn't apply the ratcheted key, so the sender kept encrypting with the initial key and the test never exercised the retry. It now callsratchetKey(1).Both tests fail on
mainwithDataCryptor: valid key missing for participant receiverand pass with this change.npx vitest run src/e2ee(97 passed),npx tsc --noEmit, eslint and prettier pass. Includes a patch changeset.This PR was prepared with AI assistance and reviewed/tested by the author.