From dae503284f47eb7e5eed71140f9fceaa2ff420c2 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Thu, 12 Jun 2014 17:38:48 +0200 Subject: canonicalize: more stuff --- .../keychain/keyimport/ParcelableKeyRing.java | 2 +- .../keychain/pgp/UncachedKeyRing.java | 79 +++++++++++++--------- .../keychain/service/OperationResultParcel.java | 17 +++-- .../keychain/ui/ImportKeysActivity.java | 2 + OpenKeychain/src/main/res/values/strings.xml | 17 +++-- 5 files changed, 70 insertions(+), 47 deletions(-) (limited to 'OpenKeychain/src/main') diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/ParcelableKeyRing.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/ParcelableKeyRing.java index 5da6c4cd3..fdf561aaf 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/ParcelableKeyRing.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/ParcelableKeyRing.java @@ -3,7 +3,7 @@ package org.sufficientlysecure.keychain.keyimport; import android.os.Parcel; import android.os.Parcelable; -/** This is a trivial wrapper around UncachedKeyRing which implements Parcelable. It exists +/** This is a trivial wrapper around keyring bytes which implements Parcelable. It exists * for the sole purpose of keeping spongycastle and android imports in separate packages. */ public class ParcelableKeyRing implements Parcelable { 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 215c17590..a8e4820cf 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java @@ -15,7 +15,6 @@ import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException; import org.sufficientlysecure.keychain.service.OperationResultParcel.OperationLog; import org.sufficientlysecure.keychain.service.OperationResultParcel.LogLevel; import org.sufficientlysecure.keychain.service.OperationResultParcel.LogType; -import org.sufficientlysecure.keychain.service.OperationResults.SaveKeyringResult; import org.sufficientlysecure.keychain.util.IterableIterator; import org.sufficientlysecure.keychain.util.Log; @@ -24,7 +23,6 @@ import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.util.ArrayList; import java.util.HashSet; import java.util.Iterator; import java.util.List; @@ -229,7 +227,7 @@ public class UncachedKeyRing { if (type != PGPSignature.KEY_REVOCATION) { // Unknown type, just remove - log.add(LogLevel.WARN, LogType.MSG_KC_CERT_BAD_TYPE, new String[]{ + log.add(LogLevel.WARN, LogType.MSG_KC_SUB_BAD_TYPE, new String[]{ "0x" + Integer.toString(type, 16) }, indent); modified = PGPPublicKey.removeCertification(modified, zert); @@ -380,81 +378,98 @@ public class UncachedKeyRing { } // Process all keys - // NOTE we rely here on the special property that this iterator is independent from the keyring! for (PGPPublicKey key : new IterableIterator(ring.getPublicKeys())) { - // Only care about the master key here! + // Don't care about the master key here, that one gets special treatment above if (key.isMasterKey()) { continue; } - log.add(LogLevel.DEBUG, LogType.MSG_KC_SUBKEY, + log.add(LogLevel.DEBUG, LogType.MSG_KC_SUB, new String[]{PgpKeyHelper.convertKeyIdToHex(key.getKeyID())}, indent); indent += 1; // A subkey needs exactly one subkey binding certificate, and optionally one revocation // certificate. PGPPublicKey modified = key; - PGPSignature cert = null, revocation = null; + PGPSignature selfCert = null, revocation = null; for (PGPSignature zig : new IterableIterator(key.getSignatures())) { // remove from keyring (for now) modified = PGPPublicKey.removeCertification(modified, zig); - WrappedSignature sig = new WrappedSignature(zig); - int type = sig.getSignatureType(); + WrappedSignature cert = new WrappedSignature(zig); + int type = cert.getSignatureType(); // filter out bad key types... - if (sig.getKeyId() != masterKey.getKeyID()) { - log.add(LogLevel.WARN, LogType.MSG_KC_CERT_BAD_KEYID, null, indent); + if (cert.getKeyId() != masterKey.getKeyID()) { + log.add(LogLevel.WARN, LogType.MSG_KC_SUB_BAD_KEYID, null, indent); continue; } if (type != PGPSignature.SUBKEY_BINDING && type != PGPSignature.SUBKEY_REVOCATION) { - log.add(LogLevel.WARN, LogType.MSG_KC_CERT_BAD_TYPE, new String[]{ + log.add(LogLevel.WARN, LogType.MSG_KC_SUB_BAD_TYPE, new String[]{ "0x" + Integer.toString(type, 16) }, indent); continue; } - // make sure the certificate checks out - try { - sig.init(masterKey); - if (!sig.verifySignature(masterKey, key)) { - log.add(LogLevel.WARN, LogType.MSG_KC_CERT_BAD, null, indent); + if (type == PGPSignature.SUBKEY_BINDING) { + // TODO verify primary key binding signature for signing keys! + + // make sure the certificate checks out + try { + cert.init(masterKey); + if (!cert.verifySignature(masterKey, key)) { + log.add(LogLevel.WARN, LogType.MSG_KC_SUB_BAD, null, indent); + log.add(LogLevel.WARN, LogType.MSG_KC_SUB, new String[] { + cert.getCreationTime().toString() + }, indent); + continue; + } + } catch (PgpGeneralException e) { + log.add(LogLevel.WARN, LogType.MSG_KC_SUB_BAD_ERR, null, indent); continue; } - } catch (PgpGeneralException e) { - log.add(LogLevel.WARN, LogType.MSG_KC_CERT_BAD_ERR, null, indent); - continue; - } - if (type == PGPSignature.SUBKEY_BINDING) { - // TODO verify primary key binding signature for signing keys! // if we already have a cert, and this one is not newer: skip it - if (cert != null && cert.getCreationTime().before(sig.getCreationTime())) { + if (selfCert != null && selfCert.getCreationTime().before(cert.getCreationTime())) { continue; } - cert = zig; + selfCert = zig; // if this is newer than a possibly existing revocation, drop that one - if (revocation != null && cert.getCreationTime().after(revocation.getCreationTime())) { + if (revocation != null && selfCert.getCreationTime().after(revocation.getCreationTime())) { revocation = null; } - // it must be a revocation, then (we made sure above) + + // it must be a revocation, then (we made sure above) } else { + + // make sure the certificate checks out + try { + cert.init(masterKey); + if (!cert.verifySignature(key)) { + log.add(LogLevel.WARN, LogType.MSG_KC_SUB_REVOKE_BAD, null, indent); + continue; + } + } catch (PgpGeneralException e) { + log.add(LogLevel.WARN, LogType.MSG_KC_SUB_REVOKE_BAD_ERR, null, indent); + continue; + } + // if there is no binding (yet), or the revocation is newer than the binding: keep it - if (cert == null || cert.getCreationTime().before(sig.getCreationTime())) { + if (selfCert == null || selfCert.getCreationTime().before(cert.getCreationTime())) { revocation = zig; } } } // it is not properly bound? error! - if (cert == null) { + if (selfCert == null) { ring = PGPPublicKeyRing.removePublicKey(ring, modified); - log.add(LogLevel.ERROR, LogType.MSG_KC_SUBKEY_NO_CERT, + log.add(LogLevel.ERROR, LogType.MSG_KC_SUB_NO_CERT, new String[]{PgpKeyHelper.convertKeyIdToHex(key.getKeyID())}, indent); indent -= 1; continue; } // re-add certification - modified = PGPPublicKey.addCertification(modified, cert); + modified = PGPPublicKey.addCertification(modified, selfCert); // add revocation, if any if (revocation != null) { modified = PGPPublicKey.addCertification(modified, revocation); @@ -462,7 +477,7 @@ public class UncachedKeyRing { // replace pubkey in keyring ring = PGPPublicKeyRing.insertPublicKey(ring, modified); - log.add(LogLevel.DEBUG, LogType.MSG_KC_SUBKEY_SUCCESS, null, indent); + log.add(LogLevel.DEBUG, LogType.MSG_KC_SUB_SUCCESS, null, indent); indent -= 1; } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java index 48f40026e..5c223e870 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java @@ -158,18 +158,21 @@ public class OperationResultParcel implements Parcelable { // keyring canonicalization MSG_KC (R.string.msg_kc), - MSG_KC_CERT_BAD_ERR (R.string.msg_kc_cert_bad_err), - MSG_KC_CERT_BAD_KEYID (R.string.msg_kc_cert_bad_keyid), - MSG_KC_CERT_BAD (R.string.msg_kc_cert_bad), - MSG_KC_CERT_BAD_TYPE (R.string.msg_kc_cert_bad_type), MSG_KC_MASTER (R.string.msg_kc_master), MSG_KC_MASTER_SUCCESS (R.string.msg_kc_master_success), MSG_KC_REVOKE_BAD_ERR (R.string.msg_kc_revoke_bad_err), MSG_KC_REVOKE_BAD (R.string.msg_kc_revoke_bad), MSG_KC_REVOKE_DUP (R.string.msg_kc_revoke_dup), - MSG_KC_SUBKEY_NO_CERT (R.string.msg_kc_subkey_no_cert), - MSG_KC_SUBKEY (R.string.msg_kc_subkey), - MSG_KC_SUBKEY_SUCCESS (R.string.msg_kc_subkey_success), + MSG_KC_SUB (R.string.msg_kc_sub), + MSG_KC_SUB_BAD_ERR(R.string.msg_kc_sub_bad_err), + MSG_KC_SUB_BAD_KEYID(R.string.msg_kc_sub_bad_keyid), + MSG_KC_SUB_BAD(R.string.msg_kc_sub_bad), + MSG_KC_SUB_BAD_TYPE(R.string.msg_kc_sub_bad_type), + MSG_KC_SUB_NO_CERT(R.string.msg_kc_sub_no_cert), + MSG_KC_SUB_REVOKE_BAD_ERR (R.string.msg_kc_sub_revoke_bad_err), + MSG_KC_SUB_REVOKE_BAD (R.string.msg_kc_sub_revoke_bad), + MSG_KC_SUB_REVOKE_DUP (R.string.msg_kc_sub_revoke_dup), + MSG_KC_SUB_SUCCESS (R.string.msg_kc_sub_success), MSG_KC_SUCCESS_REMOVED (R.string.msg_kc_success_removed), MSG_KC_SUCCESS (R.string.msg_kc_success), MSG_KC_UID_BAD_ERR (R.string.msg_kc_uid_bad_err), diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ImportKeysActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ImportKeysActivity.java index 7fa18406c..f389726ff 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ImportKeysActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ImportKeysActivity.java @@ -456,10 +456,12 @@ public class ImportKeysActivity extends ActionBarActivity implements ActionBar.O } */ + /* if (ACTION_IMPORT_KEY_FROM_KEYSERVER_AND_RETURN.equals(getIntent().getAction())) { ImportKeysActivity.this.setResult(Activity.RESULT_OK, mPendingIntentData); finish(); } + */ } } }; diff --git a/OpenKeychain/src/main/res/values/strings.xml b/OpenKeychain/src/main/res/values/strings.xml index 27568132b..bcf66d58f 100644 --- a/OpenKeychain/src/main/res/values/strings.xml +++ b/OpenKeychain/src/main/res/values/strings.xml @@ -559,18 +559,21 @@ Canonicalizing keyring %s - Certificate verification failed! - Certificate verification failed with an error! - Certificate issuer id mismatch - Unknown certificate type: %s Processing master key OK Removing bad keyring revocation key Removing bad keyring revocation key Removing redundant keyring revocation key - No valid certificate found for %s, removing from ring! - Processing subkey %s - OK + Processing subkey %s + Subkey binding certificate verification failed! + Subkey binding certificate verification failed with an error! + Subkey binding issuer id mismatch + Unknown subkey certificate type: %s + No valid certificate found for %s, removing from ring! + Removing bad subkey revocation key + Removing bad subkey revocation key + Removing redundant keyring revocation key + OK Keyring canonicalization successful Keyring canonicalization successful, removed %s certificates Removing bad self certificate for user id %s -- cgit v1.2.3