From 9778682b9ed09036adc7c79bbfc9c2913972d102 Mon Sep 17 00:00:00 2001 From: Paul Hitt Date: Fri, 25 Sep 2026 20:20:31 -0400 Subject: [PATCH] Stretch the parental PIN, and keep app data off new phones Two findings, both surfaced by checking Android against what the iPhone app fixed on 2026-09-25. The parental PIN was the iPhone finding line for line: one round of SHA-256 with the salt beside it, in plaintext preferences, compared with `==`. Four digits is ten thousand candidates and a single SHA-256 is nanoseconds, so whoever could read the file had the PIN. It is now stretched with AccountKdf.master, PBKDF2-SHA256 at 650,000 iterations, the account passphrase's function and the one the iPhone moved to. The comparison is MessageDigest.isEqual. A PIN set before this still opens and is rewritten the first time it is entered, which is the only moment the PIN is in hand to do it. Only the three PIN fields change on that rewrite: not `enable`, which turns every gate back on, so a parent who had released a gate does not find it restored. The second is broader. The manifest turns allowBackup off and says why: the vault's key lives in the Keystore and never travels, so restoring its encrypted records elsewhere makes a vault nothing can open, presenting as a corrupt install. On Android 12 that stopped being true. Android's own documentation, for apps targeting 31 or higher: on some manufacturers' devices allowBackup="false" "disables cloud-based backup and restore ... but doesn't disable device-to-device transfers for the app". This app targets 35. So a phone-to-phone move could carry the vault records without their key, which is exactly the failure the comment was written to prevent, and the plaintext preferences with them. data_extraction_rules.xml excludes every domain, device-protected ones included, from both cloud backup and device transfer. Both sections are spelled out because the rules fail open: "If there are no rules for a particular backup mode, such as if the section is missing, that mode is fully enabled for all content." There is no cross-platform-transfer section. It exists from API 36.1, this compiles against 35, and it only sends to an iOS app named by bundle and team id, so naming none names no destination. The file says to revisit that on a target of 36.1 rather than assume. allowBackup stays: before Android 12 it is still the whole answer. Tests read the raw preferences rather than asking verify, since a verify that still hashed once passes any right-PIN/wrong-PIN check. The rules test checks all nine domains in both sections and that nothing is included, rather than that the file exists. Falsified three ways, each failing the test it should and nothing else: reverting the stretch to one SHA-256 (two PIN tests), routing the upgrade through enable (the gates test), and deleting the real device-transfer element (the rules test). The first attempt at the third was invalid and worth recording: the regex also matched the words inside the file's own comment, produced broken XML, the build failed, and the harness read the previous run's result files as a pass. Result files are now deleted before every run so a build that does not run cannot report green. App unit suite: 347 tests, the full count of @Test in app/src/test, no failures, errors or skips. Co-Authored-By: Claude Opus 5.5 --- app/src/main/AndroidManifest.xml | 8 + .../messenger/app/ParentalControls.kt | 75 ++++++++-- .../main/res/xml/data_extraction_rules.xml | 49 ++++++ .../app/NothingLeavesThePhoneTest.kt | 69 +++++++++ .../app/ParentalPinIsStretchedTest.kt | 139 ++++++++++++++++++ 5 files changed, 331 insertions(+), 9 deletions(-) create mode 100644 app/src/main/res/xml/data_extraction_rules.xml create mode 100644 app/src/test/kotlin/io/flamenet/messenger/app/NothingLeavesThePhoneTest.kt create mode 100644 app/src/test/kotlin/io/flamenet/messenger/app/ParentalPinIsStretchedTest.kt diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index 1722236..75c6e0b 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -62,6 +62,7 @@ + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/app/src/test/kotlin/io/flamenet/messenger/app/NothingLeavesThePhoneTest.kt b/app/src/test/kotlin/io/flamenet/messenger/app/NothingLeavesThePhoneTest.kt new file mode 100644 index 0000000..e1fae4b --- /dev/null +++ b/app/src/test/kotlin/io/flamenet/messenger/app/NothingLeavesThePhoneTest.kt @@ -0,0 +1,69 @@ +package io.flamenet.messenger.app + +import org.w3c.dom.Element +import java.io.File +import javax.xml.parsers.DocumentBuilderFactory +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** + * App data stays on the phone: no cloud backup, and no phone-to-phone + * transfer. + * + * `allowBackup="false"` alone stopped being enough on Android 12 for an app + * targeting 31 or higher, and this one targets 35. The data extraction rules + * are what carry the promise now, and they fail OPEN: a missing section, or + * one with no rules in it, means that mode transfers everything. So this + * checks every domain in both sections rather than that the file exists. + */ +class NothingLeavesThePhoneTest { + private val domains = + setOf( + "root", "file", "database", "sharedpref", "external", + "device_root", "device_file", "device_database", "device_sharedpref", + ) + + private val rules = File("src/main/res/xml/data_extraction_rules.xml") + private val manifest = File("src/main/AndroidManifest.xml") + + private fun section(name: String): Element? { + val doc = + DocumentBuilderFactory.newInstance().newDocumentBuilder().parse(rules) + return doc.getElementsByTagName(name).item(0) as? Element + } + + private fun excludedDomains(section: Element): Set { + val nodes = section.getElementsByTagName("exclude") + return (0 until nodes.length) + .map { nodes.item(it) as Element } + .filter { it.getAttribute("path") == "." } + .map { it.getAttribute("domain") } + .toSet() + } + + @Test + fun `the rules file is read at all`() { + assertTrue(rules.isFile, "no data_extraction_rules.xml, so nothing below means anything") + assertTrue(rules.readText().contains("")) + } + + @Test + fun `every domain is kept out of cloud backup and device transfer`() { + for (mode in listOf("cloud-backup", "device-transfer")) { + val s = section(mode) + // A missing section is not "off", it is "everything". + assertTrue(s != null, "no <$mode> section, so that mode transfers everything") + assertEquals(domains, excludedDomains(s!!), "<$mode> leaves a domain out") + assertEquals(0, s.getElementsByTagName("include").length, "<$mode> includes something") + } + } + + @Test + fun `the manifest points at the rules and still turns backup off`() { + val m = manifest.readText() + assertTrue(m.contains("android:dataExtractionRules=\"@xml/data_extraction_rules\"")) + // Still the whole answer on the Android versions before 12. + assertTrue(m.contains("android:allowBackup=\"false\"")) + } +} diff --git a/app/src/test/kotlin/io/flamenet/messenger/app/ParentalPinIsStretchedTest.kt b/app/src/test/kotlin/io/flamenet/messenger/app/ParentalPinIsStretchedTest.kt new file mode 100644 index 0000000..67163b9 --- /dev/null +++ b/app/src/test/kotlin/io/flamenet/messenger/app/ParentalPinIsStretchedTest.kt @@ -0,0 +1,139 @@ +package io.flamenet.messenger.app + +import android.content.Context +import io.flamenet.e2e.AccountKdf +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.RuntimeEnvironment +import org.robolectric.annotation.Config +import java.io.File +import java.security.MessageDigest +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertNotEquals +import kotlin.test.assertNull +import kotlin.test.assertTrue + +/** + * The parental PIN is stretched, and a PIN set before that still works. + * + * Until 2026-09-25 this was one round of SHA-256 with the salt beside it, the + * same finding the iPhone app fixed that day. Four digits is ten thousand + * candidates, so a single SHA-256 left the PIN readable by anybody who could + * read the file. These tests read the raw preferences rather than asking + * `verify`, because a `verify` that still hashed once would pass every + * right-PIN/wrong-PIN check there is. + */ +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [34]) +class ParentalPinIsStretchedTest { + private val context: Context get() = RuntimeEnvironment.getApplication() + private val prefs get() = context.getSharedPreferences("parental-controls", Context.MODE_PRIVATE) + + private fun hex(b: ByteArray) = b.joinToString("") { "%02x".format(it) } + + private fun unhex(s: String) = s.chunked(2).map { it.toInt(16).toByte() }.toByteArray() + + private fun sha256Once( + pin: String, + salt: ByteArray, + ) = MessageDigest.getInstance("SHA-256").apply { update(salt) }.digest(pin.toByteArray()) + + /** Write a record exactly as every build before the stretch did. */ + private fun seedLegacy( + pin: String, + contacts: Boolean = true, + stories: Boolean = true, + calls: Boolean = true, + ): String { + val salt = ByteArray(16) { it.toByte() } + val hash = hex(sha256Once(pin, salt)) + prefs + .edit() + .clear() + .putString("pinSalt", hex(salt)) + .putString("pinHash", hash) + .putBoolean(ParentalControls.CONTACTS_GATED, contacts) + .putBoolean(ParentalControls.STORIES_HIDDEN, stories) + .putBoolean(ParentalControls.CALLS_DISABLED, calls) + .commit() + return hash + } + + @BeforeTest + fun clean() { + prefs.edit().clear().commit() + } + + @Test + fun `a new PIN is stretched, not hashed once`() { + ParentalControls.enable(context, "4821") + val salt = unhex(prefs.getString("pinSalt", null)!!) + val stored = prefs.getString("pinHash", null)!! + + assertEquals("pbkdf2-sha256-650000", prefs.getString("pinKdf", null)) + assertEquals(hex(AccountKdf.master("4821", salt)), stored, "not the account KDF") + assertNotEquals(hex(sha256Once("4821", salt)), stored, "still one round of SHA-256") + } + + @Test + fun `the right PIN opens it and the wrong one does not`() { + ParentalControls.enable(context, "4821") + assertTrue(ParentalControls.verify(context, "4821")) + assertFalse(ParentalControls.verify(context, "4820")) + assertFalse(ParentalControls.verify(context, "")) + } + + @Test + fun `a PIN set before the stretch still opens, and is upgraded`() { + val oldHash = seedLegacy("1234") + assertNull(prefs.getString("pinKdf", null), "the seed is not a legacy record") + + assertTrue(ParentalControls.verify(context, "1234"), "an existing PIN stopped working") + + assertEquals("pbkdf2-sha256-650000", prefs.getString("pinKdf", null), "not upgraded") + val salt = unhex(prefs.getString("pinSalt", null)!!) + val newHash = prefs.getString("pinHash", null)!! + assertNotEquals(oldHash, newHash) + assertEquals(hex(AccountKdf.master("1234", salt)), newHash) + // And it still opens afterwards, now through the stretched path. + assertTrue(ParentalControls.verify(context, "1234")) + } + + @Test + fun `a wrong PIN on an old record changes nothing`() { + val oldHash = seedLegacy("1234") + assertFalse(ParentalControls.verify(context, "9999")) + assertNull(prefs.getString("pinKdf", null), "upgraded on a wrong PIN") + assertEquals(oldHash, prefs.getString("pinHash", null)) + } + + /** + * The trap the iPhone port named: upgrading through `enable` would turn + * every gate back on, and a parent who had released one would find it + * restored without having touched it. + */ + @Test + fun `upgrading keeps the gates as the parent left them`() { + seedLegacy("1234", contacts = false, stories = true, calls = false) + assertTrue(ParentalControls.verify(context, "1234")) + assertFalse(prefs.getBoolean(ParentalControls.CONTACTS_GATED, true), "contacts gate came back on") + assertTrue(prefs.getBoolean(ParentalControls.STORIES_HIDDEN, false), "stories gate was dropped") + assertFalse(prefs.getBoolean(ParentalControls.CALLS_DISABLED, true), "calls gate came back on") + } + + /** + * `==` on the hex strings stopped at the first differing character. Read + * from source because timing is not something a unit test can measure + * reliably, and this is the line that decides it. + */ + @Test + fun `the comparison is constant time`() { + val source = File("src/main/kotlin/io/flamenet/messenger/app/ParentalControls.kt").readText() + assertTrue(source.contains("fun verify("), "ParentalControls.kt was not read") + assertTrue(source.contains("MessageDigest.isEqual(candidate, stored)")) + assertFalse(source.contains("== stored"), "a short-circuiting comparison is back") + } +}