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 ++ .../keychain/service/OperationResultParcel.java | 11 ++- OpenKeychain/src/main/res/values/strings.xml | 19 +++-- 4 files changed, 98 insertions(+), 19 deletions(-) (limited to 'OpenKeychain/src/main') 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; + } } 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 f83dc796f..e7fb951cd 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java @@ -161,12 +161,17 @@ public class OperationResultParcel implements Parcelable { 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_LOCAL (R.string.msg_kc_revoke_bad_local), + MSG_KC_REVOKE_BAD_TIME (R.string.msg_kc_revoke_bad_time), + MSG_KC_REVOKE_BAD_TYPE (R.string.msg_kc_revoke_bad_type), MSG_KC_REVOKE_BAD (R.string.msg_kc_revoke_bad), MSG_KC_REVOKE_DUP (R.string.msg_kc_revoke_dup), MSG_KC_SUB (R.string.msg_kc_sub), + MSG_KC_SUB_BAD(R.string.msg_kc_sub_bad), MSG_KC_SUB_BAD_ERR(R.string.msg_kc_sub_bad_err), + MSG_KC_SUB_BAD_LOCAL(R.string.msg_kc_sub_bad_local), 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_TIME(R.string.msg_kc_sub_bad_time), 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), @@ -176,11 +181,13 @@ public class OperationResultParcel implements Parcelable { 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), + MSG_KC_UID_BAD_LOCAL (R.string.msg_kc_uid_bad_local), + MSG_KC_UID_BAD_TIME (R.string.msg_kc_uid_bad_time), + MSG_KC_UID_BAD_TYPE (R.string.msg_kc_uid_bad_type), MSG_KC_UID_BAD (R.string.msg_kc_uid_bad), MSG_KC_UID_DUP (R.string.msg_kc_uid_dup), MSG_KC_UID_REVOKE_DUP (R.string.msg_kc_uid_revoke_dup), MSG_KC_UID_REVOKE_OLD (R.string.msg_kc_uid_revoke_old), - MSG_KC_UID_UNKNOWN_CERT (R.string.msg_kc_uid_unknown_cert), ; private final int mMsgId; diff --git a/OpenKeychain/src/main/res/values/strings.xml b/OpenKeychain/src/main/res/values/strings.xml index c57347055..1f0cf0c4e 100644 --- a/OpenKeychain/src/main/res/values/strings.xml +++ b/OpenKeychain/src/main/res/values/strings.xml @@ -561,15 +561,20 @@ Canonicalizing keyring %s Processing master key OK - Removing bad keyring revocation key - Removing bad keyring revocation key - Removing redundant keyring revocation key + Removing bad keyring revocation certificate + Removing keyring revocation certificate with "local" flag + Removing keyring revocation certificate with future timestamp + Removing master key certificate of unknown type (%s) + Removing bad keyring revocation certificate + Removing redundant keyring revocation certificate Processing subkey %s Removing invalid subkey binding certificate - Removing bad subkey binding certificate! + Removing bad subkey binding certificate + Removing subkey binding certificate with "local" flag Subkey binding issuer id mismatch + Removing subkey binding certificate with future timestamp Unknown subkey certificate type: %s - No valid certificate found for %s, removing from ring! + No valid certificate found for %s, removing from ring Removing bad subkey revocation key Removing bad subkey revocation key Removing redundant keyring revocation key @@ -577,11 +582,13 @@ Keyring canonicalization successful Keyring canonicalization successful, removed %s certificates Removing bad self certificate for user id %s + Removing user id certificate with "local" flag + Removing user id with future timestamp + Removing user id certificate of unknown type (%s) Removing bad self certificate for user id "%s" Removing outdated self certificate for user id "%s" Removing redundant revocation certificate for user id "%s" Removing outdated revocation certificate for user id "%s" - Removing unknown certificate of type 0x%1$s from user id "%2$s" Certifier -- cgit v1.2.3