test: finish testSubkeyAdd

OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/support/KeyringTestingHelper.java

@@ -29,7 +29,6 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.util.Collection;
 import java.util.HashSet;
-import java.util.Set;
 import java.util.SortedSet;
 
 /**
@@ -67,18 +66,18 @@ public class KeyringTestingHelper {
         return saveSuccess;
     }
 
-    public static class Packet implements Comparable<Packet> {
+    public static class RawPacket implements Comparable<RawPacket> {
         public int position;
         public int tag;
         public int length;
         public byte[] buf;
 
-        public int compareTo(Packet other) {
+        public int compareTo(RawPacket other) {
             return Integer.compare(position, other.position);
         }
 
         public boolean equals(Object other) {
-            return other instanceof Packet && Arrays.areEqual(this.buf, ((Packet) other).buf);
+            return other instanceof RawPacket && Arrays.areEqual(this.buf, ((RawPacket) other).buf);
         }
 
         public int hashCode() {
@@ -88,14 +87,14 @@ public class KeyringTestingHelper {
     }
 
     public static boolean diffKeyrings(byte[] ringA, byte[] ringB,
-                                       SortedSet<Packet> onlyA, SortedSet<Packet> onlyB)
+                                       SortedSet<RawPacket> onlyA, SortedSet<RawPacket> onlyB)
             throws IOException {
         InputStream streamA = new ByteArrayInputStream(ringA);
         InputStream streamB = new ByteArrayInputStream(ringB);
 
-        HashSet<Packet> a = new HashSet<Packet>(), b = new HashSet<Packet>();
+        HashSet<RawPacket> a = new HashSet<RawPacket>(), b = new HashSet<RawPacket>();
 
-        Packet p;
+        RawPacket p;
         int pos = 0;
         while(true) {
             p = readPacket(streamA);
@@ -123,7 +122,7 @@ public class KeyringTestingHelper {
         return !onlyA.isEmpty() || !onlyB.isEmpty();
     }
 
-    private static Packet readPacket(InputStream in) throws IOException {
+    private static RawPacket readPacket(InputStream in) throws IOException {
 
         // save here. this is tag + length, max 6 bytes
         in.mark(6);
@@ -196,7 +195,7 @@ public class KeyringTestingHelper {
         if (in.read(buf) != headerLength+bodyLen) {
             throw new IOException("read length mismatch!");
         }
-        Packet p = new Packet();
+        RawPacket p = new RawPacket();
         p.tag = tag;
         p.length = bodyLen;
         p.buf = buf;

OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/PgpKeyOperationTest.java

@@ -7,8 +7,12 @@ import org.junit.BeforeClass;
 import org.junit.runner.RunWith;
 import org.robolectric.*;
 import org.robolectric.shadows.ShadowLog;
-import org.spongycastle.bcpg.PacketTags;
+import org.spongycastle.bcpg.BCPGInputStream;
+import org.spongycastle.bcpg.Packet;
+import org.spongycastle.bcpg.SecretKeyPacket;
+import org.spongycastle.bcpg.SignaturePacket;
 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.PgpKeyOperation;
@@ -19,9 +23,10 @@ import org.sufficientlysecure.keychain.service.SaveKeyringParcel;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel.SubkeyAdd;
 import org.sufficientlysecure.keychain.support.KeyringBuilder;
 import org.sufficientlysecure.keychain.support.KeyringTestingHelper;
-import org.sufficientlysecure.keychain.support.KeyringTestingHelper.Packet;
+import org.sufficientlysecure.keychain.support.KeyringTestingHelper.RawPacket;
 import org.sufficientlysecure.keychain.support.TestDataUtil;
 
+import java.io.ByteArrayInputStream;
 import java.util.Iterator;
 import java.util.TreeSet;
 
@@ -79,20 +84,36 @@ public class PgpKeyOperationTest {
         parcel.addSubKeys.add(new SubkeyAdd(algorithm.rsa, 1024, KeyFlags.SIGN_DATA, null));
 
         OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
-        UncachedKeyRing modified = op.modifySecretKeyRing(ring, parcel, "swag", log, 0);
+        UncachedKeyRing rawModified = op.modifySecretKeyRing(ring, parcel, "swag", log, 0);
 
-        Assert.assertNotNull("key modification failed", modified);
+        Assert.assertNotNull("key modification failed", rawModified);
 
-        TreeSet<Packet> onlyA = new TreeSet<Packet>();
+        UncachedKeyRing modified = rawModified.canonicalize(log, 0);
-        TreeSet<Packet> onlyB = new TreeSet<Packet>();
+
-        Assert.assertTrue("keyrings do not differ", KeyringTestingHelper.diffKeyrings(
+        TreeSet<RawPacket> onlyA = new TreeSet<RawPacket>();
+        TreeSet<RawPacket> onlyB = new TreeSet<RawPacket>();
+        Assert.assertTrue("key must be constant through canonicalization",
+                !KeyringTestingHelper.diffKeyrings(
+                        modified.getEncoded(), rawModified.getEncoded(), onlyA, onlyB));
+
+        Assert.assertTrue("keyring must differ from original", KeyringTestingHelper.diffKeyrings(
                 ring.getUncached().getEncoded(), modified.getEncoded(), onlyA, onlyB));
 
-        Assert.assertEquals("no extra packets in original", onlyA.size(), 0);
+        Assert.assertEquals("no extra packets in original", 0, onlyA.size());
-        Assert.assertEquals("two extra packets in modified", onlyB.size(), 2);
+        Assert.assertEquals("exactly two extra packets in modified", 2, onlyB.size());
-        Iterator<Packet> it = onlyB.iterator();
+
-        Assert.assertEquals("first new packet must be secret subkey", it.next().tag, PacketTags.SECRET_SUBKEY);
+        Iterator<RawPacket> it = onlyB.iterator();
-        Assert.assertEquals("second new packet must be signature", it.next().tag, PacketTags.SIGNATURE);
+        Packet p;
+
+        p = new BCPGInputStream(new ByteArrayInputStream(it.next().buf)).readPacket();
+        Assert.assertTrue("first new packet must be secret subkey", p instanceof SecretKeyPacket);
+
+        p = new BCPGInputStream(new ByteArrayInputStream(it.next().buf)).readPacket();
+        Assert.assertTrue("second new packet must be signature", p instanceof SignaturePacket);
+        Assert.assertEquals("signature type must be subkey binding certificate",
+                PGPSignature.SUBKEY_BINDING, ((SignaturePacket) p).getSignatureType());
+        Assert.assertEquals("signature must have been created by master key",
+                ring.getMasterKeyId(), ((SignaturePacket) p).getKeyID());
 
     }
 
@@ -109,8 +130,8 @@ public class PgpKeyOperationTest {
             throw new AssertionError("Canonicalization failed; messages: [" + log + "]");
         }
 
-        TreeSet onlyA = new TreeSet<KeyringTestingHelper.Packet>();
+        TreeSet onlyA = new TreeSet<RawPacket>();
-        TreeSet onlyB = new TreeSet<KeyringTestingHelper.Packet>();
+        TreeSet onlyB = new TreeSet<RawPacket>();
         Assert.assertTrue("keyrings differ", !KeyringTestingHelper.diffKeyrings(
                 expectedKeyRing.getEncoded(), expectedKeyRing.getEncoded(), onlyA, onlyB));