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 ac9bed898..b6778f26c 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,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 { + public static class RawPacket implements Comparable { 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 onlyA, SortedSet onlyB) + SortedSet onlyA, SortedSet onlyB) throws IOException { InputStream streamA = new ByteArrayInputStream(ringA); InputStream streamB = new ByteArrayInputStream(ringB); - HashSet a = new HashSet(), b = new HashSet(); + HashSet a = new HashSet(), b = new HashSet(); - 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; 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 4d422f257..992ed31ce 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 @@ -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 onlyA = new TreeSet(); - TreeSet onlyB = new TreeSet(); - Assert.assertTrue("keyrings do not differ", KeyringTestingHelper.diffKeyrings( + UncachedKeyRing modified = rawModified.canonicalize(log, 0); + + TreeSet onlyA = new TreeSet(); + TreeSet onlyB = new TreeSet(); + 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("two extra packets in modified", onlyB.size(), 2); - Iterator it = onlyB.iterator(); - Assert.assertEquals("first new packet must be secret subkey", it.next().tag, PacketTags.SECRET_SUBKEY); - Assert.assertEquals("second new packet must be signature", it.next().tag, PacketTags.SIGNATURE); + Assert.assertEquals("no extra packets in original", 0, onlyA.size()); + Assert.assertEquals("exactly two extra packets in modified", 2, onlyB.size()); + + Iterator it = onlyB.iterator(); + 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(); - TreeSet onlyB = new TreeSet(); + TreeSet onlyA = new TreeSet(); + TreeSet onlyB = new TreeSet(); Assert.assertTrue("keyrings differ", !KeyringTestingHelper.diffKeyrings( expectedKeyRing.getEncoded(), expectedKeyRing.getEncoded(), onlyA, onlyB));