diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index 3d0de58..3205b75 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -17,6 +17,8 @@ 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 ed3fe0d..25278b8 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt @@ -1165,6 +1165,9 @@ class MessengerViewModel(app: Application) : AndroidViewModel(app) { */ private fun describe(failure: Throwable): String = when (failure) { is AccountsClient.Failure.BadCredentials -> "That handle or passphrase isn't right." + is AccountsClient.Failure.SessionExpired -> + "Your session expired. Sign out and back in to keep going." + is AccountsClient.Failure.HandleTaken -> "That handle is already taken." is AccountsClient.Failure.RateLimited -> "Too many attempts. Wait a few minutes." is AccountsClient.Failure.Transport -> failure.message.orEmpty() diff --git a/app/src/main/res/drawable/ic_launcher_foreground.xml b/app/src/main/res/drawable/ic_launcher_foreground.xml new file mode 100644 index 0000000..28fe24d --- /dev/null +++ b/app/src/main/res/drawable/ic_launcher_foreground.xml @@ -0,0 +1,27 @@ + + + + + + + + diff --git a/app/src/main/res/drawable/ic_launcher_monochrome.xml b/app/src/main/res/drawable/ic_launcher_monochrome.xml new file mode 100644 index 0000000..8570b52 --- /dev/null +++ b/app/src/main/res/drawable/ic_launcher_monochrome.xml @@ -0,0 +1,19 @@ + + + + + + + diff --git a/app/src/main/res/mipmap-anydpi-v26/ic_launcher.xml b/app/src/main/res/mipmap-anydpi-v26/ic_launcher.xml new file mode 100644 index 0000000..c78bee3 --- /dev/null +++ b/app/src/main/res/mipmap-anydpi-v26/ic_launcher.xml @@ -0,0 +1,6 @@ + + + + + + diff --git a/app/src/main/res/mipmap-anydpi-v26/ic_launcher_round.xml b/app/src/main/res/mipmap-anydpi-v26/ic_launcher_round.xml new file mode 100644 index 0000000..c78bee3 --- /dev/null +++ b/app/src/main/res/mipmap-anydpi-v26/ic_launcher_round.xml @@ -0,0 +1,6 @@ + + + + + + diff --git a/app/src/main/res/values/ic_launcher_background.xml b/app/src/main/res/values/ic_launcher_background.xml new file mode 100644 index 0000000..4b9e5bc --- /dev/null +++ b/app/src/main/res/values/ic_launcher_background.xml @@ -0,0 +1,5 @@ + + + + #121A2E + diff --git a/client/src/main/kotlin/io/flamenet/client/AccountsClient.kt b/client/src/main/kotlin/io/flamenet/client/AccountsClient.kt index 3a0b81e..7af3234 100644 --- a/client/src/main/kotlin/io/flamenet/client/AccountsClient.kt +++ b/client/src/main/kotlin/io/flamenet/client/AccountsClient.kt @@ -56,6 +56,19 @@ class AccountsClient( sealed class Failure(message: String) : Exception(message) { /** Wrong handle and wrong passphrase are the same answer, on purpose. */ object BadCredentials : Failure("that handle or passphrase isn't right") + + /** + * The session is no longer accepted — expired, or revoked elsewhere. + * + * 🔴 **Not the same as a wrong passphrase, and saying so matters.** + * Every 401 and 403 used to become [BadCredentials], so a session that + * simply aged out told the user "that handle or passphrase isn't + * right" — in the middle of sending a message, hours after they signed + * in successfully. The natural reading is that their password stopped + * working or was changed by someone else, and the natural response is + * to change a password that was never wrong. + */ + object SessionExpired : Failure("your session has expired — sign in again") object HandleTaken : Failure("that handle is already taken") object RateLimited : Failure("too many attempts — wait a few minutes") class Service(val status: Int) : Failure("the account service returned $status") @@ -114,10 +127,13 @@ class AccountsClient( .url(base.newBuilder().addPathSegments(path).build()) .post(body.toString().toRequestBody(JSON)) .header("Accept", "application/json") - .build() + .build(), + // Only these two actually present a passphrase, so only these two can + // meaningfully report one as wrong. + carriesCredentials = path == "sessions" || path == "accounts", ) - private fun send(request: Request): JSONObject { + private fun send(request: Request, carriesCredentials: Boolean = false): JSONObject { val response = try { http.newCall(request).execute() } catch (e: Exception) { @@ -131,7 +147,12 @@ class AccountsClient( } catch (_: Exception) { throw Failure.Transport("unreadable response") } - 401, 403 -> throw Failure.BadCredentials + // ⚠️ The SAME status means different things depending on what + // was sent. Rejecting a passphrase is BadCredentials; refusing + // a token the user cannot see is an expired session. + 401, 403 -> + throw if (carriesCredentials) Failure.BadCredentials + else Failure.SessionExpired 409 -> throw Failure.HandleTaken 429 -> throw Failure.RateLimited else -> throw Failure.Service(it.code) diff --git a/client/src/main/kotlin/io/flamenet/client/Trust.kt b/client/src/main/kotlin/io/flamenet/client/Trust.kt index 40e19c1..f90fbd0 100644 --- a/client/src/main/kotlin/io/flamenet/client/Trust.kt +++ b/client/src/main/kotlin/io/flamenet/client/Trust.kt @@ -128,6 +128,7 @@ class Trust( // 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) { + markChanged(userId, existing) return IdentityTrust.CHANGED } @@ -144,7 +145,15 @@ class Trust( // pass, and that is precisely the attack: the victim's real devices // still work, so nothing looks wrong, while a device the attacker // controls receives a copy of everything. - if (!encoded.all { existing.identities.contains(it) }) return IdentityTrust.CHANGED + if (!encoded.all { existing.identities.contains(it) }) { + markChanged(userId, existing) + return IdentityTrust.CHANGED + } + // Back to matching: an earlier scare that resolved itself should not + // leave the contact stuck behind an Accept button for ever. + if (existing.changed) { + vault.saveTrust(userId, existing.copy(changed = false)) + } return if (existing.verified) IdentityTrust.VERIFIED else IdentityTrust.PINNED } @@ -197,6 +206,13 @@ class Trust( /** 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 + // 🔴 CHANGED first, and it is persisted rather than recomputed. This + // function used to be unable to return it at all: CHANGED was worked + // out inside `record()` during a send and then dropped, so the header + // said "keys unchanged since you met" while every send was refused for + // exactly that reason — and the Accept button, which only shows for + // CHANGED, could never appear. + if (t.changed) return IdentityTrust.CHANGED if (t.identities.isEmpty()) return IdentityTrust.UNKNOWN return if (t.verified) IdentityTrust.VERIFIED else IdentityTrust.PINNED } @@ -239,10 +255,25 @@ class Trust( ?: vault.loadTrust(userId)?.fnid vault.saveTrust( userId, - Vault.Trust(offered.map(Wire::b64), verified = false, fnid = address), + // changed = false: this IS the acceptance. Leaving it set would + // re-raise the banner on the very keys the user just approved. + Vault.Trust(offered.map(Wire::b64), verified = false, fnid = address, + changed = false), ) } + /** + * Remember that something other than the pinned keys was offered. + * + * ⚠️ The pinned `identities` are left exactly as they were, and `verified` + * is dropped: whatever was checked in person was checked against the OLD + * key, so the tick cannot carry over to a key nobody has compared. + */ + private fun markChanged(userId: Long, existing: Vault.Trust) { + if (existing.changed && !existing.verified) return + vault.saveTrust(userId, existing.copy(changed = true, verified = false)) + } + /** Forget a person entirely — used when they are removed as a contact. */ fun forget(userId: Long) = vault.forgetTrust(userId) diff --git a/client/src/test/kotlin/io/flamenet/client/AccountsClientStatusTest.kt b/client/src/test/kotlin/io/flamenet/client/AccountsClientStatusTest.kt new file mode 100644 index 0000000..22c5702 --- /dev/null +++ b/client/src/test/kotlin/io/flamenet/client/AccountsClientStatusTest.kt @@ -0,0 +1,54 @@ +package io.flamenet.client + +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import kotlin.test.AfterTest +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertFailsWith + +/** + * What a 401 from the account service is allowed to mean. + * + * 🔴 Guards a bug found by driving the app: every 401 and 403 became + * [AccountsClient.Failure.BadCredentials], so a session that simply aged out + * told the user "that handle or passphrase isn't right" — mid-send, hours + * after a successful sign-in. The obvious reading is that the password stopped + * working, and the obvious response is to change one that was never wrong. + */ +class AccountsClientStatusTest { + + private lateinit var server: MockWebServer + + @BeforeTest fun start() { server = MockWebServer().also { it.start() } } + @AfterTest fun stop() = server.shutdown() + + private fun client() = AccountsClient(server.url("/").toString()) + + private fun unauthorized() = MockResponse().setResponseCode(401) + .setHeader("Content-Type", "application/json") + .setBody("""{"error":true,"reason":"unauthorized"}""") + + @Test + fun `a rejected passphrase is reported as bad credentials`() { + // The params lookup succeeds; the session request is what gets refused. + server.enqueue(MockResponse().setResponseCode(200) + .setHeader("Content-Type", "application/json") + .setBody("""{"kdfSalt":"AAAAAAAAAAAAAAAAAAAAAA==","kdfIterations":1,"kdfID":1}""")) + server.enqueue(unauthorized()) + + assertFailsWith { + client().signIn("someone", "wrong-passphrase") + } + } + + @Test + fun `a refused token is an expired session, not a wrong passphrase`() { + server.enqueue(unauthorized()) + // Any authenticated call that does NOT present a passphrase. Telling + // this user their passphrase is wrong would be a lie. + assertFailsWith { + client().parameters("someone") + } + } +} diff --git a/client/src/test/kotlin/io/flamenet/client/TrustTest.kt b/client/src/test/kotlin/io/flamenet/client/TrustTest.kt index 63ced60..c55985f 100644 --- a/client/src/test/kotlin/io/flamenet/client/TrustTest.kt +++ b/client/src/test/kotlin/io/flamenet/client/TrustTest.kt @@ -52,6 +52,64 @@ class TrustTest { private fun key(seed: Byte) = ByteArray(32) { seed } + // -- A detected change has to SURVIVE ------------------------------------ + + /** + * 🔴 The bug these guard, found by driving two real apps: `record()` worked + * out CHANGED during a send and then threw it away, because nothing stored + * it. `state()` could only answer UNKNOWN, VERIFIED or PINNED — so the chat + * header went on saying "keys unchanged since you met" while every send was + * refused with "identity keys changed", and the "Accept new keys" button, + * which only renders for CHANGED, could never appear. A contact who + * reinstalled was unmessageable for ever with no way out. + */ + @Test + fun `a detected key change is still CHANGED when asked again later`() { + val t = trust() + t.record(7, listOf(key(1))) + assertEquals(IdentityTrust.PINNED, t.state(7)) + + assertEquals(IdentityTrust.CHANGED, t.record(7, listOf(key(2)))) + // The point: ASK AGAIN. This is what the UI does, and it used to be + // told everything was fine. + assertEquals(IdentityTrust.CHANGED, t.state(7), + "the change was detected and then forgotten") + } + + @Test + fun `the pinned keys are kept, not replaced, by a detected change`() { + val t = trust() + t.record(7, listOf(key(1))) + t.record(7, listOf(key(2))) + // Still the ORIGINAL key. Detecting a substitution must not adopt it. + assertEquals(listOf(Wire.b64(key(1))), vault.loadTrust(7)?.identities) + } + + @Test + fun `a change drops the verified tick`() { + val t = trust() + t.record(7, listOf(key(1))) + t.setVerified(7, true) + assertEquals(IdentityTrust.VERIFIED, t.state(7)) + + t.record(7, listOf(key(2))) + // Whatever was compared in person was compared against the OLD key. + assertEquals(IdentityTrust.CHANGED, t.state(7)) + assertFalse(vault.loadTrust(7)!!.verified) + } + + @Test + fun `keys that match again clear the banner`() { + val t = trust() + t.record(7, listOf(key(1))) + t.record(7, listOf(key(2))) + assertEquals(IdentityTrust.CHANGED, t.state(7)) + + // A transient wrong answer should not strand the contact for ever. + assertEquals(IdentityTrust.PINNED, t.record(7, listOf(key(1)))) + assertEquals(IdentityTrust.PINNED, t.state(7)) + } + // -- Pinning ------------------------------------------------------------- @Test diff --git a/engine/src/main/kotlin/io/flamenet/e2e/Vault.kt b/engine/src/main/kotlin/io/flamenet/e2e/Vault.kt index 0f8b463..75525bb 100644 --- a/engine/src/main/kotlin/io/flamenet/e2e/Vault.kt +++ b/engine/src/main/kotlin/io/flamenet/e2e/Vault.kt @@ -219,6 +219,23 @@ class Vault(private val storage: Storage, private val key: SecretKey) { * purpose, so the client cannot tell them apart either. */ val fnid: String? = null, + + /** + * A key change was detected and has NOT been accepted yet. + * + * 🔴 **Without this the detection is thrown away.** `record()` computes + * CHANGED while a send is in flight and returns it; nothing persisted + * it, so `state()` could only ever answer UNKNOWN, VERIFIED or PINNED. + * The chat header therefore went on saying "keys unchanged since you + * met" while every send failed with "identity keys changed", and the + * "Accept new keys" button — which only renders for CHANGED — could + * never appear. A contact who reinstalled became permanently + * unmessageable, with the UI insisting nothing was wrong. + * + * ⚠️ The pinned `identities` stay as they were. This flag records that + * something ELSE was offered, not that we believe it. + */ + val changed: Boolean = false, ) fun loadTrust(userId: Long): Trust? = readJson(trustName(userId))?.let { o -> @@ -227,13 +244,15 @@ class Vault(private val storage: Storage, private val key: SecretKey) { identities = (0 until a.length()).map { a.getString(it) }, verified = o.optBoolean("verified", false), fnid = o.optString("fnid").ifEmpty { null }, + changed = o.optBoolean("changed", false), ) } 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)) + .put("fnid", trust.fnid ?: JSONObject.NULL) + .put("changed", trust.changed)) fun forgetTrust(userId: Long) = storage.delete(trustName(userId))