amethyst/docs/plans/2026-07-01-privacy-lock-security-review.md

---
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<Long?>`.
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.