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

8 files changed, 796 insertions, 82 deletions

diff --git a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/operations/CertifyOperationTest.java b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/operations/CertifyOperationTest.java
new file mode 100644
index 000000000..c05f4a029
--- /dev/null
+++ b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/operations/CertifyOperationTest.java
@@ -0,0 +1,238 @@
+/*
+ * Copyright (C) 2014 Vincent Breitmoser <v.breitmoser@mugenguild.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.operations;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.robolectric.Robolectric;
+import org.robolectric.RobolectricTestRunner;
+import org.robolectric.shadows.ShadowLog;
+import org.spongycastle.bcpg.sig.KeyFlags;
+import org.spongycastle.jce.provider.BouncyCastleProvider;
+import org.sufficientlysecure.keychain.operations.results.CertifyResult;
+import org.sufficientlysecure.keychain.operations.results.OperationResult.LogType;
+import org.sufficientlysecure.keychain.operations.results.PgpEditKeyResult;
+import org.sufficientlysecure.keychain.operations.results.ExportResult;
+import org.sufficientlysecure.keychain.operations.results.OperationResult.OperationLog;
+import org.sufficientlysecure.keychain.pgp.CanonicalizedPublicKeyRing;
+import org.sufficientlysecure.keychain.pgp.PgpDecryptVerify;
+import org.sufficientlysecure.keychain.pgp.PgpKeyOperation;
+import org.sufficientlysecure.keychain.pgp.UncachedKeyRing;
+import org.sufficientlysecure.keychain.pgp.UncachedKeyRing.IteratorWithIOThrow;
+import org.sufficientlysecure.keychain.pgp.WrappedSignature;
+import org.sufficientlysecure.keychain.provider.KeychainContract.Certs;
+import org.sufficientlysecure.keychain.provider.ProviderHelper;
+import org.sufficientlysecure.keychain.service.CertifyActionsParcel;
+import org.sufficientlysecure.keychain.service.CertifyActionsParcel.CertifyAction;
+import org.sufficientlysecure.keychain.service.SaveKeyringParcel;
+import org.sufficientlysecure.keychain.service.SaveKeyringParcel.Algorithm;
+import org.sufficientlysecure.keychain.service.SaveKeyringParcel.ChangeUnlockParcel;
+import org.sufficientlysecure.keychain.util.InputData;
+import org.sufficientlysecure.keychain.util.ProgressScaler;
+import org.sufficientlysecure.keychain.util.TestingUtils;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.OutputStream;
+import java.io.PrintStream;
+import java.security.Security;
+import java.util.Iterator;
+
+@RunWith(RobolectricTestRunner.class)
+@org.robolectric.annotation.Config(emulateSdk = 18) // Robolectric doesn't yet support 19
+public class CertifyOperationTest {
+
+    static String mPassphrase = TestingUtils.genPassphrase(true);
+
+    static UncachedKeyRing mStaticRing1, mStaticRing2;
+    static String mKeyPhrase1 = TestingUtils.genPassphrase(true);
+    static String mKeyPhrase2 = TestingUtils.genPassphrase(true);
+
+    static PrintStream oldShadowStream;
+
+    @BeforeClass
+    public static void setUpOnce() throws Exception {
+        Security.insertProviderAt(new BouncyCastleProvider(), 1);
+        oldShadowStream = ShadowLog.stream;
+        // ShadowLog.stream = System.out;
+
+        PgpKeyOperation op = new PgpKeyOperation(null);
+
+        {
+            SaveKeyringParcel parcel = new SaveKeyringParcel();
+            parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
+                    Algorithm.RSA, 1024, null, KeyFlags.CERTIFY_OTHER, 0L));
+            parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
+                    Algorithm.DSA, 1024, null, KeyFlags.SIGN_DATA, 0L));
+            parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
+                    Algorithm.ELGAMAL, 1024, null, KeyFlags.ENCRYPT_COMMS, 0L));
+            parcel.mAddUserIds.add("derp");
+            parcel.mNewUnlock = new ChangeUnlockParcel(mKeyPhrase1);
+
+            PgpEditKeyResult result = op.createSecretKeyRing(parcel);
+            Assert.assertTrue("initial test key creation must succeed", result.success());
+            Assert.assertNotNull("initial test key creation must succeed", result.getRing());
+
+            mStaticRing1 = result.getRing();
+        }
+
+        {
+            SaveKeyringParcel parcel = new SaveKeyringParcel();
+            parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
+                    Algorithm.RSA, 1024, null, KeyFlags.CERTIFY_OTHER, 0L));
+            parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
+                    Algorithm.DSA, 1024, null, KeyFlags.SIGN_DATA, 0L));
+            parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
+                    Algorithm.ELGAMAL, 1024, null, KeyFlags.ENCRYPT_COMMS, 0L));
+            parcel.mAddUserIds.add("ditz");
+            parcel.mNewUnlock = new ChangeUnlockParcel(null, "1234");
+
+            PgpEditKeyResult result = op.createSecretKeyRing(parcel);
+            Assert.assertTrue("initial test key creation must succeed", result.success());
+            Assert.assertNotNull("initial test key creation must succeed", result.getRing());
+
+            mStaticRing2 = result.getRing();
+        }
+
+    }
+
+    @Before
+    public void setUp() throws Exception {
+        ProviderHelper providerHelper = new ProviderHelper(Robolectric.application);
+
+        // don't log verbosely here, we're not here to test imports
+        ShadowLog.stream = oldShadowStream;
+
+        providerHelper.saveSecretKeyRing(mStaticRing1, new ProgressScaler());
+        providerHelper.savePublicKeyRing(mStaticRing2.extractPublicKeyRing(), new ProgressScaler());
+
+        // ok NOW log verbosely!
+        ShadowLog.stream = System.out;
+    }
+
+    @Test
+    public void testSelfCertifyFlag() throws Exception {
+
+        CanonicalizedPublicKeyRing ring = new ProviderHelper(Robolectric.application)
+                .getCanonicalizedPublicKeyRing(mStaticRing1.getMasterKeyId());
+        Assert.assertEquals("secret key must be marked self-certified in database",
+                // TODO this should be more correctly be VERIFIED_SELF at some point!
+                Certs.VERIFIED_SECRET, ring.getVerified());
+
+    }
+
+    @Test
+    public void testCertify() throws Exception {
+        CertifyOperation op = operationWithFakePassphraseCache(
+                mStaticRing1.getMasterKeyId(), mStaticRing1.getMasterKeyId(), mKeyPhrase1);
+
+        {
+            CanonicalizedPublicKeyRing ring = new ProviderHelper(Robolectric.application)
+                    .getCanonicalizedPublicKeyRing(mStaticRing2.getMasterKeyId());
+            Assert.assertEquals("public key must not be marked verified prior to certification",
+                    Certs.UNVERIFIED, ring.getVerified());
+        }
+
+        CertifyActionsParcel actions = new CertifyActionsParcel(mStaticRing1.getMasterKeyId());
+        actions.add(new CertifyAction(mStaticRing2.getMasterKeyId()));
+        CertifyResult result = op.certify(actions, null);
+
+        Assert.assertTrue("certification must succeed", result.success());
+
+        {
+            CanonicalizedPublicKeyRing ring = new ProviderHelper(Robolectric.application)
+                    .getCanonicalizedPublicKeyRing(mStaticRing2.getMasterKeyId());
+            Assert.assertEquals("new key must be verified now",
+                    Certs.VERIFIED_SECRET, ring.getVerified());
+        }
+
+    }
+
+    @Test
+    public void testCertifySelf() throws Exception {
+        CertifyOperation op = operationWithFakePassphraseCache(
+                mStaticRing1.getMasterKeyId(), mStaticRing1.getMasterKeyId(), mKeyPhrase1);
+
+        CertifyActionsParcel actions = new CertifyActionsParcel(mStaticRing1.getMasterKeyId());
+        actions.add(new CertifyAction(mStaticRing1.getMasterKeyId()));
+
+        CertifyResult result = op.certify(actions, null);
+
+        Assert.assertFalse("certification with itself must fail!", result.success());
+        Assert.assertTrue("error msg must be about self certification",
+                result.getLog().containsType(LogType.MSG_CRT_ERROR_SELF));
+    }
+
+    @Test
+    public void testCertifyNonexistent() throws Exception {
+
+        CertifyOperation op = operationWithFakePassphraseCache(null, null, mKeyPhrase1);
+
+        {
+            CertifyActionsParcel actions = new CertifyActionsParcel(mStaticRing1.getMasterKeyId());
+            actions.add(new CertifyAction(1234L));
+
+            CertifyResult result = op.certify(actions, null);
+
+            Assert.assertFalse("certification of nonexistent key must fail", result.success());
+            Assert.assertTrue("must contain error msg about not found",
+                    result.getLog().containsType(LogType.MSG_CRT_WARN_NOT_FOUND));
+        }
+
+        {
+            CertifyActionsParcel actions = new CertifyActionsParcel(1234L);
+            actions.add(new CertifyAction(mStaticRing1.getMasterKeyId()));
+
+            CertifyResult result = op.certify(actions, null);
+
+            Assert.assertFalse("certification of nonexistent key must fail", result.success());
+            Assert.assertTrue("must contain error msg about not found",
+                    result.getLog().containsType(LogType.MSG_CRT_ERROR_MASTER_NOT_FOUND));
+        }
+
+    }
+
+    private CertifyOperation operationWithFakePassphraseCache(
+            final Long checkMasterKeyId, final Long checkSubKeyId, final String passphrase) {
+
+        return new CertifyOperation(Robolectric.application,
+                new ProviderHelper(Robolectric.application),
+                null, null) {
+            @Override
+            public String getCachedPassphrase(long masterKeyId, long subKeyId)
+                    throws NoSecretKeyException {
+                if (checkMasterKeyId != null) {
+                    Assert.assertEquals("requested passphrase should be for expected master key id",
+                            (long) checkMasterKeyId, masterKeyId);
+                }
+                if (checkSubKeyId != null) {
+                    Assert.assertEquals("requested passphrase should be for expected sub key id",
+                            (long) checkSubKeyId, subKeyId);
+                }
+                if (passphrase == null) {
+                    return null;
+                }
+                return passphrase;
+            }
+        };
+    }
+
+}
diff --git a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/operations/ExportTest.java b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/operations/ExportTest.java
new file mode 100644
index 000000000..b6fdbfc6c
--- /dev/null
+++ b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/operations/ExportTest.java
@@ -0,0 +1,223 @@
+/*
+ * Copyright (C) 2014 Vincent Breitmoser <v.breitmoser@mugenguild.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.operations;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.robolectric.Robolectric;
+import org.robolectric.RobolectricTestRunner;
+import org.robolectric.shadows.ShadowLog;
+import org.spongycastle.bcpg.sig.KeyFlags;
+import org.spongycastle.jce.provider.BouncyCastleProvider;
+import org.sufficientlysecure.keychain.operations.results.PgpEditKeyResult;
+import org.sufficientlysecure.keychain.operations.results.ExportResult;
+import org.sufficientlysecure.keychain.operations.results.OperationResult.OperationLog;
+import org.sufficientlysecure.keychain.pgp.PgpKeyOperation;
+import org.sufficientlysecure.keychain.pgp.UncachedKeyRing;
+import org.sufficientlysecure.keychain.pgp.UncachedKeyRing.IteratorWithIOThrow;
+import org.sufficientlysecure.keychain.pgp.WrappedSignature;
+import org.sufficientlysecure.keychain.provider.ProviderHelper;
+import org.sufficientlysecure.keychain.service.SaveKeyringParcel;
+import org.sufficientlysecure.keychain.service.SaveKeyringParcel.Algorithm;
+import org.sufficientlysecure.keychain.service.SaveKeyringParcel.ChangeUnlockParcel;
+import org.sufficientlysecure.keychain.util.ProgressScaler;
+import org.sufficientlysecure.keychain.util.TestingUtils;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.PrintStream;
+import java.security.Security;
+import java.util.Iterator;
+
+@RunWith(RobolectricTestRunner.class)
+@org.robolectric.annotation.Config(emulateSdk = 18) // Robolectric doesn't yet support 19
+public class ExportTest {
+
+    static String mPassphrase = TestingUtils.genPassphrase(true);
+
+    static UncachedKeyRing mStaticRing1, mStaticRing2;
+    static String mKeyPhrase1 = TestingUtils.genPassphrase(true);
+    static String mKeyPhrase2 = TestingUtils.genPassphrase(true);
+
+    static PrintStream oldShadowStream;
+
+    @BeforeClass
+    public static void setUpOnce() throws Exception {
+        Security.insertProviderAt(new BouncyCastleProvider(), 1);
+        oldShadowStream = ShadowLog.stream;
+        // ShadowLog.stream = System.out;
+
+        PgpKeyOperation op = new PgpKeyOperation(null);
+
+        {
+            SaveKeyringParcel parcel = new SaveKeyringParcel();
+            parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
+                    Algorithm.RSA, 1024, null, KeyFlags.CERTIFY_OTHER, 0L));
+            parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
+                    Algorithm.DSA, 1024, null, KeyFlags.SIGN_DATA, 0L));
+            parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
+                    Algorithm.ELGAMAL, 1024, null, KeyFlags.ENCRYPT_COMMS, 0L));
+            parcel.mAddUserIds.add("snips");
+            parcel.mNewUnlock = new ChangeUnlockParcel(mKeyPhrase1);
+
+            PgpEditKeyResult result = op.createSecretKeyRing(parcel);
+            Assert.assertTrue("initial test key creation must succeed", result.success());
+            Assert.assertNotNull("initial test key creation must succeed", result.getRing());
+
+            mStaticRing1 = result.getRing();
+        }
+
+        {
+            SaveKeyringParcel parcel = new SaveKeyringParcel();
+            parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
+                    Algorithm.RSA, 1024, null, KeyFlags.CERTIFY_OTHER, 0L));
+            parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
+                    Algorithm.DSA, 1024, null, KeyFlags.SIGN_DATA, 0L));
+            parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
+                    Algorithm.ELGAMAL, 1024, null, KeyFlags.ENCRYPT_COMMS, 0L));
+            parcel.mAddUserIds.add("snails");
+            parcel.mNewUnlock = new ChangeUnlockParcel(null, "1234");
+
+            PgpEditKeyResult result = op.createSecretKeyRing(parcel);
+            Assert.assertTrue("initial test key creation must succeed", result.success());
+            Assert.assertNotNull("initial test key creation must succeed", result.getRing());
+
+            mStaticRing2 = result.getRing();
+        }
+
+    }
+
+    @Before
+    public void setUp() {
+        ProviderHelper providerHelper = new ProviderHelper(Robolectric.application);
+
+        // don't log verbosely here, we're not here to test imports
+        ShadowLog.stream = oldShadowStream;
+
+        providerHelper.saveSecretKeyRing(mStaticRing1, new ProgressScaler());
+        providerHelper.saveSecretKeyRing(mStaticRing2, new ProgressScaler());
+
+        // ok NOW log verbosely!
+        ShadowLog.stream = System.out;
+    }
+
+    @Test
+    public void testExportAll() throws Exception {
+        ImportExportOperation op = new ImportExportOperation(Robolectric.application,
+                new ProviderHelper(Robolectric.application), null);
+
+        // make sure there is a local cert (so the later checks that there are none are meaningful)
+        Assert.assertTrue("second keyring has local certification", checkForLocal(mStaticRing2));
+
+        ByteArrayOutputStream out = new ByteArrayOutputStream();
+        ExportResult result = op.exportKeyRings(new OperationLog(), null, false, out);
+
+        Assert.assertTrue("export must be a success", result.success());
+
+        long masterKeyId1, masterKeyId2;
+        if (mStaticRing1.getMasterKeyId() < mStaticRing2.getMasterKeyId()) {
+            masterKeyId1 = mStaticRing1.getMasterKeyId();
+            masterKeyId2 = mStaticRing2.getMasterKeyId();
+        } else {
+            masterKeyId2 = mStaticRing1.getMasterKeyId();
+            masterKeyId1 = mStaticRing2.getMasterKeyId();
+        }
+
+        IteratorWithIOThrow<UncachedKeyRing> unc =
+                UncachedKeyRing.fromStream(new ByteArrayInputStream(out.toByteArray()));
+
+        {
+            Assert.assertTrue("export must have two keys (1/2)", unc.hasNext());
+            UncachedKeyRing ring = unc.next();
+            Assert.assertEquals("first exported key has correct masterkeyid",
+                    masterKeyId1, ring.getMasterKeyId());
+            Assert.assertFalse("first exported key must not be secret", ring.isSecret());
+            Assert.assertFalse("there must be no local signatures in an exported keyring",
+                    checkForLocal(ring));
+        }
+
+        {
+            Assert.assertTrue("export must have two keys (2/2)", unc.hasNext());
+            UncachedKeyRing ring = unc.next();
+            Assert.assertEquals("second exported key has correct masterkeyid",
+                    masterKeyId2, ring.getMasterKeyId());
+            Assert.assertFalse("second exported key must not be secret", ring.isSecret());
+            Assert.assertFalse("there must be no local signatures in an exported keyring",
+                    checkForLocal(ring));
+        }
+
+        out = new ByteArrayOutputStream();
+        result = op.exportKeyRings(new OperationLog(), null, true, out);
+
+        Assert.assertTrue("export must be a success", result.success());
+
+        unc = UncachedKeyRing.fromStream(new ByteArrayInputStream(out.toByteArray()));
+
+        {
+            Assert.assertTrue("export must have four keys (1/4)", unc.hasNext());
+            UncachedKeyRing ring = unc.next();
+            Assert.assertEquals("1/4 exported key has correct masterkeyid",
+                    masterKeyId1, ring.getMasterKeyId());
+            Assert.assertFalse("1/4 exported key must not be public", ring.isSecret());
+            Assert.assertFalse("there must be no local signatures in an exported keyring",
+                    checkForLocal(ring));
+
+            Assert.assertTrue("export must have four keys (2/4)", unc.hasNext());
+            ring = unc.next();
+            Assert.assertEquals("2/4 exported key has correct masterkeyid",
+                    masterKeyId1, ring.getMasterKeyId());
+            Assert.assertTrue("2/4 exported key must be public", ring.isSecret());
+            Assert.assertFalse("there must be no local signatures in an exported keyring",
+                    checkForLocal(ring));
+        }
+
+        {
+            Assert.assertTrue("export must have four keys (3/4)", unc.hasNext());
+            UncachedKeyRing ring = unc.next();
+            Assert.assertEquals("3/4 exported key has correct masterkeyid",
+                    masterKeyId2, ring.getMasterKeyId());
+            Assert.assertFalse("3/4 exported key must not be public", ring.isSecret());
+            Assert.assertFalse("there must be no local signatures in an exported keyring",
+                    checkForLocal(ring));
+
+            Assert.assertTrue("export must have four keys (4/4)", unc.hasNext());
+            ring = unc.next();
+            Assert.assertEquals("4/4 exported key has correct masterkeyid",
+                    masterKeyId2, ring.getMasterKeyId());
+            Assert.assertTrue("4/4 exported key must be public", ring.isSecret());
+            Assert.assertFalse("there must be no local signatures in an exported keyring",
+                    checkForLocal(ring));
+        }
+
+    }
+
+    /** This function checks whether or not there are any local signatures in a keyring. */
+    private boolean checkForLocal(UncachedKeyRing ring) {
+        Iterator<WrappedSignature> sigs = ring.getPublicKey().getSignatures();
+        while (sigs.hasNext()) {
+            if (sigs.next().isLocal()) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+}
diff --git a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/PgpEncryptDecryptTest.java b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/PgpEncryptDecryptTest.java
index ebe31c9f0..008edcda4 100644
--- a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/PgpEncryptDecryptTest.java
+++ b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/PgpEncryptDecryptTest.java
@@ -22,25 +22,32 @@ import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.openintents.openpgp.OpenPgpSignatureResult;
 import org.robolectric.*;
 import org.robolectric.shadows.ShadowLog;
 import org.spongycastle.bcpg.sig.KeyFlags;
 import org.spongycastle.jce.provider.BouncyCastleProvider;
 import org.spongycastle.openpgp.PGPEncryptedData;
+import org.sufficientlysecure.keychain.operations.results.PgpEditKeyResult;
 import org.sufficientlysecure.keychain.pgp.PgpSignEncrypt.Builder;
+import org.sufficientlysecure.keychain.provider.KeychainContract.KeyRingData;
 import org.sufficientlysecure.keychain.provider.ProviderHelper;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel.Algorithm;
 import org.sufficientlysecure.keychain.operations.results.DecryptVerifyResult;
-import org.sufficientlysecure.keychain.operations.results.EditKeyResult;
 import org.sufficientlysecure.keychain.operations.results.SignEncryptResult;
+import org.sufficientlysecure.keychain.service.SaveKeyringParcel.ChangeUnlockParcel;
+import org.sufficientlysecure.keychain.support.KeyringTestingHelper;
 import org.sufficientlysecure.keychain.util.InputData;
 import org.sufficientlysecure.keychain.util.ProgressScaler;
 import org.sufficientlysecure.keychain.util.TestingUtils;
 
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
+import java.io.OutputStream;
+import java.io.PrintStream;
 import java.security.Security;
+import java.util.HashSet;
 
 @RunWith(RobolectricTestRunner.class)
 @org.robolectric.annotation.Config(emulateSdk = 18) // Robolectric doesn't yet support 19
@@ -52,10 +59,13 @@ public class PgpEncryptDecryptTest {
     static String mKeyPhrase1 = TestingUtils.genPassphrase(true);
     static String mKeyPhrase2 = TestingUtils.genPassphrase(true);
 
+    static PrintStream oldShadowStream;
+
     @BeforeClass
     public static void setUpOnce() throws Exception {
         Security.insertProviderAt(new BouncyCastleProvider(), 1);
-        ShadowLog.stream = System.out;
+        oldShadowStream = ShadowLog.stream;
+        // ShadowLog.stream = System.out;
 
         PgpKeyOperation op = new PgpKeyOperation(null);
 
@@ -68,9 +78,9 @@ public class PgpEncryptDecryptTest {
             parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
                     Algorithm.ELGAMAL, 1024, null, KeyFlags.ENCRYPT_COMMS, 0L));
             parcel.mAddUserIds.add("bloom");
-            parcel.mNewUnlock = mKeyPhrase1;
+            parcel.mNewUnlock = new ChangeUnlockParcel(mKeyPhrase1);
 
-            EditKeyResult result = op.createSecretKeyRing(parcel);
+            PgpEditKeyResult result = op.createSecretKeyRing(parcel);
             Assert.assertTrue("initial test key creation must succeed", result.success());
             Assert.assertNotNull("initial test key creation must succeed", result.getRing());
 
@@ -86,9 +96,9 @@ public class PgpEncryptDecryptTest {
             parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
                     Algorithm.ELGAMAL, 1024, null, KeyFlags.ENCRYPT_COMMS, 0L));
             parcel.mAddUserIds.add("belle");
-            parcel.mNewUnlock = mKeyPhrase2;
+            parcel.mNewUnlock = new ChangeUnlockParcel(mKeyPhrase2);
 
-            EditKeyResult result = op.createSecretKeyRing(parcel);
+            PgpEditKeyResult result = op.createSecretKeyRing(parcel);
             Assert.assertTrue("initial test key creation must succeed", result.success());
             Assert.assertNotNull("initial test key creation must succeed", result.getRing());
 
@@ -101,8 +111,14 @@ public class PgpEncryptDecryptTest {
     public void setUp() {
         ProviderHelper providerHelper = new ProviderHelper(Robolectric.application);
 
+        // don't log verbosely here, we're not here to test imports
+        ShadowLog.stream = oldShadowStream;
+
         providerHelper.saveSecretKeyRing(mStaticRing1, new ProgressScaler());
-        providerHelper.saveSecretKeyRing(mStaticRing1, new ProgressScaler());
+        providerHelper.saveSecretKeyRing(mStaticRing2, new ProgressScaler());
+
+        // ok NOW log verbosely!
+        ShadowLog.stream = System.out;
     }
 
     @Test
@@ -118,7 +134,7 @@ public class PgpEncryptDecryptTest {
             InputData data = new InputData(in, in.available());
             Builder b = new PgpSignEncrypt.Builder(Robolectric.application,
                     new ProviderHelper(Robolectric.application),
-                    null, // new DummyPassphraseCache(mPassphrase, 0L)
+                    null,
                     data, out);
 
             b.setSymmetricPassphrase(mPassphrase);
@@ -215,11 +231,8 @@ public class PgpEncryptDecryptTest {
             ByteArrayInputStream in = new ByteArrayInputStream(ciphertext);
             InputData data = new InputData(in, in.available());
 
-            PgpDecryptVerify.Builder b = new PgpDecryptVerify.Builder(
-                    Robolectric.application,
-                    new ProviderHelper(Robolectric.application),
-                    null, // new DummyPassphraseCache(null, null),
-                    data, out);
+
+            PgpDecryptVerify.Builder b = builderWithFakePassphraseCache(data, out, null, null, null);
             b.setPassphrase(mKeyPhrase1);
             DecryptVerifyResult result = b.build().execute();
             Assert.assertTrue("decryption with provided passphrase must succeed", result.success());
@@ -230,74 +243,230 @@ public class PgpEncryptDecryptTest {
 
         // TODO how to test passphrase cache?
 
-        /*{ // decryption with passphrase cached should succeed
+        { // decryption with passphrase cached should succeed
 
             ByteArrayOutputStream out = new ByteArrayOutputStream();
             ByteArrayInputStream in = new ByteArrayInputStream(ciphertext);
             InputData data = new InputData(in, in.available());
 
-            PassphraseCacheService.addCachedPassphrase(
-                    Robolectric.application, mStaticRing1.getMasterKeyId(),
-                    mStaticRing1.getMasterKeyId(), mKeyPhrase1, "dummy");
+            PgpDecryptVerify.Builder b = builderWithFakePassphraseCache(data, out,
+                    mKeyPhrase1, mStaticRing1.getMasterKeyId(), null);
 
-            PgpDecryptVerify.Builder b = new PgpDecryptVerify.Builder(
-                    Robolectric.application,
-                    new ProviderHelper(Robolectric.application),
-                    null, // new DummyPassphraseCache(mKeyPhrase1, null),
-                    data, out);
             DecryptVerifyResult result = b.build().execute();
             Assert.assertTrue("decryption with cached passphrase must succeed", result.success());
             Assert.assertArrayEquals("decrypted ciphertext with cached passphrase  should equal plaintext",
                     out.toByteArray(), plaintext.getBytes());
             Assert.assertNull("signature should be empty", result.getSignatureResult());
-        }*/
+        }
 
-        /*{ // decryption with no passphrase provided should return status pending
+        { // decryption with no passphrase provided should return status pending
 
             ByteArrayOutputStream out = new ByteArrayOutputStream();
             ByteArrayInputStream in = new ByteArrayInputStream(ciphertext);
             InputData data = new InputData(in, in.available());
 
-            PgpDecryptVerify.Builder b = new PgpDecryptVerify.Builder(
-                    Robolectric.application,
-                    new ProviderHelper(Robolectric.application),
-                    null, // new DummyPassphraseCache(null, null),
-                    data, out);
+            PgpDecryptVerify.Builder b = builderWithFakePassphraseCache(data, out,
+                    null, mStaticRing1.getMasterKeyId(), null);
             DecryptVerifyResult result = b.build().execute();
             Assert.assertFalse("decryption with no passphrase must return pending", result.success());
             Assert.assertTrue("decryption with no passphrase should return pending", result.isPending());
             Assert.assertEquals("decryption with no passphrase should return pending passphrase",
                     DecryptVerifyResult.RESULT_PENDING_ASYM_PASSPHRASE, result.getResult());
-        }*/
+        }
 
     }
 
-    static class DummyPassphraseCache implements PassphraseCacheInterface {
+    @Test
+    public void testMultiAsymmetricEncryptDecrypt() {
+
+        String plaintext = "dies ist ein plaintext ☭" + TestingUtils.genPassphrase(true);
+        byte[] ciphertext;
+
+        { // encrypt data with a given passphrase
+            ByteArrayOutputStream out = new ByteArrayOutputStream();
+            ByteArrayInputStream in = new ByteArrayInputStream(plaintext.getBytes());
+
+            InputData data = new InputData(in, in.available());
+            Builder b = new PgpSignEncrypt.Builder(
+                    Robolectric.application,
+                    new ProviderHelper(Robolectric.application),
+                    null, // new DummyPassphraseCache(mPassphrase, 0L),
+                    data, out);
+
+            b.setEncryptionMasterKeyIds(new long[] {
+                    mStaticRing1.getMasterKeyId(),
+                    mStaticRing2.getMasterKeyId()
+            });
+            b.setSymmetricEncryptionAlgorithm(PGPEncryptedData.AES_128);
+            SignEncryptResult result = b.build().execute();
+            Assert.assertTrue("encryption must succeed", result.success());
 
-        String mPassphrase;
-        Long mExpectedId;
-        public DummyPassphraseCache(String passphrase, Long expectedId) {
-            mPassphrase = passphrase;
-            mExpectedId = expectedId;
+            ciphertext = out.toByteArray();
         }
 
-        @Override
-        public String getCachedPassphrase(long masterKeyId, long subKeyId) throws NoSecretKeyException {
-            if (mExpectedId != null){
-                Assert.assertEquals("requested passphrase must be for expected id",
-                        (long) mExpectedId, subKeyId);
-            }
-            return mPassphrase;
+        { // decryption with passphrase cached should succeed for the first key
+
+            ByteArrayOutputStream out = new ByteArrayOutputStream();
+            ByteArrayInputStream in = new ByteArrayInputStream(ciphertext);
+            InputData data = new InputData(in, in.available());
+
+            PgpDecryptVerify.Builder b = builderWithFakePassphraseCache(data, out,
+                    mKeyPhrase1, mStaticRing1.getMasterKeyId(), null);
+
+            DecryptVerifyResult result = b.build().execute();
+            Assert.assertTrue("decryption with cached passphrase must succeed for the first key", result.success());
+            Assert.assertArrayEquals("decrypted ciphertext with cached passphrase  should equal plaintext",
+                    out.toByteArray(), plaintext.getBytes());
+            Assert.assertNull("signature should be empty", result.getSignatureResult());
         }
 
-        @Override
-        public String getCachedPassphrase(long subKeyId) throws NoSecretKeyException {
-            if (mExpectedId != null){
-                Assert.assertEquals("requested passphrase must be for expected id",
-                        (long) mExpectedId, subKeyId);
-            }
-            return mPassphrase;
+        { // decryption with passphrase cached should succeed for the first key
+
+            ByteArrayOutputStream out = new ByteArrayOutputStream();
+            ByteArrayInputStream in = new ByteArrayInputStream(ciphertext);
+            InputData data = new InputData(in, in.available());
+
+            // allow only the second to decrypt
+            HashSet<Long> allowed = new HashSet<Long>();
+            allowed.add(mStaticRing2.getMasterKeyId());
+
+            // provide passphrase for the second, and check that the first is never asked for!
+            PgpDecryptVerify.Builder b = builderWithFakePassphraseCache(data, out,
+                    mKeyPhrase2, mStaticRing2.getMasterKeyId(), null);
+            b.setAllowedKeyIds(allowed);
+
+            DecryptVerifyResult result = b.build().execute();
+            Assert.assertTrue("decryption with cached passphrase must succeed for the first key", result.success());
+            Assert.assertArrayEquals("decrypted ciphertext with cached passphrase  should equal plaintext",
+                    out.toByteArray(), plaintext.getBytes());
+            Assert.assertNull("signature should be empty", result.getSignatureResult());
         }
+
+        { // decryption with passphrase cached should succeed for the other key if first is gone
+
+            // delete first key from database
+            new ProviderHelper(Robolectric.application).getContentResolver().delete(
+                    KeyRingData.buildPublicKeyRingUri(mStaticRing1.getMasterKeyId()), null, null
+            );
+
+            ByteArrayOutputStream out = new ByteArrayOutputStream();
+            ByteArrayInputStream in = new ByteArrayInputStream(ciphertext);
+            InputData data = new InputData(in, in.available());
+
+            PgpDecryptVerify.Builder b = builderWithFakePassphraseCache(data, out,
+                    mKeyPhrase2, mStaticRing2.getMasterKeyId(), null);
+
+            DecryptVerifyResult result = b.build().execute();
+            Assert.assertTrue("decryption with cached passphrase must succeed", result.success());
+            Assert.assertArrayEquals("decrypted ciphertext with cached passphrase  should equal plaintext",
+                    out.toByteArray(), plaintext.getBytes());
+            Assert.assertNull("signature should be empty", result.getSignatureResult());
+        }
+
+    }
+
+    @Test
+    public void testMultiAsymmetricSignEncryptDecryptVerify() {
+
+        String plaintext = "dies ist ein plaintext ☭" + TestingUtils.genPassphrase(true);
+        byte[] ciphertext;
+
+        { // encrypt data with a given passphrase
+            ByteArrayOutputStream out = new ByteArrayOutputStream();
+            ByteArrayInputStream in = new ByteArrayInputStream(plaintext.getBytes());
+
+            InputData data = new InputData(in, in.available());
+            Builder b = new PgpSignEncrypt.Builder(
+                    Robolectric.application,
+                    new ProviderHelper(Robolectric.application),
+                    null, // new DummyPassphraseCache(mPassphrase, 0L),
+                    data, out);
+
+            b.setEncryptionMasterKeyIds(new long[] {
+                    mStaticRing1.getMasterKeyId(),
+                    mStaticRing2.getMasterKeyId()
+            });
+            b.setSignatureMasterKeyId(mStaticRing1.getMasterKeyId());
+            b.setSignatureSubKeyId(KeyringTestingHelper.getSubkeyId(mStaticRing1, 1));
+            b.setSignaturePassphrase(mKeyPhrase1);
+            b.setSymmetricEncryptionAlgorithm(PGPEncryptedData.AES_128);
+            SignEncryptResult result = b.build().execute();
+            Assert.assertTrue("encryption must succeed", result.success());
+
+            ciphertext = out.toByteArray();
+        }
+
+        { // decryption with passphrase cached should succeed for the first key
+
+            ByteArrayOutputStream out = new ByteArrayOutputStream();
+            ByteArrayInputStream in = new ByteArrayInputStream(ciphertext);
+            InputData data = new InputData(in, in.available());
+
+            PgpDecryptVerify.Builder b = builderWithFakePassphraseCache(data, out,
+                    mKeyPhrase1, mStaticRing1.getMasterKeyId(), null);
+
+            DecryptVerifyResult result = b.build().execute();
+            Assert.assertTrue("decryption with cached passphrase must succeed for the first key", result.success());
+            Assert.assertArrayEquals("decrypted ciphertext with cached passphrase  should equal plaintext",
+                    out.toByteArray(), plaintext.getBytes());
+            Assert.assertEquals("signature should be verified and certified",
+                    OpenPgpSignatureResult.SIGNATURE_SUCCESS_CERTIFIED, result.getSignatureResult().getStatus());
+        }
+
+        { // decryption with passphrase cached should succeed for the other key if first is gone
+
+            // delete first key from database
+            new ProviderHelper(Robolectric.application).getContentResolver().delete(
+                    KeyRingData.buildPublicKeyRingUri(mStaticRing1.getMasterKeyId()), null, null
+            );
+
+            ByteArrayOutputStream out = new ByteArrayOutputStream();
+            ByteArrayInputStream in = new ByteArrayInputStream(ciphertext);
+            InputData data = new InputData(in, in.available());
+
+            PgpDecryptVerify.Builder b = builderWithFakePassphraseCache(data, out,
+                    mKeyPhrase2, mStaticRing2.getMasterKeyId(), null);
+
+            DecryptVerifyResult result = b.build().execute();
+            Assert.assertTrue("decryption with cached passphrase must succeed", result.success());
+            Assert.assertArrayEquals("decrypted ciphertext with cached passphrase  should equal plaintext",
+                    out.toByteArray(), plaintext.getBytes());
+            Assert.assertEquals("signature key should be missing",
+                    OpenPgpSignatureResult.SIGNATURE_KEY_MISSING,
+                    result.getSignatureResult().getStatus());
+        }
+
+    }
+
+    private PgpDecryptVerify.Builder builderWithFakePassphraseCache (
+            InputData data, OutputStream out,
+            final String passphrase, final Long checkMasterKeyId, final Long checkSubKeyId) {
+
+        return new PgpDecryptVerify.Builder(Robolectric.application,
+                new ProviderHelper(Robolectric.application),
+                null,
+                data, out) {
+            public PgpDecryptVerify build() {
+                return new PgpDecryptVerify(this) {
+                    @Override
+                    public String getCachedPassphrase(long masterKeyId, long subKeyId)
+                            throws NoSecretKeyException {
+                        if (checkMasterKeyId != null) {
+                            Assert.assertEquals("requested passphrase should be for expected master key id",
+                                    (long) checkMasterKeyId, masterKeyId);
+                        }
+                        if (checkSubKeyId != null) {
+                            Assert.assertEquals("requested passphrase should be for expected sub key id",
+                                    (long) checkSubKeyId, subKeyId);
+                        }
+                        if (passphrase == null) {
+                            return null;
+                        }
+                        return passphrase;
+                    }
+                };
+            }
+        };
     }
 
 }
diff --git a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperationTest.java b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperationTest.java
index 79778c6d0..52115a76d 100644
--- a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperationTest.java
+++ b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperationTest.java
@@ -40,9 +40,11 @@ import org.spongycastle.jce.provider.BouncyCastleProvider;
 import org.spongycastle.openpgp.PGPSignature;
 import org.sufficientlysecure.keychain.operations.results.OperationResult.LogType;
 import org.sufficientlysecure.keychain.operations.results.OperationResult.OperationLog;
-import org.sufficientlysecure.keychain.operations.results.EditKeyResult;
+import org.sufficientlysecure.keychain.operations.results.PgpEditKeyResult;
+import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKey.SecretKeyType;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel.Algorithm;
+import org.sufficientlysecure.keychain.service.SaveKeyringParcel.ChangeUnlockParcel;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel.SubkeyAdd;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel.SubkeyChange;
 import org.sufficientlysecure.keychain.support.KeyringBuilder;
@@ -91,14 +93,15 @@ public class PgpKeyOperationTest {
 
         parcel.mAddUserIds.add("twi");
         parcel.mAddUserIds.add("pink");
-        parcel.mNewUnlock = passphrase;
+        parcel.mNewUnlock = new ChangeUnlockParcel(passphrase);
         PgpKeyOperation op = new PgpKeyOperation(null);
 
-        EditKeyResult result = op.createSecretKeyRing(parcel);
+        PgpEditKeyResult result = op.createSecretKeyRing(parcel);
         Assert.assertTrue("initial test key creation must succeed", result.success());
         Assert.assertNotNull("initial test key creation must succeed", result.getRing());
 
         staticRing = result.getRing();
+        staticRing = staticRing.canonicalize(new OperationLog(), 0).getUncachedKeyRing();
 
         // we sleep here for a second, to make sure all new certificates have different timestamps
         Thread.sleep(1000);
@@ -127,7 +130,7 @@ public class PgpKeyOperationTest {
             parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
                     Algorithm.RSA, new Random().nextInt(256)+255, null, KeyFlags.CERTIFY_OTHER, 0L));
             parcel.mAddUserIds.add("shy");
-            parcel.mNewUnlock = passphrase;
+            parcel.mNewUnlock = new ChangeUnlockParcel(passphrase);
 
             assertFailure("creating ring with < 512 bytes keysize should fail", parcel,
                     LogType.MSG_CR_ERROR_KEYSIZE_512);
@@ -138,7 +141,7 @@ public class PgpKeyOperationTest {
             parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
                     Algorithm.ELGAMAL, 1024, null, KeyFlags.CERTIFY_OTHER, 0L));
             parcel.mAddUserIds.add("shy");
-            parcel.mNewUnlock = passphrase;
+            parcel.mNewUnlock = new ChangeUnlockParcel(passphrase);
 
             assertFailure("creating ring with ElGamal master key should fail", parcel,
                     LogType.MSG_CR_ERROR_FLAGS_ELGAMAL);
@@ -149,7 +152,7 @@ public class PgpKeyOperationTest {
             parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
                     Algorithm.RSA, 1024, null, KeyFlags.CERTIFY_OTHER, null));
             parcel.mAddUserIds.add("lotus");
-            parcel.mNewUnlock = passphrase;
+            parcel.mNewUnlock = new ChangeUnlockParcel(passphrase);
 
             assertFailure("creating master key with null expiry should fail", parcel,
                     LogType.MSG_CR_ERROR_NULL_EXPIRY);
@@ -160,7 +163,7 @@ public class PgpKeyOperationTest {
             parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
                     Algorithm.RSA, 1024, null, KeyFlags.SIGN_DATA, 0L));
             parcel.mAddUserIds.add("shy");
-            parcel.mNewUnlock = passphrase;
+            parcel.mNewUnlock = new ChangeUnlockParcel(passphrase);
 
             assertFailure("creating ring with non-certifying master key should fail", parcel,
                     LogType.MSG_CR_ERROR_NO_CERTIFY);
@@ -170,7 +173,7 @@ public class PgpKeyOperationTest {
             parcel.reset();
             parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
                     Algorithm.RSA, 1024, null, KeyFlags.CERTIFY_OTHER, 0L));
-            parcel.mNewUnlock = passphrase;
+            parcel.mNewUnlock = new ChangeUnlockParcel(passphrase);
 
             assertFailure("creating ring without user ids should fail", parcel,
                     LogType.MSG_CR_ERROR_NO_USER_ID);
@@ -179,7 +182,7 @@ public class PgpKeyOperationTest {
         {
             parcel.reset();
             parcel.mAddUserIds.add("shy");
-            parcel.mNewUnlock = passphrase;
+            parcel.mNewUnlock = new ChangeUnlockParcel(passphrase);
 
             assertFailure("creating ring with no master key should fail", parcel,
                     LogType.MSG_CR_ERROR_NO_MASTER);
@@ -910,8 +913,10 @@ public class PgpKeyOperationTest {
     public void testPassphraseChange() throws Exception {
 
         // change passphrase to empty
-        parcel.mNewUnlock = "";
-        UncachedKeyRing modified = applyModificationWithChecks(parcel, ring, onlyA, onlyB);
+        parcel.mNewUnlock = new ChangeUnlockParcel("");
+        // note that canonicalization here necessarily strips the empty notation packet
+        UncachedKeyRing modified = applyModificationWithChecks(parcel, ring, onlyA, onlyB,
+                passphrase);
 
         Assert.assertEquals("exactly three packets should have been modified (the secret keys)",
                 3, onlyB.size());
@@ -923,7 +928,7 @@ public class PgpKeyOperationTest {
 
         // modify keyring, change to non-empty passphrase
         String otherPassphrase = TestingUtils.genPassphrase(true);
-        parcel.mNewUnlock = otherPassphrase;
+        parcel.mNewUnlock = new ChangeUnlockParcel(otherPassphrase);
         modified = applyModificationWithChecks(parcel, modified, onlyA, onlyB, "");
 
         Assert.assertEquals("exactly three packets should have been modified (the secret keys)",
@@ -948,7 +953,7 @@ public class PgpKeyOperationTest {
                 PacketTags.SECRET_SUBKEY, sKeyNoPassphrase.tag);
 
         String otherPassphrase2 = TestingUtils.genPassphrase(true);
-        parcel.mNewUnlock = otherPassphrase2;
+        parcel.mNewUnlock = new ChangeUnlockParcel(otherPassphrase2);
         {
             // if we replace a secret key with one without passphrase
             modified = KeyringTestingHelper.removePacket(modified, sKeyNoPassphrase.position);
@@ -957,7 +962,7 @@ public class PgpKeyOperationTest {
             // we should still be able to modify it (and change its passphrase) without errors
             PgpKeyOperation op = new PgpKeyOperation(null);
             CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(modified.getEncoded(), false, 0);
-            EditKeyResult result = op.modifySecretKeyRing(secretRing, parcel, otherPassphrase);
+            PgpEditKeyResult result = op.modifySecretKeyRing(secretRing, parcel, otherPassphrase);
             Assert.assertTrue("key modification must succeed", result.success());
             Assert.assertFalse("log must not contain a warning",
                     result.getLog().containsWarnings());
@@ -973,7 +978,7 @@ public class PgpKeyOperationTest {
 
             PgpKeyOperation op = new PgpKeyOperation(null);
             CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(modified.getEncoded(), false, 0);
-            EditKeyResult result = op.modifySecretKeyRing(secretRing, parcel, otherPassphrase2);
+            PgpEditKeyResult result = op.modifySecretKeyRing(secretRing, parcel, otherPassphrase2);
             Assert.assertTrue("key modification must succeed", result.success());
             Assert.assertTrue("log must contain a failed passphrase change warning",
                     result.getLog().containsType(LogType.MSG_MF_PASSPHRASE_FAIL));
@@ -981,6 +986,46 @@ public class PgpKeyOperationTest {
 
     }
 
+    @Test
+    public void testUnlockPin() throws Exception {
+
+        String pin = "5235125";
+
+        // change passphrase to a pin type
+        parcel.mNewUnlock = new ChangeUnlockParcel(null, pin);
+        UncachedKeyRing modified = applyModificationWithChecks(parcel, ring, onlyA, onlyB);
+
+        Assert.assertEquals("exactly three packets should have been added (the secret keys + notation packet)",
+                3, onlyA.size());
+        Assert.assertEquals("exactly four packets should have been added (the secret keys + notation packet)",
+                4, onlyB.size());
+
+        RawPacket dkSig = onlyB.get(1);
+        Assert.assertEquals("second modified packet should be notation data",
+                PacketTags.SIGNATURE, dkSig.tag);
+
+        // check that notation data contains pin
+        CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(
+                modified.getEncoded(), false, 0);
+        Assert.assertEquals("secret key type should be 'pin' after this",
+                SecretKeyType.PIN,
+                secretRing.getSecretKey().getSecretKeyType());
+
+        // need to sleep for a sec, so the timestamp changes for notation data
+        Thread.sleep(1000);
+
+        {
+            parcel.mNewUnlock = new ChangeUnlockParcel("phrayse", null);
+            applyModificationWithChecks(parcel, modified, onlyA, onlyB, pin, true, false);
+
+            Assert.assertEquals("exactly four packets should have been removed (the secret keys + notation packet)",
+                    4, onlyA.size());
+            Assert.assertEquals("exactly three packets should have been added (no more notation packet)",
+                    3, onlyB.size());
+        }
+
+    }
+
     private static UncachedKeyRing applyModificationWithChecks(SaveKeyringParcel parcel,
                                                                UncachedKeyRing ring,
                                                                ArrayList<RawPacket> onlyA,
@@ -1011,7 +1056,7 @@ public class PgpKeyOperationTest {
             CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ring.getEncoded(), false, 0);
 
             PgpKeyOperation op = new PgpKeyOperation(null);
-            EditKeyResult result = op.modifySecretKeyRing(secretRing, parcel, passphrase);
+            PgpEditKeyResult result = op.modifySecretKeyRing(secretRing, parcel, passphrase);
             Assert.assertTrue("key modification must succeed", result.success());
             UncachedKeyRing rawModified = result.getRing();
             Assert.assertNotNull("key modification must not return null", rawModified);
@@ -1068,7 +1113,7 @@ public class PgpKeyOperationTest {
 
     private void assertFailure(String reason, SaveKeyringParcel parcel, LogType expected) {
 
-        EditKeyResult result = op.createSecretKeyRing(parcel);
+        PgpEditKeyResult result = op.createSecretKeyRing(parcel);
 
         Assert.assertFalse(reason, result.success());
         Assert.assertNull(reason, result.getRing());
@@ -1082,7 +1127,7 @@ public class PgpKeyOperationTest {
             throws Exception {
 
         CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ring.getEncoded(), false, 0);
-        EditKeyResult result = op.modifySecretKeyRing(secretRing, parcel, passphrase);
+        PgpEditKeyResult result = op.modifySecretKeyRing(secretRing, parcel, passphrase);
 
         Assert.assertFalse(reason, result.success());
         Assert.assertNull(reason, result.getRing());
@@ -1096,7 +1141,7 @@ public class PgpKeyOperationTest {
             throws Exception {
 
         CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ring.getEncoded(), false, 0);
-        EditKeyResult result = op.modifySecretKeyRing(secretRing, parcel, passphrase);
+        PgpEditKeyResult result = op.modifySecretKeyRing(secretRing, parcel, passphrase);
 
         Assert.assertFalse(reason, result.success());
         Assert.assertNull(reason, result.getRing());
@@ -1107,7 +1152,7 @@ public class PgpKeyOperationTest {
 
     private UncachedKeyRing assertCreateSuccess(String reason, SaveKeyringParcel parcel) {
 
-        EditKeyResult result = op.createSecretKeyRing(parcel);
+        PgpEditKeyResult result = op.createSecretKeyRing(parcel);
 
         Assert.assertTrue(reason, result.success());
         Assert.assertNotNull(reason, result.getRing());
diff --git a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringCanonicalizeTest.java b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringCanonicalizeTest.java
index cbdbd0e72..721d1a51d 100644
--- a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringCanonicalizeTest.java
+++ b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringCanonicalizeTest.java
@@ -53,11 +53,12 @@ import org.spongycastle.openpgp.operator.jcajce.JcePBESecretKeyEncryptorBuilder;
 import org.spongycastle.util.Strings;
 import org.sufficientlysecure.keychain.Constants;
 import org.sufficientlysecure.keychain.operations.results.OperationResult;
-import org.sufficientlysecure.keychain.operations.results.EditKeyResult;
+import org.sufficientlysecure.keychain.operations.results.PgpEditKeyResult;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel.Algorithm;
 import org.sufficientlysecure.keychain.operations.results.OperationResult.LogType;
 import org.sufficientlysecure.keychain.operations.results.OperationResult.OperationLog;
+import org.sufficientlysecure.keychain.service.SaveKeyringParcel.ChangeUnlockParcel;
 import org.sufficientlysecure.keychain.support.KeyringTestingHelper;
 import org.sufficientlysecure.keychain.support.KeyringTestingHelper.RawPacket;
 
@@ -104,14 +105,16 @@ public class UncachedKeyringCanonicalizeTest {
         parcel.mAddUserIds.add("twi");
         parcel.mAddUserIds.add("pink");
         // passphrase is tested in PgpKeyOperationTest, just use empty here
-        parcel.mNewUnlock = "";
+        parcel.mNewUnlock = new ChangeUnlockParcel("");
         PgpKeyOperation op = new PgpKeyOperation(null);
 
-        EditKeyResult result = op.createSecretKeyRing(parcel);
+        PgpEditKeyResult result = op.createSecretKeyRing(parcel);
         Assert.assertTrue("initial test key creation must succeed", result.success());
         staticRing = result.getRing();
         Assert.assertNotNull("initial test key creation must succeed", staticRing);
 
+        staticRing = staticRing.canonicalize(new OperationLog(), 0).getUncachedKeyRing();
+
         // just for later reference
         totalPackets = 9;
 
diff --git a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringMergeTest.java b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringMergeTest.java
index 006c77fbc..7f6f480d4 100644
--- a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringMergeTest.java
+++ b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringMergeTest.java
@@ -33,9 +33,11 @@ import org.spongycastle.bcpg.sig.KeyFlags;
 import org.spongycastle.jce.provider.BouncyCastleProvider;
 import org.spongycastle.util.Strings;
 import org.sufficientlysecure.keychain.operations.results.OperationResult;
-import org.sufficientlysecure.keychain.operations.results.EditKeyResult;
+import org.sufficientlysecure.keychain.operations.results.PgpEditKeyResult;
+import org.sufficientlysecure.keychain.operations.results.OperationResult.OperationLog;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel.Algorithm;
+import org.sufficientlysecure.keychain.service.SaveKeyringParcel.ChangeUnlockParcel;
 import org.sufficientlysecure.keychain.support.KeyringTestingHelper;
 import org.sufficientlysecure.keychain.support.KeyringTestingHelper.RawPacket;
 import org.sufficientlysecure.keychain.util.ProgressScaler;
@@ -96,13 +98,14 @@ public class UncachedKeyringMergeTest {
             parcel.mAddUserIds.add("twi");
             parcel.mAddUserIds.add("pink");
             // passphrase is tested in PgpKeyOperationTest, just use empty here
-            parcel.mNewUnlock = "";
+            parcel.mNewUnlock = new ChangeUnlockParcel("");
             PgpKeyOperation op = new PgpKeyOperation(null);
 
             OperationResult.OperationLog log = new OperationResult.OperationLog();
 
-            EditKeyResult result = op.createSecretKeyRing(parcel);
+            PgpEditKeyResult result = op.createSecretKeyRing(parcel);
             staticRingA = result.getRing();
+            staticRingA = staticRingA.canonicalize(new OperationLog(), 0).getUncachedKeyRing();
         }
 
         {
@@ -112,12 +115,13 @@ public class UncachedKeyringMergeTest {
 
             parcel.mAddUserIds.add("shy");
             // passphrase is tested in PgpKeyOperationTest, just use empty here
-            parcel.mNewUnlock = "";
+            parcel.mNewUnlock = new ChangeUnlockParcel("");
             PgpKeyOperation op = new PgpKeyOperation(null);
 
             OperationResult.OperationLog log = new OperationResult.OperationLog();
-            EditKeyResult result = op.createSecretKeyRing(parcel);
+            PgpEditKeyResult result = op.createSecretKeyRing(parcel);
             staticRingB = result.getRing();
+            staticRingB = staticRingB.canonicalize(new OperationLog(), 0).getUncachedKeyRing();
         }
 
         Assert.assertNotNull("initial test key creation must succeed", staticRingA);
diff --git a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringTest.java b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringTest.java
index d8d825783..a3c58a5c8 100644
--- a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringTest.java
+++ b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringTest.java
@@ -26,13 +26,16 @@ import org.junit.runner.RunWith;
 import org.robolectric.RobolectricTestRunner;
 import org.robolectric.shadows.ShadowLog;
 import org.spongycastle.bcpg.sig.KeyFlags;
-import org.sufficientlysecure.keychain.operations.results.EditKeyResult;
+import org.sufficientlysecure.keychain.operations.results.PgpEditKeyResult;
+import org.sufficientlysecure.keychain.pgp.UncachedKeyRing.IteratorWithIOThrow;
 import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel.Algorithm;
+import org.sufficientlysecure.keychain.service.SaveKeyringParcel.ChangeUnlockParcel;
 
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
+import java.io.IOException;
 import java.util.Iterator;
 
 @RunWith(RobolectricTestRunner.class)
@@ -57,10 +60,10 @@ public class UncachedKeyringTest {
         parcel.mAddUserIds.add("twi");
         parcel.mAddUserIds.add("pink");
         // passphrase is tested in PgpKeyOperationTest, just use empty here
-        parcel.mNewUnlock = "";
+        parcel.mNewUnlock = new ChangeUnlockParcel("");
         PgpKeyOperation op = new PgpKeyOperation(null);
 
-        EditKeyResult result = op.createSecretKeyRing(parcel);
+        PgpEditKeyResult result = op.createSecretKeyRing(parcel);
         staticRing = result.getRing();
         staticPubRing = staticRing.extractPublicKeyRing();
 
@@ -108,7 +111,7 @@ public class UncachedKeyringTest {
         ring.encodeArmored(out, "OpenKeychain");
         pubRing.encodeArmored(out, "OpenKeychain");
 
-        Iterator<UncachedKeyRing> it =
+        IteratorWithIOThrow<UncachedKeyRing> it =
                 UncachedKeyRing.fromStream(new ByteArrayInputStream(out.toByteArray()));
         Assert.assertTrue("there should be two rings in the stream", it.hasNext());
         Assert.assertArrayEquals("first ring should be the first we put in",
@@ -128,4 +131,16 @@ public class UncachedKeyringTest {
         pubRing.extractPublicKeyRing();
     }
 
+    @Test(expected = IOException.class)
+    public void testBrokenVersionCert() throws Throwable {
+        // this is a test for one of the patches we use on top of stock bouncycastle, which
+        // returns an IOException rather than a RuntimeException in case of a bad certificate
+        // version byte
+        readRingFromResource("/test-keys/broken_cert_version.asc");
+    }
+
+    UncachedKeyRing readRingFromResource(String name) throws Throwable {
+        return UncachedKeyRing.fromStream(UncachedKeyringTest.class.getResourceAsStream(name)).next();
+    }
+
 }
diff --git a/OpenKeychain-Test/src/test/resources/test-keys/broken_cert_version.asc b/OpenKeychain-Test/src/test/resources/test-keys/broken_cert_version.asc
new file mode 100644
index 000000000..e2d2abd8e
--- /dev/null
+++ b/OpenKeychain-Test/src/test/resources/test-keys/broken_cert_version.asc
@@ -0,0 +1,17 @@
+-----BEGIN PGP PUBLIC KEY BLOCK-----
+
+mQENBFSl5DIBCADqsGJJ8RhV4Uw6a3Q48QyTMrUtvZquOTlLVaqOdEFZNL5/OBal
+prft9LNkcOkIVA89Hdn715WwkmG2OJiJoQ/ZAKwal1CPGm4Q8kZIM7k57ISJL6J5
+300e7UIznc74XbG7eFNxNcjCM9wG12vW2rFwc+ogJtkBSf0IXukPwtUkRK+H5ufO
+lpqS5NNZfiGbNQCrb+YsGZNRk4QTGR6WGyaIRHlcG8G00VPGNSauTqe/11MO9MoF
+BvPgFeur3nefWunCQ+uDmzIEs8r94gaHu3LWbctd5w5x/o/PDfTSSiO+U8zzXrKC
+4ZpEl5bk7t7jH1hYMLWyO6nn0vWTOMO1EYLBABEBAAG0GGJyb2tlbiBzaWduYXR1
+cmUgdmVyc2lvbokBOMATAQIAIgUCVKXkMgIbAwYLCQgHAwIGFQgCCQoLBBYCAwEC
+HgECF4AACgkQDe00lH/2SnprLggAh64TsdHDfIhTNc1DeJLCuvuHsitAcUdEEnue
+yJjodxboKNSplIwnmb5CpM3P8f736dNaW77Yd6aO4IeAy6cBlxT1tSRkJMsp+cBt
+kBa3lRr+GnWZlLZs3coL2g0t5RbuyYKyQxm2qvgFJGi/7Qfty5nJOW5U1ElT3VT8
+jISNdQdDAIaBsCE+TuyW3VsP3PqnJ7x14K7VhkFuCyvYB9paLcJBnan93R0Ja0Ip
+Cv1pbrNxXp0UELf0RYc2X5C1m6otZ9LKf3PmzxlEkApkb1TZUEBak2Za5p99koZT
++pg/XpZPyawi+gZeYkBAohxRGmzG/a4L+YacAZHbchfN0eG7lg==
+=mxTR
+-----END PGP PUBLIC KEY BLOCK-----