Skip to content
This repository has been archived by the owner on Jul 2, 2022. It is now read-only.

silent failure on master key corruption #33

Open
SyntaxPolice opened this issue Jul 30, 2013 · 0 comments
Open

silent failure on master key corruption #33

SyntaxPolice opened this issue Jul 30, 2013 · 0 comments

Comments

@SyntaxPolice
Copy link
Contributor

It's possible to encountered this exception, which I believe indicates that the master device key is incorrect:

java.io.IOException: KeyStore integrity check failed.
at com.android.org.bouncycastle.jce.provider.JDKKeyStore.engineLoad(JDKKeyStore.java:873)

This exception is critical; the keystore appears to be encrypted with a different device key at this point, and so all user keys appear to be unrecoverable. I don't know what caused this, and I'm working to debug it. It could be a few things:

  • I'm using a forked version of Tiqr that crashes sometimes, and potentially this key was corrupted during a crash.
  • My OS upgraded from 4.2 to 4.3. This is the only obvious change since the system worked.
  • I'm sure there are other potential causes...

Therefore, I'm not sure if this is a Tiqr bug, but there are a few minor issues in Tiqr that make this worse:

  1. When fetching a "device key" the org.tiqr.authenticator.security.Encryption. getDeviceKey function tests whether the device key exists and if it doesn't, generates a new one. However, this code is a bit problematic in the case where the device key has been corrupted, deleted, etc. That is, if the keystore is already encrypted with another key, this will silently generate a new key. Perhaps consider giving an error to the user if there are already keys on the device and you find yourself without a key to decrypt them.
  2. The function org.tiqr.authenticator.security.SecretStore._initializeKeyStore swallows all exceptions after printing a stack trace. During enrollment, this results in a silent failure where it appears that the user is enrolled, but the key was not stored. During login, this results in a null pointer exception and an app crash.

From a security perspective, I'm curious about the reason for this encryption. The user keys get stored next to the device key; if someone is able to steal the encrypted user keys, they can also presumably steal the device key, so it doesn't deter an attacker. However, if the user loses the legitimate device key (for the above reasons or for others), they lose access to all of their accounts. Seems like you are sacrificing "availability" for "confidentiality", but the confidentiality argument is unclear to me.

peace,

isaac

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

No branches or pull requests

1 participant