From f362db0ee7724a2f9b8b5e11f0cdea1870d5a28b Mon Sep 17 00:00:00 2001 From: Paul Hitt Date: Fri, 4 Sep 2026 13:38:55 -0400 Subject: [PATCH] Handle a wrong clock (ios#45, Android half) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🔴 A wrong device clock breaks a promise made to somebody else. A story disappearing after an hour is a promise the SENDER made, and it is 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. That asymmetry decides the design, and it is the whole of this change. Expiry takes the LATER of the device's clock and the relay's, never either alone. A device set back cannot extend a message's life, and a hostile relay claiming an earlier time cannot either, because the other source overrules it. The remaining failure — expiring early on a clock set forward — costs the reader something they were about to lose anyway. The skew is learned free, from the `Date` header every HTTP response already carries. Observed in `RelayClient.checked`, which is the one funnel every relay call passes through, and BEFORE the success check: a phone whose clock is wrong enough to be refused is exactly the phone that needs to be told why. Story and disappearing-message expiry on the app side now reads `DeviceClock.expirySeconds()` rather than `System.currentTimeMillis()`, at all seven sites. ⚠️ 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. 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. ⚠️ `SimpleDateFormat` is built per call rather than shared, because it is not thread-safe, with a fixed `Locale.US` and GMT: a formatter taking 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. 15 unit tests. Proven: replacing the rule with "trust the server" turns the hostile-relay test and the never-earlier test red. App builds, every JVM suite green. Co-Authored-By: Claude Opus 5 --- PARITY.md | 18 +- .../messenger/app/MessengerViewModel.kt | 3 +- .../io/flamenet/messenger/app/Session.kt | 13 +- .../kotlin/io/flamenet/client/DeviceClock.kt | 181 +++++++++++++++++ .../kotlin/io/flamenet/client/RelayClient.kt | 6 + .../io/flamenet/client/DeviceClockTest.kt | 190 ++++++++++++++++++ 6 files changed, 403 insertions(+), 8 deletions(-) create mode 100644 client/src/main/kotlin/io/flamenet/client/DeviceClock.kt create mode 100644 client/src/test/kotlin/io/flamenet/client/DeviceClockTest.kt 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) + } +} -- 2.54.0