From a8e95f676e580cfbab030aa0f58167a41149d00f Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Thu, 28 May 2015 02:27:44 +0200 Subject: don't show allowed key list if no key exists, and some minor PgpDecryptVerify changes --- .../keychain/pgp/PgpEncryptDecryptTest.java | 29 ++++++++++++++--- .../operations/results/DecryptVerifyResult.java | 11 ++++--- .../operations/results/OperationResult.java | 2 +- .../keychain/pgp/PgpDecryptVerify.java | 37 ++++++++++++++-------- .../keychain/remote/OpenPgpService.java | 19 +++++------ OpenKeychain/src/main/res/values-de/strings.xml | 2 +- OpenKeychain/src/main/res/values-es/strings.xml | 2 +- OpenKeychain/src/main/res/values-fr/strings.xml | 2 +- OpenKeychain/src/main/res/values-ja/strings.xml | 2 +- OpenKeychain/src/main/res/values-nl/strings.xml | 2 +- OpenKeychain/src/main/res/values-sr/strings.xml | 2 +- .../src/main/res/values-zh-rTW/strings.xml | 2 +- OpenKeychain/src/main/res/values/strings.xml | 2 +- 13 files changed, 75 insertions(+), 39 deletions(-) diff --git a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/PgpEncryptDecryptTest.java b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/PgpEncryptDecryptTest.java index dabfb008c..8cc5115e9 100644 --- a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/PgpEncryptDecryptTest.java +++ b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/PgpEncryptDecryptTest.java @@ -29,6 +29,7 @@ import org.robolectric.shadows.ShadowLog; import org.spongycastle.bcpg.sig.KeyFlags; import org.spongycastle.jce.provider.BouncyCastleProvider; import org.spongycastle.openpgp.PGPEncryptedData; +import org.sufficientlysecure.keychain.operations.results.OperationResult.LogType; import org.sufficientlysecure.keychain.operations.results.PgpEditKeyResult; import org.sufficientlysecure.keychain.operations.results.PgpSignEncryptResult; import org.sufficientlysecure.keychain.provider.KeychainContract.KeyRingData; @@ -214,7 +215,7 @@ public class PgpEncryptDecryptTest { String plaintext = "dies ist ein plaintext ☭" + TestingUtils.genPassphrase(true); byte[] ciphertext; - { // encrypt data with a given passphrase + { // encrypt data with key ByteArrayOutputStream out = new ByteArrayOutputStream(); ByteArrayInputStream in = new ByteArrayInputStream(plaintext.getBytes()); @@ -224,7 +225,7 @@ public class PgpEncryptDecryptTest { InputData data = new InputData(in, in.available()); PgpSignEncryptInputParcel b = new PgpSignEncryptInputParcel(); - b.setEncryptionMasterKeyIds(new long[]{ mStaticRing1.getMasterKeyId() }); + b.setEncryptionMasterKeyIds(new long[] { mStaticRing1.getMasterKeyId() }); b.setSymmetricEncryptionAlgorithm(PGPEncryptedData.AES_128); PgpSignEncryptResult result = op.execute(b, new CryptoInputParcel(), data, out); Assert.assertTrue("encryption must succeed", result.success()); @@ -334,7 +335,7 @@ public class PgpEncryptDecryptTest { out.toByteArray().length, metadata.getOriginalSize()); } - { // decryption with passphrase cached should succeed for the first key + { // decryption should succeed if key is allowed ByteArrayOutputStream out = new ByteArrayOutputStream(); ByteArrayInputStream in = new ByteArrayInputStream(ciphertext); @@ -350,12 +351,32 @@ public class PgpEncryptDecryptTest { b.setAllowedKeyIds(allowed); DecryptVerifyResult result = b.build().execute(new CryptoInputParcel()); - Assert.assertTrue("decryption with cached passphrase must succeed for the first key", result.success()); + Assert.assertTrue("decryption with cached passphrase must succeed for allowed key", result.success()); Assert.assertArrayEquals("decrypted ciphertext with cached passphrase should equal plaintext", out.toByteArray(), plaintext.getBytes()); + Assert.assertTrue("other key was skipped", result.getLog().containsType(LogType.MSG_DC_ASKIP_NOT_ALLOWED)); Assert.assertNull("signature should be empty", result.getSignatureResult()); } + { // decryption should fail if no key is allowed + + ByteArrayOutputStream out = new ByteArrayOutputStream(); + ByteArrayInputStream in = new ByteArrayInputStream(ciphertext); + InputData data = new InputData(in, in.available()); + + // provide passphrase for the second, and check that the first is never asked for! + PgpDecryptVerify.Builder b = builderWithFakePassphraseCache(data, out, + mKeyPhrase2, mStaticRing2.getMasterKeyId(), null); + // no keys allowed! + b.setAllowedKeyIds(new HashSet()); + + DecryptVerifyResult result = b.build().execute(new CryptoInputParcel()); + Assert.assertFalse("decryption must fail if no key allowed", result.success()); + Assert.assertEquals("decryption must fail with key disllowed status", + DecryptVerifyResult.RESULT_KEY_DISALLOWED, result.getResult()); + + } + { // decryption with passphrase cached should succeed for the other key if first is gone // delete first key from database diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/DecryptVerifyResult.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/DecryptVerifyResult.java index 917b3415f..7680107f8 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/DecryptVerifyResult.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/DecryptVerifyResult.java @@ -27,12 +27,19 @@ import org.sufficientlysecure.keychain.util.Passphrase; public class DecryptVerifyResult extends InputPendingResult { + public static final int RESULT_NO_DATA = RESULT_ERROR + 16; + public static final int RESULT_KEY_DISALLOWED = RESULT_ERROR + 32; + OpenPgpSignatureResult mSignatureResult; OpenPgpMetadata mDecryptMetadata; // This holds the charset which was specified in the ascii armor, if specified // https://tools.ietf.org/html/rfc4880#page56 String mCharset; + public boolean isKeysDisallowed () { + return (mResult & RESULT_KEY_DISALLOWED) == RESULT_KEY_DISALLOWED; + } + public OpenPgpSignatureResult getSignatureResult() { return mSignatureResult; } @@ -57,10 +64,6 @@ public class DecryptVerifyResult extends InputPendingResult { mCharset = charset; } - public boolean isPending() { - return (mResult & RESULT_PENDING) == RESULT_PENDING; - } - public DecryptVerifyResult(int result, OperationLog log) { super(result, log); } 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 82f157d4c..54d2e6e8a 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 @@ -598,7 +598,7 @@ public abstract class OperationResult implements Parcelable { 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), MSG_DC_ERROR_INTEGRITY_MISSING (LogLevel.ERROR, R.string.msg_dc_error_integrity_missing), - MSG_DC_ERROR_INVALID_SIGLIST(LogLevel.ERROR, R.string.msg_dc_error_invalid_siglist), + MSG_DC_ERROR_INVALID_DATA (LogLevel.ERROR, R.string.msg_dc_error_invalid_data), MSG_DC_ERROR_IO (LogLevel.ERROR, R.string.msg_dc_error_io), MSG_DC_ERROR_NO_DATA (LogLevel.ERROR, R.string.msg_dc_error_no_data), MSG_DC_ERROR_NO_KEY (LogLevel.ERROR, R.string.msg_dc_error_no_key), 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 f6580b85a..aa1125800 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpDecryptVerify.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpDecryptVerify.java @@ -384,7 +384,7 @@ public class PgpDecryptVerify extends BaseOperation { } if (enc == null) { - log.add(LogType.MSG_DC_ERROR_INVALID_SIGLIST, indent); + log.add(LogType.MSG_DC_ERROR_INVALID_DATA, indent); return new DecryptVerifyResult(DecryptVerifyResult.RESULT_ERROR, log); } @@ -419,6 +419,7 @@ public class PgpDecryptVerify extends BaseOperation { } Passphrase passphrase = null; + boolean skippedDisallowedKey = false; // go through all objects and find one we can decrypt while (it.hasNext()) { @@ -451,13 +452,6 @@ public class PgpDecryptVerify extends BaseOperation { log.add(LogType.MSG_DC_ASKIP_NO_KEY, indent + 1); continue; } - // get subkey which has been used for this encryption packet - secretEncryptionKey = secretKeyRing.getSecretKey(subKeyId); - if (secretEncryptionKey == null) { - // should actually never happen, so no need to be more specific. - log.add(LogType.MSG_DC_ASKIP_NO_KEY, indent + 1); - continue; - } // allow only specific keys for decryption? if (mAllowedKeyIds != null) { @@ -469,11 +463,20 @@ public class PgpDecryptVerify extends BaseOperation { if (!mAllowedKeyIds.contains(masterKeyId)) { // this key is in our db, but NOT allowed! // continue with the next packet in the while loop + skippedDisallowedKey = true; log.add(LogType.MSG_DC_ASKIP_NOT_ALLOWED, indent + 1); continue; } } + // get subkey which has been used for this encryption packet + secretEncryptionKey = secretKeyRing.getSecretKey(subKeyId); + if (secretEncryptionKey == null) { + // should actually never happen, so no need to be more specific. + log.add(LogType.MSG_DC_ASKIP_NO_KEY, indent + 1); + continue; + } + /* secret key exists in database and is allowed! */ asymmetricPacketFound = true; @@ -604,10 +607,18 @@ public class PgpDecryptVerify extends BaseOperation { } encryptedData = encryptedDataAsymmetric; } else { - // If we didn't find any useful data, error out + // there wasn't even any useful data + if (!anyPacketFound) { + log.add(LogType.MSG_DC_ERROR_NO_DATA, indent + 1); + return new DecryptVerifyResult(DecryptVerifyResult.RESULT_NO_DATA, log); + } + // there was data but key wasn't allowed + if (skippedDisallowedKey) { + log.add(LogType.MSG_DC_ERROR_NO_KEY, indent + 1); + return new DecryptVerifyResult(DecryptVerifyResult.RESULT_KEY_DISALLOWED, log); + } // no packet has been found where we have the corresponding secret key in our db - log.add( - anyPacketFound ? LogType.MSG_DC_ERROR_NO_KEY : LogType.MSG_DC_ERROR_NO_DATA, indent + 1); + log.add(LogType.MSG_DC_ERROR_NO_KEY, indent + 1); return new DecryptVerifyResult(DecryptVerifyResult.RESULT_ERROR, log); } @@ -910,7 +921,7 @@ public class PgpDecryptVerify extends BaseOperation { PGPSignatureList sigList = (PGPSignatureList) pgpFact.nextObject(); if (sigList == null) { - log.add(LogType.MSG_DC_ERROR_INVALID_SIGLIST, 0); + log.add(LogType.MSG_DC_ERROR_INVALID_DATA, 0); return new DecryptVerifyResult(DecryptVerifyResult.RESULT_ERROR, log); } @@ -993,7 +1004,7 @@ public class PgpDecryptVerify extends BaseOperation { } else if (o instanceof PGPSignatureList) { sigList = (PGPSignatureList) o; } else { - log.add(LogType.MSG_DC_ERROR_INVALID_SIGLIST, 0); + log.add(LogType.MSG_DC_ERROR_INVALID_DATA, 0); return new DecryptVerifyResult(DecryptVerifyResult.RESULT_ERROR, log); } 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 1e2a72c3c..19ad3f610 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java @@ -34,7 +34,6 @@ import org.openintents.openpgp.util.OpenPgpApi; import org.spongycastle.bcpg.CompressionAlgorithmTags; import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.operations.results.DecryptVerifyResult; -import org.sufficientlysecure.keychain.operations.results.OperationResult; import org.sufficientlysecure.keychain.operations.results.OperationResult.LogEntryParcel; import org.sufficientlysecure.keychain.operations.results.PgpSignEncryptResult; import org.sufficientlysecure.keychain.pgp.PgpConstants; @@ -602,9 +601,8 @@ public class OpenPgpService extends RemoteService { result.putExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_SUCCESS); return result; } else { - LogEntryParcel errorMsg = pgpResult.getLog().getLast(); - - if (errorMsg.mType == OperationResult.LogType.MSG_DC_ERROR_NO_KEY) { + // + if (pgpResult.isKeysDisallowed()) { // allow user to select allowed keys Intent result = new Intent(); result.putExtra(OpenPgpApi.RESULT_INTENT, getSelectAllowedKeysIntent(data)); @@ -612,14 +610,17 @@ public class OpenPgpService extends RemoteService { return result; } - throw new Exception(getString(errorMsg.mType.getMsgId())); + String errorMsg = getString(pgpResult.getLog().getLast().mType.getMsgId()); + Intent result = new Intent(); + result.putExtra(OpenPgpApi.RESULT_ERROR, new OpenPgpError(OpenPgpError.GENERIC_ERROR, errorMsg)); + result.putExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_ERROR); + return result; } - } catch (Exception e) { - Log.d(Constants.TAG, "decryptAndVerifyImpl", e); + } catch (IOException e) { + Log.e(Constants.TAG, "decryptAndVerifyImpl", e); Intent result = new Intent(); - result.putExtra(OpenPgpApi.RESULT_ERROR, - new OpenPgpError(OpenPgpError.GENERIC_ERROR, e.getMessage())); + result.putExtra(OpenPgpApi.RESULT_ERROR, new OpenPgpError(OpenPgpError.GENERIC_ERROR, e.getMessage())); result.putExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_ERROR); return result; } finally { diff --git a/OpenKeychain/src/main/res/values-de/strings.xml b/OpenKeychain/src/main/res/values-de/strings.xml index 8b27425ae..ea04f56b9 100644 --- a/OpenKeychain/src/main/res/values-de/strings.xml +++ b/OpenKeychain/src/main/res/values-de/strings.xml @@ -930,7 +930,7 @@ Unbekannter Fehler bei Schlüsselentsperrung! Integritätsprüfungsfehler! Fehlende Integritätsprüfung Dies kann passieren, wenn die Verschlüsselungsanwendung veraltet ist oder durch einen Downgrade-Angriff. - Keine gültigen Signaturdaten gefunden! + Keine gültigen Signaturdaten gefunden! Ein-/Ausgabefehler während Vorgang aufgetreten! Keine verschlüsselten Daten in Datenstrom gefunden! Keine verschlüsselten Daten mit bekanntem geheimen Schlüssel in Datenstrom gefunden! diff --git a/OpenKeychain/src/main/res/values-es/strings.xml b/OpenKeychain/src/main/res/values-es/strings.xml index 03f1c6265..6503fe95b 100644 --- a/OpenKeychain/src/main/res/values-es/strings.xml +++ b/OpenKeychain/src/main/res/values-es/strings.xml @@ -929,7 +929,7 @@ ¡Error desconocido al desbloquear clave! ¡Error de comprobación de integridad! ¡Verificación de integridad ausente! Esto puede ocurrir porque la aplicación de cifrado no está actualizada, o debido a un ataque desactualización. - ¡No se encontraron datos de firma válidos! + ¡No se encontraron datos de firma válidos! ¡Se encontró Excepción de E/S durante la operación! ¡No se encontraron datos cifrados en el flujo de datos (`stream`)! ¡No se encontraron datos cifrados con clave secreta (privada) conocida en el flujo de datos (`stream`)! diff --git a/OpenKeychain/src/main/res/values-fr/strings.xml b/OpenKeychain/src/main/res/values-fr/strings.xml index c440cc242..17e3fcde0 100644 --- a/OpenKeychain/src/main/res/values-fr/strings.xml +++ b/OpenKeychain/src/main/res/values-fr/strings.xml @@ -929,7 +929,7 @@ Erreur inconnue de déverrouillage de la clef ! Erreur de vérification de l\'intégrité ! Vérification de l\'intégrité absente ! Ceci peut arriver car l\'application n\'est pas à jour, ou à cause d\'une attaque par mise à niveau inférieur. - Aucune donnée de signature valide trouvée ! + Aucune donnée de signature valide trouvée ! Une exception E/S a été rencontrée durant l\'opération ! Aucune donnée chiffrée n\'a été trouvée dans le flux ! Aucune donnée chiffrée avec une clef secrète connue n\'a été trouvée dans le flux ! diff --git a/OpenKeychain/src/main/res/values-ja/strings.xml b/OpenKeychain/src/main/res/values-ja/strings.xml index 18af0b482..82ca70d67 100644 --- a/OpenKeychain/src/main/res/values-ja/strings.xml +++ b/OpenKeychain/src/main/res/values-ja/strings.xml @@ -914,7 +914,7 @@ 鍵のロック解除で不明なエラー! 完全性チェックエラー! 完全聖チェックの欠落!これは暗号化アプリケーションが期限切れになった場合、もしくは暗号強度低下攻撃がある場合に発生します。 - 正常な署名データが見付からなかった! + 正常な署名データが見付からなかった! 操作中にIO例外に当たりました! ストリーム中に暗号化されたデータが見付からなかった! ストリーム中に既知の秘密鍵で暗号化されたデータが見付からなかった! diff --git a/OpenKeychain/src/main/res/values-nl/strings.xml b/OpenKeychain/src/main/res/values-nl/strings.xml index e4d9b2c1e..f5d079dab 100644 --- a/OpenKeychain/src/main/res/values-nl/strings.xml +++ b/OpenKeychain/src/main/res/values-nl/strings.xml @@ -929,7 +929,7 @@ Onbekende fout bij ontgrendelen van sleutel! Fout bij integriteitscontrole! Integriteitscheck ontbreekt! Dit kan gebeuren omdat de versleutelingsapplicatie verouderd is, of door een downgrade-aanval. - Geen geldige ondertekeningsgegevens gevonden! + Geen geldige ondertekeningsgegevens gevonden! I/O-uitzondering tegengekomen tijdens bewerking! Geen versleutelde gegevens gevonden! Geen versleutelde gegevens met bekende geheime sleutel gevonden! diff --git a/OpenKeychain/src/main/res/values-sr/strings.xml b/OpenKeychain/src/main/res/values-sr/strings.xml index ffcd55cb1..75bbdf480 100644 --- a/OpenKeychain/src/main/res/values-sr/strings.xml +++ b/OpenKeychain/src/main/res/values-sr/strings.xml @@ -916,7 +916,7 @@ Непозната грешка откључавања кључа! Грешка провере интегритета! Недостаје провера интегритета! Ово може да се деси ако је апликација за шифровање застарела, или услед напада старијег издања. - Нису нађени исправни подаци потписа! + Нису нађени исправни подаци потписа! Наиђох на У/И изузетак током радње! Шифровани подаци нису нађени у току! Подаци шифровани познатим тајним кључем нису нађени у току! diff --git a/OpenKeychain/src/main/res/values-zh-rTW/strings.xml b/OpenKeychain/src/main/res/values-zh-rTW/strings.xml index f4b28d703..1d57ef430 100644 --- a/OpenKeychain/src/main/res/values-zh-rTW/strings.xml +++ b/OpenKeychain/src/main/res/values-zh-rTW/strings.xml @@ -511,7 +511,7 @@ 找不到金鑰！ - 找不到有效的簽名資訊！ + 找不到有效的簽名資訊！ 開始解密… diff --git a/OpenKeychain/src/main/res/values/strings.xml b/OpenKeychain/src/main/res/values/strings.xml index c83fdd1a2..9b48dce21 100644 --- a/OpenKeychain/src/main/res/values/strings.xml +++ b/OpenKeychain/src/main/res/values/strings.xml @@ -1039,7 +1039,7 @@ "Unknown error unlocking key!" "Integrity check error!" "Missing integrity check! This can happen because the encrypting application is out of date, or from a downgrade attack." - "No valid signature data found!" + "No valid OpenPGP encrypted or signed data found!" "Encountered IO Exception during operation!" "No encrypted data found in stream!" "No encrypted data with known secret key found in stream!" -- cgit v1.2.3