ENT-7039: Notary signature checking requires non-interned SecureHashes. (#7254)

There was a mistake made when we first introduced notary request signature checking, in that we didn't wrap it in SerializedBytes so it always got deserialized as part of the flow message payload. So to check the signature, it has to be re-serialized. This means for cross-version compatibility we can never change the serialized format of NotarisationRequest. In this case we need make sure that every SecureHash mentioned in that data structure is a distinct instance, even if the values are repeated / identical, as that is how it was in Corda 1.

With the introduction of interning of SecureHash, this ceased to be true once again, including undoing the attempts to force it on the sending side that had been introduced in previous versions of Corda. So here we introduce a way to force it, and consolidate the forcing to distinct SecureHash instances in the NotarisationRequest itself, rather than leaving to the caller of the constructor to remember to do it, so that serialized form will always be as per Corda 1.

core/src/main/kotlin/net/corda/core/crypto/SecureHash.kt

@@ -155,6 +155,14 @@ sealed class SecureHash(bytes: ByteArray) : OpaqueBytes(bytes) {
             }
         }
 
+        @JvmStatic
+        internal fun deintern(hash: SecureHash): SecureHash {
+            return when (hash) {
+                is SHA256 -> SHA256(hash.bytes)
+                else -> HASH(hash.algorithm, hash.bytes)
+            }
+        }
+
         /**
          * @param algorithm [MessageDigest] algorithm name, in uppercase.
          * @param value Hash value as a hexadecimal string.

core/src/main/kotlin/net/corda/core/flows/NotaryFlow.kt

@@ -5,7 +5,6 @@ import net.corda.core.CordaInternal
 import net.corda.core.DoNotImplement
 import net.corda.core.contracts.StateRef
 import net.corda.core.contracts.TimeWindow
-import net.corda.core.crypto.SecureHash
 import net.corda.core.crypto.TransactionSignature
 import net.corda.core.identity.Party
 import net.corda.core.internal.BackpressureAwareTimedFlow
@@ -174,8 +173,7 @@ class NotaryFlow {
          * same transaction.
          */
         private fun generateRequestSignature(): NotarisationRequestSignature {
-            // TODO: This is not required any more once our AMQP serialization supports turning off object referencing.
-            val notarisationRequest = NotarisationRequest(stx.inputs.map { it.copy(txhash = SecureHash.create(it.txhash.toString())) }, stx.id)
+            val notarisationRequest = NotarisationRequest(stx.inputs, stx.id)
             return notarisationRequest.generateSignature(serviceHub)
         }
     }

core/src/main/kotlin/net/corda/core/flows/NotaryWireFormat.kt

@@ -34,7 +34,9 @@ class NotarisationRequest(statesToConsume: List<StateRef>, val transactionId: Se
         private val stateRefComparator = compareBy<StateRef>({ it.txhash }, { it.index })
     }
 
+    // For compatibility reasons, each SecureHash has to be distinct, even if for same value.
     private val _statesToConsumeSorted = statesToConsume.sortedWith(stateRefComparator)
+            .map { StateRef(SecureHash.deintern(it.txhash), it.index) }
 
     /** States this request specifies to be consumed. Sorted to ensure the serialized form does not get affected by the state order. */
     val statesToConsume: List<StateRef> get() = _statesToConsumeSorted // Getter required for AMQP serialization

node/src/test/kotlin/net/corda/node/services/transactions/NonValidatingNotaryServiceTests.kt

@@ -3,8 +3,16 @@ package net.corda.node.services.transactions
 import net.corda.core.concurrent.CordaFuture
 import net.corda.core.contracts.StateAndRef
 import net.corda.core.contracts.StateRef
-import net.corda.core.crypto.*
-import net.corda.core.flows.*
+import net.corda.core.crypto.Crypto
+import net.corda.core.crypto.SecureHash
+import net.corda.core.crypto.TransactionSignature
+import net.corda.core.crypto.sign
+import net.corda.core.flows.NotarisationPayload
+import net.corda.core.flows.NotarisationRequest
+import net.corda.core.flows.NotarisationRequestSignature
+import net.corda.core.flows.NotaryError
+import net.corda.core.flows.NotaryException
+import net.corda.core.flows.NotaryFlow
 import net.corda.core.identity.Party
 import net.corda.core.internal.notary.generateSignature
 import net.corda.core.messaging.MessageRecipients
@@ -25,14 +33,20 @@ import net.corda.testing.core.dummyCommand
 import net.corda.testing.core.singleIdentity
 import net.corda.testing.node.MockNetworkNotarySpec
 import net.corda.testing.node.TestClock
-import net.corda.testing.node.internal.*
+import net.corda.testing.node.internal.DUMMY_CONTRACTS_CORDAPP
+import net.corda.testing.node.internal.InMemoryMessage
+import net.corda.testing.node.internal.InternalMockNetwork
+import net.corda.testing.node.internal.InternalMockNodeParameters
+import net.corda.testing.node.internal.MessagingServiceSpy
+import net.corda.testing.node.internal.TestStartedNode
+import net.corda.testing.node.internal.startFlow
 import org.assertj.core.api.Assertions.assertThat
 import org.junit.After
 import org.junit.Before
 import org.junit.Test
 import java.time.Duration
 import java.time.Instant
-import java.util.*
+import java.util.Random
 import kotlin.test.assertEquals
 import kotlin.test.assertFailsWith
 import kotlin.test.assertTrue

serialization/src/test/kotlin/net/corda/serialization/internal/amqp/EvolvabilityTests.kt

@@ -4,6 +4,7 @@ import net.corda.core.crypto.Crypto
 import net.corda.core.crypto.Crypto.generateKeyPair
 import net.corda.core.crypto.SignedData
 import net.corda.core.crypto.sign
+import net.corda.core.flows.NotarisationRequest
 import net.corda.core.identity.CordaX500Name
 import net.corda.core.identity.Party
 import net.corda.core.node.NetworkParameters
@@ -16,11 +17,13 @@ import net.corda.serialization.internal.amqp.custom.InstantSerializer
 import net.corda.serialization.internal.amqp.testutils.ProjectStructure.projectRootDir
 import net.corda.serialization.internal.amqp.testutils.TestSerializationOutput
 import net.corda.serialization.internal.amqp.testutils.deserialize
+import net.corda.serialization.internal.amqp.testutils.serialize
 import net.corda.serialization.internal.amqp.testutils.serializeAndReturnSchema
 import net.corda.serialization.internal.amqp.testutils.testDefaultFactory
 import net.corda.serialization.internal.amqp.testutils.testName
 import org.junit.Ignore
 import org.junit.Test
+import org.junit.jupiter.api.Assertions.assertNotSame
 import java.io.File
 import java.io.NotSerializableException
 import java.math.BigInteger
@@ -840,4 +843,43 @@ class EvolvabilityTests {
 
         assertEquals("written<not provided>", deserializedCC.data)
     }
+
+    @Test(timeout = 300_000)
+    fun notarisationRequestStabilityTest() {
+        val sf = testDefaultFactory()
+        val resource = "EvolvabilityTests.notarisationRequest"
+
+        // Stable form of NotarisationRequest from 4.9
+        /*
+        val txId = SecureHash.randomSHA256()
+        val inputTxId1 = SecureHash.randomSHA256()
+        val inputTxId2 = SecureHash.randomSHA256()
+        val notarisationRequest = NotarisationRequest(listOf(StateRef(inputTxId1, 0), StateRef(inputTxId1, 1), StateRef(inputTxId2, 0), StateRef(inputTxId2, 1)).map { it.copy(txhash = SecureHash.create(it.txhash.toString())) }, txId)
+        val currentForm = SerializationOutput(sf).serialize(notarisationRequest)
+        File(URI("$localPath/$resource")).writeBytes(currentForm.bytes)
+         */
+
+        val url = EvolvabilityTests::class.java.getResource(resource)
+        val sc2 = url.readBytes()
+        val previousForm = SerializedBytes<NotarisationRequest>(sc2)
+        val deserialized = DeserializationInput(sf).deserialize(previousForm)
+
+        assertEquals(deserialized.statesToConsume[0].txhash, deserialized.statesToConsume[1].txhash)
+        assertEquals(deserialized.statesToConsume[2].txhash, deserialized.statesToConsume[3].txhash)
+        assertNotSame(deserialized.statesToConsume[0].txhash, deserialized.statesToConsume[1].txhash)
+        assertNotSame(deserialized.statesToConsume[2].txhash, deserialized.statesToConsume[3].txhash)
+
+        val serialized = SerializationOutput(sf).serialize(deserialized)
+
+        assertEquals(previousForm, serialized)
+
+        val deserialized2 = DeserializationInput(sf).deserialize(serialized)
+        assertEquals(deserialized.transactionId, deserialized2.transactionId)
+        assertEquals(deserialized.statesToConsume, deserialized2.statesToConsume)
+
+        assertEquals(deserialized2.statesToConsume[0].txhash, deserialized2.statesToConsume[1].txhash)
+        assertEquals(deserialized2.statesToConsume[2].txhash, deserialized2.statesToConsume[3].txhash)
+        assertNotSame(deserialized2.statesToConsume[0].txhash, deserialized2.statesToConsume[1].txhash)
+        assertNotSame(deserialized2.statesToConsume[2].txhash, deserialized2.statesToConsume[3].txhash)
+    }
 }