--- title: Privacy Lock — Password Storage Security Review type: review status: active date: 2026-07-01 --- # Privacy Lock — Password Storage Security Review Review of `PasswordHasher.kt` and the prefs-file storage that backs `PrivacyLockSettings.passwordHashed`. Companion to the shipped feature at [`2026-06-30-feat-messaging-privacy-lock-plan.md`](2026-06-30-feat-messaging-privacy-lock-plan.md). ## What was reviewed - `desktopApp/.../security/PasswordHasher.kt` (88 LOC) - `commons/.../jvmAndroid/.../PreferencesPrivacyLockSettings.kt` — specifically the `passwordHashed` field and its persistence path - Storage: `java.util.prefs.Preferences.userRoot().node("com/vitorpamplona/amethyst/privacylock")` - macOS: `~/Library/Preferences/com.apple.java.util.prefs.plist` - Linux: `~/.java/.userPrefs/com/vitorpamplona/amethyst/privacylock/prefs.xml` - Windows: `HKEY_CURRENT_USER\Software\JavaSoft\Prefs\...` ## Threat model (from the parent plan) Defended: shoulder-surfing on an unattended-but-unlocked device. Explicitly NOT defended: rooted device, live RAM extraction, hostile forensics, attacker with filesystem access + unlimited compute. ## What's good | # | Finding | |---|---| | G1 | Uses PBKDF2 (a proper KDF) rather than plain SHA hashing. | | G2 | `SecureRandom` for salt generation. | | G3 | 16-byte (128-bit) salt — meets OWASP recommendation. | | G4 | 256-bit output key length — collision-resistant. | | G5 | HmacSHA256 — modern PRF; not deprecated. | | G6 | Constant-time comparison via XOR-OR loop — protects against timing side-channels. | | G7 | `PBEKeySpec.clearPassword()` in `finally` block — clears the char array PBEKeySpec holds internally. | | G8 | Fresh salt generated on every `hash()` call — no cross-user salt reuse. | | G9 | Salt embedded in hash string via `$` separator — standard practice; salt doesn't need to be secret. | | G10 | On `RemovePasswordDialog` success, the hash is fully removed from prefs (see `PrivacyLockSettingsScreen` toggle-off path). No stale hash persistence. | ## Findings ### 🟨 M1 — PBKDF2 iteration count is below current OWASP recommendation **Severity:** MEDIUM **Current:** `ITERATIONS = 100_000` **Recommended:** `600_000` (OWASP Password Storage Cheat Sheet, 2023 revision, for PBKDF2-HMAC-SHA256). **Impact:** With 100k iterations at ~50ms per verify: - Weak human password (lowercase 6-char, `26^6 = 308M` combos) crackable in **~10 minutes** on a single RTX 4090 (~500K PBKDF2-SHA256 h/s). - 6-char mixed alphanumeric (`62^6 = 56B` combos) → ~31 hours single GPU; ~1 hour on modest hash farm. At 600k iterations both times multiply by 6× — still not unbreakable, but buys real time. The extra UX cost is ~250 ms of unlock latency (100k = ~50 ms, 600k = ~300 ms) — well within the tolerable range for a lock users unlock a handful of times per session. **Fix:** One-line change: ```kotlin private const val ITERATIONS = 600_000 ``` Existing hashes remain verifiable because iteration count is embedded neither in the stored string nor the KDF spec — the verify path uses the same iteration count. This means **bumping the constant retroactively invalidates existing hashes**. To avoid forcing every existing user to re-set: either (a) accept the invalidation and prompt for a new password, or (b) add a versioned hash format (`v2$salt$hash`) and keep `v1` at 100k during a migration window. See M2 for the versioned format. ### 🟨 M2 — No versioned hash format blocks smooth KDF migration **Severity:** MEDIUM (blocks future security improvements) **Current:** Hash format is `saltB64$hashB64` — no version prefix. **Impact:** Migrating to a stronger KDF later (Argon2id, higher PBKDF2 iterations) requires either forcing every user to re-set their password or ambiguous parsing. **Fix:** Change format to `v1$saltB64$hashB64`. Update `verify()` to switch on the prefix. Old bare-format hashes gracefully migrate on next `hash()` call (which happens on Change/Remove). ### 🟨 M3 — No app-layer rate-limiting on verify attempts **Severity:** MEDIUM **Current:** `DesktopMessagesLockGate.LockScreen` and `RemovePasswordDialog` both call `PasswordHasher.verify` synchronously with no attempt counter, backoff, or lockout. **Impact:** A UI attacker with brief physical access can attempt ~20 passwords per second (limited only by PBKDF2 verify time). Over 5 minutes of unattended access that's 6,000 attempts — enough to try every 4-digit PIN, every English 3-letter word, all common patterns. **Fix:** Add exponential backoff at the state-holder level. Suggested schedule: | Failures | Lockout | |---|---| | 1–4 | 0 s | | 5 | 30 s | | 6 | 60 s | | 7 | 120 s | | 8 | 300 s (5 min) | | 9+ | 300 s (capped) | Store `failedAttempts: Int` + `lockedUntilEpochMs: Long?` in `PrivacyLockSettings`. Reset counter on successful unlock. Persist across app restarts so reboot-loop doesn't reset. Add to `MessagesLockState` as `val remainingLockoutMs: StateFlow`. Lock screen shows "Try again in 27 seconds" during lockout. ### 🟨 M4 — Hash stored in unencrypted user-readable prefs file **Severity:** MEDIUM (documented threat model already excludes this) **Current:** The hash lives in `java.util.prefs` — an unencrypted XML file (Linux) or system-wide plist (macOS) or registry entry (Windows), readable by ANY process running as the current user. **Impact:** Any other process the user launches (a browser extension, a compromised VS Code plugin, malware installed as user, another Amethyst-writing process) can read the hash and mount an offline attack at CPU/GPU speed. The parent plan's Limitations copy already acknowledges "does not protect against filesystem access" — this is not new information. But it is worth surfacing explicitly to the user in the Settings pane copy and possibly a link to a doc. **Fix:** Not blocking — matches the accepted threat model. Consider: - Update the "Limitations" card in `PrivacyLockSettingsScreen.kt` to mention "your password hash is stored in your OS user preferences file without additional encryption; anyone with access to your user account can read it. Choose a strong password (12+ characters recommended) or trust your OS-level disk encryption (FileVault / BitLocker / LUKS)." - Consider migrating storage to OS keyring (already used by `SecureKeyStorage` for the nsec). But that's a larger refactor — the keyring wraps binary payloads well but retrieval requires the OS credential every time, undermining the "quick unlock" UX. ### 🟩 L1 — String-based password intake **Severity:** LOW **Current:** Compose `TextField` returns `String`. The value lives in composable state until GC. Converted to `CharArray` before hashing but the immutable String remains. **Impact:** A memory-dump attacker can recover the password from `Snapshot`-managed String references. Matches Signal Desktop, WhatsApp Desktop, and every other JVM password field. **Fix:** Not fixable in Compose today without going out-of-tree. Accepted per the threat model (memory dumps are out of scope). ### 🟩 L2 — CharArray passed to `PasswordHasher` is not caller-side-wiped **Severity:** LOW **Current:** `PasswordField.value.toCharArray()` at call sites. The returned `CharArray` is passed to `PasswordHasher.hash` / `.verify`, then dropped. The array is not explicitly zeroed. **Impact:** Char[] contents live until GC. Same class as L1. **Fix:** Trivial: ```kotlin val ca = value.toCharArray() try { PasswordHasher.verify(ca, hash) } finally { ca.fill('') } ``` Wraps at every call site. Marginal defense-in-depth. Do it. ### 🟩 L3 — Old prefs persist after app uninstall **Severity:** LOW **Current:** `java.util.prefs` outlives app uninstall — the file/keys remain until the OS user account is deleted or someone runs the Amethyst prefs cleanup manually. **Impact:** Reinstalling the app inherits an old privacy-lock configuration (`lockEnabled=true` + stale hash) if the user forgot to disable before uninstalling. **Fix:** Document in the release notes. Consider adding a "Reset privacy-lock data" affordance in Settings. ### 🟩 L4 — `RemovePasswordDialog` "Wrong password" reveals hash existence **Severity:** LOW-INFORMATIONAL **Current:** The dialog only appears when a hash exists (`stored != null`) and shows a wrong-password error inline. This reveals hash existence to someone who opens Settings. **Impact:** Negligible — the Switch state (`lockEnabled=true`) already reveals the lock is active. Hash existence is not a secret. **Fix:** None needed. ## Comparison against similar apps (2026) | App | KDF | Params | Rate-limit | At-rest | |---|---|---|---|---| | **Amethyst (current)** | PBKDF2-SHA256 | 100k iter, 16B salt | None | Unencrypted prefs | | Signal Desktop (via SVR) | Argon2id | Enclave-tuned | 7-day after 10 fails | SGX enclave | | Bitwarden Desktop | Argon2id | 3 iter, 64 MB, 4 par | None | LocalStorage | | KeePassXC | Argon2id / AES-KDF | Auto-tuned to 1s | None | AES-256 whole DB | | macOS FileVault | PBKDF2-SHA512 | ~100k | Progressive backoff | AES-XTS | | **Amethyst (with fixes)** | PBKDF2-SHA256 | 600k iter, 16B salt | Exponential backoff | Unencrypted prefs (documented) | ## Priority summary ### P0 — ✅ done in this branch 1. **M1** — ✅ Bumped `ITERATIONS` 100k → 600k (`V1_ITERATIONS` in `PasswordHasher.kt`). Legacy 100k hashes still verify via the version prefix parse (M2). 2. **M3** — ✅ Exponential backoff wired end-to-end. `PrivacyLockSettings.failedUnlockAttempts` + `lockedUntilEpochMs` are persisted. `MessagesLockState.onFailedUnlockAttempt` / `onUnlockAttemptResetToZero` own the schedule (5 fails → 30 s, doubling, capped at 5 min). Lock screen + `RemovePasswordDialog` both show "Try again in Ns" countdown and disable the primary action during lockout. 4 unit tests cover threshold, base trip, doubling + cap, reset on success. ### P1 — ✅ M2 done, M4 deferred 3. **M2** — ✅ Versioned hash format shipped. `hash()` writes `v1$salt$hash`; `verify()` parses both bare (legacy 100k) and `v1` (600k) formats. `isLegacyFormat()` helper exposed for opportunistic re-hash callers. 4. **M4** — ⏸ Copy update deferred to a follow-up. Current Limitations card already covers "filesystem access" broadly. ### P2 — nice to have 5. **L2** — Wipe CharArray on caller side after hash/verify. 6. Future migration to Argon2id (M2 unblocks this). 7. "Reset privacy-lock data" affordance in Settings. ## Verdict The current implementation is **honest for the stated threat model (shoulder-surf)** but leaves meaningful hardening on the table. The biggest lever is P0-1 (iteration bump) which is a one-line change with 250 ms of extra unlock latency. The second-biggest is P0-2 (exponential backoff) which is a small state-holder addition. Together these lift the effective UI-attacker attempt rate from ~20 attempts/sec to ~3 attempts/min after 5 failures — a 400× improvement. Nothing in this review is a "block merge" — every finding was already implicitly acknowledged by the parent plan's threat-model framing. But the P0 items are cheap and high-value; they should be scheduled as either follow-up commits on this branch or a v1.1 PR.