sameerkapps / SecureStorage

119 stars 32 forks source link

Android release build crashes while trying to decrypt data from the Android KeyStore #52

Open lukepothier opened 4 years ago

lukepothier commented 4 years ago

I'm using version 1.3.1, targeting Android 10.0 (Q) (API 29). The issue occurred on my OnePlus 5T (Android 9 security patch 1 December 2019).

The stack trace (app identifiers replaced with *****):

2020-02-07 19:00:35.112 24401-24401/? E/AndroidRuntime: FATAL EXCEPTION: main
    Process: *****, PID: 24401
    android.runtime.JavaProxyThrowable: MvvmCross.Exceptions.MvxException: Problem navigating to ViewModel InterstitialViewModel ---> MvvmCross.Exceptions.MvxException: Failed to construct and initialize ViewModel for type *****.Core.ViewModels.InterstitialViewModel from locator MvxDefaultViewModelLocator - check InnerException for more information ---> MvvmCross.Exceptions.MvxException: Problem creating viewModel of type InterstitialViewModel ---> MvvmCross.Exceptions.MvxIoCResolveException: Failed to construct TetherDeviceService ---> System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. ---> Java.Security.GeneralSecurityException: Exception of type 'Java.Security.GeneralSecurityException' was thrown. ---> Java.Lang.Exception: Signature/MAC verification failed
       --- End of inner exception stack trace ---
      at Java.Interop.JniEnvironment+InstanceMethods.CallNonvirtualObjectMethod (Java.Interop.JniObjectReference instance, Java.Interop.JniObjectReference type, Java.Interop.JniMethodInfo method, Java.Interop.JniArgumentValue* args) <0x704fe222d0 + 0x000b7> in <c7ee2790529640cdbb5b7aa7889014aa>:0 
      at Java.Interop.JniPeerMembers+JniInstanceMethods.InvokeNonvirtualObjectMethod (System.String encodedMember, Java.Interop.IJavaPeerable self, Java.Interop.JniArgumentValue* parameters) <0x704fe25b80 + 0x000b3> in <c7ee2790529640cdbb5b7aa7889014aa>:0 
      at Javax.Crypto.Cipher.DoFinal (System.Byte[] input, System.Int32 inputOffset, System.Int32 inputLen) [0x00052] in <4993a3b290714f409bbb9e88a3193954>:0 
      at Plugin.SecureStorage.AndroidKeyStoreImplementation+AndroidKeyStore.Decrypt (System.Byte[] data) [0x0005f] in <744f44ee05674e8bafdb613951f6815b>:0 
      at Plugin.SecureStorage.AndroidKeyStoreImplementation.GetValue (System.String key, System.String defaultValue) [0x0004e] in <744f44ee05674e8bafdb613951f6815b>:0 
      at Plugin.SecureStorage.SecureStorageImplementation.GetValue (System.String key, System.String defaultValue) [0x00009] in <744f44ee05674e8bafdb613951f6815b>:0 
      at *****.Core.Services.SecureStorageService.GetValue (System.String key) [0x00005] in <dae77869776043c6bd1aebc1ccf425f7>:0 
      at *****.Core.Services.TetherDeviceService..ctor (*****.Core.Services.ISecureStorageService secureStorageService, *****.Core.Services.Abstractions.IHttpService httpService, *****.Core.Services.IRemoteConfigurationService remoteConfigurationService) [0x00031] in <dae77869776043c6bd1aebc1ccf425f7>:0 
        at (wrapper managed-to-native) System.Reflection.RuntimeConstructorInfo.InternalInvoke(System.Reflection.RuntimeConstructorInfo,object,object[],System.Exception&)
      at System.Reflection.RuntimeConstructorInfo.InternalInvoke (System.Object obj, System.Object[] parameters, System.Boolean wrapExceptions) <0x70502428a0 + 0x00037> in <e172acb0ee254f27a4fa374cc5316622>:0 
       --- End of inner exception stack trace ---
      at System.Reflection.RuntimeConstructorInfo.InternalInvoke (System.Object obj, System.Object[] parameters, System.Boolean wrapExceptions) <0x70502428a0 + 0x00080> in <e172acb0ee254f27a4fa374cc5316622>:0 
      at System.Reflection.RuntimeConstructorInfo.DoInvoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) <0x70502426d0 + 0x00177> in <e172acb0ee254f27a4fa374cc5316622>:0 
      at System.Reflection.RuntimeConstructorInfo.Invoke (System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) <0x7050242970 + 0x00037> in <e172acb0ee254f27a4fa374cc5316622>:0 
      at System.Reflection.ConstructorInfo.Invoke (System.Object[] parameters) <0x7050240ab0 + 0x00033> in <e172acb0ee254f27a4fa374cc5316622>:0 

My app immediately crashes after trying to retrieve a value from secure storage. For what it's worth, I've only observed this issue within a release build and when the value being retrieved doesn't exist yet. The offending line seems to be:

CrossSecureStorage.Current.GetValue(key);

where key is just a constant string. The app is trying to read securely stored credentials at startup to communicate with an API, but is crashing in trying to read the value on first load, possibly because the value doesn't exist yet.

I have found that setting android:allowBackup="false" in my manifest seems to be a viable workaround for this issue (in my case). I suspect that using CrossSecureStorage.Current.HasKey(key); before GetValue(key) would also solve the issue, but I haven't tried that yet.

lukepothier commented 4 years ago

I suspect that wrapping this in a try/catch and catching Java.Security.GeneralSecurityException and just returning defaultValue this issue would go away.