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

Using PBKDF2WithHmacSHA1 with 10k iterations and 48B output is too slow #18

Closed cermakcz closed 9 years ago

cermakcz commented 9 years ago

I'd very much like to use this library, but the initialization of the key takes 3 seconds on my Nexus 5. Since this is done in Application's onCreate (because I need the settings there), it's not usable.

I understand that you're trying to make it as easy to use and as secure as possible, but would you consider allowing users of the library to provide their own key? Since the encryption is really more of an obfuscation (anyone can decompile the app and generate the same key), it wouldn't be a problem for me to have less security, but 100x faster startup time :)

Also here's some discussion about the slowness: http://stackoverflow.com/questions/24652602/pbkdf2withhmacsha1-key-generation-takes-too-long-on-android

scottyab commented 9 years ago

wow, 3 seconds, i didn't know it was that long. Maybe some kind of lite mode option would suitable or as you suggest providing own key could be good. I'll review that post.

scottyab commented 9 years ago

Hey @cermakcz Confirmed. I'm seeing init time of 4.4s on nexus 4. Let's make suppling your own key an enhancement. I'd welcome PR if you have time, otherwise I'd add to my todo.

cermakcz commented 9 years ago

Hey @scottyab, I started looking into it and so far it seems that the best way to do it would be creating another constructor which would take AesCbcWithIntegrity.SecretKeys instead of the password. Plus, it'd be nice to add another method to AesCbcWithIntegrity, namely generateKeyFromPassword(String password, String salt, int iterationCount, int keyLength), which would allow easy creation of the SecreyKeys with whatever strenght the user wants. But I'm not sure if it wouldn't violate the purpose of the library - providing simple and secure encryption. What do you think?

scottyab commented 9 years ago

Great minds think alike :+1: , I've just added a new constructor to take AesCbcWithIntegrity.SecretKeys and I'm just writing a unit test for it (before pushing). As for the changes to 'AesCbcWithIntegrity', personally it makes sense, it just gives more options the default are still stronger.

cermakcz commented 9 years ago

Thanks for the change, I just started working on it when I read that you've already done it :) I'll go and see if there's something I can do about the AesCbcWithIntegrity change.

Edit: pull request here: https://github.com/tozny/java-aes-crypto/pull/12

scottyab commented 9 years ago

Thanks for that. Hoping @tozny is ok with and can merge the PR. I'll pull once accepted. In Secure Preferences I'm actually using my fork of java-aes-crypto as wanted to reference the dependancy from maven central.

cermakcz commented 9 years ago

Yeah I noticed. That's why I was hoping you'd pull it to your fork and make a new version, like you said.

scottyab commented 9 years ago

@cermakcz i've pulled your fork of java-aes-crypto into my fork added unit test and published as 0.0.3. Just waiting on maven central to propagate, then I'll release a new version of secure prefs 0.1.3 dependent on com.scottyab:aes-crypto:0.0.3

cermakcz commented 9 years ago

@scottyab Cool, thanks.

scottyab commented 9 years ago

Fixed in 0.1.3 (once propagated to maven central)

Example...

AesCbcWithIntegrity.SecretKeys myKey = AesCbcWithIntegrity.generateKeyFromPassword(Build.SERIAL,AesCbcWithIntegrity.generateSalt(),1000);
            SharedPreferences securePrefs1000 = new SecurePreferences(this, myKey, "my_prefs_1000.xml");
mkaarthick commented 8 years ago

Hi, i tried using password, but still app lags.

scottyab commented 8 years ago

@mkaarthick I'm not sure I understand your comment. You can manually pass in your own keys created with generateKeyFromPassword method that takes a int of the iterationCount. To increase the speed (but reduce security) you can lower the iterationCount (default 10,000).

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

handlePasswordChange() also need iteration count parameter. All method is private static, so I can't override. If to find method not to build library, the only way is inheritance with SecurePreference and overwrite method handlePasswordChange(). But, this is not cool.