diff --git a/engine/src/main/kotlin/io/flamenet/e2e/Vault.kt b/engine/src/main/kotlin/io/flamenet/e2e/Vault.kt index aa8f5ae..393688d 100644 --- a/engine/src/main/kotlin/io/flamenet/e2e/Vault.kt +++ b/engine/src/main/kotlin/io/flamenet/e2e/Vault.kt @@ -461,15 +461,42 @@ class Vault(private val storage: Storage, private val key: SecretKey) { Trust( identities = (0 until a.length()).map { a.getString(it) }, verified = o.optBoolean("verified", false), - fnid = o.optString("fnid").ifEmpty { null }, + // 🔴 **`"null"` is rejected, and that is not paranoia — records + // written that way are already on phones.** `saveTrust` used to + // write an explicit JSON null here, and Android's `org.json` + // coerces that to the four-character string on the way back out. + // See the note on [saveTrust]; this is the half that repairs what + // has already been stored. + fnid = o.optString("fnid").takeIf { it.isNotEmpty() && it != "null" }, changed = o.optBoolean("changed", false), ) } + /** + * 🔴 **The key is OMITTED when there is no address, never written as an + * explicit JSON null.** Android's bundled `org.json` reads a JSON null back + * as the four-character string `"null"`, so a contact pinned before their + * fnid was known reloaded with `fnid = "null"` — a non-null value that is + * not an address. What that caused: + * + * - `Trust.pin` compares a newly offered fnid against the pinned one and + * treats a difference as a different person. `"null" != "fnid:…"`, so the + * first time a real address arrived the contact was marked **CHANGED** — + * the "security keys changed" warning, which is supposed to mean an + * attack, fired on an ordinary first sighting. + * - `rememberIdentity` returns early once `fnid != null`, so the real + * address was never resolved and the record stayed wrong for good. + * - `pinnedFnid` fed `"null"` into key-log verification, which recomputes a + * leaf against the address a contact was accepted under. Every check + * would have been against the wrong subject. + * + * An absent key reads as `""` on both platforms, which is what the reader + * has always expected. + */ fun saveTrust(userId: Long, trust: Trust) = writeJson(trustName(userId), JSONObject() .put("identities", org.json.JSONArray(trust.identities)) .put("verified", trust.verified) - .put("fnid", trust.fnid ?: JSONObject.NULL) + .also { if (trust.fnid != null) it.put("fnid", trust.fnid) } .put("changed", trust.changed)) fun forgetTrust(userId: Long) = storage.delete(trustName(userId)) diff --git a/engine/src/test/kotlin/io/flamenet/e2e/VaultTest.kt b/engine/src/test/kotlin/io/flamenet/e2e/VaultTest.kt index 003acc3..da915ad 100644 --- a/engine/src/test/kotlin/io/flamenet/e2e/VaultTest.kt +++ b/engine/src/test/kotlin/io/flamenet/e2e/VaultTest.kt @@ -315,4 +315,66 @@ class VaultTest { Vault(storage, secret).saveIdentity(identity) assertNotEquals(Bytes.b64(first), Bytes.b64(storage.records.getValue("identity"))) } + + // ---- Trust records and the platform's JSON --------------------------- + + /** + * 🔴 **A contact with no known address must come back with NO address.** + * + * `saveTrust` used to write `"fnid": null` explicitly, and Android's + * bundled `org.json` reads a JSON null back as the four-character string + * `"null"` — a non-null value that is not an address. `Trust.pin` compares a + * newly offered fnid against the pinned one and treats a difference as a + * different person, so the first time a real address arrived the contact was + * marked **CHANGED**: the "security keys changed" warning, which is meant to + * mean an attack, fired on an ordinary first sighting. + * + * ⚠️ The JVM's `org.json` returns `""` there, so this passed on a laptop and + * failed on a phone. The two tests below cover both halves — what is + * written now, and what has to be tolerated because it is already on + * devices in the field. + */ + @Test + fun `a trust record with no address writes no address at all`() { + val storage = MapStorage() + val secret = key() + Vault(storage, secret).saveTrust(7, Vault.Trust(listOf("aaa"), verified = false, fnid = null)) + + assertNull(Vault(storage, secret).loadTrust(7)?.fnid, + "an absent address came back as a value") + } + + /** + * ⚠️ **This is the test that reproduces the device**, and the one above is + * not. On the JVM a real JSON null reads back as `""` whether the writer + * omits the key or not, so the round trip above passes with the bug intact. + * Handing the reader the literal string is what Android actually does, so + * writing it here is what makes a laptop run tell the truth. + */ + + @Test + fun `a record already holding the literal string null reads as no address`() { + // 🔴 Records written the old way exist on phones already. The reader has + // to repair them, or those contacts stay permanently mis-addressed — + // `rememberIdentity` returns early once `fnid != null`, so nothing would + // ever ask again. + val storage = MapStorage() + val secret = key() + val vault = Vault(storage, secret) + vault.saveTrust(7, Vault.Trust(listOf("aaa"), verified = false, fnid = "null")) + + assertNull(Vault(storage, secret).loadTrust(7)?.fnid, + "the literal string \"null\" was accepted as somebody's global address") + } + + @Test + fun `a real address still round-trips`() { + val storage = MapStorage() + val secret = key() + Vault(storage, secret).saveTrust( + 7, Vault.Trust(listOf("aaa"), verified = true, fnid = "fnid:abc123")) + val back = Vault(storage, secret).loadTrust(7) + assertEquals("fnid:abc123", back?.fnid) + assertTrue(back!!.verified) + } }