From 7074b443472b620dbfd452d1682b30407b1851b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20Sch=C3=BCrmann?= Date: Fri, 10 Apr 2015 15:58:37 +0200 Subject: Cache CryptoInputParcel in OpenPgpService --- .../keychain/remote/OpenPgpService.java | 53 ++++++++++++++++------ .../keychain/ui/NfcOperationActivity.java | 11 +++-- .../keychain/ui/PassphraseDialogActivity.java | 7 +-- .../keychain/util/ParcelableCache.java | 22 ++++++++- 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 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 { } } + 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 { 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 { 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()); + } + } -- cgit v1.2.3