fix(e2ee): location message decryption (AAD contentType, isSelf, key pinning) - #218
Open
nezumi0627 wants to merge 1 commit into
Open
fix(e2ee): location message decryption (AAD contentType, isSelf, key pinning)#218nezumi0627 wants to merge 1 commit into
nezumi0627 wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
decryptE2EELocationMessagecould never successfully decrypt a received E2EE location message, for two independent reasons:1. AAD content type: string
"LOCATION"→ NaN → 0 (≠ 15)Incoming messages carry the enum name (
"LOCATION") becauserename_thriftmaps numeric enums back to strings. The encrypt side hardcodes the correct value in the AAD:but the decrypt path passed
messageObj.contentTypestraight through ascontentType as number.getIntBytes("LOCATION")→setInt32(0, NaN)→ writes0, so the decryptor built an AAD withf = 0while the ciphertext was authenticated withf = 15— GCM auth verification always failed. The retry block then retried with identical inputs and rethrew.decryptE2EEDataMessagealready handles this correctly (LINETypes.enums.ContentType[messageObj.contentType]); the location path now does the same.2.
isSelfdefaulted totrueand was never derivedUnlike
decryptE2EETextMessage/decryptE2EEDataMessage, the location path never checked_from === profile.mid, and its default parameter wastrue. The only caller (decryptE2EEMessage) passes noisSelf, so every location message was treated as self-sent: for a received 1:1 message it fetched my own public key viagetE2EELocalPublicKey(to=myMid, …)and computed ECDH(myPriv, myPub) instead of ECDH(myPriv, senderPub) — always the wrong shared secret.It also never pinned the self private key by envelope key id (#88), so it additionally broke on key rotation — exactly what #88 fixed for the text/data paths. The function now mirrors those two:
isSelf = falsedefault, derived fromfrom === profile.midsenderKeyId/receiverKeyIdwith latest-key fallback(to, receiverKeyId)when self-sent,(_from, senderKeyId)when receivedTesting
location_decrypt.test.ts(same trap-the-primitive approach askey_selection.test.ts), 3 cases:15, self-key pinned byreceiverKeyId, peer pubKey looked up as(_from, senderKeyId)isSelfderived from mid, peer pubKey looked up as(to, receiverKeyId)deno test --allow-all— 217 passed, 0 failed.