-
-
Notifications
You must be signed in to change notification settings - Fork 522
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
Cold start on android took 5-10 seconds longer than before the library update #630
Comments
I'm also seeing this after upgrading. Downgrading to 8.1.2 fixes the issue for now. Our users on Google Pixel devices running Android 14 are seeing a delay and the app not responding for 8 to 20 seconds. Here's the ANR stack trace:
And here are some logs, if that's helpful:
|
Also experiencing this as well... In Android 14 on physical devices there's a noticeable delay when calling |
+1 |
We are also experiencing this issue. However, rolling back to version 8.1.2, we encountered the following exception during startup, which version 8.1.3 aims to fix:
It seems that the application still works; we have tested it on a Pixel 7a and a Samsung Galaxy A54. So, we are faced with a choice between a slow startup or an exception during startup. Does anyone have any thoughts on which option is better? |
We are also facing this issue... |
After some testing I think this PR #577 introduced the performance drop. |
@c-goettert, yes I agree. But as I state in my comment above, rolling back the fix you mentioned we get back the exception |
As you can read in the comment on the PR, StrongBox seems to have a poorer performance:
Since we are talking about ~5 seconds more boot time in our case, we have decided against Strongbox. In the old versions, the wrong key-size prevents the use of strongbox and throws the exception. As there is no option to deactivate strongbox, we have modified the |
Thanks for your explanation and patch file, much appreciated @git-user-1337 |
I also applied @git-user-1337 fix and it fixed it for us. Upon launching our app on Pixel phones, the biometric prompt appears promptly. However, successful biometric authentication may take a moment or it failed with the error "java.io.IOException: javax.crypto.IllegalBlockSizeException". The cause was 'Key user not authenticated in Keystore'. What I'm guessing your code fixed was the time for cold bootup for Keystore. The Keystore cold boot time in some cases went upto 13 seconds and overshot the auth validity duration (5 sec) set in (com.oblador.keychain.cipherStorage.CipherStorageKeystoreRsaEcb.java). Because of this, it always expired our Keystore authorization before biometrics could complete successfully. The other possible fix would be to increase the validity duration, however the slow app startup would still NOT be fixed and if user were to close and re-open the app fast enough, it wouldn't ask for biometrics and automatically log-in user which also would be undesirable and weird to the user. It works normally and faster now. Thanks |
I had this problem, everything worked fine 2 weeks ago, I'm trying to figure out what went wrong |
We also experience the issue. Took us quite a long time to track it down. |
Hi, can anyone of you that rolled back (disabled StrongBox) using @git-user-1337 's patch confirm that it is safe to do so for production apps? If a user has saved to keychain using StrongBox will the value still be accessible using TEE after the patch? |
@pwltr I will fix this issue soon and release the official version. I'm working on it ;) |
Awesome, thanks @DorianMazur ! |
@DorianMazur issue still exists with 9.1.0 setGenericPassword takes 9.5 sek on my pixel 7 (Android 14) and in version 8.1.2 it took only 2,4 sek. |
@Brma1048 |
@DorianMazur, we’re encountering issues specifically with Galaxy S20 devices.
We suspect this is related to the latest security update: This issue has started to affect a significant number of our users: We’re currently using version 8.2.0 with a patch that disables StrongBox to optimize startup time. Given that this error only occurs on devices that support StrongBox, do you think enabling StrongBox might have prevented the issue? I’m hoping that, by enabling StrongBox, stored secrets would remain accessible even after a security update. We’ve encountered similar errors multiple times following various Samsung security updates. Any insights on this would be greatly appreciated. |
Hi @yberstad, I think the error you're seeing should be tracked in a separate issue since it's not directly related to this one. It seems like your error is linked to this PR: #647. A recent security update caused some data to be corrupted and it's related to the PKCS7 padding, making it impossible to decrypt. You can read more about it here: https://issuetracker.google.com/issues/357896946. One possible solution is to switch to AES GCM, which you can find in this PR: #678. Since AES GCM doesn't use padding, it should help prevent this kind of issue. As for enabling or disabling StrongBox, it likely won't resolve the issue. The corrupted data will still need to be handled. I've seen other libraries include automatic mechanisms to detect and remove malformed data. I plan to implement something similar here, but there’s a lot to do, and unfortunately, my time is limited. |
Thank you so much for your quick reply, @DorianMazur! I noticed that the issue on the Google Issue Tracker is dated August 7, 2024. Could it be that Samsung is whiping some date on security updates, making it "mandatory" to reinstall the app after a security update? From what I understand in your PR, switching to AES GCM, would mean that users would need to reinstall the app? Again, thank you for your help, much appreciated! |
Hi again, @DorianMazur. Sorry for bothering you, I know your time is limited. I'm trying out your PR code, but when trying to use the AES GCM on my Pixel 7a running Android 15, as such:
I get this this exception / stacktrace:
Tried to debug it, but I wasn’t able to immediately identify the issue.
Any clue what could be wrong? I also wondering how I would know if a device supports AES GCM or not? |
Hi @yberstad, The issue I mentioned is specific to Pixel devices. It’s possible that Samsung devices got the update a bit later. I recommend switching to AES GCM going forward. This is backward compatible, meaning any data saved with AES CBC will still be accessible, as long as it’s not corrupted. The getGenericPassword method returns a Thanks for trying out the PR! There are still a few things that need testing—I’m working on that—but everything should work once I merge it. |
Thanks for taking the time to answer me, and the work you put into this , @DorianMazur! Okay, I will wait for your merge, and then try again 😃 |
@yberstad New encryption is available in 9.2.0 I also created new document that will help in choosing correct storage type -> https://oblador.github.io/react-native-keychain/docs/choosing-storage-type |
Nice, thank you so much for letting me know @DorianMazur 👏 Looking forward to test it out! |
@DorianMazur, I've finally had some time to test this, and I have a few questions if you have a moment. Question 1:
If I do not specify the
I find it a bit strange that I have to specify a authenticationPrompt title, when I use the Question 2: The same is the case for the Question 3:
Question 4:
From the comment in the code above, and from the FAQ, I get the impression that it is only possible to upgrade FB stored values, but it seems that the new package is able to read the values I stored using the 8.2.0 version. I cannot see where the AES_CBC values are converted to AES_GCM? Thanks in advance! |
Question 1 & 2: Question 3: Question 4: EDIT: I will create page in the docs, describing how to migrate data from one cipher to another, but that should be quite simple. |
I'm aware that not everything is perfectly clear right now, but I'm working hard to improve this library and make things simpler. It's a long journey! 😄 I really appreciate the simplicity of the Expo SecureStore documentation, and I'm aiming for something similar. My goal is to maintain all the current features and possibly add more in the future, while cutting down on the unnecessary complexity. There's definitely a lot of clutter right now, but I'm working on cleaning it up. |
@DorianMazur, thanks for your very quick and informative reply! ❤️ |
In library version 8.1.3, where the setUserAuthenticationParameters method was changed using KeyProperties.AUTH_BIOMETRIC_STRONG the application started taking much longer to run
The text was updated successfully, but these errors were encountered: