fix symmetric passphrase caching (fixes #1401)

5 files changed, 42 insertions, 16 deletions

diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/BaseOperation.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/BaseOperation.java
index a8d1f0313..e4026eaaf 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/BaseOperation.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/BaseOperation.java
@@ -21,6 +21,7 @@ import android.content.Context;
 import android.os.Parcelable;
 import android.support.annotation.NonNull;
 
+import org.sufficientlysecure.keychain.Constants.key;
 import org.sufficientlysecure.keychain.operations.results.OperationResult;
 import org.sufficientlysecure.keychain.pgp.PassphraseCacheInterface;
 import org.sufficientlysecure.keychain.pgp.Progressable;
@@ -111,8 +112,11 @@ public abstract class BaseOperation <T extends Parcelable> implements Passphrase
     @Override
     public Passphrase getCachedPassphrase(long subKeyId) throws NoSecretKeyException {
         try {
-            long masterKeyId = mProviderHelper.getMasterKeyId(subKeyId);
-            return getCachedPassphrase(masterKeyId, subKeyId);
+            if (subKeyId != key.symmetric) {
+                long masterKeyId = mProviderHelper.getMasterKeyId(subKeyId);
+                return getCachedPassphrase(masterKeyId, subKeyId);
+            }
+            return getCachedPassphrase(key.symmetric, key.symmetric);
         } catch (NotFoundException e) {
             throw new PassphraseCacheInterface.NoSecretKeyException();
         }
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/OperationResult.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/OperationResult.java
index 9da3b94af..88d04350c 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/OperationResult.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/OperationResult.java
@@ -605,6 +605,7 @@ public abstract class OperationResult implements Parcelable {
         MSG_DC_CLEAR_SIGNATURE_OK (LogLevel.OK, R.string.msg_dc_clear_signature_ok),
         MSG_DC_CLEAR_SIGNATURE (LogLevel.DEBUG, R.string.msg_dc_clear_signature),
         MSG_DC_ERROR_BAD_PASSPHRASE (LogLevel.ERROR, R.string.msg_dc_error_bad_passphrase),
+        MSG_DC_ERROR_SYM_PASSPHRASE (LogLevel.ERROR, R.string.msg_dc_error_sym_passphrase),
         MSG_DC_ERROR_CORRUPT_DATA (LogLevel.ERROR, R.string.msg_dc_error_corrupt_data),
         MSG_DC_ERROR_EXTRACT_KEY (LogLevel.ERROR, R.string.msg_dc_error_extract_key),
         MSG_DC_ERROR_INTEGRITY_CHECK (LogLevel.ERROR, R.string.msg_dc_error_integrity_check),
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 953150b72..7b61968a3 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpDecryptVerify.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpDecryptVerify.java
@@ -26,6 +26,7 @@ import org.openintents.openpgp.OpenPgpMetadata;
 import org.openintents.openpgp.OpenPgpSignatureResult;
 import org.spongycastle.bcpg.ArmoredInputStream;
 import org.spongycastle.openpgp.PGPCompressedData;
+import org.spongycastle.openpgp.PGPDataValidationException;
 import org.spongycastle.openpgp.PGPEncryptedData;
 import org.spongycastle.openpgp.PGPEncryptedDataList;
 import org.spongycastle.openpgp.PGPException;
@@ -47,6 +48,7 @@ import org.spongycastle.openpgp.operator.jcajce.JcaPGPDigestCalculatorProviderBu
 import org.spongycastle.openpgp.operator.jcajce.JcePBEDataDecryptorFactoryBuilder;
 import org.spongycastle.util.encoders.DecoderException;
 import org.sufficientlysecure.keychain.Constants;
+import org.sufficientlysecure.keychain.Constants.key;
 import org.sufficientlysecure.keychain.R;
 import org.sufficientlysecure.keychain.operations.BaseOperation;
 import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKey.SecretKeyType;
@@ -485,12 +487,23 @@ public class PgpDecryptVerify extends BaseOperation<PgpDecryptVerifyInputParcel>
                 // if no passphrase is given, return here
                 // indicating that a passphrase is missing!
                 if (!cryptoInput.hasPassphrase()) {
-                    log.add(LogType.MSG_DC_PENDING_PASSPHRASE, indent + 1);
-                    return new DecryptVerifyResult(log,
-                            RequiredInputParcel.createRequiredSymmetricPassphrase());
-                }
 
-                passphrase = cryptoInput.getPassphrase();
+                    try {
+                        passphrase = getCachedPassphrase(key.symmetric);
+                        log.add(LogType.MSG_DC_PASS_CACHED, indent + 1);
+                    } catch (PassphraseCacheInterface.NoSecretKeyException e) {
+                        // nvm
+                    }
+
+                    if (passphrase == null) {
+                        log.add(LogType.MSG_DC_PENDING_PASSPHRASE, indent + 1);
+                        return new DecryptVerifyResult(log,
+                                RequiredInputParcel.createRequiredSymmetricPassphrase());
+                    }
+
+                } else {
+                    passphrase = cryptoInput.getPassphrase();
+                }
 
                 // break out of while, only decrypt the first packet
                 break;
@@ -526,7 +539,14 @@ public class PgpDecryptVerify extends BaseOperation<PgpDecryptVerifyInputParcel>
                     digestCalcProvider).setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME).build(
                     passphrase.getCharArray());
 
-            clear = encryptedDataSymmetric.getDataStream(decryptorFactory);
+            try {
+                clear = encryptedDataSymmetric.getDataStream(decryptorFactory);
+            } catch (PGPDataValidationException e) {
+                log.add(LogType.MSG_DC_ERROR_SYM_PASSPHRASE, indent +1);
+                return new DecryptVerifyResult(log,
+                        RequiredInputParcel.createRequiredSymmetricPassphrase());
+            }
+
             encryptedData = encryptedDataSymmetric;
 
             symmetricEncryptionAlgo = encryptedDataSymmetric.getSymmetricAlgorithm(decryptorFactory);
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 7c0b7eaef..2a258b7e3 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/PassphraseCacheService.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/PassphraseCacheService.java
@@ -232,21 +232,21 @@ public class PassphraseCacheService extends Service {
      * Internal implementation to get cached passphrase.
      */
     private Passphrase getCachedPassphraseImpl(long masterKeyId, long subKeyId) throws ProviderHelper.NotFoundException {
+        // on "none" key, just do nothing
+        if (masterKeyId == Constants.key.none) {
+            return null;
+        }
+
         // passphrase for symmetric encryption?
         if (masterKeyId == Constants.key.symmetric) {
             Log.d(Constants.TAG, "PassphraseCacheService.getCachedPassphraseImpl() for symmetric encryption");
-            Passphrase cachedPassphrase = mPassphraseCache.get(Constants.key.symmetric).getPassphrase();
+            CachedPassphrase cachedPassphrase = mPassphraseCache.get(Constants.key.symmetric);
             if (cachedPassphrase == null) {
                 return null;
             }
             addCachedPassphrase(this, Constants.key.symmetric, Constants.key.symmetric,
-                    cachedPassphrase, getString(R.string.passp_cache_notif_pwd));
-            return cachedPassphrase;
-        }
-
-        // on "none" key, just do nothing
-        if (masterKeyId == Constants.key.none) {
-            return null;
+                    cachedPassphrase.getPassphrase(), getString(R.string.passp_cache_notif_pwd));
+            return cachedPassphrase.getPassphrase();
         }
 
         // try to get master key id which is used as an identifier for cached passphrases
diff --git a/OpenKeychain/src/main/res/values/strings.xml b/OpenKeychain/src/main/res/values/strings.xml
index a6052dffe..9135c24c5 100644
--- a/OpenKeychain/src/main/res/values/strings.xml
+++ b/OpenKeychain/src/main/res/values/strings.xml
@@ -1103,6 +1103,7 @@
     <string name="msg_dc_clear_signature">"Saving signature data for later"</string>
     <string name="msg_dc_clear">"Processing cleartext data"</string>
     <string name="msg_dc_error_bad_passphrase">"Error unlocking key, bad password!"</string>
+    <string name="msg_dc_error_sym_passphrase">"Error decrypting data! (Bad passphrase?)"</string>
     <string name="msg_dc_error_corrupt_data">"Data is corrupt!"</string>
     <string name="msg_dc_error_extract_key">"Unknown error unlocking key!"</string>
     <string name="msg_dc_error_integrity_check">"Integrity check error!"</string>