release(v4.2.1): fix concurrent-ratchet desync via OutboundQueue waiter cursor
Pull-mode httpClient + drainer + parallel RPCs against the same peer deteriorated after ~10s with `DecryptionError`. Two bugs combined: - `OutboundQueue.enqueue` woke `drain` waiters with a `since=0` snapshot, replaying already-processed events into `Shade.acceptTransferEnvelope` → `manager.decrypt` twice. The duplicate consumed an already-used skipped key and corrupted the Double Ratchet receive chain. - `ratchetDecrypt` then propagated the corruption: a same-DH message behind the chain with no cached skipped key fell through to `kdfChainKey` on the ahead state and rewound `chain.counter`, permanently desyncing the chain. Fix `OutboundQueue` to honor each waiter's `since`, and harden `ratchetDecrypt` so any future duplicate fails cleanly without mutating state. Adds regression coverage at all three layers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "@shade/core",
|
||||
"version": "4.2.0",
|
||||
"version": "4.2.1",
|
||||
"type": "module",
|
||||
"main": "src/index.ts",
|
||||
"types": "src/index.ts",
|
||||
|
||||
@@ -185,6 +185,22 @@ export async function ratchetDecrypt(
|
||||
if (!session.receiveChain) {
|
||||
throw new DecryptionError('No receiving chain available');
|
||||
}
|
||||
// Defense-in-depth: a same-DH message whose counter is already
|
||||
// behind the chain — and that did NOT match a cached skipped key —
|
||||
// is either a duplicate we already decrypted (skipped key was
|
||||
// consumed) or one whose key was evicted under cache pressure.
|
||||
// Falling through would call kdfChainKey on the *current* (ahead)
|
||||
// chainKey and then rewind `chain.counter = message.counter + 1`,
|
||||
// permanently desyncing the chain so every subsequent decrypt
|
||||
// returns wrong-key. Reject without mutating state instead.
|
||||
if (
|
||||
!isNewRatchet &&
|
||||
message.counter < session.receiveChain.counter
|
||||
) {
|
||||
throw new DecryptionError(
|
||||
'Failed to decrypt message — wrong key or tampered data',
|
||||
);
|
||||
}
|
||||
await skipMessageKeys(crypto, session, message.dhPublicKey, session.receiveChain, message.counter);
|
||||
|
||||
// Advance the receiving chain one more step to get this message's key
|
||||
|
||||
@@ -281,6 +281,41 @@ describe('Double Ratchet', () => {
|
||||
|
||||
expect(ratchetDecrypt(crypto, bob, msg)).rejects.toThrow();
|
||||
});
|
||||
|
||||
/**
|
||||
* Regression — the v4.2.0 OutboundQueue waiter-since bug delivered
|
||||
* the same envelope twice to `manager.decrypt`. The first decrypt
|
||||
* succeeded via a cached skipped key; the second one fell into the
|
||||
* `message.counter < chain.counter` path with no skipped key
|
||||
* available, advanced the chainKey ONCE and rewound `chain.counter`
|
||||
* to `message.counter + 1`, leaving the ratchet permanently
|
||||
* desynced. ratchetDecrypt now rejects without mutating state when
|
||||
* a same-DH message is behind the chain and not in skippedKeys, so
|
||||
* a downstream replay (transport bug, retry, etc.) cannot poison
|
||||
* the session for everyone else.
|
||||
*/
|
||||
test('same-DH stale message after consumed skipped key fails without corrupting state', async () => {
|
||||
const { alice, bob } = await setupPair();
|
||||
|
||||
// Alice sends 3 messages on the same DH chain.
|
||||
const m0 = await ratchetEncrypt(crypto, alice, enc.encode('m0'));
|
||||
const m1 = await ratchetEncrypt(crypto, alice, enc.encode('m1'));
|
||||
const m2 = await ratchetEncrypt(crypto, alice, enc.encode('m2'));
|
||||
|
||||
// Bob receives m1 first, caching m0's key. Then m0 (delivered
|
||||
// via the cache). After this, m0's skipped key is consumed.
|
||||
expect(dec.decode(await ratchetDecrypt(crypto, bob, m1))).toBe('m1');
|
||||
expect(dec.decode(await ratchetDecrypt(crypto, bob, m0))).toBe('m0');
|
||||
|
||||
// Replay of m0: skippedKey is gone, chain.counter is past m0.
|
||||
// Pre-fix: this would corrupt Bob's chain state; post-fix it
|
||||
// throws cleanly.
|
||||
await expect(ratchetDecrypt(crypto, bob, m0)).rejects.toThrow(DecryptionError);
|
||||
|
||||
// Bob can still decrypt the remaining valid message — chain
|
||||
// state was NOT mutated by the rejected replay.
|
||||
expect(dec.decode(await ratchetDecrypt(crypto, bob, m2))).toBe('m2');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── Long Conversation ────────────────────────────────────
|
||||
|
||||
Reference in New Issue
Block a user