work on passphrase caching, make use of cached SecretKeyType data (WIP!)

5 files changed, 70 insertions, 86 deletions

diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpDecryptVerify.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpDecryptVerify.java
index 11e986562..d7eb30f16 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpDecryptVerify.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpDecryptVerify.java
@@ -261,6 +261,8 @@ public class PgpDecryptVerify {
 
                 PGPPublicKeyEncryptedData encData = (PGPPublicKeyEncryptedData) obj;
 
+                long subKeyId = encData.getKeyID();
+
                 CanonicalizedSecretKeyRing secretKeyRing;
                 try {
                     // get actual keyring object based on master key id
@@ -276,22 +278,20 @@ public class PgpDecryptVerify {
                     continue;
                 }
                 // get subkey which has been used for this encryption packet
-                secretEncryptionKey = secretKeyRing.getSecretKey(encData.getKeyID());
+                secretEncryptionKey = secretKeyRing.getSecretKey(subKeyId);
                 if (secretEncryptionKey == null) {
                     // continue with the next packet in the while loop
                     continue;
                 }
 
-                /* secret key exists in database! */
-                long masterKeyId = secretEncryptionKey.getRing().getMasterKeyId();
-
                 // allow only specific keys for decryption?
                 if (mAllowedKeyIds != null) {
-                    Log.d(Constants.TAG, "encData.getKeyID(): " + encData.getKeyID());
+                    Log.d(Constants.TAG, "encData.getKeyID(): " + subKeyId);
                     Log.d(Constants.TAG, "mAllowedKeyIds: " + mAllowedKeyIds);
-                    Log.d(Constants.TAG, "masterKeyId: " + masterKeyId);
+                    Log.d(Constants.TAG, "masterKeyId: "
+                            + secretEncryptionKey.getRing().getMasterKeyId());
 
-                    if (!mAllowedKeyIds.contains(masterKeyId)) {
+                    if (!mAllowedKeyIds.contains(subKeyId)) {
                         // this key is in our db, but NOT allowed!
                         // continue with the next packet in the while loop
                         continue;
@@ -306,12 +306,12 @@ public class PgpDecryptVerify {
                 // if no passphrase was explicitly set try to get it from the cache service
                 if (mPassphrase == null) {
                     // returns "" if key has no passphrase
-                    mPassphrase = mPassphraseCache.getCachedPassphrase(masterKeyId);
+                    mPassphrase = mPassphraseCache.getCachedPassphrase(subKeyId);
 
                     // if passphrase was not cached, return here
                     // indicating that a passphrase is missing!
                     if (mPassphrase == null) {
-                        result.setKeyIdPassphraseNeeded(masterKeyId);
+                        result.setKeyIdPassphraseNeeded(subKeyId);
                         result.setStatus(PgpDecryptVerifyResult.KEY_PASSHRASE_NEEDED);
                         return result;
                     }
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/CachedPublicKeyRing.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/CachedPublicKeyRing.java
index 18b02156d..cfb4a915e 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/CachedPublicKeyRing.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/CachedPublicKeyRing.java
@@ -27,6 +27,7 @@ import org.sufficientlysecure.keychain.pgp.KeyRing;
 import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException;
 import org.sufficientlysecure.keychain.provider.KeychainContract.KeyRings;
 import org.sufficientlysecure.keychain.provider.KeychainContract.Keys;
+import org.sufficientlysecure.keychain.provider.ProviderHelper.NotFoundException;
 import org.sufficientlysecure.keychain.util.Log;
 
 /** This implementation of KeyRing provides a cached view of PublicKeyRing
@@ -226,16 +227,12 @@ public class CachedPublicKeyRing extends KeyRing {
         return mProviderHelper.getContentResolver().query(keysUri, null, null, null, null);
     }
 
-    public SecretKeyType getSecretKeyType(long keyId) throws PgpGeneralException {
-        try {
-            Object data = mProviderHelper.getGenericData(Keys.buildKeysUri(mUri),
-                    KeyRings.HAS_SECRET,
-                    ProviderHelper.FIELD_TYPE_INTEGER,
-                    KeyRings.KEY_ID + " = " + Long.toString(keyId));
-            return SecretKeyType.fromNum(((Long) data).intValue());
-        } catch(ProviderHelper.NotFoundException e) {
-            throw new PgpGeneralException(e);
-        }
+    public SecretKeyType getSecretKeyType(long keyId) throws NotFoundException {
+        Object data = mProviderHelper.getGenericData(Keys.buildKeysUri(mUri),
+                KeyRings.HAS_SECRET,
+                ProviderHelper.FIELD_TYPE_INTEGER,
+                KeyRings.KEY_ID + " = " + Long.toString(keyId));
+        return SecretKeyType.fromNum(((Long) data).intValue());
     }
 
 }
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java
index bfa95280e..186a0fc88 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java
@@ -141,7 +141,11 @@ public class ProviderHelper {
     public static final int FIELD_TYPE_BLOB = 5;
 
     public Object getGenericData(Uri uri, String column, int type) throws NotFoundException {
-        return getGenericData(uri, new String[]{column}, new int[]{type}, null).get(column);
+        Object result = getGenericData(uri, new String[]{column}, new int[]{type}, null).get(column);
+        if (result == null) {
+            throw new NotFoundException();
+        }
+        return result;
     }
 
     public Object getGenericData(Uri uri, String column, int type, String selection)
@@ -229,6 +233,11 @@ public class ProviderHelper {
 
     }
 
+    public long getMasterKeyId(long subKeyId) throws NotFoundException {
+        return (Long) getGenericData(KeyRings.buildUnifiedKeyRingsFindBySubkeyUri(subKeyId),
+                KeyRings.MASTER_KEY_ID, FIELD_TYPE_INTEGER);
+    }
+
     public CachedPublicKeyRing getCachedPublicKeyRing(Uri queryUri) {
         return new CachedPublicKeyRing(this, queryUri);
     }
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/PassphraseCacheService.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/PassphraseCacheService.java
index 1b357bd65..fe5c88f62 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/PassphraseCacheService.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/PassphraseCacheService.java
@@ -41,10 +41,13 @@ import org.spongycastle.bcpg.S2K;
 import org.sufficientlysecure.keychain.Constants;
 import org.sufficientlysecure.keychain.R;
 import org.sufficientlysecure.keychain.helper.Preferences;
+import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKey.SecretKeyType;
 import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKeyRing;
 import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException;
+import org.sufficientlysecure.keychain.provider.CachedPublicKeyRing;
 import org.sufficientlysecure.keychain.provider.KeychainContract;
 import org.sufficientlysecure.keychain.provider.ProviderHelper;
+import org.sufficientlysecure.keychain.provider.ProviderHelper.NotFoundException;
 import org.sufficientlysecure.keychain.util.Log;
 
 import java.util.Date;
@@ -180,12 +183,12 @@ public class PassphraseCacheService extends Service {
     /**
      * Internal implementation to get cached passphrase.
      *
-     * @param keyId
+     * @param subKeyId
      * @return
      */
-    private String getCachedPassphraseImpl(long keyId) throws ProviderHelper.NotFoundException {
+    private String getCachedPassphraseImpl(long subKeyId) throws ProviderHelper.NotFoundException {
         // passphrase for symmetric encryption?
-        if (keyId == Constants.key.symmetric) {
+        if (subKeyId == Constants.key.symmetric) {
             Log.d(Constants.TAG, "PassphraseCacheService.getCachedPassphraseImpl() for symmetric encryption");
             String cachedPassphrase = mPassphraseCache.get(Constants.key.symmetric).getPassphrase();
             if (cachedPassphrase == null) {
@@ -195,32 +198,39 @@ public class PassphraseCacheService extends Service {
             return cachedPassphrase;
         }
 
+        // on "none" key, just do nothing
+        if(subKeyId == Constants.key.none) {
+            return null;
+        }
+
         // try to get master key id which is used as an identifier for cached passphrases
-        Log.d(Constants.TAG, "PassphraseCacheService.getCachedPassphraseImpl() for masterKeyId " + keyId);
-        CanonicalizedSecretKeyRing key = new ProviderHelper(this).getCanonicalizedSecretKeyRing(
-                KeychainContract.KeyRings.buildUnifiedKeyRingsFindBySubkeyUri(keyId));
+        Log.d(Constants.TAG, "PassphraseCacheService.getCachedPassphraseImpl() for masterKeyId " + subKeyId);
+        // find a master key id for our key
+        long masterKeyId = new ProviderHelper(this).getMasterKeyId(subKeyId);
+        CachedPublicKeyRing keyRing = new ProviderHelper(this).getCachedPublicKeyRing(masterKeyId);
 
-        // no passphrase needed? just add empty string and return it, then
-        if (!key.hasPassphrase()) {
-            Log.d(Constants.TAG, "Key has no passphrase! Caches and returns empty passphrase!");
+        // get the type of key (from the database)
+        SecretKeyType keyType = keyRing.getSecretKeyType(subKeyId);
 
+        switch (keyType) {
             // TODO: HACK for yubikeys
-            if (key.getSecretKey().getSecretKey().getS2K().getType() == S2K.GNU_DUMMY_S2K
-                    && key.getSecretKey().getSecretKey().getS2K().getProtectionMode() == 2) {
-                // NFC!
+            case DIVERT_TO_CARD:
                 return "123456";
-            }
-
-            try {
-                addCachedPassphrase(this, keyId, "", key.getPrimaryUserIdWithFallback());
-            } catch (PgpGeneralException e) {
-                Log.d(Constants.TAG, "PgpGeneralException occured");
-            }
-            return "";
+            case PASSPHRASE_EMPTY:
+                try {
+                    addCachedPassphrase(this, subKeyId, "", keyRing.getPrimaryUserIdWithFallback());
+                } catch (PgpGeneralException e) {
+                    Log.d(Constants.TAG, "PgpGeneralException occured");
+                }
+                return "";
+            case UNAVAILABLE:
+                throw new NotFoundException("secret key for this subkey is not available");
+            case GNU_DUMMY:
+                throw new NotFoundException("secret key for stripped subkey is not available");
         }
 
         // get cached passphrase
-        CachedPassphrase cachedPassphrase = mPassphraseCache.get(keyId);
+        CachedPassphrase cachedPassphrase = mPassphraseCache.get(subKeyId);
         if (cachedPassphrase == null) {
             Log.d(Constants.TAG, "PassphraseCacheService: Passphrase not (yet) cached, returning null");
             // not really an error, just means the passphrase is not cached but not empty either
@@ -229,7 +239,7 @@ public class PassphraseCacheService extends Service {
 
         // set it again to reset the cache life cycle
         Log.d(Constants.TAG, "PassphraseCacheService: Cache passphrase again when getting it!");
-        addCachedPassphrase(this, keyId, cachedPassphrase.getPassphrase(), cachedPassphrase.getPrimaryUserID());
+        addCachedPassphrase(this, subKeyId, cachedPassphrase.getPassphrase(), cachedPassphrase.getPrimaryUserID());
         return cachedPassphrase.getPassphrase();
     }
 
@@ -312,7 +322,6 @@ public class PassphraseCacheService extends Service {
                 long keyId = intent.getLongExtra(EXTRA_KEY_ID, -1);
                 Messenger messenger = intent.getParcelableExtra(EXTRA_MESSENGER);
 
-
                 Message msg = Message.obtain();
                 try {
                     String passphrase = getCachedPassphraseImpl(keyId);
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/dialog/PassphraseDialogFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/dialog/PassphraseDialogFragment.java
index ef2659cf8..f19309b51 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/dialog/PassphraseDialogFragment.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/dialog/PassphraseDialogFragment.java
@@ -47,6 +47,7 @@ import org.sufficientlysecure.keychain.compatibility.DialogFragmentWorkaround;
 import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKey;
 import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKeyRing;
 import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException;
+import org.sufficientlysecure.keychain.provider.KeychainContract.KeyRings;
 import org.sufficientlysecure.keychain.provider.ProviderHelper;
 import org.sufficientlysecure.keychain.service.PassphraseCacheService;
 import org.sufficientlysecure.keychain.util.Log;
@@ -98,17 +99,7 @@ public class PassphraseDialogFragment extends DialogFragment implements OnEditor
      */
     public static PassphraseDialogFragment newInstance(Context context, Messenger messenger,
                                                        long secretKeyId) throws PgpGeneralException {
-        // check if secret key has a passphrase
-        if (!(secretKeyId == Constants.key.symmetric || secretKeyId == Constants.key.none)) {
-            try {
-                if (!new ProviderHelper(context).getCanonicalizedSecretKeyRing(secretKeyId).hasPassphrase()) {
-                    throw new PgpGeneralException("No passphrase! No passphrase dialog needed!");
-                }
-            } catch (ProviderHelper.NotFoundException e) {
-                throw new PgpGeneralException("Error: Key not found!", e);
-            }
-        }
-
+        // do NOT check if the key even needs a passphrase. that's not our job here.
         PassphraseDialogFragment frag = new PassphraseDialogFragment();
         Bundle args = new Bundle();
         args.putLong(ARG_SECRET_KEY_ID, secretKeyId);
@@ -141,7 +132,8 @@ public class PassphraseDialogFragment extends DialogFragment implements OnEditor
         } else {
             try {
                 ProviderHelper helper = new ProviderHelper(activity);
-                secretRing = helper.getCanonicalizedSecretKeyRing(secretKeyId);
+                secretRing = helper.getCanonicalizedSecretKeyRing(
+                        KeyRings.buildUnifiedKeyRingsFindBySubkeyUri(secretKeyId));
                 // yes the inner try/catch block is necessary, otherwise the final variable
                 // above can't be statically verified to have been set in all cases because
                 // the catch clause doesn't return.
@@ -191,51 +183,28 @@ public class PassphraseDialogFragment extends DialogFragment implements OnEditor
                     return;
                 }
 
-                CanonicalizedSecretKey unlockedSecretKey = null;
-
-                for (CanonicalizedSecretKey clickSecretKey : secretRing.secretKeyIterator()) {
-                    try {
-                        boolean unlocked = clickSecretKey.unlock(passphrase);
-                        if (unlocked) {
-                            unlockedSecretKey = clickSecretKey;
-                            break;
-                        }
-                    } catch (PgpGeneralException e) {
-                        Toast.makeText(activity, R.string.error_could_not_extract_private_key,
-                                Toast.LENGTH_SHORT).show();
-
-                        sendMessageToHandler(MESSAGE_CANCEL);
-                        return; // ran out of keys to try
-                    }
-                }
-
-                // Means we got an exception every time
-                if (unlockedSecretKey == null) {
-                    Toast.makeText(activity, R.string.wrong_passphrase,
+                try {
+                    // make sure this unlocks
+                    // TODO this is a very costly operation, we should not be doing this here!
+                    secretRing.getSecretKey(secretKeyId).unlock(passphrase);
+                } catch (PgpGeneralException e) {
+                    Toast.makeText(activity, R.string.error_could_not_extract_private_key,
                             Toast.LENGTH_SHORT).show();
 
                     sendMessageToHandler(MESSAGE_CANCEL);
-                    return;
+                    return; // ran out of keys to try
                 }
 
-                long masterKeyId = secretRing.getMasterKeyId();
-
                 // cache the new passphrase
                 Log.d(Constants.TAG, "Everything okay! Caching entered passphrase");
 
                 try {
-                    PassphraseCacheService.addCachedPassphrase(activity, masterKeyId, passphrase,
+                    PassphraseCacheService.addCachedPassphrase(activity, secretKeyId, passphrase,
                             secretRing.getPrimaryUserIdWithFallback());
                 } catch (PgpGeneralException e) {
                     Log.e(Constants.TAG, "adding of a passphrase failed", e);
                 }
 
-                if (unlockedSecretKey.getKeyId() != masterKeyId) {
-                    PassphraseCacheService.addCachedPassphrase(
-                            activity, unlockedSecretKey.getKeyId(), passphrase,
-                            unlockedSecretKey.getPrimaryUserIdWithFallback());
-                }
-
                 // also return passphrase back to activity
                 Bundle data = new Bundle();
                 data.putString(MESSAGE_DATA_PASSPHRASE, passphrase);