certify: preserve user id certificates when saving secret keys

3 files changed, 72 insertions, 7 deletions

diff --git a/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java b/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java
index 78e48b4ab..40a0b72ce 100644
--- a/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java
+++ b/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java
@@ -36,6 +36,7 @@ import org.sufficientlysecure.keychain.Id;
 import org.sufficientlysecure.keychain.R;
 import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException;
 import org.sufficientlysecure.keychain.provider.ProviderHelper;
+import org.sufficientlysecure.keychain.util.IterableIterator;
 import org.sufficientlysecure.keychain.util.Log;
 import org.sufficientlysecure.keychain.util.Primes;
 import org.sufficientlysecure.keychain.util.ProgressDialogUpdater;
@@ -46,6 +47,8 @@ import java.security.*;
 import java.util.ArrayList;
 import java.util.Date;
 import java.util.GregorianCalendar;
+import java.util.Iterator;
+import java.util.List;
 import java.util.TimeZone;
 
 public class PgpKeyOperation {
@@ -198,7 +201,7 @@ public class PgpKeyOperation {
 
     public void buildSecretKey(ArrayList<String> userIds, ArrayList<PGPSecretKey> keys,
                                ArrayList<Integer> keysUsages, ArrayList<GregorianCalendar> keysExpiryDates,
-                               long masterKeyId, String oldPassPhrase,
+                               PGPPublicKey oldPublicKey, String oldPassPhrase,
                                String newPassPhrase) throws PgpGeneralException, NoSuchProviderException,
             PGPException, NoSuchAlgorithmException, SignatureException, IOException {
 
@@ -249,9 +252,32 @@ public class PgpKeyOperation {
 
                 updateProgress(R.string.progress_certifying_master_key, 20, 100);
 
-                // TODO: if we are editing a key, keep old certs, don't remake certs we don't have to.
-
+                // re-add old certificates, or create new ones for new uids
                 for (String userId : userIds) {
+                    // re-add certs for this uid, take a note if self-signed cert is in there
+                    boolean foundSelfSign = false;
+                    Iterator<PGPSignature> it = tmpKey.getSignaturesForID(userId);
+                    if(it != null) for(PGPSignature sig : new IterableIterator<PGPSignature>(it)) {
+                        if(sig.getKeyID() == masterPublicKey.getKeyID()) {
+                            // already have a self sign? skip this other one, then.
+                            // note: PGPKeyRingGenerator adds one cert for the main user id, which
+                            // will lead to duplicates. unfortunately, if we add any other here
+                            // first, that will change the main user id order...
+                            if(foundSelfSign)
+                                continue;
+                            foundSelfSign = true;
+                        }
+                        Log.d(Constants.TAG, "adding old sig for " + userId + " from "
+                                + PgpKeyHelper.convertKeyIdToHex(sig.getKeyID()));
+                        masterPublicKey = PGPPublicKey.addCertification(masterPublicKey, userId, sig);
+                    }
+
+                    // there was an old self-signed certificate for this uid
+                    if(foundSelfSign)
+                        continue;
+
+                    Log.d(Constants.TAG, "generating self-signed cert for " + userId);
+
                     PGPContentSignerBuilder signerBuilder = new JcaPGPContentSignerBuilder(
                             masterPublicKey.getAlgorithm(), HashAlgorithmTags.SHA1)
                             .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME);
@@ -330,7 +356,7 @@ public class PgpKeyOperation {
         updateProgress(R.string.progress_adding_sub_keys, 40, 100);
 
         for (int i = 1; i < keys.size(); ++i) {
-            updateProgress(40 + 50 * (i - 1) / (keys.size() - 1), 100);
+            updateProgress(40 + 40 * (i - 1) / (keys.size() - 1), 100);
 
             PGPSecretKey subKey = keys.get(i);
             PGPPublicKey subPublicKey = subKey.getPublicKey();
@@ -400,8 +426,38 @@ public class PgpKeyOperation {
         PGPSecretKeyRing secretKeyRing = keyGen.generateSecretKeyRing();
         PGPPublicKeyRing publicKeyRing = keyGen.generatePublicKeyRing();
 
+        updateProgress(R.string.progress_re_adding_certs, 80, 100);
+
+        // re-add certificates from old public key
+        // TODO: this only takes care of user id certificates, what about others?
+        PGPPublicKey pubkey = publicKeyRing.getPublicKey();
+        for(String uid : new IterableIterator<String>(pubkey.getUserIDs())) {
+            for(PGPSignature sig : new IterableIterator<PGPSignature>(oldPublicKey.getSignaturesForID(uid), true)) {
+                // but skip self certificates
+                if(sig.getKeyID() == pubkey.getKeyID())
+                    continue;
+                pubkey = PGPPublicKey.addCertification(pubkey, uid, sig);
+            }
+        }
+        publicKeyRing = PGPPublicKeyRing.insertPublicKey(publicKeyRing, pubkey);
+
         updateProgress(R.string.progress_saving_key_ring, 90, 100);
 
+        /* additional handy debug info
+        Log.d(Constants.TAG, " ------- in private key -------");
+        for(String uid : new IterableIterator<String>(secretKeyRing.getPublicKey().getUserIDs())) {
+            for(PGPSignature sig : new IterableIterator<PGPSignature>(secretKeyRing.getPublicKey().getSignaturesForID(uid))) {
+                Log.d(Constants.TAG, "sig: " + PgpKeyHelper.convertKeyIdToHex(sig.getKeyID()) + " for " + uid);
+            }
+        }
+        Log.d(Constants.TAG, " ------- in public key -------");
+        for(String uid : new IterableIterator<String>(publicKeyRing.getPublicKey().getUserIDs())) {
+            for(PGPSignature sig : new IterableIterator<PGPSignature>(publicKeyRing.getPublicKey().getSignaturesForID(uid))) {
+                Log.d(Constants.TAG, "sig: " + PgpKeyHelper.convertKeyIdToHex(sig.getKeyID()) + " for " + uid);
+            }
+        }
+        */
+
         ProviderHelper.saveKeyRing(mContext, secretKeyRing);
         ProviderHelper.saveKeyRing(mContext, publicKeyRing);
 
diff --git a/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java b/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java
index 506ff2735..daaff5d54 100644
--- a/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java
+++ b/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java
@@ -543,8 +543,9 @@ public class KeychainIntentService extends IntentService
                             ProviderHelper.getPGPSecretKeyRingByKeyId(this, masterKeyId),
                             oldPassPhrase, newPassPhrase);
                 } else {
-                    keyOperations.buildSecretKey(userIds, keys, keysUsages, keysExpiryDates, masterKeyId,
-                            oldPassPhrase, newPassPhrase);
+                    PGPPublicKey pubkey = ProviderHelper.getPGPPublicKeyByKeyId(this, masterKeyId);
+                    keyOperations.buildSecretKey(userIds, keys, keysUsages, keysExpiryDates,
+                            pubkey, oldPassPhrase, newPassPhrase);
                 }
                 PassphraseCacheService.addCachedPassphrase(this, masterKeyId, newPassPhrase);
 
diff --git a/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/util/IterableIterator.java b/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/util/IterableIterator.java
index caaa07524..40105df4f 100644
--- a/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/util/IterableIterator.java
+++ b/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/util/IterableIterator.java
@@ -16,13 +16,21 @@
 
 package org.sufficientlysecure.keychain.util;
 
+import java.util.ArrayList;
 import java.util.Iterator;
 
 public class IterableIterator<T> implements Iterable<T> {
     private Iterator<T> mIter;
 
-    public IterableIterator(Iterator<T> iter) {
+    public IterableIterator(Iterator<T> iter, boolean failsafe) {
         mIter = iter;
+        if(failsafe && mIter == null) {
+            // is there a better way?
+            mIter = new ArrayList<T>().iterator();
+        }
+    }
+    public IterableIterator(Iterator<T> iter) {
+        this(iter, false);
     }
 
     public Iterator<T> iterator() {