From eab4c4ba8e4865153b3fa2122ea28628db5f7bb0 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Sat, 26 Jul 2014 04:35:38 +0200 Subject: [PATCH] test: (almost) full coverage for UncachedKeyRing.merge --- .../support/KeyringTestingHelper.java | 12 ++++ .../tests/UncachedKeyringMergeTest.java | 69 ++++++++++++++----- 2 files changed, 64 insertions(+), 17 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..98c7b0743 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 @@ -21,6 +21,7 @@ import android.content.Context; import org.spongycastle.util.Arrays; import org.sufficientlysecure.keychain.pgp.NullProgressable; import org.sufficientlysecure.keychain.pgp.UncachedKeyRing; +import org.sufficientlysecure.keychain.pgp.UncachedPublicKey; import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException; import org.sufficientlysecure.keychain.provider.ProviderHelper; import org.sufficientlysecure.keychain.service.OperationResults; @@ -331,6 +332,17 @@ public class KeyringTestingHelper { } + public static E getNth(Iterator 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); 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 index cde1991dd..0b249dcce 100644 --- a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/UncachedKeyringMergeTest.java +++ b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/UncachedKeyringMergeTest.java @@ -9,6 +9,7 @@ import org.robolectric.RobolectricTestRunner; import org.robolectric.shadows.ShadowLog; import org.spongycastle.bcpg.BCPGInputStream; import org.spongycastle.bcpg.Packet; +import org.spongycastle.bcpg.PacketTags; import org.spongycastle.bcpg.PublicKeyPacket; import org.spongycastle.bcpg.sig.KeyFlags; import org.sufficientlysecure.keychain.Constants; @@ -199,17 +200,8 @@ public class UncachedKeyringMergeTest { modifiedA = op.modifySecretKeyRing(secretRing, parcel, "", log, 0); modifiedB = op.modifySecretKeyRing(secretRing, parcel, "", log, 0); - { - Iterator it = modifiedA.getPublicKeys(); - it.next(); it.next(); - subKeyIdA = it.next().getKeyId(); - } - - { - Iterator it = modifiedB.getPublicKeys(); - it.next(); it.next(); - subKeyIdB = it.next().getKeyId(); - } + subKeyIdA = KeyringTestingHelper.getSubkeyId(modifiedA, 2); + subKeyIdB = KeyringTestingHelper.getSubkeyId(modifiedB, 2); } @@ -219,12 +211,7 @@ public class UncachedKeyringMergeTest { 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; - { - Iterator it = merged.getPublicKeys(); - it.next(); it.next(); - mergedKeyId = it.next().getKeyId(); - } + long mergedKeyId = KeyringTestingHelper.getSubkeyId(merged, 2); Assert.assertEquals("merged keyring must contain the new subkey", subKeyIdA, mergedKeyId); } @@ -246,6 +233,24 @@ public class UncachedKeyringMergeTest { @Test public void testAddedKeySignature() throws Exception { + + final UncachedKeyRing modified; { + parcel.reset(); + parcel.mRevokeSubKeys.add(KeyringTestingHelper.getSubkeyId(ringA, 1)); + WrappedSecretKeyRing secretRing = new WrappedSecretKeyRing( + ringA.getEncoded(), false, 0); + modified = op.modifySecretKeyRing(secretRing, parcel, "", log, 0); + } + + { + 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 @@ -272,6 +277,36 @@ public class UncachedKeyringMergeTest { ); } + { + 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);