From 5633fcc92f20ecd81e4a4ea7f22416f58d051c49 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Wed, 8 Jul 2015 04:36:43 +0200 Subject: fix symmetric passphrase caching (fixes #1401) --- .../keychain/operations/BaseOperation.java | 8 ++++-- .../operations/results/OperationResult.java | 1 + .../keychain/pgp/PgpDecryptVerify.java | 32 ++++++++++++++++++---- .../keychain/service/PassphraseCacheService.java | 16 +++++------ OpenKeychain/src/main/res/values/strings.xml | 1 + 5 files changed, 42 insertions(+), 16 deletions(-) (limited to 'OpenKeychain') 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 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 // 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 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 @@ "Saving signature data for later" "Processing cleartext data" "Error unlocking key, bad password!" + "Error decrypting data! (Bad passphrase?)" "Data is corrupt!" "Unknown error unlocking key!" "Integrity check error!" -- cgit v1.2.3