Merge branch 'canonicalize'

4 files changed, 505 insertions, 35 deletions

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..e1967429a 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.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;
@@ -55,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;
@@ -115,26 +114,20 @@ 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.
             }
         }
     }
 
-    /**
-     * Imports keys from given data. If keyIds is given only those are imported
-     */
-    public Bundle importKeyRings(List<ParcelableKeyRing> entries)
+    /** Imports keys from given data. If keyIds is given only those are imported */
+    public ImportResult importKeyRings(List<ParcelableKeyRing> entries)
             throws PgpGeneralException, PGPException, IOException {
-        Bundle returnData = new Bundle();
 
         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) {
@@ -152,14 +145,19 @@ 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
-                newKeys += 1;
+                SaveKeyringResult result;
+                if (key.isSecret()) {
+                    result = mProviderHelper.saveSecretKeyRing(key);
+                } else {
+                    result = mProviderHelper.savePublicKeyRing(key);
+                }
+                if (!result.success()) {
+                    badKeys += 1;
+                } else if (result.updated()) {
+                    oldKeys += 1;
+                } else {
+                    newKeys += 1;
+                }
 
             } catch (PgpGeneralException e) {
                 Log.e(Constants.TAG, "Encountered bad key on import!", e);
@@ -170,11 +168,31 @@ 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 = 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_WITH_ERRORS;
+                if (newKeys == 0 && oldKeys == 0) {
+                    resultType |= ImportResult.RESULT_ERROR;
+                }
+            }
+            if (log.containsWarnings()) {
+                resultType |= ImportResult.RESULT_WITH_WARNINGS;
+            }
+        }
+
+        return new ImportResult(resultType, log, newKeys, oldKeys, badKeys);
 
-        return returnData;
     }
 
     public Bundle exportKeyRings(ArrayList<Long> publicKeyRingMasterIds,
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..e309ed632 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java
@@ -2,14 +2,23 @@ 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;
+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;
+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;
 
@@ -18,7 +27,8 @@ import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
-import java.util.ArrayList;
+import java.util.Date;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Vector;
@@ -149,13 +159,13 @@ public class UncachedKeyRing {
         aos.close();
     }
 
-    public ArrayList<Long> getAvailableSubkeys() {
+    public HashSet<Long> 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<Long> result = new ArrayList<Long>();
+        HashSet<Long> result = new HashSet<Long>();
         // then, mark exactly the keys we have available
         for (PGPSecretKey sub : new IterableIterator<PGPSecretKey>(
                 ((PGPSecretKeyRing) mRing).getSecretKeys())) {
@@ -168,4 +178,411 @@ 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 "future" 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: 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.
+     *
+     * @return A canonicalized key
+     *
+     */
+    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!");
+        }
+
+        log.add(LogLevel.START, LogType.MSG_KC,
+                new String[]{PgpKeyHelper.convertKeyIdToHex(getMasterKeyId())}, indent);
+        indent += 1;
+
+        final Date now = new Date();
+
+        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<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
+                        || 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_REVOKE_BAD_TYPE, new String[]{
+                            "0x" + Integer.toString(type, 16)
+                    }, indent);
+                    modified = PGPPublicKey.removeCertification(modified, zert);
+                    removedCerts += 1;
+                    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)) {
+                        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<String>(masterKey.getUserIDs())) {
+                PGPSignature selfCert = null;
+                revocation = null;
+
+                // look through signatures for this specific key
+                for (PGPSignature zert : new IterableIterator<PGPSignature>(
+                        masterKey.getSignaturesForID(userId))) {
+                    WrappedSignature cert = new WrappedSignature(zert);
+                    long certId = cert.getKeyId();
+
+                    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;
+                    }
+
+                    // 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 (type) {
+                        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;
+
+                    }
+
+                }
+            }
+
+            // Replace modified key in the keyring
+            ring = PGPPublicKeyRing.insertPublicKey(ring, modified);
+
+            log.add(LogLevel.DEBUG, LogType.MSG_KC_MASTER_SUCCESS, null, indent);
+            indent -= 1;
+
+        }
+
+        // Process all keys
+        for (PGPPublicKey key : new IterableIterator<PGPPublicKey>(ring.getPublicKeys())) {
+            // 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_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 selfCert = null, revocation = null;
+            uids: for (PGPSignature zig : new IterableIterator<PGPSignature>(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();
+
+                // filter out bad key types...
+                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_SUB_BAD_TYPE, new String[]{
+                            "0x" + Integer.toString(type, 16)
+                    }, indent);
+                    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) {
+
+                    // 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);
+                            continue;
+                        }
+                    } catch (PgpGeneralException e) {
+                        log.add(LogLevel.WARN, LogType.MSG_KC_SUB_BAD_ERR, null, indent);
+                        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())) {
+                        revocation = null;
+                    }
+
+                // 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 (selfCert == null || selfCert.getCreationTime().before(cert.getCreationTime())) {
+                        revocation = zig;
+                    }
+                }
+            }
+
+            // it is not properly bound? error!
+            if (selfCert == null) {
+                ring = PGPPublicKeyRing.removePublicKey(ring, modified);
+
+                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, 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);
+
+            log.add(LogLevel.DEBUG, LogType.MSG_KC_SUB_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/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<PGPSignature>(
+                mPublicKey.getSignaturesOfType(isMasterKey() ? PGPSignature.KEY_REVOCATION
+                                                             : PGPSignature.SUBKEY_REVOCATION))) {
+            return true;
+        }
+        return false;
     }
 
     public Date getCreationTime() {
@@ -193,4 +200,5 @@ public class UncachedPublicKey {
             }
         };
     }
+
 }
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..be7f960a9 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) {
@@ -158,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;
+    }
 }