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

9 files changed, 315 insertions, 991 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);
     }
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java
index 5358f36e8..25a9387f4 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java
@@ -44,7 +44,6 @@ import org.sufficientlysecure.keychain.pgp.PgpKeyOperation;
 import org.sufficientlysecure.keychain.pgp.PgpSignEncrypt;
 import org.sufficientlysecure.keychain.pgp.Progressable;
 import org.sufficientlysecure.keychain.pgp.UncachedKeyRing;
-import org.sufficientlysecure.keychain.pgp.UncachedSecretKey;
 import org.sufficientlysecure.keychain.pgp.WrappedPublicKeyRing;
 import org.sufficientlysecure.keychain.pgp.WrappedSecretKey;
 import org.sufficientlysecure.keychain.pgp.WrappedSecretKeyRing;
@@ -53,6 +52,7 @@ import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralMsgIdException;
 import org.sufficientlysecure.keychain.provider.KeychainContract.KeyRings;
 import org.sufficientlysecure.keychain.provider.KeychainDatabase;
 import org.sufficientlysecure.keychain.provider.ProviderHelper;
+import org.sufficientlysecure.keychain.service.OperationResultParcel.OperationLog;
 import org.sufficientlysecure.keychain.util.InputData;
 import org.sufficientlysecure.keychain.util.Log;
 import org.sufficientlysecure.keychain.util.ProgressScaler;
@@ -87,9 +87,6 @@ public class KeychainIntentService extends IntentService
     public static final String ACTION_DECRYPT_VERIFY = Constants.INTENT_PREFIX + "DECRYPT_VERIFY";
 
     public static final String ACTION_SAVE_KEYRING = Constants.INTENT_PREFIX + "SAVE_KEYRING";
-    public static final String ACTION_GENERATE_KEY = Constants.INTENT_PREFIX + "GENERATE_KEY";
-    public static final String ACTION_GENERATE_DEFAULT_RSA_KEYS = Constants.INTENT_PREFIX
-            + "GENERATE_DEFAULT_RSA_KEYS";
 
     public static final String ACTION_DELETE_FILE_SECURELY = Constants.INTENT_PREFIX
             + "DELETE_FILE_SECURELY";
@@ -127,14 +124,7 @@ public class KeychainIntentService extends IntentService
 
     // save keyring
     public static final String SAVE_KEYRING_PARCEL = "save_parcel";
-    public static final String SAVE_KEYRING_CAN_SIGN = "can_sign";
-
-
-    // generate key
-    public static final String GENERATE_KEY_ALGORITHM = "algorithm";
-    public static final String GENERATE_KEY_KEY_SIZE = "key_size";
-    public static final String GENERATE_KEY_SYMMETRIC_PASSPHRASE = "passphrase";
-    public static final String GENERATE_KEY_MASTER_KEY = "master_key";
+    public static final String SAVE_KEYRING_PASSPHRASE = "passphrase";
 
     // delete file securely
     public static final String DELETE_FILE = "deleteFile";
@@ -164,9 +154,6 @@ public class KeychainIntentService extends IntentService
     /*
      * possible data keys as result send over messenger
      */
-    // keys
-    public static final String RESULT_NEW_KEY = "new_key";
-    public static final String RESULT_KEY_USAGES = "new_key_usages";
 
     // encrypt
     public static final String RESULT_BYTES = "encrypted_data";
@@ -490,133 +477,37 @@ public class KeychainIntentService extends IntentService
         } else if (ACTION_SAVE_KEYRING.equals(action)) {
             try {
                 /* Input */
-                OldSaveKeyringParcel saveParcel = data.getParcelable(SAVE_KEYRING_PARCEL);
-                String oldPassphrase = saveParcel.oldPassphrase;
-                String newPassphrase = saveParcel.newPassphrase;
-                boolean canSign = true;
-
-                if (data.containsKey(SAVE_KEYRING_CAN_SIGN)) {
-                    canSign = data.getBoolean(SAVE_KEYRING_CAN_SIGN);
-                }
-
-                if (newPassphrase == null) {
-                    newPassphrase = oldPassphrase;
-                }
-
-                long masterKeyId = saveParcel.keys.get(0).getKeyId();
+                SaveKeyringParcel saveParcel = data.getParcelable(SAVE_KEYRING_PARCEL);
+                long masterKeyId = saveParcel.mMasterKeyId;
 
                 /* Operation */
                 ProviderHelper providerHelper = new ProviderHelper(this);
-                if (!canSign) {
-                    setProgress(R.string.progress_building_key, 0, 100);
-                    WrappedSecretKeyRing keyRing = providerHelper.getWrappedSecretKeyRing(masterKeyId);
-                    UncachedKeyRing newKeyRing =
-                            keyRing.changeSecretKeyPassphrase(oldPassphrase, newPassphrase);
-                    setProgress(R.string.progress_saving_key_ring, 50, 100);
-                    // providerHelper.saveSecretKeyRing(newKeyRing);
-                    setProgress(R.string.progress_done, 100, 100);
-                } else {
-                    PgpKeyOperation keyOperations = new PgpKeyOperation(new ProgressScaler(this, 0, 90, 100));
-                    try {
-                        WrappedSecretKeyRing seckey = providerHelper.getWrappedSecretKeyRing(masterKeyId);
-                        WrappedPublicKeyRing pubkey = providerHelper.getWrappedPublicKeyRing(masterKeyId);
-
-                        PgpKeyOperation.Pair<UncachedKeyRing,UncachedKeyRing> pair =
-                                keyOperations.buildSecretKey(seckey, pubkey, saveParcel); // edit existing
-                        setProgress(R.string.progress_saving_key_ring, 90, 100);
-                        providerHelper.savePairedKeyRing(pair.first, pair.second);
-                    } catch (ProviderHelper.NotFoundException e) {
-                        PgpKeyOperation.Pair<UncachedKeyRing,UncachedKeyRing> pair =
-                                keyOperations.buildNewSecretKey(saveParcel); //new Keyring
-                        // save the pair
-                        setProgress(R.string.progress_saving_key_ring, 90, 100);
-                        providerHelper.savePairedKeyRing(pair.first, pair.second);
-                    }
-
-                    setProgress(R.string.progress_done, 100, 100);
+                PgpKeyOperation keyOperations = new PgpKeyOperation(new ProgressScaler(this, 0, 90, 100));
+                try {
+                    String passphrase = data.getString(SAVE_KEYRING_PASSPHRASE);
+                    WrappedSecretKeyRing secRing = providerHelper.getWrappedSecretKeyRing(masterKeyId);
+
+                    OperationLog log = new OperationLog();
+                    UncachedKeyRing ring = keyOperations.modifySecretKeyRing(secRing, saveParcel,
+                            passphrase, log, 0);
+                    setProgress(R.string.progress_saving_key_ring, 90, 100);
+                    providerHelper.saveSecretKeyRing(ring);
+                } catch (ProviderHelper.NotFoundException e) {
+                    // UncachedKeyRing ring = keyOperations.(saveParcel); //new Keyring
+                    // save the pair
+                    setProgress(R.string.progress_saving_key_ring, 90, 100);
+                    // providerHelper.saveSecretKeyRing(ring);
+                    sendErrorToHandler(e);
                 }
-                PassphraseCacheService.addCachedPassphrase(this, masterKeyId, newPassphrase);
 
-                /* Output */
-                sendMessageToHandler(KeychainIntentServiceHandler.MESSAGE_OKAY);
-            } catch (Exception e) {
-                sendErrorToHandler(e);
-            }
-        } else if (ACTION_GENERATE_KEY.equals(action)) {
-            try {
-                /* Input */
-                int algorithm = data.getInt(GENERATE_KEY_ALGORITHM);
-                String passphrase = data.getString(GENERATE_KEY_SYMMETRIC_PASSPHRASE);
-                int keysize = data.getInt(GENERATE_KEY_KEY_SIZE);
-                boolean masterKey = data.getBoolean(GENERATE_KEY_MASTER_KEY);
+                setProgress(R.string.progress_done, 100, 100);
 
-                /* Operation */
-                PgpKeyOperation keyOperations = new PgpKeyOperation(new ProgressScaler(this, 0, 100, 100));
-                byte[] newKey = keyOperations.createKey(algorithm, keysize, passphrase, masterKey);
-
-                /* Output */
-                Bundle resultData = new Bundle();
-                resultData.putByteArray(RESULT_NEW_KEY, newKey);
-
-                OtherHelper.logDebugBundle(resultData, "resultData");
-
-                sendMessageToHandler(KeychainIntentServiceHandler.MESSAGE_OKAY, resultData);
-            } catch (Exception e) {
-                sendErrorToHandler(e);
-            }
-        } else if (ACTION_GENERATE_DEFAULT_RSA_KEYS.equals(action)) {
-            // generate one RSA 4096 key for signing and one subkey for encrypting!
-            try {
-                /* Input */
-                String passphrase = data.getString(GENERATE_KEY_SYMMETRIC_PASSPHRASE);
-                ArrayList<Integer> keyUsageList = new ArrayList<Integer>();
-
-                /* Operation */
-                int keysTotal = 3;
-                int keysCreated = 0;
-                setProgress(
-                        getApplicationContext().getResources().
-                                getQuantityString(R.plurals.progress_generating, keysTotal),
-                        keysCreated,
-                        keysTotal);
-                PgpKeyOperation keyOperations = new PgpKeyOperation(new ProgressScaler(this, 0, 100, 100));
-
-                ByteArrayOutputStream os = new ByteArrayOutputStream();
-
-                byte[] buf;
-
-                buf = keyOperations.createKey(Constants.choice.algorithm.rsa,
-                        4096, passphrase, true);
-                os.write(buf);
-                keyUsageList.add(UncachedSecretKey.CERTIFY_OTHER);
-                keysCreated++;
-                setProgress(keysCreated, keysTotal);
-
-                buf = keyOperations.createKey(Constants.choice.algorithm.rsa,
-                        4096, passphrase, false);
-                os.write(buf);
-                keyUsageList.add(UncachedSecretKey.ENCRYPT_COMMS | UncachedSecretKey.ENCRYPT_STORAGE);
-                keysCreated++;
-                setProgress(keysCreated, keysTotal);
-
-                buf = keyOperations.createKey(Constants.choice.algorithm.rsa,
-                        4096, passphrase, false);
-                os.write(buf);
-                keyUsageList.add(UncachedSecretKey.SIGN_DATA);
-                keysCreated++;
-                setProgress(keysCreated, keysTotal);
-
-                // TODO: default to one master for cert, one sub for encrypt and one sub
-                //       for sign
+                if (saveParcel.newPassphrase != null) {
+                    PassphraseCacheService.addCachedPassphrase(this, masterKeyId, saveParcel.newPassphrase);
+                }
 
                 /* Output */
-                Bundle resultData = new Bundle();
-                resultData.putByteArray(RESULT_NEW_KEY, os.toByteArray());
-                resultData.putIntegerArrayList(RESULT_KEY_USAGES, keyUsageList);
-
-                OtherHelper.logDebugBundle(resultData, "resultData");
-
-                sendMessageToHandler(KeychainIntentServiceHandler.MESSAGE_OKAY, resultData);
+                sendMessageToHandler(KeychainIntentServiceHandler.MESSAGE_OKAY);
             } catch (Exception e) {
                 sendErrorToHandler(e);
             }
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OldSaveKeyringParcel.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OldSaveKeyringParcel.java
deleted file mode 100644
index b722393ad..000000000
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OldSaveKeyringParcel.java
+++ /dev/null
@@ -1,128 +0,0 @@
-/*
- * Copyright (C) 2014 Ash Hughes <ashes-iontach@hotmail.com>
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- */
-
-package org.sufficientlysecure.keychain.service;
-
-import android.os.Parcel;
-import android.os.Parcelable;
-
-import org.sufficientlysecure.keychain.Constants;
-import org.sufficientlysecure.keychain.pgp.PgpConversionHelper;
-import org.sufficientlysecure.keychain.pgp.UncachedSecretKey;
-import org.sufficientlysecure.keychain.util.IterableIterator;
-import org.sufficientlysecure.keychain.util.Log;
-
-import java.io.ByteArrayOutputStream;
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.Calendar;
-
-/** Class for parcelling data between ui and services.
- * This class is outdated and scheduled for removal, pending a rewrite of the
- * EditKeyActivity and save keyring routines.
- */
-@Deprecated
-public class OldSaveKeyringParcel implements Parcelable {
-
-    public ArrayList<String> userIds;
-    public ArrayList<String> originalIDs;
-    public ArrayList<String> deletedIDs;
-    public boolean[] newIDs;
-    public boolean primaryIDChanged;
-    public boolean[] moddedKeys;
-    public ArrayList<UncachedSecretKey> deletedKeys;
-    public ArrayList<Calendar> keysExpiryDates;
-    public ArrayList<Integer> keysUsages;
-    public String newPassphrase;
-    public String oldPassphrase;
-    public boolean[] newKeys;
-    public ArrayList<UncachedSecretKey> keys;
-    public String originalPrimaryID;
-
-    public OldSaveKeyringParcel() {}
-
-    private OldSaveKeyringParcel(Parcel source) {
-        userIds = (ArrayList<String>) source.readSerializable();
-        originalIDs = (ArrayList<String>) source.readSerializable();
-        deletedIDs = (ArrayList<String>) source.readSerializable();
-        newIDs = source.createBooleanArray();
-        primaryIDChanged = source.readByte() != 0;
-        moddedKeys = source.createBooleanArray();
-        byte[] tmp = source.createByteArray();
-        if (tmp == null) {
-            deletedKeys = null;
-        } else {
-            deletedKeys = PgpConversionHelper.BytesToPGPSecretKeyList(tmp);
-        }
-        keysExpiryDates = (ArrayList<Calendar>) source.readSerializable();
-        keysUsages = source.readArrayList(Integer.class.getClassLoader());
-        newPassphrase = source.readString();
-        oldPassphrase = source.readString();
-        newKeys = source.createBooleanArray();
-        keys = PgpConversionHelper.BytesToPGPSecretKeyList(source.createByteArray());
-        originalPrimaryID = source.readString();
-    }
-
-    @Override
-    public void writeToParcel(Parcel destination, int flags) {
-        destination.writeSerializable(userIds); //might not be the best method to store.
-        destination.writeSerializable(originalIDs);
-        destination.writeSerializable(deletedIDs);
-        destination.writeBooleanArray(newIDs);
-        destination.writeByte((byte) (primaryIDChanged ? 1 : 0));
-        destination.writeBooleanArray(moddedKeys);
-        destination.writeByteArray(encodeArrayList(deletedKeys));
-        destination.writeSerializable(keysExpiryDates);
-        destination.writeList(keysUsages);
-        destination.writeString(newPassphrase);
-        destination.writeString(oldPassphrase);
-        destination.writeBooleanArray(newKeys);
-        destination.writeByteArray(encodeArrayList(keys));
-        destination.writeString(originalPrimaryID);
-    }
-
-    public static final Creator<OldSaveKeyringParcel> CREATOR = new Creator<OldSaveKeyringParcel>() {
-        public OldSaveKeyringParcel createFromParcel(final Parcel source) {
-            return new OldSaveKeyringParcel(source);
-        }
-
-        public OldSaveKeyringParcel[] newArray(final int size) {
-            return new OldSaveKeyringParcel[size];
-        }
-    };
-
-    private static byte[] encodeArrayList(ArrayList<UncachedSecretKey> list) {
-        if(list.isEmpty()) {
-            return null;
-        }
-
-        ByteArrayOutputStream os = new ByteArrayOutputStream();
-        for(UncachedSecretKey key : new IterableIterator<UncachedSecretKey>(list.iterator())) {
-            try {
-                key.encodeSecretKey(os);
-            } catch (IOException e) {
-                Log.e(Constants.TAG, "Error while converting ArrayList<UncachedSecretKey> to byte[]!", e);
-            }
-        }
-        return os.toByteArray();
-    }
-
-    @Override
-    public int describeContents() {
-        return 0;
-    }
-}
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 cfb977911..6e49baf92 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java
@@ -223,6 +223,7 @@ public class OperationResultParcel implements Parcelable {
         MSG_KC_UID_REVOKE_DUP (R.string.msg_kc_uid_revoke_dup),
         MSG_KC_UID_REVOKE_OLD (R.string.msg_kc_uid_revoke_old),
 
+
         // keyring consolidation
         MSG_MG_PUBLIC (R.string.msg_mg_public),
         MSG_MG_SECRET (R.string.msg_mg_secret),
@@ -230,6 +231,25 @@ public class OperationResultParcel implements Parcelable {
         MSG_MG_HETEROGENEOUS (R.string.msg_mg_heterogeneous),
         MSG_MG_NEW_SUBKEY (R.string.msg_mg_new_subkey),
         MSG_MG_FOUND_NEW (R.string.msg_mg_found_new),
+
+        // secret key modify
+        MSG_MF (R.string.msg_mr),
+        MSG_MF_ERROR_ENCODE (R.string.msg_mf_error_encode),
+        MSG_MF_ERROR_PGP (R.string.msg_mf_error_pgp),
+        MSG_MF_ERROR_SIG (R.string.msg_mf_error_sig),
+        MSG_MF_PASSPHRASE (R.string.msg_mf_passphrase),
+        MSG_MF_SUBKEY_CHANGE (R.string.msg_mf_subkey_change),
+        MSG_MF_SUBKEY_MISSING (R.string.msg_mf_subkey_missing),
+        MSG_MF_SUBKEY_NEW_ID (R.string.msg_mf_subkey_new_id),
+        MSG_MF_SUBKEY_NEW (R.string.msg_mf_subkey_new),
+        MSG_MF_SUBKEY_PAST_EXPIRY (R.string.msg_mf_subkey_past_expiry),
+        MSG_MF_SUBKEY_REVOKE (R.string.msg_mf_subkey_revoke),
+        MSG_MF_SUCCESS (R.string.msg_mf_success),
+        MSG_MF_UID_ADD (R.string.msg_mf_uid_add),
+        MSG_MF_UID_PRIMARY (R.string.msg_mf_uid_primary),
+        MSG_MF_UID_REVOKE (R.string.msg_mf_uid_revoke),
+        MSG_MF_UNLOCK_ERROR (R.string.msg_mf_unlock_error),
+        MSG_MF_UNLOCK (R.string.msg_mf_unlock),
         ;
 
         private final int mMsgId;
@@ -280,6 +300,10 @@ public class OperationResultParcel implements Parcelable {
             add(new OperationResultParcel.LogEntryParcel(level, type, parameters, indent));
         }
 
+        public void add(LogLevel level, LogType type, int indent) {
+            add(new OperationResultParcel.LogEntryParcel(level, type, null, indent));
+        }
+
         public boolean containsWarnings() {
             for(LogEntryParcel entry : new IterableIterator<LogEntryParcel>(iterator())) {
                 if (entry.mLevel == LogLevel.WARN || entry.mLevel == LogLevel.ERROR) {
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/SaveKeyringParcel.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/SaveKeyringParcel.java
index 3514ab2e5..c68b7c189 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/SaveKeyringParcel.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/SaveKeyringParcel.java
@@ -23,16 +23,16 @@ import java.util.HashMap;
 public class SaveKeyringParcel implements Parcelable {
 
     // the master key id to be edited
-    private final long mMasterKeyId;
+    public final long mMasterKeyId;
     // the key fingerprint, for safety
-    private final byte[] mFingerprint;
+    public final byte[] mFingerprint;
 
     public String newPassphrase;
 
     public String[] addUserIds;
     public SubkeyAdd[] addSubKeys;
 
-    public HashMap<Long, SubkeyChange> changeSubKeys;
+    public SubkeyChange[] changeSubKeys;
     public String changePrimaryUserId;
 
     public String[] revokeUserIds;
@@ -76,7 +76,7 @@ public class SaveKeyringParcel implements Parcelable {
         addUserIds = source.createStringArray();
         addSubKeys = (SubkeyAdd[]) source.readSerializable();
 
-        changeSubKeys = (HashMap<Long,SubkeyChange>) source.readSerializable();
+        changeSubKeys = (SubkeyChange[]) source.readSerializable();
         changePrimaryUserId = source.readString();
 
         revokeUserIds = source.createStringArray();
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyActivity.java
index bcb2286ab..4309e3505 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyActivity.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyActivity.java
@@ -57,7 +57,6 @@ import org.sufficientlysecure.keychain.provider.KeychainContract.KeyRings;
 import org.sufficientlysecure.keychain.provider.ProviderHelper;
 import org.sufficientlysecure.keychain.service.KeychainIntentService;
 import org.sufficientlysecure.keychain.service.KeychainIntentServiceHandler;
-import org.sufficientlysecure.keychain.service.OldSaveKeyringParcel;
 import org.sufficientlysecure.keychain.service.PassphraseCacheService;
 import org.sufficientlysecure.keychain.ui.dialog.CustomAlertDialogBuilder;
 import org.sufficientlysecure.keychain.ui.dialog.PassphraseDialogFragment;
@@ -199,13 +198,10 @@ public class EditKeyActivity extends ActionBarActivity implements EditorListener
 
             // generate key
             if (extras.containsKey(EXTRA_GENERATE_DEFAULT_KEYS)) {
+                /*
                 boolean generateDefaultKeys = extras.getBoolean(EXTRA_GENERATE_DEFAULT_KEYS);
                 if (generateDefaultKeys) {
 
-                    // Send all information needed to service generate keys in other thread
-                    final Intent serviceIntent = new Intent(this, KeychainIntentService.class);
-                    serviceIntent.setAction(KeychainIntentService.ACTION_GENERATE_DEFAULT_RSA_KEYS);
-
                     // fill values for this action
                     Bundle data = new Bundle();
                     data.putString(KeychainIntentService.GENERATE_KEY_SYMMETRIC_PASSPHRASE,
@@ -265,6 +261,7 @@ public class EditKeyActivity extends ActionBarActivity implements EditorListener
                     // start service with intent
                     startService(serviceIntent);
                 }
+                */
             }
         } else {
             buildLayout(false);
@@ -547,6 +544,7 @@ public class EditKeyActivity extends ActionBarActivity implements EditorListener
     }
 
     private void finallySaveClicked() {
+            /*
         try {
             // Send all information needed to service to edit key in other thread
             Intent intent = new Intent(this, KeychainIntentService.class);
@@ -609,6 +607,7 @@ public class EditKeyActivity extends ActionBarActivity implements EditorListener
             AppMsg.makeText(this, getString(R.string.error_message, e.getMessage()),
                     AppMsg.STYLE_ALERT).show();
         }
+            */
     }
 
     private void cancelClicked() {
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/widget/SectionView.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/widget/SectionView.java
index 3e8e18ce5..b7336318f 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/widget/SectionView.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/widget/SectionView.java
@@ -346,13 +346,8 @@ public class SectionView extends LinearLayout implements OnClickListener, Editor
     }
 
     private void createKey() {
-        // Send all information needed to service to edit key in other thread
-        final Intent intent = new Intent(mActivity, KeychainIntentService.class);
-
-        intent.setAction(KeychainIntentService.ACTION_GENERATE_KEY);
 
         // fill values for this action
-        Bundle data = new Bundle();
         Boolean isMasterKey;
 
         String passphrase;
@@ -365,6 +360,7 @@ public class SectionView extends LinearLayout implements OnClickListener, Editor
             passphrase = "";
             isMasterKey = true;
         }
+        /*
         data.putBoolean(KeychainIntentService.GENERATE_KEY_MASTER_KEY, isMasterKey);
         data.putString(KeychainIntentService.GENERATE_KEY_SYMMETRIC_PASSPHRASE, passphrase);
         data.putInt(KeychainIntentService.GENERATE_KEY_ALGORITHM, mNewKeyAlgorithmChoice.getId());
@@ -410,6 +406,8 @@ public class SectionView extends LinearLayout implements OnClickListener, Editor
 
         // start service with intent
         mActivity.startService(intent);
+        */
+
     }
 
     private void addGeneratedKeyToView(UncachedSecretKey newKey) {
diff --git a/OpenKeychain/src/main/res/values/strings.xml b/OpenKeychain/src/main/res/values/strings.xml
index 6b921b681..3221cd9cd 100644
--- a/OpenKeychain/src/main/res/values/strings.xml
+++ b/OpenKeychain/src/main/res/values/strings.xml
@@ -600,7 +600,6 @@
     <string name="msg_kc_uid_revoke_old">Removing outdated revocation certificate for user id "%s"</string>
     <string name="msg_kc_uid_no_cert">No valid self-certificate found for user id %s, removing from ring</string>
 
-
     <!-- Keyring merging log entries -->
     <string name="msg_mg_public">Merging into public keyring %s</string>
     <string name="msg_mg_secret">Merging into secret keyring %s</string>
@@ -609,6 +608,25 @@
     <string name="msg_mg_new_subkey">Adding new subkey %s</string>
     <string name="msg_mg_found_new">Found %s new certificates in keyring</string>
 
+    <!-- modifySecretKeyRing -->
+    <string name="msg_mr">Modifying keyring %s</string>
+    <string name="msg_mf_error_encode">Encoding exception!</string>
+    <string name="msg_mf_error_pgp">PGP internal exception!</string>
+    <string name="msg_mf_error_sig">Signature exception!</string>
+    <string name="msg_mf_passphrase">Changing passphrase</string>
+    <string name="msg_mf_subkey_change">Modifying subkey %s</string>
+    <string name="msg_mf_subkey_missing">Tried to operate on missing subkey %s!</string>
+    <string name="msg_mf_subkey_new">Generating new %1$s bit %2$s subkey</string>
+    <string name="msg_mf_subkey_new_id">New subkey id: %s</string>
+    <string name="msg_mf_subkey_past_expiry">Expiry date cannot be in the past!</string>
+    <string name="msg_mf_subkey_revoke">Revoking subkey %s</string>
+    <string name="msg_mf_success">Keyring successfully modified</string>
+    <string name="msg_mf_uid_add">Adding user id %s</string>
+    <string name="msg_mf_uid_primary">Changing primary uid to %s</string>
+    <string name="msg_mf_uid_revoke">Revoking user id %s</string>
+    <string name="msg_mf_unlock_error">Error unlocking keyring!</string>
+    <string name="msg_mf_unlock">Unlocking keyring</string>
+
     <!-- unsorted -->
     <string name="section_certifier_id">Certifier</string>
     <string name="section_cert">Certificate Details</string>