prevented passphrase caching on revocation

This commit is contained in:
Adithya Abraham Philip
2015-07-10 02:02:27 +05:30
parent d1409fd5c8
commit faa66d6140
6 changed files with 59 additions and 17 deletions

View File

@@ -161,7 +161,7 @@ public class EditKeyOperation extends BaseOperation<SaveKeyringParcel> {
} }
// There is a new passphrase - cache it // 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); log.add(LogType.MSG_ED_CACHING_NEW, 1);
// NOTE: Don't cache empty passphrases! Important for MOVE_KEY_TO_CARD // NOTE: Don't cache empty passphrases! Important for MOVE_KEY_TO_CARD

View File

@@ -32,6 +32,9 @@ public class RevokeOperation extends BaseOperation<RevokeKeyringParcel> {
public OperationResult execute(RevokeKeyringParcel revokeKeyringParcel, public OperationResult execute(RevokeKeyringParcel revokeKeyringParcel,
CryptoInputParcel cryptoInputParcel) { CryptoInputParcel cryptoInputParcel) {
// we don't cache passphrases during revocation
cryptoInputParcel.mCachePassphrase = false;
long masterKeyId = revokeKeyringParcel.mMasterKeyId; long masterKeyId = revokeKeyringParcel.mMasterKeyId;
OperationResult.OperationLog log = new OperationResult.OperationLog(); OperationResult.OperationLog log = new OperationResult.OperationLog();

View File

@@ -34,12 +34,15 @@ import java.util.Map;
public class CryptoInputParcel implements Parcelable { public class CryptoInputParcel implements Parcelable {
final Date mSignatureTime; final Date mSignatureTime;
final Passphrase mPassphrase; public Passphrase mPassphrase;
// used to supply an explicit proxy to operations that require it // used to supply an explicit proxy to operations that require it
// this is not final so it can be added to an existing CryptoInputParcel // 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 // (e.g) CertifyOperation with upload might require both passphrase and orbot to be enabled
private ParcelableProxy mParcelableProxy; 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 // this map contains both decrypted session keys and signed hashes to be
// used in the crypto operation described by this parcel. // used in the crypto operation described by this parcel.
private HashMap<ByteBuffer, byte[]> mCryptoData = new HashMap<>(); private HashMap<ByteBuffer, byte[]> mCryptoData = new HashMap<>();
@@ -47,21 +50,25 @@ public class CryptoInputParcel implements Parcelable {
public CryptoInputParcel() { public CryptoInputParcel() {
mSignatureTime = new Date(); mSignatureTime = new Date();
mPassphrase = null; mPassphrase = null;
mCachePassphrase = true;
} }
public CryptoInputParcel(Date signatureTime, Passphrase passphrase) { public CryptoInputParcel(Date signatureTime, Passphrase passphrase) {
mSignatureTime = signatureTime == null ? new Date() : signatureTime; mSignatureTime = signatureTime == null ? new Date() : signatureTime;
mPassphrase = passphrase; mPassphrase = passphrase;
mCachePassphrase = true;
} }
public CryptoInputParcel(Passphrase passphrase) { public CryptoInputParcel(Passphrase passphrase) {
mSignatureTime = new Date(); mSignatureTime = new Date();
mPassphrase = passphrase; mPassphrase = passphrase;
mCachePassphrase = true;
} }
public CryptoInputParcel(Date signatureTime) { public CryptoInputParcel(Date signatureTime) {
mSignatureTime = signatureTime == null ? new Date() : signatureTime; mSignatureTime = signatureTime == null ? new Date() : signatureTime;
mPassphrase = null; mPassphrase = null;
mCachePassphrase = true;
} }
public CryptoInputParcel(ParcelableProxy parcelableProxy) { public CryptoInputParcel(ParcelableProxy parcelableProxy) {
@@ -69,10 +76,17 @@ public class CryptoInputParcel implements Parcelable {
mParcelableProxy = parcelableProxy; mParcelableProxy = parcelableProxy;
} }
public CryptoInputParcel(boolean cachePassphrase) {
mSignatureTime = new Date();
mPassphrase = null;
mCachePassphrase = cachePassphrase;
}
protected CryptoInputParcel(Parcel source) { protected CryptoInputParcel(Parcel source) {
mSignatureTime = new Date(source.readLong()); mSignatureTime = new Date(source.readLong());
mPassphrase = source.readParcelable(getClass().getClassLoader()); mPassphrase = source.readParcelable(getClass().getClassLoader());
mParcelableProxy = source.readParcelable(getClass().getClassLoader()); mParcelableProxy = source.readParcelable(getClass().getClassLoader());
mCachePassphrase = source.readByte() != 0;
{ {
int count = source.readInt(); int count = source.readInt();
@@ -96,6 +110,7 @@ public class CryptoInputParcel implements Parcelable {
dest.writeLong(mSignatureTime.getTime()); dest.writeLong(mSignatureTime.getTime());
dest.writeParcelable(mPassphrase, 0); dest.writeParcelable(mPassphrase, 0);
dest.writeParcelable(mParcelableProxy, 0); dest.writeParcelable(mParcelableProxy, 0);
dest.writeByte((byte) (mCachePassphrase ? 1 : 0));
dest.writeInt(mCryptoData.size()); dest.writeInt(mCryptoData.size());
for (HashMap.Entry<ByteBuffer, byte[]> entry : mCryptoData.entrySet()) { for (HashMap.Entry<ByteBuffer, byte[]> entry : mCryptoData.entrySet()) {

View File

@@ -31,6 +31,7 @@ import org.sufficientlysecure.keychain.provider.KeychainContract;
import org.sufficientlysecure.keychain.provider.ProviderHelper; import org.sufficientlysecure.keychain.provider.ProviderHelper;
import org.sufficientlysecure.keychain.service.DeleteKeyringParcel; import org.sufficientlysecure.keychain.service.DeleteKeyringParcel;
import org.sufficientlysecure.keychain.service.RevokeKeyringParcel; 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.base.CryptoOperationHelper;
import org.sufficientlysecure.keychain.ui.dialog.CustomAlertDialogBuilder; import org.sufficientlysecure.keychain.ui.dialog.CustomAlertDialogBuilder;
import org.sufficientlysecure.keychain.util.Log; import org.sufficientlysecure.keychain.util.Log;
@@ -111,7 +112,7 @@ public class DeleteKeyDialogActivity extends FragmentActivity {
} }
private void startRevocationOperation() { private void startRevocationOperation() {
mRevokeOpHelper.cryptoOperation(); mRevokeOpHelper.cryptoOperation(new CryptoInputParcel(false));
} }
private void startDeletionOperation() { private void startDeletionOperation() {

View File

@@ -72,11 +72,14 @@ public class PassphraseDialogActivity extends FragmentActivity {
public static final String EXTRA_REQUIRED_INPUT = "required_input"; public static final String EXTRA_REQUIRED_INPUT = "required_input";
public static final String EXTRA_SUBKEY_ID = "secret_key_id"; public static final String EXTRA_SUBKEY_ID = "secret_key_id";
public static final String EXTRA_CRYPTO_INPUT = "crypto_input";
// special extra for OpenPgpService // special extra for OpenPgpService
public static final String EXTRA_SERVICE_INTENT = "data"; public static final String EXTRA_SERVICE_INTENT = "data";
private long mSubKeyId; private long mSubKeyId;
private CryptoInputParcel mCryptoInputParcel;
@Override @Override
protected void onCreate(Bundle savedInstanceState) { protected void onCreate(Bundle savedInstanceState) {
super.onCreate(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) // this activity itself has no content view (see manifest)
if (getIntent().hasExtra(EXTRA_SUBKEY_ID)) { if (getIntent().hasExtra(EXTRA_SUBKEY_ID)) {
@@ -330,11 +335,16 @@ public class PassphraseDialogActivity extends FragmentActivity {
public void onClick(View v) { public void onClick(View v) {
final Passphrase passphrase = new Passphrase(mPassphraseEditText); final Passphrase passphrase = new Passphrase(mPassphraseEditText);
CryptoInputParcel cryptoInputParcel =
((PassphraseDialogActivity) getActivity()).mCryptoInputParcel;
// Early breakout if we are dealing with a symmetric key // Early breakout if we are dealing with a symmetric key
if (mSecretRing == null) { if (mSecretRing == null) {
PassphraseCacheService.addCachedPassphrase(getActivity(), if (cryptoInputParcel.mCachePassphrase) {
Constants.key.symmetric, Constants.key.symmetric, passphrase, PassphraseCacheService.addCachedPassphrase(getActivity(),
getString(R.string.passp_cache_notif_pwd)); Constants.key.symmetric, Constants.key.symmetric, passphrase,
getString(R.string.passp_cache_notif_pwd));
}
finishCaching(passphrase); finishCaching(passphrase);
return; return;
@@ -387,15 +397,24 @@ public class PassphraseDialogActivity extends FragmentActivity {
return; return;
} }
// cache the new passphrase // cache the new passphrase as specified in CryptoInputParcel
Log.d(Constants.TAG, "Everything okay! Caching entered passphrase"); Log.d(Constants.TAG, "Everything okay!");
try { CryptoInputParcel cryptoInputParcel
PassphraseCacheService.addCachedPassphrase(getActivity(), = ((PassphraseDialogActivity) getActivity()).mCryptoInputParcel;
mSecretRing.getMasterKeyId(), mSubKeyId, passphrase,
mSecretRing.getPrimaryUserIdWithFallback()); if (cryptoInputParcel.mCachePassphrase) {
} catch (PgpKeyNotFoundException e) { Log.d(Constants.TAG, "Caching entered passphrase");
Log.e(Constants.TAG, "adding of a passphrase failed", e);
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); finishCaching(passphrase);
@@ -411,9 +430,12 @@ public class PassphraseDialogActivity extends FragmentActivity {
return; return;
} }
CryptoInputParcel inputParcel = new CryptoInputParcel(null, passphrase); CryptoInputParcel inputParcel =
((PassphraseDialogActivity) getActivity()).mCryptoInputParcel;
inputParcel.mPassphrase = passphrase;
if (mServiceIntent != null) { if (mServiceIntent != null) {
CryptoInputParcelCacheService.addCryptoInputParcel(getActivity(), mServiceIntent, inputParcel); CryptoInputParcelCacheService.addCryptoInputParcel(getActivity(), mServiceIntent,
inputParcel);
getActivity().setResult(RESULT_OK, mServiceIntent); getActivity().setResult(RESULT_OK, mServiceIntent);
} else { } else {
// also return passphrase back to activity // also return passphrase back to activity

View File

@@ -118,7 +118,7 @@ public class CryptoOperationHelper<T extends Parcelable, S extends OperationResu
Activity activity = mUseFragment ? mFragment.getActivity() : mActivity; Activity activity = mUseFragment ? mFragment.getActivity() : mActivity;
switch (requiredInput.mType) { switch (requiredInput.mType) {
// TODO: Verify that all started activities add to cryptoInputParcel if necessary (like OrbotRequiredDialogActivity) // TODO: make NfcOperationActivity add to cryptoInputParcel
// always use CryptoOperationHelper.startActivityForResult! // always use CryptoOperationHelper.startActivityForResult!
case NFC_MOVE_KEY_TO_CARD: case NFC_MOVE_KEY_TO_CARD:
case NFC_DECRYPT: case NFC_DECRYPT:
@@ -133,6 +133,7 @@ public class CryptoOperationHelper<T extends Parcelable, S extends OperationResu
case PASSPHRASE_SYMMETRIC: { case PASSPHRASE_SYMMETRIC: {
Intent intent = new Intent(activity, PassphraseDialogActivity.class); Intent intent = new Intent(activity, PassphraseDialogActivity.class);
intent.putExtra(PassphraseDialogActivity.EXTRA_REQUIRED_INPUT, requiredInput); intent.putExtra(PassphraseDialogActivity.EXTRA_REQUIRED_INPUT, requiredInput);
intent.putExtra(PassphraseDialogActivity.EXTRA_CRYPTO_INPUT, cryptoInputParcel);
startActivityForResult(intent, REQUEST_CODE_PASSPHRASE); startActivityForResult(intent, REQUEST_CODE_PASSPHRASE);
return; return;
} }