From c5ce794ef782b6809c3f0b3cacfa3bcd6a5a85b7 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Thu, 31 Jul 2014 22:36:04 +0200 Subject: [PATCH 1/2] more fixes on canonicalization and progress --- .../keychain/pgp/PgpKeyOperation.java | 8 ++--- .../keychain/pgp/UncachedKeyRing.java | 35 ++++++++++--------- 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java index b1bd6a77a..861f93446 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java @@ -354,7 +354,7 @@ public class PgpKeyOperation { subProgressPush(15, 25); for (int i = 0; i < saveParcel.mAddUserIds.size(); i++) { - progress(R.string.progress_modify_adduid, (i-1) * (100 / saveParcel.mAddSubKeys.size())); + progress(R.string.progress_modify_adduid, (i-1) * (100 / saveParcel.mAddUserIds.size())); String userId = saveParcel.mAddUserIds.get(i); log.add(LogLevel.INFO, LogType.MSG_MF_UID_ADD, indent); @@ -399,7 +399,7 @@ public class PgpKeyOperation { subProgressPush(25, 40); for (int i = 0; i < saveParcel.mRevokeUserIds.size(); i++) { - progress(R.string.progress_modify_revokeuid, (i-1) * (100 / saveParcel.mAddSubKeys.size())); + progress(R.string.progress_modify_revokeuid, (i-1) * (100 / saveParcel.mRevokeUserIds.size())); String userId = saveParcel.mRevokeUserIds.get(i); log.add(LogLevel.INFO, LogType.MSG_MF_UID_REVOKE, indent, userId); @@ -522,7 +522,7 @@ public class PgpKeyOperation { subProgressPush(50, 60); for (int i = 0; i < saveParcel.mChangeSubKeys.size(); i++) { - progress(R.string.progress_modify_subkeychange, (i-1) * (100 / saveParcel.mAddSubKeys.size())); + progress(R.string.progress_modify_subkeychange, (i-1) * (100 / saveParcel.mChangeSubKeys.size())); SaveKeyringParcel.SubkeyChange change = saveParcel.mChangeSubKeys.get(i); log.add(LogLevel.INFO, LogType.MSG_MF_SUBKEY_CHANGE, indent, PgpKeyHelper.convertKeyIdToHex(change.mKeyId)); @@ -583,7 +583,7 @@ public class PgpKeyOperation { subProgressPush(60, 70); for (int i = 0; i < saveParcel.mRevokeSubKeys.size(); i++) { - progress(R.string.progress_modify_subkeyrevoke, (i-1) * (100 / saveParcel.mAddSubKeys.size())); + progress(R.string.progress_modify_subkeyrevoke, (i-1) * (100 / saveParcel.mRevokeSubKeys.size())); long revocation = saveParcel.mRevokeSubKeys.get(i); log.add(LogLevel.INFO, LogType.MSG_MF_SUBKEY_REVOKE, indent, PgpKeyHelper.convertKeyIdToHex(revocation)); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java index 502e3a70c..690317170 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java @@ -300,14 +300,14 @@ public class UncachedKeyRing { revocation = zert; // more revocations? at least one is superfluous, then. } else if (revocation.getCreationTime().before(zert.getCreationTime())) { + log.add(LogLevel.INFO, LogType.MSG_KC_REVOKE_DUP, indent); modified = PGPPublicKey.removeCertification(modified, revocation); redundantCerts += 1; - log.add(LogLevel.INFO, LogType.MSG_KC_REVOKE_DUP, indent); revocation = zert; } else { + log.add(LogLevel.INFO, LogType.MSG_KC_REVOKE_DUP, indent); modified = PGPPublicKey.removeCertification(modified, zert); redundantCerts += 1; - log.add(LogLevel.INFO, LogType.MSG_KC_REVOKE_DUP, indent); } } @@ -331,12 +331,13 @@ public class UncachedKeyRing { indent, "0x" + Integer.toString(zert.getSignatureType(), 16)); modified = PGPPublicKey.removeCertification(modified, userId, zert); badCerts += 1; + continue; } if (cert.getCreationTime().after(now)) { // Creation date in the future? No way! log.add(LogLevel.WARN, LogType.MSG_KC_UID_BAD_TIME, indent); - modified = PGPPublicKey.removeCertification(modified, zert); + modified = PGPPublicKey.removeCertification(modified, userId, zert); badCerts += 1; continue; } @@ -344,7 +345,7 @@ public class UncachedKeyRing { if (cert.isLocal()) { // Creation date in the future? No way! log.add(LogLevel.WARN, LogType.MSG_KC_UID_BAD_LOCAL, indent); - modified = PGPPublicKey.removeCertification(modified, zert); + modified = PGPPublicKey.removeCertification(modified, userId, zert); badCerts += 1; continue; } @@ -387,35 +388,35 @@ public class UncachedKeyRing { if (selfCert == null) { selfCert = zert; } else if (selfCert.getCreationTime().before(cert.getCreationTime())) { + log.add(LogLevel.DEBUG, LogType.MSG_KC_UID_DUP, + indent, userId); modified = PGPPublicKey.removeCertification(modified, userId, selfCert); redundantCerts += 1; - log.add(LogLevel.DEBUG, LogType.MSG_KC_UID_DUP, - indent, userId); selfCert = zert; } else { - modified = PGPPublicKey.removeCertification(modified, userId, zert); - redundantCerts += 1; log.add(LogLevel.DEBUG, LogType.MSG_KC_UID_DUP, indent, userId); + modified = PGPPublicKey.removeCertification(modified, userId, zert); + redundantCerts += 1; } // If there is a revocation certificate, and it's older than this, drop it if (revocation != null && revocation.getCreationTime().before(selfCert.getCreationTime())) { + log.add(LogLevel.DEBUG, LogType.MSG_KC_UID_REVOKE_OLD, + indent, userId); modified = PGPPublicKey.removeCertification(modified, userId, revocation); revocation = null; redundantCerts += 1; - log.add(LogLevel.DEBUG, LogType.MSG_KC_UID_REVOKE_OLD, - indent, userId); } break; case PGPSignature.CERTIFICATION_REVOCATION: // If this is older than the (latest) self cert, drop it if (selfCert != null && selfCert.getCreationTime().after(zert.getCreationTime())) { - modified = PGPPublicKey.removeCertification(modified, userId, zert); - redundantCerts += 1; log.add(LogLevel.DEBUG, LogType.MSG_KC_UID_REVOKE_OLD, indent, userId); + modified = PGPPublicKey.removeCertification(modified, userId, zert); + redundantCerts += 1; continue; } // first revocation? remember it. @@ -423,16 +424,16 @@ public class UncachedKeyRing { revocation = zert; // more revocations? at least one is superfluous, then. } else if (revocation.getCreationTime().before(cert.getCreationTime())) { + log.add(LogLevel.DEBUG, LogType.MSG_KC_UID_REVOKE_DUP, + indent, userId); modified = PGPPublicKey.removeCertification(modified, userId, revocation); redundantCerts += 1; - log.add(LogLevel.DEBUG, LogType.MSG_KC_UID_REVOKE_DUP, - indent, userId); revocation = zert; } else { - modified = PGPPublicKey.removeCertification(modified, userId, zert); - redundantCerts += 1; log.add(LogLevel.DEBUG, LogType.MSG_KC_UID_REVOKE_DUP, indent, userId); + modified = PGPPublicKey.removeCertification(modified, userId, zert); + redundantCerts += 1; } break; @@ -442,9 +443,9 @@ public class UncachedKeyRing { // If no valid certificate (if only a revocation) remains, drop it if (selfCert == null && revocation == null) { - modified = PGPPublicKey.removeCertification(modified, userId); log.add(LogLevel.ERROR, LogType.MSG_KC_UID_REMOVE, indent, userId); + modified = PGPPublicKey.removeCertification(modified, userId); } } From caad5d1fc15312da3cc665bf7a5c42b64306c140 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Thu, 31 Jul 2014 23:07:20 +0200 Subject: [PATCH 2/2] fix bug in UploadKeyActivity, minor work on uri handling --- .../keychain/provider/ProviderHelper.java | 6 +++ .../keychain/ui/UploadKeyActivity.java | 3 +- .../keychain/ui/ViewKeyActivity.java | 37 +++++++------------ 3 files changed, 21 insertions(+), 25 deletions(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java index 9b35903f6..75d167d6b 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java @@ -233,6 +233,12 @@ public class ProviderHelper { } private KeyRing getCanonicalizedKeyRing(Uri queryUri, boolean secret) throws NotFoundException { + + // if this is not a unified query, we /will/ get hard to trace errors below! + if ( ! queryUri.getPath().contains("unified")) { + throw new RuntimeException("only unified uris can be passed to getCanonicalizedKeyRing!"); + } + Cursor cursor = mContentResolver.query(queryUri, new String[]{ // we pick from cache only information that is not easily available from keyrings diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/UploadKeyActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/UploadKeyActivity.java index dbd1b7507..a2c5a0d90 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/UploadKeyActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/UploadKeyActivity.java @@ -36,6 +36,7 @@ import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.R; import org.sufficientlysecure.keychain.helper.Preferences; import org.sufficientlysecure.keychain.provider.KeychainContract; +import org.sufficientlysecure.keychain.provider.KeychainContract.KeyRings; import org.sufficientlysecure.keychain.service.KeychainIntentService; import org.sufficientlysecure.keychain.service.KeychainIntentServiceHandler; import org.sufficientlysecure.keychain.util.Log; @@ -92,7 +93,7 @@ public class UploadKeyActivity extends ActionBarActivity { intent.setAction(KeychainIntentService.ACTION_UPLOAD_KEYRING); // set data uri as path to keyring - Uri blobUri = KeychainContract.KeyRingData.buildPublicKeyRingUri(mDataUri); + Uri blobUri = KeyRings.buildUnifiedKeyRingUri(mDataUri); intent.setData(blobUri); // fill values for this action diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ViewKeyActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ViewKeyActivity.java index 03e446723..b6b35a804 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ViewKeyActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ViewKeyActivity.java @@ -145,20 +145,27 @@ public class ViewKeyActivity extends ActionBarActivity implements switchToTab = intent.getExtras().getInt(EXTRA_SELECTED_TAB); } - Uri dataUri = getDataUri(); - if (dataUri == null) { - Log.e(Constants.TAG, "Data missing. Should be Uri of key!"); + mDataUri = getIntent().getData(); + if (mDataUri == null) { + Log.e(Constants.TAG, "Data missing. Should be uri of key!"); finish(); return; } + if (mDataUri.getHost().equals(ContactsContract.AUTHORITY)) { + mDataUri = ContactHelper.dataUriFromContactUri(this, mDataUri); + } - loadData(dataUri); + Log.i(Constants.TAG, "mDataUri: " + mDataUri.toString()); - initNfc(dataUri); + // Prepare the loaders. Either re-connect with an existing ones, + // or start new ones. + getSupportLoaderManager().initLoader(LOADER_ID_UNIFIED, null, this); + + initNfc(mDataUri); mShowAdvancedTabs = false; - initTabs(dataUri); + initTabs(mDataUri); // switch to tab selected by extra mViewPager.setCurrentItem(switchToTab); @@ -235,24 +242,6 @@ public class ViewKeyActivity extends ActionBarActivity implements mSlidingTabLayout.setViewPager(mViewPager); } - private Uri getDataUri() { - Uri dataUri = getIntent().getData(); - if (dataUri != null && dataUri.getHost().equals(ContactsContract.AUTHORITY)) { - dataUri = ContactHelper.dataUriFromContactUri(this, dataUri); - } - return dataUri; - } - - private void loadData(Uri dataUri) { - mDataUri = dataUri; - - Log.i(Constants.TAG, "mDataUri: " + mDataUri.toString()); - - // Prepare the loaders. Either re-connect with an existing ones, - // or start new ones. - getSupportLoaderManager().initLoader(LOADER_ID_UNIFIED, null, this); - } - @Override public boolean onCreateOptionsMenu(Menu menu) { super.onCreateOptionsMenu(menu);