CORDA-1171 - When a double-spend occurs, do not send the consuming tr…

…ansaction id and requesting party back to the client - this might lead to privacy leak. Only the transaction id hash is now returned. (corda#2746)
mejuhi · Mar 8, 2018 · c367df0 · c367df0
1 parent cdf2a94
commit c367df0
Show file tree

Hide file tree

Showing 17 changed files with 215 additions and 173 deletions.
diff --git a/.ci/api-current.txt b/.ci/api-current.txt
@@ -1373,12 +1373,10 @@ public static final class net.corda.core.flows.NotarisationRequest$Companion ext
 @net.corda.core.serialization.CordaSerializable public abstract class net.corda.core.flows.NotaryError extends java.lang.Object
 ##
 @net.corda.core.serialization.CordaSerializable public static final class net.corda.core.flows.NotaryError$Conflict extends net.corda.core.flows.NotaryError
-  public <init>(net.corda.core.crypto.SecureHash, net.corda.core.crypto.SignedData)
   @org.jetbrains.annotations.NotNull public final net.corda.core.crypto.SecureHash component1()
-  @org.jetbrains.annotations.NotNull public final net.corda.core.crypto.SignedData component2()
-  @org.jetbrains.annotations.NotNull public final net.corda.core.flows.NotaryError$Conflict copy(net.corda.core.crypto.SecureHash, net.corda.core.crypto.SignedData)
+  @org.jetbrains.annotations.NotNull public final Map component2()
+  @org.jetbrains.annotations.NotNull public final net.corda.core.flows.NotaryError$Conflict copy(net.corda.core.crypto.SecureHash, Map)
   public boolean equals(Object)
-  @org.jetbrains.annotations.NotNull public final net.corda.core.crypto.SignedData getConflict()
   @org.jetbrains.annotations.NotNull public final net.corda.core.crypto.SecureHash getTxId()
   public int hashCode()
   @org.jetbrains.annotations.NotNull public String toString()
@@ -1429,13 +1427,13 @@ public static final class net.corda.core.flows.NotaryError$TimeWindowInvalid$Com
   public static final net.corda.core.flows.NotaryError$WrongNotary INSTANCE
 ##
 @net.corda.core.serialization.CordaSerializable public final class net.corda.core.flows.NotaryException extends net.corda.core.flows.FlowException
-  public <init>(net.corda.core.flows.NotaryError)
+  public <init>(net.corda.core.flows.NotaryError, net.corda.core.crypto.SecureHash)
   @org.jetbrains.annotations.NotNull public final net.corda.core.flows.NotaryError getError()
 ##
 public final class net.corda.core.flows.NotaryFlow extends java.lang.Object
   public <init>()
 ##
-@net.corda.core.flows.InitiatingFlow public static class net.corda.core.flows.NotaryFlow$Client extends net.corda.core.flows.FlowLogic
+@net.corda.core.flows.InitiatingFlow @net.corda.core.DoNotImplement public static class net.corda.core.flows.NotaryFlow$Client extends net.corda.core.flows.FlowLogic
   public <init>(net.corda.core.transactions.SignedTransaction)
   public <init>(net.corda.core.transactions.SignedTransaction, net.corda.core.utilities.ProgressTracker)
   @co.paralleluniverse.fibers.Suspendable @org.jetbrains.annotations.NotNull public List call()

diff --git a/core/src/main/kotlin/net/corda/core/flows/NotarisationRequest.kt b/core/src/main/kotlin/net/corda/core/flows/NotarisationRequest.kt
@@ -1,8 +1,7 @@
 package net.corda.core.flows
 
 import net.corda.core.contracts.StateRef
-import net.corda.core.crypto.DigitalSignature
-import net.corda.core.crypto.SecureHash
+import net.corda.core.crypto.*
 import net.corda.core.identity.Party
 import net.corda.core.serialization.CordaSerializable
 import net.corda.core.serialization.serialize
@@ -43,7 +42,7 @@ class NotarisationRequest(statesToConsume: List<StateRef>, val transactionId: Se
         val signature = requestSignature.digitalSignature
         if (intendedSigner.owningKey != signature.by) {
             val errorMessage = "Expected a signature by ${intendedSigner.owningKey}, but received by ${signature.by}}"
-            throw NotaryException(NotaryError.RequestSignatureInvalid(IllegalArgumentException(errorMessage)))
+            throw NotaryInternalException(NotaryError.RequestSignatureInvalid(IllegalArgumentException(errorMessage)))
         }
         // TODO: if requestSignature was generated over an old version of NotarisationRequest, we need to be able to
         // reserialize it in that version to get the exact same bytes. Modify the serialization logic once that's
@@ -59,7 +58,7 @@ class NotarisationRequest(statesToConsume: List<StateRef>, val transactionId: Se
             when (e) {
                 is InvalidKeyException, is SignatureException -> {
                     val error = NotaryError.RequestSignatureInvalid(e)
-                    throw NotaryException(error)
+                    throw NotaryInternalException(error)
                 }
                 else -> throw e
             }
@@ -98,4 +97,8 @@ data class NotarisationPayload(val transaction: Any, val requestSignature: Notar
      * Should only be used by non-validating notaries.
      */
     val coreTransaction get() = transaction as CoreTransaction
-}
+}
+
+/** Payload returned by the notary service flow to the client. */
+@CordaSerializable
+data class NotarisationResponse(val signatures: List<TransactionSignature>)
diff --git a/core/src/main/kotlin/net/corda/core/flows/NotaryFlow.kt b/core/src/main/kotlin/net/corda/core/flows/NotaryFlow.kt
@@ -1,27 +1,24 @@
 package net.corda.core.flows
 
 import co.paralleluniverse.fibers.Suspendable
+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.SignedData
 import net.corda.core.crypto.TransactionSignature
-import net.corda.core.crypto.keys
 import net.corda.core.identity.Party
 import net.corda.core.internal.FetchDataFlow
 import net.corda.core.internal.generateSignature
+import net.corda.core.internal.validateSignatures
 import net.corda.core.node.services.NotaryService
 import net.corda.core.node.services.TrustedAuthorityNotaryService
-import net.corda.core.node.services.UniquenessProvider
 import net.corda.core.serialization.CordaSerializable
-import net.corda.core.transactions.CoreTransaction
 import net.corda.core.transactions.ContractUpgradeWireTransaction
 import net.corda.core.transactions.SignedTransaction
 import net.corda.core.transactions.WireTransaction
 import net.corda.core.utilities.ProgressTracker
 import net.corda.core.utilities.UntrustworthyData
 import net.corda.core.utilities.unwrap
-import java.security.SignatureException
 import java.time.Instant
 import java.util.function.Predicate
 
@@ -36,6 +33,7 @@ class NotaryFlow {
      * @throws NotaryException in case the any of the inputs to the transaction have been consumed
      *                         by another transaction or the time-window is invalid.
      */
+    @DoNotImplement
     @InitiatingFlow
     open class Client(private val stx: SignedTransaction,
                       override val progressTracker: ProgressTracker) : FlowLogic<List<TransactionSignature>>() {
@@ -68,44 +66,32 @@ class NotaryFlow {
             check(serviceHub.loadStates(stx.inputs.toSet()).all { it.state.notary == notaryParty }) {
                 "Input states must have the same Notary"
             }
-
-            try {
-                stx.resolveTransactionWithSignatures(serviceHub).verifySignaturesExcept(notaryParty.owningKey)
-            } catch (ex: SignatureException) {
-                throw NotaryException(NotaryError.TransactionInvalid(ex))
-            }
+            stx.resolveTransactionWithSignatures(serviceHub).verifySignaturesExcept(notaryParty.owningKey)
             return notaryParty
         }
 
         /** Notarises the transaction with the [notaryParty], obtains the notary's signature(s). */
         @Throws(NotaryException::class)
         @Suspendable
-        protected fun notarise(notaryParty: Party): UntrustworthyData<List<TransactionSignature>> {
-            return try {
-                val session = initiateFlow(notaryParty)
-                val requestSignature = NotarisationRequest(stx.inputs, stx.id).generateSignature(serviceHub)
-                if (serviceHub.networkMapCache.isValidatingNotary(notaryParty)) {
-                    sendAndReceiveValidating(session, requestSignature)
-                } else {
-                    sendAndReceiveNonValidating(notaryParty, session, requestSignature)
-                }
-            } catch (e: NotaryException) {
-                if (e.error is NotaryError.Conflict) {
-                    e.error.conflict.verified()
-                }
-                throw e
+        protected fun notarise(notaryParty: Party): UntrustworthyData<NotarisationResponse> {
+            val session = initiateFlow(notaryParty)
+            val requestSignature = NotarisationRequest(stx.inputs, stx.id).generateSignature(serviceHub)
+            return if (serviceHub.networkMapCache.isValidatingNotary(notaryParty)) {
+                sendAndReceiveValidating(session, requestSignature)
+            } else {
+                sendAndReceiveNonValidating(notaryParty, session, requestSignature)
             }
         }
 
         @Suspendable
-        private fun sendAndReceiveValidating(session: FlowSession, signature: NotarisationRequestSignature): UntrustworthyData<List<TransactionSignature>> {
+        private fun sendAndReceiveValidating(session: FlowSession, signature: NotarisationRequestSignature): UntrustworthyData<NotarisationResponse> {
             val payload = NotarisationPayload(stx, signature)
             subFlow(NotarySendTransactionFlow(session, payload))
             return session.receive()
         }
 
         @Suspendable
-        private fun sendAndReceiveNonValidating(notaryParty: Party, session: FlowSession, signature: NotarisationRequestSignature): UntrustworthyData<List<TransactionSignature>> {
+        private fun sendAndReceiveNonValidating(notaryParty: Party, session: FlowSession, signature: NotarisationRequestSignature): UntrustworthyData<NotarisationResponse> {
             val ctx = stx.coreTransaction
             val tx = when (ctx) {
                 is ContractUpgradeWireTransaction -> ctx.buildFilteredTransaction()
@@ -116,18 +102,13 @@ class NotaryFlow {
         }
 
         /** Checks that the notary's signature(s) is/are valid. */
-        protected fun validateResponse(response: UntrustworthyData<List<TransactionSignature>>, notaryParty: Party): List<TransactionSignature> {
-            return response.unwrap { signatures ->
-                signatures.forEach { validateSignature(it, stx.id, notaryParty) }
-                signatures
+        protected fun validateResponse(response: UntrustworthyData<NotarisationResponse>, notaryParty: Party): List<TransactionSignature> {
+            return response.unwrap {
+                it.validateSignatures(stx.id, notaryParty)
+                it.signatures
             }
         }
 
-        private fun validateSignature(sig: TransactionSignature, txId: SecureHash, notaryParty: Party) {
-            check(sig.by in notaryParty.owningKey.keys) { "Invalid signer for the notary result" }
-            sig.verify(txId)
-        }
-
         /**
          * The [NotarySendTransactionFlow] flow is similar to [SendTransactionFlow], but uses [NotarisationPayload] as the
          * initial message, and retries message delivery.
@@ -156,11 +137,17 @@ class NotaryFlow {
             check(serviceHub.myInfo.legalIdentities.any { serviceHub.networkMapCache.isNotary(it) }) {
                 "We are not a notary on the network"
             }
-            val (id, inputs, timeWindow, notary) = receiveAndVerifyTx()
-            checkNotary(notary)
-            service.validateTimeWindow(timeWindow)
-            service.commitInputStates(inputs, id, otherSideSession.counterparty)
-            signAndSendResponse(id)
+            var txId: SecureHash? = null
+            try {
+                val parts = receiveAndVerifyTx()
+                txId = parts.id
+                checkNotary(parts.notary)
+                service.validateTimeWindow(parts.timestamp)
+                service.commitInputStates(parts.inputs, txId, otherSideSession.counterparty)
+                signTransactionAndSendResponse(txId)
+            } catch (e: NotaryInternalException) {
+                throw NotaryException(e.error, txId)
+            }
             return null
         }
 
@@ -175,14 +162,14 @@ class NotaryFlow {
         @Suspendable
         protected fun checkNotary(notary: Party?) {
             if (notary?.owningKey != service.notaryIdentityKey) {
-                throw NotaryException(NotaryError.WrongNotary)
+                throw NotaryInternalException(NotaryError.WrongNotary)
             }
         }
 
         @Suspendable
-        private fun signAndSendResponse(txId: SecureHash) {
+        private fun signTransactionAndSendResponse(txId: SecureHash) {
             val signature = service.sign(txId)
-            otherSideSession.send(listOf(signature))
+            otherSideSession.send(NotarisationResponse(listOf(signature)))
         }
     }
 }
@@ -197,14 +184,27 @@ data class TransactionParts(val id: SecureHash, val inputs: List<StateRef>, val
  * Exception thrown by the notary service if any issues are encountered while trying to commit a transaction. The
  * underlying [error] specifies the cause of failure.
  */
-class NotaryException(val error: NotaryError) : FlowException("Unable to notarise: $error")
+class NotaryException(
+        /** Cause of notarisation failure. */
+        val error: NotaryError,
+        /** Id of the transaction to be notarised. Can be _null_ if an error occurred before the id could be resolved. */
+        val txId: SecureHash? = null
+) : FlowException("Unable to notarise transaction${txId ?: " "}: $error")
+
+/** Exception internal to the notary service. Does not get exposed to CorDapps and flows calling [NotaryFlow.Client]. */
+class NotaryInternalException(val error: NotaryError) : FlowException("Unable to notarise: $error")
 
 /** Specifies the cause for notarisation request failure. */
 @CordaSerializable
 sealed class NotaryError {
-    /** Occurs when one or more input states of transaction with [txId] have already been consumed by another transaction. */
-    data class Conflict(val txId: SecureHash, val conflict: SignedData<UniquenessProvider.Conflict>) : NotaryError() {
-        override fun toString() = "One or more input states for transaction $txId have been used in another transaction"
+    /** Occurs when one or more input states have already been consumed by another transaction. */
+    data class Conflict(
+            /** Id of the transaction that was attempted to be notarised. */
+            val txId: SecureHash,
+            /** Specifies which states have already been consumed in another transaction. */
+            val consumedStates: Map<StateRef, StateConsumptionDetails>
+    ) : NotaryError() {
+        override fun toString() = "One or more input states have been used in another transaction"
     }
 
     /** Occurs when time specified in the [TimeWindow] command is outside the allowed tolerance. */
@@ -236,3 +236,15 @@ sealed class NotaryError {
         override fun toString() = cause.toString()
     }
 }
+
+/** Contains information about the consuming transaction for a particular state. */
+// TODO: include notary timestamp?
+@CordaSerializable
+data class StateConsumptionDetails(
+        /**
+         * Hash of the consuming transaction id.
+         *
+         * Note that this is NOT the transaction id itself – revealing it could lead to privacy leaks.
+         */
+        val hashOfTransactionId: SecureHash
+)
diff --git a/core/src/main/kotlin/net/corda/core/internal/NotaryUtils.kt b/core/src/main/kotlin/net/corda/core/internal/NotaryUtils.kt
@@ -0,0 +1,16 @@
+package net.corda.core.internal
+
+import net.corda.core.crypto.SecureHash
+import net.corda.core.crypto.isFulfilledBy
+import net.corda.core.flows.NotarisationResponse
+import net.corda.core.identity.Party
+
+/**
+ * Checks that there are sufficient signatures to satisfy the notary signing requirement and validates the signatures
+ * against the given transaction id.
+ */
+fun NotarisationResponse.validateSignatures(txId: SecureHash, notary: Party) {
+    val signingKeys = signatures.map { it.by }
+    require(notary.owningKey.isFulfilledBy(signingKeys)) { "Insufficient signatures to fulfill the notary signing requirement for $notary" }
+    signatures.forEach { it.verify(txId) }
+}
diff --git a/core/src/main/kotlin/net/corda/core/node/services/NotaryService.kt b/core/src/main/kotlin/net/corda/core/node/services/NotaryService.kt
@@ -8,7 +8,6 @@ import net.corda.core.flows.*
 import net.corda.core.identity.Party
 import net.corda.core.node.ServiceHub
 import net.corda.core.serialization.SingletonSerializeAsToken
-import net.corda.core.serialization.serialize
 import net.corda.core.utilities.contextLogger
 import org.slf4j.Logger
 import java.security.PublicKey
@@ -30,18 +29,19 @@ abstract class NotaryService : SingletonSerializeAsToken() {
         }
 
         /**
-         * Checks if the current instant provided by the clock falls within the specified time window.
+         * Checks if the current instant provided by the clock falls within the specified time window. Should only be
+         * used by a notary service flow.
          *
-         * @throws NotaryException if current time is outside the specified time window. The exception contains
+         * @throws NotaryInternalException if current time is outside the specified time window. The exception contains
          *                         the [NotaryError.TimeWindowInvalid] error.
          */
         @JvmStatic
-        @Throws(NotaryException::class)
+        @Throws(NotaryInternalException::class)
         fun validateTimeWindow(clock: Clock, timeWindow: TimeWindow?) {
             if (timeWindow == null) return
             val currentTime = clock.instant()
             if (currentTime !in timeWindow) {
-                throw NotaryException(
+                throw NotaryInternalException(
                         NotaryError.TimeWindowInvalid(currentTime, timeWindow)
                 )
             }
@@ -82,28 +82,24 @@ abstract class TrustedAuthorityNotaryService : NotaryService() {
     fun commitInputStates(inputs: List<StateRef>, txId: SecureHash, caller: Party) {
         try {
             uniquenessProvider.commit(inputs, txId, caller)
-        } catch (e: UniquenessException) {
-            val conflicts = inputs.filterIndexed { i, stateRef ->
-                val consumingTx = e.error.stateHistory[stateRef]
-                consumingTx != null && consumingTx != UniquenessProvider.ConsumingTx(txId, i, caller)
-            }
-            if (conflicts.isNotEmpty()) {
-                // TODO: Create a new UniquenessException that only contains the conflicts filtered above.
-                log.warn("Notary conflicts for $txId: $conflicts")
-                throw notaryException(txId, e)
-            }
+        } catch (e: NotaryInternalException) {
+            if (e.error is NotaryError.Conflict) {
+                val conflicts = inputs.filterIndexed { _, stateRef ->
+                    val cause = e.error.consumedStates[stateRef]
+                    cause != null && cause.hashOfTransactionId != txId.sha256()
+                }
+                if (conflicts.isNotEmpty()) {
+                    // TODO: Create a new UniquenessException that only contains the conflicts filtered above.
+                    log.info("Notary conflicts for $txId: $conflicts")
+                    throw e
+                }
+            } else throw e
         } catch (e: Exception) {
             log.error("Internal error", e)
-            throw NotaryException(NotaryError.General(Exception("Service unavailable, please try again later")))
+            throw NotaryInternalException(NotaryError.General(Exception("Service unavailable, please try again later")))
         }
     }
 
-    private fun notaryException(txId: SecureHash, e: UniquenessException): NotaryException {
-        val conflictData = e.error.serialize()
-        val signedConflict = SignedData(conflictData, sign(conflictData.bytes))
-        return NotaryException(NotaryError.Conflict(txId, signedConflict))
-    }
-
     /** Sign a [ByteArray] input. */
     fun sign(bits: ByteArray): DigitalSignature.WithKey {
         return services.keyManagementService.sign(bits, notaryIdentityKey)
@@ -117,6 +113,8 @@ abstract class TrustedAuthorityNotaryService : NotaryService() {
 
     // TODO: Sign multiple transactions at once by building their Merkle tree and then signing over its root.
 
-    @Deprecated("This property is no longer used") @Suppress("DEPRECATION")
-    protected open val timeWindowChecker: TimeWindowChecker get() = throw UnsupportedOperationException("No default implementation, need to override")
+    @Deprecated("This property is no longer used")
+    @Suppress("DEPRECATION")
+    protected open val timeWindowChecker: TimeWindowChecker
+        get() = throw UnsupportedOperationException("No default implementation, need to override")
 }