From ccdf82c00a9d730d9ea2d4ed0b68669702ff22b7 Mon Sep 17 00:00:00 2001 From: Paul Hitt Date: Tue, 1 Sep 2026 14:08:42 -0400 Subject: [PATCH] Signing out no longer costs every contact a prompt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `signOut` destroyed the vault unconditionally, so signing back in was a genuinely NEW device: every contact had the old key pinned, saw a safety-number change, and could not message you until they accepted it. For someone stepping away for an afternoon that is a strange price, and it is why signing out and back in kept looking like the app breaking. Two doors now, matching iOS. Plain sign-out ends the session and keeps the device, so coming back is silent. "Sign out and erase this device" is its own row, confirms in place like the delete control beside it, and retires the device on the relay BEFORE destroying the keys that authorise saying so — otherwise the account goes on publishing a mailbox nobody reads, while the relay reports every copy delivered. Also: changing your password erased the phone. `updatePassword` signed out to drop the stale credential, which is right, and took the destructive door to do it, which was not — a password change made you a new device to everyone you had ever spoken to. It takes the plain one now. Account deletion asks for the erasing door explicitly. ⚠️ Keeping the keys does not leave the mailbox reachable: a relay token is minted against the account credential, which both doors clear. Co-Authored-By: Claude Opus 4.8 --- .../io/flamenet/messenger/app/ChatScreens.kt | 24 ++++++++++- .../messenger/app/MessengerViewModel.kt | 42 ++++++++++++++++--- .../io/flamenet/client/Conversations.kt | 25 ++++++++--- .../kotlin/io/flamenet/client/RelayClient.kt | 25 ++++++++++- .../main/kotlin/io/flamenet/client/Trust.kt | 36 +++++++++++++++- .../io/flamenet/client/LiveKeyLogTest.kt | 14 +++---- .../flamenet/client/LiveSealedSenderTest.kt | 2 +- .../io/flamenet/client/LiveShapesTest.kt | 2 +- .../io/flamenet/client/LiveSignUpTest.kt | 2 +- 9 files changed, 146 insertions(+), 26 deletions(-) diff --git a/app/src/main/kotlin/io/flamenet/messenger/app/ChatScreens.kt b/app/src/main/kotlin/io/flamenet/messenger/app/ChatScreens.kt index d21e040..77da1d8 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/ChatScreens.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/ChatScreens.kt @@ -712,6 +712,7 @@ fun ColumnScope.AccountScreen( var displayName by remember { mutableStateOf("") } var password by remember { mutableStateOf("") } var confirmingDelete by remember { mutableStateOf(false) } + var confirmingErase by remember { mutableStateOf(false) } Text("Account", style = MaterialTheme.typography.headlineMedium) Text( @@ -876,9 +877,28 @@ fun ColumnScope.AccountScreen( SettingsSection { SettingsRow( label = "Sign out", - description = "Erases this phone's keys and stored messages. They cannot be recovered.", + description = "Ends this session and keeps your messages on this phone, " + + "so signing back in picks up where you left off.", tag = "sign-out", - onClick = model::signOut, + onClick = { model.signOut() }, + ) + // 🔴 **The one that erases is its own row, and is not the default.** + // Both used to be the same button: signing out for an afternoon + // destroyed this phone's identity, so coming back made every contact + // accept a safety-number change first. Erasing is a real thing people + // need — handing a phone on — but it is the rarer thing, and it must be + // chosen rather than stumbled into. Confirms in place, like the pair + // below it, because a dialog is dismissed reflexively. + SettingsRow( + label = if (confirmingErase) "Really erase — this cannot be undone" + else "Sign out and erase this device", + description = "Also removes this phone's keys and stored messages. " + + "Choose this if you are handing the phone on.", + tag = "sign-out-erase", + onClick = { + if (confirmingErase) model.signOut(erasingDevice = true) + else confirmingErase = true + }, ) SettingsRow( label = if (confirmingDelete) "Really delete — this cannot be undone" diff --git a/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt b/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt index 1bf3b85..7335dbe 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt @@ -1392,7 +1392,7 @@ class MessengerViewModel(app: Application) : AndroidViewModel(app) { fun deleteAccount() = viewModelScope.launch { val current = session ?: return@launch val done = withContext(Dispatchers.IO) { runCatching { current.api.deleteAccount() } } - done.onSuccess { signOut() } + done.onSuccess { signOut(erasingDevice = true) } .onFailure { failure -> update { it.copy(error = describe(failure)) } } } @@ -2454,13 +2454,45 @@ class MessengerViewModel(app: Application) : AndroidViewModel(app) { + "new device.") } - fun signOut() = viewModelScope.launch { + /** + * End the session, and optionally erase this phone with it. + * + * 🔴 **Signing out used to cost every one of your contacts a prompt.** It + * destroyed the vault unconditionally, so signing back in was a genuinely + * NEW device: every contact had the old key pinned, saw a safety-number + * change, and could not message you until they accepted it. For someone + * stepping away for an afternoon that is a strange price to pay, and it is + * why signing out and back in kept looking like the app breaking. + * + * Two doors now. Plain sign-out ends the session and keeps the device, so + * coming back is silent. Erasing is for handing the phone on, and for + * deleting the account, and the settings screen says which is which. + * + * ⚠️ Keeping the keys does not leave the mailbox reachable: a relay token + * is minted against the account credential, which this clears either way. + */ + fun signOut(erasingDevice: Boolean = false) = viewModelScope.launch { polling?.cancel() polling = null + val current = session + val myDevice = identity?.deviceId session = null - withContext(Dispatchers.IO) { KeystoreVault.destroy(getApplication()) } - identity = null - vault = KeystoreVault.open(getApplication()) + if (erasingDevice) { + withContext(Dispatchers.IO) { + // ⚠️ Tell the relay BEFORE destroying the keys that authorise + // saying it. Otherwise the account goes on publishing a device + // nobody will ever read again, and fan-out is per device — so + // contacts keep addressing copies to a dead mailbox while the + // relay reports every one of them delivered. Best effort: a bad + // network must not leave the phone half-erased. + if (myDevice != null) runCatching { current?.revokeDevice(myDevice) } + KeystoreVault.destroy(getApplication()) + } + identity = null + vault = KeystoreVault.open(getApplication()) + } else { + identity = null + } _phase.value = Phase.SignedOut() } diff --git a/client/src/main/kotlin/io/flamenet/client/Conversations.kt b/client/src/main/kotlin/io/flamenet/client/Conversations.kt index 72b057a..ea62792 100644 --- a/client/src/main/kotlin/io/flamenet/client/Conversations.kt +++ b/client/src/main/kotlin/io/flamenet/client/Conversations.kt @@ -501,7 +501,11 @@ class Conversations( * request happens to verify is the mistake this whole section exists to * avoid. */ - private fun guardIdentity(toUserId: Long, bundles: List) { + private fun guardIdentity( + toUserId: Long, + bundles: List, + servedFnid: String? = null, + ) { val theirs = bundles.filter { it.deviceId != identity.deviceId } if (theirs.isEmpty()) return @@ -513,7 +517,11 @@ class Conversations( ) { throw IdentityChangedException(toUserId) } - theirs.forEach { trust.enforceKeyLogIfServed(it, toUserId) } + // The address just served wins over the pinned one: a device that + // registered a moment ago is logged under whatever the account claims + // NOW, and the pin may predate the claim. + val fnid = servedFnid ?: pinned + theirs.forEach { trust.enforceKeyLogIfServed(it, toUserId, fnid) } } /** @@ -567,8 +575,15 @@ class Conversations( val needsEstablish = stubs.any { it.deviceId != identity.deviceId && existingSession(it.deviceId) == null } + // ⚠️ The address comes back with the bundles and is REMEMBERED. The + // directory route serves no fnid, so a later send that needs no bundle + // would otherwise be unable to check the log again. + var servedFnid: String? = null val bundles = if (!needsEstablish) stubs else { - val full = relay.fetchKeys(toUserId).associateBy { it.deviceId } + val keys = relay.fetchKeys(toUserId) + servedFnid = keys.fnid + keys.fnid?.let { trust.rememberFnid(toUserId, it) } + val full = keys.bundles.associateBy { it.deviceId } // Union: every device the directory knows, carrying a full bundle // where the key route returned one. stubs.map { full[it.deviceId] ?: it } @@ -580,7 +595,7 @@ class Conversations( System.err.println("fnmsg-send: to=$toUserId stubs=${stubs.size} " + "bundles=${bundles.size} mine=${identity.deviceId}") try { - guardIdentity(toUserId, bundles) + guardIdentity(toUserId, bundles, servedFnid) } catch (e: Exception) { System.err.println("fnmsg-send: REFUSED by guardIdentity " + "(${e.javaClass.simpleName}: ${e.message}) — nothing will be sent. " + @@ -1253,7 +1268,7 @@ class Conversations( /** How many prekeys we have left with the relay, and a top-up when it is low. */ fun replenishPreKeysIfLow(threshold: Int = 5, batch: Int = 20): Int { - val mine = relay.fetchKeys(myUserId ?: return 0) + val mine = relay.fetchKeys(myUserId ?: return 0).bundles .firstOrNull { it.deviceId == identity.deviceId } ?: return 0 // The relay pops one per fetch, so seeing none here means the pool was // empty before this call, not that this call drained it. diff --git a/client/src/main/kotlin/io/flamenet/client/RelayClient.kt b/client/src/main/kotlin/io/flamenet/client/RelayClient.kt index 5b6c85f..a171457 100644 --- a/client/src/main/kotlin/io/flamenet/client/RelayClient.kt +++ b/client/src/main/kotlin/io/flamenet/client/RelayClient.kt @@ -138,10 +138,31 @@ class RelayClient( * starting a session. Polling it drains the pool and pushes every peer onto * the last-resort key, which costs initial forward secrecy for everyone. */ - fun fetchKeys(userId: Long): List { + /** A peer's bundles, and the address their key-log entries are filed under. */ + data class Keys(val fnid: String?, val bundles: List) + + /** + * 🔴 **The `fnid` is not decoration; without it the key log cannot be + * checked.** The relay files a leaf under `fnid:
` once an account + * has claimed one, and under `local:/` before that. A + * verifier that does not know the address computes the other subject, finds + * no matching leaf, and refuses to send — reporting "has published keys, + * but none match what this relay served" for a conversation in which + * nothing is wrong. + * + * This response has always carried it and this method always dropped it, so + * every newly registered device on an account with an address was + * unverifiable and therefore unsendable-to. It presents as messaging + * breaking the moment somebody reinstalls. + */ + fun fetchKeys(userId: Long): Keys { val body = getJson("/e2e/keys/$userId") val devices = body.optJSONArray("devices") ?: JSONArray() - return (0 until devices.length()).map { PeerBundle.fromWire(devices.getJSONObject(it)) } + return Keys( + fnid = body.optString("fnid").ifEmpty { null }, + bundles = (0 until devices.length()) + .map { PeerBundle.fromWire(devices.getJSONObject(it)) }, + ) } /** `GET /e2e/devices/{user}` — who a user is, costing no prekey. */ diff --git a/client/src/main/kotlin/io/flamenet/client/Trust.kt b/client/src/main/kotlin/io/flamenet/client/Trust.kt index 506bb48..3d48183 100644 --- a/client/src/main/kotlin/io/flamenet/client/Trust.kt +++ b/client/src/main/kotlin/io/flamenet/client/Trust.kt @@ -247,6 +247,21 @@ class Trust( /** The address this contact was pinned under, if any. */ fun pinnedFnid(userId: Long): String? = vault.loadTrust(userId)?.fnid + /** + * Record the address the relay served for this contact. + * + * ⚠️ Not a trust decision — it changes nothing about which identity keys + * are accepted. It only records the SUBJECT their key-log leaves are filed + * under, which a verifier needs and the device directory does not serve. + * Storing it means a later send that establishes no new session can still + * check the log. + */ + fun rememberFnid(userId: Long, fnid: String) { + val existing = vault.loadTrust(userId) ?: return + if (existing.fnid == fnid) return + vault.saveTrust(userId, existing.copy(fnid = fnid)) + } + /** Trust as it stands, for UI that has not triggered a fetch. */ fun state(userId: Long): IdentityTrust { val t = vault.loadTrust(userId) ?: return IdentityTrust.UNKNOWN @@ -289,8 +304,25 @@ class Trust( * the tick in place would show "verified" for something nobody verified. */ fun acceptIdentityChange(userId: Long, handle: String? = null) { - val offered = relay.fetchKeys(userId).map { it.identitySigning } - if (offered.isEmpty()) return + // ⚠️ The address is taken from the SAME response that carries the keys. + // Accepting a change and then filing it under a stale address leaves + // the key log uncheckable for the very keys just accepted. + val served = relay.fetchKeys(userId) + val offered = served.bundles.map { it.identitySigning } + // 🔴 **An empty answer is a FAILURE, not an acceptance of nothing.** + // This returned quietly, so a fetch that was rate-limited, timed out or + // 500'd left the pin exactly as it was — while the UI reported the + // change accepted. The person taps "accept", nothing happens, every + // message stays refused, and the app insists the problem is on the + // other end. Observed after a burst of retries tripped the per-contact + // key-fetch limit, which is precisely when somebody is trying to + // recover a conversation and most needs this to work. + if (offered.isEmpty()) { + throw RelayException.Malformed( + "the server returned no keys for this contact, so there was " + + "nothing to accept — try again in a moment") + } + served.fnid?.let { rememberFnid(userId, it) } // Re-resolve rather than keep the old address: if what changed WAS the // address, keeping it would leave every later check recomputing against // a name this account no longer publishes under, and honest keys would diff --git a/client/src/test/kotlin/io/flamenet/client/LiveKeyLogTest.kt b/client/src/test/kotlin/io/flamenet/client/LiveKeyLogTest.kt index 03dd1b6..9666227 100644 --- a/client/src/test/kotlin/io/flamenet/client/LiveKeyLogTest.kt +++ b/client/src/test/kotlin/io/flamenet/client/LiveKeyLogTest.kt @@ -85,7 +85,7 @@ class LiveKeyLogTest { // was served. Bob genuinely published this key a moment ago, so the // relay can prove it — and if this throws, enforcement is refusing // honest keys, which is worse than not enforcing at all. - val bundle = alice.relay.fetchKeys(bob.userId).single { it.deviceId == bob.identity.deviceId } + val bundle = alice.relay.fetchKeys(bob.userId).bundles.single { it.deviceId == bob.identity.deviceId } val trust = alice.trust() trust.enforceKeyLog(bundle, bob.userId) @@ -109,7 +109,7 @@ class LiveKeyLogTest { val alice = Actor(url, "c") val bob = Actor(url, "d") - val real = alice.relay.fetchKeys(bob.userId).single { it.deviceId == bob.identity.deviceId } + val real = alice.relay.fetchKeys(bob.userId).bundles.single { it.deviceId == bob.identity.deviceId } val substituted = io.flamenet.e2e.PeerBundle( deviceId = real.deviceId, identityDH = real.identityDH, @@ -141,7 +141,7 @@ class LiveKeyLogTest { val bob = Actor(url, "f") val trust = alice.trust() - val first = alice.relay.fetchKeys(bob.userId).single { it.deviceId == bob.identity.deviceId } + val first = alice.relay.fetchKeys(bob.userId).bundles.single { it.deviceId == bob.identity.deviceId } trust.enforceKeyLog(first, bob.userId) val pinned = alice.vault.loadKeyLogPin(alice.relay.host)!! @@ -149,7 +149,7 @@ class LiveKeyLogTest { // second call must produce and verify a real consistency proof from the // pinned size to the new one. val second = Actor(url, "g") - val third = alice.relay.fetchKeys(second.userId) + val third = alice.relay.fetchKeys(second.userId).bundles .single { it.deviceId == second.identity.deviceId } trust.enforceKeyLog(third, second.userId) @@ -175,7 +175,7 @@ class LiveKeyLogTest { relay = alice.relay.host, pin = Vault.KeyLogPin(head.getInt("size"), Wire.b64(ByteArray(32) { 0x5A })) ) - val bundle = alice.relay.fetchKeys(bob.userId).single { it.deviceId == bob.identity.deviceId } + val bundle = alice.relay.fetchKeys(bob.userId).bundles.single { it.deviceId == bob.identity.deviceId } assertFailsWith { alice.trust().enforceKeyLog(bundle, bob.userId) } @@ -218,7 +218,7 @@ class LiveKeyLogTest { .also { it.useRelayAuth(second) } .registerDevice(second, second.generateOneTimePreKeys(3)) - val bundle = alice.relay.fetchKeys(bob.userId).single { it.deviceId == second.deviceId } + val bundle = alice.relay.fetchKeys(bob.userId).bundles.single { it.deviceId == second.deviceId } val trust = alice.trust() // 1. Nothing pinned: the honest key is refused, and says why — there @@ -254,7 +254,7 @@ class LiveKeyLogTest { val bob = Actor(url, "o") val trust = alice.trust() - val bundle = alice.relay.fetchKeys(bob.userId).single { it.deviceId == bob.identity.deviceId } + val bundle = alice.relay.fetchKeys(bob.userId).bundles.single { it.deviceId == bob.identity.deviceId } // Pin Bob under an address, with his real keys. alice.vault.saveTrust( diff --git a/client/src/test/kotlin/io/flamenet/client/LiveSealedSenderTest.kt b/client/src/test/kotlin/io/flamenet/client/LiveSealedSenderTest.kt index 88eceae..fe4cedb 100644 --- a/client/src/test/kotlin/io/flamenet/client/LiveSealedSenderTest.kt +++ b/client/src/test/kotlin/io/flamenet/client/LiveSealedSenderTest.kt @@ -145,7 +145,7 @@ class LiveSealedSenderTest { // A submission presenting the OLD key must be refused outright. Sent // through the client rather than by hand, so this exercises the same // path a real sealed send takes. - val bundle = alice.relay.fetchKeys(bob.userId).first { it.deviceId == bob.identity.deviceId } + val bundle = alice.relay.fetchKeys(bob.userId).bundles.first { it.deviceId == bob.identity.deviceId } val envelope = io.flamenet.e2e.E2ESession.initiate(alice.identity, bundle).encrypt("stale") val blob = SealedSender.seal( envelope, alice.userId, alice.identity.deviceId, bundle.deviceId, bundle.identityDH) diff --git a/client/src/test/kotlin/io/flamenet/client/LiveShapesTest.kt b/client/src/test/kotlin/io/flamenet/client/LiveShapesTest.kt index 75bde53..7d32a2d 100644 --- a/client/src/test/kotlin/io/flamenet/client/LiveShapesTest.kt +++ b/client/src/test/kotlin/io/flamenet/client/LiveShapesTest.kt @@ -381,7 +381,7 @@ class LiveShapesTest { // Reachable means a stranger can fetch a bundle WITH a one-time prekey. // A device published without them can send and can never be started // with, which looks identical from the account's own side. - val bundles = relay.fetchKeys(account.userId) + val bundles = relay.fetchKeys(account.userId).bundles val mine = bundles.firstOrNull { it.deviceId == identity.deviceId } assertTrue(mine != null, "$base served no bundle for the device just registered") assertTrue(mine!!.oneTimePreKey != null, "$base served no one-time prekey") diff --git a/client/src/test/kotlin/io/flamenet/client/LiveSignUpTest.kt b/client/src/test/kotlin/io/flamenet/client/LiveSignUpTest.kt index cfd13a9..f0715a3 100644 --- a/client/src/test/kotlin/io/flamenet/client/LiveSignUpTest.kt +++ b/client/src/test/kotlin/io/flamenet/client/LiveSignUpTest.kt @@ -59,7 +59,7 @@ class LiveSignUpTest { // The device is only really registered if a stranger can fetch a bundle // for it — that is what makes this phone reachable. - val bundles = client.fetchKeys(account.userId) + val bundles = client.fetchKeys(account.userId).bundles assertTrue(bundles.any { it.deviceId == identity.deviceId }, "the relay did not serve a bundle for the device we just registered") assertTrue(bundles.first { it.deviceId == identity.deviceId }.oneTimePreKey != null,