From 16a0045f9d6b817732b839966e203e85ecae267d Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Tue, 4 Apr 2017 16:37:34 +0200 Subject: [PATCH 1/4] use external provider for resolving encryption keys in api --- .../provider/KeychainExternalContract.java | 3 +- .../remote/KeychainExternalProvider.java | 26 +- .../keychain/remote/OpenPgpService.java | 79 ++--- .../remote/OpenPgpServiceKeyIdExtractor.java | 275 +++++++++++------- .../remote/KeychainExternalProviderTest.java | 19 +- .../OpenPgpServiceKeyIdExtractorTest.java | 150 ++++------ 6 files changed, 312 insertions(+), 240 deletions(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainExternalContract.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainExternalContract.java index 733b7794f..d1042ba0d 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainExternalContract.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainExternalContract.java @@ -37,8 +37,9 @@ public class KeychainExternalContract { public static class EmailStatus implements BaseColumns { public static final String EMAIL_ADDRESS = "email_address"; - public static final String EMAIL_STATUS = "email_status"; public static final String USER_ID = "user_id"; + public static final String USER_ID_STATUS = "email_status"; + public static final String MASTER_KEY_ID = "master_key_id"; public static final Uri CONTENT_URI = BASE_CONTENT_URI_EXTERNAL.buildUpon() .appendPath(BASE_EMAIL_STATUS).build(); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/KeychainExternalProvider.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/KeychainExternalProvider.java index 975392d22..8e4bf7a6c 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/KeychainExternalProvider.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/KeychainExternalProvider.java @@ -34,6 +34,7 @@ import android.os.Binder; import android.support.annotation.NonNull; import android.text.TextUtils; +import org.sufficientlysecure.keychain.BuildConfig; import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.provider.ApiDataAccessObject; import org.sufficientlysecure.keychain.provider.KeychainContract; @@ -50,6 +51,7 @@ import org.sufficientlysecure.keychain.util.Log; public class KeychainExternalProvider extends ContentProvider implements SimpleContentResolverInterface { private static final int EMAIL_STATUS = 101; + private static final int EMAIL_STATUS_INTERNAL = 102; private static final int API_APPS = 301; private static final int API_APPS_BY_PACKAGE_NAME = 302; @@ -78,8 +80,9 @@ public class KeychainExternalProvider extends ContentProvider implements SimpleC * */ matcher.addURI(authority, KeychainExternalContract.BASE_EMAIL_STATUS, EMAIL_STATUS); + matcher.addURI(authority, KeychainExternalContract.BASE_EMAIL_STATUS + "/*", EMAIL_STATUS_INTERNAL); - matcher.addURI(KeychainContract.CONTENT_AUTHORITY, KeychainContract.BASE_API_APPS, API_APPS); + // can only query status of calling app - for internal use only! matcher.addURI(KeychainContract.CONTENT_AUTHORITY, KeychainContract.BASE_API_APPS + "/*", API_APPS_BY_PACKAGE_NAME); return matcher; @@ -134,9 +137,19 @@ public class KeychainExternalProvider extends ContentProvider implements SimpleC SQLiteDatabase db = getDb().getReadableDatabase(); + String callingPackageName = mApiPermissionHelper.getCurrentCallingPackage(); + switch (match) { + case EMAIL_STATUS_INTERNAL: + if (!BuildConfig.APPLICATION_ID.equals(callingPackageName)) { + throw new AccessControlException("This URI can only be called internally!"); + } + + // override package name to use any external + // callingPackageName = uri.getLastPathSegment(); + case EMAIL_STATUS: { - boolean callerIsAllowed = mApiPermissionHelper.isAllowedIgnoreErrors(); + boolean callerIsAllowed = (match == EMAIL_STATUS_INTERNAL) || mApiPermissionHelper.isAllowedIgnoreErrors(); if (!callerIsAllowed) { throw new AccessControlException("An application must register before use of KeychainExternalProvider!"); } @@ -152,14 +165,19 @@ public class KeychainExternalProvider extends ContentProvider implements SimpleC projectionMap.put(EmailStatus._ID, "email AS _id"); projectionMap.put(EmailStatus.EMAIL_ADDRESS, // this is actually the queried address TEMP_TABLE_QUERIED_ADDRESSES + "." + TEMP_TABLE_COLUMN_ADDRES + " AS " + EmailStatus.EMAIL_ADDRESS); + projectionMap.put(EmailStatus.USER_ID, + Tables.USER_PACKETS + "." + UserPackets.USER_ID + " AS " + EmailStatus.USER_ID); // we take the minimum (>0) here, where "1" is "verified by known secret key", "2" is "self-certified" - projectionMap.put(EmailStatus.EMAIL_STATUS, "CASE ( MIN (" + Certs.VERIFIED + " ) ) " + projectionMap.put(EmailStatus.USER_ID_STATUS, "CASE ( MIN (" + Certs.VERIFIED + " ) ) " // remap to keep this provider contract independent from our internal representation + " WHEN NULL THEN 1" + " WHEN " + Certs.VERIFIED_SELF + " THEN 1" + " WHEN " + Certs.VERIFIED_SECRET + " THEN 2" - + " END AS " + EmailStatus.EMAIL_STATUS); + + " WHEN NULL THEN NULL" + + " END AS " + EmailStatus.USER_ID_STATUS); projectionMap.put(EmailStatus.USER_ID, Tables.USER_PACKETS + "." + UserPackets.USER_ID + " AS " + EmailStatus.USER_ID); + projectionMap.put(EmailStatus.MASTER_KEY_ID, + Tables.USER_PACKETS + "." + UserPackets.MASTER_KEY_ID + " AS " + EmailStatus.MASTER_KEY_ID); qb.setProjectionMap(projectionMap); if (projection == null) { 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 252124043..ea0e5bed2 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java @@ -66,6 +66,7 @@ import org.sufficientlysecure.keychain.provider.KeyRepository; import org.sufficientlysecure.keychain.provider.KeychainContract; import org.sufficientlysecure.keychain.provider.KeychainContract.KeyRings; import org.sufficientlysecure.keychain.remote.OpenPgpServiceKeyIdExtractor.KeyIdResult; +import org.sufficientlysecure.keychain.remote.OpenPgpServiceKeyIdExtractor.KeyIdResultStatus; import org.sufficientlysecure.keychain.service.BackupKeyringParcel; import org.sufficientlysecure.keychain.service.input.CryptoInputParcel; import org.sufficientlysecure.keychain.service.input.RequiredInputParcel; @@ -213,51 +214,52 @@ public class OpenPgpService extends Service { compressionId = PgpSecurityConstants.OpenKeychainCompressionAlgorithmTags.UNCOMPRESSED; } - KeyIdResult keyIdResult = mKeyIdExtractor.returnKeyIdsFromIntent(data, false); - if (keyIdResult.hasResultIntent()) { - return keyIdResult.getResultIntent(); - } - long[] keyIds = keyIdResult.getKeyIds(); - - // TODO this is not correct! - long inputLength = inputStream.available(); - InputData inputData = new InputData(inputStream, inputLength, originalFilename); - PgpSignEncryptData pgpData = new PgpSignEncryptData(); pgpData.setEnableAsciiArmorOutput(asciiArmor) .setVersionHeader(null) - .setCompressionAlgorithm(compressionId) - .setSymmetricEncryptionAlgorithm(PgpSecurityConstants.OpenKeychainSymmetricKeyAlgorithmTags.USE_DEFAULT) - .setEncryptionMasterKeyIds(keyIds); + .setCompressionAlgorithm(compressionId); if (sign) { - Intent signKeyIdIntent = getSignKeyMasterId(data); // NOTE: Fallback to return account settings (Old API) if (signKeyIdIntent.getIntExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_ERROR) - == OpenPgpApi.RESULT_CODE_USER_INTERACTION_REQUIRED) { + != OpenPgpApi.RESULT_CODE_SUCCESS) { return signKeyIdIntent; } + long signKeyId = signKeyIdIntent.getLongExtra(OpenPgpApi.EXTRA_SIGN_KEY_ID, Constants.key.none); if (signKeyId == Constants.key.none) { throw new Exception("No signing key given"); - } else { - pgpData.setSignatureMasterKeyId(signKeyId); + } + long signSubKeyId = mKeyRepository.getCachedPublicKeyRing(signKeyId).getSecretSignId(); - // get first usable subkey capable of signing - try { - long signSubKeyId = mKeyRepository.getCachedPublicKeyRing( - pgpData.getSignatureMasterKeyId()).getSecretSignId(); - pgpData.setSignatureSubKeyId(signSubKeyId); - } catch (PgpKeyNotFoundException e) { - throw new Exception("signing subkey not found!", e); - } + pgpData.setSignatureMasterKeyId(signKeyId) + .setSignatureSubKeyId(signSubKeyId) + .setAdditionalEncryptId(signKeyId); + } + + KeyIdResult keyIdResult = mKeyIdExtractor.returnKeyIdsFromIntent(data, false, + mApiPermissionHelper.getCurrentCallingPackage()); + + boolean isOpportunistic = data.getBooleanExtra(OpenPgpApi.EXTRA_OPPORTUNISTIC_ENCRYPTION, false); + KeyIdResultStatus keyIdResultStatus = keyIdResult.getStatus(); + + if (keyIdResult.hasKeySelectionPendingIntent()) { + if ((keyIdResultStatus == KeyIdResultStatus.MISSING || keyIdResultStatus == KeyIdResultStatus.NO_KEYS || + keyIdResultStatus == KeyIdResultStatus.NO_KEYS_ERROR) && isOpportunistic) { + Intent result = new Intent(); + result.putExtra(OpenPgpApi.RESULT_ERROR, + new OpenPgpError(OpenPgpError.OPPORTUNISTIC_MISSING_KEYS, "missing keys in opportunistic mode")); + result.putExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_ERROR); + return result; } - // sign and encrypt - pgpData.setSignatureHashAlgorithm(PgpSecurityConstants.OpenKeychainHashAlgorithmTags.USE_DEFAULT) - .setAdditionalEncryptId(signKeyId); // add sign key for encryption + Intent result = new Intent(); + result.putExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_USER_INTERACTION_REQUIRED); + result.putExtra(OpenPgpApi.RESULT_INTENT, keyIdResult.getKeySelectionPendingIntent()); + return result; } + pgpData.setEncryptionMasterKeyIds(keyIdResult.getKeyIds()); PgpSignEncryptInputParcel pseInput = new PgpSignEncryptInputParcel(pgpData); pseInput.setAllowedKeyIds(getAllowedKeyIds()); @@ -268,13 +270,15 @@ public class OpenPgpService extends Service { } // override passphrase in input parcel if given by API call if (data.hasExtra(OpenPgpApi.EXTRA_PASSPHRASE)) { - inputParcel.mPassphrase = - new Passphrase(data.getCharArrayExtra(OpenPgpApi.EXTRA_PASSPHRASE)); + inputParcel.mPassphrase = new Passphrase(data.getCharArrayExtra(OpenPgpApi.EXTRA_PASSPHRASE)); } - PgpSignEncryptOperation op = new PgpSignEncryptOperation(this, mKeyRepository, null); + // TODO this is not correct! + long inputLength = inputStream.available(); + InputData inputData = new InputData(inputStream, inputLength, originalFilename); // execute PGP operation! + PgpSignEncryptOperation op = new PgpSignEncryptOperation(this, mKeyRepository, null); PgpSignEncryptResult pgpResult = op.execute(pseInput, inputParcel, inputData, outputStream); if (pgpResult.isPending()) { @@ -544,8 +548,7 @@ public class OpenPgpService extends Service { // if data already contains EXTRA_SIGN_KEY_ID, it has been executed again // after user interaction. Then, we just need to return the long again! if (data.hasExtra(OpenPgpApi.EXTRA_SIGN_KEY_ID)) { - long signKeyId = data.getLongExtra(OpenPgpApi.EXTRA_SIGN_KEY_ID, - Constants.key.none); + long signKeyId = data.getLongExtra(OpenPgpApi.EXTRA_SIGN_KEY_ID, Constants.key.none); Intent result = new Intent(); result.putExtra(OpenPgpApi.EXTRA_SIGN_KEY_ID, signKeyId); @@ -567,9 +570,13 @@ public class OpenPgpService extends Service { } private Intent getKeyIdsImpl(Intent data) { - KeyIdResult keyIdResult = mKeyIdExtractor.returnKeyIdsFromIntent(data, true); - if (keyIdResult.hasResultIntent()) { - return keyIdResult.getResultIntent(); + KeyIdResult keyIdResult = mKeyIdExtractor.returnKeyIdsFromIntent(data, true, + mApiPermissionHelper.getCurrentCallingPackage()); + if (keyIdResult.hasKeySelectionPendingIntent()) { + Intent result = new Intent(); + result.putExtra(OpenPgpApi.RESULT_INTENT, keyIdResult.getKeySelectionPendingIntent()); + result.putExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_USER_INTERACTION_REQUIRED); + return result; } long[] keyIds = keyIdResult.getKeyIds(); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpServiceKeyIdExtractor.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpServiceKeyIdExtractor.java index 3725df272..5a88e4154 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpServiceKeyIdExtractor.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpServiceKeyIdExtractor.java @@ -2,7 +2,9 @@ package org.sufficientlysecure.keychain.remote; import java.util.ArrayList; +import java.util.HashMap; import java.util.HashSet; +import java.util.Map.Entry; import android.app.PendingIntent; import android.content.ContentResolver; @@ -11,31 +13,23 @@ import android.database.Cursor; import android.net.Uri; import android.support.annotation.VisibleForTesting; -import org.openintents.openpgp.OpenPgpError; import org.openintents.openpgp.util.OpenPgpApi; -import org.openintents.openpgp.util.OpenPgpUtils; import org.sufficientlysecure.keychain.Constants; -import org.sufficientlysecure.keychain.pgp.KeyRing; -import org.sufficientlysecure.keychain.provider.KeychainContract; -import org.sufficientlysecure.keychain.provider.KeychainContract.KeyRings; -import org.sufficientlysecure.keychain.provider.KeychainDatabase.Tables; +import org.sufficientlysecure.keychain.provider.KeychainExternalContract.EmailStatus; import org.sufficientlysecure.keychain.ui.util.KeyFormattingUtils; import org.sufficientlysecure.keychain.util.Log; class OpenPgpServiceKeyIdExtractor { @VisibleForTesting - static final String[] KEY_SEARCH_PROJECTION = new String[]{ - KeyRings._ID, - KeyRings.MASTER_KEY_ID, - KeyRings.IS_EXPIRED, // referenced in where clause! - KeyRings.IS_REVOKED, // referenced in where clause! + static final String[] PROJECTION_KEY_SEARCH = { + "email_address", + "master_key_id", + "email_status", }; + private static final int INDEX_EMAIL_ADDRESS = 0; private static final int INDEX_MASTER_KEY_ID = 1; - - // do not pre-select revoked or expired keys - private static final String KEY_SEARCH_WHERE = Tables.KEYS + "." + KeychainContract.KeyRings.IS_REVOKED - + " = 0 AND " + KeychainContract.KeyRings.IS_EXPIRED + " = 0"; + private static final int INDEX_EMAIL_STATUS = 2; private final ApiPendingIntentFactory apiPendingIntentFactory; @@ -53,150 +47,223 @@ class OpenPgpServiceKeyIdExtractor { } - KeyIdResult returnKeyIdsFromIntent(Intent data, boolean askIfNoUserIdsProvided) { - HashSet encryptKeyIds = new HashSet<>(); - + KeyIdResult returnKeyIdsFromIntent(Intent data, boolean askIfNoUserIdsProvided, String callingPackageName) { boolean hasKeysFromSelectPubkeyActivity = data.hasExtra(OpenPgpApi.EXTRA_KEY_IDS_SELECTED); + + KeyIdResult result; if (hasKeysFromSelectPubkeyActivity) { + HashSet encryptKeyIds = new HashSet<>(); for (long keyId : data.getLongArrayExtra(OpenPgpApi.EXTRA_KEY_IDS_SELECTED)) { encryptKeyIds.add(keyId); } + result = createKeysOkResult(encryptKeyIds, false); } else if (data.hasExtra(OpenPgpApi.EXTRA_USER_IDS) || askIfNoUserIdsProvided) { String[] userIds = data.getStringArrayExtra(OpenPgpApi.EXTRA_USER_IDS); - boolean isOpportunistic = data.getBooleanExtra(OpenPgpApi.EXTRA_OPPORTUNISTIC_ENCRYPTION, false); - KeyIdResult result = returnKeyIdsFromEmails(data, userIds, isOpportunistic); - - if (result.mResultIntent != null) { - return result; - } - encryptKeyIds.addAll(result.mKeyIds); + result = returnKeyIdsFromEmails(data, userIds, callingPackageName); + } else { + result = createNoKeysResult(); } // add key ids from non-ambiguous key id extra if (data.hasExtra(OpenPgpApi.EXTRA_KEY_IDS)) { + HashSet explicitKeyIds = new HashSet<>(); for (long keyId : data.getLongArrayExtra(OpenPgpApi.EXTRA_KEY_IDS)) { - encryptKeyIds.add(keyId); + explicitKeyIds.add(keyId); } + result = result.withExplicitKeyIds(explicitKeyIds); } - if (encryptKeyIds.isEmpty()) { - Intent result = new Intent(); - result.putExtra(OpenPgpApi.RESULT_ERROR, - new OpenPgpError(OpenPgpError.NO_USER_IDS, "No encryption keys or user ids specified!" + - "(pass empty user id array to get dialog without preselection)")); - result.putExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_ERROR); - return new KeyIdResult(result); - } - - return new KeyIdResult(encryptKeyIds); + return result; } - private KeyIdResult returnKeyIdsFromEmails(Intent data, String[] encryptionUserIds, boolean isOpportunistic) { + private KeyIdResult returnKeyIdsFromEmails(Intent data, String[] encryptionUserIds, String callingPackageName) { boolean hasUserIds = (encryptionUserIds != null && encryptionUserIds.length > 0); + boolean anyKeyNotVerified = false; HashSet keyIds = new HashSet<>(); ArrayList missingEmails = new ArrayList<>(); ArrayList duplicateEmails = new ArrayList<>(); + HashMap keyRows = new HashMap<>(); if (hasUserIds) { - for (String rawUserId : encryptionUserIds) { - OpenPgpUtils.UserId userId = KeyRing.splitUserId(rawUserId); - String email = userId.email != null ? userId.email : rawUserId; - // try to find the key for this specific email - Uri uri = KeyRings.buildUnifiedKeyRingsFindByEmailUri(email); - Cursor cursor = contentResolver.query(uri, KEY_SEARCH_PROJECTION, KEY_SEARCH_WHERE, null, null); - if (cursor == null) { - throw new IllegalStateException("Internal error, received null cursor!"); - } - try { - // result should be one entry containing the key id - if (cursor.moveToFirst()) { - long id = cursor.getLong(INDEX_MASTER_KEY_ID); - keyIds.add(id); + Uri queryUri = EmailStatus.CONTENT_URI.buildUpon().appendPath(callingPackageName).build(); + Cursor cursor = contentResolver.query(queryUri, PROJECTION_KEY_SEARCH, null, encryptionUserIds, null); + if (cursor == null) { + throw new IllegalStateException("Internal error, received null cursor!"); + } - // another entry for this email -> two keys with the same email inside user id - if (!cursor.isLast()) { - Log.d(Constants.TAG, "more than one user id with the same email"); - duplicateEmails.add(email); + try { + while (cursor.moveToNext()) { + String queryAddress = cursor.getString(INDEX_EMAIL_ADDRESS); + Long masterKeyId = cursor.isNull(INDEX_MASTER_KEY_ID) ? null : cursor.getLong(INDEX_MASTER_KEY_ID); + int verified = cursor.getInt(INDEX_EMAIL_STATUS); - // also pre-select - while (cursor.moveToNext()) { - long duplicateId = cursor.getLong(INDEX_MASTER_KEY_ID); - keyIds.add(duplicateId); - } - } - } else { - missingEmails.add(email); - Log.d(Constants.TAG, "user id missing"); + KeyRow row = new KeyRow(masterKeyId, verified == 2); + if (!keyRows.containsKey(queryAddress)) { + keyRows.put(queryAddress, row); + continue; + } + + KeyRow previousRow = keyRows.get(queryAddress); + if (previousRow.masterKeyId == null) { + keyRows.put(queryAddress, row); + } else if (!previousRow.verified && row.verified) { + keyRows.put(queryAddress, row); + } else if (previousRow.verified == row.verified) { + previousRow.hasDuplicate = true; } - } finally { - cursor.close(); } + } finally { + cursor.close(); + } + + for (Entry entry : keyRows.entrySet()) { + String queriedAddress = entry.getKey(); + KeyRow keyRow = entry.getValue(); + + if (keyRow.masterKeyId == null) { + missingEmails.add(queriedAddress); + continue; + } + + keyIds.add(keyRow.masterKeyId); + + if (keyRow.hasDuplicate) { + duplicateEmails.add(queriedAddress); + } + + if (!keyRow.verified) { + anyKeyNotVerified = true; + } + } + + if (keyRows.size() != encryptionUserIds.length) { + Log.e(Constants.TAG, "Number of rows doesn't match number of retrieved rows! Probably a bug?"); } } - boolean hasMissingUserIds = !missingEmails.isEmpty(); - boolean hasDuplicateUserIds = !duplicateEmails.isEmpty(); - if (isOpportunistic && (!hasUserIds || hasMissingUserIds)) { - Intent result = new Intent(); - result.putExtra(OpenPgpApi.RESULT_ERROR, - new OpenPgpError(OpenPgpError.OPPORTUNISTIC_MISSING_KEYS, "missing keys in opportunistic mode")); - result.putExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_ERROR); - return new KeyIdResult(result); + long[] keyIdsArray = KeyFormattingUtils.getUnboxedLongArray(keyIds); + PendingIntent pi = apiPendingIntentFactory.createSelectPublicKeyPendingIntent(data, keyIdsArray, + missingEmails, duplicateEmails, false); + + if (!missingEmails.isEmpty()) { + return createMissingKeysResult(pi); } - if (!hasUserIds || hasMissingUserIds || hasDuplicateUserIds) { - long[] keyIdsArray = KeyFormattingUtils.getUnboxedLongArray(keyIds); - PendingIntent pi = apiPendingIntentFactory.createSelectPublicKeyPendingIntent(data, keyIdsArray, - missingEmails, duplicateEmails, hasUserIds); - - // return PendingIntent to be executed by client - Intent result = new Intent(); - result.putExtra(OpenPgpApi.RESULT_INTENT, pi); - result.putExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_USER_INTERACTION_REQUIRED); - return new KeyIdResult(result); + if (!duplicateEmails.isEmpty()) { + return createDuplicateKeysResult(pi); } if (keyIds.isEmpty()) { - throw new AssertionError("keyIdsArray.length == 0, should never happen!"); + return createNoKeysResult(pi); } - return new KeyIdResult(keyIds); + boolean allKeysConfirmed = !anyKeyNotVerified; + return createKeysOkResult(keyIds, allKeysConfirmed); + } + + private static class KeyRow { + private final Long masterKeyId; + private final boolean verified; + private boolean hasDuplicate; + + KeyRow(Long masterKeyId, boolean verified) { + this.masterKeyId = masterKeyId; + this.verified = verified; + } } static class KeyIdResult { - private final Intent mResultIntent; - private final HashSet mKeyIds; + private final PendingIntent mKeySelectionPendingIntent; + private final HashSet mUserKeyIds; + private final HashSet mExplicitKeyIds; + private final KeyIdResultStatus mStatus; + private final boolean mAllKeysConfirmed; - private KeyIdResult(Intent resultIntent) { - mResultIntent = resultIntent; - mKeyIds = null; + private KeyIdResult(PendingIntent keySelectionPendingIntent, KeyIdResultStatus keyIdResultStatus) { + mKeySelectionPendingIntent = keySelectionPendingIntent; + mUserKeyIds = null; + mAllKeysConfirmed = false; + mStatus = keyIdResultStatus; + mExplicitKeyIds = null; } - private KeyIdResult(HashSet keyIds) { - mResultIntent = null; - mKeyIds = keyIds; + private KeyIdResult(HashSet keyIds, boolean allKeysConfirmed, KeyIdResultStatus keyIdResultStatus) { + mKeySelectionPendingIntent = null; + mUserKeyIds = keyIds; + mAllKeysConfirmed = allKeysConfirmed; + mStatus = keyIdResultStatus; + mExplicitKeyIds = null; } - boolean hasResultIntent() { - return mResultIntent != null; + private KeyIdResult(KeyIdResult keyIdResult, HashSet explicitKeyIds) { + mKeySelectionPendingIntent = keyIdResult.mKeySelectionPendingIntent; + mUserKeyIds = keyIdResult.mUserKeyIds; + mAllKeysConfirmed = keyIdResult.mAllKeysConfirmed; + mStatus = keyIdResult.mStatus; + mExplicitKeyIds = explicitKeyIds; } - Intent getResultIntent() { - if (mResultIntent == null) { + + boolean hasKeySelectionPendingIntent() { + return mKeySelectionPendingIntent != null; + } + + PendingIntent getKeySelectionPendingIntent() { + if (mKeySelectionPendingIntent == null) { throw new AssertionError("result intent must not be null when getResultIntent is called!"); } - if (mKeyIds != null) { + if (mUserKeyIds != null) { throw new AssertionError("key ids must be null when getKeyIds is called!"); } - return mResultIntent; + return mKeySelectionPendingIntent; } + long[] getKeyIds() { - if (mResultIntent != null) { + if (mKeySelectionPendingIntent != null) { throw new AssertionError("result intent must be null when getKeyIds is called!"); } - if (mKeyIds == null) { - throw new AssertionError("key ids must not be null when getKeyIds is called!"); + HashSet allKeyIds = new HashSet<>(); + if (mUserKeyIds != null) { + allKeyIds.addAll(mUserKeyIds); } - return KeyFormattingUtils.getUnboxedLongArray(mKeyIds); + if (mExplicitKeyIds != null) { + allKeyIds.addAll(mExplicitKeyIds); + } + return KeyFormattingUtils.getUnboxedLongArray(allKeyIds); + } + + boolean isAllKeysConfirmed() { + return mAllKeysConfirmed; + } + + private KeyIdResult withExplicitKeyIds(HashSet explicitKeyIds) { + return new KeyIdResult(this, explicitKeyIds); + } + + KeyIdResultStatus getStatus() { + return mStatus; } } + enum KeyIdResultStatus { + OK, MISSING, DUPLICATE, NO_KEYS, NO_KEYS_ERROR + } + + private KeyIdResult createKeysOkResult(HashSet encryptKeyIds, boolean allKeysConfirmed) { + return new KeyIdResult(encryptKeyIds, allKeysConfirmed, KeyIdResultStatus.OK); + } + + private static KeyIdResult createNoKeysResult(PendingIntent pendingIntent) { + return new KeyIdResult(pendingIntent, KeyIdResultStatus.NO_KEYS); + } + + private static KeyIdResult createNoKeysResult() { + return new KeyIdResult(null, KeyIdResultStatus.NO_KEYS_ERROR); + } + + private static KeyIdResult createDuplicateKeysResult(PendingIntent pendingIntent) { + return new KeyIdResult(pendingIntent, KeyIdResultStatus.DUPLICATE); + } + + private static KeyIdResult createMissingKeysResult(PendingIntent pendingIntent) { + return new KeyIdResult(pendingIntent, KeyIdResultStatus.MISSING); + } } diff --git a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/remote/KeychainExternalProviderTest.java b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/remote/KeychainExternalProviderTest.java index 9568e33c3..d406342cc 100644 --- a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/remote/KeychainExternalProviderTest.java +++ b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/remote/KeychainExternalProviderTest.java @@ -78,6 +78,15 @@ public class KeychainExternalProviderTest { ); } + @Test(expected = AccessControlException.class) + public void testPermission__withExplicitPackage() throws Exception { + contentResolver.query( + EmailStatus.CONTENT_URI.buildUpon().appendPath("fake_pkg").build(), + new String[] { EmailStatus.EMAIL_ADDRESS, EmailStatus.EMAIL_ADDRESS, EmailStatus.USER_ID }, + null, new String [] { }, null + ); + } + @Test(expected = AccessControlException.class) public void testPermission__withWrongPackageCert() throws Exception { apiDao.deleteApiApp(PACKAGE_NAME); @@ -94,7 +103,7 @@ public class KeychainExternalProviderTest { public void testQuery__withNonExistentAddress() throws Exception { Cursor cursor = contentResolver.query( EmailStatus.CONTENT_URI, new String[] { - EmailStatus.EMAIL_ADDRESS, EmailStatus.EMAIL_STATUS, EmailStatus.USER_ID }, + EmailStatus.EMAIL_ADDRESS, EmailStatus.USER_ID_STATUS, EmailStatus.USER_ID }, null, new String [] { MAIL_ADDRESS_1 }, null ); @@ -111,7 +120,7 @@ public class KeychainExternalProviderTest { Cursor cursor = contentResolver.query( EmailStatus.CONTENT_URI, new String[] { - EmailStatus.EMAIL_ADDRESS, EmailStatus.EMAIL_STATUS, EmailStatus.USER_ID }, + EmailStatus.EMAIL_ADDRESS, EmailStatus.USER_ID_STATUS, EmailStatus.USER_ID }, null, new String [] { MAIL_ADDRESS_1 }, null ); @@ -129,7 +138,7 @@ public class KeychainExternalProviderTest { Cursor cursor = contentResolver.query( EmailStatus.CONTENT_URI, new String[] { - EmailStatus.EMAIL_ADDRESS, EmailStatus.EMAIL_STATUS, EmailStatus.USER_ID }, + EmailStatus.EMAIL_ADDRESS, EmailStatus.USER_ID_STATUS, EmailStatus.USER_ID }, null, new String [] { MAIL_ADDRESS_1, MAIL_ADDRESS_2 }, null ); @@ -151,7 +160,7 @@ public class KeychainExternalProviderTest { Cursor cursor = contentResolver.query( EmailStatus.CONTENT_URI, new String[] { - EmailStatus.EMAIL_ADDRESS, EmailStatus.EMAIL_STATUS, EmailStatus.USER_ID }, + EmailStatus.EMAIL_ADDRESS, EmailStatus.USER_ID_STATUS, EmailStatus.USER_ID }, null, new String [] { MAIL_ADDRESS_SEC_1 }, null ); @@ -172,7 +181,7 @@ public class KeychainExternalProviderTest { Cursor cursor = contentResolver.query( EmailStatus.CONTENT_URI, new String[] { - EmailStatus.EMAIL_ADDRESS, EmailStatus.EMAIL_STATUS, EmailStatus.USER_ID }, + EmailStatus.EMAIL_ADDRESS, EmailStatus.USER_ID_STATUS, EmailStatus.USER_ID }, null, new String [] { MAIL_ADDRESS_1 }, null ); diff --git a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/remote/OpenPgpServiceKeyIdExtractorTest.java b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/remote/OpenPgpServiceKeyIdExtractorTest.java index 3198f3e44..2cf52a05a 100644 --- a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/remote/OpenPgpServiceKeyIdExtractorTest.java +++ b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/remote/OpenPgpServiceKeyIdExtractorTest.java @@ -1,27 +1,28 @@ package org.sufficientlysecure.keychain.remote; +import java.util.ArrayList; +import java.util.Arrays; + import android.app.PendingIntent; import android.content.ContentResolver; import android.content.Intent; -import android.database.Cursor; import android.database.MatrixCursor; import android.net.Uri; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; -import org.openintents.openpgp.OpenPgpError; import org.openintents.openpgp.util.OpenPgpApi; +import org.sufficientlysecure.keychain.BuildConfig; import org.sufficientlysecure.keychain.KeychainTestRunner; import org.sufficientlysecure.keychain.remote.OpenPgpServiceKeyIdExtractor.KeyIdResult; - -import java.util.ArrayList; -import java.util.Arrays; +import org.sufficientlysecure.keychain.remote.OpenPgpServiceKeyIdExtractor.KeyIdResultStatus; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; @@ -51,9 +52,11 @@ public class OpenPgpServiceKeyIdExtractorTest { Intent intent = new Intent(); intent.putExtra(OpenPgpApi.EXTRA_KEY_IDS, KEY_IDS); - KeyIdResult keyIdResult = openPgpServiceKeyIdExtractor.returnKeyIdsFromIntent(intent, false); + KeyIdResult keyIdResult = openPgpServiceKeyIdExtractor.returnKeyIdsFromIntent(intent, false, + BuildConfig.APPLICATION_ID); - assertFalse(keyIdResult.hasResultIntent()); + assertEquals(KeyIdResultStatus.NO_KEYS_ERROR, keyIdResult.getStatus()); + assertFalse(keyIdResult.hasKeySelectionPendingIntent()); assertArrayEqualsSorted(KEY_IDS, keyIdResult.getKeyIds()); } @@ -63,9 +66,11 @@ public class OpenPgpServiceKeyIdExtractorTest { intent.putExtra(OpenPgpApi.EXTRA_KEY_IDS_SELECTED, KEY_IDS); intent.putExtra(OpenPgpApi.EXTRA_USER_IDS, USER_IDS); // should be ignored - KeyIdResult keyIdResult = openPgpServiceKeyIdExtractor.returnKeyIdsFromIntent(intent, false); + KeyIdResult keyIdResult = openPgpServiceKeyIdExtractor.returnKeyIdsFromIntent(intent, false, + BuildConfig.APPLICATION_ID); - assertFalse(keyIdResult.hasResultIntent()); + assertEquals(KeyIdResultStatus.OK, keyIdResult.getStatus()); + assertFalse(keyIdResult.hasKeySelectionPendingIntent()); assertArrayEqualsSorted(KEY_IDS, keyIdResult.getKeyIds()); } @@ -80,30 +85,30 @@ public class OpenPgpServiceKeyIdExtractorTest { setupPendingIntentFactoryResult(pendingIntent); - KeyIdResult keyIdResult = openPgpServiceKeyIdExtractor.returnKeyIdsFromIntent(intent, false); + KeyIdResult keyIdResult = openPgpServiceKeyIdExtractor.returnKeyIdsFromIntent(intent, false, + BuildConfig.APPLICATION_ID); - assertTrue(keyIdResult.hasResultIntent()); - Intent resultIntent = keyIdResult.getResultIntent(); - assertEquals(OpenPgpApi.RESULT_CODE_USER_INTERACTION_REQUIRED, - resultIntent.getIntExtra(OpenPgpApi.RESULT_CODE, Integer.MAX_VALUE)); - assertEquals(pendingIntent, resultIntent.getParcelableExtra(OpenPgpApi.RESULT_INTENT)); + assertEquals(KeyIdResultStatus.NO_KEYS, keyIdResult.getStatus()); + assertTrue(keyIdResult.hasKeySelectionPendingIntent()); } @Test public void returnKeyIdsFromIntent__withNoData() throws Exception { Intent intent = new Intent(); + intent.putExtra(OpenPgpApi.EXTRA_USER_IDS, new String[] { }); + + PendingIntent pendingIntent = mock(PendingIntent.class); + setupPendingIntentFactoryResult(pendingIntent); - KeyIdResult keyIdResult = openPgpServiceKeyIdExtractor.returnKeyIdsFromIntent(intent, false); + KeyIdResult keyIdResult = openPgpServiceKeyIdExtractor.returnKeyIdsFromIntent(intent, false, + BuildConfig.APPLICATION_ID); - assertTrue(keyIdResult.hasResultIntent()); - Intent resultIntent = keyIdResult.getResultIntent(); - assertEquals(OpenPgpApi.RESULT_CODE_ERROR, - resultIntent.getIntExtra(OpenPgpApi.RESULT_CODE, Integer.MAX_VALUE)); - assertEquals(OpenPgpError.NO_USER_IDS, - resultIntent.getParcelableExtra(OpenPgpApi.RESULT_ERROR).getErrorId()); + assertEquals(KeyIdResultStatus.NO_KEYS, keyIdResult.getStatus()); + assertTrue(keyIdResult.hasKeySelectionPendingIntent()); + assertSame(pendingIntent, keyIdResult.getKeySelectionPendingIntent()); } @Test @@ -114,14 +119,13 @@ public class OpenPgpServiceKeyIdExtractorTest { PendingIntent pendingIntent = mock(PendingIntent.class); setupPendingIntentFactoryResult(pendingIntent); - KeyIdResult keyIdResult = openPgpServiceKeyIdExtractor.returnKeyIdsFromIntent(intent, false); + KeyIdResult keyIdResult = openPgpServiceKeyIdExtractor.returnKeyIdsFromIntent(intent, false, + BuildConfig.APPLICATION_ID); - assertTrue(keyIdResult.hasResultIntent()); - Intent resultIntent = keyIdResult.getResultIntent(); - assertEquals(OpenPgpApi.RESULT_CODE_USER_INTERACTION_REQUIRED, - resultIntent.getIntExtra(OpenPgpApi.RESULT_CODE, Integer.MAX_VALUE)); - assertEquals(pendingIntent, resultIntent.getParcelableExtra(OpenPgpApi.RESULT_INTENT)); + assertEquals(KeyIdResultStatus.NO_KEYS, keyIdResult.getStatus()); + assertTrue(keyIdResult.hasKeySelectionPendingIntent()); + assertSame(pendingIntent, keyIdResult.getKeySelectionPendingIntent()); } @Test @@ -134,33 +138,13 @@ public class OpenPgpServiceKeyIdExtractorTest { setupPendingIntentFactoryResult(pendingIntent); - KeyIdResult keyIdResult = openPgpServiceKeyIdExtractor.returnKeyIdsFromIntent(intent, true); + KeyIdResult keyIdResult = openPgpServiceKeyIdExtractor.returnKeyIdsFromIntent(intent, true, + BuildConfig.APPLICATION_ID); - assertTrue(keyIdResult.hasResultIntent()); - Intent resultIntent = keyIdResult.getResultIntent(); - assertEquals(OpenPgpApi.RESULT_CODE_USER_INTERACTION_REQUIRED, - resultIntent.getIntExtra(OpenPgpApi.RESULT_CODE, Integer.MAX_VALUE)); - assertEquals(pendingIntent, resultIntent.getParcelableExtra(OpenPgpApi.RESULT_INTENT)); - } - - @Test - public void returnKeyIdsFromIntent__withUserIds__withEmptyQueryResult__inOpportunisticMode() throws Exception { - Intent intent = new Intent(); - intent.putExtra(OpenPgpApi.EXTRA_USER_IDS, USER_IDS); - intent.putExtra(OpenPgpApi.EXTRA_OPPORTUNISTIC_ENCRYPTION, true); - - setupContentResolverResult(); - - - KeyIdResult keyIdResult = openPgpServiceKeyIdExtractor.returnKeyIdsFromIntent(intent, false); - - - assertTrue(keyIdResult.hasResultIntent()); - Intent resultIntent = keyIdResult.getResultIntent(); - assertEquals(OpenPgpApi.RESULT_CODE_ERROR, resultIntent.getIntExtra(OpenPgpApi.RESULT_CODE, Integer.MAX_VALUE)); - assertEquals(OpenPgpError.OPPORTUNISTIC_MISSING_KEYS, - resultIntent.getParcelableExtra(OpenPgpApi.RESULT_ERROR).getErrorId()); + assertEquals(KeyIdResultStatus.NO_KEYS, keyIdResult.getStatus()); + assertTrue(keyIdResult.hasKeySelectionPendingIntent()); + assertSame(pendingIntent, keyIdResult.getKeySelectionPendingIntent()); } @Test @@ -168,16 +152,15 @@ public class OpenPgpServiceKeyIdExtractorTest { Intent intent = new Intent(); intent.putExtra(OpenPgpApi.EXTRA_USER_IDS, USER_IDS); - setupContentResolverResult(new long[][] { - new long[] { 123L }, - new long[] { 234L } - }); + setupContentResolverResult(USER_IDS, new Long[] { 123L, 234L }, new int[] { 0, 0 }); - KeyIdResult keyIdResult = openPgpServiceKeyIdExtractor.returnKeyIdsFromIntent(intent, false); + KeyIdResult keyIdResult = openPgpServiceKeyIdExtractor.returnKeyIdsFromIntent(intent, false, + BuildConfig.APPLICATION_ID); - assertFalse(keyIdResult.hasResultIntent()); + assertEquals(KeyIdResultStatus.OK, keyIdResult.getStatus()); + assertFalse(keyIdResult.hasKeySelectionPendingIntent()); assertArrayEqualsSorted(KEY_IDS, keyIdResult.getKeyIds()); } @@ -186,23 +169,20 @@ public class OpenPgpServiceKeyIdExtractorTest { Intent intent = new Intent(); intent.putExtra(OpenPgpApi.EXTRA_USER_IDS, USER_IDS); - setupContentResolverResult(new long[][] { - new long[] { 123L, 345L }, - new long[] { 234L } - }); + setupContentResolverResult(new String[] { + USER_IDS[0], USER_IDS[0], USER_IDS[1] + }, new Long[] { 123L, 345L, 234L }, new int[] { 0, 0, 0 }); PendingIntent pendingIntent = mock(PendingIntent.class); setupPendingIntentFactoryResult(pendingIntent); - KeyIdResult keyIdResult = openPgpServiceKeyIdExtractor.returnKeyIdsFromIntent(intent, false); + KeyIdResult keyIdResult = openPgpServiceKeyIdExtractor.returnKeyIdsFromIntent(intent, false, + BuildConfig.APPLICATION_ID); - assertTrue(keyIdResult.hasResultIntent()); - Intent resultIntent = keyIdResult.getResultIntent(); - assertEquals(OpenPgpApi.RESULT_CODE_USER_INTERACTION_REQUIRED, - resultIntent.getIntExtra(OpenPgpApi.RESULT_CODE, Integer.MAX_VALUE)); - assertEquals(pendingIntent, resultIntent.getParcelableExtra(OpenPgpApi.RESULT_INTENT)); + assertEquals(KeyIdResultStatus.DUPLICATE, keyIdResult.getStatus()); + assertTrue(keyIdResult.hasKeySelectionPendingIntent()); } @Test @@ -210,46 +190,36 @@ public class OpenPgpServiceKeyIdExtractorTest { Intent intent = new Intent(); intent.putExtra(OpenPgpApi.EXTRA_USER_IDS, USER_IDS); - setupContentResolverResult(new long[][] { - new long[] { }, - new long[] { 234L } - }); + setupContentResolverResult(USER_IDS, new Long[] { null, 234L }, new int[] { 0, 0 }); PendingIntent pendingIntent = mock(PendingIntent.class); setupPendingIntentFactoryResult(pendingIntent); - KeyIdResult keyIdResult = openPgpServiceKeyIdExtractor.returnKeyIdsFromIntent(intent, false); + KeyIdResult keyIdResult = openPgpServiceKeyIdExtractor.returnKeyIdsFromIntent(intent, false, + BuildConfig.APPLICATION_ID); - assertTrue(keyIdResult.hasResultIntent()); - Intent resultIntent = keyIdResult.getResultIntent(); - assertEquals(OpenPgpApi.RESULT_CODE_USER_INTERACTION_REQUIRED, - resultIntent.getIntExtra(OpenPgpApi.RESULT_CODE, Integer.MAX_VALUE)); - assertEquals(pendingIntent, resultIntent.getParcelableExtra(OpenPgpApi.RESULT_INTENT)); + assertEquals(KeyIdResultStatus.MISSING, keyIdResult.getStatus()); + assertTrue(keyIdResult.hasKeySelectionPendingIntent()); } private void setupContentResolverResult() { - MatrixCursor resultCursor = new MatrixCursor(OpenPgpServiceKeyIdExtractor.KEY_SEARCH_PROJECTION); + MatrixCursor resultCursor = new MatrixCursor(OpenPgpServiceKeyIdExtractor.PROJECTION_KEY_SEARCH); when(contentResolver.query( any(Uri.class), any(String[].class), any(String.class), any(String[].class), any(String.class))) .thenReturn(resultCursor); } - private void setupContentResolverResult(long[][] resultKeyIds) { - Cursor[] resultCursors = new MatrixCursor[resultKeyIds.length]; - for (int i = 0; i < resultKeyIds.length; i++) { - MatrixCursor resultCursor = new MatrixCursor(OpenPgpServiceKeyIdExtractor.KEY_SEARCH_PROJECTION); - for (long keyId : resultKeyIds[i]) { - resultCursor.addRow(new Object[] { keyId, keyId, 0L, 0L }); - } - - resultCursors[i] = resultCursor; + private void setupContentResolverResult(String[] userIds, Long[] resultKeyIds, int[] verified) { + MatrixCursor resultCursor = new MatrixCursor(OpenPgpServiceKeyIdExtractor.PROJECTION_KEY_SEARCH); + for (int i = 0; i < userIds.length; i++) { + resultCursor.addRow(new Object[] { userIds[i], resultKeyIds[i], verified[i] }); } when(contentResolver.query( any(Uri.class), any(String[].class), any(String.class), any(String[].class), any(String.class))) - .thenReturn(resultCursors[0], Arrays.copyOfRange(resultCursors, 1, resultCursors.length)); + .thenReturn(resultCursor); } private void setupPendingIntentFactoryResult(PendingIntent pendingIntent) { From 2a26908fb65f1753ba4fa7835e5d88beba1ae8f1 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Fri, 3 Mar 2017 16:52:06 +0100 Subject: [PATCH 2/4] extract creation of error result intent into method --- .../keychain/remote/OpenPgpService.java | 55 ++++++------------- 1 file changed, 18 insertions(+), 37 deletions(-) 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 ea0e5bed2..80ed02663 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java @@ -189,11 +189,7 @@ public class OpenPgpService extends Service { } } catch (Exception e) { Log.d(Constants.TAG, "signImpl", e); - Intent result = new Intent(); - result.putExtra(OpenPgpApi.RESULT_ERROR, - new OpenPgpError(OpenPgpError.GENERIC_ERROR, e.getMessage())); - result.putExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_ERROR); - return result; + return createErrorResultIntent(OpenPgpError.GENERIC_ERROR, e.getMessage()); } } @@ -247,11 +243,8 @@ public class OpenPgpService extends Service { if (keyIdResult.hasKeySelectionPendingIntent()) { if ((keyIdResultStatus == KeyIdResultStatus.MISSING || keyIdResultStatus == KeyIdResultStatus.NO_KEYS || keyIdResultStatus == KeyIdResultStatus.NO_KEYS_ERROR) && isOpportunistic) { - Intent result = new Intent(); - result.putExtra(OpenPgpApi.RESULT_ERROR, - new OpenPgpError(OpenPgpError.OPPORTUNISTIC_MISSING_KEYS, "missing keys in opportunistic mode")); - result.putExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_ERROR); - return result; + return createErrorResultIntent(OpenPgpError.OPPORTUNISTIC_MISSING_KEYS, + "missing keys in opportunistic mode"); } Intent result = new Intent(); @@ -301,11 +294,7 @@ public class OpenPgpService extends Service { } } catch (Exception e) { Log.d(Constants.TAG, "encryptAndSignImpl", e); - Intent result = new Intent(); - result.putExtra(OpenPgpApi.RESULT_ERROR, - new OpenPgpError(OpenPgpError.GENERIC_ERROR, e.getMessage())); - result.putExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_ERROR); - return result; + return createErrorResultIntent(OpenPgpError.GENERIC_ERROR, e.getMessage()); } } @@ -389,18 +378,12 @@ public class OpenPgpService extends Service { } 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; + return createErrorResultIntent(OpenPgpError.GENERIC_ERROR, errorMsg); } } catch (Exception 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_CODE, OpenPgpApi.RESULT_CODE_ERROR); - return result; + return createErrorResultIntent(OpenPgpError.GENERIC_ERROR, e.getMessage()); } } @@ -536,14 +519,19 @@ public class OpenPgpService extends Service { } } catch (Exception e) { Log.d(Constants.TAG, "getKeyImpl", e); - Intent result = new Intent(); - result.putExtra(OpenPgpApi.RESULT_ERROR, - new OpenPgpError(OpenPgpError.GENERIC_ERROR, e.getMessage())); - result.putExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_ERROR); - return result; + return createErrorResultIntent(OpenPgpError.GENERIC_ERROR, e.getMessage()); } } + @NonNull + private Intent createErrorResultIntent(int errorCode, String errorMsg) { + Intent result = new Intent(); + result.putExtra(OpenPgpApi.RESULT_ERROR, + new OpenPgpError(errorCode, errorMsg)); + result.putExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_ERROR); + return result; + } + private Intent getSignKeyIdImpl(Intent data) { // if data already contains EXTRA_SIGN_KEY_ID, it has been executed again // after user interaction. Then, we just need to return the long again! @@ -613,18 +601,11 @@ public class OpenPgpService extends Service { } else { // should not happen normally... 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; + return createErrorResultIntent(OpenPgpError.GENERIC_ERROR, errorMsg); } } catch (Exception e) { Log.d(Constants.TAG, "backupImpl", e); - Intent result = new Intent(); - result.putExtra(OpenPgpApi.RESULT_ERROR, - new OpenPgpError(OpenPgpError.GENERIC_ERROR, e.getMessage())); - result.putExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_ERROR); - return result; + return createErrorResultIntent(OpenPgpError.GENERIC_ERROR, e.getMessage()); } } From 612cd8904669b8a32d96f86b9e96d1e863870da7 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Tue, 28 Feb 2017 19:06:00 +0100 Subject: [PATCH 3/4] service: support EXTRA_DRY_RUN to get key status for a sign/encrypt run --- .../keychain/remote/OpenPgpService.java | 42 ++++++++++++++++++- extern/openpgp-api-lib | 2 +- 2 files changed, 42 insertions(+), 2 deletions(-) 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 80ed02663..99ac09169 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java @@ -116,7 +116,7 @@ public class OpenPgpService extends Service { Intent signKeyIdIntent = getSignKeyMasterId(data); // NOTE: Fallback to return account settings (Old API) - if (signKeyIdIntent.getIntExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_ERROR) + if (signKeyIdIntent.getIntExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_SUCCESS) == OpenPgpApi.RESULT_CODE_USER_INTERACTION_REQUIRED) { return signKeyIdIntent; } @@ -237,8 +237,12 @@ public class OpenPgpService extends Service { KeyIdResult keyIdResult = mKeyIdExtractor.returnKeyIdsFromIntent(data, false, mApiPermissionHelper.getCurrentCallingPackage()); + boolean isDryRun = data.getBooleanExtra(OpenPgpApi.EXTRA_DRY_RUN, false); boolean isOpportunistic = data.getBooleanExtra(OpenPgpApi.EXTRA_OPPORTUNISTIC_ENCRYPTION, false); KeyIdResultStatus keyIdResultStatus = keyIdResult.getStatus(); + if (isDryRun) { + return getDryRunStatusResult(keyIdResult); + } if (keyIdResult.hasKeySelectionPendingIntent()) { if ((keyIdResultStatus == KeyIdResultStatus.MISSING || keyIdResultStatus == KeyIdResultStatus.NO_KEYS || @@ -298,6 +302,42 @@ public class OpenPgpService extends Service { } } + @NonNull + private Intent getDryRunStatusResult(KeyIdResult keyIdResult) { + switch (keyIdResult.getStatus()) { + case MISSING: { + Intent result = new Intent(); + result.putExtra(OpenPgpApi.RESULT_ERROR, + new OpenPgpError(OpenPgpError.OPPORTUNISTIC_MISSING_KEYS, "missing keys in opportunistic mode")); + result.putExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_ERROR); + return result; + } + case NO_KEYS: + case NO_KEYS_ERROR: { + Intent result = new Intent(); + result.putExtra(OpenPgpApi.RESULT_ERROR, + new OpenPgpError(OpenPgpError.NO_USER_IDS, "empty recipient list")); + result.putExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_ERROR); + return result; + } + case DUPLICATE: { + Intent result = new Intent(); + result.putExtra(OpenPgpApi.RESULT_KEYS_CONFIRMED, false); + result.putExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_SUCCESS); + return result; + } + case OK: { + Intent result = new Intent(); + result.putExtra(OpenPgpApi.RESULT_KEYS_CONFIRMED, keyIdResult.isAllKeysConfirmed()); + result.putExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_SUCCESS); + return result; + } + default: { + throw new IllegalStateException("unhandled case!"); + } + } + } + private Intent decryptAndVerifyImpl(Intent data, InputStream inputStream, OutputStream outputStream, boolean decryptMetadataOnly, Progressable progressable) { try { diff --git a/extern/openpgp-api-lib b/extern/openpgp-api-lib index 5aa2affb7..f28cb9294 160000 --- a/extern/openpgp-api-lib +++ b/extern/openpgp-api-lib @@ -1 +1 @@ -Subproject commit 5aa2affb752c066232bd7e7bb7c22739fa9f3526 +Subproject commit f28cb92944efbf56fa0582769a97b23c4272f6ac From 269f98bee00beffd67b116b754f609b07f2a5683 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Wed, 26 Apr 2017 11:35:40 +0200 Subject: [PATCH 4/4] make KeyIdExtractor code more readable --- .../provider/KeychainExternalContract.java | 2 + .../remote/KeychainExternalProvider.java | 7 +- .../remote/OpenPgpServiceKeyIdExtractor.java | 152 +++++++++++------- 3 files changed, 95 insertions(+), 66 deletions(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainExternalContract.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainExternalContract.java index d1042ba0d..f4d9d657b 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainExternalContract.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainExternalContract.java @@ -25,6 +25,8 @@ import org.sufficientlysecure.keychain.Constants; public class KeychainExternalContract { + public static final int KEY_STATUS_UNVERIFIED = 1; + public static final int KEY_STATUS_VERIFIED = 2; // this is in KeychainExternalContract already, but we want to be double // sure this isn't mixed up with the internal one! diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/KeychainExternalProvider.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/KeychainExternalProvider.java index 8e4bf7a6c..29d874f9a 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/KeychainExternalProvider.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/KeychainExternalProvider.java @@ -170,10 +170,9 @@ public class KeychainExternalProvider extends ContentProvider implements SimpleC // we take the minimum (>0) here, where "1" is "verified by known secret key", "2" is "self-certified" projectionMap.put(EmailStatus.USER_ID_STATUS, "CASE ( MIN (" + Certs.VERIFIED + " ) ) " // remap to keep this provider contract independent from our internal representation - + " WHEN NULL THEN 1" - + " WHEN " + Certs.VERIFIED_SELF + " THEN 1" - + " WHEN " + Certs.VERIFIED_SECRET + " THEN 2" - + " WHEN NULL THEN NULL" + + " WHEN " + Certs.VERIFIED_SELF + " THEN " + KeychainExternalContract.KEY_STATUS_UNVERIFIED + + " WHEN " + Certs.VERIFIED_SECRET + " THEN " + KeychainExternalContract.KEY_STATUS_VERIFIED + + " WHEN NULL THEN " + KeychainExternalContract.KEY_STATUS_UNVERIFIED + " END AS " + EmailStatus.USER_ID_STATUS); projectionMap.put(EmailStatus.USER_ID, Tables.USER_PACKETS + "." + UserPackets.USER_ID + " AS " + EmailStatus.USER_ID); projectionMap.put(EmailStatus.MASTER_KEY_ID, diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpServiceKeyIdExtractor.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpServiceKeyIdExtractor.java index 5a88e4154..db0e5e03b 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpServiceKeyIdExtractor.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpServiceKeyIdExtractor.java @@ -11,10 +11,12 @@ import android.content.ContentResolver; import android.content.Intent; import android.database.Cursor; import android.net.Uri; +import android.support.annotation.NonNull; import android.support.annotation.VisibleForTesting; import org.openintents.openpgp.util.OpenPgpApi; import org.sufficientlysecure.keychain.Constants; +import org.sufficientlysecure.keychain.provider.KeychainExternalContract; import org.sufficientlysecure.keychain.provider.KeychainExternalContract.EmailStatus; import org.sufficientlysecure.keychain.ui.util.KeyFormattingUtils; import org.sufficientlysecure.keychain.util.Log; @@ -76,97 +78,108 @@ class OpenPgpServiceKeyIdExtractor { return result; } - private KeyIdResult returnKeyIdsFromEmails(Intent data, String[] encryptionUserIds, String callingPackageName) { - boolean hasUserIds = (encryptionUserIds != null && encryptionUserIds.length > 0); + private KeyIdResult returnKeyIdsFromEmails(Intent data, String[] encryptionAddresses, String callingPackageName) { + boolean hasAddresses = (encryptionAddresses != null && encryptionAddresses.length > 0); - boolean anyKeyNotVerified = false; + boolean allKeysConfirmed = false; HashSet keyIds = new HashSet<>(); ArrayList missingEmails = new ArrayList<>(); ArrayList duplicateEmails = new ArrayList<>(); - HashMap keyRows = new HashMap<>(); - if (hasUserIds) { - Uri queryUri = EmailStatus.CONTENT_URI.buildUpon().appendPath(callingPackageName).build(); - Cursor cursor = contentResolver.query(queryUri, PROJECTION_KEY_SEARCH, null, encryptionUserIds, null); - if (cursor == null) { - throw new IllegalStateException("Internal error, received null cursor!"); - } - try { - while (cursor.moveToNext()) { - String queryAddress = cursor.getString(INDEX_EMAIL_ADDRESS); - Long masterKeyId = cursor.isNull(INDEX_MASTER_KEY_ID) ? null : cursor.getLong(INDEX_MASTER_KEY_ID); - int verified = cursor.getInt(INDEX_EMAIL_STATUS); + if (hasAddresses) { + HashMap keyRows = getStatusMapForQueriedAddresses(encryptionAddresses, callingPackageName); - KeyRow row = new KeyRow(masterKeyId, verified == 2); - if (!keyRows.containsKey(queryAddress)) { - keyRows.put(queryAddress, row); - continue; - } - - KeyRow previousRow = keyRows.get(queryAddress); - if (previousRow.masterKeyId == null) { - keyRows.put(queryAddress, row); - } else if (!previousRow.verified && row.verified) { - keyRows.put(queryAddress, row); - } else if (previousRow.verified == row.verified) { - previousRow.hasDuplicate = true; - } - } - } finally { - cursor.close(); - } - - for (Entry entry : keyRows.entrySet()) { + boolean anyKeyNotVerified = false; + for (Entry entry : keyRows.entrySet()) { String queriedAddress = entry.getKey(); - KeyRow keyRow = entry.getValue(); + UserIdStatus userIdStatus = entry.getValue(); - if (keyRow.masterKeyId == null) { + if (userIdStatus.masterKeyId == null) { missingEmails.add(queriedAddress); continue; } - keyIds.add(keyRow.masterKeyId); + keyIds.add(userIdStatus.masterKeyId); - if (keyRow.hasDuplicate) { + if (userIdStatus.hasDuplicate) { duplicateEmails.add(queriedAddress); } - if (!keyRow.verified) { + if (!userIdStatus.verified) { anyKeyNotVerified = true; } } - if (keyRows.size() != encryptionUserIds.length) { + if (keyRows.size() != encryptionAddresses.length) { Log.e(Constants.TAG, "Number of rows doesn't match number of retrieved rows! Probably a bug?"); } + + allKeysConfirmed = !anyKeyNotVerified; } - long[] keyIdsArray = KeyFormattingUtils.getUnboxedLongArray(keyIds); - PendingIntent pi = apiPendingIntentFactory.createSelectPublicKeyPendingIntent(data, keyIdsArray, - missingEmails, duplicateEmails, false); - if (!missingEmails.isEmpty()) { - return createMissingKeysResult(pi); + return createMissingKeysResult(data, keyIds, missingEmails, duplicateEmails); } if (!duplicateEmails.isEmpty()) { - return createDuplicateKeysResult(pi); + return createDuplicateKeysResult(data, keyIds, missingEmails, duplicateEmails); } if (keyIds.isEmpty()) { - return createNoKeysResult(pi); + return createNoKeysResult(data, keyIds, missingEmails, duplicateEmails); } - boolean allKeysConfirmed = !anyKeyNotVerified; return createKeysOkResult(keyIds, allKeysConfirmed); } - private static class KeyRow { + /** This method queries the KeychainExternalProvider for all addresses given in encryptionUserIds. + * It returns a map with one UserIdStatus per queried address. If multiple key candidates exist, + * the one with the highest verification status is selected. If two candidates with the same + * verification status exist, the first one is returned and marked as having a duplicate. + */ + @NonNull + private HashMap getStatusMapForQueriedAddresses(String[] encryptionUserIds, String callingPackageName) { + HashMap keyRows = new HashMap<>(); + Uri queryUri = EmailStatus.CONTENT_URI.buildUpon().appendPath(callingPackageName).build(); + Cursor cursor = contentResolver.query(queryUri, PROJECTION_KEY_SEARCH, null, encryptionUserIds, null); + if (cursor == null) { + throw new IllegalStateException("Internal error, received null cursor!"); + } + + try { + while (cursor.moveToNext()) { + String queryAddress = cursor.getString(INDEX_EMAIL_ADDRESS); + Long masterKeyId = cursor.isNull(INDEX_MASTER_KEY_ID) ? null : cursor.getLong(INDEX_MASTER_KEY_ID); + boolean isVerified = cursor.getInt(INDEX_EMAIL_STATUS) == KeychainExternalContract.KEY_STATUS_VERIFIED; + UserIdStatus userIdStatus = new UserIdStatus(masterKeyId, isVerified); + + boolean seenBefore = keyRows.containsKey(queryAddress); + if (!seenBefore) { + keyRows.put(queryAddress, userIdStatus); + continue; + } + + UserIdStatus previousUserIdStatus = keyRows.get(queryAddress); + if (previousUserIdStatus.masterKeyId == null) { + keyRows.put(queryAddress, userIdStatus); + } else if (!previousUserIdStatus.verified && userIdStatus.verified) { + keyRows.put(queryAddress, userIdStatus); + } else if (previousUserIdStatus.verified == userIdStatus.verified) { + previousUserIdStatus.hasDuplicate = true; + } + } + } finally { + cursor.close(); + } + return keyRows; + } + + private static class UserIdStatus { private final Long masterKeyId; private final boolean verified; private boolean hasDuplicate; - KeyRow(Long masterKeyId, boolean verified) { + UserIdStatus(Long masterKeyId, boolean verified) { this.masterKeyId = masterKeyId; this.verified = verified; } @@ -251,19 +264,34 @@ class OpenPgpServiceKeyIdExtractor { return new KeyIdResult(encryptKeyIds, allKeysConfirmed, KeyIdResultStatus.OK); } - private static KeyIdResult createNoKeysResult(PendingIntent pendingIntent) { - return new KeyIdResult(pendingIntent, KeyIdResultStatus.NO_KEYS); + private KeyIdResult createNoKeysResult(Intent data, + HashSet selectedKeyIds, ArrayList missingEmails, ArrayList duplicateEmails) { + long[] keyIdsArray = KeyFormattingUtils.getUnboxedLongArray(selectedKeyIds); + PendingIntent selectKeyPendingIntent = apiPendingIntentFactory.createSelectPublicKeyPendingIntent( + data, keyIdsArray, missingEmails, duplicateEmails, false); + + return new KeyIdResult(selectKeyPendingIntent, KeyIdResultStatus.NO_KEYS); } - private static KeyIdResult createNoKeysResult() { + private KeyIdResult createDuplicateKeysResult(Intent data, + HashSet selectedKeyIds, ArrayList missingEmails, ArrayList duplicateEmails) { + long[] keyIdsArray = KeyFormattingUtils.getUnboxedLongArray(selectedKeyIds); + PendingIntent selectKeyPendingIntent = apiPendingIntentFactory.createSelectPublicKeyPendingIntent( + data, keyIdsArray, missingEmails, duplicateEmails, false); + + return new KeyIdResult(selectKeyPendingIntent, KeyIdResultStatus.DUPLICATE); + } + + private KeyIdResult createMissingKeysResult(Intent data, + HashSet selectedKeyIds, ArrayList missingEmails, ArrayList duplicateEmails) { + long[] keyIdsArray = KeyFormattingUtils.getUnboxedLongArray(selectedKeyIds); + PendingIntent selectKeyPendingIntent = apiPendingIntentFactory.createSelectPublicKeyPendingIntent( + data, keyIdsArray, missingEmails, duplicateEmails, false); + + return new KeyIdResult(selectKeyPendingIntent, KeyIdResultStatus.MISSING); + } + + private KeyIdResult createNoKeysResult() { return new KeyIdResult(null, KeyIdResultStatus.NO_KEYS_ERROR); } - - private static KeyIdResult createDuplicateKeysResult(PendingIntent pendingIntent) { - return new KeyIdResult(pendingIntent, KeyIdResultStatus.DUPLICATE); - } - - private static KeyIdResult createMissingKeysResult(PendingIntent pendingIntent) { - return new KeyIdResult(pendingIntent, KeyIdResultStatus.MISSING); - } }