From c84a1ecfff8d3f21ac83ef7b041f22f11e38936b Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Thu, 5 Jun 2014 14:06:07 +0200 Subject: import-log: add parcelable prototype --- .../keychain/pgp/OperationResultParcel.java | 123 +++++++++++++++++++++ 1 file changed, 123 insertions(+) create mode 100644 OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OperationResultParcel.java (limited to 'OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp') diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OperationResultParcel.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OperationResultParcel.java new file mode 100644 index 000000000..497c3dd71 --- /dev/null +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OperationResultParcel.java @@ -0,0 +1,123 @@ +package org.sufficientlysecure.keychain.pgp; + +import android.R; +import android.os.Parcel; +import android.os.Parcelable; + +import java.util.ArrayList; + +/** Represent the result of an operation. + * + * This class holds a result and the log of an operation. It can be subclassed + * to include typed additional information specific to the operation. To keep + * the class structure (somewhat) simple, this class contains an exhaustive + * list (ie, enum) of all possible log types, which should in all cases be tied + * to string resource ids. + * + */ +public class OperationResultParcel implements Parcelable { + /** Holds the overall result. A value of 0 is considered a success, all + * other values may represent failure or varying degrees of success. */ + final int mResult; + + /// A list of log entries tied to the operation result. + final ArrayList mLog; + + public OperationResultParcel(int result, ArrayList log) { + mResult = result; + mLog = log; + } + + public OperationResultParcel(Parcel source) { + mResult = source.readInt(); + mLog = source.createTypedArrayList(LogEntryParcel.CREATOR); + } + + public boolean isSuccessful() { + return mResult == 0; + } + + /** One entry in the log. */ + public static class LogEntryParcel implements Parcelable { + final LogType mType; + final LogLevel mLevel; + final String[] mParameters; + + public LogEntryParcel(LogType type, LogLevel level, String[] parameters) { + mType = type; + mLevel = level; + mParameters = parameters; + } + + public LogEntryParcel(Parcel source) { + mType = LogType.values()[source.readInt()]; + mLevel = LogLevel.values()[source.readInt()]; + mParameters = source.createStringArray(); + } + + @Override + public int describeContents() { + return 0; + } + + @Override + public void writeToParcel(Parcel dest, int flags) { + dest.writeInt(mType.ordinal()); + dest.writeInt(mLevel.ordinal()); + dest.writeStringArray(mParameters); + } + + public static final Creator CREATOR = new Creator() { + public LogEntryParcel createFromParcel(final Parcel source) { + return new LogEntryParcel(source); + } + + public LogEntryParcel[] newArray(final int size) { + return new LogEntryParcel[size]; + } + }; + + } + + public static enum LogType { + // TODO add actual log entry types here + MSG_IMPORT_OK (R.string.copy), + MSG_IMPORT_FAILED (R.string.cancel); + + private int mMsgId; + LogType(int msgId) { + mMsgId = msgId; + } + } + + /** Enumeration of possible log levels. */ + public static enum LogLevel { + DEBUG, + INFO, + WARN, + /** If any ERROR log entry is included in the result, the overall operation should have failed. */ + ERROR, + } + + @Override + public int describeContents() { + return 0; + } + + @Override + public void writeToParcel(Parcel dest, int flags) { + dest.writeInt(mResult); + dest.writeTypedList(mLog); + } + + public static final Creator CREATOR = new Creator() { + public OperationResultParcel createFromParcel(final Parcel source) { + return new OperationResultParcel(source); + } + + public OperationResultParcel[] newArray(final int size) { + return new OperationResultParcel[size]; + } + }; + +} -- cgit v1.2.3 From b995b836a38ef8a87aa189f408f8542bf5a2c94c Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Fri, 6 Jun 2014 16:14:15 +0200 Subject: import-log: improve operationresultparcel, add indentation --- .../keychain/pgp/OperationResultParcel.java | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) (limited to 'OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp') diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OperationResultParcel.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OperationResultParcel.java index 497c3dd71..f2da4389d 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OperationResultParcel.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OperationResultParcel.java @@ -1,9 +1,10 @@ package org.sufficientlysecure.keychain.pgp; -import android.R; import android.os.Parcel; import android.os.Parcelable; +import org.sufficientlysecure.keychain.R; + import java.util.ArrayList; /** Represent the result of an operation. @@ -39,20 +40,26 @@ public class OperationResultParcel implements Parcelable { /** One entry in the log. */ public static class LogEntryParcel implements Parcelable { - final LogType mType; final LogLevel mLevel; + final LogType mType; final String[] mParameters; + final int mIndent; - public LogEntryParcel(LogType type, LogLevel level, String[] parameters) { - mType = type; + public LogEntryParcel(LogLevel level, LogType type, String[] parameters, int indent) { mLevel = level; + mType = type; mParameters = parameters; + mIndent = indent; + } + public LogEntryParcel(LogLevel level, LogType type, String[] parameters) { + this(level, type, parameters, 0); } public LogEntryParcel(Parcel source) { - mType = LogType.values()[source.readInt()]; mLevel = LogLevel.values()[source.readInt()]; + mType = LogType.values()[source.readInt()]; mParameters = source.createStringArray(); + mIndent = source.readInt(); } @Override @@ -62,9 +69,10 @@ public class OperationResultParcel implements Parcelable { @Override public void writeToParcel(Parcel dest, int flags) { - dest.writeInt(mType.ordinal()); dest.writeInt(mLevel.ordinal()); + dest.writeInt(mType.ordinal()); dest.writeStringArray(mParameters); + dest.writeInt(mIndent); } public static final Creator CREATOR = new Creator() { -- cgit v1.2.3 From 787f6edf3260056030025388b921e9d6ce996d92 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Fri, 6 Jun 2014 16:15:27 +0200 Subject: import-log: add log statements in import routine --- .../keychain/pgp/OperationResultParcel.java | 40 ++++++++++++++++++++-- 1 file changed, 37 insertions(+), 3 deletions(-) (limited to 'OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp') diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OperationResultParcel.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OperationResultParcel.java index f2da4389d..8110590b1 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OperationResultParcel.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OperationResultParcel.java @@ -88,9 +88,43 @@ public class OperationResultParcel implements Parcelable { } public static enum LogType { - // TODO add actual log entry types here - MSG_IMPORT_OK (R.string.copy), - MSG_IMPORT_FAILED (R.string.cancel); + MSG_IP_APPLY_BATCH (R.string.msg_ip_apply_batch), + MSG_IP_BAD_TYPE_SECRET (R.string.msg_ip_bad_type_secret), + MSG_IP_DELETE_OLD_FAIL (R.string.msg_ip_delete_old_fail), + MSG_IP_DELETE_OLD_OK (R.string.msg_ip_delete_old_ok), + MSG_IP_ENCODE_FAIL (R.string.msg_ip_encode_fail), + MSG_IP_FAIL_IO_EXC (R.string.msg_ip_fail_io_exc), + MSG_IP_FAIL_OP_EX (R.string.msg_ip_fail_op_ex), + MSG_IP_FAIL_REMOTE_EX (R.string.msg_ip_fail_remote_ex), + MSG_IP_IMPORTING (R.string.msg_ip_importing), + MSG_IP_INSERT_KEYRING (R.string.msg_ip_insert_keyring), + MSG_IP_INSERT_SUBKEY (R.string.msg_ip_insert_subkey), + MSG_IP_INSERT_SUBKEYS (R.string.msg_ip_insert_subkeys), + MSG_IP_PRESERVING_SECRET (R.string.msg_ip_preserving_secret), + MSG_IP_REINSERT_SECRET (R.string.msg_ip_reinsert_secret), + MSG_IP_SUCCESS (R.string.msg_ip_success), + MSG_IP_TRUST_RETRIEVE (R.string.msg_ip_trust_retrieve), + MSG_IP_TRUST_USING (R.string.msg_ip_trust_using), + MSG_IP_TRUST_USING_SEC (R.string.msg_ip_trust_using_sec), + MSG_IP_UID_CERT_BAD (R.string.msg_ip_uid_cert_bad), + MSG_IP_UID_CERT_ERROR (R.string.msg_ip_uid_cert_error), + MSG_IP_UID_CERT_GOOD (R.string.msg_ip_uid_cert_good), + MSG_IP_UID_CERTS_UNKNOWN (R.string.msg_ip_uid_certs_unknown), + MSG_IP_UID_CLASSIFYING (R.string.msg_ip_uid_classifying), + MSG_IP_UID_INSERT (R.string.msg_ip_uid_insert), + MSG_IP_UID_PROCESSING (R.string.msg_ip_uid_processing), + MSG_IP_UID_SELF_BAD (R.string.msg_ip_uid_self_bad), + MSG_IP_UID_SELF_GOOD (R.string.msg_ip_uid_self_good), + MSG_IP_UID_SELF_IGNORING_OLD (R.string.msg_ip_uid_self_ignoring_old), + MSG_IP_UID_SELF_NEWER (R.string.msg_ip_uid_self_newer), + MSG_IS_BAD_TYPE_PUBLIC (R.string.msg_is_bad_type_public), + MSG_IS_IMPORTING (R.string.msg_is_importing), + MSG_IS_IMPORTING_SUBKEYS (R.string.msg_is_importing_subkeys), + MSG_IS_IO_EXCPTION (R.string.msg_is_io_excption), + MSG_IS_SUBKEY_NONEXISTENT (R.string.msg_is_subkey_nonexistent), + MSG_IS_SUBKEY_OK (R.string.msg_is_subkey_ok), + MSG_IS_SUCCESS (R.string.msg_is_success), + ; private int mMsgId; LogType(int msgId) { -- cgit v1.2.3 From 118225d7d2b6401006352897386a40131c6cd854 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Fri, 6 Jun 2014 17:28:36 +0200 Subject: import-log: add output to logcat (for debugging) --- .../keychain/pgp/OperationResultParcel.java | 5 ++++- .../sufficientlysecure/keychain/pgp/PgpImportExport.java | 15 ++++++++------- 2 files changed, 12 insertions(+), 8 deletions(-) (limited to 'OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp') diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OperationResultParcel.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OperationResultParcel.java index 8110590b1..ccb4b935c 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OperationResultParcel.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OperationResultParcel.java @@ -126,10 +126,13 @@ public class OperationResultParcel implements Parcelable { MSG_IS_SUCCESS (R.string.msg_is_success), ; - private int mMsgId; + private final int mMsgId; LogType(int msgId) { mMsgId = msgId; } + public int getMsgId() { + return mMsgId; + } } /** Enumeration of possible log levels. */ diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpImportExport.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpImportExport.java index 14ec67e64..5ce0b11dd 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpImportExport.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpImportExport.java @@ -152,13 +152,14 @@ public class PgpImportExport { } } - mProviderHelper.savePublicKeyRing(key); - /*switch(status) { - case RETURN_UPDATED: oldKeys++; break; - case RETURN_OK: newKeys++; break; - case RETURN_BAD: badKeys++; break; - }*/ - // TODO proper import feedback + mProviderHelper.resetLog(); + OperationResultParcel result = mProviderHelper.savePublicKeyRing(key); + for(OperationResultParcel.LogEntryParcel loge : result.mLog) { + Log.d(Constants.TAG, + loge.mIndent + + new String(new char[loge.mIndent]).replace("\0", " ") + + mContext.getString(loge.mType.getMsgId(), (Object[]) loge.mParameters)); + } newKeys += 1; } catch (PgpGeneralException e) { -- cgit v1.2.3 From c36b311d5f5c2b43ec3f1549d9a059fb0de3bde2 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Fri, 6 Jun 2014 17:29:39 +0200 Subject: import-log: better stripped key logging --- .../org/sufficientlysecure/keychain/pgp/OperationResultParcel.java | 1 + .../java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) (limited to 'OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp') diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OperationResultParcel.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OperationResultParcel.java index ccb4b935c..216e4b497 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OperationResultParcel.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OperationResultParcel.java @@ -123,6 +123,7 @@ public class OperationResultParcel implements Parcelable { MSG_IS_IO_EXCPTION (R.string.msg_is_io_excption), MSG_IS_SUBKEY_NONEXISTENT (R.string.msg_is_subkey_nonexistent), MSG_IS_SUBKEY_OK (R.string.msg_is_subkey_ok), + MSG_IS_SUBKEY_STRIPPED (R.string.msg_is_subkey_stripped), MSG_IS_SUCCESS (R.string.msg_is_success), ; 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 02e5411ca..1264c8c36 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java @@ -19,6 +19,7 @@ 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; import java.util.Vector; @@ -149,13 +150,13 @@ public class UncachedKeyRing { aos.close(); } - public ArrayList getAvailableSubkeys() { + public HashSet getAvailableSubkeys() { if(!isSecret()) { throw new RuntimeException("Tried to find available subkeys from non-secret keys. " + "This is a programming error and should never happen!"); } - ArrayList result = new ArrayList(); + HashSet result = new HashSet(); // then, mark exactly the keys we have available for (PGPSecretKey sub : new IterableIterator( ((PGPSecretKeyRing) mRing).getSecretKeys())) { -- cgit v1.2.3 From 067ffa876d4ec4e1f2ee63c851cb3c48f7eb1aa2 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Tue, 10 Jun 2014 01:30:20 +0200 Subject: import-log: add OperationResults, use it in ImportKeys operation --- .../keychain/pgp/OperationResultParcel.java | 169 --------------------- .../keychain/pgp/PgpImportExport.java | 45 ++++-- 2 files changed, 29 insertions(+), 185 deletions(-) delete mode 100644 OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OperationResultParcel.java (limited to 'OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp') diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OperationResultParcel.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OperationResultParcel.java deleted file mode 100644 index 216e4b497..000000000 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OperationResultParcel.java +++ /dev/null @@ -1,169 +0,0 @@ -package org.sufficientlysecure.keychain.pgp; - -import android.os.Parcel; -import android.os.Parcelable; - -import org.sufficientlysecure.keychain.R; - -import java.util.ArrayList; - -/** Represent the result of an operation. - * - * This class holds a result and the log of an operation. It can be subclassed - * to include typed additional information specific to the operation. To keep - * the class structure (somewhat) simple, this class contains an exhaustive - * list (ie, enum) of all possible log types, which should in all cases be tied - * to string resource ids. - * - */ -public class OperationResultParcel implements Parcelable { - /** Holds the overall result. A value of 0 is considered a success, all - * other values may represent failure or varying degrees of success. */ - final int mResult; - - /// A list of log entries tied to the operation result. - final ArrayList mLog; - - public OperationResultParcel(int result, ArrayList log) { - mResult = result; - mLog = log; - } - - public OperationResultParcel(Parcel source) { - mResult = source.readInt(); - mLog = source.createTypedArrayList(LogEntryParcel.CREATOR); - } - - public boolean isSuccessful() { - return mResult == 0; - } - - /** One entry in the log. */ - public static class LogEntryParcel implements Parcelable { - final LogLevel mLevel; - final LogType mType; - final String[] mParameters; - final int mIndent; - - public LogEntryParcel(LogLevel level, LogType type, String[] parameters, int indent) { - mLevel = level; - mType = type; - mParameters = parameters; - mIndent = indent; - } - public LogEntryParcel(LogLevel level, LogType type, String[] parameters) { - this(level, type, parameters, 0); - } - - public LogEntryParcel(Parcel source) { - mLevel = LogLevel.values()[source.readInt()]; - mType = LogType.values()[source.readInt()]; - mParameters = source.createStringArray(); - mIndent = source.readInt(); - } - - @Override - public int describeContents() { - return 0; - } - - @Override - public void writeToParcel(Parcel dest, int flags) { - dest.writeInt(mLevel.ordinal()); - dest.writeInt(mType.ordinal()); - dest.writeStringArray(mParameters); - dest.writeInt(mIndent); - } - - public static final Creator CREATOR = new Creator() { - public LogEntryParcel createFromParcel(final Parcel source) { - return new LogEntryParcel(source); - } - - public LogEntryParcel[] newArray(final int size) { - return new LogEntryParcel[size]; - } - }; - - } - - public static enum LogType { - MSG_IP_APPLY_BATCH (R.string.msg_ip_apply_batch), - MSG_IP_BAD_TYPE_SECRET (R.string.msg_ip_bad_type_secret), - MSG_IP_DELETE_OLD_FAIL (R.string.msg_ip_delete_old_fail), - MSG_IP_DELETE_OLD_OK (R.string.msg_ip_delete_old_ok), - MSG_IP_ENCODE_FAIL (R.string.msg_ip_encode_fail), - MSG_IP_FAIL_IO_EXC (R.string.msg_ip_fail_io_exc), - MSG_IP_FAIL_OP_EX (R.string.msg_ip_fail_op_ex), - MSG_IP_FAIL_REMOTE_EX (R.string.msg_ip_fail_remote_ex), - MSG_IP_IMPORTING (R.string.msg_ip_importing), - MSG_IP_INSERT_KEYRING (R.string.msg_ip_insert_keyring), - MSG_IP_INSERT_SUBKEY (R.string.msg_ip_insert_subkey), - MSG_IP_INSERT_SUBKEYS (R.string.msg_ip_insert_subkeys), - MSG_IP_PRESERVING_SECRET (R.string.msg_ip_preserving_secret), - MSG_IP_REINSERT_SECRET (R.string.msg_ip_reinsert_secret), - MSG_IP_SUCCESS (R.string.msg_ip_success), - MSG_IP_TRUST_RETRIEVE (R.string.msg_ip_trust_retrieve), - MSG_IP_TRUST_USING (R.string.msg_ip_trust_using), - MSG_IP_TRUST_USING_SEC (R.string.msg_ip_trust_using_sec), - MSG_IP_UID_CERT_BAD (R.string.msg_ip_uid_cert_bad), - MSG_IP_UID_CERT_ERROR (R.string.msg_ip_uid_cert_error), - MSG_IP_UID_CERT_GOOD (R.string.msg_ip_uid_cert_good), - MSG_IP_UID_CERTS_UNKNOWN (R.string.msg_ip_uid_certs_unknown), - MSG_IP_UID_CLASSIFYING (R.string.msg_ip_uid_classifying), - MSG_IP_UID_INSERT (R.string.msg_ip_uid_insert), - MSG_IP_UID_PROCESSING (R.string.msg_ip_uid_processing), - MSG_IP_UID_SELF_BAD (R.string.msg_ip_uid_self_bad), - MSG_IP_UID_SELF_GOOD (R.string.msg_ip_uid_self_good), - MSG_IP_UID_SELF_IGNORING_OLD (R.string.msg_ip_uid_self_ignoring_old), - MSG_IP_UID_SELF_NEWER (R.string.msg_ip_uid_self_newer), - MSG_IS_BAD_TYPE_PUBLIC (R.string.msg_is_bad_type_public), - MSG_IS_IMPORTING (R.string.msg_is_importing), - MSG_IS_IMPORTING_SUBKEYS (R.string.msg_is_importing_subkeys), - MSG_IS_IO_EXCPTION (R.string.msg_is_io_excption), - MSG_IS_SUBKEY_NONEXISTENT (R.string.msg_is_subkey_nonexistent), - MSG_IS_SUBKEY_OK (R.string.msg_is_subkey_ok), - MSG_IS_SUBKEY_STRIPPED (R.string.msg_is_subkey_stripped), - MSG_IS_SUCCESS (R.string.msg_is_success), - ; - - private final int mMsgId; - LogType(int msgId) { - mMsgId = msgId; - } - public int getMsgId() { - return mMsgId; - } - } - - /** Enumeration of possible log levels. */ - public static enum LogLevel { - DEBUG, - INFO, - WARN, - /** If any ERROR log entry is included in the result, the overall operation should have failed. */ - ERROR, - } - - @Override - public int describeContents() { - return 0; - } - - @Override - public void writeToParcel(Parcel dest, int flags) { - dest.writeInt(mResult); - dest.writeTypedList(mLog); - } - - public static final Creator CREATOR = new Creator() { - public OperationResultParcel createFromParcel(final Parcel source) { - return new OperationResultParcel(source); - } - - public OperationResultParcel[] newArray(final int size) { - return new OperationResultParcel[size]; - } - }; - -} diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpImportExport.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpImportExport.java index 5ce0b11dd..ebc5a7868 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpImportExport.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpImportExport.java @@ -33,6 +33,9 @@ import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException; import org.sufficientlysecure.keychain.provider.KeychainContract; import org.sufficientlysecure.keychain.provider.ProviderHelper; import org.sufficientlysecure.keychain.service.KeychainIntentService; +import org.sufficientlysecure.keychain.service.OperationResultParcel; +import org.sufficientlysecure.keychain.service.OperationResultParcel.OperationLog; +import org.sufficientlysecure.keychain.service.OperationResults.ImportResult; import org.sufficientlysecure.keychain.util.Log; import java.io.ByteArrayOutputStream; @@ -123,12 +126,9 @@ public class PgpImportExport { } } - /** - * Imports keys from given data. If keyIds is given only those are imported - */ - public Bundle importKeyRings(List entries) + /** Imports keys from given data. If keyIds is given only those are imported */ + public ImportResult importKeyRings(List entries) throws PgpGeneralException, PGPException, IOException { - Bundle returnData = new Bundle(); updateProgress(R.string.progress_importing, 0, 100); @@ -152,14 +152,8 @@ public class PgpImportExport { } } - mProviderHelper.resetLog(); OperationResultParcel result = mProviderHelper.savePublicKeyRing(key); - for(OperationResultParcel.LogEntryParcel loge : result.mLog) { - Log.d(Constants.TAG, - loge.mIndent - + new String(new char[loge.mIndent]).replace("\0", " ") - + mContext.getString(loge.mType.getMsgId(), (Object[]) loge.mParameters)); - } + newKeys += 1; } catch (PgpGeneralException e) { @@ -171,11 +165,30 @@ public class PgpImportExport { updateProgress(position / entries.size() * 100, 100); } - returnData.putInt(KeychainIntentService.RESULT_IMPORT_ADDED, newKeys); - returnData.putInt(KeychainIntentService.RESULT_IMPORT_UPDATED, oldKeys); - returnData.putInt(KeychainIntentService.RESULT_IMPORT_BAD, badKeys); + OperationLog log = mProviderHelper.getLog(); + int resultType; + // Any key imported - overall success + if (newKeys > 0 || oldKeys > 0) { + if (badKeys > 0) { + resultType = ImportResult.RESULT_PARTIAL_WITH_ERRORS; + } else if (log.containsWarnings()) { + resultType = ImportResult.RESULT_OK_WITH_WARNINGS; + } else if (newKeys > 0 && oldKeys > 0) { + resultType = ImportResult.RESULT_OK_BOTHKEYS; + } else if (newKeys > 0) { + resultType = ImportResult.RESULT_OK_NEWKEYS; + } else { + resultType = ImportResult.RESULT_OK_UPDATED; + } + // No keys imported, overall failure + } else if (badKeys > 0) { + resultType = ImportResult.RESULT_FAIL_ERROR; + } else { + resultType = ImportResult.RESULT_FAIL_NOTHING; + } + + return new ImportResult(resultType, log, newKeys, oldKeys, badKeys); - return returnData; } public Bundle exportKeyRings(ArrayList publicKeyRingMasterIds, -- cgit v1.2.3 From cdc61c43927f11835fffa090ccb045d206728692 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Tue, 10 Jun 2014 01:51:16 +0200 Subject: canonicalize: first step(s) --- .../keychain/pgp/UncachedKeyRing.java | 55 ++++++++++++++++++++++ .../keychain/pgp/UncachedPublicKey.java | 12 ++++- 2 files changed, 65 insertions(+), 2 deletions(-) (limited to 'OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp') 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 1264c8c36..624b7d068 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java @@ -7,9 +7,14 @@ import org.spongycastle.openpgp.PGPObjectFactory; import org.spongycastle.openpgp.PGPPublicKey; import org.spongycastle.openpgp.PGPSecretKey; import org.spongycastle.openpgp.PGPSecretKeyRing; +import org.spongycastle.openpgp.PGPSignature; import org.spongycastle.openpgp.PGPUtil; import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException; +import org.sufficientlysecure.keychain.service.OperationResultParcel; +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.util.IterableIterator; import org.sufficientlysecure.keychain.util.Log; @@ -169,4 +174,54 @@ public class UncachedKeyRing { return result; } + /** "Canonicalizes" a key, removing inconsistencies in the process. This operation can be + * applied to public keyrings only. + * + * More specifically: + * - Remove all non-verifying self-certificates + * - Remove all expired self-certificates + * - Remove all certificates flagged as "local" + * - Remove all certificates which are superseded by a newer one on the same target + * + * After this cleaning, a number of checks are done: + * - See if each subkey retains a valid self certificate + * - See if each user id retains a valid self certificate + * + * This operation writes an OperationLog which can be used as part of a OperationResultParcel. + * + * If any of these checks fail, the operation as a whole fails and the keyring is declared + * unusable. (TODO: allow forcing of import?) + * + * TODO implement + * + * @return A canonicalized key + * + */ + public UncachedKeyRing canonicalize(OperationLog log) { + if(isSecret()) { + throw new RuntimeException("Tried to canonicalize non-secret keyring. " + + "This is a programming error and should never happen!"); + } + + // dummy + log.add(LogLevel.INFO, LogType.MSG_IP_BAD_TYPE_SECRET, null, 0); + + /* + // Remove all non-verifying self certificates + for (PGPPublicKey key : new IterableIterator(mRing.getPublicKeys())) { + + for (PGPSignature sig : new IterableIterator( + key.getSignaturesOfType(isMasterKey() ? PGPSignature.KEY_REVOCATION + : PGPSignature.SUBKEY_REVOCATION))) { + return true; + } + + }*/ + + return this; + + + } + + } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedPublicKey.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedPublicKey.java index 108c8c8c3..33db7771b 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedPublicKey.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedPublicKey.java @@ -2,6 +2,7 @@ package org.sufficientlysecure.keychain.pgp; import org.spongycastle.bcpg.SignatureSubpacketTags; import org.spongycastle.bcpg.sig.KeyFlags; +import org.spongycastle.openpgp.PGPException; import org.spongycastle.openpgp.PGPPublicKey; import org.spongycastle.openpgp.PGPSignature; import org.spongycastle.openpgp.PGPSignatureSubpacketVector; @@ -9,6 +10,7 @@ import org.spongycastle.openpgp.operator.jcajce.JcaPGPContentVerifierBuilderProv import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.util.IterableIterator; +import java.security.SignatureException; import java.util.ArrayList; import java.util.Calendar; import java.util.Date; @@ -28,8 +30,13 @@ public class UncachedPublicKey { } /** The revocation signature is NOT checked here, so this may be false! */ - public boolean maybeRevoked() { - return mPublicKey.isRevoked(); + public boolean isRevoked() { + for (PGPSignature sig : new IterableIterator( + mPublicKey.getSignaturesOfType(isMasterKey() ? PGPSignature.KEY_REVOCATION + : PGPSignature.SUBKEY_REVOCATION))) { + return true; + } + return false; } public Date getCreationTime() { @@ -193,4 +200,5 @@ public class UncachedPublicKey { } }; } + } -- cgit v1.2.3 From eac582a313c779e77b0fd67358417d512680facd Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Tue, 10 Jun 2014 13:52:05 +0200 Subject: import-log: some cosmetics --- .../java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp') 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 624b7d068..7853d0b00 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java @@ -204,7 +204,8 @@ public class UncachedKeyRing { } // dummy - log.add(LogLevel.INFO, LogType.MSG_IP_BAD_TYPE_SECRET, null, 0); + log.add(LogLevel.START, LogType.MSG_KC, + new String[] { PgpKeyHelper.convertKeyIdToHex(getMasterKeyId()) }, 0); /* // Remove all non-verifying self certificates @@ -218,6 +219,8 @@ public class UncachedKeyRing { }*/ + log.add(LogLevel.OK, LogType.MSG_KC_SUCCESS, null, 0); + return this; -- cgit v1.2.3 From e41e6ea0deec06703c5b9c80e429aff8ab110534 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Tue, 10 Jun 2014 15:27:26 +0200 Subject: import-log: more interface work --- .../main/java/org/sufficientlysecure/keychain/pgp/PgpImportExport.java | 2 -- 1 file changed, 2 deletions(-) (limited to 'OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp') diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpImportExport.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpImportExport.java index ebc5a7868..4cb41708d 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpImportExport.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpImportExport.java @@ -171,8 +171,6 @@ public class PgpImportExport { if (newKeys > 0 || oldKeys > 0) { if (badKeys > 0) { resultType = ImportResult.RESULT_PARTIAL_WITH_ERRORS; - } else if (log.containsWarnings()) { - resultType = ImportResult.RESULT_OK_WITH_WARNINGS; } else if (newKeys > 0 && oldKeys > 0) { resultType = ImportResult.RESULT_OK_BOTHKEYS; } else if (newKeys > 0) { -- cgit v1.2.3 From f38556cab1215f1e49f2e6c2e90627a4c00b02d5 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Tue, 10 Jun 2014 16:24:04 +0200 Subject: import-log: switch to flags instead of statuses for result int --- .../keychain/pgp/PgpImportExport.java | 44 ++++++++++++---------- 1 file changed, 25 insertions(+), 19 deletions(-) (limited to 'OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp') diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpImportExport.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpImportExport.java index 4cb41708d..bafb086d0 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpImportExport.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpImportExport.java @@ -33,9 +33,9 @@ import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException; import org.sufficientlysecure.keychain.provider.KeychainContract; import org.sufficientlysecure.keychain.provider.ProviderHelper; import org.sufficientlysecure.keychain.service.KeychainIntentService; -import org.sufficientlysecure.keychain.service.OperationResultParcel; import org.sufficientlysecure.keychain.service.OperationResultParcel.OperationLog; import org.sufficientlysecure.keychain.service.OperationResults.ImportResult; +import org.sufficientlysecure.keychain.service.OperationResults.SaveKeyringResult; import org.sufficientlysecure.keychain.util.Log; import java.io.ByteArrayOutputStream; @@ -152,9 +152,12 @@ public class PgpImportExport { } } - OperationResultParcel result = mProviderHelper.savePublicKeyRing(key); - - newKeys += 1; + SaveKeyringResult result = mProviderHelper.savePublicKeyRing(key); + if (result.updated()) { + newKeys += 1; + } else { + oldKeys += 1; + } } catch (PgpGeneralException e) { Log.e(Constants.TAG, "Encountered bad key on import!", e); @@ -166,23 +169,26 @@ public class PgpImportExport { } OperationLog log = mProviderHelper.getLog(); - int resultType; - // Any key imported - overall success - if (newKeys > 0 || oldKeys > 0) { + int resultType = 0; + // special return case: no new keys at all + if (badKeys == 0 && newKeys == 0 && oldKeys == 0) { + resultType = ImportResult.RESULT_FAIL_NOTHING; + } else { + if (newKeys > 0) { + resultType |= ImportResult.RESULT_OK_NEWKEYS; + } + if (oldKeys > 0) { + resultType |= ImportResult.RESULT_OK_UPDATED; + } if (badKeys > 0) { - resultType = ImportResult.RESULT_PARTIAL_WITH_ERRORS; - } else if (newKeys > 0 && oldKeys > 0) { - resultType = ImportResult.RESULT_OK_BOTHKEYS; - } else if (newKeys > 0) { - resultType = ImportResult.RESULT_OK_NEWKEYS; - } else { - resultType = ImportResult.RESULT_OK_UPDATED; + resultType |= ImportResult.RESULT_WITH_ERRORS; + if (newKeys == 0 && oldKeys == 0) { + resultType |= ImportResult.RESULT_ERROR; + } + } + if (log.containsWarnings()) { + resultType |= ImportResult.RESULT_WITH_WARNINGS; } - // No keys imported, overall failure - } else if (badKeys > 0) { - resultType = ImportResult.RESULT_FAIL_ERROR; - } else { - resultType = ImportResult.RESULT_FAIL_NOTHING; } return new ImportResult(resultType, log, newKeys, oldKeys, badKeys); -- cgit v1.2.3 From dea98a4a7e3143acfc01ce1567a9d17c25025b4d Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Thu, 12 Jun 2014 01:52:41 +0200 Subject: import-log: properly distinguish return states --- .../keychain/pgp/PgpImportExport.java | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) (limited to 'OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp') diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpImportExport.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpImportExport.java index bafb086d0..bb45cc7db 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpImportExport.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpImportExport.java @@ -58,10 +58,6 @@ public class PgpImportExport { private ProviderHelper mProviderHelper; - public static final int RETURN_OK = 0; - public static final int RETURN_BAD = -2; - public static final int RETURN_UPDATED = 1; - public PgpImportExport(Context context, Progressable progressable) { super(); this.mContext = context; @@ -118,10 +114,9 @@ public class PgpImportExport { if (aos != null) { aos.close(); } - if (bos != null) { - bos.close(); - } + bos.close(); } catch (IOException e) { + // this is just a finally thing, no matter if it doesn't work out. } } } @@ -153,10 +148,12 @@ public class PgpImportExport { } SaveKeyringResult result = mProviderHelper.savePublicKeyRing(key); - if (result.updated()) { - newKeys += 1; - } else { + if (!result.success()) { + badKeys += 1; + } else if (result.updated()) { oldKeys += 1; + } else { + newKeys += 1; } } catch (PgpGeneralException e) { -- cgit v1.2.3 From 466eddb0051d8b19576b60be7e3769f13b308a57 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Thu, 12 Jun 2014 15:36:35 +0200 Subject: canonicalize: implementation, first draft --- .../keychain/pgp/UncachedKeyRing.java | 294 +++++++++++++++++++-- .../keychain/pgp/WrappedSignature.java | 26 +- 2 files changed, 295 insertions(+), 25 deletions(-) (limited to 'OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp') 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 7853d0b00..215c17590 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java @@ -5,16 +5,17 @@ import org.spongycastle.bcpg.S2K; import org.spongycastle.openpgp.PGPKeyRing; import org.spongycastle.openpgp.PGPObjectFactory; import org.spongycastle.openpgp.PGPPublicKey; +import org.spongycastle.openpgp.PGPPublicKeyRing; import org.spongycastle.openpgp.PGPSecretKey; import org.spongycastle.openpgp.PGPSecretKeyRing; import org.spongycastle.openpgp.PGPSignature; import org.spongycastle.openpgp.PGPUtil; import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException; -import org.sufficientlysecure.keychain.service.OperationResultParcel; 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; @@ -179,51 +180,300 @@ public class UncachedKeyRing { * * More specifically: * - Remove all non-verifying self-certificates - * - Remove all expired self-certificates * - Remove all certificates flagged as "local" * - Remove all certificates which are superseded by a newer one on the same target * - * After this cleaning, a number of checks are done: + * After this cleaning, a number of checks are done: TODO implement * - See if each subkey retains a valid self certificate * - See if each user id retains a valid self certificate * * This operation writes an OperationLog which can be used as part of a OperationResultParcel. * - * If any of these checks fail, the operation as a whole fails and the keyring is declared - * unusable. (TODO: allow forcing of import?) - * - * TODO implement - * * @return A canonicalized key * */ - public UncachedKeyRing canonicalize(OperationLog log) { - if(isSecret()) { + public UncachedKeyRing canonicalize(OperationLog log, int indent) { + if (isSecret()) { throw new RuntimeException("Tried to canonicalize non-secret keyring. " + "This is a programming error and should never happen!"); } - // dummy log.add(LogLevel.START, LogType.MSG_KC, - new String[] { PgpKeyHelper.convertKeyIdToHex(getMasterKeyId()) }, 0); + new String[]{PgpKeyHelper.convertKeyIdToHex(getMasterKeyId())}, indent); + indent += 1; + + int removedCerts = 0; + + PGPPublicKeyRing ring = (PGPPublicKeyRing) mRing; + PGPPublicKey masterKey = mRing.getPublicKey(); + final long masterKeyId = masterKey.getKeyID(); + + { + log.add(LogLevel.DEBUG, LogType.MSG_KC_MASTER, + new String[]{PgpKeyHelper.convertKeyIdToHex(masterKey.getKeyID())}, indent); + indent += 1; + + PGPPublicKey modified = masterKey; + PGPSignature revocation = null; + for (PGPSignature zert : new IterableIterator(masterKey.getSignatures())) { + int type = zert.getSignatureType(); + // Disregard certifications on user ids, we will deal with those later + if (type == PGPSignature.NO_CERTIFICATION + || type == PGPSignature.DEFAULT_CERTIFICATION + || type == PGPSignature.CASUAL_CERTIFICATION + || type == PGPSignature.POSITIVE_CERTIFICATION + || type == PGPSignature.CERTIFICATION_REVOCATION) { + continue; + } + WrappedSignature cert = new WrappedSignature(zert); + + if (type != PGPSignature.KEY_REVOCATION) { + // Unknown type, just remove + log.add(LogLevel.WARN, LogType.MSG_KC_CERT_BAD_TYPE, new String[]{ + "0x" + Integer.toString(type, 16) + }, indent); + modified = PGPPublicKey.removeCertification(modified, zert); + removedCerts += 1; + continue; + } + + try { + cert.init(masterKey); + if (!cert.verifySignature(masterKey)) { + log.add(LogLevel.WARN, LogType.MSG_KC_REVOKE_BAD, null, indent); + modified = PGPPublicKey.removeCertification(modified, zert); + removedCerts += 1; + continue; + } + } catch (PgpGeneralException e) { + log.add(LogLevel.WARN, LogType.MSG_KC_REVOKE_BAD_ERR, null, indent); + modified = PGPPublicKey.removeCertification(modified, zert); + removedCerts += 1; + continue; + } + + // first revocation? fine then. + if (revocation == null) { + revocation = zert; + // more revocations? at least one is superfluous, then. + } else if (revocation.getCreationTime().before(zert.getCreationTime())) { + modified = PGPPublicKey.removeCertification(modified, revocation); + removedCerts += 1; + log.add(LogLevel.INFO, LogType.MSG_KC_REVOKE_DUP, null, indent); + revocation = zert; + } else { + modified = PGPPublicKey.removeCertification(modified, zert); + removedCerts += 1; + log.add(LogLevel.INFO, LogType.MSG_KC_REVOKE_DUP, null, indent); + } + } + + for (String userId : new IterableIterator(masterKey.getUserIDs())) { + PGPSignature selfCert = null; + revocation = null; + + // look through signatures for this specific key + for (PGPSignature zert : new IterableIterator( + masterKey.getSignaturesForID(userId))) { + WrappedSignature cert = new WrappedSignature(zert); + long certId = cert.getKeyId(); + + // If this is a foreign signature, never mind + if (certId != masterKeyId) { + continue; + } + + // Otherwise, first make sure it checks out + try { + cert.init(masterKey); + if (!cert.verifySignature(masterKey, userId)) { + log.add(LogLevel.WARN, LogType.MSG_KC_UID_BAD, + new String[] { userId }, indent); + modified = PGPPublicKey.removeCertification(modified, userId, zert); + removedCerts += 1; + continue; + } + } catch (PgpGeneralException e) { + log.add(LogLevel.WARN, LogType.MSG_KC_UID_BAD_ERR, + new String[] { userId }, indent); + modified = PGPPublicKey.removeCertification(modified, userId, zert); + removedCerts += 1; + continue; + } + + switch (zert.getSignatureType()) { + case PGPSignature.DEFAULT_CERTIFICATION: + case PGPSignature.NO_CERTIFICATION: + case PGPSignature.CASUAL_CERTIFICATION: + case PGPSignature.POSITIVE_CERTIFICATION: + if (selfCert == null) { + selfCert = zert; + } else if (selfCert.getCreationTime().before(cert.getCreationTime())) { + modified = PGPPublicKey.removeCertification(modified, userId, selfCert); + removedCerts += 1; + log.add(LogLevel.INFO, LogType.MSG_KC_UID_DUP, + new String[] { userId }, indent); + selfCert = zert; + } else { + modified = PGPPublicKey.removeCertification(modified, userId, zert); + removedCerts += 1; + log.add(LogLevel.INFO, LogType.MSG_KC_UID_DUP, + new String[] { userId }, indent); + } + // If there is a revocation certificate, and it's older than this, drop it + if (revocation != null + && revocation.getCreationTime().before(selfCert.getCreationTime())) { + modified = PGPPublicKey.removeCertification(modified, userId, revocation); + revocation = null; + removedCerts += 1; + log.add(LogLevel.INFO, LogType.MSG_KC_UID_REVOKE_OLD, + new String[] { userId }, indent); + } + 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); + removedCerts += 1; + log.add(LogLevel.INFO, LogType.MSG_KC_UID_REVOKE_OLD, + new String[] { userId }, indent); + continue; + } + // first revocation? remember it. + if (revocation == null) { + revocation = zert; + // more revocations? at least one is superfluous, then. + } else if (revocation.getCreationTime().before(cert.getCreationTime())) { + modified = PGPPublicKey.removeCertification(modified, userId, revocation); + removedCerts += 1; + log.add(LogLevel.INFO, LogType.MSG_KC_UID_REVOKE_DUP, + new String[] { userId }, indent); + revocation = zert; + } else { + modified = PGPPublicKey.removeCertification(modified, userId, zert); + removedCerts += 1; + log.add(LogLevel.INFO, LogType.MSG_KC_UID_REVOKE_DUP, + new String[] { userId }, indent); + } + break; + + default: + log.add(LogLevel.WARN, LogType.MSG_KC_UID_UNKNOWN_CERT, + new String[] { + "0x" + Integer.toString(zert.getSignatureType(), 16), + userId + }, indent); + modified = PGPPublicKey.removeCertification(modified, userId, zert); + removedCerts += 1; + } + + } + } + + // Replace modified key in the keyring + ring = PGPPublicKeyRing.insertPublicKey(ring, modified); - /* - // Remove all non-verifying self certificates - for (PGPPublicKey key : new IterableIterator(mRing.getPublicKeys())) { + log.add(LogLevel.DEBUG, LogType.MSG_KC_MASTER_SUCCESS, null, indent); + indent -= 1; + + } - for (PGPSignature sig : new IterableIterator( - key.getSignaturesOfType(isMasterKey() ? PGPSignature.KEY_REVOCATION - : PGPSignature.SUBKEY_REVOCATION))) { - return true; + // 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! + if (key.isMasterKey()) { + continue; + } + log.add(LogLevel.DEBUG, LogType.MSG_KC_SUBKEY, + 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; + 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(); + + // filter out bad key types... + if (sig.getKeyId() != masterKey.getKeyID()) { + log.add(LogLevel.WARN, LogType.MSG_KC_CERT_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[]{ + "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); + 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())) { + continue; + } + cert = zig; + // if this is newer than a possibly existing revocation, drop that one + if (revocation != null && cert.getCreationTime().after(revocation.getCreationTime())) { + revocation = null; + } + // it must be a revocation, then (we made sure above) + } else { + // if there is no binding (yet), or the revocation is newer than the binding: keep it + if (cert == null || cert.getCreationTime().before(sig.getCreationTime())) { + revocation = zig; + } + } } - }*/ + // it is not properly bound? error! + if (cert == null) { + ring = PGPPublicKeyRing.removePublicKey(ring, modified); - log.add(LogLevel.OK, LogType.MSG_KC_SUCCESS, null, 0); + log.add(LogLevel.ERROR, LogType.MSG_KC_SUBKEY_NO_CERT, + new String[]{PgpKeyHelper.convertKeyIdToHex(key.getKeyID())}, indent); + indent -= 1; + continue; + } - return this; + // re-add certification + modified = PGPPublicKey.addCertification(modified, cert); + // add revocation, if any + if (revocation != null) { + modified = PGPPublicKey.addCertification(modified, revocation); + } + // replace pubkey in keyring + ring = PGPPublicKeyRing.insertPublicKey(ring, modified); + log.add(LogLevel.DEBUG, LogType.MSG_KC_SUBKEY_SUCCESS, null, indent); + indent -= 1; + } + + if (removedCerts > 0) { + log.add(LogLevel.OK, LogType.MSG_KC_SUCCESS_REMOVED, + new String[] { Integer.toString(removedCerts) }, indent); + } else { + log.add(LogLevel.OK, LogType.MSG_KC_SUCCESS, null, indent); + } + return new UncachedKeyRing(ring); } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedSignature.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedSignature.java index 1b7a5e8ba..093b6c96f 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedSignature.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedSignature.java @@ -35,7 +35,7 @@ public class WrappedSignature { final PGPSignature mSig; - protected WrappedSignature(PGPSignature sig) { + WrappedSignature(PGPSignature sig) { mSig = sig; } @@ -88,7 +88,7 @@ public class WrappedSignature { init(key.getPublicKey()); } - protected void init(PGPPublicKey key) throws PgpGeneralException { + void init(PGPPublicKey key) throws PgpGeneralException { try { JcaPGPContentVerifierBuilderProvider contentVerifierBuilderProvider = new JcaPGPContentVerifierBuilderProvider() @@ -125,7 +125,27 @@ public class WrappedSignature { } } - protected boolean verifySignature(PGPPublicKey key, String uid) throws PgpGeneralException { + boolean verifySignature(PGPPublicKey key) throws PgpGeneralException { + try { + return mSig.verifyCertification(key); + } catch (SignatureException e) { + throw new PgpGeneralException("Sign!", e); + } catch (PGPException e) { + throw new PgpGeneralException("Error!", e); + } + } + + boolean verifySignature(PGPPublicKey masterKey, PGPPublicKey subKey) throws PgpGeneralException { + try { + return mSig.verifyCertification(masterKey, subKey); + } catch (SignatureException e) { + throw new PgpGeneralException("Sign!", e); + } catch (PGPException e) { + throw new PgpGeneralException("Error!", e); + } + } + + boolean verifySignature(PGPPublicKey key, String uid) throws PgpGeneralException { try { return mSig.verifyCertification(uid, key); } catch (SignatureException e) { -- cgit v1.2.3 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/pgp/UncachedKeyRing.java | 79 +++++++++++++--------- 1 file changed, 47 insertions(+), 32 deletions(-) (limited to 'OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp') 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; } -- cgit v1.2.3 From e4a7d4f6e5dc6eb0acac2aa4945852ae2f1d8bb8 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Thu, 12 Jun 2014 18:10:48 +0200 Subject: import-log: minor improvements --- .../org/sufficientlysecure/keychain/pgp/PgpImportExport.java | 11 +++++++---- .../org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java | 3 --- 2 files changed, 7 insertions(+), 7 deletions(-) (limited to 'OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp') diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpImportExport.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpImportExport.java index bb45cc7db..e1967429a 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpImportExport.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpImportExport.java @@ -127,9 +127,7 @@ public class PgpImportExport { updateProgress(R.string.progress_importing, 0, 100); - int newKeys = 0; - int oldKeys = 0; - int badKeys = 0; + int newKeys = 0, oldKeys = 0, badKeys = 0; int position = 0; for (ParcelableKeyRing entry : entries) { @@ -147,7 +145,12 @@ public class PgpImportExport { } } - SaveKeyringResult result = mProviderHelper.savePublicKeyRing(key); + SaveKeyringResult result; + if (key.isSecret()) { + result = mProviderHelper.saveSecretKeyRing(key); + } else { + result = mProviderHelper.savePublicKeyRing(key); + } if (!result.success()) { badKeys += 1; } else if (result.updated()) { 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 a8e4820cf..1edc529c6 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java @@ -416,9 +416,6 @@ public class UncachedKeyRing { 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) { -- cgit v1.2.3 From 0594d9156ef148e3dd4ad7301cb4c036893dd383 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Thu, 12 Jun 2014 21:57:03 +0200 Subject: canonicalize: filter out future and local certificates --- .../keychain/pgp/UncachedKeyRing.java | 80 +++++++++++++++++++--- .../keychain/pgp/WrappedSignature.java | 7 ++ 2 files changed, 76 insertions(+), 11 deletions(-) (limited to 'OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp') 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 1edc529c6..8019b2b52 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java @@ -2,6 +2,7 @@ package org.sufficientlysecure.keychain.pgp; import org.spongycastle.bcpg.ArmoredOutputStream; import org.spongycastle.bcpg.S2K; +import org.spongycastle.bcpg.SignatureSubpacketTags; import org.spongycastle.openpgp.PGPKeyRing; import org.spongycastle.openpgp.PGPObjectFactory; import org.spongycastle.openpgp.PGPPublicKey; @@ -23,6 +24,7 @@ import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.util.Date; import java.util.HashSet; import java.util.Iterator; import java.util.List; @@ -178,6 +180,7 @@ public class UncachedKeyRing { * * More specifically: * - Remove all non-verifying self-certificates + * - Remove all "future" self-certificates * - Remove all certificates flagged as "local" * - Remove all certificates which are superseded by a newer one on the same target * @@ -200,6 +203,8 @@ public class UncachedKeyRing { new String[]{PgpKeyHelper.convertKeyIdToHex(getMasterKeyId())}, indent); indent += 1; + final Date now = new Date(); + int removedCerts = 0; PGPPublicKeyRing ring = (PGPPublicKeyRing) mRing; @@ -215,6 +220,7 @@ public class UncachedKeyRing { PGPSignature revocation = null; for (PGPSignature zert : new IterableIterator(masterKey.getSignatures())) { int type = zert.getSignatureType(); + // Disregard certifications on user ids, we will deal with those later if (type == PGPSignature.NO_CERTIFICATION || type == PGPSignature.DEFAULT_CERTIFICATION @@ -227,7 +233,7 @@ public class UncachedKeyRing { if (type != PGPSignature.KEY_REVOCATION) { // Unknown type, just remove - log.add(LogLevel.WARN, LogType.MSG_KC_SUB_BAD_TYPE, new String[]{ + log.add(LogLevel.WARN, LogType.MSG_KC_REVOKE_BAD_TYPE, new String[]{ "0x" + Integer.toString(type, 16) }, indent); modified = PGPPublicKey.removeCertification(modified, zert); @@ -235,6 +241,22 @@ public class UncachedKeyRing { continue; } + if (cert.getCreationTime().after(now)) { + // Creation date in the future? No way! + log.add(LogLevel.WARN, LogType.MSG_KC_REVOKE_BAD_TIME, null, indent); + modified = PGPPublicKey.removeCertification(modified, zert); + removedCerts += 1; + continue; + } + + if (cert.isLocal()) { + // Creation date in the future? No way! + log.add(LogLevel.WARN, LogType.MSG_KC_REVOKE_BAD_LOCAL, null, indent); + modified = PGPPublicKey.removeCertification(modified, zert); + removedCerts += 1; + continue; + } + try { cert.init(masterKey); if (!cert.verifySignature(masterKey)) { @@ -276,7 +298,38 @@ public class UncachedKeyRing { WrappedSignature cert = new WrappedSignature(zert); long certId = cert.getKeyId(); - // If this is a foreign signature, never mind + + int type = zert.getSignatureType(); + if (type != PGPSignature.DEFAULT_CERTIFICATION + && type != PGPSignature.NO_CERTIFICATION + && type != PGPSignature.CASUAL_CERTIFICATION + && type != PGPSignature.POSITIVE_CERTIFICATION + && type != PGPSignature.CERTIFICATION_REVOCATION) { + log.add(LogLevel.WARN, LogType.MSG_KC_UID_BAD_TYPE, + new String[] { + "0x" + Integer.toString(zert.getSignatureType(), 16) + }, indent); + modified = PGPPublicKey.removeCertification(modified, userId, zert); + removedCerts += 1; + } + + if (cert.getCreationTime().after(now)) { + // Creation date in the future? No way! + log.add(LogLevel.WARN, LogType.MSG_KC_REVOKE_BAD_TIME, null, indent); + modified = PGPPublicKey.removeCertification(modified, zert); + removedCerts += 1; + continue; + } + + if (cert.isLocal()) { + // Creation date in the future? No way! + log.add(LogLevel.WARN, LogType.MSG_KC_REVOKE_BAD_LOCAL, null, indent); + modified = PGPPublicKey.removeCertification(modified, zert); + removedCerts += 1; + continue; + } + + // If this is a foreign signature, never mind any further if (certId != masterKeyId) { continue; } @@ -299,7 +352,7 @@ public class UncachedKeyRing { continue; } - switch (zert.getSignatureType()) { + switch (type) { case PGPSignature.DEFAULT_CERTIFICATION: case PGPSignature.NO_CERTIFICATION: case PGPSignature.CASUAL_CERTIFICATION: @@ -356,14 +409,6 @@ public class UncachedKeyRing { } break; - default: - log.add(LogLevel.WARN, LogType.MSG_KC_UID_UNKNOWN_CERT, - new String[] { - "0x" + Integer.toString(zert.getSignatureType(), 16), - userId - }, indent); - modified = PGPPublicKey.removeCertification(modified, userId, zert); - removedCerts += 1; } } @@ -401,6 +446,7 @@ public class UncachedKeyRing { 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_SUB_BAD_TYPE, new String[]{ "0x" + Integer.toString(type, 16) @@ -408,6 +454,18 @@ public class UncachedKeyRing { continue; } + if (cert.getCreationTime().after(now)) { + // Creation date in the future? No way! + log.add(LogLevel.WARN, LogType.MSG_KC_SUB_BAD_TIME, null, indent); + continue; + } + + if (cert.isLocal()) { + // Creation date in the future? No way! + log.add(LogLevel.WARN, LogType.MSG_KC_SUB_BAD_LOCAL, null, indent); + continue; + } + if (type == PGPSignature.SUBKEY_BINDING) { // TODO verify primary key binding signature for signing keys! diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedSignature.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedSignature.java index 093b6c96f..be7f960a9 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedSignature.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedSignature.java @@ -178,4 +178,11 @@ public class WrappedSignature { return new WrappedSignature(signatures.get(0)); } + public boolean isLocal() { + if (!mSig.getHashedSubPackets().hasSubpacket(SignatureSubpacketTags.EXPORTABLE)) { + return false; + } + SignatureSubpacket p = mSig.getHashedSubPackets().getSubpacket(SignatureSubpacketTags.EXPORTABLE); + return p.getData()[0] == 0; + } } -- cgit v1.2.3 From 073433fa747d698c6666081b2ee312062ecf2115 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Thu, 12 Jun 2014 23:10:44 +0200 Subject: canonicalize: require primary key binding certificates for signing subkeys --- .../keychain/pgp/UncachedKeyRing.java | 44 ++++++++++++++++++++-- 1 file changed, 41 insertions(+), 3 deletions(-) (limited to 'OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp') 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 8019b2b52..e309ed632 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java @@ -3,6 +3,8 @@ package org.sufficientlysecure.keychain.pgp; import org.spongycastle.bcpg.ArmoredOutputStream; import org.spongycastle.bcpg.S2K; import org.spongycastle.bcpg.SignatureSubpacketTags; +import org.spongycastle.bcpg.sig.KeyFlags; +import org.spongycastle.openpgp.PGPKeyFlags; import org.spongycastle.openpgp.PGPKeyRing; import org.spongycastle.openpgp.PGPObjectFactory; import org.spongycastle.openpgp.PGPPublicKey; @@ -10,6 +12,7 @@ import org.spongycastle.openpgp.PGPPublicKeyRing; import org.spongycastle.openpgp.PGPSecretKey; import org.spongycastle.openpgp.PGPSecretKeyRing; import org.spongycastle.openpgp.PGPSignature; +import org.spongycastle.openpgp.PGPSignatureList; import org.spongycastle.openpgp.PGPUtil; import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException; @@ -298,7 +301,6 @@ public class UncachedKeyRing { WrappedSignature cert = new WrappedSignature(zert); long certId = cert.getKeyId(); - int type = zert.getSignatureType(); if (type != PGPSignature.DEFAULT_CERTIFICATION && type != PGPSignature.NO_CERTIFICATION @@ -435,9 +437,12 @@ public class UncachedKeyRing { // certificate. PGPPublicKey modified = key; PGPSignature selfCert = null, revocation = null; - for (PGPSignature zig : new IterableIterator(key.getSignatures())) { + uids: for (PGPSignature zig : new IterableIterator(key.getSignatures())) { // remove from keyring (for now) modified = PGPPublicKey.removeCertification(modified, zig); + // add this too, easier than adding it for every single "continue" case + removedCerts += 1; + WrappedSignature cert = new WrappedSignature(zig); int type = cert.getSignatureType(); @@ -467,7 +472,6 @@ public class UncachedKeyRing { } if (type == PGPSignature.SUBKEY_BINDING) { - // TODO verify primary key binding signature for signing keys! // make sure the certificate checks out try { @@ -481,10 +485,42 @@ public class UncachedKeyRing { continue; } + if (zig.getHashedSubPackets().hasSubpacket(SignatureSubpacketTags.KEY_FLAGS)) { + int flags = ((KeyFlags) zig.getHashedSubPackets() + .getSubpacket(SignatureSubpacketTags.KEY_FLAGS)).getFlags(); + // If this subkey is allowed to sign data, + if ((flags & PGPKeyFlags.CAN_SIGN) == PGPKeyFlags.CAN_SIGN) { + try { + PGPSignatureList list = zig.getUnhashedSubPackets().getEmbeddedSignatures(); + boolean ok = false; + for (int i = 0; i < list.size(); i++) { + WrappedSignature subsig = new WrappedSignature(list.get(i)); + if (subsig.getSignatureType() == PGPSignature.PRIMARYKEY_BINDING) { + subsig.init(key); + if (subsig.verifySignature(masterKey, key)) { + ok = true; + } else { + log.add(LogLevel.WARN, LogType.MSG_KC_SUB_PRIMARY_BAD, null, indent); + continue uids; + } + } + } + if (!ok) { + log.add(LogLevel.WARN, LogType.MSG_KC_SUB_PRIMARY_NONE, null, indent); + continue; + } + } catch (Exception e) { + log.add(LogLevel.WARN, LogType.MSG_KC_SUB_PRIMARY_BAD_ERR, null, indent); + continue; + } + } + } + // if we already have a cert, and this one is not newer: skip it if (selfCert != null && selfCert.getCreationTime().before(cert.getCreationTime())) { continue; } + selfCert = zig; // if this is newer than a possibly existing revocation, drop that one if (revocation != null && selfCert.getCreationTime().after(revocation.getCreationTime())) { @@ -525,9 +561,11 @@ public class UncachedKeyRing { // re-add certification modified = PGPPublicKey.addCertification(modified, selfCert); + removedCerts -= 1; // add revocation, if any if (revocation != null) { modified = PGPPublicKey.addCertification(modified, revocation); + removedCerts -= 1; } // replace pubkey in keyring ring = PGPPublicKeyRing.insertPublicKey(ring, modified); -- cgit v1.2.3