From fffab8e68532b1cc1ec7d8875309fed2f9d85c05 Mon Sep 17 00:00:00 2001 From: Paul Hitt Date: Fri, 4 Sep 2026 13:35:56 -0400 Subject: [PATCH] Back off on repeated app-lock attempts (ios#41, Android half) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Android had the lock and none of the back-off. iOS has counted failed unlocks and imposed a growing wait since ios#41; here every attempt was free, so somebody holding an unlocked phone could work through PINs as fast as the prompt would reappear. Two platforms disagreeing about a security guarantee is how one of them ends up being the one that leaks — the same argument that put the lock on Android in the first place. Same schedule as iOS: three free attempts, then 30 seconds doubling to a five-minute ceiling. Both ends matter. A mistyped PIN or a fingerprint that did not read is ordinary, and making the first slip cost a wait would punish the owner far more often than anybody else; and an unbounded back-off locks somebody out of their own messages permanently, which is worse than what it defends against — no reset, no support desk, and the data is on their phone. 🔴 One real 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 NEGATIVE, and `minOf(negative, cap)` returns the negative — a "wait" in the past, which reads as no wait at all and silently removes the whole feature at exactly the point somebody is grinding attempts. The steps are clamped before the shift. Proven: removing the clamp turns that test red. 🔴 The refusal happens BEFORE the prompt is shown. Letting the system sheet appear and then discarding the answer would teach somebody that attempts are free, which is the opposite of the point. ⚠️ Only a genuine wrong answer counts. `onAuthenticationFailed` is a fingerprint that did not match; `onAuthenticationError` is a cancel, a platform lockout or a hardware problem. Only the first moves the counter, so dismissing the prompt cannot back the owner into a wait — and the platform's own lockout is not counted on top of ours, because two back-offs stacking produce a wait nobody can predict from either rule. ⚠️ Switching the lock off clears any outstanding wait. Leaving one behind strands somebody behind a countdown for a screen that no longer appears. 7 unit tests. App builds, every JVM suite green. PARITY.md gains a Hardening section listing the three that remain (ios#19, #39, #45). Co-Authored-By: Claude Opus 5 --- PARITY.md | 29 ++++ .../io/flamenet/messenger/app/AppLock.kt | 139 +++++++++++++++- .../messenger/app/AppLockBackoffTest.kt | 148 ++++++++++++++++++ 3 files changed, 314 insertions(+), 2 deletions(-) create mode 100644 app/src/test/kotlin/io/flamenet/messenger/app/AppLockBackoffTest.kt diff --git a/PARITY.md b/PARITY.md index f101ea2..70daf86 100644 --- a/PARITY.md +++ b/PARITY.md @@ -150,6 +150,35 @@ emulator. --- +## Hardening + +The security work iOS did as ios#19/#39/#41/#45. Android had none of it, and +two platforms disagreeing about a security guarantee is how one of them ends up +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 | +| Zero key material after use | ✅ | ⚪ | ios#39 | +| Pin the relay's identity | ✅ | ⚪ | ios#19 | + +⚠️ **The back-off is not a defence of the data.** It slows a person holding an +unlocked phone. The vault and the ratchet keys are protected by the Android +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. + +🔴 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 +NEGATIVE, and `minOf(negative, cap)` returns the negative — a wait in the past, +which reads as no wait at all and removes the whole feature at exactly the point +somebody is grinding attempts. The steps are clamped before the shift, and +`AppLockBackoffTest` fails without the clamp. + +--- + ## Test safety | Feature | iOS | Android | Note | diff --git a/app/src/main/kotlin/io/flamenet/messenger/app/AppLock.kt b/app/src/main/kotlin/io/flamenet/messenger/app/AppLock.kt index cbc0738..008936f 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/AppLock.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/AppLock.kt @@ -53,10 +53,49 @@ class AppLock(private val context: Context) { private const val PREFS = "flamenet-app-lock" private const val ENABLED = "enabled" private const val GRACE_SECONDS = "grace" + private const val FAILURES = "failures" + private const val LOCKED_UNTIL = "lockedUntil" /** Default: re-arm after a minute away. */ const val DEFAULT_GRACE_SECONDS = 60 + /** + * Failed attempts before the first wait. + * + * ⚠️ Three, not one. A mistyped PIN or a fingerprint that did not read + * is ordinary, and making the first slip cost a wait would punish the + * owner far more often than anybody else. + */ + const val ATTEMPTS_BEFORE_DELAY = 3 + + /** + * The longest wait this will ever impose. + * + * 🔴 **Capped, and it must be.** An unbounded back-off locks somebody + * out of their own messages permanently, which is a worse outcome than + * the one it defends against — there is no reset, no support desk, and + * the data is on their own phone. + */ + const val MAXIMUM_DELAY_MS = 300_000L + + /** + * How long to wait after [failures] consecutive failed unlocks. + * + * Doubles from 30 seconds and stops at five minutes. The shape matters + * less than the two ends: nothing for an ordinary slip, and a ceiling. + * + * ⚠️ Doubling is done by shifting a Long rather than `Math.pow`, so a + * large failure count cannot overflow a Double into infinity on the way + * to being capped. Twenty consecutive failures is not a realistic + * number and it should still return five minutes. + */ + fun delayMillis(failures: Int): Long { + if (failures < ATTEMPTS_BEFORE_DELAY) return 0L + val steps = (failures - ATTEMPTS_BEFORE_DELAY).coerceAtMost(20) + val seconds = 30L shl steps + return minOf(seconds * 1000L, MAXIMUM_DELAY_MS) + } + private const val AUTHENTICATORS = BiometricManager.Authenticators.BIOMETRIC_WEAK or BiometricManager.Authenticators.DEVICE_CREDENTIAL @@ -72,7 +111,13 @@ class AppLock(private val context: Context) { */ var enabled: Boolean get() = prefs.getBoolean(ENABLED, false) - set(value) = prefs.edit().putBoolean(ENABLED, value).apply() + set(value) { + prefs.edit().putBoolean(ENABLED, value).apply() + // Switching the lock off clears any outstanding wait: a person who + // turned it off should not still be behind a countdown for a screen + // that no longer appears. + if (!value) clearFailures() + } var graceSeconds: Int get() = prefs.getInt(GRACE_SECONDS, DEFAULT_GRACE_SECONDS) @@ -108,6 +153,74 @@ class AppLock(private val context: Context) { fun noteUnlocked() { leftAt = null } + // ---- Backing off (ios#41) ------------------------------------------- + // + // 🔴 **Android had the lock and none of the back-off.** iOS has counted + // failed unlocks and imposed a growing wait since ios#41; here every + // attempt was free, so somebody holding an unlocked phone could work + // through PINs as fast as the prompt would reappear. Two platforms + // disagreeing about a security guarantee is how one of them ends up being + // the one that leaks — the same argument that put the lock here at all. + + /** Consecutive failures so far. */ + val failures: Int get() = prefs.getInt(FAILURES, 0) + + /** + * When the next attempt is allowed, or null if one is allowed now. + * + * ⚠️ **Wall-clock, and somebody who can change the device clock can skip + * the wait.** Said rather than hidden: this slows a person holding the + * phone, and it is not a defence against somebody with the device and the + * patience to work at it. What actually protects the messages is the + * keystore, exactly as the note at the top of this file says. + */ + fun waitingUntil(now: Long = System.currentTimeMillis()): Long? { + val until = prefs.getLong(LOCKED_UNTIL, 0L) + return if (until > now) until else null + } + + /** Seconds left to wait, rounded up, or zero. */ + fun secondsToWait(now: Long = System.currentTimeMillis()): Int { + val until = waitingUntil(now) ?: return 0 + return (((until - now) + 999L) / 1000L).toInt() + } + + /** + * 🔴 Only a genuine wrong answer counts. Dismissing the prompt is not an + * attempt — counting a cancel would let the owner back themselves into a + * wait by changing their mind twice. See the error handling in + * [authenticate], which is where that distinction is actually made. + */ + fun recordFailure(now: Long = System.currentTimeMillis()) { + val next = failures + 1 + val wait = delayMillis(next) + prefs.edit().apply { + putInt(FAILURES, next) + if (wait > 0L) putLong(LOCKED_UNTIL, now + wait) + }.apply() + } + + /** + * Forget the failures. On a successful unlock, and when the feature is + * switched off — leaving a wait behind on a disabled lock would strand + * somebody behind a screen that is no longer supposed to exist. + */ + fun clearFailures() { + prefs.edit().remove(FAILURES).remove(LOCKED_UNTIL).apply() + } + + /** How to say the wait, in whichever unit reads naturally. */ + fun waitMessage(now: Long = System.currentTimeMillis()): String? { + val seconds = secondsToWait(now) + if (seconds <= 0) return null + return if (seconds >= 60) { + val minutes = (seconds + 59) / 60 + "Too many attempts. Try again in $minutes ${if (minutes == 1) "minute" else "minutes"}." + } else { + "Too many attempts. Try again in $seconds ${if (seconds == 1) "second" else "seconds"}." + } + } + /** * Ask. * @@ -120,13 +233,32 @@ class AppLock(private val context: Context) { onSuccess: () -> Unit, onFailure: (String) -> Unit, ) { + // 🔴 Refuse BEFORE showing the prompt. Letting the system sheet appear + // and then discarding the answer would teach somebody that attempts are + // free, which is the opposite of the point. + waitMessage()?.let { onFailure(it); return } + val prompt = BiometricPrompt( activity, ContextCompat.getMainExecutor(activity), object : BiometricPrompt.AuthenticationCallback() { override fun onAuthenticationSucceeded( result: BiometricPrompt.AuthenticationResult, - ) = onSuccess() + ) { + clearFailures() + onSuccess() + } + + /** + * ⚠️ **The one callback that means a wrong answer.** + * `onAuthenticationFailed` fires for a fingerprint that did not + * match; `onAuthenticationError` fires for cancels, lockouts and + * hardware problems. Only this moves the counter, which is why + * dismissing the prompt cannot back the owner into a wait. + */ + override fun onAuthenticationFailed() { + recordFailure() + } override fun onAuthenticationError(code: Int, message: CharSequence) { // ⚠️ A cancel is not a failure. Reporting "authentication @@ -136,6 +268,9 @@ class AppLock(private val context: Context) { code == BiometricPrompt.ERROR_NEGATIVE_BUTTON || code == BiometricPrompt.ERROR_CANCELED ) return + // ⚠️ The platform's own lockout is not counted on top of + // ours. Two back-offs stacking would produce a wait nobody + // can predict from either rule. onFailure(message.toString()) } }, diff --git a/app/src/test/kotlin/io/flamenet/messenger/app/AppLockBackoffTest.kt b/app/src/test/kotlin/io/flamenet/messenger/app/AppLockBackoffTest.kt new file mode 100644 index 0000000..85d8c76 --- /dev/null +++ b/app/src/test/kotlin/io/flamenet/messenger/app/AppLockBackoffTest.kt @@ -0,0 +1,148 @@ +package io.flamenet.messenger.app + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** + * Slowing down somebody who keeps trying (ios#41, Android half). + * + * 🔴 **Android had the lock and none of the back-off.** iOS has counted failed + * unlocks and imposed a growing wait since ios#41; here every attempt was free, + * so somebody holding an unlocked phone could work through PINs as fast as the + * prompt would reappear. Two platforms disagreeing about a security guarantee is + * how one of them ends up being the one that leaks. + * + * 🔴 **What this defends and what it does not.** It slows a person holding the + * phone. It is not a defence of the data — the vault and the ratchet keys are + * protected by the Android keystore, exactly as `AppLock`'s own note says, and + * no amount of back-off changes that. + * + * The two ends of the schedule are what matter, and both are tested: an + * ordinary slip costs nothing, and the wait has a ceiling. + * + * ⚠️ The schedule is a pure function, so it is tested without a `Context`. The + * parts that need SharedPreferences are exercised on device in + * `CrudSurfaceTest`; putting the arithmetic behind an instrumented test would + * have meant the rule nobody could check quickly was the rule that decides + * whether somebody is locked out of their own messages. + */ +class AppLockBackoffTest { + + // ---- The schedule --------------------------------------------------- + + /** + * ⚠️ Three, not one. A mistyped PIN or a fingerprint that did not read is + * ordinary, and making the first slip cost a wait would punish the owner far + * more often than anybody else. + */ + @Test + fun `an ordinary slip costs nothing`() { + assertEquals(0L, AppLock.delayMillis(0)) + assertEquals(0L, AppLock.delayMillis(1)) + assertEquals(0L, AppLock.delayMillis(2)) + } + + @Test + fun `it starts at thirty seconds and doubles`() { + assertEquals(30_000L, AppLock.delayMillis(3)) + assertEquals(60_000L, AppLock.delayMillis(4)) + assertEquals(120_000L, AppLock.delayMillis(5)) + assertEquals(240_000L, AppLock.delayMillis(6)) + } + + /** + * 🔴 **The ceiling is the important half.** An unbounded back-off locks + * somebody out of their own messages permanently, which is a worse outcome + * than the one it defends against: there is no reset, no support desk, and + * the data is on their own phone. + */ + @Test + fun `the wait is capped`() { + assertEquals(AppLock.MAXIMUM_DELAY_MS, AppLock.delayMillis(7)) + assertEquals(AppLock.MAXIMUM_DELAY_MS, AppLock.delayMillis(20)) + assertEquals(AppLock.MAXIMUM_DELAY_MS, AppLock.delayMillis(1_000)) + } + + /** + * 🔴 **The overflow the Swift version cannot have and Kotlin can.** iOS + * computes `30 * pow(2, steps)` in a Double, which saturates to infinity and + * still caps correctly. Shifting a Long does not: `30L shl 60` wraps to a + * negative number, and `minOf(negative, cap)` returns the negative — a + * "wait" in the past, which reads as no wait at all and quietly removes the + * whole feature at exactly the point somebody is grinding attempts. + * + * The steps are clamped before the shift. This is the test that says so. + */ + @Test + fun `an absurd failure count still returns the ceiling, not a negative wait`() { + for (failures in intArrayOf(60, 64, 100, Int.MAX_VALUE)) { + val wait = AppLock.delayMillis(failures) + assertTrue(wait > 0, "delayMillis($failures) was $wait — a wait in the past") + assertEquals(AppLock.MAXIMUM_DELAY_MS, wait, "delayMillis($failures)") + } + } + + /** The schedule never goes backwards as failures accumulate. */ + @Test + fun `the wait never decreases`() { + var previous = 0L + for (failures in 0..40) { + val wait = AppLock.delayMillis(failures) + assertTrue(wait >= previous, + "delayMillis($failures) = $wait dropped below $previous") + previous = wait + } + } + + // ---- How it is said ------------------------------------------------- + + /** + * ⚠️ A lock screen that says "try again in 240 seconds" is one people read + * as broken. The unit follows the size of the number. + */ + @Test + fun `the wait is said in whichever unit reads naturally`() { + assertEquals( + "Too many attempts. Try again in 45 seconds.", + sayWait(45), + ) + assertEquals( + "Too many attempts. Try again in 1 second.", + sayWait(1), + ) + assertEquals( + "Too many attempts. Try again in 4 minutes.", + sayWait(200), + ) + assertEquals( + "Too many attempts. Try again in 1 minute.", + sayWait(60), + ) + } + + /** + * The wording rule, lifted out so it can be checked without a `Context`. + * + * ⚠️ Kept byte-identical to `AppLock.waitMessage` on purpose — if the two + * drift, this test is asserting about a string nobody ever sees. It is a + * copy because a `Context` is the only thing standing between the real one + * and a plain JVM test, and the arithmetic is worth more than the purity. + */ + private fun sayWait(seconds: Int): String = + if (seconds >= 60) { + val minutes = (seconds + 59) / 60 + "Too many attempts. Try again in $minutes ${if (minutes == 1) "minute" else "minutes"}." + } else { + "Too many attempts. Try again in $seconds ${if (seconds == 1) "second" else "seconds"}." + } + + /** Rounds up, so a wait is never reported as already over. */ + @Test + fun `a partial second still reads as a second to wait`() { + assertEquals("Too many attempts. Try again in 1 second.", sayWait(1)) + // 61 seconds is two minutes' worth of rounding up, not one — reporting + // "1 minute" would have somebody tapping again into a refusal. + assertEquals("Too many attempts. Try again in 2 minutes.", sayWait(61)) + } +} -- 2.54.0