CORDA-2613: FinalityHandler is no longer gated on app target versions

To allow rolling upgrades where members of a BN may not all upgrade at the same time, the FinalityHandler is now always enabled, and not disabled if no old apps are installed.

Previously any attempt to use the FinalityHandler in such a scenerio would send the flow to the hospital, where manual intervention would be required to recover the transaction.
This commit is contained in:
Shams Asari 2019-02-17 16:02:36 +00:00 committed by Mike Hearn
parent 6673e499a1
commit b2d0ba7e14
5 changed files with 6 additions and 62 deletions

2
.idea/compiler.xml generated
View File

@ -291,6 +291,8 @@
<module name="workflows_integrationTest" target="1.8" />
<module name="workflows_main" target="1.8" />
<module name="workflows_test" target="1.8" />
<module name="worldmap_main" target="1.8" />
<module name="worldmap_test" target="1.8" />
</bytecodeTargetLevel>
</component>
<component name="JavacSettings">

View File

@ -686,30 +686,11 @@ abstract class AbstractNode<S>(val configuration: NodeConfiguration,
flowManager.registerInitiatedCoreFlowFactory(ContractUpgradeFlow.Initiate::class, NotaryChangeHandler::class, ::ContractUpgradeHandler)
}
// The FinalityHandler is insecure as it blindly accepts any and all transactions into the node's local vault without doing any checks.
// To plug this hole, the sending-side FinalityFlow has been made inlined with an inlined ReceiveFinalityFlow counterpart. The old
// FinalityFlow API is gated to only work with old CorDapps (those whose target platform version < 4), and the FinalityHandler will only
// work if there is at least one old CorDapp loaded (to preserve backwards compatibility).
//
// If an attempt is made to send us a transaction via FinalityHandler, and it's disabled, we will reject the request at the session-init
// level by throwing a FinalityHandlerDisabled exception. This is picked up by the flow hospital which will not send the error back
// (immediately) and instead pause the request by keeping it un-acknowledged in the message broker. This means the request isn't lost
// across node restarts and allows the node operator time to accept or reject the request.
// TODO Add public API to allow the node operator to accept or reject
// Ideally we should be disabling the FinalityHandler if it's not needed, to prevent any party from submitting transactions to us without
// us checking. Previously this was gated on app target version and if there were no apps with target version <= 3 then the handler would
// be disabled. However this prevents seemless rolling-upgrades and so it was removed until a better solution comes along.
private fun installFinalityHandler() {
// Disable the insecure FinalityHandler if none of the loaded CorDapps are old enough to require it.
val cordappsNeedingFinalityHandler = cordappLoader.cordapps.filter { it.targetPlatformVersion < 4 }
if (cordappsNeedingFinalityHandler.isEmpty()) {
log.info("FinalityHandler is disabled as there are no CorDapps loaded which require it")
} else {
log.warn("FinalityHandler is enabled as there are CorDapps that require it: ${cordappsNeedingFinalityHandler.map { it.info }}. " +
"This is insecure and it is strongly recommended that newer versions of these CorDapps be used instead.")
}
val disabled = cordappsNeedingFinalityHandler.isEmpty()
flowManager.registerInitiatedCoreFlowFactory(FinalityFlow::class, FinalityHandler::class) {
if (disabled) throw SessionRejectException.FinalityHandlerDisabled()
FinalityHandler(it)
}
flowManager.registerInitiatedCoreFlowFactory(FinalityFlow::class, FinalityHandler::class, ::FinalityHandler)
}
protected open fun makeTransactionStorage(transactionCacheSizeBytes: Long): WritableTransactionStorage {

View File

@ -12,8 +12,4 @@ open class SessionRejectException(message: String) : CordaException(message) {
class NotAFlow(val initiatorClass: Class<*>) : SessionRejectException("${initiatorClass.name} is not a flow")
class NotRegistered(val initiatorFlowClass: Class<out FlowLogic<*>>) : SessionRejectException("${initiatorFlowClass.name} is not registered")
class FinalityHandlerDisabled : SessionRejectException("Counterparty attempting to use the old insecure API of FinalityFlow. However this " +
"API is disabled on this node since there no CorDapps installed that require it. It may be that the counterparty is running an " +
"older verison of a CorDapp.")
}

View File

@ -47,9 +47,6 @@ class StaffedFlowHospital(private val flowMessaging: FlowMessaging, private val
// installed on restart, at which point the message will be able proceed as normal. If not then it will need
// to be dropped manually.
Outcome.OVERNIGHT_OBSERVATION
} else if (error is SessionRejectException.FinalityHandlerDisabled) {
// TODO We need a way to be able to give the green light to such a session-init message
Outcome.OVERNIGHT_OBSERVATION
} else {
Outcome.UNTREATABLE
}

View File

@ -19,7 +19,6 @@ import net.corda.testing.core.BOB_NAME
import net.corda.testing.core.singleIdentity
import net.corda.testing.node.internal.*
import org.assertj.core.api.Assertions.assertThat
import org.assertj.core.api.Assertions.assertThatThrownBy
import org.junit.After
import org.junit.Test
import rx.Observable
@ -61,37 +60,6 @@ class FinalityHandlerTest {
assertThat(bob.getTransaction(stx.id)).isNull()
}
@Test
fun `disabled if there are no old CorDapps loaded`() {
val alice = mockNet.createNode(InternalMockNodeParameters(legalName = ALICE_NAME, additionalCordapps = FINANCE_CORDAPPS))
val bob = mockNet.createNode(InternalMockNodeParameters(
legalName = BOB_NAME,
// Make sure the target version is 4, and not the current platform version which may be greater
additionalCordapps = setOf(cordappWithPackages("net.corda.finance").copy(targetPlatformVersion = 4))
))
val stx = alice.issueCashTo(bob)
val finalityFuture = alice.finaliseWithOldApi(stx)
val record = bob.medicalRecordsOfType<MedicalRecord.SessionInit>()
.toBlocking()
.first()
assertThat(record.outcome).isEqualTo(Outcome.OVERNIGHT_OBSERVATION)
assertThat(record.sender).isEqualTo(alice.info.singleIdentity())
assertThat(record.initiatorFlowClassName).isEqualTo(FinalityFlow::class.java.name)
assertThat(bob.getTransaction(stx.id)).isNull()
// Drop the session-init so that Alice gets the error message
assertThat(finalityFuture).isNotDone()
bob.smm.flowHospital.dropSessionInit(record.id)
mockNet.runNetwork()
assertThatThrownBy {
finalityFuture.getOrThrow()
}.hasMessageContaining("Counterparty attempting to use the old insecure API of FinalityFlow")
}
private fun TestStartedNode.issueCashTo(recipient: TestStartedNode): SignedTransaction {
return TransactionBuilder(mockNet.defaultNotaryIdentity).let {
Cash().generateIssue(