Merge branch 'master' of github.com:open-keychain/open-keychain

6 files changed, 242 insertions, 99 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 e1967429a..3681d62d8 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpImportExport.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpImportExport.java
@@ -37,6 +37,7 @@ import org.sufficientlysecure.keychain.service.OperationResultParcel.OperationLo
 import org.sufficientlysecure.keychain.service.OperationResults.ImportResult;
 import org.sufficientlysecure.keychain.service.OperationResults.SaveKeyringResult;
 import org.sufficientlysecure.keychain.util.Log;
+import org.sufficientlysecure.keychain.util.ProgressScaler;
 
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
@@ -130,6 +131,7 @@ public class PgpImportExport {
         int newKeys = 0, oldKeys = 0, badKeys = 0;
 
         int position = 0;
+        int progSteps = 100 / entries.size();
         for (ParcelableKeyRing entry : entries) {
             try {
                 UncachedKeyRing key = UncachedKeyRing.decodeFromData(entry.getBytes());
@@ -149,7 +151,8 @@ public class PgpImportExport {
                 if (key.isSecret()) {
                     result = mProviderHelper.saveSecretKeyRing(key);
                 } else {
-                    result = mProviderHelper.savePublicKeyRing(key);
+                    result = mProviderHelper.savePublicKeyRing(key,
+                            new ProgressScaler(mProgressable, position, (position+1)*progSteps, 100));
                 }
                 if (!result.success()) {
                     badKeys += 1;
@@ -165,7 +168,6 @@ public class PgpImportExport {
             }
             // update progress
             position++;
-            updateProgress(position / entries.size() * 100, 100);
         }
 
         OperationLog log = mProviderHelper.getLog();
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 e309ed632..78620405f 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java
@@ -178,18 +178,21 @@ public class UncachedKeyRing {
         return result;
     }
 
-    /** "Canonicalizes" a key, removing inconsistencies in the process. This operation can be
-     * applied to public keyrings only.
+    /** "Canonicalizes" a key, removing inconsistencies in the process.
      *
      * 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
+     *  - Remove all certificates which are superseded by a newer one on the same target,
+     *      including revocations with later re-certifications.
+     *  - Remove all certificates of unknown type:
+     *   - key revocation signatures on the master key
+     *   - subkey binding signatures for subkeys
+     *   - certifications and certification revocations for user ids
+     *  - If a subkey retains no valid subkey binding certificate, remove it
+     *  - If a user id retains no valid self certificate, remove it
+     *  - If the key is a secret key, remove all certificates by foreign keys
      *
      * This operation writes an OperationLog which can be used as part of a OperationResultParcel.
      *
@@ -197,20 +200,16 @@ public class UncachedKeyRing {
      *
      */
     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,
+        log.add(LogLevel.START, isSecret() ? LogType.MSG_KC_SECRET : LogType.MSG_KC_PUBLIC,
                 new String[]{PgpKeyHelper.convertKeyIdToHex(getMasterKeyId())}, indent);
         indent += 1;
 
         final Date now = new Date();
 
-        int removedCerts = 0;
+        int redundantCerts = 0, badCerts = 0;
 
-        PGPPublicKeyRing ring = (PGPPublicKeyRing) mRing;
+        PGPKeyRing ring = mRing;
         PGPPublicKey masterKey = mRing.getPublicKey();
         final long masterKeyId = masterKey.getKeyID();
 
@@ -240,7 +239,7 @@ public class UncachedKeyRing {
                             "0x" + Integer.toString(type, 16)
                     }, indent);
                     modified = PGPPublicKey.removeCertification(modified, zert);
-                    removedCerts += 1;
+                    badCerts += 1;
                     continue;
                 }
 
@@ -248,7 +247,7 @@ public class UncachedKeyRing {
                     // 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;
+                    badCerts += 1;
                     continue;
                 }
 
@@ -256,7 +255,7 @@ public class UncachedKeyRing {
                     // 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;
+                    badCerts += 1;
                     continue;
                 }
 
@@ -265,13 +264,13 @@ public class UncachedKeyRing {
                     if (!cert.verifySignature(masterKey)) {
                         log.add(LogLevel.WARN, LogType.MSG_KC_REVOKE_BAD, null, indent);
                         modified = PGPPublicKey.removeCertification(modified, zert);
-                        removedCerts += 1;
+                        badCerts += 1;
                         continue;
                     }
                 } catch (PgpGeneralException e) {
                     log.add(LogLevel.WARN, LogType.MSG_KC_REVOKE_BAD_ERR, null, indent);
                     modified = PGPPublicKey.removeCertification(modified, zert);
-                    removedCerts += 1;
+                    badCerts += 1;
                     continue;
                 }
 
@@ -281,12 +280,12 @@ public class UncachedKeyRing {
                     // more revocations? at least one is superfluous, then.
                 } else if (revocation.getCreationTime().before(zert.getCreationTime())) {
                     modified = PGPPublicKey.removeCertification(modified, revocation);
-                    removedCerts += 1;
+                    redundantCerts += 1;
                     log.add(LogLevel.INFO, LogType.MSG_KC_REVOKE_DUP, null, indent);
                     revocation = zert;
                 } else {
                     modified = PGPPublicKey.removeCertification(modified, zert);
-                    removedCerts += 1;
+                    redundantCerts += 1;
                     log.add(LogLevel.INFO, LogType.MSG_KC_REVOKE_DUP, null, indent);
                 }
             }
@@ -312,14 +311,14 @@ public class UncachedKeyRing {
                                         "0x" + Integer.toString(zert.getSignatureType(), 16)
                                 }, indent);
                         modified = PGPPublicKey.removeCertification(modified, userId, zert);
-                        removedCerts += 1;
+                        badCerts += 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;
+                        badCerts += 1;
                         continue;
                     }
 
@@ -327,12 +326,19 @@ public class UncachedKeyRing {
                         // 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;
+                        badCerts += 1;
                         continue;
                     }
 
-                    // If this is a foreign signature, never mind any further
+                    // If this is a foreign signature, ...
                     if (certId != masterKeyId) {
+                        // never mind any further for public keys, but remove them from secret ones
+                        if (isSecret()) {
+                            log.add(LogLevel.WARN, LogType.MSG_KC_UID_FOREIGN,
+                                    new String[] { PgpKeyHelper.convertKeyIdToHex(certId) }, indent);
+                            modified = PGPPublicKey.removeCertification(modified, userId, zert);
+                            badCerts += 1;
+                        }
                         continue;
                     }
 
@@ -343,14 +349,14 @@ public class UncachedKeyRing {
                             log.add(LogLevel.WARN, LogType.MSG_KC_UID_BAD,
                                     new String[] { userId }, indent);
                             modified = PGPPublicKey.removeCertification(modified, userId, zert);
-                            removedCerts += 1;
+                            badCerts += 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;
+                        badCerts += 1;
                         continue;
                     }
 
@@ -363,14 +369,14 @@ public class UncachedKeyRing {
                                 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,
+                                redundantCerts += 1;
+                                log.add(LogLevel.DEBUG, 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,
+                                redundantCerts += 1;
+                                log.add(LogLevel.DEBUG, LogType.MSG_KC_UID_DUP,
                                         new String[] { userId }, indent);
                             }
                             // If there is a revocation certificate, and it's older than this, drop it
@@ -378,8 +384,8 @@ public class UncachedKeyRing {
                                     && 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,
+                                redundantCerts += 1;
+                                log.add(LogLevel.DEBUG, LogType.MSG_KC_UID_REVOKE_OLD,
                                         new String[] { userId }, indent);
                             }
                             break;
@@ -388,8 +394,8 @@ public class UncachedKeyRing {
                             // 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,
+                                redundantCerts += 1;
+                                log.add(LogLevel.DEBUG, LogType.MSG_KC_UID_REVOKE_OLD,
                                         new String[] { userId }, indent);
                                 continue;
                             }
@@ -399,14 +405,14 @@ public class UncachedKeyRing {
                                 // 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,
+                                redundantCerts += 1;
+                                log.add(LogLevel.DEBUG, 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,
+                                redundantCerts += 1;
+                                log.add(LogLevel.DEBUG, LogType.MSG_KC_UID_REVOKE_DUP,
                                         new String[] { userId }, indent);
                             }
                             break;
@@ -414,12 +420,23 @@ public class UncachedKeyRing {
                     }
 
                 }
+
+                // If no valid certificate (if only a revocation) remains, drop it
+                if (selfCert == null && revocation == null) {
+                    modified = PGPPublicKey.removeCertification(modified, userId);
+                    log.add(LogLevel.ERROR, LogType.MSG_KC_UID_REVOKE_DUP,
+                            new String[] { userId }, indent);
+                }
             }
 
-            // Replace modified key in the keyring
-            ring = PGPPublicKeyRing.insertPublicKey(ring, modified);
+            // If NO user ids remain, error out!
+            if (!modified.getUserIDs().hasNext()) {
+                log.add(LogLevel.ERROR, LogType.MSG_KC_FATAL_NO_UID, null, indent);
+                return null;
+            }
 
-            log.add(LogLevel.DEBUG, LogType.MSG_KC_MASTER_SUCCESS, null, indent);
+            // Replace modified key in the keyring
+            ring = replacePublicKey(ring, modified);
             indent -= 1;
 
         }
@@ -437,18 +454,17 @@ public class UncachedKeyRing {
             // certificate.
             PGPPublicKey modified = key;
             PGPSignature selfCert = null, revocation = null;
-            uids: for (PGPSignature zig : new IterableIterator<PGPSignature>(key.getSignatures())) {
+            uids: for (PGPSignature zert : 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;
+                modified = PGPPublicKey.removeCertification(modified, zert);
 
-                WrappedSignature cert = new WrappedSignature(zig);
+                WrappedSignature cert = new WrappedSignature(zert);
                 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);
+                    badCerts += 1;
                     continue;
                 }
 
@@ -456,18 +472,21 @@ public class UncachedKeyRing {
                     log.add(LogLevel.WARN, LogType.MSG_KC_SUB_BAD_TYPE, new String[]{
                             "0x" + Integer.toString(type, 16)
                     }, indent);
+                    badCerts += 1;
                     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);
+                    badCerts += 1;
                     continue;
                 }
 
                 if (cert.isLocal()) {
                     // Creation date in the future? No way!
                     log.add(LogLevel.WARN, LogType.MSG_KC_SUB_BAD_LOCAL, null, indent);
+                    badCerts += 1;
                     continue;
                 }
 
@@ -478,20 +497,22 @@ public class UncachedKeyRing {
                         cert.init(masterKey);
                         if (!cert.verifySignature(masterKey, key)) {
                             log.add(LogLevel.WARN, LogType.MSG_KC_SUB_BAD, null, indent);
+                            badCerts += 1;
                             continue;
                         }
                     } catch (PgpGeneralException e) {
                         log.add(LogLevel.WARN, LogType.MSG_KC_SUB_BAD_ERR, null, indent);
+                        badCerts += 1;
                         continue;
                     }
 
-                    if (zig.getHashedSubPackets().hasSubpacket(SignatureSubpacketTags.KEY_FLAGS)) {
-                        int flags = ((KeyFlags) zig.getHashedSubPackets()
+                    if (zert.getHashedSubPackets().hasSubpacket(SignatureSubpacketTags.KEY_FLAGS)) {
+                        int flags = ((KeyFlags) zert.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();
+                                PGPSignatureList list = zert.getUnhashedSubPackets().getEmbeddedSignatures();
                                 boolean ok = false;
                                 for (int i = 0; i < list.size(); i++) {
                                     WrappedSignature subsig = new WrappedSignature(list.get(i));
@@ -501,16 +522,19 @@ public class UncachedKeyRing {
                                             ok = true;
                                         } else {
                                             log.add(LogLevel.WARN, LogType.MSG_KC_SUB_PRIMARY_BAD, null, indent);
+                                            badCerts += 1;
                                             continue uids;
                                         }
                                     }
                                 }
                                 if (!ok) {
                                     log.add(LogLevel.WARN, LogType.MSG_KC_SUB_PRIMARY_NONE, null, indent);
+                                    badCerts += 1;
                                     continue;
                                 }
                             } catch (Exception e) {
                                 log.add(LogLevel.WARN, LogType.MSG_KC_SUB_PRIMARY_BAD_ERR, null, indent);
+                                badCerts += 1;
                                 continue;
                             }
                         }
@@ -518,10 +542,11 @@ public class UncachedKeyRing {
 
                     // if we already have a cert, and this one is not newer: skip it
                     if (selfCert != null && selfCert.getCreationTime().before(cert.getCreationTime())) {
+                        redundantCerts += 1;
                         continue;
                     }
 
-                    selfCert = zig;
+                    selfCert = zert;
                     // if this is newer than a possibly existing revocation, drop that one
                     if (revocation != null && selfCert.getCreationTime().after(revocation.getCreationTime())) {
                         revocation = null;
@@ -535,48 +560,56 @@ public class UncachedKeyRing {
                         cert.init(masterKey);
                         if (!cert.verifySignature(key)) {
                             log.add(LogLevel.WARN, LogType.MSG_KC_SUB_REVOKE_BAD, null, indent);
+                            badCerts += 1;
                             continue;
                         }
                     } catch (PgpGeneralException e) {
                         log.add(LogLevel.WARN, LogType.MSG_KC_SUB_REVOKE_BAD_ERR, null, indent);
+                        badCerts += 1;
                         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;
+                    if (selfCert != null && selfCert.getCreationTime().after(cert.getCreationTime())) {
+                        redundantCerts += 1;
+                        continue;
                     }
+
+                    revocation = zert;
                 }
             }
 
             // it is not properly bound? error!
             if (selfCert == null) {
-                ring = PGPPublicKeyRing.removePublicKey(ring, modified);
+                ring = replacePublicKey(ring, modified);
 
                 log.add(LogLevel.ERROR, LogType.MSG_KC_SUB_NO_CERT,
-                        new String[]{PgpKeyHelper.convertKeyIdToHex(key.getKeyID())}, indent);
+                        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);
+            ring = replacePublicKey(ring, modified);
             indent -= 1;
         }
 
-        if (removedCerts > 0) {
-            log.add(LogLevel.OK, LogType.MSG_KC_SUCCESS_REMOVED,
-                    new String[] { Integer.toString(removedCerts) }, indent);
+        if (badCerts > 0 && redundantCerts > 0) {
+            log.add(LogLevel.OK, LogType.MSG_KC_SUCCESS_BAD_AND_RED,
+                    new String[] { Integer.toString(badCerts),
+                            Integer.toString(redundantCerts) }, indent);
+        } else if (badCerts > 0) {
+            log.add(LogLevel.OK, LogType.MSG_KC_SUCCESS_BAD,
+                    new String[] { Integer.toString(badCerts) }, indent);
+        } else if (redundantCerts > 0) {
+            log.add(LogLevel.OK, LogType.MSG_KC_SUCCESS_REDUNDANT,
+                    new String[] { Integer.toString(redundantCerts) }, indent);
         } else {
             log.add(LogLevel.OK, LogType.MSG_KC_SUCCESS, null, indent);
         }
@@ -584,5 +617,14 @@ public class UncachedKeyRing {
         return new UncachedKeyRing(ring);
     }
 
+    private static PGPKeyRing replacePublicKey(PGPKeyRing ring, PGPPublicKey key) {
+        if (ring instanceof PGPPublicKeyRing) {
+            return PGPPublicKeyRing.insertPublicKey((PGPPublicKeyRing) ring, key);
+        }
+        PGPSecretKeyRing secRing = (PGPSecretKeyRing) ring;
+        PGPSecretKey sKey = secRing.getSecretKey(key.getKeyID());
+        sKey = PGPSecretKey.replacePublicKey(sKey, key);
+        return PGPSecretKeyRing.insertSecretKey(secRing, sKey);
+    }
 
 }
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 be7f960a9..196ac1dee 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedSignature.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedSignature.java
@@ -179,7 +179,8 @@ public class WrappedSignature {
     }
 
     public boolean isLocal() {
-        if (!mSig.getHashedSubPackets().hasSubpacket(SignatureSubpacketTags.EXPORTABLE)) {
+        if (!mSig.hasSubpackets()
+                || !mSig.getHashedSubPackets().hasSubpacket(SignatureSubpacketTags.EXPORTABLE)) {
             return false;
         }
         SignatureSubpacket p = mSig.getHashedSubPackets().getSubpacket(SignatureSubpacketTags.EXPORTABLE);
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java
index 102c8e6d0..0218b457b 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java
@@ -29,6 +29,7 @@ import android.support.v4.util.LongSparseArray;
 
 import org.sufficientlysecure.keychain.Constants;
 import org.sufficientlysecure.keychain.pgp.KeyRing;
+import org.sufficientlysecure.keychain.pgp.Progressable;
 import org.sufficientlysecure.keychain.pgp.WrappedPublicKey;
 import org.sufficientlysecure.keychain.service.OperationResultParcel.LogType;
 import org.sufficientlysecure.keychain.service.OperationResultParcel.LogLevel;
@@ -259,11 +260,29 @@ public class ProviderHelper {
         }
     }
 
+    public SaveKeyringResult savePublicKeyRing(UncachedKeyRing keyRing) {
+        return savePublicKeyRing(keyRing, new Progressable() {
+            @Override
+            public void setProgress(String message, int current, int total) {
+                return;
+            }
+
+            @Override
+            public void setProgress(int resourceId, int current, int total) {
+                return;
+            }
+
+            @Override
+            public void setProgress(int current, int total) {
+                return;
+            }
+        });
+    }
     /**
      * Saves PGPPublicKeyRing with its keys and userIds in DB
      */
     @SuppressWarnings("unchecked")
-    public SaveKeyringResult savePublicKeyRing(UncachedKeyRing keyRing) {
+    public SaveKeyringResult savePublicKeyRing(UncachedKeyRing keyRing, Progressable progress) {
         if (keyRing.isSecret()) {
             log(LogLevel.ERROR, LogType.MSG_IP_BAD_TYPE_SECRET);
             return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog);
@@ -279,6 +298,9 @@ public class ProviderHelper {
 
         // Canonicalize this key, to assert a number of assumptions made about it.
         keyRing = keyRing.canonicalize(mLog, mIndent);
+        if (keyRing == null) {
+            return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog);
+        }
 
         UncachedPublicKey masterKey = keyRing.getPublicKey();
 
@@ -287,6 +309,7 @@ public class ProviderHelper {
         try {
             secretRing = getWrappedSecretKeyRing(masterKeyId).getUncached();
             log(LogLevel.DEBUG, LogType.MSG_IP_PRESERVING_SECRET);
+            progress.setProgress(LogType.MSG_IP_PRESERVING_SECRET.getMsgId(), 30, 100);
         } catch (NotFoundException e) {
             secretRing = null;
         }
@@ -302,7 +325,6 @@ public class ProviderHelper {
 
             log(LogLevel.INFO, LogType.MSG_IP_INSERT_KEYRING);
             { // insert keyring
-                // insert new version of this keyRing
                 ContentValues values = new ContentValues();
                 values.put(KeyRingData.MASTER_KEY_ID, masterKeyId);
                 try {
@@ -317,13 +339,15 @@ public class ProviderHelper {
             }
 
             log(LogLevel.INFO, LogType.MSG_IP_INSERT_SUBKEYS);
+            progress.setProgress(LogType.MSG_IP_INSERT_SUBKEYS.getMsgId(), 40, 100);
             mIndent += 1;
             { // insert subkeys
                 Uri uri = Keys.buildKeysUri(Long.toString(masterKeyId));
                 int rank = 0;
                 for (UncachedPublicKey key : new IterableIterator<UncachedPublicKey>(keyRing.getPublicKeys())) {
-                    log(LogLevel.DEBUG, LogType.MSG_IP_SUBKEY, new String[]{
-                            PgpKeyHelper.convertKeyIdToHex(key.getKeyId())
+                    long keyId = key.getKeyId();
+                    log(LogLevel.DEBUG, keyId == masterKeyId ? LogType.MSG_IP_MASTER : LogType.MSG_IP_SUBKEY, new String[]{
+                            PgpKeyHelper.convertKeyIdToHex(keyId)
                     });
                     mIndent += 1;
 
@@ -341,21 +365,41 @@ public class ProviderHelper {
                     values.put(Keys.CAN_ENCRYPT, e);
                     values.put(Keys.CAN_SIGN, s);
                     values.put(Keys.IS_REVOKED, key.isRevoked());
-                    if (c) {
-                        if (e) {
-                            log(LogLevel.DEBUG, s ? LogType.MSG_IP_SUBKEY_FLAGS_CES
-                                    : LogType.MSG_IP_SUBKEY_FLAGS_CEX, null);
+                    if (masterKeyId == keyId) {
+                        if (c) {
+                            if (e) {
+                                log(LogLevel.DEBUG, s ? LogType.MSG_IP_MASTER_FLAGS_CES
+                                        : LogType.MSG_IP_MASTER_FLAGS_CEX, null);
+                            } else {
+                                log(LogLevel.DEBUG, s ? LogType.MSG_IP_MASTER_FLAGS_CXS
+                                        : LogType.MSG_IP_MASTER_FLAGS_CXX, null);
+                            }
                         } else {
-                            log(LogLevel.DEBUG, s ? LogType.MSG_IP_SUBKEY_FLAGS_CXS
-                                    : LogType.MSG_IP_SUBKEY_FLAGS_CXX, null);
+                            if (e) {
+                                log(LogLevel.DEBUG, s ? LogType.MSG_IP_MASTER_FLAGS_XES
+                                        : LogType.MSG_IP_MASTER_FLAGS_XEX, null);
+                            } else {
+                                log(LogLevel.DEBUG, s ? LogType.MSG_IP_MASTER_FLAGS_XXS
+                                        : LogType.MSG_IP_MASTER_FLAGS_XXX, null);
+                            }
                         }
                     } else {
-                        if (e) {
-                            log(LogLevel.DEBUG, s ? LogType.MSG_IP_SUBKEY_FLAGS_XES
-                                    : LogType.MSG_IP_SUBKEY_FLAGS_XEX, null);
+                        if (c) {
+                            if (e) {
+                                log(LogLevel.DEBUG, s ? LogType.MSG_IP_SUBKEY_FLAGS_CES
+                                        : LogType.MSG_IP_SUBKEY_FLAGS_CEX, null);
+                            } else {
+                                log(LogLevel.DEBUG, s ? LogType.MSG_IP_SUBKEY_FLAGS_CXS
+                                        : LogType.MSG_IP_SUBKEY_FLAGS_CXX, null);
+                            }
                         } else {
-                            log(LogLevel.DEBUG, s ? LogType.MSG_IP_SUBKEY_FLAGS_XXS
-                                    : LogType.MSG_IP_SUBKEY_FLAGS_XXX, null);
+                            if (e) {
+                                log(LogLevel.DEBUG, s ? LogType.MSG_IP_SUBKEY_FLAGS_XES
+                                        : LogType.MSG_IP_SUBKEY_FLAGS_XEX, null);
+                            } else {
+                                log(LogLevel.DEBUG, s ? LogType.MSG_IP_SUBKEY_FLAGS_XXS
+                                        : LogType.MSG_IP_SUBKEY_FLAGS_XXX, null);
+                            }
                         }
                     }
 
@@ -365,13 +409,13 @@ public class ProviderHelper {
                     if (expiryDate != null) {
                         values.put(Keys.EXPIRY, expiryDate.getTime() / 1000);
                         if (key.isExpired()) {
-                            log(LogLevel.DEBUG, LogType.MSG_IP_SUBKEY_EXPIRED, new String[]{
-                                    expiryDate.toString()
-                            });
+                            log(LogLevel.DEBUG, keyId == masterKeyId ?
+                                    LogType.MSG_IP_MASTER_EXPIRED : LogType.MSG_IP_SUBKEY_EXPIRED,
+                                    new String[]{ expiryDate.toString() });
                         } else {
-                            log(LogLevel.DEBUG, LogType.MSG_IP_SUBKEY_EXPIRES, new String[]{
-                                    expiryDate.toString()
-                            });
+                            log(LogLevel.DEBUG, keyId == masterKeyId ?
+                                    LogType.MSG_IP_MASTER_EXPIRES : LogType.MSG_IP_SUBKEY_EXPIRES,
+                                    new String[] { expiryDate.toString() });
                         }
                     }
 
@@ -415,10 +459,9 @@ public class ProviderHelper {
                             if (!cert.isRevocation()) {
                                 item.selfCert = cert;
                                 item.isPrimary = cert.isPrimaryUserId();
-                                log(LogLevel.DEBUG, LogType.MSG_IP_UID_SELF_GOOD);
                             } else {
                                 item.isRevoked = true;
-                                log(LogLevel.DEBUG, LogType.MSG_IP_UID_REVOKED);
+                                log(LogLevel.INFO, LogType.MSG_IP_UID_REVOKED);
                             }
 
                         }
@@ -457,6 +500,7 @@ public class ProviderHelper {
             }
             mIndent -= 1;
 
+            progress.setProgress(LogType.MSG_IP_UID_REORDER.getMsgId(), 80, 100);
             log(LogLevel.DEBUG, LogType.MSG_IP_UID_REORDER);
             // primary before regular before revoked (see UserIdItem.compareTo)
             // this is a stable sort, so the order of keys is otherwise preserved.
@@ -479,7 +523,6 @@ public class ProviderHelper {
                 }
             }
 
-            log(LogLevel.DEBUG, LogType.MSG_IP_PREPARE_SUCCESS);
             mIndent -= 1;
 
         } catch (IOException e) {
@@ -501,6 +544,7 @@ public class ProviderHelper {
             }
 
             log(LogLevel.DEBUG, LogType.MSG_IP_APPLY_BATCH);
+            progress.setProgress(LogType.MSG_IP_APPLY_BATCH.getMsgId(), 90, 100);
             mContentResolver.applyBatch(KeychainContract.CONTENT_AUTHORITY, operations);
 
             // Save the saved keyring (if any)
@@ -514,6 +558,7 @@ public class ProviderHelper {
 
             mIndent -= 1;
             log(LogLevel.OK, LogType.MSG_IP_SUCCESS);
+            progress.setProgress(LogType.MSG_IP_SUCCESS.getMsgId(), 100, 100);
             return new SaveKeyringResult(result, mLog);
 
         } catch (RemoteException e) {
@@ -564,6 +609,12 @@ public class ProviderHelper {
             return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog);
         }
 
+        // Canonicalize this key, to assert a number of assumptions made about it.
+        keyRing = keyRing.canonicalize(mLog, mIndent);
+        if (keyRing == null) {
+            return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog);
+        }
+
         long masterKeyId = keyRing.getMasterKeyId();
         log(LogLevel.START, LogType.MSG_IS,
                 new String[]{ PgpKeyHelper.convertKeyIdToHex(masterKeyId) });
@@ -579,7 +630,10 @@ public class ProviderHelper {
             values.put(KeyRingData.KEY_RING_DATA, keyRing.getEncoded());
             // insert new version of this keyRing
             Uri uri = KeyRingData.buildSecretKeyRingUri(Long.toString(masterKeyId));
-            mContentResolver.insert(uri, values);
+            if (mContentResolver.insert(uri, values) == null) {
+                log(LogLevel.ERROR, LogType.MSG_IS_DB_EXCEPTION);
+                return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog);
+            }
         } catch (IOException e) {
             Log.e(Constants.TAG, "Failed to encode key!", e);
             log(LogLevel.ERROR, LogType.MSG_IS_IO_EXCPTION);
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 b5f01ce4d..f1f6c304a 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java
@@ -16,6 +16,7 @@ import java.util.ArrayList;
  * 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, the number specifying varying degrees of success. The first bit
@@ -101,6 +102,23 @@ public class OperationResultParcel implements Parcelable {
 
     }
 
+    /** This is an enum of all possible log events.
+     *
+     * Element names should generally be prefixed with MSG_XX_ where XX is an
+     * identifier based on the related activity.
+     *
+     * Log messages should occur for each distinguishable action group.  For
+     * each such group, one message is displayed followed by warnings or
+     * errors, and optionally subactions. The granularity should generally be
+     * optimistic: No "success" messages are printed except for the outermost
+     * operations - the success of an action group is indicated by the
+     * beginning message of the next action group.
+     *
+     * Log messages should be in present tense, There should be no trailing
+     * punctuation, except for error messages which may end in an exclamation
+     * mark.
+     *
+     */
     public static enum LogType {
 
         // import public
@@ -114,15 +132,24 @@ public class OperationResultParcel implements Parcelable {
         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_INSERT_KEYRING (R.string.msg_ip_insert_keyring),
-        MSG_IP_INSERT_SUBKEYS (R.string.msg_ip_insert_subkeys),
+        MSG_IP_INSERT_SUBKEYS (R.string.msg_ip_insert_keys),
         MSG_IP_PREPARE (R.string.msg_ip_prepare),
-        MSG_IP_PREPARE_SUCCESS(R.string.msg_ip_prepare_success),
         MSG_IP_PRESERVING_SECRET (R.string.msg_ip_preserving_secret),
         MSG_IP_REINSERT_SECRET (R.string.msg_ip_reinsert_secret),
+        MSG_IP_MASTER (R.string.msg_ip_master),
+        MSG_IP_MASTER_EXPIRED (R.string.msg_ip_master_expired),
+        MSG_IP_MASTER_EXPIRES (R.string.msg_ip_master_expires),
+        MSG_IP_MASTER_FLAGS_CES (R.string.msg_ip_master_flags_ces),
+        MSG_IP_MASTER_FLAGS_CEX (R.string.msg_ip_master_flags_cex),
+        MSG_IP_MASTER_FLAGS_CXS (R.string.msg_ip_master_flags_cxs),
+        MSG_IP_MASTER_FLAGS_XES (R.string.msg_ip_master_flags_xes),
+        MSG_IP_MASTER_FLAGS_CXX (R.string.msg_ip_master_flags_cxx),
+        MSG_IP_MASTER_FLAGS_XEX (R.string.msg_ip_master_flags_xex),
+        MSG_IP_MASTER_FLAGS_XXS (R.string.msg_ip_master_flags_xxs),
+        MSG_IP_MASTER_FLAGS_XXX (R.string.msg_ip_master_flags_xxx),
         MSG_IP_SUBKEY (R.string.msg_ip_subkey),
         MSG_IP_SUBKEY_EXPIRED (R.string.msg_ip_subkey_expired),
         MSG_IP_SUBKEY_EXPIRES (R.string.msg_ip_subkey_expires),
-        MSG_IP_SUBKEY_FLAGS (R.string.msg_ip_subkey_flags),
         MSG_IP_SUBKEY_FLAGS_CES (R.string.msg_ip_subkey_flags_ces),
         MSG_IP_SUBKEY_FLAGS_CEX (R.string.msg_ip_subkey_flags_cex),
         MSG_IP_SUBKEY_FLAGS_CXS (R.string.msg_ip_subkey_flags_cxs),
@@ -140,11 +167,11 @@ public class OperationResultParcel implements Parcelable {
         MSG_IP_UID_REORDER(R.string.msg_ip_uid_reorder),
         MSG_IP_UID_PROCESSING (R.string.msg_ip_uid_processing),
         MSG_IP_UID_REVOKED (R.string.msg_ip_uid_revoked),
-        MSG_IP_UID_SELF_GOOD (R.string.msg_ip_uid_self_good),
 
         // import secret
         MSG_IS(R.string.msg_is),
         MSG_IS_BAD_TYPE_PUBLIC (R.string.msg_is_bad_type_public),
+        MSG_IS_DB_EXCEPTION (R.string.msg_is_db_exception),
         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),
@@ -153,9 +180,10 @@ public class OperationResultParcel implements Parcelable {
         MSG_IS_SUCCESS (R.string.msg_is_success),
 
         // keyring canonicalization
-        MSG_KC (R.string.msg_kc),
+        MSG_KC_PUBLIC (R.string.msg_kc_public),
+        MSG_KC_SECRET (R.string.msg_kc_secret),
+        MSG_KC_FATAL_NO_UID (R.string.msg_kc_fatal_no_uid),
         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),
@@ -176,8 +204,9 @@ public class OperationResultParcel implements Parcelable {
         MSG_KC_SUB_REVOKE_BAD_ERR (R.string.msg_kc_sub_revoke_bad_err),
         MSG_KC_SUB_REVOKE_BAD (R.string.msg_kc_sub_revoke_bad),
         MSG_KC_SUB_REVOKE_DUP (R.string.msg_kc_sub_revoke_dup),
-        MSG_KC_SUB_SUCCESS (R.string.msg_kc_sub_success),
-        MSG_KC_SUCCESS_REMOVED (R.string.msg_kc_success_removed),
+        MSG_KC_SUCCESS_BAD (R.string.msg_kc_success_bad),
+        MSG_KC_SUCCESS_BAD_AND_RED (R.string.msg_kc_success_bad_and_red),
+        MSG_KC_SUCCESS_REDUNDANT (R.string.msg_kc_success_redundant),
         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),
@@ -185,6 +214,8 @@ public class OperationResultParcel implements Parcelable {
         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_FOREIGN (R.string.msg_kc_uid_foreign),
+        MSG_KC_UID_NO_CERT (R.string.msg_kc_uid_no_cert),
         MSG_KC_UID_REVOKE_DUP (R.string.msg_kc_uid_revoke_dup),
         MSG_KC_UID_REVOKE_OLD (R.string.msg_kc_uid_revoke_old),
         ;
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ViewKeyActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ViewKeyActivity.java
index c3ca0334f..1912b6e7d 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ViewKeyActivity.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ViewKeyActivity.java
@@ -56,6 +56,7 @@ import org.sufficientlysecure.keychain.pgp.PgpKeyHelper;
 import org.sufficientlysecure.keychain.provider.KeychainContract;
 import org.sufficientlysecure.keychain.provider.ProviderHelper;
 import org.sufficientlysecure.keychain.ui.adapter.PagerTabStripAdapter;
+import org.sufficientlysecure.keychain.ui.widget.SlidingTabLayout.TabColorizer;
 import org.sufficientlysecure.keychain.util.Log;
 import org.sufficientlysecure.keychain.ui.widget.SlidingTabLayout;
 
@@ -121,6 +122,18 @@ public class ViewKeyActivity extends ActionBarActivity implements
         mViewPager = (ViewPager) findViewById(R.id.view_key_pager);
         mSlidingTabLayout = (SlidingTabLayout) findViewById(R.id.view_key_sliding_tab_layout);
 
+        mSlidingTabLayout.setCustomTabColorizer(new TabColorizer() {
+            @Override
+            public int getIndicatorColor(int position) {
+                return position == TAB_CERTS || position == TAB_KEYS ? 0xFFFF4444 : 0xFFAA66CC;
+            }
+
+            @Override
+            public int getDividerColor(int position) {
+                return 0;
+            }
+        });
+
         int switchToTab = TAB_MAIN;
         Intent intent = getIntent();
         if (intent.getExtras() != null && intent.getExtras().containsKey(EXTRA_SELECTED_TAB)) {