Better error handling for passphrase cache if key is missing

7 files changed, 115 insertions, 71 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 6fa380357..7d113241b 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpDecryptVerify.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpDecryptVerify.java
@@ -46,6 +46,7 @@ import org.sufficientlysecure.keychain.R;
 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.InputData;
 import org.sufficientlysecure.keychain.util.Log;
 import org.sufficientlysecure.keychain.util.ProgressScaler;
@@ -163,7 +164,8 @@ public class PgpDecryptVerify {
     }
 
     public interface PassphraseCache {
-        public String getCachedPassphrase(long masterKeyId);
+        public String getCachedPassphrase(long masterKeyId)
+                throws NoSecretKeyException;
     }
 
     public static class InvalidDataException extends Exception {
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java
index 50c8fcf3b..1d3d7d02f 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java
@@ -168,8 +168,7 @@ public class OpenPgpService extends RemoteService {
             }
             if (passphrase == null) {
                 // get PendingIntent for passphrase input, add it to given params and return to client
-                Intent passphraseBundle = getPassphraseBundleIntent(data, accSettings.getKeyId());
-                return passphraseBundle;
+                return getPassphraseBundleIntent(data, accSettings.getKeyId());
             }
 
             // Get Input- and OutputStream from ParcelFileDescriptor
@@ -289,8 +288,7 @@ public class OpenPgpService extends RemoteService {
                     }
                     if (passphrase == null) {
                         // get PendingIntent for passphrase input, add it to given params and return to client
-                        Intent passphraseBundle = getPassphraseBundleIntent(data, accSettings.getKeyId());
-                        return passphraseBundle;
+                        return getPassphraseBundleIntent(data, accSettings.getKeyId());
                     }
 
                     // sign and encrypt
@@ -358,9 +356,13 @@ public class OpenPgpService extends RemoteService {
                         new ProviderHelper(this),
                         new PgpDecryptVerify.PassphraseCache() {
                             @Override
-                            public String getCachedPassphrase(long masterKeyId) {
-                                return PassphraseCacheService.getCachedPassphrase(
-                                        OpenPgpService.this, masterKeyId);
+                            public String getCachedPassphrase(long masterKeyId) throws PgpDecryptVerify.NoSecretKeyException {
+                                try {
+                                    return PassphraseCacheService.getCachedPassphrase(
+                                            OpenPgpService.this, masterKeyId);
+                                } catch (PassphraseCacheService.KeyNotFoundException e) {
+                                    throw new PgpDecryptVerify.NoSecretKeyException();
+                                }
                             }
                         },
                         inputData, os
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java
index 54895973b..443f66d58 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java
@@ -307,9 +307,13 @@ public class KeychainIntentService extends IntentService
                         new ProviderHelper(this),
                         new PgpDecryptVerify.PassphraseCache() {
                             @Override
-                            public String getCachedPassphrase(long masterKeyId) {
-                                return PassphraseCacheService.getCachedPassphrase(
-                                        KeychainIntentService.this, masterKeyId);
+                            public String getCachedPassphrase(long masterKeyId) throws PgpDecryptVerify.NoSecretKeyException {
+                                try {
+                                    return PassphraseCacheService.getCachedPassphrase(
+                                            KeychainIntentService.this, masterKeyId);
+                                } catch (PassphraseCacheService.KeyNotFoundException e) {
+                                    throw new PgpDecryptVerify.NoSecretKeyException();
+                                }
                             }
                         },
                         inputData, outStream
@@ -351,9 +355,13 @@ public class KeychainIntentService extends IntentService
                         new ProviderHelper(this),
                         new PgpDecryptVerify.PassphraseCache() {
                             @Override
-                            public String getCachedPassphrase(long masterKeyId) {
-                                return PassphraseCacheService.getCachedPassphrase(
-                                        KeychainIntentService.this, masterKeyId);
+                            public String getCachedPassphrase(long masterKeyId) throws PgpDecryptVerify.NoSecretKeyException {
+                                try {
+                                    return PassphraseCacheService.getCachedPassphrase(
+                                            KeychainIntentService.this, masterKeyId);
+                                } catch (PassphraseCacheService.KeyNotFoundException e) {
+                                    throw new PgpDecryptVerify.NoSecretKeyException();
+                                }
                             }
                         },
                         inputData, null
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 e95e737d5..ae1b026a5 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/PassphraseCacheService.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/PassphraseCacheService.java
@@ -76,12 +76,24 @@ public class PassphraseCacheService extends Service {
 
     private static final int NOTIFICATION_ID = 1;
 
+    private static final int MSG_PASSPHRASE_CACHE_GET_OKAY = 1;
+    private static final int MSG_PASSPHRASE_CACHE_GET_KEY_NO_FOUND = 2;
+
     private BroadcastReceiver mIntentReceiver;
 
     private LongSparseArray<CachedPassphrase> mPassphraseCache = new LongSparseArray<CachedPassphrase>();
 
     Context mContext;
 
+    public static class KeyNotFoundException extends Exception {
+        public KeyNotFoundException() {
+        }
+
+        public KeyNotFoundException(String name) {
+            super(name);
+        }
+    }
+
     /**
      * This caches a new passphrase in memory by sending a new command to the service. An android
      * service is only run once. Thus, when the service is already started, new commands just add
@@ -114,24 +126,23 @@ public class PassphraseCacheService extends Service {
      * @param keyId
      * @return passphrase or null (if no passphrase is cached for this keyId)
      */
-    public static String getCachedPassphrase(Context context, long keyId) {
+    public static String getCachedPassphrase(Context context, long keyId) throws KeyNotFoundException {
         Log.d(Constants.TAG, "PassphraseCacheService.getCachedPassphrase() get masterKeyId for " + keyId);
 
         Intent intent = new Intent(context, PassphraseCacheService.class);
         intent.setAction(ACTION_PASSPHRASE_CACHE_GET);
 
         final Object mutex = new Object();
-        final Bundle returnBundle = new Bundle();
+        final Message returnMessage = Message.obtain();
 
         HandlerThread handlerThread = new HandlerThread("getPassphraseThread");
         handlerThread.start();
         Handler returnHandler = new Handler(handlerThread.getLooper()) {
             @Override
             public void handleMessage(Message message) {
-                if (message.obj != null) {
-                    String passphrase = ((Bundle) message.obj).getString(EXTRA_PASSPHRASE);
-                    returnBundle.putString(EXTRA_PASSPHRASE, passphrase);
-                }
+                // copy over result to handle after mutex.wait
+                returnMessage.what = message.what;
+                returnMessage.copyFrom(message);
                 synchronized (mutex) {
                     mutex.notify();
                 }
@@ -155,10 +166,13 @@ public class PassphraseCacheService extends Service {
             }
         }
 
-        if (returnBundle.containsKey(EXTRA_PASSPHRASE)) {
-            return returnBundle.getString(EXTRA_PASSPHRASE);
-        } else {
-            return null;
+        switch (returnMessage.what) {
+            case MSG_PASSPHRASE_CACHE_GET_OKAY:
+                return returnMessage.getData().getString(EXTRA_PASSPHRASE);
+            case MSG_PASSPHRASE_CACHE_GET_KEY_NO_FOUND:
+                throw new KeyNotFoundException();
+            default:
+                throw new KeyNotFoundException("should not happen!");
         }
     }
 
@@ -168,7 +182,7 @@ public class PassphraseCacheService extends Service {
      * @param keyId
      * @return
      */
-    private String getCachedPassphraseImpl(long keyId) {
+    private String getCachedPassphraseImpl(long keyId) throws ProviderHelper.NotFoundException {
         // passphrase for symmetric encryption?
         if (keyId == Constants.key.symmetric) {
             Log.d(Constants.TAG, "PassphraseCacheService.getCachedPassphraseImpl() for symmetric encryption");
@@ -181,39 +195,33 @@ public class PassphraseCacheService extends Service {
         }
 
         // try to get master key id which is used as an identifier for cached passphrases
-        try {
-            Log.d(Constants.TAG, "PassphraseCacheService.getCachedPassphraseImpl() for masterKeyId " + keyId);
-            CanonicalizedSecretKeyRing key = new ProviderHelper(this).getCanonicalizedSecretKeyRing(
-                    KeychainContract.KeyRings.buildUnifiedKeyRingsFindBySubkeyUri(keyId));
-            // 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!");
-
-                try {
-                    addCachedPassphrase(this, keyId, "", key.getPrimaryUserIdWithFallback());
-                } catch (PgpGeneralException e) {
-                    Log.d(Constants.TAG, "PgpGeneralException occured");
-                }
-                return "";
-            }
+        Log.d(Constants.TAG, "PassphraseCacheService.getCachedPassphraseImpl() for masterKeyId " + keyId);
+        CanonicalizedSecretKeyRing key = new ProviderHelper(this).getCanonicalizedSecretKeyRing(
+                KeychainContract.KeyRings.buildUnifiedKeyRingsFindBySubkeyUri(keyId));
+        // 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 cached passphrase
-            CachedPassphrase cachedPassphrase = mPassphraseCache.get(keyId);
-            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
-                return null;
+            try {
+                addCachedPassphrase(this, keyId, "", key.getPrimaryUserIdWithFallback());
+            } catch (PgpGeneralException e) {
+                Log.d(Constants.TAG, "PgpGeneralException occured");
             }
+            return "";
+        }
 
-            // 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());
-            return cachedPassphrase.getPassphrase();
-
-        } catch (ProviderHelper.NotFoundException e) {
-            Log.e(Constants.TAG, "PassphraseCacheService: Passphrase for unknown key was requested!");
+        // get cached passphrase
+        CachedPassphrase cachedPassphrase = mPassphraseCache.get(keyId);
+        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
             return null;
         }
+
+        // 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());
+        return cachedPassphrase.getPassphrase();
     }
 
     /**
@@ -295,12 +303,19 @@ public class PassphraseCacheService extends Service {
                 long keyId = intent.getLongExtra(EXTRA_KEY_ID, -1);
                 Messenger messenger = intent.getParcelableExtra(EXTRA_MESSENGER);
 
-                String passphrase = getCachedPassphraseImpl(keyId);
 
                 Message msg = Message.obtain();
-                Bundle bundle = new Bundle();
-                bundle.putString(EXTRA_PASSPHRASE, passphrase);
-                msg.obj = bundle;
+                try {
+                    String passphrase = getCachedPassphraseImpl(keyId);
+                    msg.what = MSG_PASSPHRASE_CACHE_GET_OKAY;
+                    Bundle bundle = new Bundle();
+                    bundle.putString(EXTRA_PASSPHRASE, passphrase);
+                    msg.setData(bundle);
+                } catch (ProviderHelper.NotFoundException e) {
+                    Log.e(Constants.TAG, "PassphraseCacheService: Passphrase for unknown key was requested!");
+                    msg.what = MSG_PASSPHRASE_CACHE_GET_KEY_NO_FOUND;
+                }
+
                 try {
                     messenger.send(msg);
                 } catch (RemoteException e) {
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CertifyKeyActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CertifyKeyActivity.java
index 467e0c14a..c1986825c 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CertifyKeyActivity.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CertifyKeyActivity.java
@@ -231,7 +231,14 @@ public class CertifyKeyActivity extends ActionBarActivity implements
      */
     private void initiateCertifying() {
         // get the user's passphrase for this key (if required)
-        String passphrase = PassphraseCacheService.getCachedPassphrase(this, mMasterKeyId);
+        String passphrase = null;
+        try {
+            passphrase = PassphraseCacheService.getCachedPassphrase(this, mMasterKeyId);
+        } catch (PassphraseCacheService.KeyNotFoundException e) {
+            Log.e(Constants.TAG, "Key not found!", e);
+            finish();
+            return;
+        }
         if (passphrase == null) {
             PassphraseDialogFragment.show(this, mMasterKeyId,
                     new Handler() {
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyFragment.java
index ae294547f..03074bb6a 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyFragment.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyFragment.java
@@ -475,8 +475,14 @@ public class EditKeyFragment extends LoaderFragment implements
     }
 
     private void cachePassphraseForEdit() {
-        mCurrentPassphrase = PassphraseCacheService.getCachedPassphrase(getActivity(),
-                mSaveKeyringParcel.mMasterKeyId);
+        try {
+            mCurrentPassphrase = PassphraseCacheService.getCachedPassphrase(getActivity(),
+                    mSaveKeyringParcel.mMasterKeyId);
+        } catch (PassphraseCacheService.KeyNotFoundException e) {
+            Log.e(Constants.TAG, "Key not found!", e);
+            getActivity().finish();
+            return;
+        }
         if (mCurrentPassphrase == null) {
             PassphraseDialogFragment.show(getActivity(), mSaveKeyringParcel.mMasterKeyId,
                     new Handler() {
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EncryptActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EncryptActivity.java
index 33deddd81..7e08f6b7c 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EncryptActivity.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EncryptActivity.java
@@ -450,20 +450,24 @@ public class EncryptActivity extends DrawerActivity implements EncryptActivityIn
                 return false;
             }
 
-            if (mSigningKeyId != 0 && PassphraseCacheService.getCachedPassphrase(this, mSigningKeyId) == null) {
-                PassphraseDialogFragment.show(this, mSigningKeyId,
-                        new Handler() {
-                            @Override
-                            public void handleMessage(Message message) {
-                                if (message.what == PassphraseDialogFragment.MESSAGE_OKAY) {
-                                    // restart
-                                    startEncrypt();
+            try {
+                if (mSigningKeyId != 0 && PassphraseCacheService.getCachedPassphrase(this, mSigningKeyId) == null) {
+                    PassphraseDialogFragment.show(this, mSigningKeyId,
+                            new Handler() {
+                                @Override
+                                public void handleMessage(Message message) {
+                                    if (message.what == PassphraseDialogFragment.MESSAGE_OKAY) {
+                                        // restart
+                                        startEncrypt();
+                                    }
                                 }
                             }
-                        }
-                );
+                    );
 
-                return false;
+                    return false;
+                }
+            } catch (PassphraseCacheService.KeyNotFoundException e) {
+                Log.e(Constants.TAG, "Key not found!", e);
             }
         }
         return true;