GeneralSecurityException: decryption failed

[zsweigart]

GeneralSecurityException: decryption failed
  File "AeadWrapper.java", line 82, in decrypt
  File "TinkHelper.kt", line 33, in decrypt

This error occurs with version 1.3.0-rc1 on an Android device running 9.0 but is fixed in versino 1.3.0-rc4. We can't upgrade to 1.3.0-rc4 because of #301.

Will the 1.3.0 release contain the fix for the decryption error but not the issue with firebase?

[thaidn]

Could you please provide more information? For example, why do you think this is fixed in 1.3.0-rc4?

Will the 1.3.0 release contain the fix for the decryption error but not the issue with firebase?

Yes. 1.3.0 cannot be used with Firebase, I'm so sorry about this, but there's little we can do at the moment.

I hope that we can make 1.4.0 work with Firebase.

[amytang0]

Hi, is there an updated timeline on 1.4.0? The roadmap says Feb 2019 (https://github.com/google/tink/blob/master/docs/ROADMAP.md). We are possibly going to roll our own branch with Firebase-compatible protobufs since this is blocking us

[thaidn]

We're working very hard on 1.4.0. The main blocker is Tink for Python, but it's getting ready. We think it should be ready by end of April.

Note that it's very unlikely that we will be able to resolve the protobuf issue with 1.4.0.

[amytang0]

Thanks for the update!

[simtse]

Yes. 1.3.0 cannot be used with Firebase, I'm so sorry about this, but there's little we can do at the moment.

We've been using 1.3.0 and we have firebase-core and firebase-messaging and generally, users are able to encrypt and decrypt. But we have a significant amount of users that also get GeneralSecurityException. @thaidn can you elaborate more on how 1.3.0 tink doesn't work with firbase?

@amytang0 what modifications did you have to have Tink become Firebase-compatible?

[thaidn]

@simtse #301 is being fixed. Could you kindly upgrade to HEAD-SNAPSHOT to try it out?

[simtse]

Thanks @thaidn Would you be able to elaborate on the commits and causes for the fixes? I'm puzzled specifically about this and the Protobuf errors. Seemed like a series of fixes if addressing both the Protobuf and decryption problems, but I don't know for sure why.

[simtse]

So I'm still getting this GeneralSecurityException: decryption failed that I originally thought was my own code. But now, it seems that AndroidX's EncryptedSharedPrefences also runs into this problem when I'm just saving (encrypting) and fetching (decrypting) Strings.

Caused by: java.lang.SecurityException: Could not decrypt value. decryption failed
	at androidx.security.crypto.EncryptedSharedPreferences.getDecryptedObject(EncryptedSharedPreferences.java:33)
	at androidx.security.crypto.EncryptedSharedPreferences.getString(EncryptedSharedPreferences.java:1)
	... 20 more
Caused by: java.security.GeneralSecurityException: decryption failed
	at com.google.crypto.tink.aead.AeadWrapper$WrappedAead.decrypt(AeadWrapper.java:15)
	at androidx.security.crypto.EncryptedSharedPreferences.getDecryptedObject(EncryptedSharedPreferences.java:5)
	... 21 more

I'm currently using Tink 1.4.0 rc2, and AndroidX security 1.0.0 rc02 which uses Tink 1.4.0 rc2 underneath.

I don't think there is anything special about what I am storing. For example, I am storing strings that look like this

qwerty-abcd3986858483-efgh38751234-jklm23452345-tuvw92850103

The error rate I'm seeing is around 0.3% of users which isn't a small number of users. And this happens when using Tink with the Android keystore

However, if we were to use Tink without the Android Keystore, I barely get any errors in decryption errors. Error rate for that is 0.0001%.

So I'm wondering if this is related to @thaidn your other comment where I first asked

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?

And you @thaidn responded

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

to which I responded

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]

@simtse could you please provide a stack trace?

Edit: ah sorry, saw your stack trace. I'm investigating and will provide a more meaningful update later.

[simtse]

Not much difference with the AndroidX stacktrace posted above (I think that is what you want)

Original  exception is private in our repo
 ...
Caused by: java.security.GeneralSecurityException: decryption failed
	at com.google.crypto.tink.aead.AeadWrapper$WrappedAead.decrypt(AeadWrapper.java:15)
	at com.example.security.TinkCryptographer.decrypt(TinkCryptographer.kt:10)
	at com.example.security.TokenDecrypter.decrypt(TokenDecrypter.kt:7)
	... 21 more

We don't see the logs from AndroidKeysetManager like the following because we haven't run into this issue locally.:

// So it's okay to ignore the failure and try to read the keyset in cleartext.
Log.i(TAG, "cannot decrypt keyset: " + e);

We haven't been able to reproduce this in debug and locally though we've seen the issue affect more than 19,000 users over the last 3 months in production.

Can you clarify which stacktrace you're looking for?

[simtse]

Sorry @thaidn, I think I'm taking you on a confusing conversation in two issues. #339 But I think what you're saying there with the Android keystore is a culprit to this that I can't put my finger on for sure because we haven't found how to trigger this unreliable behavior.

[thaidn]

Have you observed any logging statements from AndroidKeysetManager on these devices?

Edit: oh you said you haven't seen any log. That's strange. I still don't understand what happened.

[simtse]

😞 No we haven't observed or seen the logging statements because we can't reproduce locally that represents what's happening to our users in production. We also won't see the info logs from our production users in our reports because the AndroidKeysetManager is using the base Log.i() method isn't captured in our log files that we send when we capture feedback

[simtse]

That's strange. I still don't understand what happened.

A theory is that the AndroidKeysetManager failed to read the keyset and decrypt it with Android keystore, then because of then, it would either create a new encrypted keyset or use the current keyset (that couldn't be decrypted) as a clear text keyset.

Then since that keyset is not matched to the encrypted cipher text, it fails to decrypt. I've had some tests that had

1. Encrypted a known hard-coded string and saved them several months ago
2. Check if I can decrypt them on a regular basis.
3. When the another cipher text started to fail to decrypt, these hard coded strings that were encrypted and stored would also fail to decrypt.

[eygraber]

I've been having the same problem with androidx security RC 2 (see here and here). I'm at 10k crashes in the past 30 days.

[thaidn]

Thank you all for your reports and comments.

Here's what we're doing to do:

1/ Tink will disable Android Keystore by default. Users can enable it by setting a master key URI, but Tink will run a self-test and disable it if it detects any issues. We recommend keeping Android Keystore disabled unless you have a very strong reason to depend on it.

2/ Tink will only generate a new keyset or a new master key if there's no existing key material. Currently Tink attempts to generate a new keyset or a new master key whenever it can't read the existing key material. This behavior is useful in certain cases, but it hides the actual problems in Android Keystore from Tink users.

[eygraber]

Will clients already affected by the AndroidKeystore issue be able to recover when this new idea is implemented?

[thaidn]

@eygraber Unfortunately I'm not sure. If you had some data encrypted with a keyset that is in turn wrapped by a master key in Android Keystore, you won't be able to recover the data if the master key is lost :-(. I'm really sorry that Android Keystore is so unreliable. I regret the decision to use it in Tink in the first place.

The new change will only tell you exactly what happened, as Tink shouldn't hide the error anymore. The best way to move forward on devices that are having problems is to disable Android Keystore, by not setting a master key URI. Again, existing data will be lost but new data should be safe.

If you really have to use Android Keystore, you can still do that by setting a master key URI. Tink always does a self-test and disables it if necessary. Though on devices Android Keystore transitions from working to not working, data during that period will be lost.

Edit: add the last paragraph.

[simtse]

Thanks @thaidn I'll bring this back to my team and I'll need to internalize what this means for us and our users. But sounds like if we set a master key, then Tink will try to continue to use the Android keystore. But if not, then it will use Tink with the keyset saved in SharedPreferences as clear text.

2/ Tink will only generate a new keyset or a new master key if there's no existing key material. Currently Tink attempts to generate a new keyset or a new master key whenever it can't read the existing key material. This behavior is useful in certain cases, but it hides the actual problems in Android Keystore from Tink users.

I think this was happening to me because it coudln't read the key material, but there wasn't any extra feedback or callback to notify my code that this regeneration happened. Are you suggesting that there will be an API change to notify the callers that this regeneration or what happened underneath so the calling code can make adjustments?

[thaidn]

https://github.com/google/tink/releases/tag/v1.4.0 was released a few minutes ago. It includes several, backward-compatible changes to the Android Keystore integration. Please consult the release notes.

I think this was happening to me because it coudln't read the key material, but there wasn't any extra feedback or callback to notify my code that this regeneration happened. Are you suggesting that there will be an API change to notify the callers that this regeneration or what happened underneath so the calling code can make adjustments?

As explained in the Javadoc:

• If the key material doesn't exist, and you tell Tink to generate a new one (by specifying a key template), Tink will generate one. If the key material does exist, but is corrupt, Tink will throw InvalidKeyException.

• If the master key in Android Keystore doesn't exist, and you tell Tink to generate a new one (by specifying a master key URI), Tink will generate a new one. If the master key does exist, but is unusable, Tink will throw KeyStoreException.

Please comment if you have any other questions.