From faa66d6140e7f68d920e612c21fa01139b12170e Mon Sep 17 00:00:00 2001 From: Adithya Abraham Philip Date: Fri, 10 Jul 2015 02:02:27 +0530 Subject: prevented passphrase caching on revocation --- .../keychain/operations/EditKeyOperation.java | 2 +- .../keychain/operations/RevokeOperation.java | 3 ++ .../keychain/service/input/CryptoInputParcel.java | 17 +++++++- .../keychain/ui/DeleteKeyDialogActivity.java | 3 +- .../keychain/ui/PassphraseDialogActivity.java | 48 ++++++++++++++++------ .../keychain/ui/base/CryptoOperationHelper.java | 3 +- 6 files changed, 59 insertions(+), 17 deletions(-) (limited to 'OpenKeychain/src/main/java/org/sufficientlysecure') diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/EditKeyOperation.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/EditKeyOperation.java index f8f7e79a3..6a1c01dd1 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/EditKeyOperation.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/EditKeyOperation.java @@ -161,7 +161,7 @@ public class EditKeyOperation extends BaseOperation { } // There is a new passphrase - cache it - if (saveParcel.mNewUnlock != null) { + if (saveParcel.mNewUnlock != null && cryptoInput.mCachePassphrase) { log.add(LogType.MSG_ED_CACHING_NEW, 1); // NOTE: Don't cache empty passphrases! Important for MOVE_KEY_TO_CARD diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/RevokeOperation.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/RevokeOperation.java index 157e36e04..0fc3f1bc3 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/RevokeOperation.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/RevokeOperation.java @@ -32,6 +32,9 @@ public class RevokeOperation extends BaseOperation { public OperationResult execute(RevokeKeyringParcel revokeKeyringParcel, CryptoInputParcel cryptoInputParcel) { + // we don't cache passphrases during revocation + cryptoInputParcel.mCachePassphrase = false; + long masterKeyId = revokeKeyringParcel.mMasterKeyId; OperationResult.OperationLog log = new OperationResult.OperationLog(); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/input/CryptoInputParcel.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/input/CryptoInputParcel.java index d4cebe67c..9ba9601e5 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/input/CryptoInputParcel.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/input/CryptoInputParcel.java @@ -34,12 +34,15 @@ import java.util.Map; public class CryptoInputParcel implements Parcelable { final Date mSignatureTime; - final Passphrase mPassphrase; + public Passphrase mPassphrase; // used to supply an explicit proxy to operations that require it // this is not final so it can be added to an existing CryptoInputParcel // (e.g) CertifyOperation with upload might require both passphrase and orbot to be enabled private ParcelableProxy mParcelableProxy; + // specifies whether passphrases should be cached + public boolean mCachePassphrase = true; + // this map contains both decrypted session keys and signed hashes to be // used in the crypto operation described by this parcel. private HashMap mCryptoData = new HashMap<>(); @@ -47,21 +50,25 @@ public class CryptoInputParcel implements Parcelable { public CryptoInputParcel() { mSignatureTime = new Date(); mPassphrase = null; + mCachePassphrase = true; } public CryptoInputParcel(Date signatureTime, Passphrase passphrase) { mSignatureTime = signatureTime == null ? new Date() : signatureTime; mPassphrase = passphrase; + mCachePassphrase = true; } public CryptoInputParcel(Passphrase passphrase) { mSignatureTime = new Date(); mPassphrase = passphrase; + mCachePassphrase = true; } public CryptoInputParcel(Date signatureTime) { mSignatureTime = signatureTime == null ? new Date() : signatureTime; mPassphrase = null; + mCachePassphrase = true; } public CryptoInputParcel(ParcelableProxy parcelableProxy) { @@ -69,10 +76,17 @@ public class CryptoInputParcel implements Parcelable { mParcelableProxy = parcelableProxy; } + public CryptoInputParcel(boolean cachePassphrase) { + mSignatureTime = new Date(); + mPassphrase = null; + mCachePassphrase = cachePassphrase; + } + protected CryptoInputParcel(Parcel source) { mSignatureTime = new Date(source.readLong()); mPassphrase = source.readParcelable(getClass().getClassLoader()); mParcelableProxy = source.readParcelable(getClass().getClassLoader()); + mCachePassphrase = source.readByte() != 0; { int count = source.readInt(); @@ -96,6 +110,7 @@ public class CryptoInputParcel implements Parcelable { dest.writeLong(mSignatureTime.getTime()); dest.writeParcelable(mPassphrase, 0); dest.writeParcelable(mParcelableProxy, 0); + dest.writeByte((byte) (mCachePassphrase ? 1 : 0)); dest.writeInt(mCryptoData.size()); for (HashMap.Entry entry : mCryptoData.entrySet()) { diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/DeleteKeyDialogActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/DeleteKeyDialogActivity.java index b89c476d6..98d3cf7b0 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/DeleteKeyDialogActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/DeleteKeyDialogActivity.java @@ -31,6 +31,7 @@ import org.sufficientlysecure.keychain.provider.KeychainContract; import org.sufficientlysecure.keychain.provider.ProviderHelper; import org.sufficientlysecure.keychain.service.DeleteKeyringParcel; import org.sufficientlysecure.keychain.service.RevokeKeyringParcel; +import org.sufficientlysecure.keychain.service.input.CryptoInputParcel; import org.sufficientlysecure.keychain.ui.base.CryptoOperationHelper; import org.sufficientlysecure.keychain.ui.dialog.CustomAlertDialogBuilder; import org.sufficientlysecure.keychain.util.Log; @@ -111,7 +112,7 @@ public class DeleteKeyDialogActivity extends FragmentActivity { } private void startRevocationOperation() { - mRevokeOpHelper.cryptoOperation(); + mRevokeOpHelper.cryptoOperation(new CryptoInputParcel(false)); } private void startDeletionOperation() { 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 004d1d127..c9f6423d6 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/PassphraseDialogActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/PassphraseDialogActivity.java @@ -72,11 +72,14 @@ public class PassphraseDialogActivity extends FragmentActivity { public static final String EXTRA_REQUIRED_INPUT = "required_input"; public static final String EXTRA_SUBKEY_ID = "secret_key_id"; + public static final String EXTRA_CRYPTO_INPUT = "crypto_input"; // special extra for OpenPgpService public static final String EXTRA_SERVICE_INTENT = "data"; private long mSubKeyId; + private CryptoInputParcel mCryptoInputParcel; + @Override protected void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); @@ -90,6 +93,8 @@ public class PassphraseDialogActivity extends FragmentActivity { ); } + mCryptoInputParcel = getIntent().getParcelableExtra(EXTRA_CRYPTO_INPUT); + // this activity itself has no content view (see manifest) if (getIntent().hasExtra(EXTRA_SUBKEY_ID)) { @@ -330,11 +335,16 @@ public class PassphraseDialogActivity extends FragmentActivity { public void onClick(View v) { final Passphrase passphrase = new Passphrase(mPassphraseEditText); + CryptoInputParcel cryptoInputParcel = + ((PassphraseDialogActivity) getActivity()).mCryptoInputParcel; + // Early breakout if we are dealing with a symmetric key if (mSecretRing == null) { - PassphraseCacheService.addCachedPassphrase(getActivity(), - Constants.key.symmetric, Constants.key.symmetric, passphrase, - getString(R.string.passp_cache_notif_pwd)); + if (cryptoInputParcel.mCachePassphrase) { + PassphraseCacheService.addCachedPassphrase(getActivity(), + Constants.key.symmetric, Constants.key.symmetric, passphrase, + getString(R.string.passp_cache_notif_pwd)); + } finishCaching(passphrase); return; @@ -387,15 +397,24 @@ public class PassphraseDialogActivity extends FragmentActivity { return; } - // cache the new passphrase - Log.d(Constants.TAG, "Everything okay! Caching entered passphrase"); + // cache the new passphrase as specified in CryptoInputParcel + Log.d(Constants.TAG, "Everything okay!"); - try { - PassphraseCacheService.addCachedPassphrase(getActivity(), - mSecretRing.getMasterKeyId(), mSubKeyId, passphrase, - mSecretRing.getPrimaryUserIdWithFallback()); - } catch (PgpKeyNotFoundException e) { - Log.e(Constants.TAG, "adding of a passphrase failed", e); + CryptoInputParcel cryptoInputParcel + = ((PassphraseDialogActivity) getActivity()).mCryptoInputParcel; + + if (cryptoInputParcel.mCachePassphrase) { + Log.d(Constants.TAG, "Caching entered passphrase"); + + try { + PassphraseCacheService.addCachedPassphrase(getActivity(), + mSecretRing.getMasterKeyId(), mSubKeyId, passphrase, + mSecretRing.getPrimaryUserIdWithFallback()); + } catch (PgpKeyNotFoundException e) { + Log.e(Constants.TAG, "adding of a passphrase failed", e); + } + } else { + Log.d(Constants.TAG, "Not caching entered passphrase!"); } finishCaching(passphrase); @@ -411,9 +430,12 @@ public class PassphraseDialogActivity extends FragmentActivity { return; } - CryptoInputParcel inputParcel = new CryptoInputParcel(null, passphrase); + CryptoInputParcel inputParcel = + ((PassphraseDialogActivity) getActivity()).mCryptoInputParcel; + inputParcel.mPassphrase = passphrase; if (mServiceIntent != null) { - CryptoInputParcelCacheService.addCryptoInputParcel(getActivity(), mServiceIntent, inputParcel); + CryptoInputParcelCacheService.addCryptoInputParcel(getActivity(), mServiceIntent, + inputParcel); getActivity().setResult(RESULT_OK, mServiceIntent); } else { // also return passphrase back to activity diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/CryptoOperationHelper.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/CryptoOperationHelper.java index 4cb3d5841..398b3e778 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/CryptoOperationHelper.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/CryptoOperationHelper.java @@ -118,7 +118,7 @@ public class CryptoOperationHelper