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

Commit

Permalink
Return a better error when a keyset gets decrypted with the wrong mas…
Browse files Browse the repository at this point in the history
…ter key.

We also need to better handle when the master key is missing, but for that we first need to refactor the code a bit.

PiperOrigin-RevId: 503393910
  • Loading branch information
juergw authored and Copybara-Service committed Jan 20, 2023
1 parent 311ec9f commit 592c2eb
Showing 1 changed file with 11 additions and 3 deletions.
Expand Up @@ -326,12 +326,13 @@ private KeysetManager readOrGenerateNewKeyset() throws GeneralSecurityException,
return manager;
}

@SuppressWarnings("UnusedException")
private KeysetManager read() throws GeneralSecurityException, IOException {
if (masterKey != null) {
try {
return KeysetManager.withKeysetHandle(KeysetHandle.read(reader, masterKey));
} catch (InvalidProtocolBufferException | GeneralSecurityException ex) {
// Swallow the exception and attempt to read the keyset in cleartext.
// Attempt to read the keyset in cleartext.
// This edge case may happen when either
// - the keyset was generated on a pre M phone which is then upgraded to M or newer, or
// - the keyset was generated with Keystore being disabled, then Keystore is enabled.
Expand All @@ -340,9 +341,16 @@ private KeysetManager read() throws GeneralSecurityException, IOException {
// cleartext value that it controls. This does not introduce new security risks because to
// overwrite the encrypted keyset in private preferences of an app, said adversaries must
// have the same privilege as the app, thus they can call Android Keystore to read or
// write
// the encrypted keyset in the first place.
// write the encrypted keyset in the first place.
Log.w(TAG, "cannot decrypt keyset: ", ex);
try {
return KeysetManager.withKeysetHandle(CleartextKeysetHandle.read(reader));
} catch (InvalidProtocolBufferException ex2) {
// Raising a InvalidProtocolBufferException error here would be confusing, because
// parsing probably failed because the keyset was encrypted but we were not able to
// decrypt it. It is better to throw the error above.
throw ex;
}
}
}

Expand Down

0 comments on commit 592c2eb

Please sign in to comment.