React-native-keychain: getGenericCredentials never resolves or rejects due to ShortBufferException in Google Pixel 4

[HuiSF]

Description

When use setGenericCredentials to write a long content (string length > 2400) as password into key store (AESCBC) in Google Pixel 4, getGenericCredentials will never resolve, nor ever reject.

Details

react-native-keychain version 6.1.1.

When getGenericCredentials hangs, below Java exception was logged

08-21 20:54:31.465 22572 25907 W CipherStorageBase: ShortBufferException is not expected
08-21 20:54:31.465 22572 25907 W CipherStorageBase: java.lang.IllegalStateException: ShortBufferException is not expected
08-21 20:54:31.465 22572 25907 W CipherStorageBase:     at javax.crypto.CipherInputStream.getMoreData(CipherInputStream.java:136)
08-21 20:54:31.465 22572 25907 W CipherStorageBase:     at javax.crypto.CipherInputStream.read(CipherInputStream.java:249)
08-21 20:54:31.465 22572 25907 W CipherStorageBase:     at javax.crypto.CipherInputStream.read(CipherInputStream.java:225)
08-21 20:54:31.465 22572 25907 W CipherStorageBase:     at com.oblador.keychain.cipherStorage.CipherStorageBase.copy(CipherStorageBase.java:487)
08-21 20:54:31.465 22572 25907 W CipherStorageBase:     at com.oblador.keychain.cipherStorage.CipherStorageBase.decryptBytes(CipherStorageBase.java:364)
08-21 20:54:31.465 22572 25907 W CipherStorageBase:     at com.oblador.keychain.cipherStorage.CipherStorageKeystoreAesCbc.decryptBytes(CipherStorageKeystoreAesCbc.java:233)
08-21 20:54:31.465 22572 25907 W CipherStorageBase:     at com.oblador.keychain.cipherStorage.CipherStorageKeystoreAesCbc.decrypt(CipherStorageKeystoreAesCbc.java:142)
08-21 20:54:31.465 22572 25907 W CipherStorageBase:     at com.oblador.keychain.cipherStorage.CipherStorageKeystoreAesCbc.decrypt(CipherStorageKeystoreAesCbc.java:160)
08-21 20:54:31.465 22572 25907 W CipherStorageBase:     at com.oblador.keychain.KeychainModule.decryptToResult(KeychainModule.java:623)
08-21 20:54:31.465 22572 25907 W CipherStorageBase:     at com.oblador.keychain.KeychainModule.decryptCredentials(KeychainModule.java:590)
08-21 20:54:31.465 22572 25907 W CipherStorageBase:     at com.oblador.keychain.KeychainModule.getGenericPassword(KeychainModule.java:295)
08-21 20:54:31.465 22572 25907 W CipherStorageBase:     at com.oblador.keychain.KeychainModule.getInternetCredentialsForServer(KeychainModule.java:394)
08-21 20:54:31.465 22572 25907 W CipherStorageBase:     at java.lang.reflect.Method.invoke(Native Method)
08-21 20:54:31.465 22572 25907 W CipherStorageBase:     at com.facebook.react.bridge.JavaMethodWrapper.invoke(JavaMethodWrapper.java:371)
08-21 20:54:31.465 22572 25907 W CipherStorageBase:     at com.facebook.react.bridge.JavaModuleWrapper.invoke(JavaModuleWrapper.java:150)
08-21 20:54:31.465 22572 25907 W CipherStorageBase:     at com.facebook.react.bridge.queue.NativeRunnable.run(Native Method)
08-21 20:54:31.465 22572 25907 W CipherStorageBase:     at android.os.Handler.handleCallback(Handler.java:883)
08-21 20:54:31.465 22572 25907 W CipherStorageBase:     at android.os.Handler.dispatchMessage(Handler.java:100)
08-21 20:54:31.465 22572 25907 W CipherStorageBase:     at com.facebook.react.bridge.queue.MessageQueueThreadHandler.dispatchMessage(MessageQueueThreadHandler.java:26)
08-21 20:54:31.465 22572 25907 W CipherStorageBase:     at android.os.Looper.loop(Looper.java:214)
08-21 20:54:31.465 22572 25907 W CipherStorageBase:     at com.facebook.react.bridge.queue.MessageQueueThreadImpl$4.run(MessageQueueThreadImpl.java:225)
08-21 20:54:31.465 22572 25907 W CipherStorageBase:     at java.lang.Thread.run(Thread.java:919)
08-21 20:54:31.465 22572 25907 W CipherStorageBase: Caused by: javax.crypto.ShortBufferException: Output buffer too short. Produced: 1037, available: 560
08-21 20:54:31.465 22572 25907 W CipherStorageBase:     at android.security.keystore.AndroidKeyStoreCipherSpiBase.engineDoFinal(AndroidKeyStoreCipherSpiBase.java:537)
08-21 20:54:31.465 22572 25907 W CipherStorageBase:     at javax.crypto.Cipher.doFinal(Cipher.java:2002)
08-21 20:54:31.465 22572 25907 W CipherStorageBase:     at javax.crypto.CipherInputStream.getMoreData(CipherInputStream.java:130)
08-21 20:54:31.465 22572 25907 W CipherStorageBase:     ... 21 more

It looks like a silent failure as getGenericCredentials doesn't reject on this exception.
Tested with short strings as password, this issue didn't happen.
In addition, this issue seems to happen only in Google Pixel 4 (Android 9), as tested in Samsung Galaxy S9+ (Android 9), it worked fine.

Noticed:

CipherStorageBase: Caused by: javax.crypto.ShortBufferException: Output buffer too short. Produced: 1037, available: 560

I've read issue #184 and its related issues, and they sound similar to this issue, although the size of data that causes this exception seems arbitrary.

Could anyone please provide any hints/help please?

[cshfang]

It seems there was a PR (https://github.com/oblador/react-native-keychain/pull/288) a while back to fix this exact issue.

This seems like a regression since making similar changes as that PR (diff below) seems to resolve this issue. For some reason, the change seemed to have gotten lost along the way.

I think issue https://github.com/oblador/react-native-keychain/issues/371 is likely related to this one as the exception thrown by copy() just causes the call to hang indefinitely.

diff --git a/android/src/main/java/com/oblador/keychain/cipherStorage/CipherStorageBase.java b/android/src/main/java/com/oblador/keychain/cipherStorage/CipherStorageBase.java
index be40431..d8ca2fc 100644
--- a/android/src/main/java/com/oblador/keychain/cipherStorage/CipherStorageBase.java
+++ b/android/src/main/java/com/oblador/keychain/cipherStorage/CipherStorageBase.java
@@ -360,11 +360,8 @@ abstract public class CipherStorageBase implements CipherStorage {
         handler.initialize(cipher, key, in);
       }

-      try (CipherInputStream decrypt = new CipherInputStream(in, cipher)) {
-        copy(decrypt, output);
-      }
-
-      return new String(output.toByteArray(), UTF8);
+      byte[] decryptedBytes = cipher.doFinal(bytes, IV.IV_LENGTH, bytes.length - IV.IV_LENGTH);
+      return new String(decryptedBytes, UTF8);
     } catch (Throwable fail) {
       Log.w(LOG_TAG, fail.getMessage(), fail);

@@ -471,23 +468,6 @@ abstract public class CipherStorageBase implements CipherStorage {
     //noinspection ConstantConditions
     return TextUtils.isEmpty(service) ? fallback : service;
   }
-
-  /**
-   * Copy input stream to output.
-   *
-   * @param in  instance of input stream.
-   * @param out instance of output stream.
-   * @throws IOException read/write operation failure.
-   */
-  public static void copy(@NonNull final InputStream in, @NonNull final OutputStream out) throws IOException {
-    // Transfer bytes from in to out
-    final byte[] buf = new byte[BUFFER_READ_WRITE_SIZE];
-    int len;
-
-    while ((len = in.read(buf)) > 0) {
-      out.write(buf, 0, len);
-    }
-  }
   //endregion

   //region Nested declarations

[aeirola]

Yeah, the change seems to have been inadvertently reverted in the large refactoring that took place in #260 with the introduction of https://github.com/oblador/react-native-keychain/blob/master/android/src/main/java/com/oblador/keychain/cipherStorage/CipherStorageBase.java. Unfortunately I don't have any Pixel 3 or pixel 4 devices available at the moment, but if anyone who does have one would be willing to test the change and make a PR, I'd be happy to review it.

[cshfang]

PR here https://github.com/oblador/react-native-keychain/pull/385

This was tested against a Pixel 4 XL and seems to be working. Granted, this was not rigorously tested and I do not have physical access to the rest of the Pixel family devices.

[aeirola]

@cshfang Thanks for the quick PR! 🙇 I'll take a look and test locally, although unfortunately without any new Pixel devices. I'll ask around in the community if there would be anyone from the various bug reports willing to verify the fix.

[aeirola]

@HuiSF Thanks for reporting the issue. We have a fix in place for in in the PR #385. Due to the issue being present in only a handful of the devices, of which we don't have a complete set available for testing, I'd like to ask if you would be able to help test out the fix with your device in your specific scenario. You can install it directly to your project using the command npm install github:cshfang/react-native-keychain#fix-strongbox-decryption.

[HuiSF]

Hi @aeirola sorry for the late response. Unfortunately I'm not an Android user myself... but the fix seems working now in the Pixel 4 device which the issue was found with. Thanks for following up.

And thanks to @cshfang for implementing the fix.