Tiqr / tiqr

Obsolete Github repo for the tiqr.org project. Note that the repository is split into several individual repos, all with a tiqr- prefix
35 stars 16 forks source link

silent failure on master key corruption #33

Open SyntaxPolice opened 11 years ago

SyntaxPolice commented 11 years ago

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:

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