Skip to content
This repository has been archived by the owner on Apr 17, 2024. It is now read-only.

Android (tink : 1.3.0-rc3) KeyStoreException + UnrecoverableKeyException #339

Closed
ArjunAchatz opened this issue Mar 30, 2020 · 20 comments
Closed

Comments

@ArjunAchatz
Copy link

Hello,

I'd like to report the follow exceptions happening with Android; we are using 1.3.0-rc3. I am not able to reproduce these issues; however they have been seen in the wild.

OS Issue observed : Android 6,7,8,9
Devices : Samsung Galaxy S6, S7 edge. LG g4, g5, g6

KeyStoreException (Invalid Key Blob) with stack trace :

android.security.KeyStore.getKeyStoreException (KeyStore.java:708)
android.security.keystore.AndroidKeyStoreProvider.loadAndroidKeyStoreSecretKeyFromKeystore (AndroidKeyStoreProvider.java:283)
android.security.keystore.AndroidKeyStoreSpi.engineGetKey (AndroidKeyStoreSpi.java:98)
java.security.KeyStore.getKey (KeyStore.java:1062)
com.google.crypto.tink.integration.android.AndroidKeystoreAesGcm. (AndroidKeystoreAesGcm.java:48)
com.google.crypto.tink.integration.android.AndroidKeystoreKmsClient.getAead (AndroidKeystoreKmsClient.java:111)
com.google.crypto.tink.integration.android.AndroidKeystoreKmsClient.getOrGenerateNewAeadKey (AndroidKeystoreKmsClient.java:130)
com.google.crypto.tink.integration.android.AndroidKeysetManager. (AndroidKeysetManager.java:118)
com.google.crypto.tink.integration.android.AndroidKeysetManager. (AndroidKeysetManager.java:88)
com.google.crypto.tink.integration.android.AndroidKeysetManager$Builder.build (AndroidKeysetManager.java:185)
com.loblaw.pcoptimum.android.app.utils.Crypto$AEAD.getAndroidKeysetManager (Crypto.java:204)

KeyStoreException (Unknown error) with stack trace :

android.security.KeyStore.getKeyStoreException (KeyStore.java:1107)
android.security.keystore.AndroidKeyStoreProvider.loadAndroidKeyStoreSecretKeyFromKeystore (AndroidKeyStoreProvider.java:283)
android.security.keystore.AndroidKeyStoreSpi.engineGetKey (AndroidKeyStoreSpi.java:98)
java.security.KeyStore.getKey (KeyStore.java:825)
com.google.crypto.tink.integration.android.AndroidKeystoreAesGcm. (AndroidKeystoreAesGcm.java:48)
com.google.crypto.tink.integration.android.AndroidKeystoreKmsClient.getAead (AndroidKeystoreKmsClient.java:111)
com.google.crypto.tink.integration.android.AndroidKeystoreKmsClient.getOrGenerateNewAeadKey (AndroidKeystoreKmsClient.java:130)
com.google.crypto.tink.integration.android.AndroidKeysetManager. (AndroidKeysetManager.java:118)
com.google.crypto.tink.integration.android.AndroidKeysetManager. (AndroidKeysetManager.java:88)
com.google.crypto.tink.integration.android.AndroidKeysetManager$Builder.build (AndroidKeysetManager.java:185)

UnrecoverableKeyException (-49) with stack trace :

android.security.KeyStore.getKeyStoreException (KeyStore.java:839)
android.security.keystore.AndroidKeyStoreProvider.getKeyCharacteristics (AndroidKeyStoreProvider.java:236)
android.security.keystore.AndroidKeyStoreProvider.loadAndroidKeyStoreKeyFromKeystore (AndroidKeyStoreProvider.java:356)
android.security.keystore.AndroidKeyStoreSpi.engineGetKey (AndroidKeyStoreSpi.java:101)
java.security.KeyStore.getKey (KeyStore.java:1062)
com.google.crypto.tink.integration.android.AndroidKeystoreAesGcm. (AndroidKeystoreAesGcm.java:48)
com.google.crypto.tink.integration.android.AndroidKeystoreKmsClient.getAead (AndroidKeystoreKmsClient.java:111)
com.google.crypto.tink.integration.android.AndroidKeystoreKmsClient.getOrGenerateNewAeadKey (AndroidKeystoreKmsClient.java:130)
com.google.crypto.tink.integration.android.AndroidKeysetManager. (AndroidKeysetManager.java:118)
com.google.crypto.tink.integration.android.AndroidKeysetManager. (AndroidKeysetManager.java:88)
com.google.crypto.tink.integration.android.AndroidKeysetManager$Builder.build (AndroidKeysetManager.java:185)

@thaidn
Copy link
Contributor

thaidn commented Apr 3, 2020

Yeah this is a known issue with Android Keystore. A small number of Android devices were shipped with broken Keystore implementations.

Do you know the Android versions and device models where this happened?

@ArjunAchatz
Copy link
Author

Yes I can give you a more detailed list :

Note: LG phones account for 40%, Samsung 36% of the errors we've logged

Samsung Galaxy S6 - Android 7
Samsung Galaxy Note 4 - Android 6.0.1
Samsung Tab S2 - Android 7
LG G4 - Android 6
LG G5 - Android 7
LG G6 - Android 9
OnePlus3T - Andriod 9
ZenFone 3 Max (ZC520TL) - Android 8.1.0

@thaidn
Copy link
Contributor

thaidn commented Apr 3, 2020

Thanks.

We plan to add a self-test to detect if Android Keystore is broken. If it is, we bail and just store the keys in private shared preferences. It is less secure, but it's better than nothing.

@ArjunAchatz
Copy link
Author

Yes I agree. It is better than nothing, and on-top of that, we can somewhat rely on the protection of the app sandboxing. Any idea on when this fix would be put in ?

@ArjunAchatz
Copy link
Author

Also when you implementing this, will there be a way to know that Keystore failed, and that SharedPreferences was used ?

@thaidn
Copy link
Contributor

thaidn commented Apr 15, 2020

Also when you implementing this, will there be a way to know that Keystore failed, and that SharedPreferences was used ?

Good question. I'm not sure. How's about a log statement?

@simtse
Copy link
Contributor

simtse commented Apr 22, 2020

We are asking the same question now that we are finding similar reports as @ArjunAchatz that there will be devices that just won't be able to have a stable keystore. Any progress on this other than a log statement? Is there something in the KeysetHandle that can return what type of storage is used for the keys?

Btw, I still see this on that latest stable 1.3 release. No the RC releases.
I see this happen also with the AndroidX security library, which is also using 1.3 Tink as of the latest RC.

@simtse
Copy link
Contributor

simtse commented May 4, 2020

@thaidn Other than using the private shared preferences, is there a way to reset the key that the AndroidKeystoreAesGcm uses? For example, if the result of not being able to read the keyset means cleaning user data in the app, the underlying key created by Tink for the app in the Android keystore would probably still be broken.

But if Tink had a mechanism to clear/reset/remove the key from the Android keystore, then the user wouldn't need to fully reinstall the app in order to start with a fresh key in the key store.

@thaidn
Copy link
Contributor

thaidn commented May 9, 2020

@simtse does https://developer.android.com/reference/java/security/KeyStore#deleteEntry(java.lang.String) work for you? This will delete the master key.

@simtse
Copy link
Contributor

simtse commented May 13, 2020

Yes I probably could do that. Seems like an implementation detail to know to remove the prefix "android-keystore:// inside of AndroidKeystoreKmsClient. So I think I've heard of some consumers of Tink remove directly like this, but it seems like a more extreme case that should be done in conjunction with Tink so the keyset could be reset along with a new Android keystore key.

I don't mind removing as you suggested, but I'm also curious what you think about putting some mechanism in the future for this kind of reset of the master key.

Slightly similar, but separate question. If Tink fails to decrypt the keyset with the Android keystore, does it just create a brand new Keyset and stores it into the shared preferences?

@thaidn
Copy link
Contributor

thaidn commented May 15, 2020

@simtse our release candidates are pretty stable. At Google everybody uses Tink at HEAD, and public release candidates are built from HEAD.

Also when you implementing this, will there be a way to know that Keystore failed, and that SharedPreferences was used ?

We're adding a isUsingKeystore() method to AndroidKeysetManager.

I don't mind removing as you suggested, but I'm also curious what you think about putting some mechanism in the future for this kind of reset of the master key.

How's about a delete(String keyUri) method in AndroidKeystoreKmsClient?

Slightly similar, but separate question. If Tink fails to decrypt the keyset with the Android keystore, does it just create a brand new Keyset and stores it into the shared preferences?

No, if it can't decrypt it assumes that the keyset is in cleartext.

Edit: s/if it can decrypt/if it can't decrypt/

@thaidn thaidn closed this as completed in 1b1b414 May 15, 2020
@simtse
Copy link
Contributor

simtse commented Jun 10, 2020

How's about a delete(String keyUri) method in AndroidKeystoreKmsClient?

That might be useful. I think I would need to create some redundancy to be able to recover since if the keyset couldn't be decrypted, anything encrypted by the original keyset will no longer work. I would need some other backup somewhere in the app to create a new keyset from a new key in the Android keystore. Or else, users would fail to access the encrypted content.

No, if it can decrypt it assumes that the keyset is in cleartext.

So this is a little scary possibly. It sounds like it transitions to using the keyset as if it were clear text. And anything that was encrypted by the encrypted keyset would not be decryptable with the clear text keyset.

And, that also means that any new content encrypted with the clear text keyset would be less secure. If someone were to gain access (via root) to the keyset, then they could probably use the keyset (which is clear text now) to decrypt content.

@thaidn am I thinking this right?

@thaidn
Copy link
Contributor

thaidn commented Jun 10, 2020

No, if it can decrypt it assumes that the keyset is in cleartext.

There's a typo in my reply. If Tink can't decrypt it assumes the keyset is in cleartext. If the keyset was actually encrypted, Tink won't be able to read it and will end up generating a brand new keyset. You're right that anything encrypted with the old keyset will be lost. This is not ideal, but if the keyset is corrupted there's nothing Tink can do.

And, that also means that any new content encrypted with the clear text keyset would be less secure. If someone were to gain access (via root) to the keyset, then they could probably use the keyset (which is clear text now) to decrypt content.

Tink always attempts to encrypt the keyset before writing to shared preferences. It only stores in cleartext when encryption fails (i.e., because of failure in Android Keystore).

Root attackers can always call Android Keystore to decrypt the keyset. In fact we're thinking about getting rid of Android Keystore, which is too unstable on many devices to be useful. Most decryption errors in Tink or AndroidX Security are caused by Android Keystore.

@thaidn thaidn closed this as completed Jun 10, 2020
@thaidn
Copy link
Contributor

thaidn commented Jun 10, 2020

@simtse
Copy link
Contributor

simtse commented Jun 10, 2020

Root attackers can always call Android Keystore to decrypt the keyset. In fact we're thinking about getting rid of Android Keystore, which is too unstable on many devices to be useful. Most decryption errors in Tink or AndroidX Security are caused by Android Keystore.

That would be wonderful to move away from the Android keystore if it is unreliable, and to something more reliable. Is that something that will be included into Tink by default and not needing to have our own KMS?

@thaidn
Copy link
Contributor

thaidn commented Jun 10, 2020

We plan to add a self-test to detect if Android Keystore is broken. If it is, we bail and just store the keys in private shared preferences. It is less secure, but it's better than nothing.

We've also added this self-test check.

Storing the keyset in shared preferences isn't too bad. In most Android devices, shared preferences are encrypted by Android full disk encryption, which derives encryption keys from the screen unlock passcode and a hardware key.

In order to read the keyset off shared preferences of your app, the attacker must have the capability to execute code as your app. This can only happen if:

1/ The attacker exploits a code execution vulnerability in your app. In this case, encrypting the keyset won't help because the attacker can also call Android Keystore (as your app) to decrypt it.

2/ The attacker exploits a specific vulnerability in your app that allows to read arbitrary shared preferences. I haven't seen any app having this issue.

3/ Shared preferences are automatically backed up. The back ups are stored in the user's Google Drive -- where it's protected by the user's Google Account credentials. If the attacker compromises the user's Google Account, they can get access to the keyset.

One countermeasure is to exclude the shared preferences file containing the keyset from auto backup. Though this can lead to data loss, so it's a trade-off.

4/ The attacker gets root on the device. Similar to 1/, encrypting won't help.

@thaidn
Copy link
Contributor

thaidn commented Jun 10, 2020

@chadbrubaker to keep me honest

@msfjarvis
Copy link

OP mentioned OnePlus 3T on Android 9 as a broken device and while the self-test from the master branch passes on it (verified independently with a sample app), the device still appears to not have a usable KeyStore implementation (android-password-store/Android-Password-Store#1144). Is there anything we can do here?

@msfjarvis
Copy link

Hey @thaidn do you have anything else we can try?

@RomanMinenok
Copy link

@thaidn androidx.security:security-crypto:1.0.0-rc04 which uses Tink 1.5 still has this issue. So did you add a fallback to use regular shared preferences in case something like this happens or decided not to do it?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants