Merge branch 'master' into yubikey

Conflicts:
	.gitmodules
	OpenKeychain/src/main/java/org/sufficientlysecure/keychain/helper/Preferences.java
	OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedSecretKey.java
	OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java

7 files changed, 1077 insertions, 431 deletions

diff --git a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/support/KeyringTestingHelper.java b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/support/KeyringTestingHelper.java
index 3fa668e6e..015e134ea 100644
--- a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/support/KeyringTestingHelper.java
+++ b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/support/KeyringTestingHelper.java
@@ -29,6 +29,7 @@ import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
@@ -61,7 +62,7 @@ public class KeyringTestingHelper {
         boolean saveSuccess = saveKeyringResult.success();
 
         // Now re-retrieve the saved key. Should not throw an exception.
-        providerHelper.getWrappedPublicKeyRing(masterKeyId);
+        providerHelper.getCanonicalizedPublicKeyRing(masterKeyId);
 
         // A different ID should still fail
         retrieveKeyAndExpectNotFound(providerHelper, masterKeyId - 1);
@@ -331,13 +332,32 @@ public class KeyringTestingHelper {
 
     }
 
+    public static <E> E getNth(Iterator<E> it, int position) {
+        while(position-- > 0) {
+            it.next();
+        }
+        return it.next();
+    }
+
+    public static long getSubkeyId(UncachedKeyRing ring, int position) {
+        return getNth(ring.getPublicKeys(), position).getKeyId();
+    }
+
     private void retrieveKeyAndExpectNotFound(ProviderHelper providerHelper, long masterKeyId) {
         try {
-            providerHelper.getWrappedPublicKeyRing(masterKeyId);
+            providerHelper.getCanonicalizedPublicKeyRing(masterKeyId);
             throw new AssertionError("Was expecting the previous call to fail!");
         } catch (ProviderHelper.NotFoundException expectedException) {
             // good
         }
     }
 
+    public static <E> List<E> itToList(Iterator<E> it) {
+        List<E> result = new ArrayList<E>();
+        while(it.hasNext()) {
+            result.add(it.next());
+        }
+        return result;
+    }
+
 }
diff --git a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/support/ProviderHelperStub.java b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/support/ProviderHelperStub.java
index f06fe0072..2cd0c67a2 100644
--- a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/support/ProviderHelperStub.java
+++ b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/support/ProviderHelperStub.java
@@ -20,7 +20,7 @@ package org.sufficientlysecure.keychain.support;
 import android.content.Context;
 import android.net.Uri;
 
-import org.sufficientlysecure.keychain.pgp.WrappedPublicKeyRing;
+import org.sufficientlysecure.keychain.pgp.CanonicalizedPublicKeyRing;
 import org.sufficientlysecure.keychain.provider.ProviderHelper;
 
 /**
@@ -32,8 +32,8 @@ class ProviderHelperStub extends ProviderHelper {
     }
 
     @Override
-    public WrappedPublicKeyRing getWrappedPublicKeyRing(Uri id) throws NotFoundException {
+    public CanonicalizedPublicKeyRing getCanonicalizedPublicKeyRing(Uri id) throws NotFoundException {
         byte[] data = TestDataUtil.readFully(getClass().getResourceAsStream("/public-key-for-sample.blob"));
-        return new WrappedPublicKeyRing(data, false, 0);
+        return new CanonicalizedPublicKeyRing(data, 0);
     }
 }
diff --git a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/support/UncachedKeyringTestingHelper.java b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/support/UncachedKeyringTestingHelper.java
deleted file mode 100644
index 6467d3f32..000000000
--- a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/support/UncachedKeyringTestingHelper.java
+++ /dev/null
@@ -1,297 +0,0 @@
-/*
- * Copyright (C) Art O Cathain
- *
- * 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.support;
-
-import org.spongycastle.bcpg.BCPGKey;
-import org.spongycastle.bcpg.PublicKeyPacket;
-import org.spongycastle.bcpg.SignatureSubpacket;
-import org.spongycastle.openpgp.PGPException;
-import org.spongycastle.openpgp.PGPPublicKey;
-import org.spongycastle.openpgp.PGPSignature;
-import org.spongycastle.openpgp.PGPSignatureSubpacketVector;
-import org.spongycastle.openpgp.PGPUserAttributeSubpacketVector;
-import org.sufficientlysecure.keychain.pgp.UncachedKeyRing;
-import org.sufficientlysecure.keychain.pgp.UncachedPublicKey;
-import org.sufficientlysecure.keychain.service.OperationResultParcel;
-
-import java.util.Arrays;
-
-/**
- * Created by art on 28/06/14.
- */
-public class UncachedKeyringTestingHelper {
-
-    public static boolean compareRing(UncachedKeyRing keyRing1, UncachedKeyRing keyRing2) {
-        OperationResultParcel.OperationLog operationLog = new OperationResultParcel.OperationLog();
-        UncachedKeyRing canonicalized = keyRing1.canonicalize(operationLog, 0);
-
-        if (canonicalized == null) {
-            throw new AssertionError("Canonicalization failed; messages: [" + operationLog.toList() + "]");
-        }
-
-        return TestDataUtil.iterEquals(canonicalized.getPublicKeys(), keyRing2.getPublicKeys(), new
-                TestDataUtil.EqualityChecker<UncachedPublicKey>() {
-                    @Override
-                    public boolean areEquals(UncachedPublicKey lhs, UncachedPublicKey rhs) {
-                        return comparePublicKey(lhs, rhs);
-                    }
-                });
-    }
-
-    public static boolean comparePublicKey(UncachedPublicKey key1, UncachedPublicKey key2) {
-        boolean equal = true;
-
-        if (key1.canAuthenticate() != key2.canAuthenticate()) {
-            return false;
-        }
-        if (key1.canCertify() != key2.canCertify()) {
-            return false;
-        }
-        if (key1.canEncrypt() != key2.canEncrypt()) {
-            return false;
-        }
-        if (key1.canSign() != key2.canSign()) {
-            return false;
-        }
-        if (key1.getAlgorithm() != key2.getAlgorithm()) {
-            return false;
-        }
-        if (key1.getBitStrength() != key2.getBitStrength()) {
-            return false;
-        }
-        if (!TestDataUtil.equals(key1.getCreationTime(), key2.getCreationTime())) {
-            return false;
-        }
-        if (!TestDataUtil.equals(key1.getExpiryTime(), key2.getExpiryTime())) {
-            return false;
-        }
-        if (!Arrays.equals(key1.getFingerprint(), key2.getFingerprint())) {
-            return false;
-        }
-        if (key1.getKeyId() != key2.getKeyId()) {
-            return false;
-        }
-        if (key1.getKeyUsage() != key2.getKeyUsage()) {
-            return false;
-        }
-        if (!TestDataUtil.equals(key1.getPrimaryUserId(), key2.getPrimaryUserId())) {
-            return false;
-        }
-
-        // Ooops, getPublicKey is due to disappear. But then how to compare?
-        if (!keysAreEqual(key1.getPublicKey(), key2.getPublicKey())) {
-            return false;
-        }
-
-        return equal;
-    }
-
-    public static boolean keysAreEqual(PGPPublicKey a, PGPPublicKey b) {
-
-        if (a.getAlgorithm() != b.getAlgorithm()) {
-            return false;
-        }
-
-        if (a.getBitStrength() != b.getBitStrength()) {
-            return false;
-        }
-
-        if (!TestDataUtil.equals(a.getCreationTime(), b.getCreationTime())) {
-            return false;
-        }
-
-        if (!Arrays.equals(a.getFingerprint(), b.getFingerprint())) {
-            return false;
-        }
-
-        if (a.getKeyID() != b.getKeyID()) {
-            return false;
-        }
-
-        if (!pubKeyPacketsAreEqual(a.getPublicKeyPacket(), b.getPublicKeyPacket())) {
-            return false;
-        }
-
-        if (a.getVersion() != b.getVersion()) {
-            return false;
-        }
-
-        if (a.getValidDays() != b.getValidDays()) {
-            return false;
-        }
-
-        if (a.getValidSeconds() != b.getValidSeconds()) {
-            return false;
-        }
-
-        if (!Arrays.equals(a.getTrustData(), b.getTrustData())) {
-            return false;
-        }
-
-        if (!TestDataUtil.iterEquals(a.getUserIDs(), b.getUserIDs())) {
-            return false;
-        }
-
-        if (!TestDataUtil.iterEquals(a.getUserAttributes(), b.getUserAttributes(),
-                new TestDataUtil.EqualityChecker<PGPUserAttributeSubpacketVector>() {
-                    public boolean areEquals(PGPUserAttributeSubpacketVector lhs, PGPUserAttributeSubpacketVector rhs) {
-                        // For once, BC defines equals, so we use it implicitly.
-                        return TestDataUtil.equals(lhs, rhs);
-                    }
-                }
-        )) {
-            return false;
-        }
-
-
-        if (!TestDataUtil.iterEquals(a.getSignatures(), b.getSignatures(),
-                new TestDataUtil.EqualityChecker<PGPSignature>() {
-                    public boolean areEquals(PGPSignature lhs, PGPSignature rhs) {
-                        return signaturesAreEqual(lhs, rhs);
-                    }
-                }
-        )) {
-            return false;
-        }
-
-        return true;
-    }
-
-    public static boolean signaturesAreEqual(PGPSignature a, PGPSignature b) {
-
-        if (a.getVersion() != b.getVersion()) {
-            return false;
-        }
-
-        if (a.getKeyAlgorithm() != b.getKeyAlgorithm()) {
-            return false;
-        }
-
-        if (a.getHashAlgorithm() != b.getHashAlgorithm()) {
-            return false;
-        }
-
-        if (a.getSignatureType() != b.getSignatureType()) {
-            return false;
-        }
-
-        try {
-            if (!Arrays.equals(a.getSignature(), b.getSignature())) {
-                return false;
-            }
-        } catch (PGPException ex) {
-            throw new RuntimeException(ex);
-        }
-
-        if (a.getKeyID() != b.getKeyID()) {
-            return false;
-        }
-
-        if (!TestDataUtil.equals(a.getCreationTime(), b.getCreationTime())) {
-            return false;
-        }
-
-        if (!Arrays.equals(a.getSignatureTrailer(), b.getSignatureTrailer())) {
-            return false;
-        }
-
-        if (!subPacketVectorsAreEqual(a.getHashedSubPackets(), b.getHashedSubPackets())) {
-            return false;
-        }
-
-        if (!subPacketVectorsAreEqual(a.getUnhashedSubPackets(), b.getUnhashedSubPackets())) {
-            return false;
-        }
-
-        return true;
-    }
-
-    private static boolean subPacketVectorsAreEqual(PGPSignatureSubpacketVector aHashedSubPackets, PGPSignatureSubpacketVector bHashedSubPackets) {
-        for (int i = 0; i < Byte.MAX_VALUE; i++) {
-            if (!TestDataUtil.iterEquals(Arrays.asList(aHashedSubPackets.getSubpackets(i)).iterator(),
-                    Arrays.asList(bHashedSubPackets.getSubpackets(i)).iterator(),
-                    new TestDataUtil.EqualityChecker<SignatureSubpacket>() {
-                        @Override
-                        public boolean areEquals(SignatureSubpacket lhs, SignatureSubpacket rhs) {
-                            return signatureSubpacketsAreEqual(lhs, rhs);
-                        }
-                    }
-            )) {
-                return false;
-            }
-
-        }
-        return true;
-    }
-
-    private static boolean signatureSubpacketsAreEqual(SignatureSubpacket lhs, SignatureSubpacket rhs) {
-        if (lhs.getType() != rhs.getType()) {
-            return false;
-        }
-        if (!Arrays.equals(lhs.getData(), rhs.getData())) {
-            return false;
-        }
-        return true;
-    }
-
-    public static boolean pubKeyPacketsAreEqual(PublicKeyPacket a, PublicKeyPacket b) {
-
-        if (a.getAlgorithm() != b.getAlgorithm()) {
-            return false;
-        }
-
-        if (!bcpgKeysAreEqual(a.getKey(), b.getKey())) {
-            return false;
-        }
-
-        if (!TestDataUtil.equals(a.getTime(), b.getTime())) {
-            return false;
-        }
-
-        if (a.getValidDays() != b.getValidDays()) {
-            return false;
-        }
-
-        if (a.getVersion() != b.getVersion()) {
-            return false;
-        }
-
-        return true;
-    }
-
-    public static boolean bcpgKeysAreEqual(BCPGKey a, BCPGKey b) {
-
-        if (!TestDataUtil.equals(a.getFormat(), b.getFormat())) {
-            return false;
-        }
-
-        if (!Arrays.equals(a.getEncoded(), b.getEncoded())) {
-            return false;
-        }
-
-        return true;
-    }
-
-
-    public void doTestCanonicalize(UncachedKeyRing inputKeyRing, UncachedKeyRing expectedKeyRing) {
-        if (!compareRing(inputKeyRing, expectedKeyRing)) {
-            throw new AssertionError("Expected [" + inputKeyRing + "] to match [" + expectedKeyRing + "]");
-        }
-    }
-
-}
diff --git a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/PgpKeyOperationTest.java b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/PgpKeyOperationTest.java
index 5c6072c25..4f6694049 100644
--- a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/PgpKeyOperationTest.java
+++ b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/PgpKeyOperationTest.java
@@ -19,12 +19,13 @@ import org.spongycastle.bcpg.sig.KeyFlags;
 import org.spongycastle.openpgp.PGPSignature;
 import org.sufficientlysecure.keychain.Constants;
 import org.sufficientlysecure.keychain.Constants.choice.algorithm;
+import org.sufficientlysecure.keychain.pgp.CanonicalizedKeyRing;
+import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKeyRing;
 import org.sufficientlysecure.keychain.pgp.PgpKeyOperation;
 import org.sufficientlysecure.keychain.pgp.UncachedKeyRing;
 import org.sufficientlysecure.keychain.pgp.UncachedPublicKey;
-import org.sufficientlysecure.keychain.pgp.WrappedSecretKeyRing;
 import org.sufficientlysecure.keychain.pgp.WrappedSignature;
-import org.sufficientlysecure.keychain.service.OperationResultParcel;
+import org.sufficientlysecure.keychain.service.OperationResultParcel.OperationLog;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel.SubkeyAdd;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel.SubkeyChange;
@@ -82,8 +83,7 @@ public class PgpKeyOperationTest {
         parcel.mNewPassphrase = passphrase;
         PgpKeyOperation op = new PgpKeyOperation(null);
 
-        OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
-        staticRing = op.createSecretKeyRing(parcel, log, 0);
+        staticRing = op.createSecretKeyRing(parcel).getRing();
 
         Assert.assertNotNull("initial test key creation must succeed", staticRing);
 
@@ -107,9 +107,7 @@ public class PgpKeyOperationTest {
     }
 
     @Test
-    public void testAlgorithmChoice() {
-
-        OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
+    public void createSecretKeyRingTests() {
 
         {
             parcel.reset();
@@ -118,7 +116,7 @@ public class PgpKeyOperationTest {
             parcel.mAddUserIds.add("shy");
             parcel.mNewPassphrase = passphrase;
 
-            UncachedKeyRing ring = op.createSecretKeyRing(parcel, log, 0);
+            UncachedKeyRing ring = op.createSecretKeyRing(parcel).getRing();
 
             Assert.assertNull("creating ring with < 512 bytes keysize should fail", ring);
         }
@@ -130,7 +128,7 @@ public class PgpKeyOperationTest {
             parcel.mAddUserIds.add("shy");
             parcel.mNewPassphrase = passphrase;
 
-            UncachedKeyRing ring = op.createSecretKeyRing(parcel, log, 0);
+            UncachedKeyRing ring = op.createSecretKeyRing(parcel).getRing();
 
             Assert.assertNull("creating ring with ElGamal master key should fail", ring);
         }
@@ -142,7 +140,7 @@ public class PgpKeyOperationTest {
             parcel.mAddUserIds.add("shy");
             parcel.mNewPassphrase = passphrase;
 
-            UncachedKeyRing ring = op.createSecretKeyRing(parcel, log, 0);
+            UncachedKeyRing ring = op.createSecretKeyRing(parcel).getRing();
             Assert.assertNull("creating ring with bad algorithm choice should fail", ring);
         }
 
@@ -153,7 +151,7 @@ public class PgpKeyOperationTest {
             parcel.mAddUserIds.add("shy");
             parcel.mNewPassphrase = passphrase;
 
-            UncachedKeyRing ring = op.createSecretKeyRing(parcel, log, 0);
+            UncachedKeyRing ring = op.createSecretKeyRing(parcel).getRing();
             Assert.assertNull("creating ring with non-certifying master key should fail", ring);
         }
 
@@ -163,7 +161,7 @@ public class PgpKeyOperationTest {
                     Constants.choice.algorithm.rsa, 1024, KeyFlags.CERTIFY_OTHER, null));
             parcel.mNewPassphrase = passphrase;
 
-            UncachedKeyRing ring = op.createSecretKeyRing(parcel, log, 0);
+            UncachedKeyRing ring = op.createSecretKeyRing(parcel).getRing();
             Assert.assertNull("creating ring without user ids should fail", ring);
         }
 
@@ -172,7 +170,7 @@ public class PgpKeyOperationTest {
             parcel.mAddUserIds.add("shy");
             parcel.mNewPassphrase = passphrase;
 
-            UncachedKeyRing ring = op.createSecretKeyRing(parcel, log, 0);
+            UncachedKeyRing ring = op.createSecretKeyRing(parcel).getRing();
             Assert.assertNull("creating ring without subkeys should fail", ring);
         }
 
@@ -186,11 +184,10 @@ public class PgpKeyOperationTest {
         parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
                 Constants.choice.algorithm.rsa, 1024, KeyFlags.CERTIFY_OTHER | KeyFlags.SIGN_DATA, null));
         parcel.mAddUserIds.add("luna");
-        OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
-        ring = op.createSecretKeyRing(parcel, log, 0);
+        ring = op.createSecretKeyRing(parcel).getRing();
 
         Assert.assertEquals("the keyring should contain only the master key",
-                1, ring.getAvailableSubkeys().size());
+                1, KeyringTestingHelper.itToList(ring.getPublicKeys()).size());
         Assert.assertEquals("first (master) key must have both flags",
                 KeyFlags.CERTIFY_OTHER | KeyFlags.SIGN_DATA, ring.getPublicKey().getKeyUsage());
 
@@ -212,7 +209,7 @@ public class PgpKeyOperationTest {
                 2, ring.getPublicKey().getUnorderedUserIds().size());
 
         Assert.assertEquals("number of subkeys must be three",
-                3, ring.getAvailableSubkeys().size());
+                3, KeyringTestingHelper.itToList(ring.getPublicKeys()).size());
 
         Assert.assertTrue("key ring should have been created in the last 120 seconds",
                 ring.getPublicKey().getCreationTime().after(new Date(new Date().getTime()-1000*120)));
@@ -250,9 +247,8 @@ public class PgpKeyOperationTest {
             parcel.mMasterKeyId = ring.getMasterKeyId() -1;
             parcel.mFingerprint = ring.getFingerprint();
 
-            WrappedSecretKeyRing secretRing = new WrappedSecretKeyRing(ring.getEncoded(), false, 0);
-            OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
-            UncachedKeyRing modified = op.modifySecretKeyRing(secretRing, parcel, passphrase, log, 0);
+            CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ring.getEncoded(), false, 0);
+            UncachedKeyRing modified = op.modifySecretKeyRing(secretRing, parcel, passphrase).getRing();
 
             Assert.assertNull("keyring modification with bad master key id should fail", modified);
         }
@@ -263,9 +259,8 @@ public class PgpKeyOperationTest {
             parcel.mMasterKeyId = null;
             parcel.mFingerprint = ring.getFingerprint();
 
-            WrappedSecretKeyRing secretRing = new WrappedSecretKeyRing(ring.getEncoded(), false, 0);
-            OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
-            UncachedKeyRing modified = op.modifySecretKeyRing(secretRing, parcel, passphrase, log, 0);
+            CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ring.getEncoded(), false, 0);
+            UncachedKeyRing modified = op.modifySecretKeyRing(secretRing, parcel, passphrase).getRing();
 
             Assert.assertNull("keyring modification with null master key id should fail", modified);
         }
@@ -277,9 +272,8 @@ public class PgpKeyOperationTest {
             // some byte, off by one
             parcel.mFingerprint[5] += 1;
 
-            WrappedSecretKeyRing secretRing = new WrappedSecretKeyRing(ring.getEncoded(), false, 0);
-            OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
-            UncachedKeyRing modified = op.modifySecretKeyRing(secretRing, parcel, passphrase, log, 0);
+            CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ring.getEncoded(), false, 0);
+            UncachedKeyRing modified = op.modifySecretKeyRing(secretRing, parcel, passphrase).getRing();
 
             Assert.assertNull("keyring modification with bad fingerprint should fail", modified);
         }
@@ -289,17 +283,19 @@ public class PgpKeyOperationTest {
             parcel.mMasterKeyId = ring.getMasterKeyId();
             parcel.mFingerprint = null;
 
-            WrappedSecretKeyRing secretRing = new WrappedSecretKeyRing(ring.getEncoded(), false, 0);
-            OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
-            UncachedKeyRing modified = op.modifySecretKeyRing(secretRing, parcel, passphrase, log, 0);
+            CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ring.getEncoded(), false, 0);
+            UncachedKeyRing modified = op.modifySecretKeyRing(secretRing, parcel, passphrase).getRing();
 
             Assert.assertNull("keyring modification with null fingerprint should fail", modified);
         }
 
         {
-            WrappedSecretKeyRing secretRing = new WrappedSecretKeyRing(ring.getEncoded(), false, 0);
-            OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
-            UncachedKeyRing modified = op.modifySecretKeyRing(secretRing, parcel, "bad passphrase", log, 0);
+            String badphrase = "";
+            if (badphrase.equals(passphrase)) {
+                badphrase = "a";
+            }
+            CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ring.getEncoded(), false, 0);
+            UncachedKeyRing modified = op.modifySecretKeyRing(secretRing, parcel, badphrase).getRing();
 
             Assert.assertNull("keyring modification with bad passphrase should fail", modified);
         }
@@ -352,11 +348,11 @@ public class PgpKeyOperationTest {
 
         { // bad keysize should fail
             parcel.reset();
-            parcel.mAddSubKeys.add(new SubkeyAdd(algorithm.rsa, 77, KeyFlags.SIGN_DATA, null));
+            parcel.mAddSubKeys.add(new SubkeyAdd(
+                    algorithm.rsa, new Random().nextInt(512), KeyFlags.SIGN_DATA, null));
 
-            WrappedSecretKeyRing secretRing = new WrappedSecretKeyRing(ring.getEncoded(), false, 0);
-            OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
-            modified = op.modifySecretKeyRing(secretRing, parcel, passphrase, log, 0);
+            CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ring.getEncoded(), false, 0);
+            modified = op.modifySecretKeyRing(secretRing, parcel, passphrase).getRing();
 
             Assert.assertNull("creating a subkey with keysize < 512 should fail", modified);
         }
@@ -366,9 +362,8 @@ public class PgpKeyOperationTest {
             parcel.mAddSubKeys.add(new SubkeyAdd(algorithm.rsa, 1024, KeyFlags.SIGN_DATA,
                     new Date().getTime()/1000-10));
 
-            WrappedSecretKeyRing secretRing = new WrappedSecretKeyRing(ring.getEncoded(), false, 0);
-            OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
-            modified = op.modifySecretKeyRing(secretRing, parcel, passphrase, log, 0);
+            CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ring.getEncoded(), false, 0);
+            modified = op.modifySecretKeyRing(secretRing, parcel, passphrase).getRing();
 
             Assert.assertNull("creating subkey with past expiry date should fail", modified);
         }
@@ -379,12 +374,7 @@ public class PgpKeyOperationTest {
     public void testSubkeyModify() throws Exception {
 
         long expiry = new Date().getTime()/1000 + 1024;
-        long keyId;
-        {
-            Iterator<UncachedPublicKey> it = ring.getPublicKeys();
-            it.next();
-            keyId = it.next().getKeyId();
-        }
+        long keyId = KeyringTestingHelper.getSubkeyId(ring, 1);
 
         UncachedKeyRing modified = ring;
         {
@@ -440,9 +430,8 @@ public class PgpKeyOperationTest {
             parcel.reset();
             parcel.mChangeSubKeys.add(new SubkeyChange(keyId, null, new Date().getTime()/1000-10));
 
-            WrappedSecretKeyRing secretRing = new WrappedSecretKeyRing(ring.getEncoded(), false, 0);
-            OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
-            modified = op.modifySecretKeyRing(secretRing, parcel, passphrase, log, 0);
+            CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ring.getEncoded(), false, 0);
+            modified = op.modifySecretKeyRing(secretRing, parcel, passphrase).getRing();
 
             Assert.assertNull("setting subkey expiry to a past date should fail", modified);
         }
@@ -451,9 +440,8 @@ public class PgpKeyOperationTest {
             parcel.reset();
             parcel.mChangeSubKeys.add(new SubkeyChange(123, null, null));
 
-            WrappedSecretKeyRing secretRing = new WrappedSecretKeyRing(ring.getEncoded(), false, 0);
-            OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
-            modified = op.modifySecretKeyRing(secretRing, parcel, passphrase, log, 0);
+            CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ring.getEncoded(), false, 0);
+            modified = op.modifySecretKeyRing(secretRing, parcel, passphrase).getRing();
 
             Assert.assertNull("modifying non-existent subkey should fail", modified);
         }
@@ -463,13 +451,7 @@ public class PgpKeyOperationTest {
     @Test
     public void testSubkeyRevoke() throws Exception {
 
-        long keyId;
-        {
-            Iterator<UncachedPublicKey> it = ring.getPublicKeys();
-            it.next();
-            keyId = it.next().getKeyId();
-        }
-
+        long keyId = KeyringTestingHelper.getSubkeyId(ring, 1);
         int flags = ring.getPublicKey(keyId).getKeyUsage();
 
         UncachedKeyRing modified;
@@ -479,9 +461,8 @@ public class PgpKeyOperationTest {
             parcel.reset();
             parcel.mRevokeSubKeys.add(123L);
 
-            WrappedSecretKeyRing secretRing = new WrappedSecretKeyRing(ring.getEncoded(), false, 0);
-            OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
-            UncachedKeyRing otherModified = op.modifySecretKeyRing(secretRing, parcel, passphrase, log, 0);
+            CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ring.getEncoded(), false, 0);
+            UncachedKeyRing otherModified = op.modifySecretKeyRing(secretRing, parcel, passphrase).getRing();
 
             Assert.assertNull("revoking a nonexistent subkey should fail", otherModified);
 
@@ -582,9 +563,8 @@ public class PgpKeyOperationTest {
             parcel.reset();
             parcel.mChangePrimaryUserId = uid;
 
-            WrappedSecretKeyRing secretRing = new WrappedSecretKeyRing(modified.getEncoded(), false, 0);
-            OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
-            UncachedKeyRing otherModified = op.modifySecretKeyRing(secretRing, parcel, passphrase, log, 0);
+            CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(modified.getEncoded(), false, 0);
+            UncachedKeyRing otherModified = op.modifySecretKeyRing(secretRing, parcel, passphrase).getRing();
 
             Assert.assertNull("setting primary user id to a revoked user id should fail", otherModified);
 
@@ -629,6 +609,14 @@ public class PgpKeyOperationTest {
     @Test
     public void testUserIdAdd() throws Exception {
 
+        {
+            parcel.mAddUserIds.add("");
+            CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ring.getEncoded(), false, 0);
+            UncachedKeyRing modified = op.modifySecretKeyRing(secretRing, parcel, passphrase).getRing();
+            Assert.assertNull("adding an empty user id should fail", modified);
+        }
+
+        parcel.reset();
         parcel.mAddUserIds.add("rainbow");
 
         UncachedKeyRing modified = applyModificationWithChecks(parcel, ring, onlyA, onlyB);
@@ -689,10 +677,12 @@ public class PgpKeyOperationTest {
             parcel.reset();
             //noinspection SpellCheckingInspection
             parcel.mChangePrimaryUserId = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA";
+            if (parcel.mChangePrimaryUserId.equals(passphrase)) {
+                parcel.mChangePrimaryUserId += "A";
+            }
 
-            WrappedSecretKeyRing secretRing = new WrappedSecretKeyRing(ring.getEncoded(), false, 0);
-            OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
-            modified = op.modifySecretKeyRing(secretRing, parcel, passphrase, log, 0);
+            CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ring.getEncoded(), false, 0);
+            modified = op.modifySecretKeyRing(secretRing, parcel, passphrase).getRing();
 
             Assert.assertNull("changing primary user id to a non-existent one should fail", modified);
         }
@@ -716,14 +706,14 @@ public class PgpKeyOperationTest {
                                                                ArrayList<RawPacket> onlyB,
                                                                boolean canonicalize,
                                                                boolean constantCanonicalize) {
+
         try {
 
             Assert.assertTrue("modified keyring must be secret", ring.isSecret());
-            WrappedSecretKeyRing secretRing = new WrappedSecretKeyRing(ring.getEncoded(), false, 0);
+            CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ring.getEncoded(), false, 0);
 
             PgpKeyOperation op = new PgpKeyOperation(null);
-            OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
-            UncachedKeyRing rawModified = op.modifySecretKeyRing(secretRing, parcel, passphrase, log, 0);
+            UncachedKeyRing rawModified = op.modifySecretKeyRing(secretRing, parcel, passphrase).getRing();
             Assert.assertNotNull("key modification failed", rawModified);
 
             if (!canonicalize) {
@@ -732,7 +722,7 @@ public class PgpKeyOperationTest {
                 return rawModified;
             }
 
-            UncachedKeyRing modified = rawModified.canonicalize(log, 0);
+            CanonicalizedKeyRing modified = rawModified.canonicalize(new OperationLog(), 0);
             if (constantCanonicalize) {
                 Assert.assertTrue("key must be constant through canonicalization",
                         !KeyringTestingHelper.diffKeyrings(
@@ -741,7 +731,8 @@ public class PgpKeyOperationTest {
             }
             Assert.assertTrue("keyring must differ from original", KeyringTestingHelper.diffKeyrings(
                     ring.getEncoded(), modified.getEncoded(), onlyA, onlyB));
-            return modified;
+
+            return modified.getUncachedKeyRing();
 
         } catch (IOException e) {
             throw new AssertionFailedError("error during encoding!");
@@ -754,12 +745,8 @@ public class PgpKeyOperationTest {
         UncachedKeyRing expectedKeyRing = KeyringBuilder.correctRing();
         UncachedKeyRing inputKeyRing = KeyringBuilder.ringWithExtraIncorrectSignature();
 
-        OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
-        UncachedKeyRing canonicalizedRing = inputKeyRing.canonicalize(log, 0);
-
-        if (canonicalizedRing == null) {
-            throw new AssertionError("Canonicalization failed; messages: [" + log + "]");
-        }
+        CanonicalizedKeyRing canonicalized = inputKeyRing.canonicalize(new OperationLog(), 0);
+        Assert.assertNotNull("canonicalization must succeed", canonicalized);
 
         ArrayList onlyA = new ArrayList<RawPacket>();
         ArrayList onlyB = new ArrayList<RawPacket>();
diff --git a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/UncachedKeyringCanonicalizeTest.java b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/UncachedKeyringCanonicalizeTest.java
index 6f3cf31b5..97e0d8a68 100644
--- a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/UncachedKeyringCanonicalizeTest.java
+++ b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/UncachedKeyringCanonicalizeTest.java
@@ -12,20 +12,43 @@ import org.spongycastle.bcpg.Packet;
 import org.spongycastle.bcpg.PacketTags;
 import org.spongycastle.bcpg.UserIDPacket;
 import org.spongycastle.bcpg.sig.KeyFlags;
+import org.spongycastle.openpgp.PGPPrivateKey;
+import org.spongycastle.openpgp.PGPPublicKey;
+import org.spongycastle.openpgp.PGPSecretKey;
+import org.spongycastle.openpgp.PGPSecretKeyRing;
+import org.spongycastle.openpgp.PGPSignature;
+import org.spongycastle.openpgp.PGPSignatureGenerator;
+import org.spongycastle.openpgp.PGPSignatureSubpacketGenerator;
+import org.spongycastle.openpgp.PGPUtil;
+import org.spongycastle.openpgp.operator.PBESecretKeyDecryptor;
+import org.spongycastle.openpgp.operator.PGPContentSignerBuilder;
+import org.spongycastle.openpgp.operator.jcajce.JcaKeyFingerprintCalculator;
+import org.spongycastle.openpgp.operator.jcajce.JcaPGPContentSignerBuilder;
+import org.spongycastle.openpgp.operator.jcajce.JcePBESecretKeyDecryptorBuilder;
 import org.sufficientlysecure.keychain.Constants;
+import org.sufficientlysecure.keychain.pgp.CanonicalizedKeyRing;
 import org.sufficientlysecure.keychain.pgp.PgpKeyOperation;
 import org.sufficientlysecure.keychain.pgp.UncachedKeyRing;
 import org.sufficientlysecure.keychain.pgp.UncachedPublicKey;
 import org.sufficientlysecure.keychain.pgp.WrappedSignature;
 import org.sufficientlysecure.keychain.service.OperationResultParcel;
+import org.sufficientlysecure.keychain.service.OperationResults.EditKeyResult;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel;
 import org.sufficientlysecure.keychain.support.KeyringTestingHelper;
 import org.sufficientlysecure.keychain.support.KeyringTestingHelper.RawPacket;
 
 import java.io.ByteArrayInputStream;
 import java.util.ArrayList;
+import java.util.Date;
 import java.util.Iterator;
 
+
+/** Tests for the UncachedKeyring.canonicalize method.
+ *
+ * This is a complex and crypto-relevant method, which takes care of sanitizing keyrings.
+ * Test cases are made for all its assertions.
+ */
+
 @RunWith(RobolectricTestRunner.class)
 @org.robolectric.annotation.Config(emulateSdk = 18) // Robolectric doesn't yet support 19
 public class UncachedKeyringCanonicalizeTest {
@@ -36,6 +59,8 @@ public class UncachedKeyringCanonicalizeTest {
     ArrayList<RawPacket> onlyA = new ArrayList<RawPacket>();
     ArrayList<RawPacket> onlyB = new ArrayList<RawPacket>();
     OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
+    PGPSignatureSubpacketGenerator subHashedPacketsGen;
+    PGPSecretKey secretKey;
 
     @BeforeClass
     public static void setUpOnce() throws Exception {
@@ -55,9 +80,9 @@ public class UncachedKeyringCanonicalizeTest {
         parcel.mNewPassphrase = "";
         PgpKeyOperation op = new PgpKeyOperation(null);
 
-        OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
-        staticRing = op.createSecretKeyRing(parcel, log, 0);
-
+        EditKeyResult 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);
 
         // just for later reference
@@ -71,8 +96,13 @@ public class UncachedKeyringCanonicalizeTest {
         // show Log.x messages in system.out
         ShadowLog.stream = System.out;
         ring = staticRing;
+
+        subHashedPacketsGen = new PGPSignatureSubpacketGenerator();
+        secretKey = new PGPSecretKeyRing(ring.getEncoded(), new JcaKeyFingerprintCalculator())
+                .getSecretKey();
     }
 
+    /** Make sure the assumptions made about the generated ring packet structure are valid. */
     @Test public void testGeneratedRingStructure() throws Exception {
 
         Iterator<RawPacket> it = KeyringTestingHelper.parseKeyring(ring.getEncoded());
@@ -107,43 +137,6 @@ public class UncachedKeyringCanonicalizeTest {
 
     }
 
-    @Test public void testBrokenSignature() throws Exception {
-
-        byte[] brokenSig;
-        {
-            UncachedPublicKey masterKey = ring.getPublicKey();
-            WrappedSignature sig = masterKey.getSignaturesForId("twi").next();
-            brokenSig = sig.getEncoded();
-            // break the signature
-            brokenSig[brokenSig.length - 5] += 1;
-        }
-
-        byte[] reng = ring.getEncoded();
-        for(int i = 0; i < totalPackets; i++) {
-
-            byte[] brokenBytes = KeyringTestingHelper.injectPacket(reng, brokenSig, i);
-            Assert.assertEquals("broken ring must be original + injected size",
-                    reng.length + brokenSig.length, brokenBytes.length);
-
-            try {
-                UncachedKeyRing brokenRing = UncachedKeyRing.decodeFromData(brokenBytes);
-
-                brokenRing = brokenRing.canonicalize(log, 0);
-                if (brokenRing == null) {
-                    System.out.println("ok, canonicalization failed.");
-                    continue;
-                }
-
-                Assert.assertArrayEquals("injected bad signature must be gone after canonicalization",
-                        ring.getEncoded(), brokenRing.getEncoded());
-
-            } catch (Exception e) {
-                System.out.println("ok, rejected with: " + e.getMessage());
-            }
-        }
-
-    }
-
     @Test public void testUidSignature() throws Exception {
 
         UncachedPublicKey masterKey = ring.getPublicKey();
@@ -156,31 +149,29 @@ public class UncachedKeyringCanonicalizeTest {
 
         { // bad certificates get stripped
             UncachedKeyRing modified = KeyringTestingHelper.injectPacket(ring, brokenSig.getEncoded(), 3);
-            modified = modified.canonicalize(log, 0);
+            CanonicalizedKeyRing canonicalized = modified.canonicalize(log, 0);
 
             Assert.assertTrue("canonicalized keyring with invalid extra sig must be same as original one",
                     !KeyringTestingHelper.diffKeyrings(
-                        ring.getEncoded(), modified.getEncoded(), onlyA, onlyB));
+                        ring.getEncoded(), canonicalized.getEncoded(), onlyA, onlyB));
         }
 
         // remove user id certificate for one user
         final UncachedKeyRing base = KeyringTestingHelper.removePacket(ring, 2);
 
         { // user id without certificate should be removed
-            UncachedKeyRing modified = base.canonicalize(log, 0);
+            CanonicalizedKeyRing modified = base.canonicalize(log, 0);
             Assert.assertTrue("canonicalized keyring must differ", KeyringTestingHelper.diffKeyrings(
                     ring.getEncoded(), modified.getEncoded(), onlyA, onlyB));
 
             Assert.assertEquals("two packets should be stripped after canonicalization", 2, onlyA.size());
             Assert.assertEquals("no new packets after canonicalization", 0, onlyB.size());
 
-            Packet p;
-            p = new BCPGInputStream(new ByteArrayInputStream(onlyA.get(0).buf)).readPacket();
+            Packet p = new BCPGInputStream(new ByteArrayInputStream(onlyA.get(0).buf)).readPacket();
             Assert.assertTrue("first stripped packet must be user id", p instanceof UserIDPacket);
             Assert.assertEquals("missing user id must be the expected one",
                     "twi", ((UserIDPacket) p).getID());
 
-            p = new BCPGInputStream(new ByteArrayInputStream(onlyA.get(1).buf)).readPacket();
             Assert.assertArrayEquals("second stripped packet must be signature we removed",
                     sig.getEncoded(), onlyA.get(1).buf);
 
@@ -189,25 +180,448 @@ public class UncachedKeyringCanonicalizeTest {
         { // add error to signature
 
             UncachedKeyRing modified = KeyringTestingHelper.injectPacket(base, brokenSig.getEncoded(), 3);
-            modified = modified.canonicalize(log, 0);
+            CanonicalizedKeyRing canonicalized = modified.canonicalize(log, 0);
 
             Assert.assertTrue("canonicalized keyring must differ", KeyringTestingHelper.diffKeyrings(
-                    ring.getEncoded(), modified.getEncoded(), onlyA, onlyB));
+                    ring.getEncoded(), canonicalized.getEncoded(), onlyA, onlyB));
 
             Assert.assertEquals("two packets should be missing after canonicalization", 2, onlyA.size());
             Assert.assertEquals("no new packets after canonicalization", 0, onlyB.size());
 
-            Packet p;
-            p = new BCPGInputStream(new ByteArrayInputStream(onlyA.get(0).buf)).readPacket();
+            Packet p = new BCPGInputStream(new ByteArrayInputStream(onlyA.get(0).buf)).readPacket();
             Assert.assertTrue("first stripped packet must be user id", p instanceof UserIDPacket);
             Assert.assertEquals("missing user id must be the expected one",
                     "twi", ((UserIDPacket) p).getID());
 
-            p = new BCPGInputStream(new ByteArrayInputStream(onlyA.get(1).buf)).readPacket();
             Assert.assertArrayEquals("second stripped packet must be signature we removed",
                     sig.getEncoded(), onlyA.get(1).buf);
         }
 
     }
 
+    @Test public void testUidDestroy() throws Exception {
+
+        // signature for "twi"
+        ring = KeyringTestingHelper.removePacket(ring, 2);
+        // signature for "pink"
+        ring = KeyringTestingHelper.removePacket(ring, 3);
+
+        // canonicalization should fail, because there are no valid uids left
+        CanonicalizedKeyRing canonicalized = ring.canonicalize(log, 0);
+        Assert.assertNull("canonicalization of keyring with no valid uids should fail", canonicalized);
+
+    }
+
+    @Test public void testRevocationRedundant() throws Exception {
+
+        PGPSignature revocation = forgeSignature(
+                secretKey, PGPSignature.KEY_REVOCATION, subHashedPacketsGen, secretKey.getPublicKey());
+
+        UncachedKeyRing modified = KeyringTestingHelper.injectPacket(ring, revocation.getEncoded(), 1);
+
+        // try to add the same packet again, it should be rejected in all positions
+        injectEverywhere(modified, revocation.getEncoded());
+
+        // an older (but different!) revocation should be rejected as well
+        subHashedPacketsGen.setSignatureCreationTime(false, new Date(new Date().getTime() -1000*1000));
+        revocation = forgeSignature(
+                secretKey, PGPSignature.KEY_REVOCATION, subHashedPacketsGen, secretKey.getPublicKey());
+
+        injectEverywhere(modified, revocation.getEncoded());
+
+    }
+
+    @Test public void testUidRedundant() throws Exception {
+
+        // an older uid certificate should be rejected
+        subHashedPacketsGen.setSignatureCreationTime(false, new Date(new Date().getTime() -1000*1000));
+        PGPSignature revocation = forgeSignature(
+                secretKey, PGPSignature.DEFAULT_CERTIFICATION, subHashedPacketsGen, "twi", secretKey.getPublicKey());
+
+        injectEverywhere(ring, revocation.getEncoded());
+
+    }
+
+    @Test public void testUidRevocationOutdated() throws Exception {
+        // an older uid revocation cert should be rejected
+        subHashedPacketsGen.setSignatureCreationTime(false, new Date(new Date().getTime() -1000*1000));
+        PGPSignature revocation = forgeSignature(
+                secretKey, PGPSignature.CERTIFICATION_REVOCATION, subHashedPacketsGen, "twi", secretKey.getPublicKey());
+
+        injectEverywhere(ring, revocation.getEncoded());
+
+    }
+
+    @Test public void testUidRevocationRedundant() throws Exception {
+
+        PGPSignature revocation = forgeSignature(
+                secretKey, PGPSignature.CERTIFICATION_REVOCATION, subHashedPacketsGen, "twi", secretKey.getPublicKey());
+
+        // add that revocation to the base, and check if the redundant one will be rejected as well
+        UncachedKeyRing modified = KeyringTestingHelper.injectPacket(ring, revocation.getEncoded(), 2);
+
+        injectEverywhere(modified, revocation.getEncoded());
+
+        // an older (but different!) uid revocation should be rejected as well
+        subHashedPacketsGen.setSignatureCreationTime(false, new Date(new Date().getTime() -1000*1000));
+        revocation = forgeSignature(
+                secretKey, PGPSignature.CERTIFICATION_REVOCATION, subHashedPacketsGen, "twi", secretKey.getPublicKey());
+
+        injectEverywhere(modified, revocation.getEncoded());
+
+    }
+
+    @Test public void testSignatureBroken() throws Exception {
+
+        injectEverytype(secretKey, ring, subHashedPacketsGen, true);
+
+    }
+
+    @Test public void testForeignSignature() throws Exception {
+
+        SaveKeyringParcel parcel = new SaveKeyringParcel();
+        parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
+                Constants.choice.algorithm.rsa, 1024, KeyFlags.CERTIFY_OTHER, null));
+        parcel.mAddUserIds.add("trix");
+        PgpKeyOperation op = new PgpKeyOperation(null);
+
+        OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
+        UncachedKeyRing foreign = op.createSecretKeyRing(parcel).getRing();
+
+        Assert.assertNotNull("initial test key creation must succeed", foreign);
+        PGPSecretKey foreignSecretKey =
+                new PGPSecretKeyRing(foreign.getEncoded(), new JcaKeyFingerprintCalculator())
+                .getSecretKey();
+
+        injectEverytype(foreignSecretKey, ring, subHashedPacketsGen);
+
+    }
+
+    @Test public void testSignatureFuture() throws Exception {
+
+        // generate future
+        subHashedPacketsGen.setSignatureCreationTime(false, new Date(new Date().getTime() + 1000 * 1000));
+
+        injectEverytype(secretKey, ring, subHashedPacketsGen);
+
+
+    }
+
+    @Test public void testSignatureLocal() throws Exception {
+
+        // generate future
+        subHashedPacketsGen.setSignatureCreationTime(false, new Date());
+        subHashedPacketsGen.setExportable(false, false);
+
+        injectEverytype(secretKey, ring, subHashedPacketsGen);
+
+    }
+
+    @Test public void testSubkeyDestroy() throws Exception {
+
+        // signature for second key (first subkey)
+        UncachedKeyRing modified = KeyringTestingHelper.removePacket(ring, 6);
+
+        // canonicalization should fail, because there are no valid uids left
+        CanonicalizedKeyRing canonicalized = modified.canonicalize(log, 0);
+        Assert.assertTrue("keyring with missing subkey binding sig should differ from intact one after canonicalization",
+                KeyringTestingHelper.diffKeyrings(ring.getEncoded(), canonicalized.getEncoded(),
+                        onlyA, onlyB)
+        );
+
+        Assert.assertEquals("canonicalized keyring should have two extra packets", 2, onlyA.size());
+        Assert.assertEquals("canonicalized keyring should have no extra packets", 0, onlyB.size());
+
+        Assert.assertEquals("first missing packet should be the subkey",
+                PacketTags.SECRET_SUBKEY, onlyA.get(0).tag);
+        Assert.assertEquals("second missing packet should be subkey's signature",
+                PacketTags.SIGNATURE, onlyA.get(1).tag);
+        Assert.assertEquals("second missing packet should be next to subkey",
+                onlyA.get(0).position + 1, onlyA.get(1).position);
+
+    }
+
+    @Test public void testSubkeyBindingNoPKB() throws Exception {
+
+        UncachedPublicKey pKey = KeyringTestingHelper.getNth(ring.getPublicKeys(), 1);
+        Assert.assertTrue("second subkey must be able to sign", pKey.canSign());
+
+        PGPSignature sig;
+
+        subHashedPacketsGen.setKeyFlags(false, KeyFlags.SIGN_DATA);
+
+        {
+            // forge a (newer) signature, which has the sign flag but no primary key binding sig
+            PGPSignatureSubpacketGenerator unhashedSubs = new PGPSignatureSubpacketGenerator();
+
+            // just add any random signature, because why not
+            unhashedSubs.setEmbeddedSignature(false, forgeSignature(
+                            secretKey, PGPSignature.POSITIVE_CERTIFICATION, subHashedPacketsGen,
+                            secretKey.getPublicKey()
+                    )
+            );
+
+            sig = forgeSignature(
+                    secretKey, PGPSignature.SUBKEY_BINDING, subHashedPacketsGen, unhashedSubs,
+                    secretKey.getPublicKey(), pKey.getPublicKey());
+
+            // inject in the right position
+            UncachedKeyRing modified = KeyringTestingHelper.injectPacket(ring, sig.getEncoded(), 6);
+
+            // canonicalize, and check if we lose the bad signature
+            CanonicalizedKeyRing canonicalized = modified.canonicalize(log, 0);
+            Assert.assertFalse("subkey binding signature should be gone after canonicalization",
+                    KeyringTestingHelper.diffKeyrings(ring.getEncoded(), canonicalized.getEncoded(),
+                            onlyA, onlyB)
+            );
+        }
+
+        { // now try one with a /bad/ primary key binding signature
+
+            PGPSignatureSubpacketGenerator unhashedSubs = new PGPSignatureSubpacketGenerator();
+            // this one is signed by the primary key itself, not the subkey - but it IS primary binding
+            unhashedSubs.setEmbeddedSignature(false, forgeSignature(
+                            secretKey, PGPSignature.PRIMARYKEY_BINDING, subHashedPacketsGen,
+                            secretKey.getPublicKey(), pKey.getPublicKey()
+                    )
+            );
+
+            sig = forgeSignature(
+                    secretKey, PGPSignature.SUBKEY_BINDING, subHashedPacketsGen, unhashedSubs,
+                    secretKey.getPublicKey(), pKey.getPublicKey());
+
+            // inject in the right position
+            UncachedKeyRing modified = KeyringTestingHelper.injectPacket(ring, sig.getEncoded(), 6);
+
+            // canonicalize, and check if we lose the bad signature
+            CanonicalizedKeyRing canonicalized = modified.canonicalize(log, 0);
+            Assert.assertFalse("subkey binding signature should be gone after canonicalization",
+                    KeyringTestingHelper.diffKeyrings(ring.getEncoded(), canonicalized.getEncoded(),
+                            onlyA, onlyB)
+            );
+        }
+
+    }
+
+    @Test public void testSubkeyBindingRedundant() throws Exception {
+
+        UncachedPublicKey pKey = KeyringTestingHelper.getNth(ring.getPublicKeys(), 2);
+
+        subHashedPacketsGen.setKeyFlags(false, KeyFlags.ENCRYPT_COMMS);
+        PGPSignature sig2 = forgeSignature(
+                secretKey, PGPSignature.SUBKEY_BINDING, subHashedPacketsGen,
+                secretKey.getPublicKey(), pKey.getPublicKey());
+
+        subHashedPacketsGen.setSignatureCreationTime(false, new Date(new Date().getTime() -1000*1000));
+        PGPSignature sig1 = forgeSignature(
+                secretKey, PGPSignature.SUBKEY_REVOCATION, subHashedPacketsGen,
+                secretKey.getPublicKey(), pKey.getPublicKey());
+
+        subHashedPacketsGen = new PGPSignatureSubpacketGenerator();
+        subHashedPacketsGen.setSignatureCreationTime(false, new Date(new Date().getTime() -100*1000));
+        PGPSignature sig3 = forgeSignature(
+                secretKey, PGPSignature.SUBKEY_BINDING, subHashedPacketsGen,
+                secretKey.getPublicKey(), pKey.getPublicKey());
+
+        UncachedKeyRing modified = KeyringTestingHelper.injectPacket(ring, sig1.getEncoded(), 8);
+        modified = KeyringTestingHelper.injectPacket(modified, sig2.getEncoded(), 9);
+        modified = KeyringTestingHelper.injectPacket(modified, sig1.getEncoded(), 10);
+        modified = KeyringTestingHelper.injectPacket(modified, sig3.getEncoded(), 11);
+
+        // canonicalize, and check if we lose the bad signature
+        CanonicalizedKeyRing canonicalized = modified.canonicalize(log, 0);
+        Assert.assertTrue("subkey binding signature should be gone after canonicalization",
+                KeyringTestingHelper.diffKeyrings(modified.getEncoded(), canonicalized.getEncoded(),
+                        onlyA, onlyB)
+        );
+
+        Assert.assertEquals("canonicalized keyring should have lost two packets", 3, onlyA.size());
+        Assert.assertEquals("canonicalized keyring should have no extra packets", 0, onlyB.size());
+
+        Assert.assertEquals("first missing packet should be the subkey",
+                PacketTags.SIGNATURE, onlyA.get(0).tag);
+        Assert.assertEquals("second missing packet should be a signature",
+                PacketTags.SIGNATURE, onlyA.get(1).tag);
+        Assert.assertEquals("second missing packet should be a signature",
+                PacketTags.SIGNATURE, onlyA.get(2).tag);
+
+    }
+
+    private static final int[] sigtypes_direct = new int[] {
+        PGPSignature.KEY_REVOCATION,
+        PGPSignature.DIRECT_KEY,
+    };
+    private static final int[] sigtypes_uid = new int[] {
+        PGPSignature.DEFAULT_CERTIFICATION,
+        PGPSignature.NO_CERTIFICATION,
+        PGPSignature.CASUAL_CERTIFICATION,
+        PGPSignature.POSITIVE_CERTIFICATION,
+        PGPSignature.CERTIFICATION_REVOCATION,
+    };
+    private static final int[] sigtypes_subkey = new int[] {
+        PGPSignature.SUBKEY_BINDING,
+        PGPSignature.PRIMARYKEY_BINDING,
+        PGPSignature.SUBKEY_REVOCATION,
+    };
+
+    private static void injectEverytype(PGPSecretKey secretKey,
+                                        UncachedKeyRing ring,
+                                        PGPSignatureSubpacketGenerator subHashedPacketsGen)
+            throws Exception {
+        injectEverytype(secretKey, ring, subHashedPacketsGen, false);
+    }
+
+    private static void injectEverytype(PGPSecretKey secretKey,
+                                        UncachedKeyRing ring,
+                                        PGPSignatureSubpacketGenerator subHashedPacketsGen,
+                                        boolean breakSig)
+            throws Exception {
+
+        for (int sigtype : sigtypes_direct) {
+            PGPSignature sig = forgeSignature(
+                    secretKey, sigtype, subHashedPacketsGen, secretKey.getPublicKey());
+            byte[] encoded = sig.getEncoded();
+            if (breakSig) {
+                encoded[encoded.length-10] += 1;
+            }
+            injectEverywhere(ring, encoded);
+        }
+
+        for (int sigtype : sigtypes_uid) {
+            PGPSignature sig = forgeSignature(
+                    secretKey, sigtype, subHashedPacketsGen, "twi", secretKey.getPublicKey());
+
+            byte[] encoded = sig.getEncoded();
+            if (breakSig) {
+                encoded[encoded.length-10] += 1;
+            }
+            injectEverywhere(ring, encoded);
+        }
+
+        for (int sigtype : sigtypes_subkey) {
+            PGPSignature sig = forgeSignature(
+                    secretKey, sigtype, subHashedPacketsGen,
+                    secretKey.getPublicKey(), secretKey.getPublicKey());
+
+            byte[] encoded = sig.getEncoded();
+            if (breakSig) {
+                encoded[encoded.length-10] += 1;
+            }
+            injectEverywhere(ring, encoded);
+        }
+
+    }
+
+    private static void injectEverywhere(UncachedKeyRing ring, byte[] packet) throws Exception {
+
+        OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
+
+        byte[] encodedRing = ring.getEncoded();
+
+        for(int i = 0; i < totalPackets; i++) {
+
+            byte[] brokenEncoded = KeyringTestingHelper.injectPacket(encodedRing, packet, i);
+
+            try {
+
+                UncachedKeyRing brokenRing = UncachedKeyRing.decodeFromData(brokenEncoded);
+
+                CanonicalizedKeyRing canonicalized = brokenRing.canonicalize(log, 0);
+                if (canonicalized == null) {
+                    System.out.println("ok, canonicalization failed.");
+                    continue;
+                }
+
+                Assert.assertArrayEquals("injected bad signature must be gone after canonicalization",
+                        ring.getEncoded(), canonicalized.getEncoded());
+
+            } catch (Exception e) {
+                System.out.println("ok, rejected with: " + e.getMessage());
+            }
+        }
+
+    }
+
+    private static PGPSignature forgeSignature(PGPSecretKey key, int type,
+                                               PGPSignatureSubpacketGenerator subpackets,
+                                               PGPPublicKey publicKey)
+            throws Exception {
+
+        PBESecretKeyDecryptor keyDecryptor = new JcePBESecretKeyDecryptorBuilder().setProvider(
+                Constants.BOUNCY_CASTLE_PROVIDER_NAME).build("".toCharArray());
+        PGPPrivateKey privateKey = key.extractPrivateKey(keyDecryptor);
+
+        PGPContentSignerBuilder signerBuilder = new JcaPGPContentSignerBuilder(
+                publicKey.getAlgorithm(), PGPUtil.SHA1)
+                .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME);
+
+        PGPSignatureGenerator sGen = new PGPSignatureGenerator(signerBuilder);
+        sGen.setHashedSubpackets(subpackets.generate());
+        sGen.init(type, privateKey);
+        return sGen.generateCertification(publicKey);
+
+    }
+
+    private static PGPSignature forgeSignature(PGPSecretKey key, int type,
+                                               PGPSignatureSubpacketGenerator subpackets,
+                                               String userId, PGPPublicKey publicKey)
+            throws Exception {
+
+        PBESecretKeyDecryptor keyDecryptor = new JcePBESecretKeyDecryptorBuilder().setProvider(
+                Constants.BOUNCY_CASTLE_PROVIDER_NAME).build("".toCharArray());
+        PGPPrivateKey privateKey = key.extractPrivateKey(keyDecryptor);
+
+        PGPContentSignerBuilder signerBuilder = new JcaPGPContentSignerBuilder(
+                publicKey.getAlgorithm(), PGPUtil.SHA1)
+                .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME);
+
+        PGPSignatureGenerator sGen = new PGPSignatureGenerator(signerBuilder);
+        sGen.setHashedSubpackets(subpackets.generate());
+        sGen.init(type, privateKey);
+        return sGen.generateCertification(userId, publicKey);
+
+    }
+
+    private static PGPSignature forgeSignature(PGPSecretKey key, int type,
+                                               PGPSignatureSubpacketGenerator subpackets,
+                                               PGPPublicKey publicKey, PGPPublicKey signedKey)
+            throws Exception {
+
+        PBESecretKeyDecryptor keyDecryptor = new JcePBESecretKeyDecryptorBuilder().setProvider(
+                Constants.BOUNCY_CASTLE_PROVIDER_NAME).build("".toCharArray());
+        PGPPrivateKey privateKey = key.extractPrivateKey(keyDecryptor);
+
+        PGPContentSignerBuilder signerBuilder = new JcaPGPContentSignerBuilder(
+                publicKey.getAlgorithm(), PGPUtil.SHA1)
+                .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME);
+
+        PGPSignatureGenerator sGen = new PGPSignatureGenerator(signerBuilder);
+        sGen.setHashedSubpackets(subpackets.generate());
+        sGen.init(type, privateKey);
+        return sGen.generateCertification(publicKey, signedKey);
+
+    }
+
+    private static PGPSignature forgeSignature(PGPSecretKey key, int type,
+                                               PGPSignatureSubpacketGenerator hashedSubs,
+                                               PGPSignatureSubpacketGenerator unhashedSubs,
+                                               PGPPublicKey publicKey, PGPPublicKey signedKey)
+            throws Exception {
+
+        PBESecretKeyDecryptor keyDecryptor = new JcePBESecretKeyDecryptorBuilder().setProvider(
+                Constants.BOUNCY_CASTLE_PROVIDER_NAME).build("".toCharArray());
+        PGPPrivateKey privateKey = key.extractPrivateKey(keyDecryptor);
+
+        PGPContentSignerBuilder signerBuilder = new JcaPGPContentSignerBuilder(
+                publicKey.getAlgorithm(), PGPUtil.SHA1)
+                .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME);
+
+        PGPSignatureGenerator sGen = new PGPSignatureGenerator(signerBuilder);
+        sGen.setHashedSubpackets(hashedSubs.generate());
+        sGen.setUnhashedSubpackets(unhashedSubs.generate());
+        sGen.init(type, privateKey);
+        return sGen.generateCertification(publicKey, signedKey);
+
+    }
+
 }
diff --git a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/UncachedKeyringMergeTest.java b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/UncachedKeyringMergeTest.java
new file mode 100644
index 000000000..977ddfc52
--- /dev/null
+++ b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/UncachedKeyringMergeTest.java
@@ -0,0 +1,397 @@
+package org.sufficientlysecure.keychain.tests;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.robolectric.RobolectricTestRunner;
+import org.robolectric.shadows.ShadowLog;
+import org.spongycastle.bcpg.PacketTags;
+import org.spongycastle.bcpg.sig.KeyFlags;
+import org.sufficientlysecure.keychain.Constants;
+import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKeyRing;
+import org.sufficientlysecure.keychain.pgp.PgpKeyOperation;
+import org.sufficientlysecure.keychain.pgp.UncachedKeyRing;
+import org.sufficientlysecure.keychain.pgp.UncachedPublicKey;
+import org.sufficientlysecure.keychain.pgp.CanonicalizedPublicKeyRing;
+import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKey;
+import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKeyRing;
+import org.sufficientlysecure.keychain.service.OperationResultParcel;
+import org.sufficientlysecure.keychain.service.OperationResults.EditKeyResult;
+import org.sufficientlysecure.keychain.service.SaveKeyringParcel;
+import org.sufficientlysecure.keychain.support.KeyringTestingHelper;
+import org.sufficientlysecure.keychain.support.KeyringTestingHelper.RawPacket;
+import org.sufficientlysecure.keychain.util.ProgressScaler;
+
+import java.util.ArrayList;
+import java.util.Iterator;
+
+/** Tests for the UncachedKeyring.merge method.
+ *
+ * This is another complex, crypto-related method. It merges information from one keyring into
+ * another, keeping information from the base (ie, called object) keyring in case of conflicts.
+ * The types of keys may be Public or Secret and can be mixed, For mixed types the result type
+ * will be the same as the base keyring.
+ *
+ * Test cases:
+ *  - Merging keyrings with different masterKeyIds should fail
+ *  - Merging a key with itself should be a no-operation
+ *  - Merging a key with an extra revocation certificate, it should have that certificate
+ *  - Merging a key with an extra user id, it should have that extra user id and its certificates
+ *  - Merging a key with an extra user id certificate, it should have that certificate
+ *  - Merging a key with an extra subkey, it should have that subkey
+ *  - Merging a key with an extra subkey certificate, it should have that certificate
+ *  - All of the above operations should work regardless of the key types. This means in particular
+ *      that for new subkeys, an equivalent subkey of the proper type must be generated.
+ *  - In case of two secret keys with the same id but different S2K, the key of the base keyring
+ *      should be preferred (TODO or should it?)
+ *
+ * Note that the merge operation does not care about certificate validity, a bad certificate or
+ * packet will be copied regardless. Filtering out bad packets is done with canonicalization.
+ *
+ */
+@RunWith(RobolectricTestRunner.class)
+@org.robolectric.annotation.Config(emulateSdk = 18) // Robolectric doesn't yet support 19
+public class UncachedKeyringMergeTest {
+
+    static UncachedKeyRing staticRingA, staticRingB;
+    UncachedKeyRing ringA, ringB;
+    ArrayList<RawPacket> onlyA = new ArrayList<RawPacket>();
+    ArrayList<RawPacket> onlyB = new ArrayList<RawPacket>();
+    OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
+    PgpKeyOperation op;
+    SaveKeyringParcel parcel;
+
+    @BeforeClass
+    public static void setUpOnce() throws Exception {
+        ShadowLog.stream = System.out;
+
+        {
+            SaveKeyringParcel parcel = new SaveKeyringParcel();
+            parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
+                    Constants.choice.algorithm.rsa, 1024, KeyFlags.CERTIFY_OTHER, null));
+            parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
+                    Constants.choice.algorithm.rsa, 1024, KeyFlags.SIGN_DATA, null));
+
+            parcel.mAddUserIds.add("twi");
+            parcel.mAddUserIds.add("pink");
+            // passphrase is tested in PgpKeyOperationTest, just use empty here
+            parcel.mNewPassphrase = "";
+            PgpKeyOperation op = new PgpKeyOperation(null);
+
+            OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
+
+            EditKeyResult result = op.createSecretKeyRing(parcel);
+            staticRingA = result.getRing();
+        }
+
+        {
+            SaveKeyringParcel parcel = new SaveKeyringParcel();
+            parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
+                    Constants.choice.algorithm.rsa, 1024, KeyFlags.CERTIFY_OTHER, null));
+
+            parcel.mAddUserIds.add("shy");
+            // passphrase is tested in PgpKeyOperationTest, just use empty here
+            parcel.mNewPassphrase = "";
+            PgpKeyOperation op = new PgpKeyOperation(null);
+
+            OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
+            EditKeyResult result = op.createSecretKeyRing(parcel);
+            staticRingB = result.getRing();
+        }
+
+        Assert.assertNotNull("initial test key creation must succeed", staticRingA);
+        Assert.assertNotNull("initial test key creation must succeed", staticRingB);
+
+        // we sleep here for a second, to make sure all new certificates have different timestamps
+        Thread.sleep(1000);
+    }
+
+    @Before
+    public void setUp() throws Exception {
+        // show Log.x messages in system.out
+        ShadowLog.stream = System.out;
+        ringA = staticRingA;
+        ringB = staticRingB;
+
+        // setting up some parameters just to reduce code duplication
+        op = new PgpKeyOperation(new ProgressScaler(null, 0, 100, 100));
+
+        // set this up, gonna need it more than once
+        parcel = new SaveKeyringParcel();
+        parcel.mMasterKeyId = ringA.getMasterKeyId();
+        parcel.mFingerprint = ringA.getFingerprint();
+    }
+
+    public void testSelfNoOp() throws Exception {
+
+        UncachedKeyRing merged = mergeWithChecks(ringA, ringA, null);
+        Assert.assertArrayEquals("keyring merged with itself must be identical",
+                ringA.getEncoded(), merged.getEncoded()
+        );
+
+    }
+
+    @Test
+    public void testDifferentMasterKeyIds() throws Exception {
+
+        Assert.assertNotEquals("generated key ids must be different",
+                ringA.getMasterKeyId(), ringB.getMasterKeyId());
+
+        Assert.assertNull("merging keys with differing key ids must fail",
+                ringA.merge(ringB, log, 0));
+        Assert.assertNull("merging keys with differing key ids must fail",
+                ringB.merge(ringA, log, 0));
+
+    }
+
+    @Test
+    public void testAddedUserId() throws Exception {
+
+        UncachedKeyRing modifiedA, modifiedB; {
+            CanonicalizedSecretKeyRing secretRing =
+                    new CanonicalizedSecretKeyRing(ringA.getEncoded(), false, 0);
+
+            parcel.reset();
+            parcel.mAddUserIds.add("flim");
+            modifiedA = op.modifySecretKeyRing(secretRing, parcel, "").getRing();
+
+            parcel.reset();
+            parcel.mAddUserIds.add("flam");
+            modifiedB = op.modifySecretKeyRing(secretRing, parcel, "").getRing();
+        }
+
+        { // merge A into base
+            UncachedKeyRing merged = mergeWithChecks(ringA, modifiedA);
+
+            Assert.assertEquals("merged keyring must have lost no packets", 0, onlyA.size());
+            Assert.assertEquals("merged keyring must have gained two packets", 2, onlyB.size());
+            Assert.assertTrue("merged keyring must contain new user id",
+                    merged.getPublicKey().getUnorderedUserIds().contains("flim"));
+        }
+
+        { // merge A into B
+            UncachedKeyRing merged = mergeWithChecks(modifiedA, modifiedB, ringA);
+
+            Assert.assertEquals("merged keyring must have lost no packets", 0, onlyA.size());
+            Assert.assertEquals("merged keyring must have gained four packets", 4, onlyB.size());
+            Assert.assertTrue("merged keyring must contain first new user id",
+                    merged.getPublicKey().getUnorderedUserIds().contains("flim"));
+            Assert.assertTrue("merged keyring must contain second new user id",
+                    merged.getPublicKey().getUnorderedUserIds().contains("flam"));
+
+        }
+
+    }
+
+    @Test
+    public void testAddedSubkeyId() throws Exception {
+
+        UncachedKeyRing modifiedA, modifiedB;
+        long subKeyIdA, subKeyIdB;
+        {
+            CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ringA.getEncoded(), false, 0);
+
+            parcel.reset();
+            parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
+                    Constants.choice.algorithm.rsa, 1024, KeyFlags.SIGN_DATA, null));
+            modifiedA = op.modifySecretKeyRing(secretRing, parcel, "").getRing();
+            modifiedB = op.modifySecretKeyRing(secretRing, parcel, "").getRing();
+
+            subKeyIdA = KeyringTestingHelper.getSubkeyId(modifiedA, 2);
+            subKeyIdB = KeyringTestingHelper.getSubkeyId(modifiedB, 2);
+
+        }
+
+        {
+            UncachedKeyRing merged = mergeWithChecks(ringA, modifiedA);
+
+            Assert.assertEquals("merged keyring must have lost no packets", 0, onlyA.size());
+            Assert.assertEquals("merged keyring must have gained two packets", 2, onlyB.size());
+
+            long mergedKeyId = KeyringTestingHelper.getSubkeyId(merged, 2);
+            Assert.assertEquals("merged keyring must contain the new subkey", subKeyIdA, mergedKeyId);
+        }
+
+        {
+            UncachedKeyRing merged = mergeWithChecks(modifiedA, modifiedB, ringA);
+
+            Assert.assertEquals("merged keyring must have lost no packets", 0, onlyA.size());
+            Assert.assertEquals("merged keyring must have gained four packets", 4, onlyB.size());
+
+            Iterator<UncachedPublicKey> it = merged.getPublicKeys();
+            it.next(); it.next();
+            Assert.assertEquals("merged keyring must contain the new subkey",
+                    subKeyIdA, it.next().getKeyId());
+            Assert.assertEquals("merged keyring must contain both new subkeys",
+                    subKeyIdB, it.next().getKeyId());
+        }
+
+    }
+
+    @Test
+    public void testAddedKeySignature() throws Exception {
+
+        final UncachedKeyRing modified; {
+            parcel.reset();
+            parcel.mRevokeSubKeys.add(KeyringTestingHelper.getSubkeyId(ringA, 1));
+            CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(
+                    ringA.getEncoded(), false, 0);
+            modified = op.modifySecretKeyRing(secretRing, parcel, "").getRing();
+        }
+
+        {
+            UncachedKeyRing merged = ringA.merge(modified, log, 0);
+            Assert.assertNotNull("merge must succeed", merged);
+            Assert.assertFalse(
+                    "merging keyring with extra signatures into its base should yield that same keyring",
+                    KeyringTestingHelper.diffKeyrings(merged.getEncoded(), modified.getEncoded(), onlyA, onlyB)
+            );
+        }
+
+    }
+
+    @Test
+    public void testAddedUserIdSignature() throws Exception {
+
+        final UncachedKeyRing pubRing = ringA.extractPublicKeyRing();
+
+        final UncachedKeyRing modified; {
+            CanonicalizedPublicKeyRing publicRing = new CanonicalizedPublicKeyRing(
+                    pubRing.getEncoded(), 0);
+
+            CanonicalizedSecretKey secretKey = new CanonicalizedSecretKeyRing(
+                    ringB.getEncoded(), false, 0).getSecretKey();
+            secretKey.unlock("");
+            // sign all user ids
+            modified = secretKey.certifyUserIds(publicRing, publicRing.getPublicKey().getUnorderedUserIds());
+        }
+
+        {
+            UncachedKeyRing merged = ringA.merge(modified, log, 0);
+            Assert.assertNotNull("merge must succeed", merged);
+            Assert.assertArrayEquals("foreign signatures should not be merged into secret key",
+                    ringA.getEncoded(), merged.getEncoded()
+            );
+        }
+
+        {
+            byte[] sig = KeyringTestingHelper.getNth(
+                    modified.getPublicKey().getSignaturesForId("twi"), 1).getEncoded();
+
+            // inject the (foreign!) signature into subkey signature position
+            UncachedKeyRing moreModified = KeyringTestingHelper.injectPacket(modified, sig, 1);
+
+            UncachedKeyRing merged = ringA.merge(moreModified, log, 0);
+            Assert.assertNotNull("merge must succeed", merged);
+            Assert.assertArrayEquals("foreign signatures should not be merged into secret key",
+                    ringA.getEncoded(), merged.getEncoded()
+            );
+
+            merged = pubRing.merge(moreModified, log, 0);
+            Assert.assertNotNull("merge must succeed", merged);
+            Assert.assertTrue(
+                    "merged keyring should contain new signature",
+                    KeyringTestingHelper.diffKeyrings(pubRing.getEncoded(), merged.getEncoded(), onlyA, onlyB)
+            );
+            Assert.assertEquals("merged keyring should be missing no packets", 0, onlyA.size());
+            Assert.assertEquals("merged keyring should contain exactly two more packets", 2, onlyB.size());
+            Assert.assertEquals("first added packet should be a signature",
+                    PacketTags.SIGNATURE, onlyB.get(0).tag);
+            Assert.assertEquals("first added packet should be in the position we injected it at",
+                    1, onlyB.get(0).position);
+            Assert.assertEquals("second added packet should be a signature",
+                    PacketTags.SIGNATURE, onlyB.get(1).tag);
+
+        }
+
+        {
+            UncachedKeyRing merged = pubRing.merge(modified, log, 0);
+            Assert.assertNotNull("merge must succeed", merged);
+            Assert.assertFalse(
+                    "merging keyring with extra signatures into its base should yield that same keyring",
+                    KeyringTestingHelper.diffKeyrings(merged.getEncoded(), modified.getEncoded(), onlyA, onlyB)
+            );
+        }
+    }
+
+    private UncachedKeyRing mergeWithChecks(UncachedKeyRing a, UncachedKeyRing b)
+            throws Exception {
+        return mergeWithChecks(a, b, a);
+    }
+
+    private UncachedKeyRing mergeWithChecks(UncachedKeyRing a, UncachedKeyRing b,
+                                            UncachedKeyRing base)
+            throws Exception {
+
+        Assert.assertTrue("merging keyring must be secret type", a.isSecret());
+        Assert.assertTrue("merged keyring must be secret type", b.isSecret());
+
+        final UncachedKeyRing resultA;
+        UncachedKeyRing resultB;
+
+        { // sec + sec
+            resultA = a.merge(b, log, 0);
+            Assert.assertNotNull("merge must succeed as sec(a)+sec(b)", resultA);
+
+            resultB = b.merge(a, log, 0);
+            Assert.assertNotNull("merge must succeed as sec(b)+sec(a)", resultB);
+
+            // check commutativity, if requested
+            Assert.assertFalse("result of merge must be commutative",
+                    KeyringTestingHelper.diffKeyrings(
+                            resultA.getEncoded(), resultB.getEncoded(), onlyA, onlyB)
+            );
+        }
+
+        final UncachedKeyRing pubA = a.extractPublicKeyRing();
+        final UncachedKeyRing pubB = b.extractPublicKeyRing();
+
+        { // sec + pub, pub + sec, and pub + pub
+
+            try {
+                resultB = a.merge(pubB, log, 0);
+                Assert.assertNotNull("merge must succeed as sec(a)+pub(b)", resultA);
+
+                Assert.assertFalse("result of sec(a)+pub(b) must be same as sec(a)+sec(b)",
+                        KeyringTestingHelper.diffKeyrings(
+                                resultA.getEncoded(), resultB.getEncoded(), onlyA, onlyB)
+                );
+            } catch (RuntimeException e) {
+                System.out.println("special case, dummy key generation not in yet");
+            }
+
+            final UncachedKeyRing pubResult = resultA.extractPublicKeyRing();
+
+            resultB = pubA.merge(b, log, 0);
+            Assert.assertNotNull("merge must succeed as pub(a)+sec(b)", resultA);
+
+            Assert.assertFalse("result of pub(a)+sec(b) must be same as pub(sec(a)+sec(b))",
+                    KeyringTestingHelper.diffKeyrings(
+                            pubResult.getEncoded(), resultB.getEncoded(), onlyA, onlyB)
+            );
+
+            resultB = pubA.merge(pubB, log, 0);
+            Assert.assertNotNull("merge must succeed as pub(a)+pub(b)", resultA);
+
+            Assert.assertFalse("result of pub(a)+pub(b) must be same as pub(sec(a)+sec(b))",
+                    KeyringTestingHelper.diffKeyrings(
+                            pubResult.getEncoded(), resultB.getEncoded(), onlyA, onlyB)
+            );
+
+        }
+
+        if (base != null) {
+            // set up onlyA and onlyB to be a diff to the base
+            Assert.assertTrue("merged keyring must differ from base",
+                    KeyringTestingHelper.diffKeyrings(
+                            base.getEncoded(), resultA.getEncoded(), onlyA, onlyB)
+            );
+        }
+
+        return resultA;
+
+    }
+
+}
diff --git a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/UncachedKeyringTest.java b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/UncachedKeyringTest.java
new file mode 100644
index 000000000..15aaa4c5d
--- /dev/null
+++ b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/UncachedKeyringTest.java
@@ -0,0 +1,125 @@
+package org.sufficientlysecure.keychain.tests;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.robolectric.RobolectricTestRunner;
+import org.robolectric.shadows.ShadowLog;
+import org.spongycastle.bcpg.sig.KeyFlags;
+import org.sufficientlysecure.keychain.Constants;
+import org.sufficientlysecure.keychain.pgp.PgpKeyOperation;
+import org.sufficientlysecure.keychain.pgp.UncachedKeyRing;
+import org.sufficientlysecure.keychain.pgp.UncachedPublicKey;
+import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException;
+import org.sufficientlysecure.keychain.service.OperationResultParcel;
+import org.sufficientlysecure.keychain.service.OperationResults.EditKeyResult;
+import org.sufficientlysecure.keychain.service.SaveKeyringParcel;
+import org.sufficientlysecure.keychain.support.KeyringTestingHelper.RawPacket;
+import org.sufficientlysecure.keychain.util.ProgressScaler;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.List;
+
+@RunWith(RobolectricTestRunner.class)
+@org.robolectric.annotation.Config(emulateSdk = 18) // Robolectric doesn't yet support 19
+public class UncachedKeyringTest {
+
+    static UncachedKeyRing staticRing, staticPubRing;
+    UncachedKeyRing ring, pubRing;
+    ArrayList<RawPacket> onlyA = new ArrayList<RawPacket>();
+    ArrayList<RawPacket> onlyB = new ArrayList<RawPacket>();
+    PgpKeyOperation op;
+    SaveKeyringParcel parcel;
+
+    @BeforeClass
+    public static void setUpOnce() throws Exception {
+        ShadowLog.stream = System.out;
+
+        SaveKeyringParcel parcel = new SaveKeyringParcel();
+        parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
+                Constants.choice.algorithm.rsa, 1024, KeyFlags.CERTIFY_OTHER, null));
+        parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
+                Constants.choice.algorithm.rsa, 1024, KeyFlags.SIGN_DATA, null));
+        parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
+                Constants.choice.algorithm.rsa, 1024, KeyFlags.ENCRYPT_COMMS, null));
+
+        parcel.mAddUserIds.add("twi");
+        parcel.mAddUserIds.add("pink");
+        // passphrase is tested in PgpKeyOperationTest, just use empty here
+        parcel.mNewPassphrase = "";
+        PgpKeyOperation op = new PgpKeyOperation(null);
+
+        EditKeyResult result = op.createSecretKeyRing(parcel);
+        staticRing = result.getRing();
+        staticPubRing = staticRing.extractPublicKeyRing();
+
+        Assert.assertNotNull("initial test key creation must succeed", staticRing);
+
+        // we sleep here for a second, to make sure all new certificates have different timestamps
+        Thread.sleep(1000);
+    }
+
+
+    @Before
+    public void setUp() throws Exception {
+        // show Log.x messages in system.out
+        ShadowLog.stream = System.out;
+        ring = staticRing;
+        pubRing = staticPubRing;
+    }
+
+    @Test(expected = UnsupportedOperationException.class)
+    public void testPublicKeyItRemove() throws Exception {
+        Iterator<UncachedPublicKey> it = ring.getPublicKeys();
+        it.remove();
+    }
+
+    @Test(expected = PgpGeneralException.class)
+    public void testDecodeFromEmpty() throws Exception {
+        UncachedKeyRing.decodeFromData(new byte[0]);
+    }
+
+    @Test
+    public void testArmorIdentity() throws Exception {
+        ByteArrayOutputStream out = new ByteArrayOutputStream();
+        ring.encodeArmored(out, "OpenKeychain");
+
+        Assert.assertArrayEquals("armor encoded and decoded ring should be identical to original",
+            ring.getEncoded(),
+            UncachedKeyRing.decodeFromData(out.toByteArray()).getEncoded());
+    }
+
+    @Test(expected = PgpGeneralException.class)
+    public void testDecodeEncodeMulti() throws Exception {
+        ByteArrayOutputStream out = new ByteArrayOutputStream();
+
+        // encode secret and public ring in here
+        ring.encodeArmored(out, "OpenKeychain");
+        pubRing.encodeArmored(out, "OpenKeychain");
+
+        Iterator<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",
+                ring.getEncoded(), it.next().getEncoded());
+        Assert.assertTrue("there should be two rings in the stream", it.hasNext());
+        Assert.assertArrayEquals("second ring should be the second we put in",
+                pubRing.getEncoded(), it.next().getEncoded());
+        Assert.assertFalse("there should be two rings in the stream", it.hasNext());
+
+        // this should fail with PgpGeneralException, since it expects exactly one ring
+        UncachedKeyRing.decodeFromData(out.toByteArray());
+    }
+
+    @Test(expected = RuntimeException.class)
+    public void testPublicExtractPublic() throws Exception {
+        // can't do this, either!
+        pubRing.extractPublicKeyRing();
+    }
+
+}