canonicalize: filter out future and local certificates

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<PGPSignature>(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!
 

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;
+    }
 }

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_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_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_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;

OpenKeychain/src/main/res/values/strings.xml

@@ -561,15 +561,20 @@
     <string name="msg_kc">Canonicalizing keyring %s</string>
     <string name="msg_kc_master">Processing master key</string>
     <string name="msg_kc_master_success">OK</string>
-    <string name="msg_kc_revoke_bad_err">Removing bad keyring revocation key</string>
+    <string name="msg_kc_revoke_bad_err">Removing bad keyring revocation certificate</string>
-    <string name="msg_kc_revoke_bad">Removing bad keyring revocation key</string>
+    <string name="msg_kc_revoke_bad_local">Removing keyring revocation certificate with "local" flag</string>
-    <string name="msg_kc_revoke_dup">Removing redundant keyring revocation key</string>
+    <string name="msg_kc_revoke_bad_time">Removing keyring revocation certificate with future timestamp</string>
+    <string name="msg_kc_revoke_bad_type">Removing master key certificate of unknown type (%s)</string>
+    <string name="msg_kc_revoke_bad">Removing bad keyring revocation certificate</string>
+    <string name="msg_kc_revoke_dup">Removing redundant keyring revocation certificate</string>
     <string name="msg_kc_sub">Processing subkey %s</string>
     <string name="msg_kc_sub_bad">Removing invalid subkey binding certificate</string>
-    <string name="msg_kc_sub_bad_err">Removing bad subkey binding certificate!</string>
+    <string name="msg_kc_sub_bad_err">Removing bad subkey binding certificate</string>
+    <string name="msg_kc_sub_bad_local">Removing subkey binding certificate with "local" flag</string>
     <string name="msg_kc_sub_bad_keyid">Subkey binding issuer id mismatch</string>
+    <string name="msg_kc_sub_bad_time">Removing subkey binding certificate with future timestamp</string>
     <string name="msg_kc_sub_bad_type">Unknown subkey certificate type: %s</string>
-    <string name="msg_kc_sub_no_cert">No valid certificate found for %s, removing from ring!</string>
+    <string name="msg_kc_sub_no_cert">No valid certificate found for %s, removing from ring</string>
     <string name="msg_kc_sub_revoke_bad_err">Removing bad subkey revocation key</string>
     <string name="msg_kc_sub_revoke_bad">Removing bad subkey revocation key</string>
     <string name="msg_kc_sub_revoke_dup">Removing redundant keyring revocation key</string>
@@ -577,11 +582,13 @@
     <string name="msg_kc_success">Keyring canonicalization successful</string>
     <string name="msg_kc_success_removed">Keyring canonicalization successful, removed %s certificates</string>
     <string name="msg_kc_uid_bad_err">Removing bad self certificate for user id %s</string>
+    <string name="msg_kc_uid_bad_local">Removing user id certificate with "local" flag</string>
+    <string name="msg_kc_uid_bad_time">Removing user id with future timestamp</string>
+    <string name="msg_kc_uid_bad_type">Removing user id certificate of unknown type (%s)</string>
     <string name="msg_kc_uid_bad">Removing bad self certificate for user id "%s"</string>
     <string name="msg_kc_uid_dup">Removing outdated self certificate for user id "%s"</string>
     <string name="msg_kc_uid_revoke_dup">Removing redundant revocation certificate for user id "%s"</string>
     <string name="msg_kc_uid_revoke_old">Removing outdated revocation certificate for user id "%s"</string>
-    <string name="msg_kc_uid_unknown_cert">Removing unknown certificate of type 0x%1$s from user id "%2$s"</string>
 
     <!-- unsorted -->
     <string name="section_certifier_id">Certifier</string>