Merge branch 'new-edit'

Conflicts:
	OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java
	OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java
	OpenKeychain/src/main/res/values/strings.xml

2 files changed, 236 insertions, 714 deletions

diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java
index 44fc4c8c9..c590200ee 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java
@@ -26,10 +26,8 @@ import org.spongycastle.jce.spec.ElGamalParameterSpec;
 import org.spongycastle.openpgp.PGPEncryptedData;
 import org.spongycastle.openpgp.PGPException;
 import org.spongycastle.openpgp.PGPKeyPair;
-import org.spongycastle.openpgp.PGPKeyRingGenerator;
 import org.spongycastle.openpgp.PGPPrivateKey;
 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;
@@ -49,7 +47,9 @@ import org.spongycastle.openpgp.operator.jcajce.JcePBESecretKeyEncryptorBuilder;
 import org.sufficientlysecure.keychain.Constants;
 import org.sufficientlysecure.keychain.R;
 import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralMsgIdException;
-import org.sufficientlysecure.keychain.service.OldSaveKeyringParcel;
+import org.sufficientlysecure.keychain.service.OperationResultParcel.LogLevel;
+import org.sufficientlysecure.keychain.service.OperationResultParcel.LogType;
+import org.sufficientlysecure.keychain.service.OperationResultParcel.OperationLog;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel;
 import org.sufficientlysecure.keychain.util.IterableIterator;
 import org.sufficientlysecure.keychain.util.Primes;
@@ -62,11 +62,9 @@ import java.security.NoSuchAlgorithmException;
 import java.security.NoSuchProviderException;
 import java.security.SecureRandom;
 import java.security.SignatureException;
-import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Calendar;
 import java.util.Date;
-import java.util.Iterator;
 import java.util.List;
 import java.util.TimeZone;
 
@@ -103,751 +101,269 @@ public class PgpKeyOperation {
         }
     }
 
-    void updateProgress(int current, int total) {
-        if (mProgress != null) {
-            mProgress.setProgress(current, total);
-        }
-    }
-
-    /**
-     * Creates new secret key.
-     *
-     * @param algorithmChoice
-     * @param keySize
-     * @param passphrase
-     * @param isMasterKey
-     * @return A newly created PGPSecretKey
-     * @throws NoSuchAlgorithmException
-     * @throws PGPException
-     * @throws NoSuchProviderException
-     * @throws PgpGeneralMsgIdException
-     * @throws InvalidAlgorithmParameterException
-     */
-
-    // TODO: key flags?
-    public byte[] createKey(int algorithmChoice, int keySize, String passphrase,
-                                  boolean isMasterKey)
-            throws NoSuchAlgorithmException, PGPException, NoSuchProviderException,
-            PgpGeneralMsgIdException, InvalidAlgorithmParameterException {
-
-        if (keySize < 512) {
-            throw new PgpGeneralMsgIdException(R.string.error_key_size_minimum512bit);
-        }
-
-        if (passphrase == null) {
-            passphrase = "";
-        }
-
-        int algorithm;
-        KeyPairGenerator keyGen;
-
-        switch (algorithmChoice) {
-            case Constants.choice.algorithm.dsa: {
-                keyGen = KeyPairGenerator.getInstance("DSA", Constants.BOUNCY_CASTLE_PROVIDER_NAME);
-                keyGen.initialize(keySize, new SecureRandom());
-                algorithm = PGPPublicKey.DSA;
-                break;
-            }
-
-            case Constants.choice.algorithm.elgamal: {
-                if (isMasterKey) {
-                    throw new PgpGeneralMsgIdException(R.string.error_master_key_must_not_be_el_gamal);
-                }
-                keyGen = KeyPairGenerator.getInstance("ElGamal", Constants.BOUNCY_CASTLE_PROVIDER_NAME);
-                BigInteger p = Primes.getBestPrime(keySize);
-                BigInteger g = new BigInteger("2");
-
-                ElGamalParameterSpec elParams = new ElGamalParameterSpec(p, g);
-
-                keyGen.initialize(elParams);
-                algorithm = PGPPublicKey.ELGAMAL_ENCRYPT;
-                break;
-            }
-
-            case Constants.choice.algorithm.rsa: {
-                keyGen = KeyPairGenerator.getInstance("RSA", Constants.BOUNCY_CASTLE_PROVIDER_NAME);
-                keyGen.initialize(keySize, new SecureRandom());
-
-                algorithm = PGPPublicKey.RSA_GENERAL;
-                break;
-            }
-
-            default: {
-                throw new PgpGeneralMsgIdException(R.string.error_unknown_algorithm_choice);
-            }
-        }
-
-        // build new key pair
-        PGPKeyPair keyPair = new JcaPGPKeyPair(algorithm, keyGen.generateKeyPair(), new Date());
-
-        // define hashing and signing algos
-        PGPDigestCalculator sha1Calc = new JcaPGPDigestCalculatorProviderBuilder().build().get(
-                HashAlgorithmTags.SHA1);
-
-        // Build key encrypter and decrypter based on passphrase
-        PBESecretKeyEncryptor keyEncryptor = new JcePBESecretKeyEncryptorBuilder(
-                PGPEncryptedData.CAST5, sha1Calc)
-                .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME).build(passphrase.toCharArray());
+    /** Creates new secret key. */
+    private PGPSecretKey createKey(int algorithmChoice, int keySize, String passphrase,
+                                  boolean isMasterKey) throws PgpGeneralMsgIdException {
 
         try {
-            return new PGPSecretKey(keyPair.getPrivateKey(), keyPair.getPublicKey(),
-                    sha1Calc, isMasterKey, keyEncryptor).getEncoded();
-        } catch(IOException e) {
-            throw new PgpGeneralMsgIdException(R.string.error_encoding);
-        }
-    }
-
-    public Pair<UncachedKeyRing,UncachedKeyRing> buildNewSecretKey(
-        OldSaveKeyringParcel saveParcel)
-            throws PgpGeneralMsgIdException, PGPException, SignatureException, IOException {
-
-        int usageId = saveParcel.keysUsages.get(0);
-        boolean canSign;
-        String mainUserId = saveParcel.userIds.get(0);
-
-        PGPSecretKey masterKey = saveParcel.keys.get(0).getSecretKeyExternal();
-
-        // this removes all userIds and certifications previously attached to the masterPublicKey
-        PGPPublicKey masterPublicKey = masterKey.getPublicKey();
-
-        PBESecretKeyDecryptor keyDecryptor = new JcePBESecretKeyDecryptorBuilder().setProvider(
-                Constants.BOUNCY_CASTLE_PROVIDER_NAME).build(saveParcel.oldPassphrase.toCharArray());
-        PGPPrivateKey masterPrivateKey = masterKey.extractPrivateKey(keyDecryptor);
-
-        updateProgress(R.string.progress_certifying_master_key, 20, 100);
-
-        for (String userId : saveParcel.userIds) {
-            PGPContentSignerBuilder signerBuilder = new JcaPGPContentSignerBuilder(
-                    masterPublicKey.getAlgorithm(), HashAlgorithmTags.SHA1)
-                    .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME);
-            PGPSignatureGenerator sGen = new PGPSignatureGenerator(signerBuilder);
-
-            sGen.init(PGPSignature.POSITIVE_CERTIFICATION, masterPrivateKey);
-
-            PGPSignature certification = sGen.generateCertification(userId, masterPublicKey);
-            masterPublicKey = PGPPublicKey.addCertification(masterPublicKey, userId, certification);
-        }
-
-        PGPKeyPair masterKeyPair = new PGPKeyPair(masterPublicKey, masterPrivateKey);
-
-        PGPSignatureSubpacketGenerator hashedPacketsGen = new PGPSignatureSubpacketGenerator();
-        PGPSignatureSubpacketGenerator unhashedPacketsGen = new PGPSignatureSubpacketGenerator();
-
-        hashedPacketsGen.setKeyFlags(true, usageId);
-
-        hashedPacketsGen.setPreferredSymmetricAlgorithms(true, PREFERRED_SYMMETRIC_ALGORITHMS);
-        hashedPacketsGen.setPreferredHashAlgorithms(true, PREFERRED_HASH_ALGORITHMS);
-        hashedPacketsGen.setPreferredCompressionAlgorithms(true, PREFERRED_COMPRESSION_ALGORITHMS);
-
-        if (saveParcel.keysExpiryDates.get(0) != null) {
-            Calendar creationDate = Calendar.getInstance(TimeZone.getTimeZone("UTC"));
-            creationDate.setTime(masterPublicKey.getCreationTime());
-            Calendar expiryDate = saveParcel.keysExpiryDates.get(0);
-            //note that the below, (a/c) - (b/c) is *not* the same as (a - b) /c
-            //here we purposefully ignore partial days in each date - long type has no fractional part!
-            long numDays = (expiryDate.getTimeInMillis() / 86400000) -
-                    (creationDate.getTimeInMillis() / 86400000);
-            if (numDays <= 0) {
-                throw new PgpGeneralMsgIdException(R.string.error_expiry_must_come_after_creation);
+            if (keySize < 512) {
+                throw new PgpGeneralMsgIdException(R.string.error_key_size_minimum512bit);
             }
-            hashedPacketsGen.setKeyExpirationTime(false, numDays * 86400);
-        } else {
-            hashedPacketsGen.setKeyExpirationTime(false, 0);
-            // do this explicitly, although since we're rebuilding,
-            // this happens anyway
-        }
-
-        updateProgress(R.string.progress_building_master_key, 30, 100);
 
-        // define hashing and signing algos
-        PGPDigestCalculator sha1Calc = new JcaPGPDigestCalculatorProviderBuilder().build().get(
-                HashAlgorithmTags.SHA1);
-        PGPContentSignerBuilder certificationSignerBuilder = new JcaPGPContentSignerBuilder(
-                masterKeyPair.getPublicKey().getAlgorithm(), HashAlgorithmTags.SHA1);
+            if (passphrase == null) {
+                passphrase = "";
+            }
 
-        // Build key encrypter based on passphrase
-        PBESecretKeyEncryptor keyEncryptor = new JcePBESecretKeyEncryptorBuilder(
-                PGPEncryptedData.CAST5, sha1Calc)
-                .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME).build(
-                        saveParcel.newPassphrase.toCharArray());
+            int algorithm;
+            KeyPairGenerator keyGen;
 
-        PGPKeyRingGenerator keyGen = new PGPKeyRingGenerator(PGPSignature.POSITIVE_CERTIFICATION,
-                masterKeyPair, mainUserId, sha1Calc, hashedPacketsGen.generate(),
-                unhashedPacketsGen.generate(), certificationSignerBuilder, keyEncryptor);
+            switch (algorithmChoice) {
+                case Constants.choice.algorithm.dsa: {
+                    keyGen = KeyPairGenerator.getInstance("DSA", Constants.BOUNCY_CASTLE_PROVIDER_NAME);
+                    keyGen.initialize(keySize, new SecureRandom());
+                    algorithm = PGPPublicKey.DSA;
+                    break;
+                }
 
-        updateProgress(R.string.progress_adding_sub_keys, 40, 100);
+                case Constants.choice.algorithm.elgamal: {
+                    if (isMasterKey) {
+                        throw new PgpGeneralMsgIdException(R.string.error_master_key_must_not_be_el_gamal);
+                    }
+                    keyGen = KeyPairGenerator.getInstance("ElGamal", Constants.BOUNCY_CASTLE_PROVIDER_NAME);
+                    BigInteger p = Primes.getBestPrime(keySize);
+                    BigInteger g = new BigInteger("2");
 
-        for (int i = 1; i < saveParcel.keys.size(); ++i) {
-            updateProgress(40 + 40 * (i - 1) / (saveParcel.keys.size() - 1), 100);
+                    ElGamalParameterSpec elParams = new ElGamalParameterSpec(p, g);
 
-            PGPSecretKey subKey = saveParcel.keys.get(i).getSecretKeyExternal();
-            PGPPublicKey subPublicKey = subKey.getPublicKey();
+                    keyGen.initialize(elParams);
+                    algorithm = PGPPublicKey.ELGAMAL_ENCRYPT;
+                    break;
+                }
 
-            PBESecretKeyDecryptor keyDecryptor2 = new JcePBESecretKeyDecryptorBuilder()
-                    .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME).build(
-                            saveParcel.oldPassphrase.toCharArray());
-            PGPPrivateKey subPrivateKey = subKey.extractPrivateKey(keyDecryptor2);
+                case Constants.choice.algorithm.rsa: {
+                    keyGen = KeyPairGenerator.getInstance("RSA", Constants.BOUNCY_CASTLE_PROVIDER_NAME);
+                    keyGen.initialize(keySize, new SecureRandom());
 
-            // TODO: now used without algorithm and creation time?! (APG 1)
-            PGPKeyPair subKeyPair = new PGPKeyPair(subPublicKey, subPrivateKey);
+                    algorithm = PGPPublicKey.RSA_GENERAL;
+                    break;
+                }
 
-            hashedPacketsGen = new PGPSignatureSubpacketGenerator();
-            unhashedPacketsGen = new PGPSignatureSubpacketGenerator();
-
-            usageId = saveParcel.keysUsages.get(i);
-            canSign = (usageId & KeyFlags.SIGN_DATA) > 0; //todo - separate function for this
-            if (canSign) {
-                Date todayDate = new Date(); //both sig times the same
-                // cross-certify signing keys
-                hashedPacketsGen.setSignatureCreationTime(false, todayDate); //set outer creation time
-                PGPSignatureSubpacketGenerator subHashedPacketsGen = new PGPSignatureSubpacketGenerator();
-                subHashedPacketsGen.setSignatureCreationTime(false, todayDate); //set inner creation time
-                PGPContentSignerBuilder signerBuilder = new JcaPGPContentSignerBuilder(
-                        subPublicKey.getAlgorithm(), PGPUtil.SHA1)
-                        .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME);
-                PGPSignatureGenerator sGen = new PGPSignatureGenerator(signerBuilder);
-                sGen.init(PGPSignature.PRIMARYKEY_BINDING, subPrivateKey);
-                sGen.setHashedSubpackets(subHashedPacketsGen.generate());
-                PGPSignature certification = sGen.generateCertification(masterPublicKey,
-                        subPublicKey);
-                unhashedPacketsGen.setEmbeddedSignature(false, certification);
-            }
-            hashedPacketsGen.setKeyFlags(false, usageId);
-
-            if (saveParcel.keysExpiryDates.get(i) != null) {
-                Calendar creationDate = Calendar.getInstance(TimeZone.getTimeZone("UTC"));
-                creationDate.setTime(subPublicKey.getCreationTime());
-                Calendar expiryDate = saveParcel.keysExpiryDates.get(i);
-                //note that the below, (a/c) - (b/c) is *not* the same as (a - b) /c
-                //here we purposefully ignore partial days in each date - long type has no fractional part!
-                long numDays = (expiryDate.getTimeInMillis() / 86400000) -
-                        (creationDate.getTimeInMillis() / 86400000);
-                if (numDays <= 0) {
-                    throw new PgpGeneralMsgIdException(R.string.error_expiry_must_come_after_creation);
+                default: {
+                    throw new PgpGeneralMsgIdException(R.string.error_unknown_algorithm_choice);
                 }
-                hashedPacketsGen.setKeyExpirationTime(false, numDays * 86400);
-            } else {
-                hashedPacketsGen.setKeyExpirationTime(false, 0);
-                // do this explicitly, although since we're rebuilding,
-                // this happens anyway
             }
 
-            keyGen.addSubKey(subKeyPair, hashedPacketsGen.generate(), unhashedPacketsGen.generate());
-        }
+            // build new key pair
+            PGPKeyPair keyPair = new JcaPGPKeyPair(algorithm, keyGen.generateKeyPair(), new Date());
 
-        PGPSecretKeyRing secretKeyRing = keyGen.generateSecretKeyRing();
-        PGPPublicKeyRing publicKeyRing = keyGen.generatePublicKeyRing();
+            // define hashing and signing algos
+            PGPDigestCalculator sha1Calc = new JcaPGPDigestCalculatorProviderBuilder().build().get(
+                    HashAlgorithmTags.SHA1);
 
-        return new Pair(new UncachedKeyRing(secretKeyRing), new UncachedKeyRing(publicKeyRing));
+            // Build key encrypter and decrypter based on passphrase
+            PBESecretKeyEncryptor keyEncryptor = new JcePBESecretKeyEncryptorBuilder(
+                    PGPEncryptedData.CAST5, sha1Calc)
+                    .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME).build(passphrase.toCharArray());
 
+            return new PGPSecretKey(keyPair.getPrivateKey(), keyPair.getPublicKey(),
+                    sha1Calc, isMasterKey, keyEncryptor);
+        } catch(NoSuchProviderException e) {
+            throw new RuntimeException(e);
+        } catch(NoSuchAlgorithmException e) {
+            throw new RuntimeException(e);
+        } catch(InvalidAlgorithmParameterException e) {
+            throw new RuntimeException(e);
+        } catch(PGPException e) {
+            throw new PgpGeneralMsgIdException(R.string.msg_mf_error_pgp, e);
+        }
     }
 
-    public Pair<UncachedKeyRing, UncachedKeyRing> buildSecretKey(WrappedSecretKeyRing wmKR,
-                                                                 WrappedPublicKeyRing wpKR,
-                                                                 OldSaveKeyringParcel saveParcel)
-            throws PgpGeneralMsgIdException, PGPException, SignatureException, IOException {
+    /** This method introduces a list of modifications specified by a SaveKeyringParcel to a
+     * WrappedSecretKeyRing.
+     *
+     * This method relies on WrappedSecretKeyRing's canonicalization property!
+     *
+     * Note that PGPPublicKeyRings can not be directly modified. Instead, the corresponding
+     * PGPSecretKeyRing must be modified and consequently consolidated with its public counterpart.
+     * This is a natural workflow since pgp keyrings are immutable data structures: Old semantics
+     * are changed by adding new certificates, which implicitly override older certificates.
+     *
+     */
+    public UncachedKeyRing modifySecretKeyRing(WrappedSecretKeyRing wsKR, SaveKeyringParcel saveParcel,
+                                               String passphrase, OperationLog log, int indent) {
 
-        PGPSecretKeyRing mKR = wmKR.getRing();
-        PGPPublicKeyRing pKR = wpKR.getRing();
+        /*
+         * 1. Unlock private key
+         * 2a. Add certificates for new user ids
+         * 2b. Add revocations for revoked user ids
+         * 3. If primary user id changed, generate new certificates for both old and new
+         * 4a. For each subkey change, generate new subkey binding certificate
+         * 4b. For each subkey revocation, generate new subkey revocation certificate
+         * 5. Generate and add new subkeys
+         * 6. If requested, change passphrase
+         */
 
+        log.add(LogLevel.START, LogType.MSG_MF, indent);
+        indent += 1;
         updateProgress(R.string.progress_building_key, 0, 100);
 
-        if (saveParcel.oldPassphrase == null) {
-            saveParcel.oldPassphrase = "";
-        }
-        if (saveParcel.newPassphrase == null) {
-            saveParcel.newPassphrase = "";
-        }
+        // We work on bouncycastle object level here
+        PGPSecretKeyRing sKR = wsKR.getRing();
+        PGPPublicKey masterPublicKey = sKR.getPublicKey();
+        PGPSecretKey masterSecretKey = sKR.getSecretKey();
 
-        /*
-        IDs - NB This might not need to happen later, if we change the way the primary ID is chosen
-            remove deleted ids
-            if the primary ID changed we need to:
-                remove all of the IDs from the keyring, saving their certifications
-                add them all in again, updating certs of IDs which have changed
-            else
-                remove changed IDs and add in with new certs
-
-            if the master key changed, we need to remove the primary ID certification, so we can add
-            the new one when it is generated, and they don't conflict
-
-        Keys
-            remove deleted keys
-            if a key is modified, re-sign it
-                do we need to remove and add in?
-
-        Todo
-            identify more things which need to be preserved - e.g. trust levels?
-                    user attributes
-        */
-
-        if (saveParcel.deletedKeys != null) {
-            for (UncachedSecretKey dKey : saveParcel.deletedKeys) {
-                mKR = PGPSecretKeyRing.removeSecretKey(mKR, dKey.getSecretKeyExternal());
+        // 1. Unlock private key
+        log.add(LogLevel.DEBUG, LogType.MSG_MF_UNLOCK, indent);
+        PGPPrivateKey masterPrivateKey; {
+            try {
+                PBESecretKeyDecryptor keyDecryptor = new JcePBESecretKeyDecryptorBuilder().setProvider(
+                        Constants.BOUNCY_CASTLE_PROVIDER_NAME).build(passphrase.toCharArray());
+                masterPrivateKey = masterSecretKey.extractPrivateKey(keyDecryptor);
+            } catch (PGPException e) {
+                log.add(LogLevel.ERROR, LogType.MSG_MF_UNLOCK_ERROR, indent+1);
+                return null;
             }
         }
-
-        PGPSecretKey masterKey = mKR.getSecretKey();
-        PGPPublicKey masterPublicKey = masterKey.getPublicKey();
-
-        int usageId = saveParcel.keysUsages.get(0);
-        boolean canSign;
-        String mainUserId = saveParcel.userIds.get(0);
-
-        PBESecretKeyDecryptor keyDecryptor = new JcePBESecretKeyDecryptorBuilder().setProvider(
-                Constants.BOUNCY_CASTLE_PROVIDER_NAME).build(saveParcel.oldPassphrase.toCharArray());
-        PGPPrivateKey masterPrivateKey = masterKey.extractPrivateKey(keyDecryptor);
-
-        updateProgress(R.string.progress_certifying_master_key, 20, 100);
-
-        boolean anyIDChanged = false;
-        for (String delID : saveParcel.deletedIDs) {
-            anyIDChanged = true;
-            masterPublicKey = PGPPublicKey.removeCertification(masterPublicKey, delID);
+        if (!Arrays.equals(saveParcel.mFingerprint, sKR.getPublicKey().getFingerprint())) {
+            return null;
         }
 
-        int userIDIndex = 0;
-
-        PGPSignatureSubpacketGenerator hashedPacketsGen = new PGPSignatureSubpacketGenerator();
-        PGPSignatureSubpacketGenerator unhashedPacketsGen = new PGPSignatureSubpacketGenerator();
-
-        hashedPacketsGen.setKeyFlags(true, usageId);
-
-        hashedPacketsGen.setPreferredSymmetricAlgorithms(true, PREFERRED_SYMMETRIC_ALGORITHMS);
-        hashedPacketsGen.setPreferredHashAlgorithms(true, PREFERRED_HASH_ALGORITHMS);
-        hashedPacketsGen.setPreferredCompressionAlgorithms(true, PREFERRED_COMPRESSION_ALGORITHMS);
-
-        if (saveParcel.keysExpiryDates.get(0) != null) {
-            Calendar creationDate = Calendar.getInstance(TimeZone.getTimeZone("UTC"));
-            creationDate.setTime(masterPublicKey.getCreationTime());
-            Calendar expiryDate = saveParcel.keysExpiryDates.get(0);
-            //note that the below, (a/c) - (b/c) is *not* the same as (a - b) /c
-            //here we purposefully ignore partial days in each date - long type has no fractional part!
-            long numDays = (expiryDate.getTimeInMillis() / 86400000) -
-                    (creationDate.getTimeInMillis() / 86400000);
-            if (numDays <= 0) {
-                throw new PgpGeneralMsgIdException(R.string.error_expiry_must_come_after_creation);
-            }
-            hashedPacketsGen.setKeyExpirationTime(false, numDays * 86400);
-        } else {
-            hashedPacketsGen.setKeyExpirationTime(false, 0);
-            // do this explicitly, although since we're rebuilding,
-            // this happens anyway
-        }
-
-        if (saveParcel.primaryIDChanged ||
-                !saveParcel.originalIDs.get(0).equals(saveParcel.userIds.get(0))) {
-            anyIDChanged = true;
-            ArrayList<Pair<String, PGPSignature>> sigList = new ArrayList<Pair<String, PGPSignature>>();
-            for (String userId : saveParcel.userIds) {
-                String origID = saveParcel.originalIDs.get(userIDIndex);
-                if (origID.equals(userId) && !saveParcel.newIDs[userIDIndex] &&
-                        !userId.equals(saveParcel.originalPrimaryID) && userIDIndex != 0) {
-                    Iterator<PGPSignature> origSigs = masterPublicKey.getSignaturesForID(origID);
-                    // TODO: make sure this iterator only has signatures we are interested in
-                    while (origSigs.hasNext()) {
-                        PGPSignature origSig = origSigs.next();
-                        sigList.add(new Pair<String, PGPSignature>(origID, origSig));
-                    }
-                } else {
-                    PGPContentSignerBuilder signerBuilder = new JcaPGPContentSignerBuilder(
-                            masterPublicKey.getAlgorithm(), HashAlgorithmTags.SHA1)
-                            .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME);
-                    PGPSignatureGenerator sGen = new PGPSignatureGenerator(signerBuilder);
-
-                    sGen.init(PGPSignature.POSITIVE_CERTIFICATION, masterPrivateKey);
-                    if (userIDIndex == 0) {
-                        sGen.setHashedSubpackets(hashedPacketsGen.generate());
-                        sGen.setUnhashedSubpackets(unhashedPacketsGen.generate());
-                    }
-                    PGPSignature certification = sGen.generateCertification(userId, masterPublicKey);
-                    sigList.add(new Pair<String, PGPSignature>(userId, certification));
-                }
-                if (!saveParcel.newIDs[userIDIndex]) {
-                    masterPublicKey = PGPPublicKey.removeCertification(masterPublicKey, origID);
-                }
-                userIDIndex++;
-            }
-            for (Pair<String, PGPSignature> toAdd : sigList) {
-                masterPublicKey =
-                        PGPPublicKey.addCertification(masterPublicKey, toAdd.first, toAdd.second);
-            }
-        } else {
-            for (String userId : saveParcel.userIds) {
-                String origID = saveParcel.originalIDs.get(userIDIndex);
-                if (!origID.equals(userId) || saveParcel.newIDs[userIDIndex]) {
-                    anyIDChanged = true;
-                    PGPContentSignerBuilder signerBuilder = new JcaPGPContentSignerBuilder(
-                            masterPublicKey.getAlgorithm(), HashAlgorithmTags.SHA1)
-                            .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME);
-                    PGPSignatureGenerator sGen = new PGPSignatureGenerator(signerBuilder);
-
-                    sGen.init(PGPSignature.POSITIVE_CERTIFICATION, masterPrivateKey);
-                    if (userIDIndex == 0) {
-                        sGen.setHashedSubpackets(hashedPacketsGen.generate());
-                        sGen.setUnhashedSubpackets(unhashedPacketsGen.generate());
-                    }
-                    PGPSignature certification = sGen.generateCertification(userId, masterPublicKey);
-                    if (!saveParcel.newIDs[userIDIndex]) {
-                        masterPublicKey = PGPPublicKey.removeCertification(masterPublicKey, origID);
-                    }
-                    masterPublicKey =
-                            PGPPublicKey.addCertification(masterPublicKey, userId, certification);
-                }
-                userIDIndex++;
-            }
-        }
-
-        ArrayList<Pair<String, PGPSignature>> sigList = new ArrayList<Pair<String, PGPSignature>>();
-        if (saveParcel.moddedKeys[0]) {
-            userIDIndex = 0;
-            for (String userId : saveParcel.userIds) {
-                String origID = saveParcel.originalIDs.get(userIDIndex);
-                if (!(origID.equals(saveParcel.originalPrimaryID) && !saveParcel.primaryIDChanged)) {
-                    Iterator<PGPSignature> sigs = masterPublicKey.getSignaturesForID(userId);
-                    // TODO: make sure this iterator only has signatures we are interested in
-                    while (sigs.hasNext()) {
-                        PGPSignature sig = sigs.next();
-                        sigList.add(new Pair<String, PGPSignature>(userId, sig));
-                    }
-                }
-                masterPublicKey = PGPPublicKey.removeCertification(masterPublicKey, userId);
-                userIDIndex++;
-            }
-            anyIDChanged = true;
-        }
+        updateProgress(R.string.progress_certifying_master_key, 20, 100);
 
-        //update the keyring with the new ID information
-        if (anyIDChanged) {
-            pKR = PGPPublicKeyRing.insertPublicKey(pKR, masterPublicKey);
-            mKR = PGPSecretKeyRing.replacePublicKeys(mKR, pKR);
-        }
+        // work on master secret key
+        try {
 
-        PGPKeyPair masterKeyPair = new PGPKeyPair(masterPublicKey, masterPrivateKey);
-
-        updateProgress(R.string.progress_building_master_key, 30, 100);
-
-        // define hashing and signing algos
-        PGPDigestCalculator sha1Calc = new JcaPGPDigestCalculatorProviderBuilder().build().get(
-                HashAlgorithmTags.SHA1);
-        PGPContentSignerBuilder certificationSignerBuilder = new JcaPGPContentSignerBuilder(
-                masterKeyPair.getPublicKey().getAlgorithm(), HashAlgorithmTags.SHA1);
-
-        // Build key encryptor based on old passphrase, as some keys may be unchanged
-        PBESecretKeyEncryptor keyEncryptor = new JcePBESecretKeyEncryptorBuilder(
-                PGPEncryptedData.CAST5, sha1Calc)
-                .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME).build(
-                        saveParcel.oldPassphrase.toCharArray());
-
-        //this generates one more signature than necessary...
-        PGPKeyRingGenerator keyGen = new PGPKeyRingGenerator(PGPSignature.POSITIVE_CERTIFICATION,
-                masterKeyPair, mainUserId, sha1Calc, hashedPacketsGen.generate(),
-                unhashedPacketsGen.generate(), certificationSignerBuilder, keyEncryptor);
-
-        for (int i = 1; i < saveParcel.keys.size(); ++i) {
-            updateProgress(40 + 50 * i / saveParcel.keys.size(), 100);
-            if (saveParcel.moddedKeys[i]) {
-                PGPSecretKey subKey = saveParcel.keys.get(i).getSecretKeyExternal();
-                PGPPublicKey subPublicKey = subKey.getPublicKey();
-
-                PBESecretKeyDecryptor keyDecryptor2;
-                if (saveParcel.newKeys[i]) {
-                    keyDecryptor2 = new JcePBESecretKeyDecryptorBuilder()
-                            .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME).build(
-                                    "".toCharArray());
-                } else {
-                    keyDecryptor2 = new JcePBESecretKeyDecryptorBuilder()
-                            .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME).build(
-                                    saveParcel.oldPassphrase.toCharArray());
-                }
-                PGPPrivateKey subPrivateKey = subKey.extractPrivateKey(keyDecryptor2);
-                PGPKeyPair subKeyPair = new PGPKeyPair(subPublicKey, subPrivateKey);
-
-                hashedPacketsGen = new PGPSignatureSubpacketGenerator();
-                unhashedPacketsGen = new PGPSignatureSubpacketGenerator();
-
-                usageId = saveParcel.keysUsages.get(i);
-                canSign = (usageId & KeyFlags.SIGN_DATA) > 0; //todo - separate function for this
-                if (canSign) {
-                    Date todayDate = new Date(); //both sig times the same
-                    // cross-certify signing keys
-                    hashedPacketsGen.setSignatureCreationTime(false, todayDate); //set outer creation time
-                    PGPSignatureSubpacketGenerator subHashedPacketsGen = new PGPSignatureSubpacketGenerator();
-                    subHashedPacketsGen.setSignatureCreationTime(false, todayDate); //set inner creation time
-                    PGPContentSignerBuilder signerBuilder = new JcaPGPContentSignerBuilder(
-                            subPublicKey.getAlgorithm(), PGPUtil.SHA1)
-                            .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME);
-                    PGPSignatureGenerator sGen = new PGPSignatureGenerator(signerBuilder);
-                    sGen.init(PGPSignature.PRIMARYKEY_BINDING, subPrivateKey);
-                    sGen.setHashedSubpackets(subHashedPacketsGen.generate());
-                    PGPSignature certification = sGen.generateCertification(masterPublicKey,
-                            subPublicKey);
-                    unhashedPacketsGen.setEmbeddedSignature(false, certification);
-                }
-                hashedPacketsGen.setKeyFlags(false, usageId);
-
-                if (saveParcel.keysExpiryDates.get(i) != null) {
-                    Calendar creationDate = Calendar.getInstance(TimeZone.getTimeZone("UTC"));
-                    creationDate.setTime(subPublicKey.getCreationTime());
-                    Calendar expiryDate = saveParcel.keysExpiryDates.get(i);
-                    // note that the below, (a/c) - (b/c) is *not* the same as (a - b) /c
-                    // here we purposefully ignore partial days in each date - long type has
-                    // no fractional part!
-                    long numDays = (expiryDate.getTimeInMillis() / 86400000) -
-                            (creationDate.getTimeInMillis() / 86400000);
-                    if (numDays <= 0) {
-                        throw new PgpGeneralMsgIdException(R.string.error_expiry_must_come_after_creation);
-                    }
-                    hashedPacketsGen.setKeyExpirationTime(false, numDays * 86400);
-                } else {
-                    hashedPacketsGen.setKeyExpirationTime(false, 0);
-                    // do this explicitly, although since we're rebuilding,
-                    // this happens anyway
-                }
+            PGPPublicKey modifiedPublicKey = masterPublicKey;
 
-                keyGen.addSubKey(subKeyPair, hashedPacketsGen.generate(), unhashedPacketsGen.generate());
-                // certifications will be discarded if the key is changed, because I think, for a start,
-                // they will be invalid. Binding certs are regenerated anyway, and other certs which
-                // need to be kept are on IDs and attributes
-                // TODO: don't let revoked keys be edited, other than removed - changing one would
-                // result in the revocation being wrong?
+            // 2a. Add certificates for new user ids
+            for (String userId : saveParcel.addUserIds) {
+                log.add(LogLevel.INFO, LogType.MSG_MF_UID_ADD, indent);
+                PGPSignature cert = generateUserIdSignature(masterPrivateKey,
+                        masterPublicKey, userId, false);
+                modifiedPublicKey = PGPPublicKey.addCertification(masterPublicKey, userId, cert);
             }
-        }
 
-        PGPSecretKeyRing updatedSecretKeyRing = keyGen.generateSecretKeyRing();
-        //finally, update the keyrings
-        Iterator<PGPSecretKey> itr = updatedSecretKeyRing.getSecretKeys();
-        while (itr.hasNext()) {
-            PGPSecretKey theNextKey = itr.next();
-            if ((theNextKey.isMasterKey() && saveParcel.moddedKeys[0]) || !theNextKey.isMasterKey()) {
-                mKR = PGPSecretKeyRing.insertSecretKey(mKR, theNextKey);
-                pKR = PGPPublicKeyRing.insertPublicKey(pKR, theNextKey.getPublicKey());
+            // 2b. Add revocations for revoked user ids
+            for (String userId : saveParcel.revokeUserIds) {
+                log.add(LogLevel.INFO, LogType.MSG_MF_UID_REVOKE, indent);
+                PGPSignature cert = generateRevocationSignature(masterPrivateKey,
+                        masterPublicKey, userId);
+                modifiedPublicKey = PGPPublicKey.addCertification(masterPublicKey, userId, cert);
             }
-        }
 
-        //replace lost IDs
-        if (saveParcel.moddedKeys[0]) {
-            masterPublicKey = mKR.getPublicKey();
-            for (Pair<String, PGPSignature> toAdd : sigList) {
-                masterPublicKey = PGPPublicKey.addCertification(masterPublicKey, toAdd.first, toAdd.second);
+            // 3. If primary user id changed, generate new certificates for both old and new
+            if (saveParcel.changePrimaryUserId != null) {
+                log.add(LogLevel.INFO, LogType.MSG_MF_UID_PRIMARY, indent);
+                // todo
             }
-            pKR = PGPPublicKeyRing.insertPublicKey(pKR, masterPublicKey);
-            mKR = PGPSecretKeyRing.replacePublicKeys(mKR, pKR);
-        }
-
-        // Build key encryptor based on new passphrase
-        PBESecretKeyEncryptor keyEncryptorNew = new JcePBESecretKeyEncryptorBuilder(
-                PGPEncryptedData.CAST5, sha1Calc)
-                .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME).build(
-                        saveParcel.newPassphrase.toCharArray());
-
-        //update the passphrase
-        mKR = PGPSecretKeyRing.copyWithNewPassword(mKR, keyDecryptor, keyEncryptorNew);
-
-        /* 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);
+            // Update the secret key ring
+            if (modifiedPublicKey != masterPublicKey) {
+                masterSecretKey = PGPSecretKey.replacePublicKey(masterSecretKey, modifiedPublicKey);
+                masterPublicKey = modifiedPublicKey;
+                sKR = PGPSecretKeyRing.insertSecretKey(sKR, masterSecretKey);
             }
-        }
-
-        */
-
-        return new Pair<UncachedKeyRing,UncachedKeyRing>(new UncachedKeyRing(pKR),
-                                                         new UncachedKeyRing(mKR));
-
-    }
-
-    public Pair<PGPSecretKeyRing, PGPPublicKeyRing> buildSecretKey(PGPSecretKeyRing sKR,
-                                                                   PGPPublicKeyRing pKR,
-                                                                   SaveKeyringParcel saveParcel,
-                                                                   String passphrase)
-            throws PgpGeneralMsgIdException, PGPException, SignatureException, IOException {
-
-        updateProgress(R.string.progress_building_key, 0, 100);
-
-        // sort these, so we can use binarySearch later on
-        Arrays.sort(saveParcel.revokeSubKeys);
-        Arrays.sort(saveParcel.revokeUserIds);
-
-        /*
-         * What's gonna happen here:
-         *
-         * 1. Unlock private key
-         *
-         * 2. Create new secret key ring
-         *
-         * 3. Copy subkeys
-         *  - Generate revocation if requested
-         *  - Copy old cert, or generate new if change requested
-         *
-         * 4. Generate and add new subkeys
-         *
-         * 5. Copy user ids
-         *  - Generate revocation if requested
-         *  - Copy old cert, or generate new if primary user id status changed
-         *
-         * 6. Add new user ids
-         *
-         * 7. Generate PublicKeyRing from SecretKeyRing
-         *
-         * 8. Return pair (PublicKeyRing,SecretKeyRing)
-         *
-         */
-
-        // 1. Unlock private key
-        updateProgress(R.string.progress_building_key, 0, 100);
 
-        PGPPublicKey masterPublicKey = sKR.getPublicKey();
-        PGPPrivateKey masterPrivateKey; {
-            PGPSecretKey masterKey = sKR.getSecretKey();
-            PBESecretKeyDecryptor keyDecryptor = new JcePBESecretKeyDecryptorBuilder().setProvider(
-                    Constants.BOUNCY_CASTLE_PROVIDER_NAME).build(passphrase.toCharArray());
-            masterPrivateKey = masterKey.extractPrivateKey(keyDecryptor);
-        }
 
-        // 2. Create new secret key ring
-        updateProgress(R.string.progress_certifying_master_key, 20, 100);
-
-        // Note we do NOT use PGPKeyRingGeneraor, it's just one level too high and does stuff
-        // we want to do manually. Instead, we simply use a list of secret keys.
-        ArrayList<PGPSecretKey> secretKeys = new ArrayList<PGPSecretKey>();
-        ArrayList<PGPPublicKey> publicKeys = new ArrayList<PGPPublicKey>();
-
-        // 3. Copy subkeys
-        // - Generate revocation if requested
-        // - Copy old cert, or generate new if change requested
-        for (PGPSecretKey sKey : new IterableIterator<PGPSecretKey>(sKR.getSecretKeys())) {
-            PGPPublicKey pKey = sKey.getPublicKey();
-            if (Arrays.binarySearch(saveParcel.revokeSubKeys, sKey.getKeyID()) >= 0) {
-                // add revocation signature to key, if there is none yet
-                if (!pKey.getSignaturesOfType(PGPSignature.SUBKEY_REVOCATION).hasNext()) {
-                    // generate revocation signature
+        // 4a. For each subkey change, generate new subkey binding certificate
+            for (SaveKeyringParcel.SubkeyChange change : saveParcel.changeSubKeys) {
+                log.add(LogLevel.INFO, LogType.MSG_MF_SUBKEY_CHANGE,
+                        new String[]{PgpKeyHelper.convertKeyIdToHex(change.mKeyId)}, indent);
+                PGPSecretKey sKey = sKR.getSecretKey(change.mKeyId);
+                if (sKey == null) {
+                    log.add(LogLevel.ERROR, LogType.MSG_MF_SUBKEY_MISSING,
+                            new String[]{PgpKeyHelper.convertKeyIdToHex(change.mKeyId)}, indent + 1);
+                    return null;
                 }
-            }
-            if (saveParcel.changeSubKeys.containsKey(sKey.getKeyID())) {
-                // change subkey flags?
-                SaveKeyringParcel.SubkeyChange change = saveParcel.changeSubKeys.get(sKey.getKeyID());
-                // remove old subkey binding signature(s?)
-                for (PGPSignature sig : new IterableIterator<PGPSignature>(
-                        pKey.getSignaturesOfType(PGPSignature.SUBKEY_BINDING))) {
-                    pKey = PGPPublicKey.removeCertification(pKey, sig);
+                PGPPublicKey pKey = sKey.getPublicKey();
+
+                if (change.mExpiry != null && new Date(change.mExpiry).before(new Date())) {
+                    log.add(LogLevel.ERROR, LogType.MSG_MF_SUBKEY_PAST_EXPIRY,
+                            new String[]{PgpKeyHelper.convertKeyIdToHex(change.mKeyId)}, indent + 1);
+                    return null;
                 }
 
                 // generate and add new signature
                 PGPSignature sig = generateSubkeyBindingSignature(masterPublicKey, masterPrivateKey,
                         sKey, pKey, change.mFlags, change.mExpiry, passphrase);
                 pKey = PGPPublicKey.addCertification(pKey, sig);
+                sKR = PGPSecretKeyRing.insertSecretKey(sKR, PGPSecretKey.replacePublicKey(sKey, pKey));
             }
-            secretKeys.add(PGPSecretKey.replacePublicKey(sKey, pKey));
-            publicKeys.add(pKey);
-        }
 
-        // 4. Generate and add new subkeys
-        // TODO
-
-        // 5. Copy user ids
-        for (String userId : new IterableIterator<String>(masterPublicKey.getUserIDs())) {
-            // - Copy old cert, or generate new if primary user id status changed
-            boolean certified = false, revoked = false;
-            for (PGPSignature sig : new IterableIterator<PGPSignature>(
-                    masterPublicKey.getSignaturesForID(userId))) {
-                // We know there are only revocation and certification types in here.
-                switch(sig.getSignatureType()) {
-                    case PGPSignature.CERTIFICATION_REVOCATION:
-                        revoked = true;
-                        continue;
-
-                    case PGPSignature.DEFAULT_CERTIFICATION:
-                    case PGPSignature.NO_CERTIFICATION:
-                    case PGPSignature.CASUAL_CERTIFICATION:
-                    case PGPSignature.POSITIVE_CERTIFICATION:
-                        // Already got one? Remove this one, then.
-                        if (certified) {
-                            masterPublicKey = PGPPublicKey.removeCertification(
-                                    masterPublicKey, userId, sig);
-                            continue;
-                        }
-                        boolean primary = userId.equals(saveParcel.changePrimaryUserId);
-                        // Generate a new one under certain circumstances
-                        if (saveParcel.changePrimaryUserId != null &&
-                                sig.getHashedSubPackets().isPrimaryUserID() != primary) {
-                            PGPSignature cert = generateUserIdSignature(
-                                    masterPrivateKey, masterPublicKey, userId, primary);
-                            PGPPublicKey.addCertification(masterPublicKey, userId, cert);
-                        }
-                        certified = true;
+            // 4b. For each subkey revocation, generate new subkey revocation certificate
+            for (long revocation : saveParcel.revokeSubKeys) {
+                log.add(LogLevel.INFO, LogType.MSG_MF_SUBKEY_REVOKE,
+                        new String[] { PgpKeyHelper.convertKeyIdToHex(revocation) }, indent);
+                PGPSecretKey sKey = sKR.getSecretKey(revocation);
+                if (sKey == null) {
+                    log.add(LogLevel.ERROR, LogType.MSG_MF_SUBKEY_MISSING,
+                            new String[] { PgpKeyHelper.convertKeyIdToHex(revocation) }, indent+1);
+                    return null;
                 }
+                PGPPublicKey pKey = sKey.getPublicKey();
+
+                // generate and add new signature
+                PGPSignature sig = generateRevocationSignature(masterPublicKey, masterPrivateKey, pKey);
+
+                pKey = PGPPublicKey.addCertification(pKey, sig);
+                sKR = PGPSecretKeyRing.insertSecretKey(sKR, PGPSecretKey.replacePublicKey(sKey, pKey));
             }
-            // - Generate revocation if requested
-            if (!revoked && Arrays.binarySearch(saveParcel.revokeUserIds, userId) >= 0) {
-                PGPSignature cert = generateRevocationSignature(masterPrivateKey,
-                        masterPublicKey, userId);
-                masterPublicKey = PGPPublicKey.addCertification(masterPublicKey, userId, cert);
-            }
-        }
 
-        // 6. Add new user ids
-        for(String userId : saveParcel.addUserIds) {
-            PGPSignature cert = generateUserIdSignature(masterPrivateKey,
-                    masterPublicKey, userId, userId.equals(saveParcel.changePrimaryUserId));
-            masterPublicKey = PGPPublicKey.addCertification(masterPublicKey, userId, cert);
-        }
+            // 5. Generate and add new subkeys
+            for (SaveKeyringParcel.SubkeyAdd add : saveParcel.addSubKeys) {
+                try {
 
-        // 7. Generate PublicKeyRing from SecretKeyRing
-        updateProgress(R.string.progress_building_master_key, 30, 100);
-        PGPSecretKeyRing ring = new PGPSecretKeyRing(secretKeys);
+                    if (add.mExpiry != null && new Date(add.mExpiry).before(new Date())) {
+                        log.add(LogLevel.ERROR, LogType.MSG_MF_SUBKEY_PAST_EXPIRY, indent +1);
+                        return null;
+                    }
 
-        // Copy all non-self uid certificates
-        for (String userId : new IterableIterator<String>(masterPublicKey.getUserIDs())) {
-            // - Copy old cert, or generate new if primary user id status changed
-            boolean certified = false, revoked = false;
-            for (PGPSignature sig : new IterableIterator<PGPSignature>(
-                    masterPublicKey.getSignaturesForID(userId))) {
+                    log.add(LogLevel.INFO, LogType.MSG_MF_SUBKEY_NEW, indent);
+                    PGPSecretKey sKey = createKey(add.mAlgorithm, add.mKeysize, passphrase, false);
+                    log.add(LogLevel.DEBUG, LogType.MSG_MF_SUBKEY_NEW_ID,
+                            new String[] { PgpKeyHelper.convertKeyIdToHex(sKey.getKeyID()) }, indent+1);
+
+                    PGPPublicKey pKey = sKey.getPublicKey();
+                    PGPSignature cert = generateSubkeyBindingSignature(masterPublicKey, masterPrivateKey,
+                            sKey, pKey, add.mFlags, add.mExpiry, passphrase);
+                    pKey = PGPPublicKey.addCertification(pKey, cert);
+                    sKey = PGPSecretKey.replacePublicKey(sKey, pKey);
+                    sKR = PGPSecretKeyRing.insertSecretKey(sKR, PGPSecretKey.replacePublicKey(sKey, pKey));
+                } catch (PgpGeneralMsgIdException e) {
+                    return null;
+                }
             }
-        }
 
-        for (PGPPublicKey newKey : publicKeys) {
-            PGPPublicKey oldKey = pKR.getPublicKey(newKey.getKeyID());
-            for (PGPSignature sig : new IterableIterator<PGPSignature>(
-                    oldKey.getSignatures())) {
+            // 6. If requested, change passphrase
+            if (saveParcel.newPassphrase != null) {
+                log.add(LogLevel.INFO, LogType.MSG_MF_PASSPHRASE, indent);
+                PGPDigestCalculator sha1Calc = new JcaPGPDigestCalculatorProviderBuilder().build()
+                        .get(HashAlgorithmTags.SHA1);
+                PBESecretKeyDecryptor keyDecryptor = new JcePBESecretKeyDecryptorBuilder().setProvider(
+                        Constants.BOUNCY_CASTLE_PROVIDER_NAME).build(passphrase.toCharArray());
+                // Build key encryptor based on new passphrase
+                PBESecretKeyEncryptor keyEncryptorNew = new JcePBESecretKeyEncryptorBuilder(
+                        PGPEncryptedData.CAST5, sha1Calc)
+                        .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME).build(
+                                saveParcel.newPassphrase.toCharArray());
+
+                sKR = PGPSecretKeyRing.copyWithNewPassword(sKR, keyDecryptor, keyEncryptorNew);
             }
-        }
-
-        // If requested, set new passphrase
-        if (saveParcel.newPassphrase != null) {
-            PGPDigestCalculator sha1Calc = new JcaPGPDigestCalculatorProviderBuilder().build()
-                    .get(HashAlgorithmTags.SHA1);
-            PBESecretKeyDecryptor keyDecryptor = new JcePBESecretKeyDecryptorBuilder().setProvider(
-                    Constants.BOUNCY_CASTLE_PROVIDER_NAME).build(passphrase.toCharArray());
-            // Build key encryptor based on new passphrase
-            PBESecretKeyEncryptor keyEncryptorNew = new JcePBESecretKeyEncryptorBuilder(
-                    PGPEncryptedData.CAST5, sha1Calc)
-                    .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME).build(
-                            saveParcel.newPassphrase.toCharArray());
 
-            sKR = PGPSecretKeyRing.copyWithNewPassword(sKR, keyDecryptor, keyEncryptorNew);
+            // This one must only be thrown by
+        } catch (IOException e) {
+            log.add(LogLevel.ERROR, LogType.MSG_MF_ERROR_ENCODE, indent+1);
+            return null;
+        } catch (PGPException e) {
+            log.add(LogLevel.ERROR, LogType.MSG_MF_ERROR_PGP, indent+1);
+            return null;
+        } catch (SignatureException e) {
+            log.add(LogLevel.ERROR, LogType.MSG_MF_ERROR_SIG, indent+1);
+            return null;
         }
 
-        // 8. Return pair (PublicKeyRing,SecretKeyRing)
-
-        return new Pair<PGPSecretKeyRing, PGPPublicKeyRing>(sKR, pKR);
+        log.add(LogLevel.OK, LogType.MSG_MF_SUCCESS, indent);
+        return new UncachedKeyRing(sKR);
 
     }
 
@@ -883,11 +399,30 @@ public class PgpKeyOperation {
         return sGen.generateCertification(userId, pKey);
     }
 
+    private static PGPSignature generateRevocationSignature(
+            PGPPublicKey masterPublicKey, PGPPrivateKey masterPrivateKey, PGPPublicKey pKey)
+            throws IOException, PGPException, SignatureException {
+        PGPContentSignerBuilder signerBuilder = new JcaPGPContentSignerBuilder(
+                pKey.getAlgorithm(), PGPUtil.SHA1)
+                .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME);
+        PGPSignatureGenerator sGen = new PGPSignatureGenerator(signerBuilder);
+        PGPSignatureSubpacketGenerator subHashedPacketsGen = new PGPSignatureSubpacketGenerator();
+        subHashedPacketsGen.setSignatureCreationTime(false, new Date());
+        sGen.setHashedSubpackets(subHashedPacketsGen.generate());
+        // Generate key revocation or subkey revocation, depending on master/subkey-ness
+        if (masterPublicKey.getKeyID() == pKey.getKeyID()) {
+            sGen.init(PGPSignature.KEY_REVOCATION, masterPrivateKey);
+            return sGen.generateCertification(masterPublicKey);
+        } else {
+            sGen.init(PGPSignature.SUBKEY_REVOCATION, masterPrivateKey);
+            return sGen.generateCertification(masterPublicKey, pKey);
+        }
+    }
+
     private static PGPSignature generateSubkeyBindingSignature(
             PGPPublicKey masterPublicKey, PGPPrivateKey masterPrivateKey,
-            PGPSecretKey sKey, PGPPublicKey pKey,
-            int flags, Long expiry, String passphrase)
-            throws PgpGeneralMsgIdException, IOException, PGPException, SignatureException {
+            PGPSecretKey sKey, PGPPublicKey pKey, int flags, Long expiry, String passphrase)
+            throws IOException, PGPException, SignatureException {
 
         // date for signing
         Date todayDate = new Date();
@@ -924,13 +459,10 @@ public class PgpKeyOperation {
         if (expiry != null) {
             Calendar creationDate = Calendar.getInstance(TimeZone.getTimeZone("UTC"));
             creationDate.setTime(pKey.getCreationTime());
-            // note that the below, (a/c) - (b/c) is *not* the same as (a - b) /c
-            // here we purposefully ignore partial days in each date - long type has
-            // no fractional part!
-            long numDays = (expiry / 86400000) -
-                    (creationDate.getTimeInMillis() / 86400000);
-            if (numDays <= 0) {
-                throw new PgpGeneralMsgIdException(R.string.error_expiry_must_come_after_creation);
+
+            // (Just making sure there's no programming error here, this MUST have been checked above!)
+            if (new Date(expiry).before(todayDate)) {
+                throw new RuntimeException("Bad subkey creation date, this is a bug!");
             }
             hashedPacketsGen.setKeyExpirationTime(false, expiry - creationDate.getTimeInMillis());
         } else {
@@ -1003,19 +535,4 @@ public class PgpKeyOperation {
         return publicKey;
     }
 
-    /**
-     * Simple static subclass that stores two values.
-     * <p/>
-     * This is only used to return a pair of values in one function above. We specifically don't use
-     * com.android.Pair to keep this class free from android dependencies.
-     */
-    public static class Pair<K, V> {
-        public final K first;
-        public final V second;
-
-        public Pair(K first, V second) {
-            this.first = first;
-            this.second = second;
-        }
-    }
 }
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/exception/PgpGeneralMsgIdException.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/exception/PgpGeneralMsgIdException.java
index 3b88897ed..3700b4c34 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/exception/PgpGeneralMsgIdException.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/exception/PgpGeneralMsgIdException.java
@@ -30,6 +30,11 @@ public class PgpGeneralMsgIdException extends Exception {
         mMessageId = messageId;
     }
 
+    public PgpGeneralMsgIdException(int messageId, Throwable cause) {
+        super("msg[" + messageId + "]", cause);
+        mMessageId = messageId;
+    }
+
     public PgpGeneralException getContextualized(Context context) {
         return new PgpGeneralException(context.getString(mMessageId), this);
     }