Android UI freezing on launch #314
Comments
|
from the provided information, I can assume a long startup time of the keychain library. errors are not critical, even expected. I log them for informing developers about device capabilities and what configuration of the cypher storage applied. |
|
Thank you for your snappy reply!
I though so, but wanted to provide them in case someone could use them for something :) |
actually it should not happen, the library did not occupy the main thread (UI thread). initialization is done in a background thread and yes it's slow on the first run, but its the result of the crypto API itself. |
|
Yeah I could see that effort had been put into doing the warming up in a background thread, but it doesn't seem to work as intended.
Meaning that it is slow on every first run/every launch of the app? |
As I said it should not slowdown the UI. Something else is eating the CPU and freezing the UI thread. So far I don't have any clue why it happens. Initialization often takes up to 10 seconds on slow devices. what device do you use? or it emulator? |
|
As stated in the first post (somewhere between all the logs 😄 ) I'm running it on a OnePlus 5T. Are you using the library in some app yourself? And are you not able to reproduce the performance issues that I'm experiencing? What device are you running? |
during working on PR other devs mention that startup was very slow. and after that report in the PR were added commits with "caching" and "warmup" logic. In test scenario that I use (Example app), startup of the app was never an issue, the first call of the keychain lib was slow, but never a startup of the app! |
|
check logs for |
|
and this is the initialization code itself. Inside constructors of the Cyphers only the facebook cypher may have some side effect due to native library loading. |
I did, and in particular this looked extreme: |
The app startup time is not that bad, but after using it in my app, I sometimes experienced an unresponsive UI. I could reproduce it in the KeychainExample app by constantly interacting with the UI (e.g. constantly tapping a text input as you can see in the screen recording in the original post), but if I didn't interact with the UI constantly, I wouldn't notice the performance issues. The app renders/launches quick enough. |
yep... but that a complete module warmup... it triggers maximum things that's are in the module to force java load classes and initialize them. I think you can investigate deeper with android studio profiler and see which method takes the most of the time. low level crypto/keystores api itself is very slow... I have no idea why it so |
|
i've faced the same issue, the warming process took long time , that cause UI freezing on low performance phones, |
And did that help on performance or what is your conclusion? |
Well yes, as i'm using RSA ,i didn't need to add Facebook || AES. |
|
@OleksandrKucherenko have you found any fix for this issue? Still seems to be a problem on older Android devices. |
|
@OleksandrKucherenko I believe it does block the UI because all the internal function are |
|
Personally commenting out the warming makes it much faster, But I need to test on more Android versions. |
@cladjules And are there any notable drawbacks from removing the warming? Does it still work? |
I have only tried on Android 10, but it's worth testing other versions, |
|
I have tried setting up the Priority to the thread to high, I cannot see anything in the warming code that would require the UI Thread, |
i tried that before, the UI freezing when i'm requesting keychain for the first time. best solution for me was to edit the warmingUp method and the keyModule method to reduce the code need to be executed. |
Android 10 looks fine though, Thanks :) |
|
We ran into this same issue with several different phone models but it did only occur in Android 10. Android 8 and 9 were not visibly affected. For example on Samsung A20 Android 10 the warmup time was anything between 5k-40k ms. |
|
@jenskuhrjorgensen 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, |
Since a concern was expressed about safety of this APK, I would like to assure you gentlemen that this APK is not malicious. It requests no permissions so it cannot harm your system. Alternatively, you can clone the source from this repo and run it in Android Studio. @jenskuhrjorgensen |
|
Hi @john-y-pazekha I also did some more investigation in the performance issues. I tried to disable the warming as suggested by others: And it helped on the initial freezing, but only delays it for the first time you save credentials. I did some logging and found out that the time consuming task is In |
Congrats 🥳 |
|
Experiencing the same issue on our app as well. Managed to work around the issue by disabling autolinking and adding the non warmed up module to my app. Would be nice if the library would support loading it without necessarily performing the warmup on start, and then allow developers to explicitly control when to perform the warmup with some For anyone wanting to do a similar workaround, these are roughly the changes I did.
module.exports = {
dependencies: {
'react-native-keychain': { platforms: { android: null } },
},
};
include ':react-native-keychain'
project(':react-native-keychain').projectDir = new File(rootProject.projectDir, '../node_modules/react-native-keychain/android')
dependencies {
implementation project(':react-native-keychain')
}
package example;
import androidx.annotation.NonNull;
import com.facebook.react.ReactPackage;
import com.facebook.react.bridge.JavaScriptModule;
import com.facebook.react.bridge.NativeModule;
import com.facebook.react.bridge.ReactApplicationContext;
import com.facebook.react.uimanager.ViewManager;
import java.util.Collections;
import java.util.List;
import com.oblador.keychain.KeychainModule;
@SuppressWarnings("unused")
public class ColdKeychainPackage implements ReactPackage {
public ColdKeychainPackage() {
}
@Override
@NonNull
public List<NativeModule> createNativeModules(@NonNull final ReactApplicationContext reactContext) {
return Collections.singletonList(new KeychainModule(reactContext));
}
@NonNull
public List<Class<? extends JavaScriptModule>> createJSModules() {
return Collections.emptyList();
}
@Override
@NonNull
public List<ViewManager> createViewManagers(@NonNull final ReactApplicationContext reactContext) {
return Collections.emptyList();
}
}
@Override
protected List<ReactPackage> getPackages() {
@SuppressWarnings("UnnecessaryLocalVariable")
List<ReactPackage> packages = new PackageList(this).getPackages();
// Packages that cannot be autolinked yet can be added manually here, for example:
// packages.add(new MyReactNativePackage());
packages.add(new ColdKeychainPackage());
return packages;
} |
|
Hello, I have taken a great inspiration from this module to write my own (because I like to understand stuff). Thank you by the way for your work ! I didn't investigate this issue in particular, but I was surprised when I saw the warming up mechanism in the code. This mechanism generates a "warmingUp" key which has no other purpose. I don't know if you are aware, but if the chosen algorithm is RSA, it will take a lot of time to generate a key pair. Just try to generate an RSA key pair in your terminal with ssh-keygen, you'll see that takes a noticeable time. Generating an RSA key pair is a random process. The algorithm tests a lot of random numbers until it finds a prime one that's big enough and then derives the private and the public key from it. So the generation can be quick or really long. It both depends on the speed of your processor and on luck. I believe this slowness issue will be solved once you remove your warmup mechanism as @cladjules commented. And from my understanding of the library, it has no other side effect. You could also use the AES algorithm instead of the RSA one. It's as secure for this use case. (But generating a key in the Keystore with the option "setUserAuthenticationRequired(true)" and immediately use it to encrypt the user password will fail if the user does not authenticate in between. I guess that's precisely why the RSA algorithm was used in the first place). I hope this helps. |
|
Just to make things clear, after reading this issue, it looks like our workarounds for now are:
Did I understand that correctly? |
|
@SudoPlz Under the hoodTo improve on my previous comment, here is what actually happens. A secure hardware generates the keys: either a secret symmetric key (AES) or a private-public key pair (RSA) in our case. That AES key or RSA public key is used to encrypt your item value and store the encrypted result on a public file. The security lies in the fact that the secure hardware makes sure that only your app can retrieve the key. You can also specify options so that the secure hardware reveals the key only after the user has authenticated with biometrics or password. The problem on Android is that if you specify that option for an AES key, to be able to encrypt your entry value with the newly created key, you would have to authenticate. So the user experience would be weird: the user types his password, then has to use his fingerprint or device password right after so the app can save the password... That is the reason why, I suppose, the authors of the library decided that in order to enable this option, the best way was to generate an RSA key pair. Indeed, the public key of the RSA key pair is always available... since it is public. Then came the drawback that generating an RSA key pair is a random process, so it sometimes takes a long time. I suppose the authors of the library thought the problem was the time it took to load the library, and decided to add a warming up mechanism, which is of course not a solution and makes the problem even worse. There are several alternatives here: In any case, the warming up mechanism should be removed. I have implemented 2/ on my project with a simplified api. I can send you the code if you wish. |
|
@giregk thank you SOOOO much for explaining, this is perfect!!! Ok so then I guess here's a scenario, for an app with low security needs, a user types in their username and password, and for whatever reason we wish to save those credentials. We also wish to retrieve that username and password only after they authenticate via biometrics (mostly because it's easier and more convenient than typing a password - because that could happen many times). Does that mean we can't use solution no2 which seems great for us as well? Here are my 2 configs currently: The The p.s: I find it super hard to figure out what the differences between FB, RSA and AES are and when to choose each one for this library, are those documented anywhere? |
About FB vs RSA vs AES
You can find a lot more details on the web about these two algorithms. The lib chooses the algorithm for you depending on your parameters if you don't specify it. About your use caseYour NO_BIOMETRICS_CONFIG should not specify the storage type to let the lib decide for you. I can't remember exactly how the code decides, but I think it won't use RSA unless you specify the biometric requirement. (But I'm not sure) In the case of your RAW_CREDS_CONFIG, my solution 2/ is fine. Biometrics is not really a security improvement anyways, so let's consider it simply as a a means to improve the UX. Here is the code I use in my project: https://github.com/giregk/react-native-simple-keychain. The api is a bit different though. Do read the native code, it will help you. |
|
@giregk I really appreciate the explanation. So it sounds like I should use AES for both my configurations That being said, I was under the impression that in order to use your 2nd solution with AES, we can't use biometrics ( So how can I achieve the following scenario: A) User types credentials is the following config the right way to do that, or will that result in biometrics being asked after the user types their credentials and tries to save them? const RAW_CREDS_CONFIG: Keychain.Options = {
accessControl: Keychain.ACCESS_CONTROL.BIOMETRY_ANY,
accessible: Keychain.ACCESSIBLE.WHEN_UNLOCKED,
authenticationPrompt: {
title: 'Unlock to log into Acuity',
},
storage: Keychain.STORAGE_TYPE.AES // <-- does aes work with biometrics?
};then await Keychain.setGenericPassword( // <-- NO biometrics requested here
username,
password,
RAW_CREDS_CONFIG,
)
// ...
await Keychain.getGenericPassword(RAW_CREDS_CONFIG); // <-- biometrics requested here |
|
@SudoPlz Try that, but I don't know if that will work. I think it will not. The only way to achieve the desired scenario without having the random freezing time on android is by changing the native implementation of the lib, like in my repo. === EDIT |
|
@giregk so for now:
are those 2 assumptions correct? |
|
@SudoPlz yes === EDIT ...assuming you use the lib as is. If you rewrite the lib from scratch, AES can work. === EDIT 2 |
|
@giregk do you know which line of code stops this library from using AES with biometrics? Or from a high level what needs to change to achieve that? |
|
@SudoPlz I think you should read the native code. I promise it will help you. It's the whole logic. But |
|
I don't know if I did anything wrong with my tests, but selecting AES with my current config which now looks like this: public static RAW_CREDS_CONFIG: Keychain.Options = {
accessControl: Keychain.ACCESS_CONTROL.BIOMETRY_ANY,
accessible: Keychain.ACCESSIBLE.WHEN_UNLOCKED,
authenticationPrompt: {
title: 'Unlock to log in',
},
storage: Keychain.STORAGE_TYPE.AES
};seems to have worked without any hustle. |
|
@SudoPlz Excellent !! I don't think you did anything wrong, it just means I was wrong when I said it would not work ! |
|
Same problem. Samsung M31 example with RAW_CREDS_CONFIG not work Manual link and build with .withoutWarmUp() not work. logcat loop: 11-18 14:23:17.266 4462 9364 D keystore: [BEGIN::end] 11-18 14:23:17.282 4462 9368 D keystore: [BEGIN::end] 11-18 14:23:17.310 4285 4285 W keymaster_tee: [WRN]begin req PARAMS: A32 B2 P64 11-18 14:23:17.334 4285 4285 W keymaster_tee: [WRN]begin req PARAMS: A32 B2 P64 11-18 14:23:17.364 4285 4285 W keymaster_tee: [WRN]begin req PARAMS: A32 B2 P64 |
|
Still on 6.1.1 here and was also experiencing UI freeze on devices. It was locking the whole phone up. NB: This was a solution for my app as I'm only concerned about AES cipher. It will not work for everyone The patch code: The technical part: |
Yep, patching this fixed the issue I was having with the device being frozen for few seconds. |
|
@oblador Are there any plans for an official fix for this on the native side of this lib? |
|
With RN 0.64 and keychain 7.0.0 The warning in log is still there for android |
Hi, can you send an example of the second case implementation? Thanks. |
Here you go: https://gist.github.com/giregk/4965b2007acbc60b5a39f4bcf4e2f7f6 |
|
Please try out 8.0.0 which has performance improvements |
|
Version 8 has still the same problems |
|
I ran into this same issue on Android 12 with a Pixel 5. Android 11 was having no issue and after upgrading to 12 it would occur. I ended up with a hacky fix of waiting for the splash screen to dismiss before calling Update: The fix did not work in a production build, running into this error: |
Hi
I'm experiencing problems with react-native-keychain on Android which unfortunately forces me to disable the library for Android (on iOS it works flawlessly). Sometimes the UI freezes for several seconds shortly after launching the app. This happens when simply importing the library without even using it, as in the following code:
As you can see in the screen recording below, the UI freezes and ignores any input for several seconds shortly after reloading the app.
Logcat shows a couple of errors, but I don't know if they are related:
Tested on OnePlus 5T (A5010) running OxygenOS 9.0.10.
Let me know if you need any more information!
@OleksandrKucherenko I know you have put a lot of effort into this version of the library, especially the Android part. Maybe you have encountered similar issues?
Best regards
Jens
The text was updated successfully, but these errors were encountered: