oblador / react-native-keychain

:key: Keychain Access for React Native
MIT License
3.17k stars 518 forks source link

getGenericPassword is too slow on android 10 #337

Open sm2017 opened 4 years ago

sm2017 commented 4 years ago

I have slow startup on android 10 , After checking , I understand that getGenericPassword methods takes 7-8 seconds on android 10 I run on android 7 and 6 too , It takes about 600-700ms

sm2017 commented 4 years ago

I downgraded to v4.0.5 , it is fast on android 10 , just 30ms

patrickschmelter commented 4 years ago

react-native-keychain: 6.0.0 react-native: 0.62.2

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.

2020-05-04 15:31:18.866 30385-30539/? W/CipherStorageBase: StrongBox security storage is not available.
    android.security.keystore.StrongBoxUnavailableException: Failed to generate key pair
        at android.security.keystore.AndroidKeyStoreKeyPairGeneratorSpi.generateKeystoreKeyPair(AndroidKeyStoreKeyPairGeneratorSpi.java:554)
        at android.security.keystore.AndroidKeyStoreKeyPairGeneratorSpi.generateKeyPair(AndroidKeyStoreKeyPairGeneratorSpi.java:499)
        at java.security.KeyPairGenerator$Delegate.generateKeyPair(KeyPairGenerator.java:727)
        at com.oblador.keychain.cipherStorage.CipherStorageKeystoreRsaEcb.generateKey(CipherStorageKeystoreRsaEcb.java:256)
        at com.oblador.keychain.cipherStorage.CipherStorageBase.tryGenerateStrongBoxSecurityKey(CipherStorageBase.java:444)
        at com.oblador.keychain.cipherStorage.CipherStorageBase.generateKeyAndStoreUnderAlias(CipherStorageBase.java:391)
        at com.oblador.keychain.KeychainModule.internalWarmingBestCipher(KeychainModule.java:171)
        at com.oblador.keychain.KeychainModule.lambda$NuQDyTTfZc67dTNiVeEDbYNRCJw(Unknown Source:0)
        at com.oblador.keychain.-$$Lambda$KeychainModule$NuQDyTTfZc67dTNiVeEDbYNRCJw.run(Unknown Source:2)
        at java.lang.Thread.run(Thread.java:764)
     Caused by: android.security.KeyStoreException: No StrongBox available
        at android.security.keystore.AndroidKeyStoreKeyPairGeneratorSpi.generateKeystoreKeyPair(AndroidKeyStoreKeyPairGeneratorSpi.java:554) 
        at android.security.keystore.AndroidKeyStoreKeyPairGeneratorSpi.generateKeyPair(AndroidKeyStoreKeyPairGeneratorSpi.java:499) 
        at java.security.KeyPairGenerator$Delegate.generateKeyPair(KeyPairGenerator.java:727) 
        at com.oblador.keychain.cipherStorage.CipherStorageKeystoreRsaEcb.generateKey(CipherStorageKeystoreRsaEcb.java:256) 
        at com.oblador.keychain.cipherStorage.CipherStorageBase.tryGenerateStrongBoxSecurityKey(CipherStorageBase.java:444) 
        at com.oblador.keychain.cipherStorage.CipherStorageBase.generateKeyAndStoreUnderAlias(CipherStorageBase.java:391) 
        at com.oblador.keychain.KeychainModule.internalWarmingBestCipher(KeychainModule.java:171) 
        at com.oblador.keychain.KeychainModule.lambda$NuQDyTTfZc67dTNiVeEDbYNRCJw(Unknown Source:0) 
        at com.oblador.keychain.-$$Lambda$KeychainModule$NuQDyTTfZc67dTNiVeEDbYNRCJw.run(Unknown Source:2) 
        at java.lang.Thread.run(Thread.java:764) 
patrickschmelter commented 4 years ago

I can confirm that downgrading to 4.0.5 solves the issue. Maybe linked to #314

sm2017 commented 4 years ago

@oblador @vonovak take a look please

sm2017 commented 4 years ago

@oblador @vonovak reply please

vonovak commented 4 years ago

@sm2017 given more than one person is seeing this, I guess the issue is indeed present. I'm dealing with other stuff now and this is not a priority for me.

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!

tommeier commented 4 years ago

We've also found the same - downgraded has solved it - thanks for spotting this @sm2017 !

deecewan commented 4 years ago

Did a bit of digging - KeystoreAESCBC seems to be very slow when running CipherStorageBase#extractKey, specifically when doing keyStore.getKey(safeAlias, null). I'm not too sure why there is a slowdown between 4.0.5 and 6.0.0 yet, because they seem to do roughly the same thing when encrypting/decrypyting for storage.

deecewan commented 4 years ago

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 System.nanoTime() right before/after the call. Samsung Galaxy S10+, running Android 10.

Edit: yup, i'm stumped. The only thing that looks different is that there is now a cache on the keystore (https://github.com/oblador/react-native-keychain/blob/e6090b2966a0965b9326da44f849978c50e6fe2a/android/src/main/java/com/oblador/keychain/cipherStorage/CipherStorageBase.java#L282) whereas that didn't used to be there

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 StrongBox is not available on the particular hardware. and throwing early in the tryGenerateStrongBoxSecurityKey (before actually attempting to generate the key) makes the behaviour fast again.

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 CertificatePolicyCache: Creating new instance of CertificatePolicyCache. Then that completes, and it seems to work fast again. Googling around for CertificatePolicyCache doesn't really yield any results 😞.

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 setGenericPassword to work fairly quickly by explicitly setting storage: Keychain.STORAGE_TYPE.AES,, but unfortunately the getGenericPassword is still slow 😞

sm2017 commented 4 years ago

@oblador @vonovak sorry for mention , As I am not an Android developer , I cannot fix this problem and make a PR I think @deecewan's comment can help to fix it , Please take a look again

OleksandrKucherenko commented 4 years ago

Possible optimization:

boolean hasStrongBox() {
       return mActivity.getPackageManager()
           .hasSystemFeature(PackageManager.FEATURE_STRONGBOX_KEYSTORE)
}

Instead of trying and catching the exception we can check the hardware features availability.

src: https://stackoverflow.com/questions/58502299/how-to-check-for-strongbox-keymaster-hardware-availability-before-key-generation

deecewan commented 4 years ago

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

OleksandrKucherenko commented 4 years ago

https://github.com/oblador/react-native-keychain/compare/master...OleksandrKucherenko:fix-storngbox-detect?expand=1

quick implementation... I don't think it will improve anything significantly, but it's a good code to have in lib.

john-y-pazekha commented 4 years ago

@sm2017 @patrickschmelter @deecewan @tommeier @vonovak

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? get-device-info.apk.zip

Best regards, John

sm2017 commented 4 years ago

@john-y-pazekha as you joined 5 hours ago to github, and send us an APK file I cannot trust you and I don't install that apk

I can send you exact details about model of device

sm2017 commented 4 years ago

@john-y-pazekha here you are IMG-20200528-WA0014

john-y-pazekha commented 4 years ago

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

john-y-pazekha commented 4 years ago

@john-y-pazekha as you joined 5 hours ago to github, and send us an APK file I cannot trust you and I don't install that apk

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 @patrickschmelter @deecewan @tommeier @vonovak Please use either way to produce the log. I really need this information.

oblador commented 4 years ago

@sm2017: I can vouch for @john-y-pazekha

sm2017 commented 4 years ago

@john-y-pazekha I tested on mi a2 Currently I have no access to the phone, may be later

forkeer commented 4 years ago

I also have this problem, has anyone reached a conclusion? Why did you update, it's corrupted and the previous versions are correct

john-y-pazekha commented 4 years ago

@forkeer Would you please include your device specs as requested here?

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? get-device-info.apk.zip

Alternatively, you can clone the source from this repo and run it.

forkeer commented 4 years ago

@forkeer Would you please include your device specs as requested here?

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? get-device-info.apk.zip Alternatively, you can clone the source from this repo and run it.

i download but cant install show error

john-y-pazekha commented 4 years ago

i download but cant install show error

I updated the APK, should work now. Sorry for the inconvenience.

Alternatively, you can clone the source from this repo and run it in Android Studio.

forkeer commented 4 years ago

{ "BOARD": "sdm660", "LCT_BSP_VERSION": "0.0.1_180722", "LCT_SINGALCARD_DULEMODE": "unknown", "CPU_ABI2": "", "HOST": "c4-miui-ota-bd46.bj", "LQI_PRODUCT_DEVICE": "unknown", "LCT_WATER_MARK": "unknown", "SUPPORTED_64_BIT_ABIS": [ "arm64-v8a" ], "CPU_ABI": "arm64-v8a", "PERMISSIONS_REVIEW_REQUIRED": true, "FINGERPRINT": "xiaomi/jasmine/jasmine_sprout:10/QKQ1.190910.002/V11.0.6.0.QDIMIXM:user/release-keys", "PRODUCT": "jasmine", "INTERNAL": "unknown", "ID": "QKQ1.190910.002", "LCT_WIFI_BRAND": "unknown", "TYPE": "user", "LCT_AUTOREGISTER_NAME": "unknown", "LCT_BUILD_TYPE": "unknown", "VERSION_TYPE": "unknown", "DEVICE": "jasmine_sprout", "BRAND": "xiaomi", "LCT_HARDWARE_PLATFORM": "unknown", "SUPPORTED_32_BIT_ABIS": [ "armeabi-v7a", "armeabi" ], "LCT_DEVICE_BLACK_UI": "unknown", "BOOTLOADER": "unknown", "CUSTOM_NAME": "unknown", "IS_EMULATOR": false, "LCT_BLUETOOTH_BRAND": "unknown", "TAGS": "release-keys", "CUSTOM_INTERNAL": "unknown", "DISPLAY": "QKQ1.190910.002 V11.0.6.0.QDIMIXM", "SUPPORTED_ABIS": [ "arm64-v8a", "armeabi-v7a", "armeabi" ], "LCT_PROJECT_NAME": "jasmine", "EXTERNAL": "unknown", "LCT_NETWORK_TYPE": "unknown", "LCT_PHONENUMBER_MINMATCH": "unknown", "SERIAL": "unknown", "BUILDDATE": "Tue Mar 10 19:55:26 WIB 2020", "DISPLAY_HARDWARE": "unknown", "LCT_ACCELERATOR_BRAND": "unknown", "LCT_DEVICE_NAME": "unknown", "LCT_OPERATOR_NAME": "unknown", "TIME": 1583844926000, "MODEL": "Mi A2", "MANUFACTURER": "Xiaomi", "USER": "builder", "HARDWARE": "qcom", "IS_DEBUGGABLE": false, "IDEAFRIEND_NEED": "unknown", "RADIO": "unknown", "UNKNOWN": "unknown", "LCT_EXTSTORAGE_TYPE": "unknown" }

john-y-pazekha commented 4 years ago

@forkeer Thanks a lot! What was the startup time?

forkeer commented 4 years ago

@forkeer Thanks a lot! What was the startup time?

From 6 to 8 seconds but averages 7 seconds

cladjules commented 4 years ago

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.

john-y-pazekha commented 4 years ago

@cladjules I could find the following Galaxies, which one exactly was it? Screenshot 2020-05-31 at 13 57 17

gentlee commented 4 years ago

Issue is easily reproduced on Xiaomi Redmi Note 8 Pro (MIUI 11.0.3.0, Android 9), app launch can take up to 45 seconds, blocking the main thread while getting generic password. Downgrading react-native-keychain to 4.0.5 fixes the issue.

On google devices and emulators it works fine.

olevett commented 4 years ago

Not sure how helpful this is, but we've been able to reproduce the slow call to getGenericPassword on at least these devices

Device Model OS Version Login Time Manufacturer Model Product
Samsung Galaxy A5 8 5-30 seconds - - -
Samsung Galaxy S6 7 1-2 seconds, sometimes >5 minutes - - -
Samsung Galaxy S7 8 1-2 seconds - - -
Samsung Galaxy S9 9 1-2 seconds - - -
Samsung Galaxy S10 9 1-2 seconds samsung SM-G973F beyond1lteeea

I can try and track down exact model numbers if that's helpful too?

john-y-pazekha commented 4 years ago

I can try and track down exact model numbers if that's helpful too?

Yes please! I'm interested in fields MANUFACTURER, MODEL, and PRODUCT. Would be fantastic if you could attach this information.

You can use this APK or the source from this repo to collect this info easily.

olevett commented 4 years ago

Thanks, will update the above table as I get information.

deecewan commented 4 years ago

i'm not sure we explicitly need more device info here. the cause is the lack of strongbox and how long the OS is taking to tell us that. there's a path to a solution that more info isn't necessarily going to help.

aranda-adapptor commented 4 years ago

I tried disabling the warming, but that didn't seem to help much. However changing the code in getGenericPassword here did help. https://github.com/oblador/react-native-keychain/blob/master/android/src/main/java/com/oblador/keychain/KeychainModule.java#L292

From this:

      // get the best storage
      final String accessControl = getAccessControlOrDefault(options);
      final boolean useBiometry = getUseBiometry(accessControl);
      final CipherStorage current = getCipherStorageForCurrentAPILevel(useBiometry);

To this (same code used in setGenericPassword):

      final CipherStorage current = getSelectedStorage(options);

My Samsung Galaxy S8 went from 3+ seconds (sometimes 5 or 10) to < 1 second. I am also specifying storage of AES explicitly in the RN options for both get and set operations. I'm guessing this will only help if you don't need RSA, ie fingerprint.

cladjules commented 4 years ago

i'm not sure we explicitly need more device info here. the cause is the lack of strongbox and how long the OS is taking to tell us that. there's a path to a solution that more info isn't necessarily going to help.

if Strongbox is truly the issue here, why not detecting it using the packageManager, that could help speed-up things

activity.getPackageManager().hasSystemFeature(PackageManager.FEATURE_STRONGBOX_KEYSTORE)
cladjules commented 4 years ago

Could you try that branch? https://github.com/cladjules/react-native-keychain/tree/android10RSA

I have added detection for system feature for the strongbox.

I don't have many devices to test, seems fairly OK on my OnePlus 7 pro.

Thanks.

aranda-adapptor commented 4 years ago

@cladjules I tried your branch. Seemed to be almost always under 1 second on Samsung Galaxy S8, so definitely an improvement when always 3+ seconds when I was on v5.0.1. I did get one instance out of about 15 where it was 7 seconds.

olevett commented 4 years ago

Unfortunately, it seems a little worse performance wise on a few test devices we've tried, though this is anecdotal rather than timed with repeats

cladjules commented 4 years ago

There will be absolutely no changes with devices that have Strongbox available. All I am doing is checking for Strongbox is available, which should improve response time for devices that don't have it?

tranquan commented 4 years ago

@cladjules I checked your branch, faster 🎉. But it still takes ~5s on my pixel, sometimes 10s

I wonder if we can config storage, securityLevel in advance, so CipherStorageBase doesn't need to do the check.

For example, if I set securityLevel is "SECURE_SOFTWARE", so it doesn't need to check for "SECURE_HARDWARE" available


I wonder why we need to check what kind of storage device support every time? Could it be check in the first time and store in SharedPreference?

For me, it's slow because of checking this RSA: CipherStorageKeystoreRsaEcb

aranda-adapptor commented 4 years ago

@tranquan not sure if will help your use case, but I had good results specifying storage option AES and altering the getGenericPassword function as per: https://github.com/oblador/react-native-keychain/issues/337#issuecomment-639323583

sm2017 commented 4 years ago

@oblador @vonovak please contribute

vonovak commented 4 years ago

please contribute

@sm2017 here's a quick answer to that: https://twitter.com/vonovak/status/1283895813903654912?s=20

hope this makes sense 🙂

nicolaslazzos commented 3 years ago

I tried disabling the warming, but that didn't seem to help much. However changing the code in getGenericPassword here did help. https://github.com/oblador/react-native-keychain/blob/master/android/src/main/java/com/oblador/keychain/KeychainModule.java#L292

From this:

      // get the best storage
      final String accessControl = getAccessControlOrDefault(options);
      final boolean useBiometry = getUseBiometry(accessControl);
      final CipherStorage current = getCipherStorageForCurrentAPILevel(useBiometry);

To this (same code used in setGenericPassword):

      final CipherStorage current = getSelectedStorage(options);

My Samsung Galaxy S8 went from 3+ seconds (sometimes 5 or 10) to < 1 second. I am also specifying storage of AES explicitly in the RN options for both get and set operations. I'm guessing this will only help if you don't need RSA, ie fingerprint.

This solves the issue for me, because after trying commenting and changing some code, i found that the getCipherStorageForCurrentAPILevel method its too slow in some devices. So if you replace that code with the getSelectedStorage method (the same used in setGenericPassword), this method first check if you are calling it with the storage option, and in that case, returns that one. If you are not passing it, then the getCipherStorageForCurrentAPILevel method is called, so the logic is the same. So if you know what type of storage are going to use (as in my case), you only need to replace that code and pass the storage option both in setGenericPassword and getGenericPassword.

Also yo need to disable the warmup.

Are there possibilities to add this change in the next release or is it made that way for some specific reason? I haven't gone far enough into the source code.

juanektbb commented 3 years ago

@nicolaslazzos @aranda-adapptor I am trying on a Samsung Galaxy S7 your method as:

...

      // get the best storage
      // final String accessControl = getAccessControlOrDefault(options);
      // final boolean useBiometry = getUseBiometry(accessControl);
      // final CipherStorage current = getCipherStorageForCurrentAPILevel(useBiometry);
      final CipherStorage current = getSelectedStorage(options);
      final String rules = getSecurityRulesOrDefault(options);

...

And disabled the warmup as the docs suggest here Link, but the code changed and I call .withoutWarmUp() in this file https://github.com/oblador/react-native-keychain/blob/master/android/src/main/java/com/oblador/keychain/KeychainPackage.java as:

  public KeychainPackage() {
    this(new KeychainModuleBuilder().withoutWarmUp());
  }

I delete the build folder of the node package and run: react-native run-android, but it still takes about 7 seconds while in iPhone, it is instantly.

Any advice on this? - Sorry if I specify too much, I am new to react native. Thanks

aranda-adapptor commented 3 years ago

@juanektbb not sure if disabling warmup is required, but for me the slowdown persisted until I made those native changes you've mentioned and explicitly passed storage: Keychain.STORAGE_TYPE.AES to both setGenericPassword and getGenericPassword in the JS.

nicolaslazzos commented 3 years ago

@nicolaslazzos @aranda-adapptor I am trying on a Samsung Galaxy S7 your method as:

...

      // get the best storage
      // final String accessControl = getAccessControlOrDefault(options);
      // final boolean useBiometry = getUseBiometry(accessControl);
      // final CipherStorage current = getCipherStorageForCurrentAPILevel(useBiometry);
      final CipherStorage current = getSelectedStorage(options);
      final String rules = getSecurityRulesOrDefault(options);

...

And disabled the warmup as the docs suggest here Link, but the code changed and I call .withoutWarmUp() in this file https://github.com/oblador/react-native-keychain/blob/master/android/src/main/java/com/oblador/keychain/KeychainPackage.java as:

  public KeychainPackage() {
    this(new KeychainModuleBuilder().withoutWarmUp());
  }

I delete the build folder of the node package and run: react-native run-android, but it still takes about 7 seconds while in iPhone, it is instantly.

Any advice on this? - Sorry if I specify too much, I am new to react native. Thanks

I dont know if i understood correctly, but you dont have to call .withoutWarmUp() in this file: https://github.com/oblador/react-native-keychain/blob/master/android/src/main/java/com/oblador/keychain/KeychainPackage.java, you have to do it in your MainApplication.java. So for that you have to import:

import com.oblador.keychain.KeychainPackage;
import com.oblador.keychain.KeychainModuleBuilder;

And then, in the getPackages() method, yo have to add the package manually like that:

packages.add(new KeychainPackage(new KeychainModuleBuilder().withoutWarmUp()));

And because react-native-keychain has autolinking, i had to disable it in order to add the package manually. For that, i created a react-native.config.js file in the root of my project with the following code:

module.exports = {
  dependencies: {
    'react-native-keychain': {
      platforms: {
        android: null,
      },
    },
  },
};

I hope it works for you!

nicolaslazzos commented 3 years ago

@juanektbb not sure if disabling warmup is required, but for me the slowdown persisted until I made those native changes you've mentioned and explicitly passed storage: Keychain.STORAGE_TYPE.AES to both setGenericPassword and getGenericPassword in the JS.

I tried with and without warmup several times, and for me is required. Its a combination of both changes. Are you using AES storage? For me i think the slowdown only happened with RSA storage.

juanektbb commented 3 years ago

@nicolaslazzos @aranda-adapptor

You guys are stars! 👍🏻 - It worked, many thanks!

For other new users like me that have the same problem: Apart from changing MainApplication.java as the @nicolaslazzos mentioned above and all the other file changes he posted. You will also need to add compile project(':react-native-keychain') inside your dependencies { } of /android/app/build.gradle

And my JS setGenericPassword for passing AES looks like: await Keychain.setGenericPassword("some_user", "123456", {storage: Keychain.STORAGE_TYPE.AES })

Good luck