Skip to content

Hash app lock secrets#149

Open
zerox80 wants to merge 1 commit intoopencloud-eu:mainfrom
zerox80:security/hash-app-lock-secrets
Open

Hash app lock secrets#149
zerox80 wants to merge 1 commit intoopencloud-eu:mainfrom
zerox80:security/hash-app-lock-secrets

Conversation

@zerox80
Copy link
Copy Markdown
Contributor

@zerox80 zerox80 commented May 8, 2026

What changed

  • Store PIN and pattern lock values as salted PBKDF2 hashes instead of plain text.
  • Migrate old plain text values after a successful unlock.
  • Stop logging the actual pattern while the user draws it.

Why

The app lock should not leave the PIN or pattern readable in preferences or logs. Existing users can still unlock once with the old value, then it gets upgraded.

Checks

  • git diff --check
  • Targeted rg scans for plain text app-lock storage and pattern value logs
  • Tests were fine

@zerox80 zerox80 marked this pull request as ready for review May 8, 2026 21:47
@zerox80 zerox80 force-pushed the security/hash-app-lock-secrets branch from 307affe to 87d5933 Compare May 9, 2026 08:07
@zerox80 zerox80 force-pushed the security/hash-app-lock-secrets branch from 84251c8 to 68ea357 Compare May 9, 2026 08:09
Copy link
Copy Markdown
Contributor

@guruz guruz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know what the intention was on how secure this app lock was needed to be. Maybe it's already considered secure enough for the use case so we don't need to bloat it? CC @kulmann

And instead of rolling your own hash etc, why not use https://developer.android.com/privacy-and-security/keystore or whatever else is current in last years?

@zerox80
Copy link
Copy Markdown
Contributor Author

zerox80 commented May 9, 2026

yeah, i see the point. my intention here was not to redesign app lock around keystore, but to remove the current plaintext storage/logging issue with a small migration-friendly change.

keystore would be the next level if we want to protect an encryption/signing key or bind access to device/user auth. that feels like a broader app-lock design decision though, especially around ux, backup/restore, key invalidation, and what threat model we actually want.

so from my side this pr is just a hardening step for the existing implementation. if we want to move app lock to a keystore-based design, i'd treat that as a separate follow-up rather than mixing it into this pr.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants