Merge pull request #7318 from corda/colljos/record_txns_signature_verification

ENT-9566 Additional Signature verification and validation: recordTransactions()

core-tests/src/test/kotlin/net/corda/coretests/contracts/ConstraintsPropagationTests.kt

@@ -10,6 +10,7 @@ import net.corda.core.crypto.SecureHash.Companion.allOnesHash
 import net.corda.core.crypto.SecureHash.Companion.zeroHash
 import net.corda.core.crypto.SignableData
 import net.corda.core.crypto.SignatureMetadata
+import net.corda.core.crypto.sign
 import net.corda.core.identity.AbstractParty
 import net.corda.core.identity.CordaX500Name
 import net.corda.core.internal.canBeTransitionedFrom
@@ -49,6 +50,7 @@ class ConstraintsPropagationTests {
     val testSerialization = SerializationEnvironmentRule()
 
     private companion object {
+        val DUMMY_NOTARY_IDENTITY = TestIdentity(DUMMY_NOTARY_NAME, 20)
         val DUMMY_NOTARY = TestIdentity(DUMMY_NOTARY_NAME, 20).party
         val ALICE = TestIdentity(CordaX500Name("ALICE", "London", "GB"))
         val ALICE_PARTY get() = ALICE.party
@@ -376,7 +378,8 @@ class ConstraintsPropagationTests {
         requireSupportedHashType(wireTransaction)
         val nodeKey = ALICE_PUBKEY
         val sigs = listOf(keyManagementService.sign(
-                SignableData(wireTransaction.id, SignatureMetadata(4, Crypto.findSignatureScheme(nodeKey).schemeNumberID)), nodeKey))
+                SignableData(wireTransaction.id, SignatureMetadata(4, Crypto.findSignatureScheme(nodeKey).schemeNumberID)), nodeKey),
+                DUMMY_NOTARY_IDENTITY.keyPair.sign(SignableData(wireTransaction.id, SignatureMetadata(4, Crypto.findSignatureScheme(DUMMY_NOTARY_IDENTITY.publicKey).schemeNumberID))))
         recordTransactions(SignedTransaction(wireTransaction, sigs))
     }
 

core-tests/src/test/kotlin/net/corda/coretests/internal/ResolveTransactionsFlowTest.kt

@@ -99,12 +99,17 @@ class ResolveTransactionsFlowTest {
     // DOCEND 1
 
     @Test(timeout=300_000)
-	fun `dependency with an error`() {
+	fun `dependency with an error fails fast upon prior attempt to record transaction with missing signature`() {
-        val stx = makeTransactions(signFirstTX = false).second
+        val exception = assertFailsWith(IllegalStateException::class) {
-        val p = TestFlow(setOf(stx.id), megaCorp)
+            val stx = makeTransactions(signFirstTX = false).second
-        val future = miniCorpNode.startFlow(p)
+            // fails fast in above operation
-        mockNet.runNetwork()
+            // prior to platform version 13, same failure would occur upon transaction resolution
-        assertFailsWith(SignedTransaction.SignaturesMissingException::class) { future.getOrThrow() }
+            val p = TestFlow(setOf(stx.id), megaCorp)
+            val future = miniCorpNode.startFlow(p)
+            mockNet.runNetwork()
+            future.getOrThrow()
+        }
+        assertTrue(exception.cause.toString().contains("SignaturesMissingException"))
     }
 
     @Test(timeout=300_000)

core-tests/src/test/kotlin/net/corda/coretests/transactions/LedgerTransactionQueryTests.kt

@@ -80,7 +80,7 @@ class LedgerTransactionQueryTests {
                         .addOutputState(dummyState, DummyContract.PROGRAM_ID)
                         .addCommand(dummyCommand())
         )
-        services.recordTransactions(fakeIssueTx)
+        services.recordTransactions(fakeIssueTx, disableSignatureVerification = true)
         val dummyStateRef = StateRef(fakeIssueTx.id, 0)
         return StateAndRef(TransactionState(dummyState, DummyContract.PROGRAM_ID, DUMMY_NOTARY, constraint = AlwaysAcceptAttachmentConstraint), dummyStateRef)
     }

core/src/main/kotlin/net/corda/core/node/ServiceHub.kt

@@ -10,9 +10,10 @@ import net.corda.core.crypto.SignableData
 import net.corda.core.crypto.SignatureMetadata
 import net.corda.core.crypto.TransactionSignature
 import net.corda.core.flows.ContractUpgradeFlow
+import net.corda.core.internal.PlatformVersionSwitches.TWO_PHASE_FINALITY
+import net.corda.core.internal.telemetry.TelemetryComponent
 import net.corda.core.node.services.*
 import net.corda.core.node.services.diagnostics.DiagnosticsService
-import net.corda.core.internal.telemetry.TelemetryComponent
 import net.corda.core.serialization.SerializeAsToken
 import net.corda.core.transactions.FilteredTransaction
 import net.corda.core.transactions.LedgerTransaction
@@ -204,6 +205,12 @@ interface ServiceHub : ServicesForResolution {
      * Stores the given [SignedTransaction]s in the local transaction storage and then sends them to the vault for
      * further processing if [notifyVault] is true. This is expected to be run within a database transaction.
      *
+     * As of platform version [TWO_PHASE_FINALITY] also performs signature verification and will throw an
+     * [IllegalStateException] with details of the cause of error upon failure.
+     * Of course, you should not be recording transactions to the ledger that are not fully signed.
+     * It is possible, but not recommended, to revert to non-signature verification behaviour by setting the system property
+     * "net.corda.recordtransaction.signature.verification.disabled" to true upon node start-up.
+     *
      * @param txs The transactions to record.
      * @param notifyVault indicate if the vault should be notified for the update.
      */
@@ -214,6 +221,12 @@ interface ServiceHub : ServicesForResolution {
     /**
      * Stores the given [SignedTransaction]s in the local transaction storage and then sends them to the vault for
      * further processing if [notifyVault] is true. This is expected to be run within a database transaction.
+     *
+     * As of platform version [TWO_PHASE_FINALITY] also performs signature verification and will throw an
+     * [IllegalStateException] with details of the cause of error upon failure.
+     * Of course, you should not be recording transactions to the ledger that are not fully signed.
+     * It is possible, but not recommended, to revert to non-signature verification behaviour by setting the system property
+     * "net.corda.recordtransaction.signature.verification.disabled" to true upon node start-up.
      */
     fun recordTransactions(notifyVault: Boolean, first: SignedTransaction, vararg remaining: SignedTransaction) {
         recordTransactions(notifyVault, listOf(first, *remaining))
@@ -224,6 +237,12 @@ interface ServiceHub : ServicesForResolution {
      * further processing if [statesToRecord] is not [StatesToRecord.NONE].
      * This is expected to be run within a database transaction.
      *
+     * As of platform version [TWO_PHASE_FINALITY] also performs signature verification and will throw an
+     * [IllegalStateException] with details of the cause of error upon failure.
+     * Of course, you should not be recording transactions to the ledger that are not fully signed.
+     * It is possible, but not recommended, to revert to non-signature verification behaviour by setting the system property
+     * "net.corda.recordtransaction.signature.verification.disabled" to true upon node start-up.
+     *
      * @param txs The transactions to record.
      * @param statesToRecord how the vault should treat the output states of the transaction.
      */
@@ -232,6 +251,13 @@ interface ServiceHub : ServicesForResolution {
     /**
      * Stores the given [SignedTransaction]s in the local transaction storage and then sends them to the vault for
      * further processing. This is expected to be run within a database transaction.
+     *
+     * As of platform version [TWO_PHASE_FINALITY] also performs signature verification and will throw an
+     * [IllegalStateException] with details of the cause of error upon failure.
+     * Of course, you should not be recording transactions to the ledger that are not fully signed.
+     * It is possible, but not recommended, to revert to non-signature verification behaviour by setting the system property
+     * "net.corda.recordtransaction.signature.verification.disabled" to true upon node start-up.
+     *
      */
     fun recordTransactions(first: SignedTransaction, vararg remaining: SignedTransaction) {
         recordTransactions(listOf(first, *remaining))
@@ -240,6 +266,13 @@ interface ServiceHub : ServicesForResolution {
     /**
      * Stores the given [SignedTransaction]s in the local transaction storage and then sends them to the vault for
      * further processing. This is expected to be run within a database transaction.
+     *
+     * As of platform version [TWO_PHASE_FINALITY] also performs signature verification and will throw an
+     * [IllegalStateException] with details of the cause of error upon failure.
+     * Of course, you should not be recording transactions to the ledger that are not fully signed.
+     * It is possible, but not recommended, to revert to non-signature verification behaviour by setting the system property
+     * "net.corda.recordtransaction.signature.verification.disabled" to true upon node start-up.
+     *
      */
     fun recordTransactions(txs: Iterable<SignedTransaction>) {
         recordTransactions(StatesToRecord.ONLY_RELEVANT, txs)

node/src/integration-test/kotlin/net/corda/contracts/SignatureConstraintMigrationFromHashConstraintsTests.kt

@@ -8,7 +8,6 @@ import net.corda.core.internal.deleteRecursively
 import net.corda.core.internal.div
 import net.corda.core.messaging.startFlow
 import net.corda.core.utilities.getOrThrow
-import net.corda.node.flows.isQuasarAgentSpecified
 import net.corda.testing.common.internal.testNetworkParameters
 import net.corda.testing.core.singleIdentity
 import net.corda.testing.driver.NodeParameters
@@ -28,8 +27,8 @@ open class SignatureConstraintMigrationFromHashConstraintsTests : SignatureConst
 
         val stateAndRef: StateAndRef<MessageState>? = internalDriver(
                 inMemoryDB = false,
-                startNodesInProcess = isQuasarAgentSpecified(),
+                networkParameters = testNetworkParameters(notaries = emptyList(), minimumPlatformVersion = 4),
-                networkParameters = testNetworkParameters(notaries = emptyList(), minimumPlatformVersion = 4)
+                systemProperties = mapOf("net.corda.recordtransaction.signature.verification.disabled" to true.toString())
         ) {
             val nodeName = {
                 val nodeHandle = startNode(NodeParameters(rpcUsers = listOf(user), additionalCordapps = listOf(oldCordapp))).getOrThrow()

node/src/integration-test/kotlin/net/corda/contracts/SignatureConstraintMigrationFromWhitelistConstraintTests.kt

@@ -9,7 +9,6 @@ import net.corda.core.internal.deleteRecursively
 import net.corda.core.internal.div
 import net.corda.core.messaging.startFlow
 import net.corda.core.utilities.getOrThrow
-import net.corda.node.flows.isQuasarAgentSpecified
 import net.corda.testing.common.internal.testNetworkParameters
 import net.corda.testing.core.singleIdentity
 import net.corda.testing.driver.NodeParameters
@@ -30,8 +29,8 @@ open class SignatureConstraintMigrationFromWhitelistConstraintTests  : Signature
 
         val stateAndRef: StateAndRef<MessageState>? = internalDriver(
                 inMemoryDB = false,
-                startNodesInProcess = isQuasarAgentSpecified(),
+                networkParameters = testNetworkParameters(notaries = emptyList(), minimumPlatformVersion = 4),
-                networkParameters = testNetworkParameters(notaries = emptyList(), minimumPlatformVersion = 4)
+                systemProperties = mapOf("net.corda.recordtransaction.signature.verification.disabled" to true.toString())
         ) {
             val nodeName = {
                 val nodeHandle = startNode(NodeParameters(rpcUsers = listOf(user), additionalCordapps = listOf(oldCordapp))).getOrThrow()
@@ -142,7 +141,7 @@ open class SignatureConstraintMigrationFromWhitelistConstraintTests  : Signature
                         )
                 ),
                 systemProperties = emptyMap(),
-                startNodesInProcess = true,
+                startNodesInProcess = false,
                 specifyExistingConstraint = true,
                 addAnotherAutomaticConstraintState = true
         )

node/src/integration-test/kotlin/net/corda/contracts/SignatureConstraintVersioningTests.kt

@@ -74,7 +74,8 @@ open class SignatureConstraintVersioningTests {
                         minimumPlatformVersion = minimumPlatformVersion,
                         whitelistedContractImplementations = whitelistedAttachmentHashes
                 ),
-                systemProperties = systemProperties
+                systemProperties = systemProperties +
+                        ("net.corda.recordtransaction.signature.verification.disabled" to true.toString())
         ) {
             // create transaction using first Cordapp
             val (nodeName, baseDirectory, issuanceTransaction) = createIssuanceTransaction(cordapp)

node/src/main/kotlin/net/corda/node/services/api/ServiceHubInternal.kt

@@ -13,9 +13,11 @@ import net.corda.core.internal.NamedCacheFactory
 import net.corda.core.internal.ResolveTransactionsFlow
 import net.corda.core.internal.ServiceHubCoreInternal
 import net.corda.core.internal.TransactionsResolver
+import net.corda.core.internal.VisibleForTesting
 import net.corda.core.internal.concurrent.OpenFuture
 import net.corda.core.internal.dependencies
 import net.corda.core.internal.requireSupportedHashType
+import net.corda.core.internal.warnOnce
 import net.corda.core.messaging.DataFeed
 import net.corda.core.messaging.StateMachineTransactionMapping
 import net.corda.core.node.NodeInfo
@@ -36,7 +38,8 @@ import net.corda.node.services.persistence.AttachmentStorageInternal
 import net.corda.node.services.statemachine.ExternalEvent
 import net.corda.node.services.statemachine.FlowStateMachineImpl
 import net.corda.nodeapi.internal.persistence.CordaPersistence
-import java.security.PublicKey
+import java.lang.IllegalStateException
+import java.security.SignatureException
 import java.util.ArrayList
 import java.util.Collections
 import java.util.HashMap
@@ -67,6 +70,7 @@ interface NetworkMapCacheInternal : NetworkMapCache, NetworkMapCacheBase {
 interface ServiceHubInternal : ServiceHubCoreInternal {
     companion object {
         private val log = contextLogger()
+        private val SIGNATURE_VERIFICATION_DISABLED = java.lang.Boolean.getBoolean("net.corda.recordtransaction.signature.verification.disabled")
 
         private fun topologicalSort(transactions: Collection<SignedTransaction>): Collection<SignedTransaction> {
             if (transactions.size == 1) return transactions
@@ -188,9 +192,28 @@ interface ServiceHubInternal : ServiceHubCoreInternal {
     fun getFlowFactory(initiatingFlowClass: Class<out FlowLogic<*>>): InitiatedFlowFactory<*>?
     val cacheFactory: NamedCacheFactory
 
-    override fun recordTransactions(statesToRecord: StatesToRecord, txs: Iterable<SignedTransaction>) {
+    override fun recordTransactions(statesToRecord: StatesToRecord, txs: Iterable<SignedTransaction>) =
+            recordTransactions(statesToRecord, txs, SIGNATURE_VERIFICATION_DISABLED)
+
+    @Suppress("NestedBlockDepth")
+    @VisibleForTesting
+    fun recordTransactions(statesToRecord: StatesToRecord, txs: Iterable<SignedTransaction>, disableSignatureVerification: Boolean) {
         txs.forEach {
             requireSupportedHashType(it)
+            if (it.coreTransaction is WireTransaction) {
+                if (disableSignatureVerification) {
+                    log.warnOnce("The current usage of recordTransactions is unsafe." +
+                            "Recording transactions without signature verification may lead to severe problems with ledger consistency.")
+                }
+                else {
+                    try {
+                        it.verifyRequiredSignatures()
+                    }
+                    catch (e: SignatureException) {
+                        throw IllegalStateException("Signature verification failed", e)
+                    }
+                }
+            }
         }
         recordTransactions(
                 statesToRecord,

node/src/test/kotlin/net/corda/node/services/NotaryChangeTests.kt

@@ -10,10 +10,12 @@ import net.corda.core.flows.StateReplacementException
 import net.corda.core.identity.CordaX500Name
 import net.corda.core.identity.Party
 import net.corda.core.node.ServiceHub
+import net.corda.core.node.StatesToRecord
 import net.corda.core.transactions.TransactionBuilder
 import net.corda.core.transactions.WireTransaction
 import net.corda.core.utilities.getOrThrow
 import net.corda.core.utilities.seconds
+import net.corda.node.services.api.ServiceHubInternal
 import net.corda.testing.contracts.DummyContract
 import net.corda.testing.core.ALICE_NAME
 import net.corda.testing.core.BOB_NAME
@@ -224,6 +226,6 @@ fun issueInvalidState(services: ServiceHub, identity: Party, notary: Party): Sta
     val tx = DummyContract.generateInitial(Random().nextInt(), notary, identity.ref(0))
     tx.setTimeWindow(Instant.now(), 30.seconds)
     val stx = services.signInitialTransaction(tx)
-    services.recordTransactions(stx)
+    (services as ServiceHubInternal).recordTransactions(StatesToRecord.ONLY_RELEVANT, listOf(stx), disableSignatureVerification = true)
     return stx.tx.outRef(0)
 }

node/src/test/kotlin/net/corda/node/services/vault/VaultWithCashTest.kt

@@ -382,8 +382,10 @@ class VaultWithCashTest {
             linearStates.forEach { println(it.state.data.linearId) }
 
             //copy transactions to notary - simulates transaction resolution
-            services.validatedTransactions.getTransaction(deals.first().ref.txhash)?.apply { notaryServices.recordTransactions(this) }
+            services.validatedTransactions.getTransaction(deals.first().ref.txhash)?.apply {
-            services.validatedTransactions.getTransaction(linearStates.first().ref.txhash)?.apply { notaryServices.recordTransactions(this) }
+                notaryServices.recordTransactions(this, disableSignatureVerification = true) }
+            services.validatedTransactions.getTransaction(linearStates.first().ref.txhash)?.apply {
+                notaryServices.recordTransactions(this, disableSignatureVerification = true) }
 
             // Create a txn consuming different contract types
             val dummyMoveBuilder = TransactionBuilder(notary = notary).apply {

testing/node-driver/src/main/kotlin/net/corda/testing/node/MockServices.kt

@@ -12,6 +12,7 @@ import net.corda.core.identity.CordaX500Name
 import net.corda.core.identity.Party
 import net.corda.core.identity.PartyAndCertificate
 import net.corda.core.internal.PLATFORM_VERSION
+import net.corda.core.internal.VisibleForTesting
 import net.corda.core.internal.requireSupportedHashType
 import net.corda.core.internal.telemetry.TelemetryComponent
 import net.corda.core.internal.telemetry.TelemetryServiceImpl
@@ -451,8 +452,18 @@ open class MockServices private constructor(
     val cordappClassloader: ClassLoader
         get() = cordappLoader.appClassLoader
 
-    override fun recordTransactions(statesToRecord: StatesToRecord, txs: Iterable<SignedTransaction>) {
+    override fun recordTransactions(statesToRecord: StatesToRecord, txs: Iterable<SignedTransaction>) =
+            recordTransactions(txs, false)
+
+    @VisibleForTesting
+    fun recordTransactions(txn: SignedTransaction, disableSignatureVerification: Boolean) =
+            recordTransactions(listOf(txn), disableSignatureVerification)
+
+    @VisibleForTesting
+    fun recordTransactions(txs: Iterable<SignedTransaction>, disableSignatureVerification: Boolean) {
         txs.forEach {
+            if (!disableSignatureVerification)
+                it.verifyRequiredSignatures()
             (validatedTransactions as WritableTransactionStorage).addTransaction(it)
         }
     }

testing/test-utils/src/main/kotlin/net/corda/testing/dsl/TestDSL.kt

@@ -23,6 +23,7 @@ import net.corda.core.transactions.SignedTransaction
 import net.corda.core.transactions.TransactionBuilder
 import net.corda.core.transactions.WireTransaction
 import net.corda.node.services.DbTransactionsResolver
+import net.corda.node.services.api.WritableTransactionStorage
 import net.corda.node.services.attachments.NodeAttachmentTrustCalculator
 import net.corda.node.services.persistence.AttachmentStorageInternal
 import net.corda.testing.core.dummyCommand
@@ -367,7 +368,10 @@ data class TestLedgerDSLInterpreter private constructor(
     override fun verifies(): EnforceVerifyOrFail {
         try {
             val usedInputs = mutableSetOf<StateRef>()
-            services.recordTransactions(transactionsUnverified.map { SignedTransaction(it, listOf(NULL_SIGNATURE)) })
+            transactionsUnverified.map {
+                (services.validatedTransactions as WritableTransactionStorage).addTransaction(SignedTransaction(it, listOf(NULL_SIGNATURE)))
+            }
+
             for ((_, value) in transactionWithLocations) {
                 val wtx = value.transaction
                 val ltx = wtx.toLedgerTransaction(services)
@@ -380,7 +384,7 @@ data class TestLedgerDSLInterpreter private constructor(
                     throw DoubleSpentInputs(txIds)
                 }
                 usedInputs.addAll(wtx.inputs)
-                services.recordTransactions(SignedTransaction(wtx, listOf(NULL_SIGNATURE)))
+                (services.validatedTransactions as WritableTransactionStorage).addTransaction(SignedTransaction(wtx, listOf(NULL_SIGNATURE)))
             }
             return EnforceVerifyOrFail.Token
         } catch (exception: TransactionVerificationException) {