From 429da85650cdf2f23c1b261f094c2741e98a72b2 Mon Sep 17 00:00:00 2001 From: Katelyn Baker Date: Mon, 5 Feb 2018 12:07:02 +0000 Subject: [PATCH] CORDA-946 - Fixes to fingerprinting breaks backward compatibility (#2453) * CORDA-946 - Fixes to fingerprinting breaks backward compatibility Demonstrated using the network map parameters signed form as that's where the problem was first seen * Review Comments --- .../serialization/amqp/EvolutionSerializer.kt | 22 +++-- .../internal/serialization/amqp/Schema.kt | 2 +- .../carpenter/AMQPSchemaExtensions.kt | 13 +-- .../serialization/amqp/EvolvabilityTests.kt | 76 ++++++++++++++++++ .../amqp/networkParams.r3corda.6a6b6f256 | Bin 0 -> 3066 bytes 5 files changed, 101 insertions(+), 12 deletions(-) create mode 100644 node-api/src/test/resources/net/corda/nodeapi/internal/serialization/amqp/networkParams.r3corda.6a6b6f256 diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/EvolutionSerializer.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/EvolutionSerializer.kt index 103e0d082e..78ca924deb 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/EvolutionSerializer.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/EvolutionSerializer.kt @@ -177,12 +177,24 @@ class EvolutionSerializerGetter : EvolutionSerializerGetterBase() { override fun getEvolutionSerializer(factory: SerializerFactory, typeNotation: TypeNotation, newSerializer: AMQPSerializer, - schemas: SerializationSchemas): AMQPSerializer = - factory.getSerializersByDescriptor().computeIfAbsent(typeNotation.descriptor.name!!) { - when (typeNotation) { - is CompositeType -> EvolutionSerializer.make(typeNotation, newSerializer as ObjectSerializer, factory) - is RestrictedType -> EnumEvolutionSerializer.make(typeNotation, newSerializer, factory, schemas) + schemas: SerializationSchemas): AMQPSerializer { + return factory.getSerializersByDescriptor().computeIfAbsent(typeNotation.descriptor.name!!) { + when (typeNotation) { + is CompositeType -> EvolutionSerializer.make(typeNotation, newSerializer as ObjectSerializer, factory) + is RestrictedType -> { + // The fingerprint of a generic collection can be changed through bug fixes to the + // fingerprinting function making it appear as if the class has altered whereas it hasn't. + // Given we don't support the evolution of these generic containers, if it appears + // one has been changed, simply return the original serializer and associate it with + // both the new and old fingerprint + if (newSerializer is CollectionSerializer || newSerializer is MapSerializer) { + newSerializer + } else { + EnumEvolutionSerializer.make(typeNotation, newSerializer, factory, schemas) + } } } + } + } } diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/Schema.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/Schema.kt index 372cfed9be..e309882d7b 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/Schema.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/Schema.kt @@ -460,6 +460,6 @@ private fun fingerprintForObject( .putUnencodedChars(prop.getter.name) .putUnencodedChars(if (prop.getter.mandatory) NOT_NULLABLE_HASH else NULLABLE_HASH) } - interfacesForSerialization(type, factory).map { fingerprintForType(it, type, alreadySeen, hasher, factory, debugIndent+4) } + interfacesForSerialization(type, factory).map { fingerprintForType(it, type, alreadySeen, hasher, factory, debugIndent+1) } return hasher } diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/AMQPSchemaExtensions.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/AMQPSchemaExtensions.kt index 1608a22e83..2c7dea9605 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/AMQPSchemaExtensions.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/AMQPSchemaExtensions.kt @@ -120,16 +120,17 @@ val typeStrToType: Map, Class> = mapOf( Pair("byte", false) to Byte::class.javaObjectType ) +fun String.stripGenerics() : String = if(this.endsWith('>')) { + this.substring(0, this.indexOf('<')) +} else this + fun AMQPField.getTypeAsClass(classloader: ClassLoader) = typeStrToType[Pair(type, mandatory)] ?: when (type) { "string" -> String::class.java "binary" -> ByteArray::class.java - "*" -> if (requires.isEmpty()) Any::class.java else classloader.loadClass(requires[0]) - else -> { - classloader.loadClass( - if (type.endsWith("?>")) { - type.substring(0, type.indexOf('<')) - } else type) + "*" -> if (requires.isEmpty()) Any::class.java else { + classloader.loadClass(requires[0].stripGenerics()) } + else -> classloader.loadClass(type.stripGenerics()) } fun AMQPField.validateType(classloader: ClassLoader) = when (type) { diff --git a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/EvolvabilityTests.kt b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/EvolvabilityTests.kt index 164feb2bab..b18abd9f82 100644 --- a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/EvolvabilityTests.kt +++ b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/EvolvabilityTests.kt @@ -1,12 +1,21 @@ package net.corda.nodeapi.internal.serialization.amqp +import net.corda.core.crypto.Crypto.generateKeyPair +import net.corda.core.crypto.SignedData +import net.corda.core.crypto.sign import net.corda.core.serialization.DeprecatedConstructorForDeserialization import net.corda.core.serialization.SerializedBytes +import net.corda.nodeapi.internal.network.NetworkParameters +import net.corda.nodeapi.internal.network.NotaryInfo import net.corda.testing.common.internal.ProjectStructure.projectRootDir +import net.corda.testing.core.DUMMY_NOTARY_NAME +import net.corda.testing.core.TestIdentity +import org.junit.Ignore import org.junit.Test import java.io.File import java.io.NotSerializableException import java.net.URI +import java.time.Instant import kotlin.test.assertEquals // To regenerate any of the binary test files do the following @@ -462,4 +471,71 @@ class EvolvabilityTests { assertEquals(f, db3.f) assertEquals(-1, db3.g) } + + // + // This test uses a NetworkParameters signed set of bytes generated by R3 Corda and + // is here to ensure we can still read them. This test exists because of the break in + // being able to deserialize an object serialized prior to some fixes to the fingerprinter. + // + // The file itself was generated from R3 Corda at commit + // 6a6b6f256 Skip cache invalidation during init() - caches are still null. + // + // To regenerate the file un-ignore the test below this one (regenerate broken network parameters), + // to regenerate at a specific version add that test to a checkout at the desired sha then take + // the resulting file and add to the repo, changing the filename as appropriate + // + @Test + fun readBrokenNetworkParameters(){ + val sf = testDefaultFactory() + sf.register(net.corda.nodeapi.internal.serialization.amqp.custom.InstantSerializer(sf)) + sf.register(net.corda.nodeapi.internal.serialization.amqp.custom.PublicKeySerializer) + + // + // filename breakdown + // networkParams - because this is a serialised set of network parameters + // r3corda - generated by R3 Corda instead of Corda + // 6a6b6f256 - Commit sha of the build that generated the file we're testing against + // + val resource = "networkParams.r3corda.6a6b6f256" + + val path = EvolvabilityTests::class.java.getResource(resource) + val f = File(path.toURI()) + val sc2 = f.readBytes() + val deserializedC = DeserializationInput(sf).deserialize(SerializedBytes>(sc2)) + val networkParams = DeserializationInput(sf).deserialize(deserializedC.raw) + + assertEquals(1000, networkParams.maxMessageSize) + assertEquals(1000, networkParams.maxTransactionSize) + assertEquals(3, networkParams.minimumPlatformVersion) + assertEquals(1, networkParams.notaries.size) + assertEquals(TestIdentity(DUMMY_NOTARY_NAME, 20).party, networkParams.notaries.firstOrNull()?.identity) + } + + // + // This test created a serialized and signed set of Network Parameters to test whether we + // can still deserialize them + // + @Test + @Ignore("This test simply regenerates the test file used for readBrokenNetworkParameters") + fun `regenerate broken network parameters`() { + // note: 6a6b6f256 is the sha that generates the file + val resource = "networkParams.." + val DUMMY_NOTARY = TestIdentity(DUMMY_NOTARY_NAME, 20).party + val networkParameters = NetworkParameters( + 3, listOf(NotaryInfo(DUMMY_NOTARY, false)),1000, 1000, Instant.EPOCH, 1 ) + + val sf = testDefaultFactory() + sf.register(net.corda.nodeapi.internal.serialization.amqp.custom.InstantSerializer(sf)) + sf.register(net.corda.nodeapi.internal.serialization.amqp.custom.PublicKeySerializer) + + val testOutput = TestSerializationOutput(true, sf) + val serialized = testOutput.serialize(networkParameters) + val keyPair = generateKeyPair() + val sig = keyPair.private.sign(serialized.bytes, keyPair.public) + val signed = SignedData(serialized, sig) + val signedAndSerialized = testOutput.serialize(signed) + + File(URI("$localPath/$resource")).writeBytes( signedAndSerialized.bytes) + } + } \ No newline at end of file diff --git a/node-api/src/test/resources/net/corda/nodeapi/internal/serialization/amqp/networkParams.r3corda.6a6b6f256 b/node-api/src/test/resources/net/corda/nodeapi/internal/serialization/amqp/networkParams.r3corda.6a6b6f256 new file mode 100644 index 0000000000000000000000000000000000000000..dcdbaa7b5f0d92f240af49f1068fcaee9c5b77f2 GIT binary patch literal 3066 zcmcImTWl0n7@ob%P7#CHh$xCF4XB|yy_YJbOt(99mtH7zw`{j2=Ire0_O!ESmYK8N zZh0|E@bYAg#>51}u@Al?FG5Vr7}RJ&q{f&iQ4+(8C{bffG=Zr9Y;`-kI)#StGM(=E z=D&Ua_y0#T%nb4gg7C~oX?O~PZ%Gi=Uxt@}u-Hg3lQjqNfa7^fqH= zSKK_1-LnVK(se-ltI#vFYdWv0X5T_vegMyQV5`u*|KKvZ-f?mYK3i9|TwkQQ zbDb0OXl&k^=uh^yCJr4?2ZoxLefwg74#lfujppZKLlZND(PYQ%2-aYjV1+yQDj~hLEE04I>6aW~pHen{!0NJd`S!M4MK)zdtH0Zd)n! z44Ol-g*7;4C3&=v)`>QVOD7*|+1Vn70!=s{*e0C+>yJa%qd%Wl>pC{E_tIZZUYaa) z`zNn$FzPOTcjgNzd;}Q$zWfjtAE+_TNDj*}YB5BaE&3BeT!{NB9pDE}#it#XoR@KU z5i4gR{N#Jb`H6Q_jLOu=V3a2^p$wZ8=`tJ@4Rcl=S^hhUOq9cLW+`gLRKR}N@9r=! zmKDckuS0X?)1bXW;`inVCAmUwR7Y&eFmvNzo*0w|APSZHT9m%9>w~z&>!?Ej!4~)J zv=txUQ!aFY5nUc47VEj0e99EY=#5c%V4D!A+GT8CabR&3(0L>aW3p#?|=QwFl)TUVUJ7{c~p;<1a4MWF>s$85xx`o}XAljdcH?Y1+Dqf>Tv5&e_X_t7Af~5EkuQd9d zsh#X>X&C`nZ8}hibw-gwDlk_ZHEi+m%^qW?s=dnckCf$qbDmd8)oWTR_GP!yw?OGN zBbPI%lL-%5%!Ifhqp(}v4HDmIm{~-L1r!-HK?(E1J_w?6V_mqak#QgNi*9H@JM_zn z-zv-|P#cU<<{8*#iA$PMpv?5JvF-!dfbKb%jx{G^bpPl^;&^4$q!_y>wyk z@6VyDXDWF6Lej}HM?U~lGYd@7^aTXV0 zu+C&Bi!pqpfT@OiD>bj3l3sT*#co{L{XyI5=(JN79!e7m7na(jPK&n`-|b5zu2tMEkn9R#I~VKfKPqv5dQD^chAcWm=Wk|j B^OXPq literal 0 HcmV?d00001