From 79844a9d29fbc58a9c84f464052be52c75fac309 Mon Sep 17 00:00:00 2001 From: Renato Alcara Date: Mon, 27 Apr 2026 16:37:20 -0300 Subject: [PATCH] fix(signal): use raw JID as transaction key to stop Bad MAC loop on own DSM MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PRODUCTION BUG: After WhatsApp rolled out LID-addressed DSM (deviceSentMessage), own messages flooded the inbound pipeline with perpetual `Bad MAC` errors, delaying delivery to the consumer by ~60s. Pedro's fork (Feb/6 snapshot, 484 commits behind) does NOT have this bug — diff isolated the regression to this file. ROOT CAUSE: `decryptMessage` was locking on `canonicalJid` (resolved via `resolveCanonicalJid`, which awaits `lidMapping.getLIDForPN`) but the `SessionCipher` is constructed from `jidToSignalProtocolAddress(jid)` — the RAW jid. So: 1. The mutex lock key (canonical LID) and the session-storage key (raw JID/PN-derived) drift apart. Two concurrent decrypts for the same logical session — one arriving as PN, one as LID — acquire DIFFERENT locks and interleave reads/writes on the same Signal session record, corrupting the ratchet. Symptom: perpetual `Bad MAC Error`. 2. Each decrypt also paid the cost of an extra `await getLIDForPN()` round trip to the LID store on the hot path — a needless await per message. `encryptMessage` had the same bug. FIX: Lock on the raw `jid` (matches what `SessionCipher` actually uses for storage). PN/LID race protection still happens upstream in `migrateSession` + `storeLIDPNMappings`, which run BEFORE decrypt and ensure the session is already stored under the correct format by the time we acquire the lock. `resolveCanonicalJid` is now unused — removed. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/Signal/libsignal.ts | 36 +++++++++++------------------------- 1 file changed, 11 insertions(+), 25 deletions(-) diff --git a/src/Signal/libsignal.ts b/src/Signal/libsignal.ts index 26b6e8e9..3420cd00 100644 --- a/src/Signal/libsignal.ts +++ b/src/Signal/libsignal.ts @@ -299,25 +299,6 @@ export function makeLibSignalRepository( // Promise instead of each spawning their own DB transactions. const migrationInFlight = new Map>() - // Resolve PN JID to its canonical LID JID for transaction locking. - // This prevents PN/LID race conditions where concurrent operations for the - // same logical contact acquire different mutex locks because one uses PN - // and the other uses LID. (Aligned with WABA behavior — all operations use LID internally.) - const resolveCanonicalJid = async(jid: string): Promise => { - if (isAnyLidUser(jid)) { - return jid - } - - if (isAnyPnUser(jid)) { - const lid = await lidMapping.getLIDForPN(jid) - if (lid) { - return lid - } - } - - return jid - } - const repository: SignalRepositoryWithLIDStore = { decryptGroupMessage({ group, authorJid, msg }) { const senderName = jidToSignalSenderKeyName(group, authorJid) @@ -408,24 +389,29 @@ export function makeLibSignalRepository( return result } - // Use canonical JID (PN→LID resolved) as transaction key to prevent - // PN/LID race conditions on the same logical session. - const canonicalJid = await resolveCanonicalJid(jid) + // Lock on the raw JID — same string libsignal uses to derive the + // SessionCipher address (line above). Locking on a *resolved* canonical + // JID would mean the lock key and the session storage key drift apart, + // letting concurrent ops on the same session interleave and corrupt the + // ratchet (observed as a perpetual Bad MAC loop on own DSM after the + // PN→LID rollout). PN/LID race protection lives upstream in + // migrateSession + storeLIDPNMappings, not here. return parsedKeys.transaction(async () => { return await doDecrypt() - }, canonicalJid) + }, jid) }, async encryptMessage({ jid, data }) { const addr = jidToSignalProtocolAddress(jid) const cipher = new libsignal.SessionCipher(storage, addr) - const canonicalJid = await resolveCanonicalJid(jid) + // Lock on raw JID — same string used to derive `addr` above. See comment + // in decryptMessage for the rationale (lock key must match session storage key). return parsedKeys.transaction(async () => { const { type: sigType, body } = await cipher.encrypt(data) const type = sigType === 3 ? 'pkmsg' : 'msg' return { type, ciphertext: Buffer.from(body, 'binary') } - }, canonicalJid) + }, jid) }, async encryptGroupMessage({ group, meId, data }) {