diff --git a/PARITY.md b/PARITY.md index c4f9054..7212b96 100644 --- a/PARITY.md +++ b/PARITY.md @@ -247,25 +247,40 @@ pinned head with a grown tree using proofs the relay generated; a root we never saw at our pinned size is a fork; and the send path itself refuses on a bad pin while NOT refusing an honestly published key with enforcement on. -๐Ÿ”ด **One real limitation, found by that live run rather than by reading.** An -account that has claimed an **fnid** is logged under it, and under +โœ… **Contacts carry the peer's global address, and enforcement uses it.** + +The relay files a leaf under an account's **fnid** once it has one, and under `local:/` before that. The subject is inside the hashed -bytes, so a verifier that does not know the fnid cannot recompute the leaf โ€” -and an honest key is refused. Proven both ways in the suite: refused without -the fnid, verifies with it. +bytes, so a verifier that does not know the address cannot recompute the leaf โ€” +which made an honest key from such an account unverifiable. That was a false +positive on a real conversation, which is how a security setting gets switched +off and left off. -That is a false positive on a real conversation, which is the worst failure a -check like this can have, because it teaches people to switch the setting off. -Nothing in the relay's contact or device routes returns a peer's fnid, so the -client cannot currently learn one. Until contact records carry it: +The address is now resolved once from the contact's handle +(`GET /e2e/account/identity/{handle}`, unauthenticated) and **pinned** beside +their identity keys, exactly as the keys are. `enforceKeyLog` recomputes against +the pinned address by default. -- enforcement is reliable for accounts that have **not** claimed an identity; -- the refusal now distinguishes "this device has published keys, none matching" - from "nothing was ever logged for this device" (`KeyNotLoggedException - .hadEntries`), so the two do not read identically; -- the fix is for contacts to carry the peer's fnid, which is stronger anyway โ€” - it checks the key was logged under *that person* rather than under whatever - subject the relay chose to file it against. +โš ๏ธ **Pinned, not looked up per check.** Re-reading it each time would let a +relay that wants to substitute a key simply also change the name it files it +under, and every verification would agree with the new one. First contact is +still trust-on-first-use โ€” taking the relay's word once is not a check; what it +buys is that every later answer has to match. The safety number covers the +first sighting and nothing here replaces it. + +๐Ÿ”ด **A changed address is an identity change**, and for a stronger reason than a +changed key: an fnid is derived from the account key, so a new address is not a +new device โ€” it is a different account key, which is to say a different person. + +Proven against `relay1` in three states, because only all three make it legible: +refused with nothing pinned, accepted once the contact carries the right +address, and refused again with a wrong one โ€” so what makes it pass is the +address being right, not the check having been softened. A changed address is +asserted to come back CHANGED against the real relay too. + +โš ๏ธ A relay that is unreachable pins **nothing** rather than pinning "no +address". Baking that in would be permanent, and the next successful lookup +would then read as the address changing โ€” the alarm, for a network blip. โš ๏ธ **Sealed sender is still absent**, so the relay continues to learn who is talking to whom. Key transparency and sealed sender protect different things 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 765e2db..5d1c274 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt @@ -308,7 +308,21 @@ class MessengerViewModel(app: Application) : AndroidViewModel(app) { fun refreshBuddies() = viewModelScope.launch { val current = session ?: return@launch - val fetched = withContext(Dispatchers.IO) { runCatching { current.api.buddies() } } + val fetched = withContext(Dispatchers.IO) { + runCatching { current.api.buddies() } + // Pin each contact's global address the first time we see them. + // Done here rather than at send time because it needs the + // handle, which the buddy list has and a user id does not โ€” and + // because a lookup on the send path is a lookup that can fail + // while somebody is trying to say something. + .onSuccess { list -> + list.forEach { buddy -> + runCatching { + current.conversations.trust.rememberIdentity(buddy.userId, buddy.handle) + } + } + } + } fetched.onSuccess { list -> update { it.copy(buddies = list, error = null) } } .onFailure { failure -> update { it.copy(error = describe(failure)) } } } @@ -383,8 +397,13 @@ class MessengerViewModel(app: Application) : AndroidViewModel(app) { */ fun acceptIdentityChange(peerId: Long) = viewModelScope.launch { val current = session ?: return@launch + // The handle, so the address is re-resolved rather than kept. If what + // changed was the address, keeping the old one would leave every later + // check recomputing against a name this account no longer publishes + // under โ€” and honest keys would stay refused for ever. + val handle = signedIn()?.buddies?.firstOrNull { it.userId == peerId }?.handle val outcome = withContext(Dispatchers.IO) { - runCatching { current.conversations.trust.acceptIdentityChange(peerId) } + runCatching { current.conversations.trust.acceptIdentityChange(peerId, handle) } } outcome.onFailure { failure -> update { it.copy(error = describe(failure)) } } update { diff --git a/client/src/main/kotlin/io/flamenet/client/Conversations.kt b/client/src/main/kotlin/io/flamenet/client/Conversations.kt index 8fb48f4..16216a4 100644 --- a/client/src/main/kotlin/io/flamenet/client/Conversations.kt +++ b/client/src/main/kotlin/io/flamenet/client/Conversations.kt @@ -168,7 +168,12 @@ class Conversations( val theirs = bundles.filter { it.deviceId != identity.deviceId } if (theirs.isEmpty()) return - if (trust.record(toUserId, theirs.map { it.identitySigning }) == IdentityTrust.CHANGED) { + // The pinned address is passed back in so a CHANGED address is caught + // here too, not only a changed key. + val pinned = trust.pinnedFnid(toUserId) + if (trust.record(toUserId, theirs.map { it.identitySigning }, pinned) + == IdentityTrust.CHANGED + ) { throw IdentityChangedException(toUserId) } if (!trust.requireKeyLog) return diff --git a/client/src/main/kotlin/io/flamenet/client/RelayClient.kt b/client/src/main/kotlin/io/flamenet/client/RelayClient.kt index ff9b088..aaffd45 100644 --- a/client/src/main/kotlin/io/flamenet/client/RelayClient.kt +++ b/client/src/main/kotlin/io/flamenet/client/RelayClient.kt @@ -273,6 +273,25 @@ class RelayClient( .put("account_key", accountKeyB64) .put("sig", signatureB64)) + /** + * `GET /e2e/account/identity/{handle}` โ€” the global address behind a local + * handle, or null when that account has not claimed one. + * + * Unauthenticated by design: it discloses a public key's address, which was + * already public, and a directory only this relay's own users can read is + * not one a federated peer could use. + * + * โš ๏ธ **404 is not "no such person".** The relay answers the same way for an + * account that does not exist and one that has never enrolled, deliberately, + * so this cannot be used to enumerate accounts. Null here means "no address + * to pin", nothing more. + */ + fun resolveIdentity(handle: String): String? = try { + getJson("/e2e/account/identity/$handle").optString("fnid").ifEmpty { null } + } catch (_: RelayException.NotFound) { + null + } + // -- Transparency log (ยง19) --------------------------------------------- // // Unauthenticated on the relay, and no proof headers are sent: the value of diff --git a/client/src/main/kotlin/io/flamenet/client/Trust.kt b/client/src/main/kotlin/io/flamenet/client/Trust.kt index 5869bcd..40e19c1 100644 --- a/client/src/main/kotlin/io/flamenet/client/Trust.kt +++ b/client/src/main/kotlin/io/flamenet/client/Trust.kt @@ -120,12 +120,23 @@ class Trust( * deliberate one โ€” the alternative is a client that cannot start a * conversation without an out-of-band channel it does not have. */ - fun record(userId: Long, offered: List): IdentityTrust { + fun record(userId: Long, offered: List, fnid: String? = null): IdentityTrust { val encoded = offered.map(Wire::b64) val existing = vault.loadTrust(userId) + + // A different address for someone we already know is not a new device. + // The fnid is derived from the account key, so a new one means a new + // account key โ€” a different person, however the relay presents it. + if (fnid != null && existing?.fnid != null && existing.fnid != fnid) { + return IdentityTrust.CHANGED + } + if (existing == null || existing.identities.isEmpty()) { if (encoded.isEmpty()) return IdentityTrust.UNKNOWN - vault.saveTrust(userId, Vault.Trust(encoded, verified = false)) + vault.saveTrust( + userId, + Vault.Trust(encoded, verified = false, fnid = fnid ?: existing?.fnid), + ) return IdentityTrust.PINNED } // โš ๏ธ EVERY offered key must already be pinned โ€” not "any". A relay @@ -137,6 +148,52 @@ class Trust( return if (existing.verified) IdentityTrust.VERIFIED else IdentityTrust.PINNED } + /** + * Learn this contact's global address and pin it. + * + * ๐Ÿ”ด **Pinned on first sight and never quietly replaced.** The address is + * what the key log files their publications under, so it decides which leaf + * a served key has to hash to. Re-reading it from the relay on every check + * would let a relay that wants to substitute a key simply also change the + * name it files it under, and every verification would agree. + * + * A DIFFERENT address later is therefore treated like a changed identity + * key, and for a stronger reason than one: an fnid is derived from the + * account key, so a new address is not a new device โ€” it is a different + * account key, which is to say a different person. + * + * โš ๏ธ First contact is still trust-on-first-use. Taking the relay's word + * once is not a check; what it buys is that every later answer has to match + * it. The safety number is what covers the first sighting, and nothing here + * replaces reading it aloud. + * + * Cheap and idempotent: does nothing once an address is pinned. Safe to + * call whenever the contact list is refreshed. + */ + fun rememberIdentity(userId: Long, handle: String): IdentityTrust { + val existing = vault.loadTrust(userId) + if (existing?.fnid != null) return state(userId) + + val resolved = try { + relay.resolveIdentity(handle) + } catch (_: Exception) { + // โš ๏ธ Unreachable is not "has no identity". Pinning null on a failed + // lookup would bake a wrong answer in permanently, and the next + // successful lookup would then read as an identity change. + return state(userId) + } ?: return state(userId) + + vault.saveTrust( + userId, + existing?.copy(fnid = resolved) + ?: Vault.Trust(identities = emptyList(), verified = false, fnid = resolved), + ) + return state(userId) + } + + /** The address this contact was pinned under, if any. */ + fun pinnedFnid(userId: Long): String? = vault.loadTrust(userId)?.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 @@ -171,10 +228,19 @@ class Trust( * key that is no longer in use and says nothing about the new one; leaving * the tick in place would show "verified" for something nobody verified. */ - fun acceptIdentityChange(userId: Long) { + fun acceptIdentityChange(userId: Long, handle: String? = null) { val offered = relay.fetchKeys(userId).map { it.identitySigning } if (offered.isEmpty()) return - vault.saveTrust(userId, Vault.Trust(offered.map(Wire::b64), verified = false)) + // 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 + // stay refused for ever. + val address = handle?.let { h -> runCatching { relay.resolveIdentity(h) }.getOrNull() } + ?: vault.loadTrust(userId)?.fnid + vault.saveTrust( + userId, + Vault.Trust(offered.map(Wire::b64), verified = false, fnid = address), + ) } /** Forget a person entirely โ€” used when they are removed as a contact. */ @@ -187,19 +253,21 @@ class Trust( * there is no sensible way to continue with a key that failed this. */ /** - * @param fnid the peer's global identity, when it is known. + * @param fnid the peer's global identity. Defaults to the address pinned + * for this contact by [rememberIdentity], which is the answer that should + * almost always be used. * - * โš ๏ธ **Required for any account that has claimed one.** The relay files a - * leaf under the fnid once an account has it, and under - * `local:/` before that; the subject is inside the - * hashed bytes, so guessing wrong makes an honest key unverifiable. Nothing - * in the relay's contact or device routes returns a peer's fnid, so this - * has to come from the contact record โ€” which is stronger anyway, because - * it checks the key was logged under *that person* rather than under - * whatever subject the relay chose. Until contacts carry it, enforcement is - * only reliable for accounts that have not claimed an identity. + * โš ๏ธ **Required for any account that has claimed one**, because the relay + * files a leaf under the fnid once an account has it and under + * `local:/` before that โ€” and the subject is inside the + * hashed bytes, so guessing wrong makes an honest key unverifiable. + * + * Taking it from the contact record rather than from the relay's current + * answer is the point: it checks the key was logged under **the person this + * contact has always been**, not under whatever subject the relay would + * like to file it against today. */ - fun enforceKeyLog(bundle: PeerBundle, userId: Long, fnid: String? = null) { + fun enforceKeyLog(bundle: PeerBundle, userId: Long, fnid: String? = pinnedFnid(userId)) { val head = head() val pin = vault.loadKeyLogPin() diff --git a/client/src/test/kotlin/io/flamenet/client/LiveKeyLogTest.kt b/client/src/test/kotlin/io/flamenet/client/LiveKeyLogTest.kt index b68c305..780c160 100644 --- a/client/src/test/kotlin/io/flamenet/client/LiveKeyLogTest.kt +++ b/client/src/test/kotlin/io/flamenet/client/LiveKeyLogTest.kt @@ -163,27 +163,21 @@ class LiveKeyLogTest { } /** - * ๐Ÿ”ด What an account with a global identity does to enforcement. + * ๐Ÿ”ด An account with a global identity, verified through the pinned address. * - * The relay files a leaf under the account's **fnid** once it has one, and + * The relay files a leaf under an account's **fnid** once it has one, and * under `local:/` before that. The subject is inside the - * hashed bytes, so a verifier that guesses wrong cannot recompute the leaf - * and cannot find it โ€” and the failure is [KeyNotLoggedException], which - * reads as "this key was never published" when the key was published - * perfectly well under a name the verifier did not know. + * hashed bytes, so a verifier that does not know the address cannot + * recompute the leaf โ€” and before contacts carried one, an honest key from + * such an account was refused. That is a false positive on a real + * conversation, which is how a security setting gets switched off. * - * That is a **false positive on an honest key**, which is the worst kind of - * failure a check like this can have: it refuses real conversations and - * teaches people to turn the setting off. - * - * This test pins the behaviour both ways so the limitation is a fact in the - * suite rather than a comment. The fix is for the contact record to carry - * the peer's fnid โ€” which is materially stronger anyway, because it checks - * the key was logged under *that person* rather than under whatever subject - * the relay chose to file it against. + * This asserts all three states, because only having all three makes the + * mechanism legible: refused with no address pinned, accepted once the + * contact carries the right one, and refused again if the address is wrong. */ @Test - fun `an account with an fnid needs its fnid to verify, and is refused without one`() { + fun `a contact carrying the peer's address verifies a key logged under it`() { val url = base ?: return val alice = Actor(url, "l") val bob = Actor(url, "m") @@ -199,25 +193,61 @@ class LiveKeyLogTest { bob.relay.claimAccountIdentity(accountKeyB64, sig) val fnid = io.flamenet.e2e.FNID.address(accountKey.publicKey) - // A device published AFTER the claim is logged under the fnid. val second = DeviceIdentity.create() - val bobSecond = RelayClient(url, bob.account.username, bob.account.appPassword) + RelayClient(url, bob.account.username, bob.account.appPassword) .also { it.useRelayAuth(second) } - bobSecond.registerDevice(second, second.generateOneTimePreKeys(3)) + .registerDevice(second, second.generateOneTimePreKeys(3)) val bundle = alice.relay.fetchKeys(bob.userId).single { it.deviceId == second.deviceId } + val trust = alice.trust() - // Without the fnid the honest key is refused โ€” but reported as "there - // are publications, none of which match", not as "never published". - val refusal = assertFailsWith { - alice.trust().enforceKeyLog(bundle, bob.userId) + // 1. Nothing pinned: the honest key is refused, and says why โ€” there + // ARE publications for this device, just none we can recompute. + val refusal = assertFailsWith { trust.enforceKeyLog(bundle, bob.userId) } + assertTrue(refusal.hadEntries, "an address miss must not read as 'never published'") + + // 2. The contact learns Bob's address from his handle, the way the + // buddy list does, and the very same key now verifies โ€” with no + // fnid passed at the call site, because it comes from the pin. + trust.rememberIdentity(bob.userId, bob.account.username) + assertEquals(fnid, trust.pinnedFnid(bob.userId)) + trust.enforceKeyLog(bundle, bob.userId) + + // 3. And a WRONG address still refuses, so what makes this pass is the + // address being right rather than the check having been softened. + assertFailsWith { + trust.enforceKeyLog(bundle, bob.userId, "fnid:definitelynotbob") } - assertTrue( - refusal.hadEntries, - "an fnid-subject miss must be distinguishable from a key that was never logged", + } + + /** + * A changed address is a changed identity, against the real relay. + * + * An fnid is derived from the account key, so a different one is a + * different account key โ€” not a new device, whatever it looks like. + */ + @Test + fun `a peer whose address changes is reported as an identity change`() { + val url = base ?: return + val alice = Actor(url, "n") + val bob = Actor(url, "o") + + val trust = alice.trust() + val bundle = alice.relay.fetchKeys(bob.userId).single { it.deviceId == bob.identity.deviceId } + + // Pin Bob under an address, with his real keys. + alice.vault.saveTrust( + bob.userId, + Vault.Trust(listOf(Wire.b64(bundle.identitySigning)), verified = false, fnid = "fnid:first"), + ) + assertEquals(IdentityTrust.PINNED, + trust.record(bob.userId, listOf(bundle.identitySigning), "fnid:first")) + + assertEquals( + IdentityTrust.CHANGED, + trust.record(bob.userId, listOf(bundle.identitySigning), "fnid:second"), + "the same keys under a new address is still a different account key", ) - // With it, the very same key verifies. - alice.trust().enforceKeyLog(bundle, bob.userId, fnid) } /** diff --git a/client/src/test/kotlin/io/flamenet/client/TrustTest.kt b/client/src/test/kotlin/io/flamenet/client/TrustTest.kt index 2094246..63ced60 100644 --- a/client/src/test/kotlin/io/flamenet/client/TrustTest.kt +++ b/client/src/test/kotlin/io/flamenet/client/TrustTest.kt @@ -130,6 +130,97 @@ class TrustTest { assertEquals(60, t.safetyNumber(7)?.length) } + // -- Global address (fnid) ----------------------------------------------- + + @Test + fun `an address is resolved once and pinned`() { + server.enqueue(json("""{"handle":"bob","fnid":"fnid:abc","audience":"r"}""")) + val t = trust() + t.rememberIdentity(7, "bob") + assertEquals("fnid:abc", t.pinnedFnid(7)) + + // Idempotent: no second request, so the queue staying empty is the + // assertion. A lookup per check is a lookup a relay can answer + // differently each time. + t.rememberIdentity(7, "bob") + assertEquals("fnid:abc", t.pinnedFnid(7)) + assertEquals(1, server.requestCount) + } + + @Test + fun `an account with no address pins nothing and is not an error`() { + server.enqueue(MockResponse().setResponseCode(404).setBody("""{"reason":"no such identity"}""")) + val t = trust() + t.rememberIdentity(7, "bob") + assertNull(t.pinnedFnid(7)) + } + + /** + * โš ๏ธ An unreachable relay must not pin "no address". + * + * Baking that in would be permanent โ€” `rememberIdentity` does nothing once + * an address is pinned โ€” and the next successful lookup would then look + * like the address changing, which is the alarm. + */ + @Test + fun `a failed lookup pins nothing rather than pinning absence`() { + server.enqueue(MockResponse().setResponseCode(503).setBody("down")) + val t = trust() + t.rememberIdentity(7, "bob") + assertNull(t.pinnedFnid(7)) + + server.enqueue(json("""{"handle":"bob","fnid":"fnid:abc","audience":"r"}""")) + t.rememberIdentity(7, "bob") + assertEquals("fnid:abc", t.pinnedFnid(7)) + } + + /** + * ๐Ÿ”ด A different address is a different account key, so a different person. + * + * Not a new device, whatever the relay presents it as: the fnid is derived + * from the account key. + */ + @Test + fun `a changed address is an identity change`() { + server.enqueue(json("""{"handle":"bob","fnid":"fnid:abc","audience":"r"}""")) + val t = trust() + t.rememberIdentity(7, "bob") + t.record(7, listOf(key(1)), "fnid:abc") + assertEquals(IdentityTrust.PINNED, t.state(7)) + + assertEquals(IdentityTrust.CHANGED, t.record(7, listOf(key(1)), "fnid:different")) + } + + @Test + fun `the same address with the same keys stays pinned`() { + server.enqueue(json("""{"handle":"bob","fnid":"fnid:abc","audience":"r"}""")) + val t = trust() + t.rememberIdentity(7, "bob") + t.record(7, listOf(key(1)), "fnid:abc") + assertEquals(IdentityTrust.PINNED, t.record(7, listOf(key(1)), "fnid:abc")) + } + + @Test + fun `enforcement recomputes against the pinned address by default`() { + server.enqueue(json("""{"handle":"bob","fnid":"fnid:abc","audience":"r"}""")) + val t = trust() + t.rememberIdentity(7, "bob") + + val b = bundle("d1", key(1)) + // The leaf the relay would have written for an account WITH this fnid. + val leaf = Wire.b64(KeyLog.leafHash(KeyLog.canonicalLeaf( + kind = "publish", subject = "fnid:abc", deviceId = b.deviceId, + ikSigB64 = Wire.b64(key(1)), ikDhB64 = Wire.b64(key(2)), + ))) + server.enqueue(json("""{"size":1,"root":"$leaf","audience":"r"}""")) + server.enqueue(json("""{"device_id":"d1","size":1,"entries":[{"index":0,"kind":"publish","leaf":"$leaf"}]}""")) + server.enqueue(json("""{"index":0,"size":1,"leaf":"$leaf","root":"$leaf","proof":[]}""")) + + // No fnid argument: it comes from the pin, which is the whole point. + t.enforceKeyLog(b, 7) + assertEquals(leaf, vault.loadKeyLogPin()?.root) + } + // -- Key-log enforcement ------------------------------------------------- @Test diff --git a/engine/src/main/kotlin/io/flamenet/e2e/Vault.kt b/engine/src/main/kotlin/io/flamenet/e2e/Vault.kt index 890ec47..bfec71e 100644 --- a/engine/src/main/kotlin/io/flamenet/e2e/Vault.kt +++ b/engine/src/main/kotlin/io/flamenet/e2e/Vault.kt @@ -174,19 +174,39 @@ class Vault(private val storage: Storage, private val key: SecretKey) { * because a person is not a device: someone with a phone and a tablet has * two, and both are legitimately theirs. */ - data class Trust(val identities: List, val verified: Boolean) + data class Trust( + val identities: List, + val verified: Boolean, + /** + * The global address this contact was first seen under, if any. + * + * โš ๏ธ **Pinned, not looked up per use.** The key log files a leaf under + * whichever subject the account had at publication time, so verifying a + * served key means recomputing against the address we already accepted + * for this person โ€” not against whatever the relay says today. Asking + * again each time would let a relay change the answer and have every + * check agree with the new one. + * + * Null means "no address seen", which is both "never claimed one" and + * "the relay would not say" โ€” the relay answers those identically on + * purpose, so the client cannot tell them apart either. + */ + val fnid: String? = null, + ) fun loadTrust(userId: Long): Trust? = readJson(trustName(userId))?.let { o -> val a = o.optJSONArray("identities") ?: org.json.JSONArray() Trust( identities = (0 until a.length()).map { a.getString(it) }, verified = o.optBoolean("verified", false), + fnid = o.optString("fnid").ifEmpty { null }, ) } fun saveTrust(userId: Long, trust: Trust) = writeJson(trustName(userId), JSONObject() .put("identities", org.json.JSONArray(trust.identities)) - .put("verified", trust.verified)) + .put("verified", trust.verified) + .put("fnid", trust.fnid ?: JSONObject.NULL)) fun forgetTrust(userId: Long) = storage.delete(trustName(userId))