Address security issue by storing hashed password rather than encrypted#29
Merged
Address security issue by storing hashed password rather than encrypted#29
Conversation
| currentAppLocker = new DefaultAppLock(currentApp); | ||
| currentAppLocker.enable(); | ||
| } | ||
| if (Build.VERSION.SDK_INT < Build.VERSION_CODES.ICE_CREAM_SANDWICH) return; |
Contributor
There was a problem hiding this comment.
Can we call here isSupportedApi instead of duplicating the code ?
Contributor
Author
Done in b715eef. Thanks for the review @daniloercoli! |
| } | ||
|
|
||
| /** | ||
| * Default App lock is available on Android-v14 or higher. |
Contributor
There was a problem hiding this comment.
Niptick: I think we should change the comment in something like:
True when an App lock is available. It could be either a the Default App lock on Android-v14 or higher, or a non default App lock.
Contributor
|
Feel free to merge this PR, once the issue with the comment reported in my last review is sorted (If you want to change it). |
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
2 participants
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.
Fix #27, most of the details are available in the issue.
I've tested with fresh installs and with upgrades from versions that used encryption-based security, both working as expected. Upgrading from the old MD5 hashing security (original security implementation) should work as it did before for unlocking, setting a new password will remove the stored MD5 password and use the security added in this PR.
In order to keep things backwards compatible
PASSWORD_ENC_SECRETmust remain as a build property. The secret is used to decrypt existing passwords only and, once decrypted, the stored password will be converted from encryption-based to hash-based security.Most of the changes are superficial (code cleanup). The relevant security changes are all in db8403c.
cc @daniloercoli