Cache CryptoInputParcel in OpenPgpService

4 files changed, 69 insertions, 24 deletions

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 9a8f5c522..39f7f815c 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java
@@ -57,6 +57,7 @@ import org.sufficientlysecure.keychain.ui.PassphraseDialogActivity;
 import org.sufficientlysecure.keychain.ui.ViewKeyActivity;
 import org.sufficientlysecure.keychain.util.InputData;
 import org.sufficientlysecure.keychain.util.Log;
+import org.sufficientlysecure.keychain.util.ParcelableCache;
 import org.sufficientlysecure.keychain.util.Passphrase;
 
 import java.io.IOException;
@@ -67,6 +68,25 @@ import java.util.Set;
 
 public class OpenPgpService extends RemoteService {
 
+    /**
+     * Instead of parceling the CryptoInputParcel, they are cached on our side to prevent
+     * leakage of passphrases, symmetric keys, an yubikey related pass-through values
+     */
+    private static ParcelableCache<CryptoInputParcel> inputParcelCache;
+    static {
+        inputParcelCache = new ParcelableCache<>();
+    }
+
+    public static void cacheCryptoInputParcel(Intent data, CryptoInputParcel inputParcel) {
+        inputParcelCache.cacheAndWriteToIntent(inputParcel, data,
+                OpenPgpApi.EXTRA_CALL_UUID1, OpenPgpApi.EXTRA_CALL_UUID2);
+    }
+
+    public static CryptoInputParcel getCryptoInputParcel(Intent data) {
+        return inputParcelCache.readFromIntentAndGetFromCache(data,
+                OpenPgpApi.EXTRA_CALL_UUID1, OpenPgpApi.EXTRA_CALL_UUID2);
+    }
+
     static final String[] EMAIL_SEARCH_PROJECTION = new String[]{
             KeyRings._ID,
             KeyRings.MASTER_KEY_ID,
@@ -263,18 +283,19 @@ public class OpenPgpService extends RemoteService {
             long inputLength = is.available();
             InputData inputData = new InputData(is, inputLength);
 
-            CryptoInputParcel cryptoInput = data.getParcelableExtra(OpenPgpApi.EXTRA_CRYPTO_INPUT);
-            if (cryptoInput == null) {
-                cryptoInput = new CryptoInputParcel();
+            CryptoInputParcel inputParcel = getCryptoInputParcel(data);
+            if (inputParcel == null) {
+                inputParcel = new CryptoInputParcel();
             }
+            // override passphrase in input parcel if given by API call
             if (data.hasExtra(OpenPgpApi.EXTRA_PASSPHRASE)) {
-                cryptoInput = new CryptoInputParcel(cryptoInput.getSignatureTime(),
+                inputParcel = new CryptoInputParcel(inputParcel.getSignatureTime(),
                         new Passphrase(data.getCharArrayExtra(OpenPgpApi.EXTRA_PASSPHRASE)));
             }
 
             // execute PGP operation!
             PgpSignEncryptOperation pse = new PgpSignEncryptOperation(this, new ProviderHelper(getContext()), null);
-            PgpSignEncryptResult pgpResult = pse.execute(pseInput, cryptoInput, inputData, os);
+            PgpSignEncryptResult pgpResult = pse.execute(pseInput, inputParcel, inputData, os);
 
             if (pgpResult.isPending()) {
 
@@ -403,19 +424,20 @@ public class OpenPgpService extends RemoteService {
                         .setAdditionalEncryptId(signKeyId); // add sign key for encryption
             }
 
-            CryptoInputParcel cryptoInput = data.getParcelableExtra(OpenPgpApi.EXTRA_CRYPTO_INPUT);
-            if (cryptoInput == null) {
-                cryptoInput = new CryptoInputParcel();
+            CryptoInputParcel inputParcel = getCryptoInputParcel(data);
+            if (inputParcel == null) {
+                inputParcel = new CryptoInputParcel();
             }
+            // override passphrase in input parcel if given by API call
             if (data.hasExtra(OpenPgpApi.EXTRA_PASSPHRASE)) {
-                cryptoInput = new CryptoInputParcel(cryptoInput.getSignatureTime(),
+                inputParcel = new CryptoInputParcel(inputParcel.getSignatureTime(),
                         new Passphrase(data.getCharArrayExtra(OpenPgpApi.EXTRA_PASSPHRASE)));
             }
 
             PgpSignEncryptOperation op = new PgpSignEncryptOperation(this, new ProviderHelper(getContext()), null);
 
             // execute PGP operation!
-            PgpSignEncryptResult pgpResult = op.execute(pseInput, cryptoInput, inputData, os);
+            PgpSignEncryptResult pgpResult = op.execute(pseInput, inputParcel, inputData, os);
 
             if (pgpResult.isPending()) {
                 RequiredInputParcel requiredInput = pgpResult.getRequiredInputParcel();
@@ -491,12 +513,13 @@ public class OpenPgpService extends RemoteService {
                     this, new ProviderHelper(getContext()), null, inputData, os
             );
 
-            CryptoInputParcel cryptoInput = data.getParcelableExtra(OpenPgpApi.EXTRA_CRYPTO_INPUT);
-            if (cryptoInput == null) {
-                cryptoInput = new CryptoInputParcel();
+            CryptoInputParcel inputParcel = getCryptoInputParcel(data);
+            if (inputParcel == null) {
+                inputParcel = new CryptoInputParcel();
             }
+            // override passphrase in input parcel if given by API call
             if (data.hasExtra(OpenPgpApi.EXTRA_PASSPHRASE)) {
-                cryptoInput = new CryptoInputParcel(cryptoInput.getSignatureTime(),
+                inputParcel = new CryptoInputParcel(inputParcel.getSignatureTime(),
                         new Passphrase(data.getCharArrayExtra(OpenPgpApi.EXTRA_PASSPHRASE)));
             }
 
@@ -509,7 +532,7 @@ public class OpenPgpService extends RemoteService {
                     .setDecryptMetadataOnly(decryptMetadataOnly)
                     .setDetachedSignature(detachedSignature);
 
-            DecryptVerifyResult pgpResult = builder.build().execute(cryptoInput);
+            DecryptVerifyResult pgpResult = builder.build().execute(inputParcel);
 
             if (pgpResult.isPending()) {
                 // prepare and return PendingIntent to be executed by client
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/NfcOperationActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/NfcOperationActivity.java
index d70b0aad1..02fc81825 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/NfcOperationActivity.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/NfcOperationActivity.java
@@ -16,6 +16,7 @@ import android.view.WindowManager;
 import org.openintents.openpgp.util.OpenPgpApi;
 import org.sufficientlysecure.keychain.Constants;
 import org.sufficientlysecure.keychain.R;
+import org.sufficientlysecure.keychain.remote.OpenPgpService;
 import org.sufficientlysecure.keychain.service.PassphraseCacheService;
 import org.sufficientlysecure.keychain.service.input.CryptoInputParcel;
 import org.sufficientlysecure.keychain.service.input.RequiredInputParcel;
@@ -69,7 +70,7 @@ public class NfcOperationActivity extends BaseNfcActivity {
     @Override
     protected void onNfcPerform() throws IOException {
 
-        CryptoInputParcel resultData = new CryptoInputParcel(mRequiredInput.mSignatureTime);
+        CryptoInputParcel inputParcel = new CryptoInputParcel(mRequiredInput.mSignatureTime);
 
         switch (mRequiredInput.mType) {
 
@@ -77,7 +78,7 @@ public class NfcOperationActivity extends BaseNfcActivity {
                 for (int i = 0; i < mRequiredInput.mInputHashes.length; i++) {
                     byte[] hash = mRequiredInput.mInputHashes[i];
                     byte[] decryptedSessionKey = nfcDecryptSessionKey(hash);
-                    resultData.addCryptoData(hash, decryptedSessionKey);
+                    inputParcel.addCryptoData(hash, decryptedSessionKey);
                 }
                 break;
 
@@ -86,17 +87,17 @@ public class NfcOperationActivity extends BaseNfcActivity {
                     byte[] hash = mRequiredInput.mInputHashes[i];
                     int algo = mRequiredInput.mSignAlgos[i];
                     byte[] signedHash = nfcCalculateSignature(hash, algo);
-                    resultData.addCryptoData(hash, signedHash);
+                    inputParcel.addCryptoData(hash, signedHash);
                 }
                 break;
         }
 
         if (mServiceIntent != null) {
-            mServiceIntent.putExtra(OpenPgpApi.EXTRA_CRYPTO_INPUT, resultData);
+            OpenPgpService.cacheCryptoInputParcel(mServiceIntent, inputParcel);
             setResult(RESULT_OK, mServiceIntent);
         } else {
             Intent result = new Intent();
-            result.putExtra(NfcOperationActivity.RESULT_DATA, resultData);
+            result.putExtra(NfcOperationActivity.RESULT_DATA, inputParcel);
             setResult(RESULT_OK, result);
         }
 
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/PassphraseDialogActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/PassphraseDialogActivity.java
index 007608f80..bf6129407 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/PassphraseDialogActivity.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/PassphraseDialogActivity.java
@@ -53,6 +53,7 @@ import org.sufficientlysecure.keychain.pgp.exception.PgpKeyNotFoundException;
 import org.sufficientlysecure.keychain.provider.CachedPublicKeyRing;
 import org.sufficientlysecure.keychain.provider.KeychainContract;
 import org.sufficientlysecure.keychain.provider.ProviderHelper;
+import org.sufficientlysecure.keychain.remote.OpenPgpService;
 import org.sufficientlysecure.keychain.service.PassphraseCacheService;
 import org.sufficientlysecure.keychain.service.input.CryptoInputParcel;
 import org.sufficientlysecure.keychain.service.input.RequiredInputParcel;
@@ -425,14 +426,14 @@ public class PassphraseDialogActivity extends FragmentActivity {
                 return;
             }
 
+            CryptoInputParcel inputParcel = new CryptoInputParcel(null, passphrase);
             if (mServiceIntent != null) {
-                // TODO really pass this through the PendingIntent?
-                mServiceIntent.putExtra(OpenPgpApi.EXTRA_CRYPTO_INPUT, new CryptoInputParcel(null, passphrase));
+                OpenPgpService.cacheCryptoInputParcel(mServiceIntent, inputParcel);
                 getActivity().setResult(RESULT_OK, mServiceIntent);
             } else {
                 // also return passphrase back to activity
                 Intent returnIntent = new Intent();
-                returnIntent.putExtra(RESULT_CRYPTO_INPUT, new CryptoInputParcel(null, passphrase));
+                returnIntent.putExtra(RESULT_CRYPTO_INPUT, inputParcel);
                 getActivity().setResult(RESULT_OK, returnIntent);
             }
 
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/ParcelableCache.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/ParcelableCache.java
index 4d723cb30..54c984190 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/ParcelableCache.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/ParcelableCache.java
@@ -17,6 +17,8 @@
 
 package org.sufficientlysecure.keychain.util;
 
+import android.content.Intent;
+import android.os.Bundle;
 import android.os.Parcel;
 
 import java.util.UUID;
@@ -75,6 +77,17 @@ public class ParcelableCache<E extends Object> {
         }
     }
 
+    public E readFromIntentAndGetFromCache(Intent data, String key1, String key2) {
+        if (!data.getExtras().containsKey(key1) || !data.getExtras().containsKey(key2)) {
+            return null;
+        }
+        long mostSig = data.getLongExtra(key1, 0);
+        long leastSig = data.getLongExtra(key2, 0);
+        UUID mTicket = new UUID(mostSig, leastSig);
+        // fetch the dehydrated log out of storage (this removes it from the dehydration pool)
+        return rehydrateParcelable(mTicket);
+    }
+
     public E readFromParcelAndGetFromCache(Parcel source) {
         long mostSig = source.readLong();
         long leastSig = source.readLong();
@@ -83,7 +96,6 @@ public class ParcelableCache<E extends Object> {
         return rehydrateParcelable(mTicket);
     }
 
-
     public void cacheAndWriteToParcel(E parcelable, Parcel dest) {
         // Get a ticket for our log.
         UUID mTicket = dehydrateParcelable(parcelable);
@@ -92,4 +104,12 @@ public class ParcelableCache<E extends Object> {
         dest.writeLong(mTicket.getLeastSignificantBits());
     }
 
+    public void cacheAndWriteToIntent(E parcelable, Intent data, String key1, String key2) {
+        // Get a ticket for our log.
+        UUID mTicket = dehydrateParcelable(parcelable);
+        // And write out the UUID most and least significant bits.
+        data.putExtra(key1, mTicket.getMostSignificantBits());
+        data.putExtra(key2, mTicket.getLeastSignificantBits());
+    }
+
 }