feat(e2ee): implement Double Ratchet for Forward Secrecy in end-to-en… - #266
feat(e2ee): implement Double Ratchet for Forward Secrecy in end-to-en…#266priyanshuvishwakarma273403 wants to merge 2 commits into
Conversation
DenizAltunkapan
left a comment
DenizAltunkapan
left a comment
There was a problem hiding this comment.
Thanks for the work here. The direction is useful, but I would not merge this version yet because the ratchet state can be advanced in unsafe cases and a malicious or corrupted payload can force expensive local state progression. Please address the two inline points before merging.
One broader note: this is a symmetric-chain ratchet, not a full Double Ratchet with a DH ratchet step. That is fine as an incremental improvement, but the PR title/description should avoid overstating the security model unless the DH ratchet is actually implemented.
| return plaintext; | ||
| } | ||
|
|
||
| if (messageCounter > state.receivingCounter) { |
There was a problem hiding this comment.
Please add a maximum skip window before entering this loop, for example something like MAX_SKIP = 1000. Right now a payload with a very large counter can force the client to run many KDF steps and store many skipped keys, which is an easy local DoS against the browser/localStorage.
|
|
||
| state.receivingChainKey = this.arrayBufferToBase64(nextChainKey); | ||
| state.receivingCounter++; | ||
| this.saveRatchetState(ourDeviceId, remoteDeviceId, state); |
There was a problem hiding this comment.
Please do not persist the advanced receiving state before decryption has succeeded. If the ciphertext is corrupted or malicious, this saves the new chain state anyway and the real message for this counter becomes undecryptable later. A safer flow is: derive the message key, try decryptWithKey, then update and save the state only after the decrypt call returns successfully.
Add MAX_SKIP to prevent DoS from large counters. Defer state save until decryption succeeds. Note: This implements a symmetric-chain ratchet, not a full DH Double Ratchet.
GideonBa
left a comment
GideonBa
left a comment
There was a problem hiding this comment.
Thanks for addressing the previous two issues; the skip window and delayed state save look much better now. I still cannot approve this while the PR presents the change as Forward Secrecy / Double Ratchet. Please either add a real DH ratchet or update the title/description and in-code wording to describe this as a symmetric-chain ratchet only.
| ourDeviceId: string, | ||
| remoteDeviceId: string, | ||
| ): Promise<RatchetState> { | ||
| const sharedSecret = await crypto.subtle.deriveBits( |
There was a problem hiding this comment.
This still initializes every chain from the static ECDH secret. If a device private key is compromised, an attacker can recompute the initial chain and derive message keys from the public counters, so this should not be described as Forward Secrecy or a full Double Ratchet unless a DH ratchet is added.
|
@priyanshuvishwakarma273403 please apply the requested changes from @GideonBa |
Description
This Pull Request implements Forward Secrecy in our End-to-End Encryption (E2EE) pipeline by introducing a Double Ratchet algorithm (specifically the symmetric KDF chain ratchet). This ensures that compromised device key material cannot expose past messages.
Key Changes
1. Symmetric KDF Chain Ratchet (
e2ee.service.ts)crypto.subtle.deriveBitswith HKDF (SHA-256) to evolve the chain key and derive a message key for each message.RatchetState): Defined a new state interface containing sending/receiving chain keys, counter states, and skipped message keys (for out-of-order messages).2to indicate ratcheted encryption.E2eeRecipientPayloadnow includes thecounter(sequence number) of the message.1(or undefined) is received, it gracefully falls back to the original ECDH shared secret decryption.2. Unit Testing (
e2ee.service.spec.ts)null.Verification & QA
ng test --watch=false --browsers=ChromeHeadlessand confirmed they all passed successfully.npm run buildcompiles with code 0.Files Modified
frontend/src/app/services/e2ee.service.tsfrontend/src/app/services/e2ee.service.spec.ts