Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
nirvik66 opened this issue Nov 3, 2016 · 12 comments
Closed

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

nirvik66 opened this issue Nov 3, 2016 · 12 comments
Labels

Comments

@nirvik66
Copy link

nirvik66 commented Nov 3, 2016

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
Copy link
Owner

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

@iamMehedi iamMehedi added the bug label Nov 5, 2016
@iamMehedi
Copy link
Owner

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

@nirvik66
Copy link
Author

nirvik66 commented Nov 7, 2016

@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
Copy link
Owner

@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
Copy link
Author

nirvik66 commented Nov 14, 2016

@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:

  • Removed all mentions about KeyProperties
  • Change the logic of generateRSAKeys
  • Added some methods to clear the keystore alias

On SecuredPreferencesStore:

  • Save a new key to be able to determine if the key we have on the keystore is good to decrypt the values
  • Modify the getSharedInstance to clear everything if the key is no longer valid

@mcassiano
Copy link

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

@iamMehedi
Copy link
Owner

@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
Copy link

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

@mcassiano
Copy link

mcassiano commented May 17, 2017

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

@iamMehedi
Copy link
Owner

yes, you need to explicitly set that.

@mcassiano
Copy link

@iamMehedi got it! thanks. :)

@nirvik66
Copy link
Author

@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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants