-
-
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
getGenericPassword is too slow on android 10 #337
Comments
I downgraded to v4.0.5 , it is fast on android 10 , just 30ms |
react-native-keychain: 6.0.0 Same issue for me. In Simulator (API28) and on a Huawei P30 Pro (API29) it works super fast, on a Samsung SM-J730F (API28) it takes way beyond 10 seconds sometimes and fails with following exception. I experience it only happening the very first time after installing the app or wiping app data.
|
I can confirm that downgrading to 4.0.5 solves the issue. Maybe linked to #314 |
@sm2017 given more than one person is seeing this, I guess the issue is indeed present. Cc @OleksandrKucherenko as he might have more insight. As always, if you have an issue, I recommend you contribute a PR with a fix. Thank you! |
We've also found the same - downgraded has solved it - thanks for spotting this @sm2017 ! |
Did a bit of digging - |
from 4.0.5: // CipherStorageKeystoreAESCBC.java line 101
key = keyStore.getKey(service, null);
// 5ms average over 5 runs from 6.0.0 // CipherStorageBase.java line 234
key = keyStore.getKey(safeAlias, null);
// 3637ms average over 5 runs Measurements taken using Edit: yup, i'm stumped. The only thing that looks different is that there is now a cache on the keystore ( Line 282 in e6090b2
Edit edit: Tried making it not a cached instance, tho i am not sure quite how or why that would fix it. It didn't fix it. So it seems that the same method call is taking longer, when the only variable is the package version 😅 Edit: looking into this on a bit further, based on @patrickschmelter's comment. Looking in the android source, it seems like OK. Looks like that's a bit of a red herring. I've been playing around a lot and it seems like the "warm up" happens, and it also gets stuck. Until a log line appears that says Final edit for a while: seems like the new RSA type is what's doing it. There seems to be a lock involved, and the system hangs until it's ready. I managed to get the |
Possible optimization:
Instead of trying and catching the exception we can check the hardware features availability. |
@OleksandrKucherenko I tried doing that, too, and it was not noticeably quicker. There is a good chance that I got it mixed up with other tests I was trying, tho. |
quick implementation... I don't think it will improve anything significantly, but it's a good code to have in lib. |
@sm2017 Gentlemen, I need your help to reproduce this issue. Since it happens only on specific combination of phone and Android version, I need to know what exactly you're running. Could you please use this APK and attach here the report it produced? Best regards, |
@john-y-pazekha as you joined 5 hours ago to github, and send us an APK file I can send you exact details about model of device |
@john-y-pazekha here you are |
@sm2017 Thanks a lot. Could you please run the APK I provided and attach the log? I'm particularly interested in VENDOR and PRODUCT fields to identify the device precisely. I need this information because we couldn't reproduce this issue on the devices we have in-house; now we're going to order new devices but we must know what exactly to order. |
I totally understand your concern. Please rest assured that this APK is not malicious. When installing an APK, you have a chance to review the permissions it requests. You can see that this one requests no permissions at all. There is no virus in it (sorry, I was too lazy, maybe in the next version :)))) Alternatively, you can clone the source from this repo and run it. @sm2017 |
@sm2017: I can vouch for @john-y-pazekha |
@john-y-pazekha I tested on mi a2 |
I also have this problem, has anyone reached a conclusion? |
@forkeer Would you please include your device specs as requested here?
|
i download but cant install |
{ |
@forkeer |
From 6 to 8 seconds but averages 7 seconds |
I am using OnePlus 7 Pro with Android 10, but I am pretty sure we had the same issue with a Galaxy S8, I will confirm. Thanks. |
+1 |
any updates ? |
I also downgraded to 4.0.5. |
Same error, how did you solve it? My app crashes in 4-5 seconds after opening and always on white screen. |
Hello, Reverting to 4.0.5 ends up not solving the issue as others have mentioned above in the thread, the whole build of the app not compiling. Any idea on how to fix this issue ? Ludovic - Codekraft |
I ended up forking this repo, using v4.0.5 on android and v7 on iOS. v4.0.5 performance is dramatically better. I was testing on a Galaxy J7. On v7 I was getting times ranging from 400ms to 18 seconds for getGenericPassword. Consistently < 40ms on v4.0.5 |
Thats because the v4.0.5 version does not have biometry to unlock the keychain. My fork is working ok with the latest version by making the changes i mentioned in the other comments. |
For me is |
Please try out 8.0.0 which has performance improvements |
Moving to version 8.0.0 didn't really help with biometric performance, but this solution finally worked ✅🎉 |
Still an issue in 8.1.1. |
Still an issue |
|
Hi, same problem with samsung s10e (android 12). |
does this PR fixes this issue? |
I still have the issue with v8.2.0 (which has this PR). With v4.0.5, it takes less than 100 milliseconds. |
Still having this issue on 8.2.0, but the fix with removing warm up fixes it. Would be nice if someone added a small gradle variable and enabled warm up only need it. Then all the manual install steps for android wouldn't be necessary. |
Hi @c-goettert @humaidk2 @owav @vafada I’ve recently migrated the project to use Android DataStore Preferences in the latest release 1.2.0. Could you please test and confirm if the bug is still present? |
Hey @DorianMazur I tried your version as a drop-in replacement but I get the following error when trying to getGenericPassword (setGenericPassword seems to work fine) : Edit : it seems this error has something to do with reloading the app without killing it. Anyway, with a clean cold start getGenericPassword takes approx 12sec, same as with v8.2.0 (as for v4.0.5, less than 100ms). Edit 2 : if it is of any help, here is my quick test code : test code
|
I tested react-native-keychain 9.0.0 on Android 10 (API 29):
Since the issue no longer occurs, I’m closing this. Feel free to reopen if needed. |
After looking into it more, this happens because of StrongBox on some devices. It's not a bug, but I'll add an option to turn off StrongBox for those who don't need the extra security. |
Indeed hot reloading was broken because of this issue, needed a CoroutineScope to cancel it on each stop. Should be fixed on the latest updates in PR #629 |
Speeds up strongbox detection on certain devices: oblador#337 (comment)
I have slow startup on android 10 , After checking , I understand that
getGenericPassword
methods takes 7-8 seconds on android 10I run on android 7 and 6 too , It takes about 600-700ms
The text was updated successfully, but these errors were encountered: