refactor(security): derive backup exclusion set from DatabaseFiles
Make BackupPolicy.EXCLUDED_DATABASE_PATHS the true single source of truth by deriving it from DatabaseFiles.NAME and DatabaseFiles.ACCOUNTS_NAME plus their SQLite sidecars via a new DatabaseFiles.fileNames() helper, instead of a hand-maintained list. This adds libremail-accounts.db (accounts + encrypted credentials, split into their own DB by #118/#111) to the never-back-up set, matching the field's stated intent, so a newly added database can never silently fall out of the exclusions again. Also fix DatabaseFiles.clear to wipe the cache DB via context.deleteDatabase(NAME), which additionally removes the -mj* master-journal temp files the hand-rolled suffix list missed. It still wipes ONLY the cache DB (NAME) and never the accounts DB (ACCOUNTS_NAME), preserving the sign-in-survives-cache-wipe separation from #111. Update the backup XML comments (data_extraction_rules.xml, backup_rules.xml) to note libremail-accounts.db is also kept off-device by the strict include- allowlist, and extend the tests to assert the accounts DB is covered by the exclusion SoT and that the derivation stays in lockstep with the XML resources. There is no active backup leak today: the XML is a strict include-allowlist, so the accounts DB was already excluded by omission. This closes the SoT drift #118 introduced and the -mj* gap, so the security posture no longer depends on the allowlist staying strict by luck. Closes #103 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,6 +1,7 @@
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
package org.libremail.backup
|
||||
|
||||
import org.libremail.data.local.DatabaseFiles
|
||||
import org.libremail.data.settings.AppSettings
|
||||
|
||||
/**
|
||||
@@ -24,13 +25,16 @@ object BackupPolicy {
|
||||
"datastore/libremail_dbkey.preferences_pb",
|
||||
)
|
||||
|
||||
/** `databases`-dir-relative names that must never leave the device (encrypted credentials + mail cache). */
|
||||
val EXCLUDED_DATABASE_PATHS: List<String> = listOf(
|
||||
"libremail.db",
|
||||
"libremail.db-wal",
|
||||
"libremail.db-shm",
|
||||
"libremail.db-journal",
|
||||
)
|
||||
/**
|
||||
* `databases`-dir-relative names that must never leave the device: the encrypted mail cache
|
||||
* ([DatabaseFiles.NAME]) AND the accounts + encrypted-credentials database
|
||||
* ([DatabaseFiles.ACCOUNTS_NAME]), each with its SQLite sidecars. Derived from [DatabaseFiles]
|
||||
* rather than hand-listed, so a newly added database can never silently fall out of the
|
||||
* never-back-up set (issue #103).
|
||||
*/
|
||||
val EXCLUDED_DATABASE_PATHS: List<String> =
|
||||
DatabaseFiles.fileNames(DatabaseFiles.NAME) +
|
||||
DatabaseFiles.fileNames(DatabaseFiles.ACCOUNTS_NAME)
|
||||
|
||||
/**
|
||||
* Whether Android Backup may run for this app. Opt-in and OFF by default: nothing is backed up
|
||||
|
||||
@@ -2,9 +2,8 @@
|
||||
package org.libremail.data.local
|
||||
|
||||
import android.content.Context
|
||||
import java.io.File
|
||||
|
||||
/** Central name and wipe helper for the Room cache database file (and its SQLite sidecars). */
|
||||
/** Central names and wipe helper for the Room database files (and their SQLite sidecars). */
|
||||
object DatabaseFiles {
|
||||
|
||||
const val NAME = "libremail.db"
|
||||
@@ -17,15 +16,27 @@ object DatabaseFiles {
|
||||
const val ACCOUNTS_NAME = "libremail-accounts.db"
|
||||
|
||||
/**
|
||||
* Delete the database and any WAL/SHM/journal sidecars. Call only when no connection is open —
|
||||
* used by the "clear + re-sync" path when the encryption key is invalidated and the encrypted
|
||||
* database can no longer be decrypted.
|
||||
* The statically-nameable SQLite sidecars that accompany a database file. A transient `-mj*`
|
||||
* master journal can also exist, but its suffix is random and so can't be listed by name —
|
||||
* [clear] leans on [Context.deleteDatabase] to sweep that one up.
|
||||
*/
|
||||
private val SIDECAR_SUFFIXES = listOf("-wal", "-shm", "-journal")
|
||||
|
||||
/**
|
||||
* [name] plus each of its statically-nameable sidecars. The single source of truth for which
|
||||
* on-disk files make up a database file; `BackupPolicy` derives its never-back-up set from this
|
||||
* so a new database (or a new sidecar suffix) can never silently fall out of the exclusions.
|
||||
*/
|
||||
fun fileNames(name: String): List<String> = listOf(name) + SIDECAR_SUFFIXES.map { name + it }
|
||||
|
||||
/**
|
||||
* Delete the cache database ([NAME]) and every sidecar — including the `-mj*` master journal a
|
||||
* hand-rolled suffix list would miss — via [Context.deleteDatabase]. NEVER touches
|
||||
* [ACCOUNTS_NAME], so a cache-key invalidation keeps the user signed in (issue #111). Call only
|
||||
* when no connection is open — used by the "clear + re-sync" path when the encryption key is
|
||||
* invalidated and the encrypted database can no longer be decrypted.
|
||||
*/
|
||||
fun clear(context: Context) {
|
||||
val db = context.getDatabasePath(NAME)
|
||||
val dir = db.parentFile ?: return
|
||||
listOf("", "-wal", "-shm", "-journal").forEach { suffix ->
|
||||
File(dir, db.name + suffix).delete()
|
||||
}
|
||||
context.deleteDatabase(NAME)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -5,10 +5,11 @@
|
||||
which applies on API 31+. Backup is still gated by LibreMailBackupAgent (opt-in, OFF by default).
|
||||
|
||||
<include> makes this a strict allowlist: ONLY the libremail_settings DataStore is backed up. The
|
||||
Keystore-sealed cache passphrase (datastore/libremail_dbkey.preferences_pb) and the encrypted
|
||||
credentials + mail-cache database (libremail.db and its -wal/-shm/-journal side files) are kept
|
||||
off-device by being omitted from the allowlist; the mail cache re-downloads on the next sync and
|
||||
accounts are re-added on a new device. (Lint's FullBackupContent rule forbids <exclude> paths
|
||||
Keystore-sealed cache passphrase (datastore/libremail_dbkey.preferences_pb), the mail-cache
|
||||
database (libremail.db) and the accounts + encrypted-credentials database (libremail-accounts.db,
|
||||
which issue #111 split out of libremail.db) — each with their -wal/-shm/-journal side files — are
|
||||
kept off-device by being omitted from the allowlist; the mail cache re-downloads on the next sync
|
||||
and accounts are re-added on a new device. (Lint's FullBackupContent rule forbids <exclude> paths
|
||||
outside an <include>, so exclusion is expressed by omission rather than explicit <exclude> entries.)
|
||||
-->
|
||||
<full-backup-content>
|
||||
|
||||
@@ -11,8 +11,11 @@
|
||||
- datastore/libremail_dbkey.preferences_pb: the Keystore-sealed SQLCipher passphrase for the
|
||||
encrypted cache. The wrapping Keystore key is non-exportable and device-bound, so the
|
||||
ciphertext is useless anywhere else.
|
||||
- libremail.db (+ -wal/-shm/-journal): encrypted IMAP/OAuth credentials and the cached mail.
|
||||
The cache re-downloads on the next sync; accounts are re-added on a new device.
|
||||
- libremail.db (+ -wal/-shm/-journal): the cached mail; re-downloads on the next sync.
|
||||
- libremail-accounts.db (+ -wal/-shm/-journal): accounts, encrypted IMAP/OAuth credentials,
|
||||
per-account settings and signatures (issue #111 moved these out of libremail.db). The
|
||||
credentials are sealed with a device-bound key, so they would only ever restore as
|
||||
undecryptable ciphertext; accounts are re-added on a new device.
|
||||
(Lint's FullBackupContent rule forbids <exclude> paths outside an <include>, so the exclusions are
|
||||
expressed by simply not listing those paths rather than as explicit <exclude> entries.)
|
||||
-->
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
package org.libremail.backup
|
||||
|
||||
import org.junit.Test
|
||||
import org.libremail.data.local.DatabaseFiles
|
||||
import org.libremail.data.settings.AppSettings
|
||||
import kotlin.test.assertEquals
|
||||
import kotlin.test.assertFalse
|
||||
@@ -37,10 +38,35 @@ class BackupPolicyTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the credentials and mail-cache database is never eligible for backup`() {
|
||||
fun `the mail-cache database is never eligible for backup`() {
|
||||
assertTrue(BackupPolicy.EXCLUDED_DATABASE_PATHS.contains("libremail.db"))
|
||||
// WAL/SHM/journal side-files can hold recently written rows too.
|
||||
assertTrue(BackupPolicy.EXCLUDED_DATABASE_PATHS.contains("libremail.db-wal"))
|
||||
assertTrue(BackupPolicy.EXCLUDED_DATABASE_PATHS.contains("libremail.db-shm"))
|
||||
assertTrue(BackupPolicy.EXCLUDED_DATABASE_PATHS.contains("libremail.db-journal"))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the accounts and credentials database is never eligible for backup`() {
|
||||
// Since #111 the accounts + encrypted IMAP/OAuth credentials live in their OWN database file
|
||||
// (libremail-accounts.db), so it must be in the never-back-up set just like the cache.
|
||||
assertTrue(BackupPolicy.EXCLUDED_DATABASE_PATHS.contains("libremail-accounts.db"))
|
||||
assertTrue(BackupPolicy.EXCLUDED_DATABASE_PATHS.contains("libremail-accounts.db-wal"))
|
||||
assertTrue(BackupPolicy.EXCLUDED_DATABASE_PATHS.contains("libremail-accounts.db-shm"))
|
||||
assertTrue(BackupPolicy.EXCLUDED_DATABASE_PATHS.contains("libremail-accounts.db-journal"))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `excluded database paths are derived from DatabaseFiles so none can silently fall out`() {
|
||||
val derived = DatabaseFiles.fileNames(DatabaseFiles.NAME) +
|
||||
DatabaseFiles.fileNames(DatabaseFiles.ACCOUNTS_NAME)
|
||||
// Derived, not hand-maintained: the exclusion set is exactly the DatabaseFiles-known files —
|
||||
// no more (nothing stale) and no less (every DB + sidecar covered).
|
||||
assertEquals(derived, BackupPolicy.EXCLUDED_DATABASE_PATHS)
|
||||
listOf(DatabaseFiles.NAME, DatabaseFiles.ACCOUNTS_NAME).forEach { name ->
|
||||
DatabaseFiles.fileNames(name).forEach { path ->
|
||||
assertTrue(path in BackupPolicy.EXCLUDED_DATABASE_PATHS, "$path must never be backed up")
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
package org.libremail.backup
|
||||
|
||||
import org.junit.Test
|
||||
import org.libremail.data.local.DatabaseFiles
|
||||
import org.w3c.dom.Element
|
||||
import java.io.File
|
||||
import javax.xml.parsers.DocumentBuilderFactory
|
||||
@@ -73,4 +74,13 @@ class DataExtractionRulesTest {
|
||||
fun `full backup content (API 29-30) mirrors the same exclusions`() {
|
||||
assertSafe(parseSection(resource("backup_rules.xml"), "full-backup-content"))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `both the cache and accounts databases are guarded against the backup allowlist`() {
|
||||
// The exclusion SoT is derived from DatabaseFiles, so both databases flow into secretPaths
|
||||
// and are asserted-absent from every include section by assertSafe above. Pin that here so
|
||||
// the accounts + credentials DB added in #111 can't quietly drop out of the guarded set.
|
||||
assertTrue(secretPaths.contains("database:${DatabaseFiles.NAME}"))
|
||||
assertTrue(secretPaths.contains("database:${DatabaseFiles.ACCOUNTS_NAME}"))
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user