diff --git a/.idea/compiler.xml b/.idea/compiler.xml index 813f60e011..a2eed4b2f3 100644 --- a/.idea/compiler.xml +++ b/.idea/compiler.xml @@ -291,6 +291,8 @@ + + diff --git a/node/src/main/kotlin/net/corda/node/internal/AbstractNode.kt b/node/src/main/kotlin/net/corda/node/internal/AbstractNode.kt index 1cdf7c2b10..b4a56d9690 100644 --- a/node/src/main/kotlin/net/corda/node/internal/AbstractNode.kt +++ b/node/src/main/kotlin/net/corda/node/internal/AbstractNode.kt @@ -686,30 +686,11 @@ abstract class AbstractNode(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 { diff --git a/node/src/main/kotlin/net/corda/node/services/statemachine/SessionRejectException.kt b/node/src/main/kotlin/net/corda/node/services/statemachine/SessionRejectException.kt index e707ddbe7d..9f52e5a0d4 100644 --- a/node/src/main/kotlin/net/corda/node/services/statemachine/SessionRejectException.kt +++ b/node/src/main/kotlin/net/corda/node/services/statemachine/SessionRejectException.kt @@ -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>) : 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.") } diff --git a/node/src/main/kotlin/net/corda/node/services/statemachine/StaffedFlowHospital.kt b/node/src/main/kotlin/net/corda/node/services/statemachine/StaffedFlowHospital.kt index 322e764463..bf5eaacbf1 100644 --- a/node/src/main/kotlin/net/corda/node/services/statemachine/StaffedFlowHospital.kt +++ b/node/src/main/kotlin/net/corda/node/services/statemachine/StaffedFlowHospital.kt @@ -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 } diff --git a/node/src/test/kotlin/net/corda/node/services/FinalityHandlerTest.kt b/node/src/test/kotlin/net/corda/node/services/FinalityHandlerTest.kt index 5615d44597..01a4a89544 100644 --- a/node/src/test/kotlin/net/corda/node/services/FinalityHandlerTest.kt +++ b/node/src/test/kotlin/net/corda/node/services/FinalityHandlerTest.kt @@ -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() - .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(