From 80c00b920b56fc2e67abdd6bfc7ce160c4e04365 Mon Sep 17 00:00:00 2001 From: Mike Hearn Date: Fri, 2 Mar 2018 15:10:54 +0100 Subject: [PATCH] Fix another non-serializable exception, add docs, fix a possible security issue. (#2707) * Fix another non-serializable exception, add docs, fix a possible security issue. * Update API definition to reflect methods added to make more exceptions serializable --- .ci/api-current.txt | 12 ++++++++ .../kotlin/net/corda/core/CordaInternal.kt | 2 +- .../net/corda/core/flows/FlowLogicRef.kt | 30 ++++++++++++++++--- .../statemachine/FlowLogicRefFactoryImpl.kt | 26 ++++++++++------ 4 files changed, 56 insertions(+), 14 deletions(-) diff --git a/.ci/api-current.txt b/.ci/api-current.txt index 34656174f5..bd17464368 100644 --- a/.ci/api-current.txt +++ b/.ci/api-current.txt @@ -563,15 +563,20 @@ public final class net.corda.core.contracts.TransactionStateKt extends java.lang ## @net.corda.core.serialization.CordaSerializable public static final class net.corda.core.contracts.TransactionVerificationException$ConflictingAttachmentsRejection extends net.corda.core.contracts.TransactionVerificationException public (net.corda.core.crypto.SecureHash, String) + @org.jetbrains.annotations.NotNull public final String getContractClass() ## @net.corda.core.serialization.CordaSerializable public static final class net.corda.core.contracts.TransactionVerificationException$ContractConstraintRejection extends net.corda.core.contracts.TransactionVerificationException public (net.corda.core.crypto.SecureHash, String) + @org.jetbrains.annotations.NotNull public final String getContractClass() ## @net.corda.core.serialization.CordaSerializable public static final class net.corda.core.contracts.TransactionVerificationException$ContractCreationError extends net.corda.core.contracts.TransactionVerificationException public (net.corda.core.crypto.SecureHash, String, Throwable) + @org.jetbrains.annotations.NotNull public final String getContractClass() ## @net.corda.core.serialization.CordaSerializable public static final class net.corda.core.contracts.TransactionVerificationException$ContractRejection extends net.corda.core.contracts.TransactionVerificationException + public (net.corda.core.crypto.SecureHash, String, Throwable) public (net.corda.core.crypto.SecureHash, net.corda.core.contracts.Contract, Throwable) + @org.jetbrains.annotations.NotNull public final String getContractClass() ## @net.corda.core.serialization.CordaSerializable public static final class net.corda.core.contracts.TransactionVerificationException$Direction extends java.lang.Enum protected (String, int) @@ -594,12 +599,17 @@ public final class net.corda.core.contracts.TransactionStateKt extends java.lang ## @net.corda.core.serialization.CordaSerializable public static final class net.corda.core.contracts.TransactionVerificationException$NotaryChangeInWrongTransactionType extends net.corda.core.contracts.TransactionVerificationException public (net.corda.core.crypto.SecureHash, net.corda.core.identity.Party, net.corda.core.identity.Party) + @org.jetbrains.annotations.NotNull public final net.corda.core.identity.Party getOutputNotary() + @org.jetbrains.annotations.NotNull public final net.corda.core.identity.Party getTxNotary() ## @net.corda.core.serialization.CordaSerializable public static final class net.corda.core.contracts.TransactionVerificationException$SignersMissing extends net.corda.core.contracts.TransactionVerificationException public (net.corda.core.crypto.SecureHash, List) + @org.jetbrains.annotations.NotNull public final List getMissing() ## @net.corda.core.serialization.CordaSerializable public static final class net.corda.core.contracts.TransactionVerificationException$TransactionMissingEncumbranceException extends net.corda.core.contracts.TransactionVerificationException public (net.corda.core.crypto.SecureHash, int, net.corda.core.contracts.TransactionVerificationException$Direction) + @org.jetbrains.annotations.NotNull public final net.corda.core.contracts.TransactionVerificationException$Direction getInOut() + public final int getMissing() ## @net.corda.core.serialization.CordaSerializable public abstract class net.corda.core.contracts.TypeOnlyCommandData extends java.lang.Object implements net.corda.core.contracts.CommandData public () @@ -1327,6 +1337,8 @@ public static final class net.corda.core.flows.FlowStackSnapshot$Frame extends j ## @net.corda.core.serialization.CordaSerializable public final class net.corda.core.flows.IllegalFlowLogicException extends java.lang.IllegalArgumentException public (Class, String) + public (String, String) + @org.jetbrains.annotations.NotNull public final String getType() ## public @interface net.corda.core.flows.InitiatedBy public abstract Class value() diff --git a/core/src/main/kotlin/net/corda/core/CordaInternal.kt b/core/src/main/kotlin/net/corda/core/CordaInternal.kt index 40f145fdec..eb13b5b3c5 100644 --- a/core/src/main/kotlin/net/corda/core/CordaInternal.kt +++ b/core/src/main/kotlin/net/corda/core/CordaInternal.kt @@ -6,6 +6,6 @@ package net.corda.core * These fields are only meant to be used by Corda internally, and are not intended to be part of the public API. */ @Retention(AnnotationRetention.BINARY) -@Target(AnnotationTarget.PROPERTY_GETTER, AnnotationTarget.PROPERTY_SETTER) +@Target(AnnotationTarget.PROPERTY_GETTER, AnnotationTarget.PROPERTY_SETTER, AnnotationTarget.FUNCTION) @MustBeDocumented annotation class CordaInternal \ No newline at end of file diff --git a/core/src/main/kotlin/net/corda/core/flows/FlowLogicRef.kt b/core/src/main/kotlin/net/corda/core/flows/FlowLogicRef.kt index 36b0971cb0..4558498114 100644 --- a/core/src/main/kotlin/net/corda/core/flows/FlowLogicRef.kt +++ b/core/src/main/kotlin/net/corda/core/flows/FlowLogicRef.kt @@ -1,10 +1,12 @@ package net.corda.core.flows +import net.corda.core.CordaInternal import net.corda.core.DoNotImplement import net.corda.core.serialization.CordaSerializable /** - * The public factory interface for creating validated FlowLogicRef instances as part of the scheduling framework. + * The public factory interface for creating validated [FlowLogicRef] instances as part of the scheduling framework. + * * Typically this would be used from within the nextScheduledActivity method of a QueryableState to specify * the flow to run at the scheduled time. */ @@ -14,20 +16,40 @@ interface FlowLogicRefFactory { * Construct a FlowLogicRef. This is intended for cases where the calling code has the relevant class already * and can provide it directly. */ - @Deprecated("This should be avoided, and the version which takes a class name used instead to avoid requiring the class on the classpath to deserialize calling code") fun create(flowClass: Class>, vararg args: Any?): FlowLogicRef + /** * Construct a FlowLogicRef. This is intended for cases where the calling code does not want to require the flow * class on the classpath for all cases where the calling code is loaded. */ fun create(flowClassName: String, vararg args: Any?): FlowLogicRef + + /** + * @suppress + * This is an internal method and should not be used: use [create] instead, which checks for the + * [SchedulableFlow] annotation. + */ + @CordaInternal fun createForRPC(flowClass: Class>, vararg args: Any?): FlowLogicRef + + /** + * Converts a [FlowLogicRef] object that was obtained from the calls above into a [FlowLogic], after doing some + * validation to ensure it points to a legitimate flow class. + */ fun toFlowLogic(ref: FlowLogicRef): FlowLogic<*> } +/** + * Thrown if the structure of a class implementing a flow is not correct. There can be several causes for this such as + * not inheriting from [FlowLogic], not having a valid constructor and so on. + * + * @property type the fully qualified name of the class that failed checks. + */ @CordaSerializable -class IllegalFlowLogicException(type: Class<*>, msg: String) : IllegalArgumentException( - "${FlowLogicRef::class.java.simpleName} cannot be constructed for ${FlowLogic::class.java.simpleName} of type ${type.name} $msg") +class IllegalFlowLogicException(val type: String, msg: String) : + IllegalArgumentException("A FlowLogicRef cannot be constructed for FlowLogic of type $type: $msg") { + constructor(type: Class<*>, msg: String) : this(type.name, msg) +} /** * A handle interface representing a [FlowLogic] instance which would be possible to safely pass out of the contract sandbox. diff --git a/node/src/main/kotlin/net/corda/node/services/statemachine/FlowLogicRefFactoryImpl.kt b/node/src/main/kotlin/net/corda/node/services/statemachine/FlowLogicRefFactoryImpl.kt index 8d1aba86c6..9ed350c080 100644 --- a/node/src/main/kotlin/net/corda/node/services/statemachine/FlowLogicRefFactoryImpl.kt +++ b/node/src/main/kotlin/net/corda/node/services/statemachine/FlowLogicRefFactoryImpl.kt @@ -41,15 +41,21 @@ class FlowLogicRefFactoryImpl(private val classloader: ClassLoader) : SingletonS } override fun create(flowClassName: String, vararg args: Any?): FlowLogicRef { - val flowClass = Class.forName(flowClassName, true, classloader).asSubclass(FlowLogic::class.java) - if (flowClass == null) { - throw IllegalArgumentException("The class $flowClassName is not a subclass of FlowLogic.") - } else { - if (!flowClass.isAnnotationPresent(SchedulableFlow::class.java)) { - throw IllegalFlowLogicException(flowClass, "because it's not a schedulable flow") - } - return createForRPC(flowClass, *args) + val flowClass = validatedFlowClassFromName(flowClassName) + if (!flowClass.isAnnotationPresent(SchedulableFlow::class.java)) { + throw IllegalFlowLogicException(flowClass, "because it's not a schedulable flow") } + return createForRPC(flowClass, *args) + } + + private fun validatedFlowClassFromName(flowClassName: String): Class> { + val forName = try { + Class.forName(flowClassName, true, classloader) + } catch (e: ClassNotFoundException) { + throw IllegalFlowLogicException(flowClassName, "Flow not found: $flowClassName") + } + return forName.asSubclass(FlowLogic::class.java) ?: + throw IllegalFlowLogicException(flowClassName, "The class $flowClassName is not a subclass of FlowLogic.") } override fun createForRPC(flowClass: Class>, vararg args: Any?): FlowLogicRef { @@ -93,7 +99,9 @@ class FlowLogicRefFactoryImpl(private val classloader: ClassLoader) : SingletonS override fun toFlowLogic(ref: FlowLogicRef): FlowLogic<*> { if (ref !is FlowLogicRefImpl) throw IllegalFlowLogicException(ref.javaClass, "FlowLogicRef was not created via correct FlowLogicRefFactory interface") - val klass = Class.forName(ref.flowLogicClassName, true, classloader).asSubclass(FlowLogic::class.java) + // We re-validate here because a FlowLogicRefImpl could have arrived via deserialization and therefore the + // class name could point to anything at all. + val klass = validatedFlowClassFromName(ref.flowLogicClassName) return createConstructor(klass, ref.args)() }