diff --git a/client/src/main/kotlin/io/flamenet/client/Conversations.kt b/client/src/main/kotlin/io/flamenet/client/Conversations.kt index b243d18..a72855f 100644 --- a/client/src/main/kotlin/io/flamenet/client/Conversations.kt +++ b/client/src/main/kotlin/io/flamenet/client/Conversations.kt @@ -290,12 +290,36 @@ class Conversations( val envelope = opened?.envelope ?: Envelope.fromPayload(message.payload) val fromDevice = opened?.claimedFromDevice ?: message.fromDevice - val session = existingSession(fromDevice) - ?: E2ESession.respond(identity, envelope) - // Nothing below this line runs unless the ratchet opened it, - // which is what turns the claim above into a fact. - val plaintext = session.decrypt(envelope) - vault.saveSession(fromDevice, session) + + // 🔴 **Three chains to try, not one.** This used to be + // `existingSession(fromDevice) ?: E2ESession.respond(...)`, + // which reads as a fallback and is not one: when a stored + // session existed it was the ONLY thing tried, `decrypt` threw + // for a peer on a different chain, and the catch below swallowed + // it. `respond` was never reached. + // + // That happens whenever both sides run X3DH before either has + // processed the other's prekey — two people opening the same + // chat at once, which is ordinary behaviour. Each adopts the + // other's chain, and from then on every message arrives on a + // chain the receiver has thrown away. + val existing = existingSession(fromDevice) + val retired = runCatching { vault.loadRetiredSession(fromDevice) }.getOrNull() + val opening = openWith(existing, envelope) + ?: openWith(retired, envelope) + // Only now build from the prekey block, and keep the chain + // being replaced — the peer is still sending on it. + ?: openWith(E2ESession.respond(identity, envelope), envelope) + ?: throw IllegalStateException("no session could open this envelope") + + val plaintext = opening.plaintext + val session = opening.session + if (session !== existing) { + if (existing != null) vault.saveRetiredSession(fromDevice, existing) + vault.saveSession(fromDevice, session) + } else { + vault.saveSession(fromDevice, session) + } val fromUser = opened?.claimedFromUser ?: message.fromUser val content = Content.parse(plaintext) consumeControl(fromUser, content) @@ -303,6 +327,13 @@ class Conversations( } catch (_: Exception) { // Deliberately swallowed and skipped. One undecryptable message // is not a reason to stop reading the inbox. + // + // ⚠️ The watermark still advances below, on purpose: an + // envelope that none of the three chains could open will not + // open on the next poll either, and holding the cursor on it + // would wedge the inbox behind one bad message. What made this + // lossy before was not the watermark — it was that only ONE + // chain was ever tried. } vault.inboxWatermark = message.id } @@ -406,6 +437,23 @@ class Conversations( private fun existingSession(peerDeviceId: String): E2ESession? = runCatching { vault.loadSession(peerDeviceId) }.getOrNull() + /** A session that opened an envelope, and what came out of it. */ + private class Opened(val session: E2ESession, val plaintext: String) + + /** + * Try one chain, and report failure as null rather than as an exception. + * + * ⚠️ **Decrypting mutates the ratchet**, so each candidate has to be a + * session object the caller is willing to keep — never the same instance + * tried twice. A failed attempt leaves that object in an indeterminate + * state and it is dropped, which is why the caller reloads rather than + * reusing. + */ + private fun openWith(session: E2ESession?, envelope: Envelope): Opened? { + if (session == null) return null + return runCatching { Opened(session, session.decrypt(envelope)) }.getOrNull() + } + /** * The session for a peer device: the stored one, or a new handshake. * diff --git a/engine/src/main/kotlin/io/flamenet/e2e/Vault.kt b/engine/src/main/kotlin/io/flamenet/e2e/Vault.kt index 8ff0853..a76d524 100644 --- a/engine/src/main/kotlin/io/flamenet/e2e/Vault.kt +++ b/engine/src/main/kotlin/io/flamenet/e2e/Vault.kt @@ -46,6 +46,11 @@ class Vault(private val storage: Storage, private val key: SecretKey) { private const val WATERMARK = "watermark" private const val CREDENTIAL = "credential" private const val SESSION_PREFIX = "session/" + + // ⚠️ Must NOT start with SESSION_PREFIX, or `knownPeers()` would list + // every retired chain as a peer and `sessionName` collisions would put + // two sessions in one record. + private const val RETIRED_PREFIX = "retired-session/" private const val HISTORY_PREFIX = "history/" private const val TRUST_PREFIX = "trust/" private const val KEYLOG_PIN = "keylog-pin" @@ -154,6 +159,26 @@ class Vault(private val storage: Storage, private val key: SecretKey) { fun saveSession(peerDeviceId: String, session: E2ESession) = writeJson(sessionName(peerDeviceId), session.archive()) + /** + * The session a re-establishment retired, kept so its in-flight messages + * can still be opened. + * + * 🔴 When both sides run X3DH before either has processed the other's + * prekey — two people opening the same chat at once — each adopts the + * other's chain. Everything the peer sends from then on travels on the + * chain being replaced, so overwriting it without a copy makes that + * direction undecryptable for the rest of the conversation, silently. + * + * ⚠️ One retired session per device: a second re-establishment displaces + * the first. Keeping the most recent is the right trade, because the older + * chain's messages have had longer to arrive. + */ + fun loadRetiredSession(peerDeviceId: String): E2ESession? = + readJson(RETIRED_PREFIX + peerDeviceId)?.let(E2ESession::fromArchive) + + fun saveRetiredSession(peerDeviceId: String, session: E2ESession) = + writeJson(RETIRED_PREFIX + peerDeviceId, session.archive()) + fun knownPeers(): List = storage.names().filter { it.startsWith(SESSION_PREFIX) }.map { it.removePrefix(SESSION_PREFIX) } @@ -357,6 +382,10 @@ class Vault(private val storage: Storage, private val key: SecretKey) { fun forgetEverything() = storage.names().forEach(storage::delete) /** Forget conversations but keep this device's identity. */ + /** ⚠️ Retired sessions too — they hold chain keys, and a sign-out that + * left them behind would keep exactly what it claims to erase. */ fun forgetAllSessions() = - storage.names().filter { it.startsWith(SESSION_PREFIX) }.forEach(storage::delete) + storage.names() + .filter { it.startsWith(SESSION_PREFIX) || it.startsWith(RETIRED_PREFIX) } + .forEach(storage::delete) } diff --git a/engine/src/test/kotlin/io/flamenet/e2e/E2ESessionTest.kt b/engine/src/test/kotlin/io/flamenet/e2e/E2ESessionTest.kt index 9aeb20b..145c6a3 100644 --- a/engine/src/test/kotlin/io/flamenet/e2e/E2ESessionTest.kt +++ b/engine/src/test/kotlin/io/flamenet/e2e/E2ESessionTest.kt @@ -86,6 +86,50 @@ class E2ESessionTest { assertEquals("and back", a.decrypt(restoredB.encrypt("and back"))) } + /** + * 🔴 **Both sides open the chat at once.** + * + * Each runs X3DH before either has seen the other's prekey, so each holds + * its own chain and each is about to be handed a message on the other's. + * This is ordinary behaviour — two people tapping a conversation at the + * same moment — not an edge case. + * + * The failure it guards is not the first message. Both first messages open + * fine, because `respond` builds the peer's chain on demand. What breaks is + * everything AFTER the swap: each side now sends on the chain the other + * threw away, and a client that kept only the newest session cannot open + * any of it. Silently — an envelope that will not decrypt is skipped, not + * reported. + * + * So this asserts the second exchange, which is the one that used to die. + */ + @Test + fun `simultaneous initiation does not strand the conversation`() { + val alice = DeviceIdentity.create() + val bob = DeviceIdentity.create() + + // Neither has heard from the other yet; both start a chain. + val aOwn = E2ESession.initiate(alice, bundleFor(bob)) + val bOwn = E2ESession.initiate(bob, bundleFor(alice)) + val aFirst = aOwn.encrypt("from alice") + val bFirst = bOwn.encrypt("from bob") + + // Each adopts the other's chain to read the message that arrived, and + // RETIRES its own rather than dropping it — the peer is still on it. + val aAdopted = E2ESession.respond(alice, bFirst) + assertEquals("from bob", aAdopted.decrypt(bFirst)) + val bAdopted = E2ESession.respond(bob, aFirst) + assertEquals("from alice", bAdopted.decrypt(aFirst)) + + // The second exchange. Alice replies on the chain she adopted; Bob has + // that chain only as the one he started, so he needs the retired copy. + val aSecond = aAdopted.encrypt("alice again") + assertEquals("alice again", bOwn.decrypt(aSecond)) + + val bSecond = bAdopted.encrypt("bob again") + assertEquals("bob again", aOwn.decrypt(bSecond)) + } + /** Skipped keys are the in-flight messages; losing them at a restart loses those. */ @Test fun `messages in flight across a restart still open`() { diff --git a/engine/src/test/kotlin/io/flamenet/e2e/VaultTest.kt b/engine/src/test/kotlin/io/flamenet/e2e/VaultTest.kt index b976285..de858a0 100644 --- a/engine/src/test/kotlin/io/flamenet/e2e/VaultTest.kt +++ b/engine/src/test/kotlin/io/flamenet/e2e/VaultTest.kt @@ -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`() {