diff --git a/PARITY.md b/PARITY.md index 70daf86..7ba7ccd 100644 --- a/PARITY.md +++ b/PARITY.md @@ -159,7 +159,7 @@ being the one that leaks. | Feature | iOS | Android | Note | |---|:--:|:--:|---| | Back off on repeated app-lock attempts | ✅ | ✅ | ios#41 — 3 free attempts, then 30s doubling to a 5-minute ceiling | -| Handle a wrong clock | ✅ | ⚪ | ios#45 | +| Handle a wrong clock | ✅ | ✅ | ios#45 — skew learned free from the `Date` header; expiry takes the LATER of device and relay | | Zero key material after use | ✅ | ⚪ | ios#39 | | Pin the relay's identity | ✅ | ⚪ | ios#19 | @@ -169,6 +169,22 @@ keystore, and no amount of back-off changes that — the same wording `AppLock` itself uses, kept identical on both platforms so neither can be read as promising more. +🔴 **The clock rule is an asymmetry, not a correction.** A story disappearing +after an hour is a promise the SENDER made, enforced on the READER's device — so +a reader whose clock runs behind keeps a message the sender was told would be +gone, and nobody involved can tell. Expiry therefore takes the LATER of the +device's clock and the relay's: a device set back cannot extend a message's +life, and neither can a relay that lies about the time, because the other source +overrules it. The remaining failure, expiring early, costs the reader something +they were about to lose anyway. + +⚠️ Android needs synchronisation iOS does not. The Swift version is +`nonisolated(unsafe) static var` because its writers sit on one actor; here the +observation is written from whichever OkHttp thread finished a call and read from +the UI thread, so the two fields are `@Volatile` and updated together under a +lock. A torn read of "skew from now, timestamp from an hour ago" would age out an +observation just made, or trust one that had not been. + 🔴 One divergence from iOS, and it is a bug the Swift version cannot have. iOS computes `30 * pow(2, steps)` in a `Double`, which saturates to infinity and still caps correctly. Kotlin shifting a `Long` does not: `30L shl 60` wraps 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 6c21d79..21c4535 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt @@ -3,6 +3,7 @@ package io.flamenet.messenger.app import android.app.Application import androidx.lifecycle.AndroidViewModel import androidx.lifecycle.viewModelScope +import io.flamenet.client.DeviceClock import io.flamenet.client.AccountsClient import io.flamenet.client.RelayClient import io.flamenet.client.RelayAccounts @@ -1890,7 +1891,7 @@ class MessengerViewModel(app: Application) : AndroidViewModel(app) { runCatching { v.mergeImportedPayload( HistoryExport.open(bytes, passphrase), - nowSeconds = System.currentTimeMillis() / 1000.0) + nowSeconds = DeviceClock.expirySeconds()) } } diff --git a/app/src/main/kotlin/io/flamenet/messenger/app/Session.kt b/app/src/main/kotlin/io/flamenet/messenger/app/Session.kt index e2b6560..e8925e3 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/Session.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/Session.kt @@ -1,6 +1,7 @@ package io.flamenet.messenger.app import io.flamenet.client.Conversations +import io.flamenet.client.DeviceClock import io.flamenet.client.MessengerApi import io.flamenet.client.RelayClient import io.flamenet.e2e.Attachment @@ -896,7 +897,7 @@ class Session( } val arrived = everything.filter { it.content.isRenderable } val touched = mutableSetOf() - val now = System.currentTimeMillis() / 1000.0 + val now = DeviceClock.expirySeconds() val incomingStories = mutableListOf() // storyKey, who reacted, the emoji ("" takes one back). val storyReactionsIncoming = mutableListOf>() @@ -1125,7 +1126,7 @@ class Session( bytes, mime, width, height, caption, recipients, lifetimeSeconds = lifetimeSeconds, groupId = gid) if (posted.reached > 0) { - val now = System.currentTimeMillis() / 1000.0 + val now = DeviceClock.expirySeconds() vault.saveStories( vault.loadStories() + Vault.Story( id = posted.ref.id, @@ -1189,7 +1190,7 @@ class Session( : io.flamenet.client.Conversations.TextStoryPosted { val posted = conversations.postTextStory(text, toUserIds, lifetimeSeconds) if (posted.reached > 0) { - val now = System.currentTimeMillis() / 1000.0 + val now = DeviceClock.expirySeconds() vault.saveStories( vault.loadStories() + Vault.Story( id = posted.key, @@ -1224,7 +1225,7 @@ class Session( /** Everything still live, newest first. */ fun stories(): List { - val now = System.currentTimeMillis() / 1000.0 + val now = DeviceClock.expirySeconds() val all = vault.loadStories() // ios#54 — anything of YOUR OWN that has expired is archived here, // before `saveStories` drops it. This is the only moment it can be @@ -1340,7 +1341,7 @@ class Session( lifetimeSeconds = lifetimeSeconds, duration = duration, streamUrl = streamUrl) if (posted.reached > 0) { - val now = System.currentTimeMillis() / 1000.0 + val now = DeviceClock.expirySeconds() vault.saveStories( vault.loadStories() + Vault.Story( id = posted.ref.id, @@ -1793,7 +1794,7 @@ class Session( } fun markStorySeen(storyId: String) { - val now = System.currentTimeMillis() / 1000.0 + val now = DeviceClock.expirySeconds() vault.saveStories( vault.loadStories().map { if (it.id == storyId) it.copy(seen = true) else it }, now, diff --git a/client/src/main/kotlin/io/flamenet/client/DeviceClock.kt b/client/src/main/kotlin/io/flamenet/client/DeviceClock.kt new file mode 100644 index 0000000..a658caf --- /dev/null +++ b/client/src/main/kotlin/io/flamenet/client/DeviceClock.kt @@ -0,0 +1,181 @@ +package io.flamenet.client + +import java.text.SimpleDateFormat +import java.util.Date +import java.util.Locale +import java.util.TimeZone +import kotlin.math.abs +import kotlin.math.roundToInt + +/** + * What the time is, when the device's own answer cannot be trusted (ios#45). + * + * 🔴 **A wrong device clock breaks a promise made to somebody else.** Almost + * everything time-dependent here is a commitment: a story disappears after an + * hour, a message clears itself after a day. Those are promises the SENDER + * made, and they are enforced on the READER's device — so a reader whose clock + * is running behind keeps a message the sender was told would be gone, and + * nobody involved can tell. + * + * That asymmetry decides the whole design. Being wrong in one direction costs + * the reader something they were going to lose anyway; being wrong in the other + * direction breaks somebody else's expectation silently. So: + * + * 🔴 **For expiry, take the LATEST plausible time.** Never the device's alone, + * never the server's alone — whichever is further ahead. A device clock set back + * cannot extend a message's life, and neither can a relay that lies about the + * time, because the other source overrules it. The cost is that a device clock + * set FORWARD expires things early, which is the failure worth having. + * + * ⚠️ **The relay is a hint, not an authority.** It is told the time by nobody we + * trust, and a hostile one would like to move time backwards so that + * disappearing messages linger. Taking the later of the two is exactly what + * stops that being useful to it. + * + * ⚠️ **Process-wide and synchronised, not a singleton object with plain vars.** + * The Swift version can be `nonisolated(unsafe) static var` because the places + * that write it are on one actor; here the observation is written from whatever + * OkHttp thread finished a call and read from the UI thread, so the fields are + * `@Volatile` and the pair is updated under a lock. A torn read of "skew from + * now, timestamp from an hour ago" would age out an observation that had just + * been made, or trust one that had not. + */ +object DeviceClock { + + /** + * Difference between the last server time seen and this device's clock, in + * milliseconds. Positive means the device is BEHIND. + * + * ⚠️ Not persisted. A skew learned yesterday says nothing about today — the + * clock may have been corrected, or the phone rebooted with a fresh answer + * from the network. + */ + @Volatile + private var skewMillis: Long? = null + + /** When the skew was learned, so a very old observation can be ignored. */ + @Volatile + private var observedAt: Long? = null + + private val lock = Any() + + /** + * How far out the clock has to be before it is worth saying anything. + * + * ⚠️ Five minutes. Ordinary drift is seconds; a phone whose clock is out by + * more than this will have visible symptoms — stories that vanish, messages + * that will not send — and being told why is better than guessing. + */ + const val SUSPECT_THRESHOLD_MS = 300_000L + + /** An observation ages out rather than being trusted for ever. */ + const val OBSERVATION_LIFETIME_MS = 3_600_000L + + /** + * Learn the skew from a response's `Date` header. + * + * ⚠️ Free: every HTTP response already carries one, so this costs no extra + * request and no extra round trip. + */ + fun observe(serverMillis: Long, deviceNow: Long = System.currentTimeMillis()) { + synchronized(lock) { + skewMillis = serverMillis - deviceNow + observedAt = deviceNow + } + } + + /** Parse and record a `Date` header, if there is a usable one. */ + fun observe(header: String?, deviceNow: Long = System.currentTimeMillis()): Boolean { + val parsed = header?.let { parseHttpDate(it) } ?: return false + observe(parsed, deviceNow) + return true + } + + /** Forget what was learned. For tests, and for signing out. */ + fun forget() { + synchronized(lock) { + skewMillis = null + observedAt = null + } + } + + /** The skew, if it is recent enough to mean anything. */ + fun usableSkew(now: Long = System.currentTimeMillis()): Long? = synchronized(lock) { + val skew = skewMillis ?: return null + val at = observedAt ?: return null + if (now - at >= OBSERVATION_LIFETIME_MS) return null + skew + } + + /** Whether the clock is far enough out to be worth telling somebody about. */ + fun isSuspect(now: Long = System.currentTimeMillis()): Boolean { + val skew = usableSkew(now) ?: return false + return abs(skew) > SUSPECT_THRESHOLD_MS + } + + /** + * What to say about it, or null when there is nothing to say. + * + * ⚠️ Names the symptom before the cause. Somebody whose stories keep + * vanishing does not think "my clock is wrong"; they think the app is + * broken. + */ + fun warning(now: Long = System.currentTimeMillis()): String? { + val skew = usableSkew(now) ?: return null + if (abs(skew) <= SUSPECT_THRESHOLD_MS) return null + val minutes = (abs(skew) / 60_000.0).roundToInt() + val direction = if (skew > 0) "behind" else "ahead" + return "This phone's clock is about $minutes minutes $direction. " + + "Stories and disappearing messages may vanish early or linger, " + + "because both are timed. Turning on automatic date and time in " + + "Settings fixes it." + } + + /** + * The time to judge an expiry against, in milliseconds. + * + * 🔴 **The later of the device's clock and the server's.** Neither alone: a + * device set back would keep a disappearing message past the moment its + * sender was told it would go, and a relay that lied about the time could + * arrange the same thing. Taking whichever is further ahead means both have + * to be wrong in the same direction to extend anything, and the remaining + * failure — expiring early — costs the reader something they were about to + * lose anyway. + */ + fun nowForExpiry(deviceNow: Long = System.currentTimeMillis()): Long { + val skew = usableSkew(deviceNow) ?: return deviceNow + return maxOf(deviceNow, deviceNow + skew) + } + + /** + * The same in seconds, which is how the vault stores expiries. + * + * ⚠️ A Double of seconds, matching `Vault.Story.expires` — converting to a + * Long of seconds here would floor away sub-second precision on a boundary + * that decides whether a story is still on screen. + */ + fun expirySeconds(deviceNow: Long = System.currentTimeMillis()): Double = + nowForExpiry(deviceNow) / 1000.0 + + /** + * RFC 7231 `Date`, which is what an HTTP header carries. + * + * ⚠️ Fixed locale and zone. A formatter that picked those up from the device + * parses nothing on a phone set to most of the world — and a `SimpleDateFormat` + * is not thread-safe, so one is built per call rather than shared. This runs + * once per HTTP response, which is nowhere near hot enough to care. + */ + private fun parseHttpDate(header: String): Long? { + val formatter = SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss zzz", Locale.US) + formatter.timeZone = TimeZone.getTimeZone("GMT") + formatter.isLenient = false + return runCatching { formatter.parse(header)?.time }.getOrNull() + } + + /** For tests that want to state a server time rather than compute one. */ + fun httpDate(millis: Long): String { + val formatter = SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss zzz", Locale.US) + formatter.timeZone = TimeZone.getTimeZone("GMT") + return formatter.format(Date(millis)) + } +} diff --git a/client/src/main/kotlin/io/flamenet/client/RelayClient.kt b/client/src/main/kotlin/io/flamenet/client/RelayClient.kt index b85db39..ab8547f 100644 --- a/client/src/main/kotlin/io/flamenet/client/RelayClient.kt +++ b/client/src/main/kotlin/io/flamenet/client/RelayClient.kt @@ -802,6 +802,12 @@ class RelayClient( } private fun checked(response: Response, path: String): Response { + // ios#45 — learn the clock skew from the header every response already + // carries. Done HERE because this is the one funnel every relay call + // passes through, and before the success check, so a 4xx teaches us the + // time just as well as a 200 does: a phone whose clock is wrong enough + // to be refused is exactly the phone that needs to be told why. + DeviceClock.observe(response.header("Date")) if (response.isSuccessful) return response val body = response.body?.string().orEmpty() val code = response.code diff --git a/client/src/test/kotlin/io/flamenet/client/DeviceClockTest.kt b/client/src/test/kotlin/io/flamenet/client/DeviceClockTest.kt new file mode 100644 index 0000000..a9f2329 --- /dev/null +++ b/client/src/test/kotlin/io/flamenet/client/DeviceClockTest.kt @@ -0,0 +1,190 @@ +package io.flamenet.client + +import kotlin.test.AfterTest +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertNull +import kotlin.test.assertTrue + +/** + * Handling a wrong device clock (ios#45, Android half). + * + * 🔴 **The rule under test is an asymmetry, not a correction.** A story + * disappearing after an hour is a promise the SENDER made, enforced on the + * READER's device. A reader whose clock runs behind keeps a message the sender + * was told would be gone, and nobody involved can tell. Being wrong the other + * way costs the reader something they were about to lose anyway. + * + * So expiry takes the LATER of the device's clock and the relay's, and these + * tests are mostly about that one decision holding in every direction. + */ +class DeviceClockTest { + + @BeforeTest fun clean() = DeviceClock.forget() + @AfterTest fun tidy() = DeviceClock.forget() + + private val now = 1_700_000_000_000L + + // ---- Learning the skew ---------------------------------------------- + + @Test + fun `a Date header teaches the skew`() { + assertTrue(DeviceClock.observe(DeviceClock.httpDate(now + 600_000L), now)) + val skew = DeviceClock.usableSkew(now) + // Positive means the device is BEHIND the server. + assertEquals(600_000L, skew) + } + + @Test + fun `an absent or unparseable header teaches nothing`() { + assertFalse(DeviceClock.observe(null as String?, now)) + assertFalse(DeviceClock.observe("not a date", now)) + assertFalse(DeviceClock.observe("", now)) + assertNull(DeviceClock.usableSkew(now)) + } + + /** + * ⚠️ A skew learned yesterday says nothing about today: the clock may have + * been corrected, or the phone rebooted with a fresh answer from the + * network. An observation that has aged out is the same as none. + */ + @Test + fun `an old observation is not used`() { + DeviceClock.observe(now + 600_000L, now) + assertEquals(600_000L, DeviceClock.usableSkew(now + 60_000L)) + assertNull(DeviceClock.usableSkew(now + DeviceClock.OBSERVATION_LIFETIME_MS)) + assertNull(DeviceClock.usableSkew(now + DeviceClock.OBSERVATION_LIFETIME_MS + 1)) + } + + // ---- The rule that matters ------------------------------------------ + + /** + * 🔴 **A device clock set BACK cannot extend a message's life.** This is the + * case the whole file exists for: the phone thinks it is earlier than it is, + * so on its own reading a disappearing message is still alive. The server's + * time overrules it. + */ + @Test + fun `a device running behind does not keep a message alive`() { + // Server says it is ten minutes later than this phone believes. + DeviceClock.observe(now + 600_000L, now) + assertEquals(now + 600_000L, DeviceClock.nowForExpiry(now)) + } + + /** + * 🔴 **And a relay that lies about the time cannot either.** A hostile relay + * would like to move time BACKWARDS so disappearing messages linger. Taking + * the later of the two means its answer is simply ignored. + */ + @Test + fun `a relay claiming an earlier time is ignored`() { + DeviceClock.observe(now - 600_000L, now) + assertEquals(now, DeviceClock.nowForExpiry(now), + "the relay's earlier time was taken, so a hostile relay can extend expiries") + } + + /** With nothing learned, the device's own clock is all there is. */ + @Test + fun `with no observation it is the device clock`() { + assertEquals(now, DeviceClock.nowForExpiry(now)) + } + + /** An aged-out observation cannot extend anything either. */ + @Test + fun `an expired observation falls back to the device clock`() { + DeviceClock.observe(now + 600_000L, now) + val later = now + DeviceClock.OBSERVATION_LIFETIME_MS + 1 + assertEquals(later, DeviceClock.nowForExpiry(later)) + } + + /** + * ⚠️ Both directions of the same rule in one place: whatever the relay says, + * the answer is never EARLIER than the device's own clock. + */ + @Test + fun `the answer is never earlier than the device clock`() { + for (offset in longArrayOf(-3_600_000L, -1L, 0L, 1L, 3_600_000L)) { + DeviceClock.forget() + DeviceClock.observe(now + offset, now) + assertTrue( + DeviceClock.nowForExpiry(now) >= now, + "an offset of $offset produced a time before the device's own", + ) + } + } + + // ---- Telling somebody ----------------------------------------------- + + @Test + fun `ordinary drift says nothing`() { + DeviceClock.observe(now + 4_000L, now) + assertFalse(DeviceClock.isSuspect(now)) + assertNull(DeviceClock.warning(now)) + } + + @Test + fun `a badly wrong clock is reported, with the symptom first`() { + DeviceClock.observe(now + 900_000L, now) + assertTrue(DeviceClock.isSuspect(now)) + val warning = DeviceClock.warning(now)!! + assertTrue(warning.contains("15 minutes behind"), warning) + // 🔴 The symptom before the cause. Somebody whose stories keep vanishing + // does not think "my clock is wrong"; they think the app is broken. + assertTrue(warning.contains("Stories and disappearing messages"), warning) + } + + @Test + fun `a clock running fast is described as ahead`() { + DeviceClock.observe(now - 900_000L, now) + assertTrue(DeviceClock.warning(now)!!.contains("15 minutes ahead")) + } + + /** Exactly at the threshold is not yet worth saying. */ + @Test + fun `the threshold is a boundary, not a range`() { + DeviceClock.observe(now + DeviceClock.SUSPECT_THRESHOLD_MS, now) + assertFalse(DeviceClock.isSuspect(now)) + DeviceClock.forget() + DeviceClock.observe(now + DeviceClock.SUSPECT_THRESHOLD_MS + 1, now) + assertTrue(DeviceClock.isSuspect(now)) + } + + // ---- Shape ---------------------------------------------------------- + + /** + * ⚠️ Seconds as a Double, matching `Vault.Story.expires`. Flooring to a Long + * of seconds here would lose the sub-second part on the boundary that + * decides whether a story is still on screen. + */ + @Test + fun `expiry seconds keep sub-second precision`() { + DeviceClock.observe(now + 1_500L, now) + assertEquals((now + 1_500L) / 1000.0, DeviceClock.expirySeconds(now)) + } + + @Test + fun `forget clears both halves of the observation`() { + DeviceClock.observe(now + 600_000L, now) + DeviceClock.forget() + assertNull(DeviceClock.usableSkew(now)) + assertFalse(DeviceClock.isSuspect(now)) + assertEquals(now, DeviceClock.nowForExpiry(now)) + } + + /** + * 🔴 The round trip through the header format, because that is what actually + * arrives. A formatter that took its locale from the device parses nothing + * on a phone set to most of the world, and the failure is silent — the + * header is simply never understood and the skew is never learned. + */ + @Test + fun `the header format round-trips`() { + val header = DeviceClock.httpDate(now) + assertTrue(header.endsWith("GMT"), header) + assertTrue(DeviceClock.observe(header, now)) + // Formatting drops milliseconds, so the skew lands within one second. + assertTrue(kotlin.math.abs(DeviceClock.usableSkew(now)!!) < 1_000L) + } +}