From d81de8509be0f37a1c1d75204d1431cb8e92a1c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20Sch=C3=BCrmann?= Date: Tue, 8 Apr 2014 23:01:38 +0200 Subject: [PATCH] Fix decryption of messages/files encrypted for multiple public keys --- .../keychain/pgp/PgpDecryptVerify.java | 87 ++++++++++++------- 1 file changed, 56 insertions(+), 31 deletions(-) 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 3e04aff47..4b6aaaa4d 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpDecryptVerify.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpDecryptVerify.java @@ -200,7 +200,7 @@ public class PgpDecryptVerify { */ private PgpDecryptVerifyResult decryptVerify(InputStream in) throws IOException, PgpGeneralException, PGPException, SignatureException { - PgpDecryptVerifyResult returnData = new PgpDecryptVerifyResult(); + PgpDecryptVerifyResult result = new PgpDecryptVerifyResult(); PGPObjectFactory pgpF = new PGPObjectFactory(in); PGPEncryptedDataList enc; @@ -228,6 +228,7 @@ public class PgpDecryptVerify { PGPPBEEncryptedData encryptedDataSymmetric = null; PGPSecretKey secretKey = null; Iterator it = enc.getEncryptedDataObjects(); + boolean asymmetricPacketFound = false; boolean symmetricPacketFound = false; // find secret key while (it.hasNext()) { @@ -245,18 +246,22 @@ public class PgpDecryptVerify { ); } catch (ProviderHelper.NotFoundException e) { Log.e(Constants.TAG, "key not found!", e); + // continue with the next packet in the while loop + continue; } // get actual keyring object based on master key id PGPSecretKeyRing secretKeyRing = ProviderHelper.getPGPSecretKeyRing(mContext, masterKeyId); - if (secretKeyRing == null) { - throw new PgpGeneralException(mContext.getString(R.string.error_no_secret_key_found)); + // continue with the next packet in the while loop + continue; } secretKey = secretKeyRing.getSecretKey(encData.getKeyID()); if (secretKey == null) { - throw new PgpGeneralException(mContext.getString(R.string.error_no_secret_key_found)); + // continue with the next packet in the while loop + continue; } - // secret key exists in database + + /* secret key exists in database! */ // allow only a specific key for decryption? if (mAllowedKeyIds != null) { @@ -265,11 +270,15 @@ public class PgpDecryptVerify { Log.d(Constants.TAG, "masterKeyId: " + masterKeyId); if (!mAllowedKeyIds.contains(masterKeyId)) { - throw new PgpGeneralException( - mContext.getString(R.string.error_no_secret_key_found)); + // this key is in our db, but NOT allowed! + // continue with the next packet in the while loop + continue; } } + /* secret key exists in database and is allowed! */ + asymmetricPacketFound = true; + encryptedDataAsymmetric = encData; // if no passphrase was explicitly set try to get it from the cache service @@ -281,16 +290,20 @@ public class PgpDecryptVerify { // if passphrase was not cached, return here // indicating that a passphrase is missing! if (mPassphrase == null) { - returnData.setKeyIdPassphraseNeeded(masterKeyId); - returnData.setStatus(PgpDecryptVerifyResult.KEY_PASSHRASE_NEEDED); - return returnData; + result.setKeyIdPassphraseNeeded(masterKeyId); + result.setStatus(PgpDecryptVerifyResult.KEY_PASSHRASE_NEEDED); + return result; } } - // break out of while, only get first object here + // break out of while, only decrypt the first packet where we have a key // TODO???: There could be more pgp objects, which are not decrypted! break; } else if (mAllowSymmetricDecryption && obj instanceof PGPPBEEncryptedData) { + /* + * When mAllowSymmetricDecryption == true and we find a data packet here, + * we do not search for other available asymmetric packets! + */ symmetricPacketFound = true; encryptedDataSymmetric = (PGPPBEEncryptedData) obj; @@ -298,11 +311,11 @@ public class PgpDecryptVerify { // if no passphrase is given, return here // indicating that a passphrase is missing! if (mPassphrase == null) { - returnData.setStatus(PgpDecryptVerifyResult.SYMMETRIC_PASSHRASE_NEEDED); - return returnData; + result.setStatus(PgpDecryptVerifyResult.SYMMETRIC_PASSHRASE_NEEDED); + return result; } - // break out of while, only get first object here + // break out of while, only decrypt the first packet // TODO???: There could be more pgp objects, which are not decrypted! break; } @@ -321,11 +334,7 @@ public class PgpDecryptVerify { encryptedData = encryptedDataSymmetric; currentProgress += 5; - } else { - if (secretKey == null) { - throw new PgpGeneralException(mContext.getString(R.string.error_no_secret_key_found)); - } - + } else if (asymmetricPacketFound) { currentProgress += 5; updateProgress(R.string.progress_extracting_key, currentProgress, 100); PGPPrivateKey privateKey; @@ -351,6 +360,9 @@ public class PgpDecryptVerify { encryptedData = encryptedDataAsymmetric; currentProgress += 5; + } else { + // no packet has been found where we have the corresponding secret key in our db + throw new PgpGeneralException(mContext.getString(R.string.error_no_secret_key_found)); } PGPObjectFactory plainFact = new PGPObjectFactory(clear); @@ -379,7 +391,7 @@ public class PgpDecryptVerify { for (int i = 0; i < sigList.size(); ++i) { signature = sigList.get(i); signatureKey = ProviderHelper - .getPGPPublicKeyRing(mContext, signature.getKeyID()).getPublicKey(); + .getPGPPublicKeyRingWithKeyId(mContext, signature.getKeyID()).getPublicKey(); if (signatureKeyId == 0) { signatureKeyId = signature.getKeyID(); } @@ -408,6 +420,7 @@ public class PgpDecryptVerify { signature.init(contentVerifierBuilderProvider, signatureKey); } else { + Log.d(Constants.TAG, "SIGNATURE_UNKNOWN_PUB_KEY"); signatureResult.setStatus(OpenPgpSignatureResult.SIGNATURE_UNKNOWN_PUB_KEY); } @@ -446,6 +459,7 @@ public class PgpDecryptVerify { try { signature.update(buffer, 0, n); } catch (SignatureException e) { + Log.d(Constants.TAG, "SIGNATURE_ERROR"); signatureResult.setStatus(OpenPgpSignatureResult.SIGNATURE_ERROR); signature = null; } @@ -479,7 +493,12 @@ public class PgpDecryptVerify { // TODO: implement CERTIFIED! if (validKeyBinding & validSignature) { + Log.d(Constants.TAG, "SIGNATURE_SUCCESS_UNCERTIFIED"); signatureResult.setStatus(OpenPgpSignatureResult.SIGNATURE_SUCCESS_UNCERTIFIED); + } else { + signatureResult.setStatus(OpenPgpSignatureResult.SIGNATURE_ERROR); + Log.e(Constants.TAG, "Error!\nvalidKeyBinding: " + validKeyBinding + + "\nvalidSignature: " + validSignature); } } } @@ -503,8 +522,8 @@ public class PgpDecryptVerify { updateProgress(R.string.progress_done, 100, 100); - returnData.setSignatureResult(signatureResult); - return returnData; + result.setSignatureResult(signatureResult); + return result; } /** @@ -522,7 +541,7 @@ public class PgpDecryptVerify { */ private PgpDecryptVerifyResult verifyCleartextSignature(ArmoredInputStream aIn) throws IOException, PgpGeneralException, PGPException, SignatureException { - PgpDecryptVerifyResult returnData = new PgpDecryptVerifyResult(); + PgpDecryptVerifyResult result = new PgpDecryptVerifyResult(); OpenPgpSignatureResult signatureResult = new OpenPgpSignatureResult(); // cleartext signatures are never encrypted ;) signatureResult.setSignatureOnly(true); @@ -569,10 +588,10 @@ public class PgpDecryptVerify { // find data about this subkey HashMap data = ProviderHelper.getGenericData(mContext, KeyRings.buildUnifiedKeyRingsFindBySubkeyUri(Long.toString(signature.getKeyID())), - new String[] { KeyRings.MASTER_KEY_ID, KeyRings.USER_ID }, - new int[] { ProviderHelper.FIELD_TYPE_INTEGER, ProviderHelper.FIELD_TYPE_STRING }); + new String[]{KeyRings.MASTER_KEY_ID, KeyRings.USER_ID}, + new int[]{ProviderHelper.FIELD_TYPE_INTEGER, ProviderHelper.FIELD_TYPE_STRING}); // any luck? otherwise, try next. - if(data.get(KeyRings.MASTER_KEY_ID) == null) { + if (data.get(KeyRings.MASTER_KEY_ID) == null) { signature = null; // do NOT reset signatureKeyId, that one is shown when no known one is found! continue; @@ -588,11 +607,12 @@ public class PgpDecryptVerify { signatureResult.setKeyId(signatureKeyId); if (signature == null) { + Log.d(Constants.TAG, "SIGNATURE_UNKNOWN_PUB_KEY"); signatureResult.setStatus(OpenPgpSignatureResult.SIGNATURE_UNKNOWN_PUB_KEY); - returnData.setSignatureResult(signatureResult); + result.setSignatureResult(signatureResult); updateProgress(R.string.progress_done, 100, 100); - return returnData; + return result; } JcaPGPContentVerifierBuilderProvider contentVerifierBuilderProvider = @@ -622,16 +642,21 @@ public class PgpDecryptVerify { boolean validKeyBinding = verifyKeyBinding(mContext, signature, signatureKey); boolean validSignature = signature.verify(); - if (validSignature & validKeyBinding) { + if (validKeyBinding & validSignature) { + Log.d(Constants.TAG, "SIGNATURE_SUCCESS_UNCERTIFIED"); signatureResult.setStatus(OpenPgpSignatureResult.SIGNATURE_SUCCESS_UNCERTIFIED); + } else { + signatureResult.setStatus(OpenPgpSignatureResult.SIGNATURE_ERROR); + Log.e(Constants.TAG, "Error!\nvalidKeyBinding: " + validKeyBinding + + "\nvalidSignature: " + validSignature); } // TODO: what about SIGNATURE_SUCCESS_CERTIFIED and SIGNATURE_ERROR???? - returnData.setSignatureResult(signatureResult); + result.setSignatureResult(signatureResult); updateProgress(R.string.progress_done, 100, 100); - return returnData; + return result; } private static boolean verifyKeyBinding(Context context,