iamMehedi / Secured-Preference-Store

A cryptography library and a SharedPreferences wrapper for Android that encrypts the content with 256 bit AES encryption. The Encryption key is securely stored in device's KeyStore.
563 stars 97 forks source link

App crashes when the users changes the lock screen pattern #5

Closed nirvik66 closed 7 years ago

nirvik66 commented 7 years ago

I'm testing on a 4.4.4 device where the screen lock was set to None. I changed it to PIN and after that I'm getting a crash. Are you experiencing the same? It's one of the problematic cases pointed on The Forgetful Keystore where the keys are lost. java.lang.RuntimeException: java.io.IOException: Error while finalizing cipher at devliving.online.securedpreferencestore.SecuredPreferenceStore.a(SourceFile:46)

I debugged the code and the keys are not generated again, the key store contains the alias but it crashes when it's trying to load the key on the RSADecrypt method while ((nextByte = cipherInputStream.read()) != -1) {

Caused by: javax.crypto.BadPaddingException: error:0407106B:rsa routines:RSA_padding_check_PKCS1_type_2:block type is not 02 at com.android.org.conscrypt.NativeCrypto.RSA_private_decrypt(Native Method) at com.android.org.conscrypt.OpenSSLCipherRSA.engineDoFinal(OpenSSLCipherRSA.java:273) at com.android.org.conscrypt.OpenSSLCipherRSA.engineDoFinal(OpenSSLCipherRSA.java:297)

iamMehedi commented 7 years ago

This seems to be somewhat related to #2 I'll test it thoroughly when I can manage some time and let you know.

iamMehedi commented 7 years ago

@nirvik66 This is related to #2. The issue should not be there on API level 21 and upper.

nirvik66 commented 7 years ago

@iamMehedi I need to support API level 18 so I modified your classes to added some logic to recover from the key store loss. Basically removing all alias in the keystore and clearing the SecuredPreferenceStore and regenerating everything. Let me know if you're interested.

iamMehedi commented 7 years ago

@nirvik66 I'd like to take a look at the changes you've made. May be we can add that behavior as an optional migration/recovery policy to the library.

nirvik66 commented 7 years ago

@iamMehedi I forked your project and I created a new branch with the changes. https://github.com/nirvik66/Secured-Preference-Store/tree/feature/fix-keystore-key-loss

You can see all the changes on the last commit on the branch but mainly,

On Encryption Manager:

On SecuredPreferencesStore:

mcassiano commented 7 years ago

@nirvik66 Did your solution fix the crashes that happened because of this? Is it stable?

iamMehedi commented 7 years ago

@mcassiano The reason for the crashes are still there (Google hasn't changed the way keystore works), but the library has a way to recover(start over, so the app doesn't crash). See here https://github.com/iamMehedi/Secured-Preference-Store#recovery

mcassiano commented 7 years ago

@iamMehedi yes, I'm aware. I was just wondering if her solution is stable enough so I can use her branch. Thanks :)

mcassiano commented 7 years ago

@iamMehedi btw, I think I missed something. You need to explicitly set the DefaultRecoveryHandler if you want this behavior, right?

iamMehedi commented 7 years ago

yes, you need to explicitly set that.

mcassiano commented 7 years ago

@iamMehedi got it! thanks. :)

nirvik66 commented 7 years ago

@mcassiano Hey, sorry for the delay! I created that branch before the library implemented the recovery mechanism. Now, I wouldn't use my fork, it's better to use the library!