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

title, type, status, date

title	type	status	date
Privacy Lock — Password Storage Security Review	review	active	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.

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:

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:

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.