Keep the chain a re-establishment replaces

When both sides open the same chat before either has processed the other's
prekey — two people tapping a conversation at the same moment — each runs X3DH
and each ends up adopting the other's chain to read what arrived. From then on
the peer sends on the chain being replaced. Overwriting it without a copy does
not lose one message; it makes that direction undecryptable for the rest of the
conversation, silently, because an envelope that will not open is skipped rather
than reported.

**iOS.** `E2EInbox` had the fallback already — `loadArchivedRatchet` is tried
before building from a prekey, and its comment describes exactly this situation.
It was unreachable: `saveArchivedRatchet` appeared once in the file, and that
call only re-saves a ratchet that was already archived. Nothing ever put a live
session there. It does now, immediately before the session it replaces is
overwritten.

**Android was worse, and had no fallback at all.** `Conversations.poll` read

    val session = existingSession(fromDevice) ?: E2ESession.respond(identity, envelope)

which looks like a fallback and is not one: with a stored session present that
was the ONLY thing tried, `decrypt` threw for a peer on another chain, and the
catch swallowed it. `respond` was never reached. Now three chains are tried in
order — the current one, the retired one, then a fresh one built from the prekey
block — and the replaced chain is kept.

⚠️ The watermark still advances on a total failure, deliberately. An envelope
none of the three chains could open will not open next poll either, and holding
the cursor there would wedge the inbox behind one bad message. What made this
lossy was never the watermark; it was that only one chain was ever tried.

`Vault` grows a retired-session slot under its own prefix — deliberately not
`SESSION_PREFIX`, or `knownPeers()` would list every retired chain as a peer —
and `forgetAllSessions` clears it too, since a sign-out that left chain keys
behind would keep exactly what it claims to erase.

Tests, and the one that matters is the storage-level one:

- `VaultTest.a retired session is kept and still opens its chain` — asserts the
  ADOPTED session cannot open the retired chain before asserting the retired one
  can, so it cannot pass by accident. **Verified load-bearing**: neutering
  `saveRetiredSession` fails it, restoring it passes.
- `VaultTest.forgetting sessions forgets the retired ones as well`.
- `E2ESessionTest.simultaneous initiation does not strand the conversation` —
  covers the engine primitives and asserts the SECOND exchange, the one that
  used to die. ⚠️ It holds both sessions in local variables, so it passes with
  or without the storage fix; it is not evidence for it.

111 engine tests, 0 failures. Both platforms compile.

⚠️ This is a real correctness defect and is fixed on its merits. Its
relationship to the one-way loss seen between two simulators is NOT established
— that symptom is the first message, and both first messages open fine either
way. Do not close that investigation on this commit.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Paul Hitt
2026-08-25 16:53:19 -04:00
co-authored by Claude Opus 5
parent b6519faada
commit 999ab2bd26
4 changed files with 197 additions and 7 deletions
@@ -69,6 +69,75 @@ class VaultTest {
assertEquals(listOf(alice.deviceId), Vault(storage, secret).knownPeers())
}
/**
* 🔴 **The chain a re-establishment replaces must survive it.**
*
* Both sides open the chat at once, so each runs X3DH before seeing the
* other's prekey and each adopts the other's chain to read what arrived.
* From then on the peer sends on the chain being replaced. A client that
* stored only the newest session could not open any of it — silently,
* because an envelope that will not decrypt is skipped, not reported.
*
* This is the storage half of the fix. `E2ESessionTest` covers the engine
* primitives, and passes with or without it, because a test holding both
* sessions in local variables never exercises what a client keeps.
*/
@Test
fun `a retired session is kept and still opens its chain`() {
val storage = MapStorage()
val secret = key()
val alice = DeviceIdentity.create()
val bob = DeviceIdentity.create()
// Bob starts his own chain before hearing from Alice.
val bobsOwn = E2ESession.initiate(bob, bundleFor(alice))
val bobsFirst = bobsOwn.encrypt("from bob")
Vault(storage, secret).saveSession(bob.deviceId, bobsOwn)
// Alice's message arrives on a different chain, so Alice's is adopted
// and Bob's own is retired rather than dropped.
val alices = E2ESession.initiate(alice, bundleFor(bob))
val alicesFirst = alices.encrypt("from alice")
val adopted = E2ESession.respond(bob, alicesFirst)
assertEquals("from alice", adopted.decrypt(alicesFirst))
val vault = Vault(storage, secret)
vault.saveRetiredSession(bob.deviceId, vault.loadSession(bob.deviceId)!!)
vault.saveSession(bob.deviceId, adopted)
// Alice is still on the chain Bob started — she answers on it.
val onTheRetiredChain = E2ESession.respond(alice, bobsFirst)
assertEquals("from bob", onTheRetiredChain.decrypt(bobsFirst))
val reply = onTheRetiredChain.encrypt("bob, this is the old chain")
val fresh = Vault(storage, secret)
assertNull(runCatching { fresh.loadSession(bob.deviceId)!!.decrypt(reply) }.getOrNull(),
"the adopted session must NOT open the retired chain — otherwise this " +
"test would pass without the retired copy existing at all")
assertEquals("bob, this is the old chain",
fresh.loadRetiredSession(bob.deviceId)!!.decrypt(reply))
}
/** Signing out must take the retired chains too — they hold chain keys. */
@Test
fun `forgetting sessions forgets the retired ones as well`() {
val storage = MapStorage()
val secret = key()
val alice = DeviceIdentity.create()
val bob = DeviceIdentity.create()
val a = E2ESession.initiate(alice, bundleFor(bob))
Vault(storage, secret).saveSession(bob.deviceId, a)
Vault(storage, secret).saveRetiredSession(bob.deviceId, a)
Vault(storage, secret).forgetAllSessions()
assertNull(Vault(storage, secret).loadSession(bob.deviceId))
assertNull(Vault(storage, secret).loadRetiredSession(bob.deviceId))
assertTrue(storage.records.keys.none { it.contains("session") },
"a record still names a session: ${storage.records.keys}")
}
/** What is on disk must not be readable, or none of this was worth doing. */
@Test
fun `nothing recognisable is written in the clear`() {