ExternalVendorCode/corda
1
0
Fork 0
mirror of https://github.com/corda/corda.git synced 2025-04-12 13:48:14 +00:00
Code Issues Actions 2 Packages Projects Releases Wiki Activity

Merge pull request #1668 from corda/feature/kat/carpenterRespectWhitelistPArt2

Browse Source 
CORDA-601 - Tests to ensure carpenter isn't exposed by unwhitlisted types
This commit is contained in:
Katelyn Baker 2017-09-27 23:36:44 +01:00 committed by GitHub
commit 006df7f23c
9 changed files with 321 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/CordaClassResolver.kt
View File

@ -11,6 +11,7 @@ import net.corda.core.serialization.ClassWhitelist
import net.corda.core.serialization.CordaSerializable
import net.corda.core.serialization.SerializationContext
import net.corda.core.utilities.loggerFor
import net.corda.nodeapi.internal.serialization.amqp.hasAnnotationInHierarchy
import java.io.PrintWriter
import java.lang.reflect.Modifier.isAbstract
import java.nio.charset.StandardCharsets
@ -115,13 +116,7 @@ class CordaClassResolver(serializationContext: SerializationContext) : DefaultCl
return (type.classLoader !is AttachmentsClassLoader)
&& !KryoSerializable::class.java.isAssignableFrom(type)
&& !type.isAnnotationPresent(DefaultSerializer::class.java)
&& (type.isAnnotationPresent(CordaSerializable::class.java) || hasInheritedAnnotation(type))
}
// Recursively check interfaces for our annotation.
private fun hasInheritedAnnotation(type: Class<*>): Boolean {
return type.interfaces.any { it.isAnnotationPresent(CordaSerializable::class.java) || hasInheritedAnnotation(it) }
|| (type.superclass != null && hasInheritedAnnotation(type.superclass))
&& (type.isAnnotationPresent(CordaSerializable::class.java) || whitelist.hasAnnotationInHierarchy(type))
}
// Need to clear out class names from attachments.

24
node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializationHelper.kt
View File

@ -1,5 +1,7 @@
package net.corda.nodeapi.internal.serialization.amqp
import net.corda.core.serialization.ClassWhitelist
import net.corda.core.serialization.CordaSerializable
import com.google.common.primitives.Primitives
import com.google.common.reflect.TypeToken
import net.corda.core.serialization.SerializationContext
@ -51,7 +53,7 @@ internal fun constructorForDeserialization(type: Type): KFunction<Any>? {
}
}
return preferredCandidate?.apply { isAccessible = true}
return preferredCandidate?.apply { isAccessible = true }
?: throw NotSerializableException("No constructor for deserialization found for $clazz.")
} else {
return null
@ -81,7 +83,7 @@ private fun <T : Any> propertiesForSerializationFromConstructor(kotlinConstructo
val name = param.name ?: throw NotSerializableException("Constructor parameter of $clazz has no name.")
val matchingProperty = properties[name] ?:
throw NotSerializableException("No property matching constructor parameter named $name of $clazz." +
" If using Java, check that you have the -parameters option specified in the Java compiler.")
" If using Java, check that you have the -parameters option specified in the Java compiler.")
// Check that the method has a getter in java.
val getter = matchingProperty.readMethod ?: throw NotSerializableException("Property has no getter method for $name of $clazz." +
" If using Java and the parameter name looks anonymous, check that you have the -parameters option specified in the Java compiler.")
@ -123,7 +125,7 @@ private fun exploreType(type: Type?, interfaces: MutableSet<Type>, serializerFac
val clazz = type?.asClass()
if (clazz != null) {
if (clazz.isInterface) {
if(serializerFactory.isNotWhitelisted(clazz)) return // We stop exploring once we reach a branch that has no `CordaSerializable` annotation or whitelisting.
if (serializerFactory.whitelist.isNotWhitelisted(clazz)) return // We stop exploring once we reach a branch that has no `CordaSerializable` annotation or whitelisting.
else interfaces += type
}
for (newInterface in clazz.genericInterfaces) {
@ -263,3 +265,19 @@ private fun Throwable.setMessage(newMsg: String) {
detailMessageField.isAccessible = true
detailMessageField.set(this, newMsg)
}
fun ClassWhitelist.requireWhitelisted(type: Type) {
if (!this.isWhitelisted(type.asClass()!!)) {
throw NotSerializableException("Class $type is not on the whitelist or annotated with @CordaSerializable.")
}
}
fun ClassWhitelist.isWhitelisted(clazz: Class<*>) = (hasListed(clazz) || hasAnnotationInHierarchy(clazz))
fun ClassWhitelist.isNotWhitelisted(clazz: Class<*>) = !(this.isWhitelisted(clazz))
// Recursively check the class, interfaces and superclasses for our annotation.
fun ClassWhitelist.hasAnnotationInHierarchy(type: Class<*>): Boolean {
return type.isAnnotationPresent(CordaSerializable::class.java)
|| type.interfaces.any { hasAnnotationInHierarchy(it) }
|| (type.superclass != null && hasAnnotationInHierarchy(type.superclass))
}

29
node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializerFactory.kt
View File

@ -4,7 +4,6 @@ import com.google.common.primitives.Primitives
import com.google.common.reflect.TypeResolver
import net.corda.core.internal.uncheckedCast
import net.corda.core.serialization.ClassWhitelist
import net.corda.core.serialization.CordaSerializable
import net.corda.nodeapi.internal.serialization.carpenter.*
import org.apache.qpid.proton.amqp.*
import java.io.NotSerializableException
@ -31,11 +30,11 @@ data class FactorySchemaAndDescriptor(val schema: Schema, val typeDescriptor: An
// TODO: need to rethink matching of constructor to properties in relation to implementing interfaces and needing those properties etc.
// TODO: need to support super classes as well as interfaces with our current code base... what's involved? If we continue to ban, what is the impact?
@ThreadSafe
class SerializerFactory(val whitelist: ClassWhitelist, cl: ClassLoader) {
open class SerializerFactory(val whitelist: ClassWhitelist, cl: ClassLoader) {
private val serializersByType = ConcurrentHashMap<Type, AMQPSerializer<Any>>()
private val serializersByDescriptor = ConcurrentHashMap<Any, AMQPSerializer<Any>>()
private val customSerializers = CopyOnWriteArrayList<CustomSerializer<out Any>>()
val classCarpenter = ClassCarpenter(cl, whitelist)
open val classCarpenter = ClassCarpenter(cl, whitelist)
val classloader: ClassLoader
get() = classCarpenter.classloader
@ -82,7 +81,7 @@ class SerializerFactory(val whitelist: ClassWhitelist, cl: ClassLoader) {
}
}
Enum::class.java.isAssignableFrom(actualClass ?: declaredClass) -> serializersByType.computeIfAbsent(actualClass ?: declaredClass) {
whitelisted(actualType)
whitelist.requireWhitelisted(actualType)
EnumSerializer(actualType, actualClass ?: declaredClass, this)
}
else -> makeClassSerializer(actualClass ?: declaredClass, actualType, declaredType)
@ -243,10 +242,10 @@ class SerializerFactory(val whitelist: ClassWhitelist, cl: ClassLoader) {
if (clazz.componentType.isPrimitive) PrimArraySerializer.make(type, this)
else ArraySerializer.make(type, this)
} else if (clazz.kotlin.objectInstance != null) {
whitelisted(clazz)
whitelist.requireWhitelisted(clazz)
SingletonSerializer(clazz, clazz.kotlin.objectInstance!!, this)
} else {
whitelisted(type)
whitelist.requireWhitelisted(type)
ObjectSerializer(type, this)
}
}
@ -271,24 +270,6 @@ class SerializerFactory(val whitelist: ClassWhitelist, cl: ClassLoader) {
return null
}
private fun whitelisted(type: Type) {
val clazz = type.asClass()!!
if (isNotWhitelisted(clazz)) {
throw NotSerializableException("Class $type is not on the whitelist or annotated with @CordaSerializable.")
}
}
// Ignore SimpleFieldAccess as we add it to anything we build in the carpenter.
internal fun isNotWhitelisted(clazz: Class<*>): Boolean = clazz == SimpleFieldAccess::class.java ||
(!whitelist.hasListed(clazz) && !hasAnnotationInHierarchy(clazz))
// Recursively check the class, interfaces and superclasses for our annotation.
private fun hasAnnotationInHierarchy(type: Class<*>): Boolean {
return type.isAnnotationPresent(CordaSerializable::class.java) ||
type.interfaces.any { hasAnnotationInHierarchy(it) }
|| (type.superclass != null && hasAnnotationInHierarchy(type.superclass))
}
private fun makeMapSerializer(declaredType: ParameterizedType): AMQPSerializer<Any> {
val rawType = declaredType.rawType as Class<*>
rawType.checkSupportedMapType()

35
node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/ClassCarpenter.kt
View File

@ -6,10 +6,8 @@ import org.objectweb.asm.ClassWriter
import org.objectweb.asm.MethodVisitor
import org.objectweb.asm.Opcodes.*
import org.objectweb.asm.Type
import java.lang.Character.isJavaIdentifierPart
import java.lang.Character.isJavaIdentifierStart
import java.util.*
/**
@ -17,6 +15,7 @@ import java.util.*
* as if `this.class.getMethod("get" + name.capitalize()).invoke(this)` had been called. It is intended as a more
* convenient alternative to reflection.
*/
@CordaSerializable
interface SimpleFieldAccess {
operator fun get(name: String): Any?
}
@ -134,7 +133,10 @@ class ClassCarpenter(cl: ClassLoader = Thread.currentThread().contextClassLoader
visit(TARGET_VERSION, ACC_PUBLIC + ACC_FINAL + ACC_SUPER + ACC_ENUM, schema.jvmName,
"L$jlEnum<L${schema.jvmName};>;", jlEnum, null)
visitAnnotation(Type.getDescriptor(CordaSerializable::class.java), true).visitEnd()
if (schema.flags.cordaSerializable()) {
visitAnnotation(Type.getDescriptor(CordaSerializable::class.java), true).visitEnd()
}
generateFields(schema)
generateStaticEnumConstructor(schema)
generateEnumConstructor()
@ -151,8 +153,10 @@ class ClassCarpenter(cl: ClassLoader = Thread.currentThread().contextClassLoader
cw.apply {
visit(TARGET_VERSION, ACC_PUBLIC + ACC_ABSTRACT + ACC_INTERFACE, schema.jvmName, null,
jlObject, interfaces)
visitAnnotation(Type.getDescriptor(CordaSerializable::class.java), true).visitEnd()
if (schema.flags.cordaSerializable()) {
visitAnnotation(Type.getDescriptor(CordaSerializable::class.java), true).visitEnd()
}
generateAbstractGetters(schema)
}.visitEnd()
}
@ -163,20 +167,25 @@ class ClassCarpenter(cl: ClassLoader = Thread.currentThread().contextClassLoader
val superName = schema.superclass?.jvmName ?: jlObject
val interfaces = schema.interfaces.map { it.name.jvm }.toMutableList()
if (SimpleFieldAccess::class.java !in schema.interfaces) {
if (SimpleFieldAccess::class.java !in schema.interfaces
&& schema.flags.cordaSerializable()
&& schema.flags.simpleFieldAccess()) {
interfaces.add(SimpleFieldAccess::class.java.name.jvm)
}
cw.apply {
visit(TARGET_VERSION, ACC_PUBLIC + ACC_SUPER, schema.jvmName, null, superName,
interfaces.toTypedArray())
visitAnnotation(Type.getDescriptor(CordaSerializable::class.java), true).visitEnd()
if (schema.flags.cordaSerializable()) {
visitAnnotation(Type.getDescriptor(CordaSerializable::class.java), true).visitEnd()
}
generateFields(schema)
generateClassConstructor(schema)
generateGetters(schema)
if (schema.superclass == null)
if (schema.superclass == null) {
generateGetMethod() // From SimplePropertyAccess
}
generateToString(schema)
}.visitEnd()
}
@ -388,11 +397,21 @@ class ClassCarpenter(cl: ClassLoader = Thread.currentThread().contextClassLoader
}
}
/**
* If a sub element isn't whitelist we will not build a class containing that type as a member. Since, by
* default, classes created by the [ClassCarpenter] are annotated as [CordaSerializable] we will always
* be able to carpent classes generated from our AMQP library as, at a base level, we will either be able to
* create the lowest level in the meta hierarchy because either all members are jvm primitives or
* whitelisted classes
*/
private fun validateSchema(schema: Schema) {
if (schema.name in _loaded) throw DuplicateNameException()
fun isJavaName(n: String) = n.isNotBlank() && isJavaIdentifierStart(n.first()) && n.all(::isJavaIdentifierPart)
require(isJavaName(schema.name.split(".").last())) { "Not a valid Java name: ${schema.name}" }
schema.fields.keys.forEach { require(isJavaName(it)) { "Not a valid Java name: $it" } }
schema.fields.forEach {
require(isJavaName(it.key)) { "Not a valid Java name: $it" }
}
// Now check each interface we've been asked to implement, as the JVM will unfortunately only catch the
// fact that we didn't implement the interface we said we would at the moment the missing method is
// actually called, which is a bit too dynamic for my tastes.

20
node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/Schema.kt
View File

@ -1,8 +1,12 @@
package net.corda.nodeapi.internal.serialization.carpenter
import kotlin.collections.LinkedHashMap
import org.objectweb.asm.ClassWriter
import org.objectweb.asm.Opcodes.*
import java.util.*
enum class SchemaFlags {
SimpleFieldAccess, CordaSerializable
}
/**
* A Schema is the representation of an object the Carpenter can contsruct
@ -20,6 +24,8 @@ abstract class Schema(
updater: (String, Field) -> Unit) {
private fun Map<String, Field>.descriptors() = LinkedHashMap(this.mapValues { it.value.descriptor })
var flags : EnumMap<SchemaFlags, Boolean> = EnumMap(SchemaFlags::class.java)
init {
fields.forEach { updater(it.key, it.value) }
@ -41,6 +47,18 @@ abstract class Schema(
val asArray: String
get() = "[L$jvmName;"
fun unsetCordaSerializable() {
flags.replace (SchemaFlags.CordaSerializable, false)
}
}
fun EnumMap<SchemaFlags, Boolean>.cordaSerializable() : Boolean {
return this.getOrDefault(SchemaFlags.CordaSerializable, true) == true
}
fun EnumMap<SchemaFlags, Boolean>.simpleFieldAccess() : Boolean {
return this.getOrDefault(SchemaFlags.SimpleFieldAccess, true) == true
}
/**

144
node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/StaticInitialisationOfSerializedObjectTest.kt Normal file
View File

@ -0,0 +1,144 @@
package net.corda.nodeapi.internal.serialization.amqp
import net.corda.core.serialization.ClassWhitelist
import net.corda.core.serialization.SerializedBytes
import net.corda.nodeapi.internal.serialization.AllWhitelist
import net.corda.nodeapi.internal.serialization.carpenter.ClassCarpenter
import org.assertj.core.api.Assertions
import org.junit.Test
import java.io.File
import java.io.NotSerializableException
import java.lang.reflect.Type
import java.util.concurrent.ConcurrentHashMap
import kotlin.test.assertEquals
class InStatic : Exception ("Help!, help!, I'm being repressed")
class C {
companion object {
init {
throw InStatic()
}
}
}
// To re-setup the resource file for the tests
// * deserializeTest
// * deserializeTest2
// comment out the companion object from here, comment out the test code and uncomment
// the generation code, then re-run the test and copy the file shown in the output print
// to the resource directory
class C2 (var b: Int) {
/*
companion object {
init {
throw InStatic()
}
}
*/
}
class StaticInitialisationOfSerializedObjectTest {
@Test(expected=java.lang.ExceptionInInitializerError::class)
fun itBlowsUp() {
C()
}
@Test
fun KotlinObjectWithCompanionObject() {
data class D (val c : C)
val sf = SerializerFactory(AllWhitelist, ClassLoader.getSystemClassLoader())
val typeMap = sf::class.java.getDeclaredField("serializersByType")
typeMap.isAccessible = true
@Suppress("UNCHECKED_CAST")
val serialisersByType = typeMap.get(sf) as ConcurrentHashMap<Type, AMQPSerializer<Any>>
// pre building a serializer, we shouldn't have anything registered
assertEquals(0, serialisersByType.size)
// build a serializer for type D without an instance of it to serialise, since
// we can't actually construct one
sf.get(null, D::class.java)
// post creation of the serializer we should have one element in the map, this
// proves we didn't statically construct an instance of C when building the serializer
assertEquals(1, serialisersByType.size)
}
@Test
fun deserializeTest() {
data class D (val c : C2)
val path = EvolvabilityTests::class.java.getResource("StaticInitialisationOfSerializedObjectTest.deserializeTest")
val f = File(path.toURI())
// Original version of the class for the serialised version of this class
//
//val sf1 = SerializerFactory(AllWhitelist, ClassLoader.getSystemClassLoader())
//val sc = SerializationOutput(sf1).serialize(D(C2(20)))
//f.writeBytes(sc.bytes)
//println (path)
class WL : ClassWhitelist {
override fun hasListed(type: Class<*>) =
type.name == "net.corda.nodeapi.internal.serialization.amqp" +
".StaticInitialisationOfSerializedObjectTest\$deserializeTest\$D"
}
val sf2 = SerializerFactory(WL(), ClassLoader.getSystemClassLoader())
val bytes = f.readBytes()
Assertions.assertThatThrownBy {
DeserializationInput(sf2).deserialize(SerializedBytes<D>(bytes))
}.isInstanceOf(NotSerializableException::class.java)
}
// Version of a serializer factory that will allow the class carpenter living on the
// factory to have a different whitelist applied to it than the factory
class TestSerializerFactory(wl1: ClassWhitelist, wl2: ClassWhitelist) :
SerializerFactory (wl1, ClassLoader.getSystemClassLoader()) {
override val classCarpenter = ClassCarpenter(ClassLoader.getSystemClassLoader(), wl2)
}
// This time have the serilization factory and the carpenter use different whitelists
@Test
fun deserializeTest2() {
data class D (val c : C2)
val path = EvolvabilityTests::class.java.getResource("StaticInitialisationOfSerializedObjectTest.deserializeTest2")
val f = File(path.toURI())
// Original version of the class for the serialised version of this class
//
//val sf1 = SerializerFactory(AllWhitelist, ClassLoader.getSystemClassLoader())
//val sc = SerializationOutput(sf1).serialize(D(C2(20)))
//f.writeBytes(sc.bytes)
//println (path)
// whitelist to be used by the serialisation factory
class WL1 : ClassWhitelist {
override fun hasListed(type: Class<*>) =
type.name == "net.corda.nodeapi.internal.serialization.amqp" +
".StaticInitialisationOfSerializedObjectTest\$deserializeTest\$D"
}
// whitelist to be used by the carpenter
class WL2 : ClassWhitelist {
override fun hasListed(type: Class<*>) = true
}
val sf2 = TestSerializerFactory(WL1(), WL2())
val bytes = f.readBytes()
// Deserializing should throw because C is not on the whitelist NOT because
// we ever went anywhere near statically constructing it prior to not actually
// creating an instance of it
Assertions.assertThatThrownBy {
DeserializationInput(sf2).deserialize(SerializedBytes<D>(bytes))
}.isInstanceOf(NotSerializableException::class.java)
}
}

103
node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/carpenter/ClassCarpenterWhitelistTest.kt Normal file
View File

@ -0,0 +1,103 @@
package net.corda.nodeapi.internal.serialization.carpenter
import net.corda.core.serialization.ClassWhitelist
import net.corda.core.serialization.CordaSerializable
import org.assertj.core.api.Assertions
import org.junit.Ignore
import org.junit.Test
import java.io.NotSerializableException
class ClassCarpenterWhitelistTest {
// whitelisting a class on the class path will mean we will carpente up a class that
// contains it as a member
@Test
fun whitelisted() {
data class A(val a: Int)
class WL : ClassWhitelist {
private val allowedClasses = hashSetOf<String>(
A::class.java.name
)
override fun hasListed(type: Class<*>): Boolean = type.name in allowedClasses
}
val cc = ClassCarpenter(whitelist = WL())
// if this works, the test works, if it throws then we're in a world of pain, we could
// go further but there are a lot of other tests that test weather we can build
// carpented objects
cc.build(ClassSchema("thing", mapOf("a" to NonNullableField(A::class.java))))
}
@Test
@Ignore("Currently the carpenter doesn't inspect it's whitelist so will carpent anything" +
"it's asked relying on the serializer factory to not ask for anything")
fun notWhitelisted() {
data class A(val a: Int)
class WL : ClassWhitelist {
override fun hasListed(type: Class<*>) = false
}
val cc = ClassCarpenter(whitelist = WL())
// Class A isn't on the whitelist, so we should fail to carpent it
Assertions.assertThatThrownBy {
cc.build(ClassSchema("thing", mapOf("a" to NonNullableField(A::class.java))))
}.isInstanceOf(NotSerializableException::class.java)
}
// despite now being whitelisted and on the class path, we will carpent this because
// it's marked as CordaSerializable
@Test
fun notWhitelistedButAnnotated() {
@CordaSerializable data class A(val a: Int)
class WL : ClassWhitelist {
override fun hasListed(type: Class<*>) = false
}
val cc = ClassCarpenter(whitelist = WL())
// again, simply not throwing here is enough to show the test worked and the carpenter
// didn't reject the type even though it wasn't on the whitelist because it was
// annotated properly
cc.build(ClassSchema("thing", mapOf("a" to NonNullableField(A::class.java))))
}
@Test
@Ignore("Currently the carpenter doesn't inspect it's whitelist so will carpent anything" +
"it's asked relying on the serializer factory to not ask for anything")
fun notWhitelistedButCarpented() {
// just have the white list reject *Everything* except ints
class WL : ClassWhitelist {
override fun hasListed(type: Class<*>) = type.name == "int"
}
val cc = ClassCarpenter(whitelist = WL())
val schema1a = ClassSchema("thing1a", mapOf("a" to NonNullableField(Int::class.java)))
// thing 1 won't be set as corda serializable, meaning we won't build schema 2
schema1a.unsetCordaSerializable()
val clazz1a = cc.build(schema1a)
val schema2 = ClassSchema("thing2", mapOf("a" to NonNullableField(clazz1a)))
// thing 2 references thing 1 which wasn't carpented as corda s erializable and thus
// this will fail
Assertions.assertThatThrownBy {
cc.build(schema2)
}.isInstanceOf(NotSerializableException::class.java)
// create a second type of schema1, this time leave it as corda serialzable
val schema1b = ClassSchema("thing1b", mapOf("a" to NonNullableField(Int::class.java)))
val clazz1b = cc.build(schema1b)
// since schema 1b was created as CordaSerializable this will work
ClassSchema("thing2", mapOf("a" to NonNullableField(clazz1b)))
}
}

BIN
node-api/src/test/resources/net/corda/nodeapi/internal/serialization/amqp/StaticInitialisationOfSerializedObjectTest.deserializeTest Normal file
View File

Binary file not shown.

BIN
node-api/src/test/resources/net/corda/nodeapi/internal/serialization/amqp/StaticInitialisationOfSerializedObjectTest.deserializeTest2 Normal file
View File

Binary file not shown.