Read the contact's address from the directory, and name the refusals

The send path fetches full bundles only when a device needs a session
established, so it does not drain one-time prekeys on every message. The
contact's address rides with those bundles -- so every later send had no
address, computed the `local:` key-log subject instead of `fnid:`, matched
no leaf, and refused its own send. flamenet-relay!82 puts the address on
the directory; this reads it.

Two bugs the wiring exposed:

  * `rememberFnid` silently overwrote a DIFFERENT address, the one thing
    its own doc comment says it must never do. An fnid is derived from the
    account key, so a new address is a different account, not a new device.
    It went unnoticed while only the once-per-conversation bundle path fed
    it; the directory feeds it every send. It marks CHANGED now.

  * `guardIdentity` read the pinned address and passed it back into the
    comparison against itself, so "a different address is a different
    person" could never fire on a send. It compares the served one now.

And the copy: only `IdentityChangedException` was named, so
`KeyNotLoggedException` and `KeyLogForkException` reached the screen as
"Something went wrong reaching that server." -- sending a person to check
their signal for a decision this phone had made.

Also: the call screen painted a flat `colorScheme.background` rectangle
over the app's gradient, which read as a blue box around the call area.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Paul Hitt
2026-09-07 17:10:52 -04:00
co-authored by Claude Opus 5
parent 0fab7cc6d3
commit 6ed7931b8b
6 changed files with 116 additions and 11 deletions
@@ -63,7 +63,13 @@ fun CallScreenBody(
Column(
modifier = Modifier
.fillMaxSize()
.background(MaterialTheme.colorScheme.background)
// ⚠️ The app's ground, not `colorScheme.background`. That token is
// a flat blue, and painting it full-screen here stamped a solid
// rectangle over the gradient every other screen shows — which
// read as "a blue box around the call area". The brush is used
// rather than transparency so this composable still stands on its
// own in the screenshot harness.
.background(groundBrush())
.padding(horizontal = Kit.margin)
.testTag("call-screen"),
horizontalAlignment = Alignment.CenterHorizontally,
@@ -5,6 +5,8 @@ import androidx.lifecycle.AndroidViewModel
import androidx.lifecycle.viewModelScope
import io.flamenet.client.ChainRpc
import io.flamenet.client.IdentityChangedException
import io.flamenet.client.KeyLogForkException
import io.flamenet.client.KeyNotLoggedException
import io.flamenet.client.DeviceClock
import io.flamenet.client.AccountsClient
import io.flamenet.client.RelayClient
@@ -3890,6 +3892,24 @@ internal fun describeFailure(failure: Throwable): String = when (failure) {
is IdentityChangedException ->
"Their security code changed. Open the conversation to review it " +
"before sending anything else."
// 🔴 **The other two refusals reached the generic network line.**
// Only `IdentityChangedException` was named, and the send path throws
// these just as often — a key the log does not account for is the
// failure a person actually hits after a contact reinstalls. "Something
// went wrong reaching that server" sent them to check their signal for a
// decision this phone made, and the server was answering fine.
is KeyNotLoggedException -> if (failure.hadEntries) {
"This contact's device is offering a key that doesn't match what " +
"the server has on record for it. Open the conversation to " +
"review their security code before sending anything else."
} else {
"This contact's device is offering a key the server never recorded. " +
"Nothing was sent. Check their security code with them directly."
}
is KeyLogForkException ->
"This server's record of published keys doesn't add up, so nothing " +
"was sent. That can mean tampering \u2014 check this contact's " +
"security code with them another way before continuing."
is Vault.Corrupt -> "This phone's stored keys could not be read."
// 🔴 **Anything not named above used to reach the screen verbatim.**
@@ -612,14 +612,21 @@ class Conversations(
val theirs = bundles.filter { it.deviceId != identity.deviceId }
if (theirs.isEmpty()) return
// The pinned address is passed back in so a CHANGED address is caught
// here too, not only a changed key.
// ⚠️ **The address compared has to be the one just SERVED.** This
// read the pin and passed it straight back into the comparison, which
// compares it against itself — so the "a different address is a
// different person" rule could never fire on a send. It reads as
// working code and tests as working code; only a relay actually
// swapping an address would have shown it.
val pinned = trust.pinnedFnid(toUserId)
if (trust.record(toUserId, theirs.map { it.identitySigning }, pinned)
if (trust.record(toUserId, theirs.map { it.identitySigning }, servedFnid ?: pinned)
== IdentityTrust.CHANGED
) {
throw IdentityChangedException(toUserId)
}
// Only once the keys are accepted — an address learned alongside keys we
// are about to refuse is not one to pin.
servedFnid?.let { trust.rememberFnid(toUserId, it) }
// 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.
@@ -664,7 +671,8 @@ class Conversations(
// "Too many attempts". The directory costs nothing; full bundles are
// fetched only when some device has no session yet, which is the one
// moment X3DH actually needs them.
val directory = relay.listDevices(toUserId)
val served = relay.listDirectory(toUserId)
val directory = served.devices
val stubs = (0 until directory.length()).map { i ->
val o = directory.getJSONObject(i)
PeerBundle(
@@ -700,13 +708,18 @@ 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
// 🔴 **The directory serves the address too, and it has to.** A key
// log leaf is written under `fnid:` once an account has claimed an
// address; a verifier that does not know the address computes
// `local:<audience>/<id>`, matches no leaf, and refuses the send as an
// unlogged key — reported as "something went wrong reaching that
// server". Bundles are fetched only when a session needs establishing,
// so every later send saw the directory alone; falling back to the pin
// failed outright for a contact pinned before addresses existed.
var servedFnid: String? = served.fnid
val bundles = if (!needsEstablish) stubs else {
val keys = relay.fetchKeys(toUserId)
servedFnid = keys.fnid
servedFnid = keys.fnid ?: servedFnid
keys.fnid?.let { trust.rememberFnid(toUserId, it) }
val full = keys.bundles.associateBy { it.deviceId }
// Union: every device the directory knows, carrying a full bundle
@@ -201,7 +201,30 @@ class RelayClient(
/** `GET /e2e/devices/{user}` — who a user is, costing no prekey. */
fun listDevices(userId: Long): JSONArray =
getJson("/e2e/devices/$userId").optJSONArray("devices") ?: JSONArray()
listDirectory(userId).devices
/**
* The directory, with the account's address alongside it.
*
* ⚠️ The address is what makes the key log checkable: a leaf is written
* under `fnid:` once an account has claimed one, and a verifier that does
* not know it computes the wrong subject and refuses the send. The send
* path deliberately fetches full bundles only when a session needs
* establishing, so on every other message this is the only place the
* address can come from.
*/
fun listDirectory(userId: Long): Directory {
val o = getJson("/e2e/devices/$userId")
return Directory(
devices = o.optJSONArray("devices") ?: JSONArray(),
// Absent on a relay older than this field, which is a relay whose
// accounts have no addresses either — null is then correct, not a
// degraded answer.
fnid = o.optString("fnid").takeIf { it.isNotEmpty() },
)
}
data class Directory(val devices: JSONArray, val fnid: String?)
/**
* `DELETE /e2e/devices/{id}` — remove one of the ACCOUNT'S OWN devices.
@@ -259,6 +259,16 @@ class Trust(
fun rememberFnid(userId: Long, fnid: String) {
val existing = vault.loadTrust(userId) ?: return
if (existing.fnid == fnid) return
// 🔴 **The paragraph above was documentation, not behaviour.** This
// overwrote a DIFFERENT address in place — the one thing it says it must
// never do — and it went unnoticed while only the bundle route carried
// an address, because that route is reached once per conversation. The
// directory carries it on every send, so a silent replacement here
// would erase the change the pin exists to notice.
if (existing.fnid != null) {
markChanged(userId, existing)
return
}
vault.saveTrust(userId, existing.copy(fnid = fnid))
}
@@ -95,6 +95,39 @@ class TrustTest {
assertEquals(listOf(Wire.b64(key(1))), vault.loadTrust(7)?.identities)
}
/**
* 🔴 **The doc comment on `rememberFnid` said this and the code did the
* opposite.** It overwrote a different address in place — the one thing an
* address pin exists to prevent, since an fnid is derived from the account
* key, so a new address is not a new device but a different account.
*
* ⚠️ It went unnoticed while only the bundle route carried an address:
* that route is reached once per conversation, when a session is first
* established. The directory carries it on every send.
*/
@Test
fun `a different address is a change, not a quiet replacement`() {
val t = trust()
t.record(7, listOf(key(1)), fnid = "fnid:first")
t.rememberFnid(7, "fnid:second")
assertEquals("fnid:first", vault.loadTrust(7)?.fnid,
"the pinned address was replaced instead of challenged")
assertEquals(IdentityTrust.CHANGED, t.state(7))
}
/** An address learned for the first time is simply pinned. */
@Test
fun `an address is adopted when none was pinned`() {
val t = trust()
t.record(7, listOf(key(1)))
assertNull(vault.loadTrust(7)?.fnid)
t.rememberFnid(7, "fnid:first")
assertEquals("fnid:first", vault.loadTrust(7)?.fnid)
assertEquals(IdentityTrust.PINNED, t.state(7))
}
@Test
fun `a change drops the verified tick`() {
val t = trust()