From 2ed485fd123620a7dfbb4f12ec1507b1bea39afc Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Sat, 31 Dec 2016 19:12:13 +0100 Subject: [PATCH 1/2] fix loop bug in key selection --- .../keychain/remote/OpenPgpService.java | 33 ++++++++++--------- .../remote/ui/RemoteSelectPubKeyActivity.java | 2 +- .../keychain/ui/BackupCodeFragment.java | 1 - extern/openpgp-api-lib | 2 +- 4 files changed, 19 insertions(+), 19 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 38c15aae5..f845a70db 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java @@ -131,14 +131,12 @@ public class OpenPgpService extends Service { } private KeyIdResult returnKeyIdsFromEmails(Intent data, String[] encryptionUserIds, boolean isOpportunistic) { - boolean noUserIdsCheck = (encryptionUserIds == null || encryptionUserIds.length == 0); - boolean missingUserIdsCheck = false; - boolean duplicateUserIdsCheck = false; + boolean hasUserIds = (encryptionUserIds != null && encryptionUserIds.length > 0); HashSet keyIds = new HashSet<>(); ArrayList missingEmails = new ArrayList<>(); ArrayList duplicateEmails = new ArrayList<>(); - if (!noUserIdsCheck) { + if (hasUserIds) { for (String rawUserId : encryptionUserIds) { OpenPgpUtils.UserId userId = KeyRing.splitUserId(rawUserId); String email = userId.email != null ? userId.email : rawUserId; @@ -151,19 +149,17 @@ public class OpenPgpService extends Service { long id = cursor.getLong(cursor.getColumnIndex(KeyRings.MASTER_KEY_ID)); keyIds.add(id); } else { - missingUserIdsCheck = true; missingEmails.add(email); Log.d(Constants.TAG, "user id missing"); } // another entry for this email -> two keys with the same email inside user id if (cursor != null && cursor.moveToNext()) { - duplicateUserIdsCheck = true; duplicateEmails.add(email); // also pre-select - long id = cursor.getLong(cursor.getColumnIndex(KeyRings.MASTER_KEY_ID)); - keyIds.add(id); - Log.d(Constants.TAG, "more than one user id with the same email"); + long id = cursor.getLong(cursor.getColumnIndex(KeyRings.MASTER_KEY_ID)); + keyIds.add(id); + Log.d(Constants.TAG, "more than one user id with the same email"); } } finally { if (cursor != null) { @@ -173,7 +169,9 @@ public class OpenPgpService extends Service { } } - if (isOpportunistic && (noUserIdsCheck || missingUserIdsCheck)) { + 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")); @@ -181,12 +179,12 @@ public class OpenPgpService extends Service { return new KeyIdResult(result); } - if (noUserIdsCheck || missingUserIdsCheck || duplicateUserIdsCheck) { + if (!hasUserIds || hasMissingUserIds || hasDuplicateUserIds) { // convert ArrayList to long[] long[] keyIdsArray = getUnboxedLongArray(keyIds); ApiPendingIntentFactory piFactory = new ApiPendingIntentFactory(getBaseContext()); PendingIntent pi = piFactory.createSelectPublicKeyPendingIntent(data, keyIdsArray, - missingEmails, duplicateEmails, noUserIdsCheck); + missingEmails, duplicateEmails, hasUserIds); // return PendingIntent to be executed by client Intent result = new Intent(); @@ -197,7 +195,7 @@ public class OpenPgpService extends Service { // everything was easy, we have exactly one key for every email if (keyIds.isEmpty()) { - Log.e(Constants.TAG, "keyIdsArray.length == 0, should never happen!"); + throw new AssertionError("keyIdsArray.length == 0, should never happen!"); } return new KeyIdResult(keyIds); @@ -321,9 +319,12 @@ public class OpenPgpService extends Service { long[] keyIds; { HashSet encryptKeyIds = new HashSet<>(); - - // get key ids based on given user ids - if (data.hasExtra(OpenPgpApi.EXTRA_USER_IDS)) { + boolean hasKeysFromSelectPubkeyActivity = data.hasExtra(OpenPgpApi.EXTRA_KEY_IDS_SELECTED); + if (hasKeysFromSelectPubkeyActivity) { + for (long keyId : data.getLongArrayExtra(OpenPgpApi.EXTRA_KEY_IDS_SELECTED)) { + encryptKeyIds.add(keyId); + } + } else if (data.hasExtra(OpenPgpApi.EXTRA_USER_IDS)) { String[] userIds = data.getStringArrayExtra(OpenPgpApi.EXTRA_USER_IDS); boolean isOpportunistic = data.getBooleanExtra(OpenPgpApi.EXTRA_OPPORTUNISTIC_ENCRYPTION, false); // give params through to activity... diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/ui/RemoteSelectPubKeyActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/ui/RemoteSelectPubKeyActivity.java index ebf53cbaa..c6dcb27f5 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/ui/RemoteSelectPubKeyActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/ui/RemoteSelectPubKeyActivity.java @@ -103,7 +103,7 @@ public class RemoteSelectPubKeyActivity extends BaseActivity { public void onClick(View v) { // add key ids to params Bundle for new request Intent resultData = extras.getParcelable(EXTRA_DATA); - resultData.putExtra(OpenPgpApi.EXTRA_KEY_IDS, + resultData.putExtra(OpenPgpApi.EXTRA_KEY_IDS_SELECTED, mSelectFragment.getSelectedMasterKeyIds()); RemoteSelectPubKeyActivity.this.setResult(RESULT_OK, resultData); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/BackupCodeFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/BackupCodeFragment.java index 012ea0610..05fcaf3e0 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/BackupCodeFragment.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/BackupCodeFragment.java @@ -24,7 +24,6 @@ import android.animation.ValueAnimator.AnimatorUpdateListener; import android.annotation.SuppressLint; import android.content.Intent; import android.net.Uri; -import android.os.AsyncTask; import android.os.Build; import android.os.Bundle; import android.os.Handler; diff --git a/extern/openpgp-api-lib b/extern/openpgp-api-lib index d0af1b5ba..03b00625e 160000 --- a/extern/openpgp-api-lib +++ b/extern/openpgp-api-lib @@ -1 +1 @@ -Subproject commit d0af1b5bae77664ca7126a113f9833a8ec2cd045 +Subproject commit 03b00625eb29c7e6c5aa6e131bd0d64e574a7886 From 27670d55e8e3ec765f02985c4ded66d35296f74d Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Wed, 4 Jan 2017 12:11:13 +0100 Subject: [PATCH 2/2] incorporate feedback --- .../keychain/remote/OpenPgpService.java | 30 +++++++++++-------- 1 file changed, 17 insertions(+), 13 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 f845a70db..c2568fab1 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java @@ -143,28 +143,32 @@ public class OpenPgpService extends Service { // try to find the key for this specific email Uri uri = KeyRings.buildUnifiedKeyRingsFindByEmailUri(email); Cursor cursor = getContentResolver().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 != null && cursor.moveToFirst()) { + if (cursor.moveToFirst()) { long id = cursor.getLong(cursor.getColumnIndex(KeyRings.MASTER_KEY_ID)); keyIds.add(id); + + // 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); + + // also pre-select + while (cursor.moveToNext()) { + long duplicateId = cursor.getLong(cursor.getColumnIndex(KeyRings.MASTER_KEY_ID)); + keyIds.add(duplicateId); + } + } } else { missingEmails.add(email); Log.d(Constants.TAG, "user id missing"); } - // another entry for this email -> two keys with the same email inside user id - if (cursor != null && cursor.moveToNext()) { - duplicateEmails.add(email); - - // also pre-select - long id = cursor.getLong(cursor.getColumnIndex(KeyRings.MASTER_KEY_ID)); - keyIds.add(id); - Log.d(Constants.TAG, "more than one user id with the same email"); - } } finally { - if (cursor != null) { - cursor.close(); - } + cursor.close(); } } }