tink icon indicating copy to clipboard operation
tink copied to clipboard

Bug: using EncryptedSharedPreferences, it can cause crashes to users right when initializing it

Open AndroidDeveloperLB opened this issue 2 years ago • 46 comments

This: https://issuetracker.google.com/issues/176215143

I was told I should write about this here: https://issuetracker.google.com/issues/185219606#comment4

AndroidDeveloperLB avatar Aug 06 '21 15:08 AndroidDeveloperLB

any comment about this ?

I detected this issue occurs in ( Not on all devices ):

  • huawei P20
  • huawei Y6II
  • Samsung Galaxy S9
  • Galaxy Grand Prime Plus
  • Asus ZenFone Max Pro M1

the master key android-keystore://_androidx_security_master_key_ exists but is unusable

oswaldo89 avatar Aug 16 '21 21:08 oswaldo89

Do you have a stack trace? How frequently has this happened?

thaidn avatar Aug 16 '21 23:08 thaidn

@thaidn Check the original post. It has enough.

AndroidDeveloperLB avatar Aug 16 '21 23:08 AndroidDeveloperLB

Do you have a stack trace? How frequently has this happened?

@thaidn

Fatal Exception: java.security.KeyStoreException: the master key android-keystore://androidx_security_master_key exists but is unusable at com.google.crypto.tink.integration.android.AndroidKeysetManager$Builder.readOrGenerateNewMasterKey(AndroidKeysetManager.java:275) at com.google.crypto.tink.integration.android.AndroidKeysetManager$Builder.build(AndroidKeysetManager.java:236) at androidx.security.crypto.EncryptedSharedPreferences.create(EncryptedSharedPreferences.java:155) at androidx.security.crypto.EncryptedSharedPreferences.create(EncryptedSharedPreferences.java:120) ... Caused by java.security.UnrecoverableKeyException: Failed to obtain information about key at android.security.keystore.AndroidKeyStoreProvider.loadAndroidKeyStoreSecretKeyFromKeystore(AndroidKeyStoreProvider.java:282) at android.security.keystore.AndroidKeyStoreSpi.engineGetKey(AndroidKeyStoreSpi.java:98) at java.security.KeyStore.getKey(KeyStore.java:825) at com.google.crypto.tink.integration.android.AndroidKeystoreAesGcm.(AndroidKeystoreAesGcm.java:58) at com.google.crypto.tink.integration.android.AndroidKeystoreKmsClient.getAead(AndroidKeystoreKmsClient.java:164) at com.google.crypto.tink.integration.android.AndroidKeysetManager$Builder.readOrGenerateNewMasterKey(AndroidKeysetManager.java:267) at com.google.crypto.tink.integration.android.AndroidKeysetManager$Builder.build(AndroidKeysetManager.java:236) at androidx.security.crypto.EncryptedSharedPreferences.create(EncryptedSharedPreferences.java:155) at androidx.security.crypto.EncryptedSharedPreferences.create(EncryptedSharedPreferences.java:120)

oswaldo89 avatar Aug 31 '21 15:08 oswaldo89

since we implemented the encrypted preferences, it happens very frequently, only to certain devices.

image

oswaldo89 avatar Aug 31 '21 15:08 oswaldo89

Do we have any fix or workaround for this crash already? Currently we have on Firebase 23 reports of this exception:

Non-fatal Exception: java.security.KeyStoreException: the master key android-keystore://_androidx_security_master_key_ exists but is unusable
       at com.google.crypto.tink.integration.android.AndroidKeysetManager$Builder.readOrGenerateNewMasterKey(AndroidKeysetManager.java:275)
       at com.google.crypto.tink.integration.android.AndroidKeysetManager$Builder.build(AndroidKeysetManager.java:236)
       at androidx.security.crypto.EncryptedSharedPreferences.create(EncryptedSharedPreferences.java:155)
       at androidx.security.crypto.EncryptedSharedPreferences.create(EncryptedSharedPreferences.java:120)
       at wit.android.bcpBankingApp.utils.security.EncryptedKeyStorage.init(EncryptedKeyStorage.java:49)
       ...

The exceptions occur in these devices, mainly in Android 7 (57%) and 6 (26%):

Huawei Honor 8 Huawei Y6II OPPO Find X3 Pro wheatek BV6300Pro

Currently we are catching the exception, but by doing so, every method invoked to the EncryptedSharedPreferences instance afterwards will throw an NPE.

Tharkius avatar Sep 02 '21 13:09 Tharkius

This is an incredibly frustrating issue and makes the latest versions of this library essentially useless. The workarounds I found were -

  1. Add
android:allowBackup="false"
android:fullBackupContent="false"

to your Application in AndroidManifest.xml. This prevents your encrypted files from getting backed up - EncryptedSharedPreferences can't open them on reinstall anyway.

  1. Don't use 1.1.0-alpha02 or 1.1.0-alpha03. From my experience, 1.1.0-alpha01 is the last working version.

One or the other may be enough, but this nasty issue popped up for me the day-of our first production release, so I threw the whole kitchen sink at it. As far as I can tell, the issue hasn't been seen since.

One other alternative I considered was just not using EncryptedSharedPreferences at all and just falling back to good ol' SharedPreferences. Obviously you lose the entire point of the library, but at least it appears to work more often than not.

jarrodrobins avatar Sep 02 '21 23:09 jarrodrobins

@jarrodrobins Wait, can I avoid the flags of backup, and use the alpha01 and it should work fine? All of the crashes I've reported won't occur again?

AndroidDeveloperLB avatar Sep 02 '21 23:09 AndroidDeveloperLB

Don't use 1.1.0-alpha02 or 1.1.0-alpha03. From my experience, 1.1.0-alpha01 is the last working version.

Wait, when you switched to 1.1.0-alpha01, the problem went away?

thaidn avatar Sep 02 '21 23:09 thaidn

@jarrodrobins Wait, can I avoid the flags of backup, and use the alpha02 and it should work fine? All of the crashes I've reported won't occur again?

We've had the app with allowBackUp as false for a long time before the crash showed up so that's not going to solve the problem.

These are very rare cases though and I'm tented to propose showing some dialog to the user explaining na critical error occurred. Because it is a critical error...

Tharkius avatar Sep 02 '21 23:09 Tharkius

That was my experience with 1.1.0-alpha01 (NOT 02 or 03), yes. It may not be necessary to disable backups, but a number of other posts (eg StackOverflow) referenced that as a possible workaround.

The 'testing' I was doing involved finding a teammate who was able to replicate the issue and having them install a bunch of different test builds changing various bits and pieces as it was not something I could replicate locally. The working build for them involved both 'fixes' but yes, it's possible changing it to alpha01 is enough.

I implemented this workaround on my project a while ago so my memory is a little hazy on every single test variant I tried, so you might want to double check on your end if you want to try downgrading alone.

https://stackoverflow.com/questions/65463893/getting-keystoreexception-and-generalsecurityexception-by-using-encryptedsharedp

This post above references the backup fix. One person in there said alpha01 alone didn't solve their issue, for what it's worth.

jarrodrobins avatar Sep 02 '21 23:09 jarrodrobins

That was my experience with 1.1.0-alpha01 (NOT 02 or 03), yes. It may not be necessary to disable backups, but a number of other posts (eg StackOverflow) referenced that as a possible workaround.

The 'testing' I was doing involved finding a teammate who was able to replicate the issue and having them install a bunch of different test builds changing various bits and pieces as it was not something I could replicate locally. The working build for them involved both 'fixes' but yes, it's possible changing it to alpha01 is enough.

I implemented this workaround on my project a while ago so my memory is a little hazy on every single test variant I tried, so you might want to double check on your end if you want to try downgrading alone.

https://stackoverflow.com/questions/65463893/getting-keystoreexception-and-generalsecurityexception-by-using-encryptedsharedp

This post above references the backup fix. One person in there said alpha01 alone didn't solve their issue, for what it's worth.

The thing is, like I said, it's a very rare occasion, you could go for a long time without having any cases thinking you solved the issue.

Tharkius avatar Sep 02 '21 23:09 Tharkius

While getting it to occur is very rare, once it does, it occurs 100% of the time following that. Like I said, I couldn't get it to occur locally, and relied on a user to help me work around it. All I know is those two fixes stopped that user from getting the crash (after getting them to install at least five other intermediate builds trying different things).

jarrodrobins avatar Sep 03 '21 00:09 jarrodrobins

But if you don't have the backup flags, would it still occur on alpha 02 ?

AndroidDeveloperLB avatar Sep 03 '21 00:09 AndroidDeveloperLB

But if you don't have the backup flags, would it still occur on alpha 02 ?

Yes... I wonder if someone over at Google could give us a hint.

Tharkius avatar Sep 03 '21 00:09 Tharkius

¯_(ツ)_/¯

When I was testing it, alpha02 still caused the crash without the backup flags. But you may want to test it yourself if you're able to replicate it.

Ideally Google would just fix the damn thing, considering it's been a problem for at least a year that I'm aware of.

jarrodrobins avatar Sep 03 '21 00:09 jarrodrobins

¯_(ツ)_/¯

When I was testing it, alpha02 still caused the crash without the backup flags. But you may want to test it yourself if you're able to replicate it.

Ideally Google would just fix the damn thing, considering it's been a problem for at least a year that I'm aware of.

Ideally yeah. I mean, we use their IDE everyday and they have all our data...

Tharkius avatar Sep 03 '21 00:09 Tharkius

While getting it to occur is very rare, once it does, it occurs 100% of the time following that. Like I said, I couldn't get it to occur locally, and relied on a user to help me work around it. All I know is those two fixes stopped that user from getting the crash (after getting them to install at least five other intermediate builds trying different things).

This is sorta expected.

First of all, I apologize for causing so many pains and frustrations. I made a wrong decision trusting Android Keystore which happens to be super unreliable, especially on exotic devices.

Let me explain what I think is happening, why there's little Google can do even though I'd spent days on this, and possible workarounds with caveats.

Background & root cause

The dependency chain is:

Users -> Jetpack Security -> Tink -> Android Keystore -> OEM firmware/hardware.

I work on Tink and contributed to Jetpack Security. I think the root cause of the crashes is neither in Tink nor Jetpack, but in Android Keystore and OEM firmware/hardware.

Shared prefs are encrypted with a Tink keyset (which is a protobuf). The keyset is encrypted with a master key stored in Android Keystore. The encrypted keyset itself is stored as a special value in the encrypted shared prefs file.

We've found that Android Keystore occasionally corrupts the master key on certain devices. We don't know why, we think it could be due to faulty OEM firmware/hardware.

When the master key is corrupted, Tink won't be able to decrypt, and return the error the master key android-keystore://_androidx_security_master_key_ exists but is unusable.

Workaround

There are two ways to recover from a corrupted master key:

1/ If you don't have any existing encrypted prefs, you can delete the master key (using deleteEntry()), and try again.

2/ If you have existing prefs, it's very likely that they will be lost forever. I'm so sorry!

To prevent further crashes, you can:

  • delete the master key, and
  • delete the encrypted prefs, and
  • delete the encrypted Tink keyset.

I'll talk to the Jetpack team to see if we can add a function to do this for you. In the meantime, you have to delete these things by yourself.

For example, if you use the following code:

    private fun getSecuredSharedPreferences(context: Context, fileName: String): SharedPreferences {
        val masterKey = MasterKey.Builder(context, MasterKey.DEFAULT_MASTER_KEY_ALIAS).setKeyScheme(MasterKey.KeyScheme.AES256_GCM).build()
        return EncryptedSharedPreferences.create(context, fileName, masterKey,
                EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV,
                EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM
        )
    }

Then you want to delete the file fileName. If you only regenerate the master key, but don't delete existing data, you will get InvalidProtocolBufferException caused by Jetpack Security/Tink trying to use the new master key to decrypt its keyset.

thaidn avatar Sep 03 '21 01:09 thaidn

Thanks for the really solid explanation @thaidn. I appreciate that this issue is across several teams and is hard for you guys to track down.

I'll give these workarounds a try. Thank you!

jarrodrobins avatar Sep 03 '21 01:09 jarrodrobins

@thaidn As you think that it's a hardware issue, can you please tell Google to add a test to licensing devices (I think it's called CTS), so that at least for new devices, this issue won't occur?

AndroidDeveloperLB avatar Sep 03 '21 10:09 AndroidDeveloperLB

While getting it to occur is very rare, once it does, it occurs 100% of the time following that. Like I said, I couldn't get it to occur locally, and relied on a user to help me work around it. All I know is those two fixes stopped that user from getting the crash (after getting them to install at least five other intermediate builds trying different things).

This is sorta expected.

First of all, I apologize for causing so many pains and frustrations. I made a wrong decision trusting Android Keystore which happens to be super unreliable, especially on exotic devices.

Let me explain what I think is happening, why there's little Google can do even though I'd spent days on this, and possible workarounds with caveats.

Background & root cause

The dependency chain is:

Users -> Jetpack Security -> Tink -> Android Keystore -> OEM firmware/hardware.

I work on Tink and contributed to Jetpack Security. I think the root cause of the crashes is neither in Tink nor Jetpack, but in Android Keystore and OEM firmware/hardware.

Shared prefs are encrypted with a Tink keyset (which is a protobuf). The keyset is encrypted with a master key stored in Android Keystore. The encrypted keyset itself is stored as a special value in the encrypted shared prefs file.

We've found that Android Keystore occasionally corrupts the master key on certain devices. We don't know why, we think it could be due to faulty OEM firmware/hardware.

When the master key is corrupted, Tink won't be able to decrypt, and return the error the master key android-keystore://_androidx_security_master_key_ exists but is unusable.

Workaround

There are two ways to recover from a corrupted master key:

1/ If you don't have any existing encrypted prefs, you can delete the master key (using deleteEntry()), and try again.

2/ If you have existing prefs, it's very likely that they will be lost forever. I'm so sorry!

To prevent further crashes, you can:

  • delete the master key, and
  • delete the encrypted prefs, and
  • delete the encrypted Tink keyset.

I'll talk to the Jetpack team to see if we can add a function to do this for you. In the meantime, you have to delete these things by yourself.

For example, if you use the following code:

    private fun getSecuredSharedPreferences(context: Context, fileName: String): SharedPreferences {
        val masterKey = MasterKey.Builder(context, MasterKey.DEFAULT_MASTER_KEY_ALIAS).setKeyScheme(MasterKey.KeyScheme.AES256_GCM).build()
        return EncryptedSharedPreferences.create(context, fileName, masterKey,
                EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV,
                EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM
        )
    }

Then you want to delete the file fileName. If you only regenerate the master key, but don't delete existing data, you will get InvalidProtocolBufferException caused by Jetpack Security/Tink trying to use the new master key to decrypt its keyset.

Thanks for your time and information. I got a question though: to delete the master key would this code suffice?

try {
    KeyStore keyStore = KeyStore.getInstance(KeyStore.getDefaultType());

    keyStore.deleteEntry(MasterKey.DEFAULT_MASTER_KEY_ALIAS);
} catch (KeyStoreException e) {
    Log.e(TAG, "Error occurred while trying to delete the master key", e);
}

Tharkius avatar Sep 03 '21 16:09 Tharkius

You have to load the Android KeyStore.

KeyStore keyStore = KeyStore.getInstance("AndroidKeyStore");
keyStore.load(/* param= */ null);
keyStore.deleteEntry(MasterKey.DEFAULT_MASTER_KEY_ALIAS);

thaidn avatar Sep 03 '21 18:09 thaidn

You have to load the Android KeyStore.

KeyStore keyStore = KeyStore.getInstance("AndroidKeyStore");
keyStore.load(/* param= */ null);
keyStore.deleteEntry(MasterKey.DEFAULT_MASTER_KEY_ALIAS);

Hmmm. That can also crash 🤔

Oh well, it's our best bet anyway. Thanks for the help again.

Tharkius avatar Sep 03 '21 18:09 Tharkius

You have to load the Android KeyStore.

KeyStore keyStore = KeyStore.getInstance("AndroidKeyStore");
keyStore.load(/* param= */ null);
keyStore.deleteEntry(MasterKey.DEFAULT_MASTER_KEY_ALIAS);

I'm using the following code in an attempt to reset the master key and all (the shared prefs file is also correctly being deleted, according to the log message):

try {
    KeyStore keyStore = KeyStore.getInstance(KEYSTORE_PROVIDER);
    File sharedPrefsFile = new File(
        context.getFilesDir().getParent() + "/shared_prefs/" + SHARED_PREFS_FILENAME + ".xml"
    );

    boolean deleted = sharedPrefsFile.delete();

    Log.d(TAG, "Shared prefs file deleted: %s", deleted);

    keyStore.load(null);
    keyStore.deleteEntry(MasterKey.DEFAULT_MASTER_KEY_ALIAS);
} catch (Exception e) {
    Log.e(TAG, "Error occurred", e);
}

But after running this, the next call to EncryptedSharedPreferences.create() fails with that protobuf exception you mentioned:

com.google.crypto.tink.shaded.protobuf.InvalidProtocolBufferException: Protocol message contained an invalid tag (zero).
        at com.google.crypto.tink.shaded.protobuf.GeneratedMessageLite.parsePartialFrom(GeneratedMessageLite.java:1566)
        at com.google.crypto.tink.shaded.protobuf.GeneratedMessageLite.parseFrom(GeneratedMessageLite.java:1664)
        at com.google.crypto.tink.proto.Keyset.parseFrom(Keyset.java:957)
        at com.google.crypto.tink.integration.android.SharedPrefKeysetReader.read(SharedPrefKeysetReader.java:84)
        at com.google.crypto.tink.CleartextKeysetHandle.read(CleartextKeysetHandle.java:58)
        at com.google.crypto.tink.integration.android.AndroidKeysetManager$Builder.read(AndroidKeysetManager.java:328)
        at com.google.crypto.tink.integration.android.AndroidKeysetManager$Builder.readOrGenerateNewKeyset(AndroidKeysetManager.java:287)
        at com.google.crypto.tink.integration.android.AndroidKeysetManager$Builder.build(AndroidKeysetManager.java:238)
        at androidx.security.crypto.EncryptedSharedPreferences.create(EncryptedSharedPreferences.java:155)
        at androidx.security.crypto.EncryptedSharedPreferences.create(EncryptedSharedPreferences.java:120)

Is there anything else I'm missing here?

Tharkius avatar Sep 06 '21 11:09 Tharkius

Then you want to delete the file fileName. If you only regenerate the master key, but don't delete existing data, you will get InvalidProtocolBufferException caused by Jetpack Security/Tink trying to use the new master key to decrypt its keyset.

I could be wrong @Tharkius but i did something like this and I no longer get that exception.

// delete the master key
KeyStore keyStore = KeyStore.getInstance(KEYSTORE_PROVIDER);
keyStore.load(null)
keyStore.deleteEntry(MasterKey.DEFAULT_MASTER_KEY_ALIAS)

// delete the encrypted prefs
context.getSharedPreferences(FILE_NAME, Context.MODE_PRIVATE).edit().clear().apply()

// create encrypted prefs

syedalattas avatar Sep 07 '21 04:09 syedalattas

Doesn't deleting the data ruin how the app might work, though ? I mean, if you expect to see some content there, now it will be gone, no?

AndroidDeveloperLB avatar Sep 07 '21 06:09 AndroidDeveloperLB

Doesn't deleting the data ruin how the app might work, though ? I mean, if you expect to see some content there, now it will be gone, no?

There's not much point in keeping data you can't decrypt anymore...

Tharkius avatar Sep 07 '21 09:09 Tharkius

So it's not a workaround to still use what you had before. It's resetting of the data you've saved, just to avoid a crash, no?

Suppose this is the code I use:

    private fun getSecuredSharedPreferences(context: Context, fileName: String): SharedPreferences {
        val masterKey = MasterKey.Builder(context, MasterKey.DEFAULT_MASTER_KEY_ALIAS).setKeyScheme(MasterKey.KeyScheme.AES256_GCM).build()
        return EncryptedSharedPreferences.create(context, fileName, masterKey,
                EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV,
                EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM
        )
    }

What exactly should I change here for this workaround?

AndroidDeveloperLB avatar Sep 07 '21 12:09 AndroidDeveloperLB

Then you want to delete the file fileName. If you only regenerate the master key, but don't delete existing data, you will get InvalidProtocolBufferException caused by Jetpack Security/Tink trying to use the new master key to decrypt its keyset.

I could be wrong @Tharkius but i did something like this and I no longer get that exception.

// delete the master key
KeyStore keyStore = KeyStore.getInstance(KEYSTORE_PROVIDER);
keyStore.load(null)
keyStore.deleteEntry(MasterKey.DEFAULT_MASTER_KEY_ALIAS)

// delete the encrypted prefs
context.getSharedPreferences(FILE_NAME, Context.MODE_PRIVATE).edit().clear().apply()

// create encrypted prefs

Thanks, but it's still not working. Even when I call clear(), the file still contains two entries "androidx_security_crypto_encrypted_prefs_key_keyset" and "androidx_security_crypto_encrypted_prefs_value_keyset", so that's prolly why I'm still getting the InvalidProtocolBufferException. And when I try to delete file, despite the method returning true, the file never gets deleted, as it is still accessible in the device explorer.

@thaidn What could be going wrong in my approach?

Tharkius avatar Sep 07 '21 12:09 Tharkius

So it's not a workaround to still use what you had before. It's resetting of the data you've saved, just to avoid a crash, no?

Suppose this is the code I use:

    private fun getSecuredSharedPreferences(context: Context, fileName: String): SharedPreferences {
        val masterKey = MasterKey.Builder(context, MasterKey.DEFAULT_MASTER_KEY_ALIAS).setKeyScheme(MasterKey.KeyScheme.AES256_GCM).build()
        return EncryptedSharedPreferences.create(context, fileName, masterKey,
                EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV,
                EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM
        )
    }

What exactly should I change here for this workaround?

No, you only need to reset the data when you catch the GeneralSecurityException. As for how to do it, that's what we're discussing.

Tharkius avatar Sep 07 '21 12:09 Tharkius

But you guys say you've tested it , no?

Also, does it handle all the exceptions I've described in the bug report?

AndroidDeveloperLB avatar Sep 07 '21 12:09 AndroidDeveloperLB

But you guys say you've tested it , no?

Also, does it handle all the exceptions I've described in the bug report?

We have to wait for Google's reply now

Tharkius avatar Sep 07 '21 13:09 Tharkius

Can you please share a sample POC that does what you suggest, using the code of getSecuredSharedPreferences that I've used ?

AndroidDeveloperLB avatar Sep 07 '21 16:09 AndroidDeveloperLB

Can you please share a sample POC that does what you suggest, using the code of getSecuredSharedPreferences that I've used ?

Like I told you before, we're still discussing the best solution... You need to wait for Google's reply.

Tharkius avatar Sep 07 '21 16:09 Tharkius

OK please let me know too, and then share your POC, ok?

AndroidDeveloperLB avatar Sep 07 '21 16:09 AndroidDeveloperLB

@thaidn What could be going wrong in my approach?

I think you have to manually delete the file. Just clearing the shared prefs doesn't work because it's an encrypted prefs that will always retain androidx_security_crypto_encrypted_prefs_key_keyset and androidx_security_crypto_encrypted_prefs_value_keyset.

Edit: hmm now I'm not sure why it doesn't work. You weren't using EncryptedSharedPreferences, but the normal shared pref API, so it should have worked well.

Maybe there's another process/thread that re-generated the shared pref file? What happens if you change the filename?

thaidn avatar Sep 07 '21 22:09 thaidn

fwiw, I've completely switched away from tink and directly use the jetpack and android apis. And it seems to solve a lot of issues. It makes debugging things much easier. Simply following https://medium.com/androiddevelopers/using-biometricprompt-with-cryptoobject-how-and-why-aace500ccdb7 gets you 99% of the way. Somehow every project using protobuf breaks on me.

Maybe worth giving it a shot before debugging through four layers of dependencies where each bug report refers to the other ones 😅

hpoul avatar Sep 08 '21 05:09 hpoul

@thaidn What could be going wrong in my approach?

I think you have to manually delete the file. Just clearing the shared prefs doesn't work because it's an encrypted prefs that will always retain androidx_security_crypto_encrypted_prefs_key_keyset and androidx_security_crypto_encrypted_prefs_value_keyset.

Edit: hmm now I'm not sure why it doesn't work. You weren't using EncryptedSharedPreferences, but the normal shared pref API, so it should have worked well.

Maybe there's another process/thread that re-generated the shared pref file? What happens if you change the filename?

Ok, to be honest, I'm not sure why the file doesn't get deleted in the app I'm working on, because it surely does in a demo app I made. Anyway, I fixed it by clearing the contents of the file with a FileWriter instead of deleting. Well, it works...

Tharkius avatar Sep 08 '21 17:09 Tharkius

OK please let me know too, and then share your POC, ok?

Here you go, this should be what you're looking for:

package com.example.tests

import android.app.AlertDialog
import android.content.DialogInterface
import android.content.Intent
import android.content.SharedPreferences
import android.os.Bundle
import android.util.Log
import androidx.appcompat.app.AppCompatActivity
import androidx.security.crypto.EncryptedSharedPreferences
import androidx.security.crypto.MasterKey
import java.io.File
import java.security.GeneralSecurityException
import java.security.KeyStore

private const val KEYSTORE_PROVIDER = "AndroidKeyStore"
private const val SHARED_PREFS_FILENAME = "keys";
private const val TAG = "MainActivity"

class MainActivity : AppCompatActivity() {
    private lateinit var sharedPreferences: SharedPreferences

    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)

        setContentView(R.layout.activity_main)

        try {
            createSharedPreferences()
        } catch (e: Exception) {
            Log.e(TAG, "Error occurred", e)

            displayErrorAlert()
        }
    }

    private fun createSharedPreferences() {
        val masterKey = MasterKey.Builder(this)
            .setKeyScheme(MasterKey.KeyScheme.AES256_GCM)
            .setUserAuthenticationRequired(false)
            .build()

        sharedPreferences = EncryptedSharedPreferences.create(
            this, SHARED_PREFS_FILENAME, masterKey,
            EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV,
            EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM
        )

        // The purpose of the code below is to simulate a crash that happens only on the first run
        val sharedPrefs: SharedPreferences = getSharedPreferences("prefs", MODE_PRIVATE)
        val num = sharedPrefs.getInt("num", 0)

        sharedPrefs.edit()
            .putInt("num", 1)
            .commit()

        if (num == 0) {
            throw GeneralSecurityException("Something went wrong...")
        }
    }

    private fun deleteSharedPreferences() {
        try {
            val sharedPrefsFile = File(
                getFilesDir().getParent() + "/shared_prefs/" + SHARED_PREFS_FILENAME + ".xml"
            )
            
            sharedPreferences.edit().clear().commit()
            
            if (sharedPrefsFile.exists()) {
                val deleted = sharedPrefsFile.delete()
                Log.d(TAG, "resetStorage() Shared prefs file deleted: $deleted; path: ${sharedPrefsFile.absolutePath}")
            } else {
                Log.d(TAG,"resetStorage() Shared prefs file non-existent; path: ${sharedPrefsFile.absolutePath}")
            }

            val keyStore = KeyStore.getInstance(KEYSTORE_PROVIDER)

            keyStore.load(null)
            keyStore.deleteEntry(MasterKey.DEFAULT_MASTER_KEY_ALIAS)
        } catch (e: Exception) {
            Log.e(TAG, "Error occurred while trying to reset shared prefs", e)
        }
    }

    private fun displayErrorAlert() {
        val alertDialog = AlertDialog.Builder(this).create()

        alertDialog.setTitle("We are truly sorry...")
        alertDialog.setMessage("Something went wrong, we will have to delete all your data. App will now close :(")

        alertDialog.setButton(
            DialogInterface.BUTTON_POSITIVE, "Ok"
        ) { dialog, which ->
            deleteSharedPreferences()
            restartApp()
        }

        alertDialog.show()
    }

    private fun restartApp() {
        val intent = Intent(this, MainActivity::class.java)
        intent.flags = Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_CLEAR_TASK

        startActivity(intent)
        finish()

        Runtime.getRuntime().exit(0)
    }
}

Tharkius avatar Sep 08 '21 17:09 Tharkius

I might had a bit different crash than you, some of crashes on my side was mainly on Samsung devices

Caused by: java.security.KeyStoreException: the master key android-keystore://key_alias exists but is unusable
  at com.google.crypto.tink.integration.android.AndroidKeysetManager$Builder.readOrGenerateNewMasterKey(AndroidKeysetManager.java:275)
  at com.google.crypto.tink.integration.android.AndroidKeysetManager$Builder.build(AndroidKeysetManager.java:236)
  at androidx.security.crypto.EncryptedSharedPreferences.create(EncryptedSharedPreferences.java:160)
  at androidx.security.crypto.EncryptedSharedPreferences.create(EncryptedSharedPreferences.java:120)
  ...
Caused by: javax.crypto.IllegalBlockSizeException
  at android.security.keystore2.AndroidKeyStoreCipherSpiBase.engineDoFinal(AndroidKeyStoreCipherSpiBase.java:613)
  at javax.crypto.Cipher.doFinal(Cipher.java:2113)
  at com.google.crypto.tink.integration.android.AndroidKeystoreAesGcm.decryptInternal(AndroidKeystoreAesGcm.java:115)
  at com.google.crypto.tink.integration.android.AndroidKeystoreAesGcm.decrypt(AndroidKeystoreAesGcm.java:101)
  at com.google.crypto.tink.integration.android.AndroidKeystoreKmsClient.validateAead(AndroidKeystoreKmsClient.java:249)
  at com.google.crypto.tink.integration.android.AndroidKeystoreKmsClient.getAead(AndroidKeystoreKmsClient.java:165)
  at com.google.crypto.tink.integration.android.AndroidKeysetManager$Builder.readOrGenerateNewMasterKey(AndroidKeysetManager.java:267)
  ...
Caused by: android.security.KeyStoreException: Invalid operation handle
  at android.security.KeyStore2.getKeyStoreException(KeyStore2.java:408)
  at android.security.KeyStoreOperation.handleExceptions(KeyStoreOperation.java:78)
  at android.security.KeyStoreOperation.finish(KeyStoreOperation.java:127)
  at android.security.keystore2.KeyStoreCryptoOperationChunkedStreamer$MainDataStream.finish(KeyStoreCryptoOperationChunkedStreamer.java:228)
  at android.security.keystore2.KeyStoreCryptoOperationChunkedStreamer.doFinal(KeyStoreCryptoOperationChunkedStreamer.java:181)
  at android.security.keystore2.AndroidKeyStoreAuthenticatedAESCipherSpi$BufferAllOutputUntilDoFinalStreamer.doFinal(AndroidKeyStoreAuthenticatedAESCipherSpi.java:396)
  at android.security.keystore2.AndroidKeyStoreCipherSpiBase.engineDoFinal(AndroidKeyStoreCipherSpiBase.java:603)
  ...

and the solution seems to be very simple: synchronize every creation of encrypted shared prefs.

Cause: As mentioned here the problem lays in samsung implementation of Cipher which is not thread safe.

I hope this will help some of you. And remember, if you use the same key to cipher anywhere else, you may need to synchronize those too.

v-mas avatar Feb 03 '22 08:02 v-mas

@v-mas I already use "synchronized" : https://issuetracker.google.com/issues/176215143#comment15

The reason is that everything related to this API could take time, so I didn't want this one to take time too.

AndroidDeveloperLB avatar Feb 03 '22 08:02 AndroidDeveloperLB