search

patrickfav / armadillo

A shared preference implementation for confidential data in Android. Per default uses AES-GCM, BCrypt and HKDF as cryptographic primitives. Uses the concept of device fingerprinting combined with optional user provided passwords and strong password hashes.
https://favr.dev/opensource/armadillo
Apache License 2.0
281 stars 52 forks source link

Support devices runing KitKat (API 19) #6 #10

Closed davidmigloz closed 5 years ago

davidmigloz commented 6 years ago
patrickfav commented 6 years ago

Hi David, thanks for the PR.

I have reservations about the change of using no AAD, because if a device updates to 21+ it will not verify, requiring the lib to persist SDK version and handle migration. This adds quite a bit of complexity and probably opens an downgrade attack vector. Do you have any idea how to tackle that issue?

Also, how serious is the issue of missing GCMParamSpec? Is this just on a handful devices? It seems weired that a class is missing from the Android SDK thats supposed to be there?

davidmigloz commented 6 years ago

Hi Patrick,

The traces of the two issues (tested in a Samsung Galaxy SII with Android 4.4.4):

Issue with the GCMParamSpec : 

07-10 08:38:49.386 4299-4299/at.favre.lib.securesharedpreferences E/AndroidRuntime: FATAL EXCEPTION: main
    Process: at.favre.lib.securesharedpreferences, PID: 4299
    java.lang.IllegalStateException: at.favre.lib.armadillo.EncryptionProtocolException: at.favre.lib.armadillo.AuthenticatedEncryptionException: could not encrypt
        at at.favre.lib.armadillo.SecureSharedPreferences.encryptToBase64(SecureSharedPreferences.java:337)
        at at.favre.lib.armadillo.SecureSharedPreferences.access$300(SecureSharedPreferences.java:31)
        at at.favre.lib.armadillo.SecureSharedPreferences$Editor.putString(SecureSharedPreferences.java:248)
        at at.favre.lib.securesharedpreferences.MainActivity$2.onClick(MainActivity.java:58)
        at android.view.View.performClick(View.java:4445)
        at android.view.View$PerformClick.run(View.java:18446)
        at android.os.Handler.handleCallback(Handler.java:733)
        at android.os.Handler.dispatchMessage(Handler.java:95)
        at android.os.Looper.loop(Looper.java:136)
        at android.app.ActivityThread.main(ActivityThread.java:5158)
        at java.lang.reflect.Method.invokeNative(Native Method)
        at java.lang.reflect.Method.invoke(Method.java:515)
        at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:732)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:566)
        at dalvik.system.NativeStart.main(Native Method)
     Caused by: at.favre.lib.armadillo.EncryptionProtocolException: at.favre.lib.armadillo.AuthenticatedEncryptionException: could not encrypt
        at at.favre.lib.armadillo.DefaultEncryptionProtocol.encrypt(DefaultEncryptionProtocol.java:81)
        at at.favre.lib.armadillo.SecureSharedPreferences.encryptToBase64(SecureSharedPreferences.java:335)
        at at.favre.lib.armadillo.SecureSharedPreferences.access$300(SecureSharedPreferences.java:31) 
        at at.favre.lib.armadillo.SecureSharedPreferences$Editor.putString(SecureSharedPreferences.java:248) 
        at at.favre.lib.securesharedpreferences.MainActivity$2.onClick(MainActivity.java:58) 
        at android.view.View.performClick(View.java:4445) 
        at android.view.View$PerformClick.run(View.java:18446) 
        at android.os.Handler.handleCallback(Handler.java:733) 
        at android.os.Handler.dispatchMessage(Handler.java:95) 
        at android.os.Looper.loop(Looper.java:136) 
        at android.app.ActivityThread.main(ActivityThread.java:5158) 
        at java.lang.reflect.Method.invokeNative(Native Method) 
        at java.lang.reflect.Method.invoke(Method.java:515) 
        at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:732) 
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:566) 
        at dalvik.system.NativeStart.main(Native Method) 
     Caused by: at.favre.lib.armadillo.AuthenticatedEncryptionException: could not encrypt
        at at.favre.lib.armadillo.AesGcmEncryption.encrypt(AesGcmEncryption.java:82)
        at at.favre.lib.armadillo.DefaultEncryptionProtocol.encrypt(DefaultEncryptionProtocol.java:70)
        at at.favre.lib.armadillo.SecureSharedPreferences.encryptToBase64(SecureSharedPreferences.java:335) 
        at at.favre.lib.armadillo.SecureSharedPreferences.access$300(SecureSharedPreferences.java:31) 
        at at.favre.lib.armadillo.SecureSharedPreferences$Editor.putString(SecureSharedPreferences.java:248) 
        at at.favre.lib.securesharedpreferences.MainActivity$2.onClick(MainActivity.java:58) 
        at android.view.View.performClick(View.java:4445) 
        at android.view.View$PerformClick.run(View.java:18446) 
        at android.os.Handler.handleCallback(Handler.java:733) 
        at android.os.Handler.dispatchMessage(Handler.java:95) 
        at android.os.Looper.loop(Looper.java:136) 
        at android.app.ActivityThread.main(ActivityThread.java:5158) 
        at java.lang.reflect.Method.invokeNative(Native Method) 
        at java.lang.reflect.Method.invoke(Method.java:515) 
        at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:732) 
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:566) 
        at dalvik.system.NativeStart.main(Native Method) 
     Caused by: java.security.InvalidAlgorithmParameterException: unknown parameter type.
        at com.android.org.bouncycastle.jcajce.provider.symmetric.util.BaseBlockCipher.engineInit(BaseBlockCipher.java:559)
        at javax.crypto.Cipher.init(Cipher.java:616)
        at javax.crypto.Cipher.init(Cipher.java:566)
        at at.favre.lib.armadillo.AesGcmEncryption.encrypt(AesGcmEncryption.java:68)
07-10 08:38:49.386 4299-4299/at.favre.lib.securesharedpreferences E/AndroidRuntime: FATAL EXCEPTION: main
    Process: at.favre.lib.securesharedpreferences, PID: 4299
    java.lang.IllegalStateException: at.favre.lib.armadillo.EncryptionProtocolException: at.favre.lib.armadillo.AuthenticatedEncryptionException: could not encrypt
        at at.favre.lib.armadillo.SecureSharedPreferences.encryptToBase64(SecureSharedPreferences.java:337)
        at at.favre.lib.armadillo.SecureSharedPreferences.access$300(SecureSharedPreferences.java:31)
        at at.favre.lib.armadillo.SecureSharedPreferences$Editor.putString(SecureSharedPreferences.java:248)
        at at.favre.lib.securesharedpreferences.MainActivity$2.onClick(MainActivity.java:58)
        at android.view.View.performClick(View.java:4445)
        at android.view.View$PerformClick.run(View.java:18446)
        at android.os.Handler.handleCallback(Handler.java:733)
        at android.os.Handler.dispatchMessage(Handler.java:95)
        at android.os.Looper.loop(Looper.java:136)
        at android.app.ActivityThread.main(ActivityThread.java:5158)
        at java.lang.reflect.Method.invokeNative(Native Method)
        at java.lang.reflect.Method.invoke(Method.java:515)
        at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:732)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:566)
        at dalvik.system.NativeStart.main(Native Method)
     Caused by: at.favre.lib.armadillo.EncryptionProtocolException: at.favre.lib.armadillo.AuthenticatedEncryptionException: could not encrypt
        at at.favre.lib.armadillo.DefaultEncryptionProtocol.encrypt(DefaultEncryptionProtocol.java:81)
        at at.favre.lib.armadillo.SecureSharedPreferences.encryptToBase64(SecureSharedPreferences.java:335)
        at at.favre.lib.armadillo.SecureSharedPreferences.access$300(SecureSharedPreferences.java:31) 
        at at.favre.lib.armadillo.SecureSharedPreferences$Editor.putString(SecureSharedPreferences.java:248) 
        at at.favre.lib.securesharedpreferences.MainActivity$2.onClick(MainActivity.java:58) 
        at android.view.View.performClick(View.java:4445) 
        at android.view.View$PerformClick.run(View.java:18446) 
        at android.os.Handler.handleCallback(Handler.java:733) 
        at android.os.Handler.dispatchMessage(Handler.java:95) 
        at android.os.Looper.loop(Looper.java:136) 
        at android.app.ActivityThread.main(ActivityThread.java:5158) 
        at java.lang.reflect.Method.invokeNative(Native Method) 
        at java.lang.reflect.Method.invoke(Method.java:515) 
        at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:732) 
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:566) 
        at dalvik.system.NativeStart.main(Native Method) 
     Caused by: at.favre.lib.armadillo.AuthenticatedEncryptionException: could not encrypt
        at at.favre.lib.armadillo.AesGcmEncryption.encrypt(AesGcmEncryption.java:82)
        at at.favre.lib.armadillo.DefaultEncryptionProtocol.encrypt(DefaultEncryptionProtocol.java:70)
        at at.favre.lib.armadillo.SecureSharedPreferences.encryptToBase64(SecureSharedPreferences.java:335) 
        at at.favre.lib.armadillo.SecureSharedPreferences.access$300(SecureSharedPreferences.java:31) 
        at at.favre.lib.armadillo.SecureSharedPreferences$Editor.putString(SecureSharedPreferences.java:248) 
        at at.favre.lib.securesharedpreferences.MainActivity$2.onClick(MainActivity.java:58) 
        at android.view.View.performClick(View.java:4445) 
        at android.view.View$PerformClick.run(View.java:18446) 
        at android.os.Handler.handleCallback(Handler.java:733) 
        at android.os.Handler.dispatchMessage(Handler.java:95) 
        at android.os.Looper.loop(Looper.java:136) 
        at android.app.ActivityThread.main(ActivityThread.java:5158) 
        at java.lang.reflect.Method.invokeNative(Native Method) 
        at java.lang.reflect.Method.invoke(Method.java:515) 
        at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:732) 
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:566) 
        at dalvik.system.NativeStart.main(Native Method) 
     Caused by: java.security.InvalidAlgorithmParameterException: unknown parameter type.
        at com.android.org.bouncycastle.jcajce.provider.symmetric.util.BaseBlockCipher.engineInit(BaseBlockCipher.java:559)
        at javax.crypto.Cipher.init(Cipher.java:616)
        at javax.crypto.Cipher.init(Cipher.java:566)
        at at.favre.lib.armadillo.AesGcmEncryption.encrypt(AesGcmEncryption.java:68)

Issue with the AAD:

07-10 08:44:56.974 4764-4764/at.favre.lib.securesharedpreferences E/AndroidRuntime: FATAL EXCEPTION: main
    Process: at.favre.lib.securesharedpreferences, PID: 4764
    java.lang.IllegalStateException: at.favre.lib.armadillo.EncryptionProtocolException: at.favre.lib.armadillo.AuthenticatedEncryptionException: could not encrypt
        at at.favre.lib.armadillo.SecureSharedPreferences.encryptToBase64(SecureSharedPreferences.java:337)
        at at.favre.lib.armadillo.SecureSharedPreferences.access$300(SecureSharedPreferences.java:31)
        at at.favre.lib.armadillo.SecureSharedPreferences$Editor.putString(SecureSharedPreferences.java:248)
        at at.favre.lib.securesharedpreferences.MainActivity$2.onClick(MainActivity.java:58)
        at android.view.View.performClick(View.java:4445)
        at android.view.View$PerformClick.run(View.java:18446)
        at android.os.Handler.handleCallback(Handler.java:733)
        at android.os.Handler.dispatchMessage(Handler.java:95)
        at android.os.Looper.loop(Looper.java:136)
        at android.app.ActivityThread.main(ActivityThread.java:5158)
        at java.lang.reflect.Method.invokeNative(Native Method)
        at java.lang.reflect.Method.invoke(Method.java:515)
        at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:732)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:566)
        at dalvik.system.NativeStart.main(Native Method)
     Caused by: at.favre.lib.armadillo.EncryptionProtocolException: at.favre.lib.armadillo.AuthenticatedEncryptionException: could not encrypt
        at at.favre.lib.armadillo.DefaultEncryptionProtocol.encrypt(DefaultEncryptionProtocol.java:81)
        at at.favre.lib.armadillo.SecureSharedPreferences.encryptToBase64(SecureSharedPreferences.java:335)
        at at.favre.lib.armadillo.SecureSharedPreferences.access$300(SecureSharedPreferences.java:31) 
        at at.favre.lib.armadillo.SecureSharedPreferences$Editor.putString(SecureSharedPreferences.java:248) 
        at at.favre.lib.securesharedpreferences.MainActivity$2.onClick(MainActivity.java:58) 
        at android.view.View.performClick(View.java:4445) 
        at android.view.View$PerformClick.run(View.java:18446) 
        at android.os.Handler.handleCallback(Handler.java:733) 
        at android.os.Handler.dispatchMessage(Handler.java:95) 
        at android.os.Looper.loop(Looper.java:136) 
        at android.app.ActivityThread.main(ActivityThread.java:5158) 
        at java.lang.reflect.Method.invokeNative(Native Method) 
        at java.lang.reflect.Method.invoke(Method.java:515) 
        at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:732) 
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:566) 
        at dalvik.system.NativeStart.main(Native Method) 
     Caused by: at.favre.lib.armadillo.AuthenticatedEncryptionException: could not encrypt
        at at.favre.lib.armadillo.AesGcmEncryption.encrypt(AesGcmEncryption.java:88)
        at at.favre.lib.armadillo.DefaultEncryptionProtocol.encrypt(DefaultEncryptionProtocol.java:70)
        at at.favre.lib.armadillo.SecureSharedPreferences.encryptToBase64(SecureSharedPreferences.java:335) 
        at at.favre.lib.armadillo.SecureSharedPreferences.access$300(SecureSharedPreferences.java:31) 
        at at.favre.lib.armadillo.SecureSharedPreferences$Editor.putString(SecureSharedPreferences.java:248) 
        at at.favre.lib.securesharedpreferences.MainActivity$2.onClick(MainActivity.java:58) 
        at android.view.View.performClick(View.java:4445) 
        at android.view.View$PerformClick.run(View.java:18446) 
        at android.os.Handler.handleCallback(Handler.java:733) 
        at android.os.Handler.dispatchMessage(Handler.java:95) 
        at android.os.Looper.loop(Looper.java:136) 
        at android.app.ActivityThread.main(ActivityThread.java:5158) 
        at java.lang.reflect.Method.invokeNative(Native Method) 
        at java.lang.reflect.Method.invoke(Method.java:515) 
        at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:732) 
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:566) 
        at dalvik.system.NativeStart.main(Native Method) 
     Caused by: java.lang.UnsupportedOperationException: This cipher does not support Authenticated Encryption with Additional Data
        at javax.crypto.CipherSpi.engineUpdateAAD(CipherSpi.java:393)
        at javax.crypto.Cipher.updateAAD(Cipher.java:1056)
        at at.favre.lib.armadillo.AesGcmEncryption.encrypt(AesGcmEncryption.java:77)
davidmigloz commented 6 years ago

I didn't think about what happens when the user updates to 21. But.. do you think it's worth to hadle it? I don't expect a lot of users updating from 19 to 21 in 2018, if they are using 19 it's probably because the manufacter droped support in that version. What do you think?

About GCMParamSpec, I have no idea how many devices don't have it implemented. The limitation of using IvParameterSpec is that we have to stick to the default tag lenght of 128 bits.

patrickfav commented 6 years ago

There are still working SII around 😆? Anyways, its seems that the last official update for that device was 4.1.2 (according to Wikipedia) wo we are dealing with custom rom issues. Not too keen on supporting those :/

So I suggest instead of patching the default encryption logic, why not just add a new implementation of AuthenticatedEncryption without AAD called e.g. KitKatSupportAesGcmEncryption. The Armadillo builder allows a lot of DI so the user could just use this implementation if he/she likes, leaving everybody else without needing to support those exotic devices with the standard impl. One would need to add a paragraph in the Readme and explain (e.g. "Known Issues") when to use this.

What do you think?

davidmigloz commented 6 years ago

Yes it has a cyanogenmod rom.. I don't have any device with pure Android 4.4. I also tested it in an emulator and I'm getting the same two exceptions. Do you have any KitKat device where it's working?

So you think is better to make the default AesGcmEncryption compatible just from API 21 and if you want to support from 19 inject KitKatSupportAesGcmEncryption?

HonzaR commented 6 years ago

Hi guys, I have few KitKat devices. And the crash happens on all of them. Today standard for new project says we should still support Kitkat devices, because there is still more than 10% active devices market share. See

https://developer.android.com/about/dashboards/

patrickfav commented 6 years ago

I just tried it with the emulator and you are right, the BC version of this build does not seem to support the IV type - my statement was based upon the assumption that this is specific issue to some roms. It is very strange that Google would expose the new APIs but would not support it? We should check if there maybe is another provider in KitKat which supports AAD and GcmIvParamSpec?

But I agree, that we should add a fallback variant in the default impl for kitkat, but maybe make it more clear that it is used (So a dev is clear that this will break data on SDK update in KitKat)

I leave this link as a reference: https://stackoverflow.com/questions/42928339/gcmparameterspec-throws-invalidalgorithmparameterexception-unknown-parameter-ty

davidmigloz commented 6 years ago

Here there is a Google implementation: https://github.com/google/tink/blob/master/java/src/main/java/com/google/crypto/tink/subtle/AesGcmJce.java

About the AAD, the docs say:

On Android KitKat (API level 19) this method does not support non null or non empty {@code associatedData}. It might not work at all in older versions.

For the params, they first check that GCMParameterSpec class exists and if not they use IvParameterSpec. I tried that and it doesn't work, because the class does exist but it's not implemented.

private static AlgorithmParameterSpec getParams(...) {
    try {
        Class.forName("javax.crypto.spec.GCMParameterSpec");
        return new GCMParameterSpec(8 * TAG_SIZE_IN_BYTES, buf, offset, len);
    } catch (ClassNotFoundException e) {
        if (SubtleUtil.isAndroid()) {
            // GCMParameterSpec should always be present in Java 7 or newer, but it's missing on Android
            // devices with API level < 19. Fortunately, with a modern copy of Conscrypt (either through
            // GMS or bundled with the app) we can initialize the cipher with just an IvParameterSpec.
            // It will use a tag size of 128 bits. We'd double check the tag size in encrypt().
            return new IvParameterSpec(buf, offset, len);
        }
    }
    throw new GeneralSecurityException("cannot use AES-GCM: javax.crypto.spec.GCMParameterSpec not found");
}
patrickfav commented 5 years ago

@davidmigloz I finally had time to tackle this issue. I implemented a version in #31 supporting migration and implementing AES/CBC + MAC for Kitkat. The work on this PR is highly appreciated, but I think its a better idea to go with #31, so I will close this PR.

If you have time please take a look at #31, since I need a second pair of eyes to check the encrypt-then-mac logic :)

davidmigloz commented 5 years ago

Sure, it's a much better approach than this :)