5.7.0 — Replace the build-time database key with a per-install key - #3
Open
aditya-I0063 wants to merge 11 commits into
Open
aditya-I0063 wants to merge 11 commits into
aditya-I0063 wants to merge 11 commits into
Conversation
Onboarding titles/descriptions, Skip/Next and the splash tagline rendered in
English in all 16 locales. Root cause: 14 strings were marked
translatable="false" in values/strings.xml, which both excluded them from every
values-*/ folder and suppressed the MissingTranslation lint check that would
have caught it. The Kotlin was already using stringResource() correctly.
Translating the headings alone would have crashed the app on first launch:
TextHighlighter located the emphasised word with fullText.indexOf("Secure") and
passed the result straight to AnnotatedString.addStyle. For any translated
heading indexOf returns -1, and addStyle(start = -1, ...) throws. The highlight
is now marked up as [[word]] inside each string resource, so translators choose
the emphasised word in their own language, and a pure-Kotlin parser strips the
markers. Unbalanced or absent markers degrade to unstyled text instead of
throwing. Headings auto-size 36-64sp so longer languages do not clip.
Also in this release:
- Arabic was offered by the in-app picker but missing from locales_config.xml,
so selecting it silently did nothing. locales_config, values-*/ and the picker
now agree on all 17 locales.
- Saving a preview without renaming it matched itself in the duplicate-heading
check, reported "heading exists" and discarded the edit.
- DetailViewModel trimmed on create but not on edit, so trailing whitespace
crept into stored passwords.
- The bottom navigation announced the raw enum name ("BANKS") in every locale,
and the alpha-0 FAB spacer was reachable by TalkBack.
- The details screen's empty state read "No Previews Found".
- PasswordGenerator used kotlin.random.Random rather than SecureRandom, and did
not guarantee a character from each selected class.
- Removed 10 string resources that were unused in code (170 entries across all
locales), so they are not translated needlessly.
- proguard-rules.pro kept net.sqlcipher.**, but the shipped artifact is
net.zetetic:sqlcipher-android. Those rules matched nothing. The library
supplies its own consumer rules; verified against mapping.txt that all 59
net.zetetic classes are kept unrenamed without any app-side rule, so the dead
rules were removed rather than replaced.
MissingTranslation, ExtraTranslation, ImpliedQuantity and the string-format
checks are now build-breaking errors, which is what stops this recurring.
Verified on an API 36 emulator: onboarding launches without crashing in hi, ar,
de, ta, ja and en, with markers stripped and text localized. 16 unit tests pass,
lint passes, release build is green and the Room schema is unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Phase 0 of 5.6.0, landed on its own so the upgrade can be verified in isolation before any behavioural change is built on top of it. AGP 9 turned out to be a structural migration rather than a version bump, and two constraints forced deviations from the plan: - AGP 9 has built-in Kotlin support and rejects the 'org.jetbrains.kotlin.android' plugin outright, so that plugin is removed. Kotlin is still pinned through the compose-compiler plugin version, and the resolved compiler is 2.3.21 (verified via the buildscript classpath, which overrides AGP's bundled 2.2.10). - The plan targeted Kotlin 2.4.20, but no KSP release exists for the 2.4.x line: the newest KSP is 2.3.12, whose symbol-processing-api depends on kotlin-stdlib 2.3.20. Since Hilt and Room both run through KSP, the buildable target is the 2.3.x line. Kotlin is therefore 2.3.21, not 2.4.20. Also required, and not anticipated by the plan: - Hilt 2.57.2 failed against AGP 9 with "Android BaseExtension not found"; upgraded to 2.60.1. - androidx.hilt 1.4.0 requires compiling against API 37, so compileSdk moves 36 -> 37. targetSdk stays at 36: compiling against newer APIs does not opt the app into new runtime behaviour, and that is a separate decision. - google-services 4.5.0 and firebase-crashlytics 3.0.8 for AGP 9 compatibility. Library versions: Compose BOM 2026.09.00, core-ktx 1.19.0, activity-compose 1.13.0, appcompat 1.8.0, lifecycle 2.11.0, navigation-compose 2.10.1, coroutines 1.11.0, datastore 1.2.1, Room 2.8.5, sqlcipher-android 4.19.0, androidx.sqlite 2.7.1, reorderable 3.1.0, firebase-bom 34.19.0. Java and jvmTarget move 22 -> 21 (LTS). androidx.biometric deliberately stays at 1.1.0: despite its age it is the newest stable release, and 1.2.0/1.4.0 are alpha-only. It already supports CryptoObject, which is all the 5.7.0 key work needs, and an alpha does not belong in the auth path of a password manager. material-icons-extended is frozen upstream at 1.7.8 and is no longer BOM-managed, so it now carries an explicit version rather than resolving to nothing. Other build hygiene: - local.properties was loaded unconditionally, throwing FileNotFoundException on any machine or CI checkout without it. Now guarded. - BuildConfig.PASS_PHRASE renamed to LEGACY_PASS_PHRASE to make its purpose explicit ahead of the 5.7.0 key migration. Same value, so existing vaults still open; AppModule carries a FIXME describing the replacement. - Dropped androidx.core:core-splashscreen, which was declared but never used (installSplashScreen is never called and no splash theme references it). - Added lifecycle-runtime-compose, needed for collectAsStateWithLifecycle. - Added mockk, turbine, kotlinx-coroutines-test and a testImplementation line for truth, which was previously androidTest-only. - opencsv is retained despite being unused: Phase 3 adopts it for the legacy CSV reader. Verified: clean assembleDebug, assembleRelease and lintDebug all green, 16 unit tests pass, all 65 net.zetetic classes still kept unrenamed by the newer R8, and onboarding still renders correctly with no crash in hi and ar on an API 36 emulator. Release APK drops 28M to 13M from the newer R8. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…zation Completes Phase 1 and brings §2.5 forward, since both rewrite the same code. ViewModels resolved strings eagerly via UiText.StringResource(...).asString(appContext). Below API 33 - which is most of this app's supported range, minSdk being 28 - AppCompatDelegate.setApplicationLocales applies the locale override per Activity only, so the injected Application context keeps the *system* locale and every string resolved there came back in the previous language after a user switched language in Settings. That is a second root cause of "the app renders in English", independent of the translatable="false" bug fixed in 5.5.2. UiEvents.ShowSnackBar now carries UiText unresolved and the screens resolve it against LocalContext, which does honour the override on every API level. UiText gains a PluralResource variant, and its subtypes are now data classes holding List<Any> rather than a class with a vararg array field, so effects compare by value - identity comparison would have silently broken the effect assertions planned for the ViewModel tests. The Add/Edit bottom sheet title was a *resolved English string* stored in SavedStateHandle. It is now a boolean the UI turns into text. Clipboard moves out of the ViewModels into a CopyToClipboard effect handled by the UI, which is also where it belongs given Android 10+ only lets a focused app write the clipboard. The new SecureClipboard sets EXTRA_IS_SENSITIVE so Android 13+ redacts the paste preview, and clears the clip after 30s - but only if the clip is still the one we wrote, checked by ClipDescription timestamp, so we never wipe something the user copied from another app meanwhile. Auto-clear is best-effort by construction and is documented as such: a delayed clear that fires while backgrounded is a silent no-op on API 29+. Copying an entry *heading* is marked non-sensitive; only the secret itself is. On Android 13+ the app's own "copied" snackbar is suppressed because the system shows its own confirmation. PreviewViewModel and DetailViewModel no longer take an Application at all. Remaining localization work from the plan: - All 19 hardcoded contentDescriptions addressed. Actionable controls get localized strings; decorative branding images get null, because a screen reader announcing "App Icon" next to a visible title is noise. - Hardcoded "Password Settings", "Length: N", "Invalid file selected." and the "$score%" percentage extracted - percent placement is locale-dependent. - First plurals resource in the project, replacing a singular-only "used N times". Quantity classes follow CLDR per language rather than mirroring English: Arabic gets six forms, Russian four, Spanish/French/Italian/Portuguese three, and Japanese/Korean/Chinese only "other". - 15 new strings plus the plural translated into all 16 locales. - Added values-night: the only theme hardcoded windowLightStatusBar=true against a Light parent, so night mode showed dark status-bar icons on a dark bar and flashed a white window on cold start. windowLightStatusBar is now unset so enableEdgeToEdge's SystemBarStyle.auto owns bar contrast, and an explicit windowBackground is defined for both modes. res/values/colors.xml was an empty <resources/> and now holds those. Lint caught a genuine defect during this work: context.getString inside a composable is not configuration-aware. Hoisted into composable scope. Verified: clean debug build, 16 unit tests, and lint all green; onboarding renders correctly in Hindi with no crash in both light and dark mode on an API 36 emulator. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three pieces of security hygiene plus the repository support Phase 3 needs. FLAG_SECURE was set on the Activity window only. Compose Dialog and ModalBottomSheet are separate windows: under the currently resolved Compose versions their securePolicy defaults to SecureFlagPolicy.Inherit, so they do inherit it today and this was a latent risk rather than an active leak - but it breaks silently if a default changes or a sheet is ever hosted off a non-secure window, and the password-analysis sheet renders weak and reused passwords. All five ModalBottomSheets and all three dialogs now pin SecureFlagPolicy.SecureOn explicitly rather than relying on inheritance. DetailViewModel kept the in-progress entry title and answer in SavedStateHandle, including freshly generated passwords. SavedStateHandle is serialized into the saved-instance-state bundle, which the system persists to disk under /data/system_ce/<user>/ - so cleartext secrets were being written outside the encrypted database. Both move to plain in-memory MutableStateFlows. The trade is that a half-typed entry is lost to process death, which is the right side of that trade for a password manager. The Add/Edit flag stays in SavedStateHandle, being a boolean rather than vault content. Added PasskeyRepository.runInTransaction and deleteAll, backed by Room's withTransaction, and used them immediately to fix reordering: both Preview and Detail reorder wrote N sequence updates in a bare loop, so an interruption left the list partially renumbered. Phase 3's import needs the same primitive to stop a malformed row leaving a half-populated vault. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Export wrote the entire vault as readable CSV into the public Downloads folder. The .passkey extension and application/passkey MIME type were cosmetic: the bytes were credentials in cleartext, visible to any app with media access and swept up by cloud backup tools. This is the single worst data-handling defect in the app. New .pkbak format: "PKBACKUP" magic, format version, KDF id and parameters, salt, nonce and length, followed by AES-256-GCM ciphertext over GZIPped JSON. The whole 51-byte header is passed to GCM as additional authenticated data, so the KDF identifier and its parameters are covered by the auth tag - otherwise an attacker could rewrite kdfId to name a weaker KDF and the file would still verify. There is a test for exactly that. Keys come from PBKDF2-HMAC-SHA512 at 600k iterations. Argon2id is stronger against GPU attack and is what the kdfId field exists to allow; it needs a native dependency and is planned alongside the 5.7.0 key work, at which point new files get kdfId 2 and this reader keeps handling kdfId 1. Files are written through SAF CreateDocument so the user picks the destination, using "wt" so an existing file is truncated rather than partially overwritten. The backup password is collected only after a destination is chosen, and entered twice with an explicit warning that it cannot be recovered. Import is hardened throughout: - format detection is by magic bytes, not filename. The old check matched a display name ending in "passkey", which would have rejected every new backup and trusted a user-supplied name to decide how to parse the contents. - the whole restore runs in one transaction, so a bad row part-way through a file can no longer leave a half-populated vault. - unknown categories map to OTHERS instead of Categories.valueOf throwing into a broad catch that silently abandoned the rest of the file. - field lengths, row counts and file size are bounded against a hostile file. - a wrong password and a corrupt file are deliberately indistinguishable, since GCM cannot tell them apart and pretending otherwise leaks whether a guess was close. The legacy plaintext CSV stays readable so existing backups still restore, now via opencsv - which was already a declared but entirely unused dependency - so quoted values containing commas parse correctly rather than being split blindly. Rows with the wrong column count are dropped and counted rather than aborting the import. The lossy "____" comma sentinel is un-escaped on read; nothing can recover a value that genuinely contained it, but that is what the file means. Only the reader survives; the CSV writer is gone. SettingsViewModel drops from 340 to 212 lines with the file I/O and MediaStore code removed. Manifest, now that SAF has removed the need for storage access: - READ_/WRITE_EXTERNAL_STORAGE deleted. They were capped at maxSdkVersion 28 while minSdk is 28, so they were live on exactly the oldest supported devices. - allowBackup="false". Platform backup would carry the SQLCipher database off the device, and with today's build-time key that copy is decryptable by anyone with the APK. backup_rules.xml and data_extraction_rules.xml were untouched IDE templates and are now correct regardless, so flipping the flag back cannot silently start uploading the vault. - INTERNET is declared explicitly. The app makes no network calls itself, but Firebase and Play in-app-update require it, and having it merged in invisibly from a library manifest is how the README came to claim otherwise. The merged manifest of a built APK also shows AD_ID and the AdServices permissions arriving from firebase-analytics, which the privacy rewrite has to disclose. - MainActivity is singleTask so a second instance cannot come up holding a stale unlocked session once app-lock lands. 24 new unit tests cover the crypto round-trip, tamper detection on both ciphertext and header, and the legacy CSV reader including the shapes the old exporter got wrong. Round-tripping a value containing commas, newlines, the ____ sentinel and emoji is now covered, none of which survived the old format. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
In-app updates were configured as AppUpdateType.IMMEDIATE - Play's blocking flavour - but were bypassable, because registerForActivityResult was given an empty callback. On RESULT_CANCELED or RESULT_IN_APP_UPDATE_FAILED nothing happened and the app continued straight into the vault, so a single back press dropped the gate for the whole session. It also blocked on every available update however trivial, since updatePriority and clientVersionStalenessDays were never consulted, and its FLEXIBLE branch was unreachable because updateType was a hardcoded val with no listener or completeUpdate() to make it work. Extracted into AppUpdateController, which: - treats a non-OK result from a required update as "still required" and shows a terminal screen with no path into the vault; - reserves IMMEDIATE for updatePriority >= 4 and uses FLEXIBLE once an update is a week stale, so ordinary releases no longer interrupt anyone. Note that inAppUpdatePriority can only be set through the Play Developer Publishing API at release-creation time and is immutable afterwards, so it belongs on the 5.7.0 release checklist rather than being discovered later; - implements FLEXIBLE properly with an InstallStateUpdatedListener registered in onStart and released in onStop, surfacing a non-blocking banner when the download completes. Deliberately not the blocking screen: a low-priority update that is merely ready should not lock someone out of their passwords, and calling completeUpdate() unprompted would restart the app from under them; - keeps the existing DEVELOPER_TRIGGERED_UPDATE_IN_PROGRESS resume, which was the one part of the original that was already right. Added a Remote Config min_supported_version_code gate. In-app updates are entirely client-side and do nothing for a sideloaded install or a device without Play, so this is the only mechanism that genuinely enforces a floor - and it is the kill switch if 5.7.0's key migration needs stopping mid-rollout. Failure to reach Remote Config leaves the app usable rather than bricking it. This has to ship in 5.6.0 precisely because 5.7.0 re-keys every existing user's database and is the release most worth forcing. Note it still does not remove the need to keep LEGACY_PASS_PHRASE indefinitely: a forced update shortens the tail of old installs but cannot eliminate it, since someone who never opens the app is never prompted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was no re-lock at all. Once past the biometric screen, backgrounding the app and returning granted full access indefinitely, with no idle timeout, and SecurityViewModel's isAuthenticated flag was written but never read - so navigation was the only thing standing between a picked-up phone and every stored password. VaultSession now holds the unlocked state that the rest of the app observes, and AppLockObserver watches ProcessLifecycleOwner rather than an Activity, so a configuration change or an internal screen transition is not mistaken for leaving the app. Timings use SystemClock.elapsedRealtime, never currentTimeMillis: wall clock lets someone extend their own grace period by changing the device clock, and it jumps on NTP sync. Foreground idle is driven by Activity.onUserInteraction, which is dispatched before the window handles the event and therefore sees touches and key presses without competing with Compose gesture consumption - considerably more robust than wrapping the tree in a pointerInput. On lock the clipboard is cleared first, since once locked there is no UI left to offer that, and navigation pops to the security gate with popUpTo(0) so every screen holding vault content is destroyed along with its nav-scoped ViewModel. The re-lock navigation is deliberately gated on the lock reason rather than merely "not unlocked": ColdStart is the initial state, and treating it as a lock would yank new users out of onboarding straight into a biometric prompt. Timeout is user-configurable in Settings - Immediately / 15s / 30s (default) / 1m / 5m / Never - persisted in DataStore. "Never" is a genuine footgun in a password manager, so choosing it surfaces an explicit warning rather than hiding behind a neutral label. All six options and the warning are translated into all 16 locales. In this release the session tracks lock state only; the database key is still the build-time constant. In 5.7.0 VaultSession also owns the decrypted key, and locking will zero it rather than flipping a boolean. Verified on an API 36 emulator: onboarding is unaffected on a fresh install, and a background/foreground cycle does not crash. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
PasswordAnalyzer decided whether a row held a secret by checking the question for the hardcoded English substrings "password", "pin", "code" and "secret". Across the app's 16 non-English locales nothing ever matched, so totalPasswords came back 0 and strengthScore came back 100: every translated user was told their vault was perfect and had no weak or reused passwords. A feature that silently lies is worse than one that is absent. Two changes fix it. Details gains an isSecret column (Room 2 -> 3, purely additive with a default, so no table rebuild) which is set explicitly when a value comes out of the password generator. And keyword matching is no longer hardcoded: the caller supplies R.array.secret_field_keywords, now translated for all 16 locales, unioned across the default and current locale so entries labelled before a language switch keep being classified. Matching is case-folded and accent-stripped, so "Contraseña" matches "contrasena". A SQL backfill in the migration was considered and rejected: the only heuristic expressible there is English keyword matching, which is precisely the bug. Existing rows default to 0 and continue to be classified by the now-localized keywords. Two scoring defects fixed while here: - reuse was penalised per distinct duplicated value rather than per affected entry, so three copies of one password scored the same as two. - reusedPasswords was keyed by the plaintext password, and that map is held in ViewModel state and rendered by the analysis sheet. It is now keyed by a truncated SHA-256 digest; there is no reason for cleartext passwords to sit in a composition-scoped object. Also added an all-identical-characters check to the weak-password heuristic. 9 new unit tests cover the locale behaviour, the isSecret flag, accent-insensitive matching, the weak heuristic and the scoring caps. MigrationTester gains 2 -> 3 and a full 1 -> 3 chain test, since someone who skipped several releases upgrades straight to the newest build; all 6 instrumented tests pass on an API 36 emulator. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…l locales The in-app About, Privacy and Terms text, README.md and Privacy Policy.md all stated that the app requires no internet permission and shares nothing with third parties. That was false in the shipped APK. Dumping the merged manifest of a build shows INTERNET, ACCESS_NETWORK_STATE, WAKE_LOCK, AD_ID and the Play AdServices permissions all arriving from firebase-analytics and the Play update library. The repo's Privacy Policy.md simultaneously contradicted the in-app copy by describing IP address and device name collection. All three in-app texts are rewritten to be accurate and, deliberately, far shorter. The previous About text was several hundred words of marketing that also claimed LiveData and MVVM (the app uses Flow), dark mode (half-implemented until this release) and Autofill (still not implemented). Accurate, concise copy is both better for users and tractable to translate. The rewritten text states plainly: vault entries are never collected, transmitted or stored by us and cannot be recovered by us; Firebase Crashlytics, Analytics and Performance Monitoring plus Play in-app updates send crash diagnostics, usage events, performance traces and device/installation identifiers including an advertising ID to Google, and never have access to vault contents; backups are the user's responsibility once written; and the vault is excluded from Android cloud backup and device transfer. Privacy Policy.md is rewritten around the actual architecture with a per-component table of what each one sends and a permission-by-permission justification. README.md drops "Fully Offline : No internet permission required", gains a security model section, and states the current database-key limitation openly rather than letting the feature list imply protection the build does not yet provide. With the text corrected it is now safe to translate, which completes the decision to localize everything: about_message, privacy_message and terms_n_condition_message are translated into all 16 locales. app_name is the only remaining translatable="false" string, being the brand name. These three are long-form legal and descriptive text produced by machine translation. They should get a native-speaker review pass before release; the technical claims in them are accurate, but phrasing and legal register have not been checked by a human for each language. Version bumped to 5.6.0 (versionCode 45). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…lots
The vault was encrypted with BuildConfig.PASS_PHRASE: one constant, identical on
every install, recoverable from the APK. The database was therefore protected
against someone who found the file alone and against nobody who also had the app.
Biometric unlock was decorative on top of that - the prompt carried no
CryptoObject, allowed BIOMETRIC_WEAK (which can never back a Keystore key), and
its only effect was setting a flag nothing read.
The key is now 32 bytes from SecureRandom, generated once per install and never
stored bare. It is wrapped three times, and any one slot opens the vault:
BIO Keystore key, per-use BIOMETRIC_STRONG, unwrapped through a CryptoObject.
Bound to the current enrolment, so changing fingerprints invalidates it -
which is the desired property for the everyday path.
CRED Keystore key, device credential, 30s validity. Deliberately NOT
invalidated by biometric enrolment, so the app self-heals after a
fingerprint change without ever creating an unauthenticated path.
REC Recovery password, Argon2id. Mandatory, because it is the only slot that
survives the user removing their device lock, which destroys every
auth-bound Keystore key. Without it there is a reachable state with
permanent, total data loss. KeySlotFile refuses to persist without it.
Unlock falls through BIO to CRED to REC. KeyPermanentlyInvalidatedException is
thrown by Cipher.init before any prompt is shown, so an invalidated enrolment is
detected silently and the user never sees a broken prompt; a successful CRED
unlock rebuilds the BIO slot behind their back.
Two hard constraints shaped this. androidx.biometric 1.1.0 throws
IllegalArgumentException below API 30 if DEVICE_CREDENTIAL is among the allowed
authenticators of a CryptoObject prompt, and minSdk is 28 - hence two distinct
prompts rather than one. And auth-bound Keystore keys cannot be generated at all
without a lock screen, so each Keystore slot is attempted independently: a device
with a PIN but no fingerprint still gets CRED, and a device with neither still
gets a working vault protected by the recovery password alone.
The re-key uses sqlcipher_export into a sidecar, not PRAGMA rekey. Rekey rewrites
every page in place with no journal protection across the operation, so an
interruption leaves an unopenable file; exporting keeps the original untouched
until a rename, which makes it atomic by construction. Key slots are persisted as
PENDING *before* the export - the reverse order has a window where the file is
re-keyed but the key exists nowhere.
Two bugs the instrumented tests caught, both of which would have shipped:
- ATTACH DATABASE does not accept a bind parameter for the path; passing one made
SQLite try to open a file literally named "?".
- the source connection is opened OPEN_READWRITE without CREATE, so ATTACH could
not create the target file. It is now created up front.
VaultDatabaseProvider replaces the eager @provides database, which forced an open
at ViewModel construction - before any authentication. The repository resolves
Room Flows through flatMapLatest over the current database, which is what lets
locking close it: collectors detach and re-subscribe instead of throwing
"attempt to re-open an already-closed object". Locking now actually closes the
database and zeroes the key bytes. The key is passed in SQLCipher raw-key form so
it skips a 256,000-round PBKDF2 on every open.
12 instrumented migration tests, including an explicit assertion that
user_version survives - sqlcipher_export does not copy it, and losing it makes
Room either re-run migrations against migrated tables or destructively recreate
them. 6 more cover the Argon2 native binding, which also gains explicit ProGuard
keeps: argon2kt ships no consumer rules and survives today only via the default
native-methods rule, which is too indirect to rely on in an unlock path.
Verified end to end on an API 36 emulator with no device lock - the hardest case,
since only the recovery slot can exist: the gate provisions, writes a REC-only
slot file, opens the vault, and the resulting database header is random bytes
where a plaintext SQLite file would read "SQLite format 3".
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…apping Three remaining items, and one significant bug that on-device testing exposed. The bug first. Keystore wrapping keys were AES, and an auth-bound symmetric key requires authentication for *every* operation - including encryption. So wrapping the vault key during provisioning threw UserNotAuthenticatedException, which the per-slot runCatching swallowed, and installs silently ended up with only the recovery slot. The device showed REC alone even with a PIN set; an instrumented capability test confirmed isDeviceSecure=true and createCredentialKey succeeding, which narrowed it to the wrap rather than the key generation. The wrapping keys are now RSA-OAEP key pairs. The public key encrypts with no authentication, so the vault key can be wrapped at setup without prompting, while the private key requires authentication to decrypt. Wrapping is free, unwrapping is gated - which is the property the whole design assumed and did not have. MGF1 is pinned to SHA-1 in the OAEP spec despite the SHA-256 digest, because AndroidKeyStore records one digest for the key and applies it to OAEP itself; passing MGF1 SHA-256 fails on several implementations. Recovery password change (Settings): requires the current password rather than reusing an in-memory key. Proving knowledge of the old password is what stops someone with a briefly unlocked phone from silently replacing the recovery credential with their own, and it means the key never has to be retained for the session. Implemented as two steps over the existing password dialog. Rescue export: the un-migrated vault can now be read straight through the legacy key and written out as an encrypted backup. Offered in two places - before the re-key for upgrading users, and again on the migration failure screen. The original database is untouched by a failed export, so a user is never stuck behind a migration they cannot complete. The failure screen still deliberately offers no "reset". Two UI defects fixed while wiring that: the export-result Toast was being shown from composition rather than an effect, and the "export a backup first" button was rendered behind a non-dismissible dialog where it could never be tapped. Device verification on a wiped API 36 emulator with a known PIN and no enrolled fingerprint: - provisioning creates REC and CRED, and correctly skips BIO, which cannot be generated without an enrolled biometric - relaunch shows the credential prompt, the PIN unwraps the key through the CRED slot, and the vault opens - backgrounding past the 30s auto-lock re-prompts instead of showing entries 21 instrumented tests pass, including a KeyStoreWrapper capability test that reports what the device can actually do, since production deliberately swallows these failures and falls back. Still unverified on hardware: the BIO slot, meaning the CryptoObject-bound fingerprint unlock and the enrollment-change fall-through. Emulator fingerprint enrolment could not be driven reliably. That path needs a physical device with fingerprints enrolled before rollout. Version bumped to 5.7.0 (versionCode 46). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Third of three stacked PRs. Merge #1 and #2 first — this branches from #2, so until then the diff here includes both.
This is the dangerous one: it re-keys the database every existing user's passwords live in. It should ship alone, on a staged rollout.
The problem
The vault was encrypted with
BuildConfig.PASS_PHRASE— one constant, identical on every install, recoverable from the APK. The database was protected against someone who found the file alone, and against nobody who also had the app. Biometric unlock was decorative on top of that: the prompt carried noCryptoObject, allowedBIOMETRIC_WEAK(which can never back a Keystore key), and its only effect was setting a flag nothing read.The design
The key is now 32 bytes from
SecureRandom, generated once per install and never stored bare. It is wrapped three times, and any one slot opens the vault:BIOBIOMETRIC_STRONG, realCryptoObjectCREDRECUnlock falls through BIO → CRED → REC.
KeyPermanentlyInvalidatedExceptionis thrown byCipher.initbefore any prompt is shown, so a changed enrolment is detected silently and the BIO slot is rebuilt behind the user's back.The wrapping keys are RSA-OAEP key pairs, not AES. An auth-bound symmetric key requires authentication for every operation including encryption, so wrapping the vault key at provisioning time throws
UserNotAuthenticatedException. With a key pair the public key encrypts freely and only the private key is gated. This was found by on-device testing — installs were silently ending up with only the recovery slot because arunCatchingwas swallowing it.The migration
Uses
sqlcipher_exportinto a sidecar, notPRAGMA rekey. Rekey rewrites every page in place with no journal protection, so an interruption leaves an unopenable file; exporting keeps the original untouched until a rename. Key slots are persisted asPENDINGbefore the export — the reverse order has a window where the file is re-keyed but the key exists nowhere.A rescue export reads the un-migrated vault straight through the legacy key and writes an encrypted backup. Offered before the re-key and again on the failure screen, so nobody is stuck behind a migration they cannot complete. The failure screen deliberately offers no "reset".
Two bugs the instrumented tests caught that would otherwise have shipped:
ATTACH DATABASEdoes not accept a bind parameter for the path, and the source connection is openedOPEN_READWRITEwithoutCREATEsoATTACHcould not create the target.Verification
21 instrumented tests, including an explicit assertion that
user_versionsurvives the export —sqlcipher_exportdoes not copy it, and losing it makes Room either re-run migrations against migrated tables or destructively recreate them.On a wiped API 36 emulator with a known PIN: provisioning creates
REC+CREDand correctly skipsBIO; relaunch prompts for the credential, the PIN unwraps the key and opens the vault; backgrounding past auto-lock re-prompts. The resulting database header is random bytes where a plaintext SQLite file readsSQLite format 3.PASS_PHRASEfrom the v5.5.1 release records and add it tolocal.properties. The migration triesBuildConfig.LEGACY_PASS_PHRASEthen the literal"null"(what a build with no property embeds). If the shipped builds used a value outside that list, existing vaults will not open. This cannot be verified from the repo.BIOslot is unverified on hardware. TheCryptoObjectfingerprint unlock and the enrolment-change fall-through have unit coverage, and the BIO-absent → CRED fall-through was exercised, but emulator fingerprint enrolment could not be driven reliably. This is the most-used path in production and needs a physical device with fingerprints enrolled.Also note
inAppUpdatePrioritycan only be set through the Play Developer Publishing API at release-creation time and is immutable afterwards — it belongs on this release's checklist.🤖 Generated with Claude Code