scottyab / secure-preferences

Android Shared preference wrapper than encrypts the values of Shared Preferences. It's not bullet proof security but rather a quick win for incrementally making your android app more secure.
1.53k stars 235 forks source link

Expand with iterationCount parameter to cover several use case #50

Closed ShinJJang closed 8 years ago

ShinJJang commented 8 years ago

Same with description in this link, https://github.com/scottyab/secure-preferences/pull/48

Hello there,

As mentioned in #18, default constructor of SecurePreference is too slow to create instance. In there, solution is to give parameter at AesCbcWithIntegrity.generateKeyFromPassword(...) like below.

AesCbcWithIntegrity.SecretKeys mykeys = AesCbcWithIntegrity.generateKeyFromPassword(password, salt, iterationCount);
SecurePreferences securePrefs = new SecurePreferences(getContext(), mykeys, "pref-file");

But if already use this library, we need to change password using handlePasswordChange(String newPassword, Context context) to solve the problem too slow to create instance. This method doesn't support not only iteration count to generate key, but also AES key.

If I solve above problem, next is to check is this migrated. Then if password changed with iteration, I make instance of SecurePreference. But also, there is blocker generateAesKeyName in constructor when pass password, not key. generateAesKeyName is default key generator of key, also don't support iteration count to generate key. But this method is private static. I can't override it. So inheritance of SecurePreference can't solve problem.

So What I did :

  1. Expand method constructor, handlePasswordChange, generateAesKeyName with iterationCount
  2. Make public generateAesKeyName for to prepare something to change logic (Maybe, protected is enough to cover this issue.)
  3. Add comment
  4. Add test to handlePasswordChange with iteration count
  5. Fix wrong method name. getUnencryptedString -> getEncryptedString (Value from sharePreference look like not encrypted, but not using original key, using hashPrefKey in this method)
  6. Fix wrong assignment to sharePrefFilename (Same with #42)

Thanks for reading. If you have any question or request, please let me know.

scottyab commented 8 years ago

Thanks for raising/fixing. PR merged. Closing.