From 954ed69102baffd60474f294dc3115b14473613a Mon Sep 17 00:00:00 2001 From: Viktor Kolomeyko <31008341+vkolomeyko@users.noreply.github.com> Date: Fri, 1 Sep 2017 14:45:14 +0100 Subject: [PATCH] CORDA-540: Introduce mandatory reason for "kryoSpecific" (#1386) * CORDA-540: Introduce mandatory reason for "kryoSpecific" ... before we forget why they are ignored in such a way * CORDA-540: Write a test that exposes list serialization problem in AMQP mode * Revert "Remove CompositeSignaturesWithKeys" This reverts commit 9b3cad3 --- .../corda/core/crypto/CompositeSignature.kt | 7 ++--- .../composite/CompositeSignaturesWithKeys.kt | 15 +++++++++++ .../corda/core/crypto/CompositeKeyTests.kt | 19 +++++++------- .../nodeapi/AttachmentClassLoaderTests.kt | 3 +-- .../serialization/ListsSerializationTest.kt | 26 +++++++++++++++++++ .../kotlin/net/corda/testing/CoreTestUtils.kt | 4 +-- 6 files changed, 58 insertions(+), 16 deletions(-) create mode 100644 core/src/main/kotlin/net/corda/core/crypto/composite/CompositeSignaturesWithKeys.kt create mode 100644 node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/ListsSerializationTest.kt diff --git a/core/src/main/kotlin/net/corda/core/crypto/CompositeSignature.kt b/core/src/main/kotlin/net/corda/core/crypto/CompositeSignature.kt index 2e84d467b4..0438a630ae 100644 --- a/core/src/main/kotlin/net/corda/core/crypto/CompositeSignature.kt +++ b/core/src/main/kotlin/net/corda/core/crypto/CompositeSignature.kt @@ -1,5 +1,6 @@ package net.corda.core.crypto +import net.corda.core.crypto.composite.CompositeSignaturesWithKeys import net.corda.core.serialization.deserialize import java.io.ByteArrayOutputStream import java.security.* @@ -75,10 +76,10 @@ class CompositeSignature : Signature(SIGNATURE_ALGORITHM) { data class State(val buffer: ByteArrayOutputStream, val verifyKey: CompositeKey) { fun engineVerify(sigBytes: ByteArray): Boolean { - val sigs = sigBytes.deserialize>() - return if (verifyKey.isFulfilledBy(sigs.map { it.by })) { + val sig = sigBytes.deserialize() + return if (verifyKey.isFulfilledBy(sig.sigs.map { it.by })) { val clearData = SecureHash.SHA256(buffer.toByteArray()) - sigs.all { it.isValid(clearData) } + sig.sigs.all { it.isValid(clearData) } } else { false } diff --git a/core/src/main/kotlin/net/corda/core/crypto/composite/CompositeSignaturesWithKeys.kt b/core/src/main/kotlin/net/corda/core/crypto/composite/CompositeSignaturesWithKeys.kt new file mode 100644 index 0000000000..f9c8da8ab3 --- /dev/null +++ b/core/src/main/kotlin/net/corda/core/crypto/composite/CompositeSignaturesWithKeys.kt @@ -0,0 +1,15 @@ +package net.corda.core.crypto.composite + +import net.corda.core.crypto.TransactionSignature +import net.corda.core.serialization.CordaSerializable + +/** + * Custom class for holding signature data. This exists for later extension work to provide a standardised cross-platform + * serialization format. + */ +@CordaSerializable +data class CompositeSignaturesWithKeys(val sigs: List) { + companion object { + val EMPTY = CompositeSignaturesWithKeys(emptyList()) + } +} diff --git a/core/src/test/kotlin/net/corda/core/crypto/CompositeKeyTests.kt b/core/src/test/kotlin/net/corda/core/crypto/CompositeKeyTests.kt index 38b75bd2f0..0b9ede1d80 100644 --- a/core/src/test/kotlin/net/corda/core/crypto/CompositeKeyTests.kt +++ b/core/src/test/kotlin/net/corda/core/crypto/CompositeKeyTests.kt @@ -1,6 +1,7 @@ package net.corda.core.crypto import net.corda.core.crypto.CompositeKey.NodeAndWeight +import net.corda.core.crypto.composite.CompositeSignaturesWithKeys import net.corda.core.internal.declaredField import net.corda.core.internal.div import net.corda.core.serialization.serialize @@ -160,17 +161,17 @@ class CompositeKeyTests : TestDependencyInjectionBase() { engine.initVerify(twoOfThree) engine.update(secureHash.bytes) - assertFalse { engine.verify(listOf(aliceSignature).serialize().bytes) } - assertFalse { engine.verify(listOf(bobSignature).serialize().bytes) } - assertFalse { engine.verify(listOf(charlieSignature).serialize().bytes) } - assertTrue { engine.verify(listOf(aliceSignature, bobSignature).serialize().bytes) } - assertTrue { engine.verify(listOf(aliceSignature, charlieSignature).serialize().bytes) } - assertTrue { engine.verify(listOf(bobSignature, charlieSignature).serialize().bytes) } - assertTrue { engine.verify(listOf(aliceSignature, bobSignature, charlieSignature).serialize().bytes) } + assertFalse { engine.verify(CompositeSignaturesWithKeys(listOf(aliceSignature)).serialize().bytes) } + assertFalse { engine.verify(CompositeSignaturesWithKeys(listOf(bobSignature)).serialize().bytes) } + assertFalse { engine.verify(CompositeSignaturesWithKeys(listOf(charlieSignature)).serialize().bytes) } + assertTrue { engine.verify(CompositeSignaturesWithKeys(listOf(aliceSignature, bobSignature)).serialize().bytes) } + assertTrue { engine.verify(CompositeSignaturesWithKeys(listOf(aliceSignature, charlieSignature)).serialize().bytes) } + assertTrue { engine.verify(CompositeSignaturesWithKeys(listOf(bobSignature, charlieSignature)).serialize().bytes) } + assertTrue { engine.verify(CompositeSignaturesWithKeys(listOf(aliceSignature, bobSignature, charlieSignature)).serialize().bytes) } // Check the underlying signature is validated val brokenBobSignature = TransactionSignature(aliceSignature.bytes, bobSignature.by, SignatureMetadata(1, Crypto.findSignatureScheme(bobSignature.by).schemeNumberID)) - assertFalse { engine.verify(listOf(aliceSignature, brokenBobSignature).serialize().bytes) } + assertFalse { engine.verify(CompositeSignaturesWithKeys(listOf(aliceSignature, brokenBobSignature)).serialize().bytes) } } @Test() @@ -216,7 +217,7 @@ class CompositeKeyTests : TestDependencyInjectionBase() { } @Test() - fun `composite key validation with graph cycle detection`() = kryoSpecific { + fun `composite key validation with graph cycle detection`() = kryoSpecific("Cycle exists in the object graph which is not currently supported in AMQP mode") { val key1 = CompositeKey.Builder().addKeys(alicePublicKey, bobPublicKey).build() as CompositeKey val key2 = CompositeKey.Builder().addKeys(alicePublicKey, key1).build() as CompositeKey val key3 = CompositeKey.Builder().addKeys(alicePublicKey, key2).build() as CompositeKey diff --git a/node-api/src/test/kotlin/net/corda/nodeapi/AttachmentClassLoaderTests.kt b/node-api/src/test/kotlin/net/corda/nodeapi/AttachmentClassLoaderTests.kt index 093479b20b..e566f957dc 100644 --- a/node-api/src/test/kotlin/net/corda/nodeapi/AttachmentClassLoaderTests.kt +++ b/node-api/src/test/kotlin/net/corda/nodeapi/AttachmentClassLoaderTests.kt @@ -331,8 +331,7 @@ class AttachmentClassLoaderTests : TestDependencyInjectionBase() { } @Test - // Kryo verifies/loads attachments on deserialization, whereas AMQP currently does not - fun `test deserialize of WireTransaction where contract cannot be found`() = kryoSpecific { + fun `test deserialize of WireTransaction where contract cannot be found`() = kryoSpecific("Kryo verifies/loads attachments on deserialization, whereas AMQP currently does not") { val child = ClassLoaderForTests() val contractClass = Class.forName("net.corda.contracts.isolated.AnotherDummyContract", true, child) val contract = contractClass.newInstance() as DummyContractBackdoor diff --git a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/ListsSerializationTest.kt b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/ListsSerializationTest.kt new file mode 100644 index 0000000000..09941c8afc --- /dev/null +++ b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/ListsSerializationTest.kt @@ -0,0 +1,26 @@ +package net.corda.nodeapi.internal.serialization + +import net.corda.core.serialization.SerializedBytes +import net.corda.core.serialization.deserialize +import net.corda.core.serialization.serialize +import net.corda.testing.TestDependencyInjectionBase +import net.corda.testing.kryoSpecific +import org.junit.Test +import kotlin.test.assertEquals + +class ListsSerializationTest : TestDependencyInjectionBase() { + + @Test + fun `check list can be serialized as root of serialization graph`() = kryoSpecific("AMQP doesn't support lists as the root of serialization graph"){ + assertEqualAfterRoundTripSerialization(listOf(1)) + assertEqualAfterRoundTripSerialization(listOf(1, 2)) + } + + private inline fun assertEqualAfterRoundTripSerialization(obj: T) { + + val serializedForm: SerializedBytes = obj.serialize() + val deserializedInstance = serializedForm.deserialize() + + assertEquals(obj, deserializedInstance) + } +} \ No newline at end of file diff --git a/test-utils/src/main/kotlin/net/corda/testing/CoreTestUtils.kt b/test-utils/src/main/kotlin/net/corda/testing/CoreTestUtils.kt index fc921d0acd..9ebc941fad 100644 --- a/test-utils/src/main/kotlin/net/corda/testing/CoreTestUtils.kt +++ b/test-utils/src/main/kotlin/net/corda/testing/CoreTestUtils.kt @@ -222,8 +222,8 @@ fun getTestPartyAndCertificate(name: X500Name, publicKey: PublicKey, trustRoot: return getTestPartyAndCertificate(Party(name, publicKey), trustRoot) } -inline fun kryoSpecific(function: () -> Unit) = if(!AMQP_ENABLED) { +inline fun kryoSpecific(reason: String, function: () -> Unit) = if(!AMQP_ENABLED) { function() } else { - loggerFor().info("Ignoring Kryo specific test") + loggerFor().info("Ignoring Kryo specific test, reason: $reason" ) } \ No newline at end of file