From 1243ca20663aa9f0d46f125559193213e99b9512 Mon Sep 17 00:00:00 2001 From: Mike Hearn Date: Wed, 17 Feb 2016 18:38:24 +0100 Subject: [PATCH] Trading: in the two party trade protocol, check that the proposed transaction is contract-valid, and that the missing signatures are what is expected. --- .../kotlin/core/TransactionVerification.kt | 1 - core/src/main/kotlin/core/Transactions.kt | 17 +++++--- .../protocols/TwoPartyTradeProtocol.kt | 41 ++++++++++--------- src/main/kotlin/core/Services.kt | 16 ++++++-- .../messaging/TwoPartyTradeProtocolTests.kt | 7 +++- 5 files changed, 52 insertions(+), 30 deletions(-) diff --git a/core/src/main/kotlin/core/TransactionVerification.kt b/core/src/main/kotlin/core/TransactionVerification.kt index 69e5f4079a..af3f6e0abc 100644 --- a/core/src/main/kotlin/core/TransactionVerification.kt +++ b/core/src/main/kotlin/core/TransactionVerification.kt @@ -23,7 +23,6 @@ class TransactionConflictException(val conflictRef: StateRef, val tx1: LedgerTra * [nonVerifiedRoots] set. Transactions in the non-verified set are ignored other than for looking up input states. */ class TransactionGroup(val transactions: Set, val nonVerifiedRoots: Set) { - /** * Verifies the group and returns the set of resolved transactions. */ diff --git a/core/src/main/kotlin/core/Transactions.kt b/core/src/main/kotlin/core/Transactions.kt index d64ff6bf68..54ac529403 100644 --- a/core/src/main/kotlin/core/Transactions.kt +++ b/core/src/main/kotlin/core/Transactions.kt @@ -121,18 +121,25 @@ data class SignedTransaction(val txBits: SerializedBytes, val s /** * Verify the signatures, deserialise the wire transaction and then check that the set of signatures found matches - * the set of pubkeys in the commands. + * the set of pubkeys in the commands. If any signatures are missing, either throws an exception (by default) or + * returns the list of keys that have missing signatures, depending on the parameter. * - * @throws SignatureException if the signature is invalid or does not match. + * @throws SignatureException if a signature is invalid, does not match or if any signature is missing. */ - fun verify() { + fun verify(throwIfSignaturesAreMissing: Boolean = true): Set { verifySignatures() // Verify that every command key was in the set that we just verified: there should be no commands that were // unverified. val cmdKeys = tx.commands.flatMap { it.pubkeys }.toSet() val sigKeys = sigs.map { it.by }.toSet() - if (!sigKeys.containsAll(cmdKeys)) - throw SignatureException("Missing signatures on transaction ${id.prefixChars()} for: ${(cmdKeys - sigKeys).map { it.toStringShort() }}") + if (sigKeys == cmdKeys) + return emptySet() + + val missing = cmdKeys - sigKeys + if (throwIfSignaturesAreMissing) + throw SignatureException("Missing signatures on transaction ${id.prefixChars()} for: ${missing.map { it.toStringShort() }}") + else + return missing } /** diff --git a/src/main/kotlin/contracts/protocols/TwoPartyTradeProtocol.kt b/src/main/kotlin/contracts/protocols/TwoPartyTradeProtocol.kt index 883f80e683..32c1ad9435 100644 --- a/src/main/kotlin/contracts/protocols/TwoPartyTradeProtocol.kt +++ b/src/main/kotlin/contracts/protocols/TwoPartyTradeProtocol.kt @@ -23,6 +23,7 @@ import core.node.TimestampingProtocol import core.utilities.trace import java.security.KeyPair import java.security.PublicKey +import java.security.SignatureException import java.time.Instant /** @@ -110,26 +111,31 @@ object TwoPartyTradeProtocol { val maybeSTX = sendAndReceive(TRADE_TOPIC, otherSide, buyerSessionID, sessionID, hello) maybeSTX.validate { - it.verifySignatures() + // Check that the tx proposed by the buyer is valid. + val missingSigs = it.verify(throwIfSignaturesAreMissing = false) + if (missingSigs != setOf(myKeyPair.public, timestampingAuthority.identity.owningKey)) + throw SignatureException("The set of missing signatures is not as expected: $missingSigs") + val wtx: WireTransaction = it.tx logger.trace { "Received partially signed transaction: ${it.id}" } checkDependencies(it) - // TODO: Verify that the transaction is contract-valid even though it lacks sufficient signatures. + // This verifies that the transaction is contract-valid, even though it is missing signatures. + serviceHub.verifyTransaction(wtx.toLedgerTransaction(serviceHub.identityService)) - requireThat { - "transaction sends us the right amount of cash" by (wtx.outputs.sumCashBy(myKeyPair.public) == price) - // There are all sorts of funny games a malicious secondary might play here, we should fix them: - // - // - This tx may attempt to send some assets we aren't intending to sell to the secondary, if - // we're reusing keys! So don't reuse keys! - // - This tx may include output states that impose odd conditions on the movement of the cash, - // once we implement state pairing. - // - // but the goal of this code is not to be fully secure, but rather, just to find good ways to - // express protocol state machines on top of the messaging layer. - } + if (wtx.outputs.sumCashBy(myKeyPair.public) != price) + throw IllegalArgumentException("Transaction is not sending us the right amounnt of cash") + + // There are all sorts of funny games a malicious secondary might play here, we should fix them: + // + // - This tx may attempt to send some assets we aren't intending to sell to the secondary, if + // we're reusing keys! So don't reuse keys! + // - This tx may include output states that impose odd conditions on the movement of the cash, + // once we implement state pairing. + // + // but the goal of this code is not to be fully secure (yet), but rather, just to find good ways to + // express protocol state machines on top of the messaging layer. return it } @@ -220,12 +226,7 @@ object TwoPartyTradeProtocol { ptx.signWith(KeyPair(k, priv)) } - val stx = ptx.toSignedTransaction(checkSufficientSignatures = false) - stx.verifySignatures() // Verifies that we generated a signed transaction correctly. - - // TODO: Could run verify() here to make sure the only signature missing is the sellers. - - return stx + return ptx.toSignedTransaction(checkSufficientSignatures = false) } private fun assembleSharedTX(tradeRequest: SellerTradeInfo): Pair> { diff --git a/src/main/kotlin/core/Services.kt b/src/main/kotlin/core/Services.kt index f584ed8a67..c2049cbba7 100644 --- a/src/main/kotlin/core/Services.kt +++ b/src/main/kotlin/core/Services.kt @@ -8,13 +8,10 @@ package core -import co.paralleluniverse.fibers.Suspendable -import core.crypto.DigitalSignature import core.crypto.SecureHash import core.crypto.generateKeyPair import core.messaging.MessagingService import core.messaging.NetworkMap -import core.serialization.SerializedBytes import java.security.KeyPair import java.security.PrivateKey import java.security.PublicKey @@ -113,4 +110,17 @@ interface ServiceHub { val storageService: StorageService val networkService: MessagingService val networkMapService: NetworkMap + + /** + * Given a [LedgerTransaction], looks up all its dependencies in the local database, uses the identity service to map + * the [SignedTransaction]s the DB gives back into [LedgerTransaction]s, and then runs the smart contracts for the + * transaction. If no exception is thrown, the transaction is valid. + */ + fun verifyTransaction(ltx: LedgerTransaction) { + val dependencies = ltx.inputs.map { + storageService.validatedTransactions[it.txhash] ?: throw TransactionResolutionException(it.txhash) + } + val ltxns = dependencies.map { it.verifyToLedgerTransaction(identityService) } + TransactionGroup(setOf(ltx), ltxns.toSet()).verify(storageService.contractPrograms) + } } diff --git a/src/test/kotlin/core/messaging/TwoPartyTradeProtocolTests.kt b/src/test/kotlin/core/messaging/TwoPartyTradeProtocolTests.kt index 0e902f619d..f737e0a28e 100644 --- a/src/test/kotlin/core/messaging/TwoPartyTradeProtocolTests.kt +++ b/src/test/kotlin/core/messaging/TwoPartyTradeProtocolTests.kt @@ -261,7 +261,12 @@ class TwoPartyTradeProtocolTests : TestWithInMemoryNetwork() { // Bob's transactions are valid, so she commits to the database RecordingMap.Put(bobsFakeCash[1].id, bobsSignedTxns[bobsFakeCash[1].id]), RecordingMap.Put(bobsFakeCash[2].id, bobsSignedTxns[bobsFakeCash[2].id]), - RecordingMap.Put(bobsFakeCash[0].id, bobsSignedTxns[bobsFakeCash[0].id]) + RecordingMap.Put(bobsFakeCash[0].id, bobsSignedTxns[bobsFakeCash[0].id]), + // Now she verifies the transaction is contract-valid (not signature valid) which means + // looking up the states again. + RecordingMap.Get(bobsFakeCash[1].id), + RecordingMap.Get(bobsFakeCash[2].id), + RecordingMap.Get(alicesFakePaper[0].id) ) assertEquals(expected, records) }