some UncachedKeyRing fixes, primary user id mostly

3 files changed, 67 insertions, 18 deletions

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 33db7771b..3171d0565 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedPublicKey.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedPublicKey.java
@@ -77,26 +77,65 @@ public class UncachedPublicKey {
         return mPublicKey.getBitStrength();
     }
 
+    /** Returns the primary user id, as indicated by the public key's self certificates.
+     *
+     * This is an expensive operation, since potentially a lot of certificates (and revocations)
+     * have to be checked, and even then the result is NOT guaranteed to be constant through a
+     * canonicalization operation.
+     *
+     * Returns null if there is no primary user id (as indicated by certificates)
+     *
+     */
     public String getPrimaryUserId() {
+        String found = null;
+        PGPSignature foundSig = null;
         for (String userId : new IterableIterator<String>(mPublicKey.getUserIDs())) {
+            PGPSignature revocation = null;
+
             for (PGPSignature sig : new IterableIterator<PGPSignature>(mPublicKey.getSignaturesForID(userId))) {
-                if (sig.getHashedSubPackets() != null
-                        && sig.getHashedSubPackets().hasSubpacket(SignatureSubpacketTags.PRIMARY_USER_ID)) {
-                    try {
+                try {
+
+                    // if this is a revocation, this is not the user id
+                    if (sig.getSignatureType() == PGPSignature.CERTIFICATION_REVOCATION) {
+                        // make sure it's actually valid
+                        sig.init(new JcaPGPContentVerifierBuilderProvider().setProvider(
+                                Constants.BOUNCY_CASTLE_PROVIDER_NAME), mPublicKey);
+                        if (!sig.verifyCertification(userId, mPublicKey)) {
+                            continue;
+                        }
+                        if (found != null && found.equals(userId)) {
+                            found = null;
+                        }
+                        revocation = sig;
+                        // this revocation may still be overridden by a newer cert
+                        continue;
+                    }
+
+                    if (sig.getHashedSubPackets() != null && sig.getHashedSubPackets().isPrimaryUserID()) {
+                        if (foundSig != null && sig.getCreationTime().before(foundSig.getCreationTime())) {
+                            continue;
+                        }
+                        // ignore if there is a newer revocation for this user id
+                        if (revocation != null && sig.getCreationTime().before(revocation.getCreationTime())) {
+                            continue;
+                        }
                         // make sure it's actually valid
                         sig.init(new JcaPGPContentVerifierBuilderProvider().setProvider(
                                 Constants.BOUNCY_CASTLE_PROVIDER_NAME), mPublicKey);
                         if (sig.verifyCertification(userId, mPublicKey)) {
-                            return userId;
+                            found = userId;
+                            foundSig = sig;
+                            // this one can't be relevant anymore at this point
+                            revocation = null;
                         }
-                    } catch (Exception e) {
-                        // nothing bad happens, the key is just not considered the primary key id
                     }
-                }
 
+                } catch (Exception e) {
+                    // nothing bad happens, the key is just not considered the primary key id
+                }
             }
         }
-        return null;
+        return found;
     }
 
     public ArrayList<String> getUnorderedUserIds() {
@@ -186,6 +225,21 @@ public class UncachedPublicKey {
         return mPublicKey;
     }
 
+    public Iterator<WrappedSignature> getSignatures() {
+        final Iterator<PGPSignature> it = mPublicKey.getSignatures();
+        return new Iterator<WrappedSignature>() {
+            public void remove() {
+                it.remove();
+            }
+            public WrappedSignature next() {
+                return new WrappedSignature(it.next());
+            }
+            public boolean hasNext() {
+                return it.hasNext();
+            }
+        };
+    }
+
     public Iterator<WrappedSignature> getSignaturesForId(String userId) {
         final Iterator<PGPSignature> it = mPublicKey.getSignaturesForID(userId);
         return new Iterator<WrappedSignature>() {
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedKeyRing.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedKeyRing.java
index 73fd92a3a..2fcd05204 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedKeyRing.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedKeyRing.java
@@ -105,8 +105,4 @@ public abstract class WrappedKeyRing extends KeyRing {
         return getRing().getEncoded();
     }
 
-    public UncachedKeyRing getUncached() {
-        return new UncachedKeyRing(getRing());
-    }
-
 }
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 b5609a327..c338c9d95 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java
@@ -450,8 +450,7 @@ public class ProviderHelper {
                             if (cert.verifySignature(masterKey, userId)) {
                                 item.trustedCerts.add(cert);
                                 log(LogLevel.INFO, LogType.MSG_IP_UID_CERT_GOOD,
-                                        PgpKeyHelper.convertKeyIdToHexShort(trustedKey.getKeyId()),
-                                        trustedKey.getPrimaryUserId()
+                                        PgpKeyHelper.convertKeyIdToHexShort(trustedKey.getKeyId())
                                 );
                             } else {
                                 log(LogLevel.WARN, LogType.MSG_IP_UID_CERT_BAD);
@@ -670,7 +669,7 @@ public class ProviderHelper {
 
             // If there is an old keyring, merge it
             try {
-                UncachedKeyRing oldPublicRing = getWrappedPublicKeyRing(masterKeyId).getUncached();
+                UncachedKeyRing oldPublicRing = getWrappedPublicKeyRing(masterKeyId).getUncachedKeyRing();
 
                 // Merge data from new public ring into the old one
                 publicRing = oldPublicRing.merge(publicRing, mLog, mIndent);
@@ -706,7 +705,7 @@ public class ProviderHelper {
             // If there is a secret key, merge new data (if any) and save the key for later
             UncachedKeyRing secretRing;
             try {
-                secretRing = getWrappedSecretKeyRing(publicRing.getMasterKeyId()).getUncached();
+                secretRing = getWrappedSecretKeyRing(publicRing.getMasterKeyId()).getUncachedKeyRing();
 
                 // Merge data from new public ring into secret one
                 secretRing = secretRing.merge(publicRing, mLog, mIndent);
@@ -754,7 +753,7 @@ public class ProviderHelper {
 
             // If there is an old secret key, merge it.
             try {
-                UncachedKeyRing oldSecretRing = getWrappedSecretKeyRing(masterKeyId).getUncached();
+                UncachedKeyRing oldSecretRing = getWrappedSecretKeyRing(masterKeyId).getUncachedKeyRing();
 
                 // Merge data from new secret ring into old one
                 secretRing = oldSecretRing.merge(secretRing, mLog, mIndent);
@@ -791,7 +790,7 @@ public class ProviderHelper {
             // Merge new data into public keyring as well, if there is any
             UncachedKeyRing publicRing;
             try {
-                UncachedKeyRing oldPublicRing = getWrappedPublicKeyRing(masterKeyId).getUncached();
+                UncachedKeyRing oldPublicRing = getWrappedPublicKeyRing(masterKeyId).getUncachedKeyRing();
 
                 // Merge data from new public ring into secret one
                 publicRing = oldPublicRing.merge(secretRing, mLog, mIndent);