tink-crypto / tink

Tink is a multi-language, cross-platform, open source library that provides cryptographic APIs that are secure, easy to use correctly, and hard(er) to misuse.
https://developers.google.com/tink
Apache License 2.0
13.5k stars 1.18k forks source link

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

Closed AndroidDeveloperLB closed 9 months ago

AndroidDeveloperLB commented 3 years ago

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

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

oswaldo89 commented 3 years ago

any comment about this ?

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

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

thaidn commented 3 years ago

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

AndroidDeveloperLB commented 3 years ago

@thaidn Check the original post. It has enough.

oswaldo89 commented 3 years ago

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

@thaidn

Fatal Exception: java.security.KeyStoreException: the master key android-keystore://_androidx_security_masterkey 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 commented 3 years ago

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

image

Tharkius commented 3 years ago

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.

jarrodrobins commented 3 years ago

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.

AndroidDeveloperLB commented 3 years ago

@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?

thaidn commented 3 years ago

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?

Tharkius commented 3 years ago

@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...

jarrodrobins commented 3 years ago

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.

Tharkius commented 3 years ago

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.

jarrodrobins commented 3 years ago

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).

AndroidDeveloperLB commented 3 years ago

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

Tharkius commented 3 years ago

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.

jarrodrobins commented 3 years ago

¯_(ツ)_/¯

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.

Tharkius commented 3 years ago

¯(ツ)

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...

thaidn commented 3 years ago

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:

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.

jarrodrobins commented 3 years ago

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!

AndroidDeveloperLB commented 3 years ago

@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?

Tharkius commented 3 years ago

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);
}
thaidn commented 3 years ago

You have to load the Android KeyStore.

KeyStore keyStore = KeyStore.getInstance("AndroidKeyStore");
keyStore.load(/* param= */ null);
keyStore.deleteEntry(MasterKey.DEFAULT_MASTER_KEY_ALIAS);
Tharkius commented 3 years ago

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 commented 3 years ago

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?

syedalattas commented 3 years ago

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
AndroidDeveloperLB commented 3 years ago

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?

Tharkius commented 3 years ago

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...

AndroidDeveloperLB commented 3 years ago

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?

Tharkius commented 3 years ago

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 commented 3 years ago

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.

AndroidDeveloperLB commented 3 years ago

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

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

Tharkius commented 3 years ago

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

AndroidDeveloperLB commented 3 years ago

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

Tharkius commented 3 years ago

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.

AndroidDeveloperLB commented 3 years ago

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

thaidn commented 3 years ago

@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?

hpoul commented 3 years ago

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 😅

Tharkius commented 3 years ago

@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 commented 3 years ago

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)
    }
}
v-mas commented 2 years ago

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.

AndroidDeveloperLB commented 2 years ago

@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.

victorlapin commented 2 years ago

Just as a heads-up, does the issue still happen if you change master key name from default to something specific to your app?

All provided examples were using MasterKey.DEFAULT_MASTER_KEY_ALIAS

AndroidDeveloperLB commented 2 years ago

@victorlapin

  1. What are you suggesting, exactly? Please show some code, and also explain if it's a valid thing to try now, if it's possible to migrate to it for existing users too.
  2. Please explain what's wrong with using DEFAULT_MASTER_KEY_ALIAS.
  3. Have you tried it on an app that has many users? Have you used something else?
victorlapin commented 2 years ago

I'm by no means a pro in this matter, but:

  1. My point is to use separate master keys for every app with encrypted sharedpreferences, to exclude racing conditions/different key owners/etc.
  2. Even if p.1 proves correct, I don't see a way to migrate preferences without recreating them.

I'm suggesting something like this, for testing:

private val KEY_ALIAS = "${context.packageName}_masterkey" private val masterKeyAlias by lazy { MasterKey.Builder(context, KEY_ALIAS) .setKeyScheme(MasterKey.KeyScheme.AES256_GCM) .build() }

  1. I've encountered this issue some time before, and I've stopped using the library. Now I'm considering it again.
Tharkius commented 2 years ago

Just as a heads-up, does the issue still happen if you change master key name from default to something specific to your app?

All provided examples were using MasterKey.DEFAULT_MASTER_KEY_ALIAS

I don't think it makes a difference but you could try it out.

Just like @thaidn mentioned above (repo owner), Android keystore cannot be trusted to work flawlessly across every device. So when this crash happens, there's not much to be done about it except to delete the master key, shared prefs and start over.

I shared some code which simulates a crash and recovers from the situation by catching it, deleting master key and prefs and then restarting the app. Check my last comment here before this one.

AndroidDeveloperLB commented 2 years ago

@victorlapin I don't think changing the file name would help here. Also there are multiple kinds of exceptions related to this, and perhaps they occur even before creating the file itself.

amirulzin commented 2 years ago

@Tharkius

The pref deletion issue stems from this:

 val sharedPrefsFile = File(
        getFilesDir().getParent() + "/shared_prefs/" + SHARED_PREFS_FILENAME + ".xml"
)

Which essentially translate to ../your-app-package/files/shared_prefs/somePref.xml

shared_pref folder is normally located directly under your-app-package e.g. ../your-app-package/shared_prefs/somePref.xml

Which is why @syedalattas answer using just the default SharedPreference API seems to works fine. Though for multi threaded environment, do use commit() instead of apply()


To save the time for future readers who eventually ends up here, I suppose this is the destructive workaround as of now:

implementation "androidx.security:security-crypto-ktx:1.1.0-alpha03"

  fun createEncryptedSharedPrefDestructively(fileName: String, context: Context): SharedPreferences {
    return try {
      createEncryptedSharedPref(fileName, context)
    } catch (e: GeneralSecurityException) {
      deleteMasterKeyEntry()
      deleteExistingPref(fileName, context)
      createEncryptedSharedPref(fileName, context)
    }
  }

  @SuppressLint("ApplySharedPref")
  private fun deleteExistingPref(fileName: String, context: Context) {
    if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) {
      context.deleteSharedPreferences(fileName)
    } else {
      context.getSharedPreferences(fileName, Context.MODE_PRIVATE)
        .edit()
        .clear()
        .commit()
    }
  }

  private fun deleteMasterKeyEntry() {
    KeyStore.getInstance("AndroidKeyStore").apply {
      load(null)
      deleteEntry(MasterKey.DEFAULT_MASTER_KEY_ALIAS)
    }
  }

  private fun createEncryptedSharedPref(fileName: String, context: Context): 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
    )
  } 

AOSP source for context.deleteSharedPreferences(fileName)

darrentaft commented 2 years ago

Having implemented the destructive workaround suggested above, I just want to add that the deleteMasterKeyEntry method can cause a runtime exception triggered by the following:

Caused by java.security.KeyStoreException: Failed to delete entry: _androidx_security_master_key_
       at android.security.keystore.AndroidKeyStoreSpi.engineDeleteEntry(AndroidKeyStoreSpi.java:863)
       at java.security.KeyStore.deleteEntry(KeyStore.java:1257)
       at com.foo.bar.util.EncryptedData.deleteMasterKeyEntry(EncryptedData.kt:66)
       at com.foo.bar.util.EncryptedData.createEncryptedSharedPrefDestructively(EncryptedData.kt:45)
       at com.foo.bar.util.EncryptedData.initialise(EncryptedData.kt:19)
       at com.foo.bar.MainActivity.onCreate(MainActivity.java:103)
Tharkius commented 2 years ago

@darrentaft

Yap, that's all the more reason not to consider using EncryptedSharedPreferences. I vehemently recommend everyone to seek other alternatives, cause this API will only bring you trouble.

AndroidDeveloperLB commented 2 years ago

@darrentaft I wrote about this here: https://issuetracker.google.com/issues/176215143#comment73