Improve the usability of SignedTransaction.verifySignatures taking into account how it is typically used.

This commit is contained in:
Mike Hearn 2016-09-06 17:23:55 +02:00
parent 2af6a70b9a
commit 49a0fd8fdd
11 changed files with 102 additions and 65 deletions

View File

@ -47,7 +47,7 @@ abstract class AbstractConserveAmount<S : FungibleAsset<T>, C : CommandData, T :
* @param tx transaction builder to add states and commands to.
* @param amountIssued the amount to be exited, represented as a quantity of issued currency.
* @param assetStates the asset states to take funds from. No checks are done about ownership of these states, it is
* the responsibility of the caller to check that they do not exit funds held by others.
* the responsibility of the caller to check that they do not attempt to exit funds held by others.
* @return the public key of the assets issuer, who must sign the transaction for it to be valid.
*/
fun generateExit(tx: TransactionBuilder, amountIssued: Amount<Issued<T>>,

View File

@ -7,7 +7,6 @@ import com.r3corda.core.contracts.*
import com.r3corda.core.crypto.DigitalSignature
import com.r3corda.core.crypto.Party
import com.r3corda.core.crypto.signWithECDSA
import com.r3corda.core.crypto.toStringsShort
import com.r3corda.core.node.NodeInfo
import com.r3corda.core.protocols.ProtocolLogic
import com.r3corda.core.random63BitValue
@ -19,7 +18,6 @@ import com.r3corda.core.utilities.ProgressTracker
import com.r3corda.core.utilities.trace
import java.security.KeyPair
import java.security.PublicKey
import java.security.SignatureException
import java.util.*
/**
@ -124,12 +122,7 @@ object TwoPartyTradeProtocol {
progressTracker.nextStep()
// Check that the tx proposed by the buyer is valid.
val missingSigs: Set<PublicKey> = it.verifySignatures(throwIfSignaturesAreMissing = false)
val expected = setOf(myKeyPair.public, notaryNode.identity.owningKey)
if (missingSigs != expected)
throw SignatureException("The set of missing signatures is not as expected: ${missingSigs.toStringsShort()} vs ${expected.toStringsShort()}")
val wtx: WireTransaction = it.tx
val wtx: WireTransaction = it.verifySignatures(myKeyPair.public, notaryNode.identity.owningKey)
logger.trace { "Received partially signed transaction: ${it.id}" }
// Download and check all the things that this transaction depends on and verify it is contract-valid,

View File

@ -6,9 +6,6 @@ import com.r3corda.core.crypto.NullPublicKey
import com.r3corda.core.crypto.SecureHash
import com.r3corda.core.serialization.OpaqueBytes
import com.r3corda.core.utilities.*
import com.r3corda.testing.LedgerDSL
import com.r3corda.testing.TestLedgerDSLInterpreter
import com.r3corda.testing.TestTransactionDSLInterpreter
import com.r3corda.testing.*
import org.junit.Test
import java.security.PublicKey

View File

@ -23,7 +23,7 @@ import java.util.*
data class SignedTransaction(val txBits: SerializedBytes<WireTransaction>,
val sigs: List<DigitalSignature.WithKey>) : NamedByHash {
init {
check(sigs.isNotEmpty())
require(sigs.isNotEmpty())
}
// TODO: This needs to be reworked to ensure that the inner WireTransaction is only ever deserialised sandboxed.
@ -34,27 +34,46 @@ data class SignedTransaction(val txBits: SerializedBytes<WireTransaction>,
/** A transaction ID is the hash of the [WireTransaction]. Thus adding or removing a signature does not change it. */
override val id: SecureHash get() = txBits.hash
class SignaturesMissingException(val missing: Set<PublicKey>, val descriptions: List<String>, override val id: SecureHash) : NamedByHash, SignatureException() {
override fun toString(): String {
return "Missing signatures for $descriptions on transaction ${id.prefixChars()} for ${missing.toStringsShort()}"
}
}
/**
* Verify the signatures, deserialise the wire transaction and then check that the set of signatures found contains
* the set of pubkeys in the signers list. 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.
* Verifies the signatures on this transaction and throws if any are missing which aren't passed as parameters.
* In this context, "verifying" means checking they are valid signatures and that their public keys are in
* the contained transactions [BaseTransaction.mustSign] property.
*
* @throws SignatureException if a signature is invalid, does not match or if any signature is missing.
* Normally you would not provide any keys to this function, but if you're in the process of building a partial
* transaction and you want to access the contents before you've signed it, you can specify your own keys here
* to bypass that check.
*
* @throws SignatureException if any signatures are invalid or unrecognised.
* @throws SignaturesMissingException if any signatures should have been present but were not.
*/
@Throws(SignatureException::class)
fun verifySignatures(throwIfSignaturesAreMissing: Boolean = true): Set<PublicKey> {
fun verifySignatures(vararg allowedToBeMissing: PublicKey): WireTransaction {
// Embedded WireTransaction is not deserialised until after we check the signatures.
for (sig in sigs)
sig.verifyWithECDSA(txBits.bits)
// Now examine the contents and ensure the sigs we have line up with the advertised list of signers.
val missing = getMissingSignatures()
if (missing.isNotEmpty() && throwIfSignaturesAreMissing) {
val missingElements = getMissingKeyDescriptions(missing)
throw SignatureException("Missing signatures for ${missingElements} on transaction ${id.prefixChars()} for ${missing.toStringsShort()}")
if (missing.isNotEmpty()) {
val allowed = setOf(*allowedToBeMissing)
val needed = missing - allowed
if (needed.isNotEmpty())
throw SignaturesMissingException(needed, getMissingKeyDescriptions(needed), id)
}
return tx
}
return missing
private fun getMissingSignatures(): Set<PublicKey> {
val requiredKeys = tx.mustSign.toSet()
val sigKeys = sigs.map { it.by }.toSet()
if (sigKeys.containsAll(requiredKeys)) return emptySet()
return requiredKeys - sigKeys
}
/**
@ -65,13 +84,11 @@ data class SignedTransaction(val txBits: SerializedBytes<WireTransaction>,
// TODO: We need a much better way of structuring this data
val missingElements = ArrayList<String>()
this.tx.commands.forEach { command ->
if (command.signers.any { signer -> missing.contains(signer) })
if (command.signers.any { it in missing })
missingElements.add(command.toString())
}
this.tx.notary?.owningKey.apply {
if (missing.contains(this))
missingElements.add("notary")
}
if (this.tx.notary?.owningKey in missing)
missingElements.add("notary")
return missingElements
}
@ -85,16 +102,6 @@ data class SignedTransaction(val txBits: SerializedBytes<WireTransaction>,
/** Alias for [withAdditionalSignatures] to let you use Kotlin operator overloading. */
operator fun plus(sigList: Collection<DigitalSignature.WithKey>) = withAdditionalSignatures(sigList)
/**
* Returns the set of missing signatures - a signature must be present for each signer public key.
*/
private fun getMissingSignatures(): Set<PublicKey> {
val requiredKeys = tx.mustSign.toSet()
val sigKeys = sigs.map { it.by }.toSet()
if (sigKeys.containsAll(requiredKeys)) return emptySet()
return requiredKeys - sigKeys
}
/**
* Calls [verifySignatures] to check all required signatures are present, and then calls
* [WireTransaction.toLedgerTransaction] with the passed in [ServiceHub] to resolve the dependencies,
@ -102,10 +109,9 @@ data class SignedTransaction(val txBits: SerializedBytes<WireTransaction>,
*
* @throws FileNotFoundException if a required attachment was not found in storage.
* @throws TransactionResolutionException if an input points to a transaction not found in storage.
* @throws SignatureException if any signatures were invalid or unrecognised
* @throws SignaturesMissingException if any signatures that should have been present are missing.
*/
@Throws(FileNotFoundException::class, TransactionResolutionException::class)
fun toLedgerTransaction(services: ServiceHub): LedgerTransaction {
verifySignatures()
return tx.toLedgerTransaction(services)
}
@Throws(FileNotFoundException::class, TransactionResolutionException::class, SignaturesMissingException::class)
fun toLedgerTransaction(services: ServiceHub) = verifySignatures().toLedgerTransaction(services)
}

View File

@ -205,5 +205,5 @@ sealed class NotaryError {
class TransactionInvalid : NotaryError()
class SignaturesMissing(val missingSigners: List<PublicKey>) : NotaryError()
class SignaturesMissing(val missingSigners: Set<PublicKey>) : NotaryError()
}

View File

@ -2,12 +2,12 @@ package com.r3corda.protocols
import co.paralleluniverse.fibers.Suspendable
import com.r3corda.core.checkedAdd
import com.r3corda.core.transactions.LedgerTransaction
import com.r3corda.core.transactions.SignedTransaction
import com.r3corda.core.transactions.WireTransaction
import com.r3corda.core.crypto.Party
import com.r3corda.core.crypto.SecureHash
import com.r3corda.core.protocols.ProtocolLogic
import com.r3corda.core.transactions.LedgerTransaction
import com.r3corda.core.transactions.SignedTransaction
import com.r3corda.core.transactions.WireTransaction
import java.util.*
// TODO: This code is currently unit tested by TwoPartyTradeProtocolTests, it should have its own tests.
@ -82,7 +82,7 @@ class ResolveTransactionsProtocol(private val txHashes: Set<SecureHash>,
// be a clearer API if we do that. But for consistency with the other c'tor we currently do not.
//
// If 'stx' is set, then 'wtx' is the contents (from the c'tor).
stx?.verifySignatures()
val wtx = stx?.verifySignatures() ?: wtx
wtx?.let {
fetchMissingAttachments(listOf(it))
val ltx = it.toLedgerTransaction(serviceHub)

View File

@ -20,7 +20,6 @@ import com.r3corda.core.utilities.trace
import java.math.BigDecimal
import java.security.KeyPair
import java.security.PublicKey
import java.security.SignatureException
import java.time.Duration
/**
@ -105,11 +104,7 @@ object TwoPartyDealProtocol {
progressTracker.nextStep()
// Check that the tx proposed by the buyer is valid.
val missingSigs = stx.verifySignatures(throwIfSignaturesAreMissing = false)
if (missingSigs != setOf(myKeyPair.public, notaryNode.identity.owningKey))
throw SignatureException("The set of missing signatures is not as expected: $missingSigs")
val wtx: WireTransaction = stx.tx
val wtx: WireTransaction = stx.verifySignatures(myKeyPair.public, notaryNode.identity.owningKey)
logger.trace { "Received partially signed transaction: ${stx.id}" }
checkDependencies(stx)

View File

@ -1,12 +1,12 @@
package com.r3corda.protocols
import co.paralleluniverse.fibers.Suspendable
import com.r3corda.core.transactions.SignedTransaction
import com.r3corda.core.contracts.TransactionVerificationException
import com.r3corda.core.transactions.WireTransaction
import com.r3corda.core.crypto.Party
import com.r3corda.core.node.services.TimestampChecker
import com.r3corda.core.node.services.UniquenessProvider
import com.r3corda.core.transactions.SignedTransaction
import com.r3corda.core.transactions.WireTransaction
import java.security.SignatureException
/**
@ -37,10 +37,11 @@ class ValidatingNotaryProtocol(otherSide: Party,
}
private fun checkSignatures(stx: SignedTransaction) {
val myKey = serviceHub.storageService.myLegalIdentity.owningKey
val missing = stx.verifySignatures(throwIfSignaturesAreMissing = false) - myKey
if (missing.isNotEmpty()) throw NotaryException(NotaryError.SignaturesMissing(missing.toList()))
try {
stx.verifySignatures(serviceHub.storageService.myLegalIdentity.owningKey)
} catch(e: SignedTransaction.SignaturesMissingException) {
throw NotaryException(NotaryError.SignaturesMissing(e.missing))
}
}
@Suspendable

View File

@ -1,17 +1,61 @@
package com.r3corda.core.contracts
import com.r3corda.contracts.asset.DUMMY_CASH_ISSUER_KEY
import com.r3corda.core.crypto.Party
import com.r3corda.core.crypto.SecureHash
import com.r3corda.core.crypto.signWithECDSA
import com.r3corda.core.serialization.SerializedBytes
import com.r3corda.core.transactions.LedgerTransaction
import com.r3corda.core.transactions.SignedTransaction
import com.r3corda.core.transactions.WireTransaction
import com.r3corda.core.utilities.DUMMY_KEY_1
import com.r3corda.core.utilities.DUMMY_KEY_2
import com.r3corda.core.utilities.DUMMY_NOTARY
import com.r3corda.core.utilities.DUMMY_NOTARY_KEY
import com.r3corda.testing.ALICE
import com.r3corda.testing.ALICE_PUBKEY
import com.r3corda.testing.BOB
import org.junit.Test
import java.security.KeyPair
import kotlin.test.assertEquals
import kotlin.test.assertFailsWith
class TransactionTypeTests {
class TransactionTests {
@Test
fun `signed transaction missing signatures`() {
val wtx = WireTransaction(
inputs = listOf(StateRef(SecureHash.randomSHA256(), 0)),
attachments = emptyList(),
outputs = emptyList(),
commands = emptyList(),
notary = DUMMY_NOTARY,
signers = listOf(DUMMY_KEY_1.public, DUMMY_KEY_2.public),
type = TransactionType.General(),
timestamp = null
)
val bits: SerializedBytes<WireTransaction> = wtx.serialized
fun make(vararg keys: KeyPair) = SignedTransaction(bits, keys.map { it.signWithECDSA(bits) })
assertFailsWith<IllegalArgumentException> { make().verifySignatures() }
assertEquals(
setOf(DUMMY_KEY_1.public),
assertFailsWith<SignedTransaction.SignaturesMissingException> { make(DUMMY_KEY_2).verifySignatures() }.missing
)
assertEquals(
setOf(DUMMY_KEY_2.public),
assertFailsWith<SignedTransaction.SignaturesMissingException> { make(DUMMY_KEY_1).verifySignatures() }.missing
)
assertEquals(
setOf(DUMMY_KEY_2.public),
assertFailsWith<SignedTransaction.SignaturesMissingException> { make(DUMMY_CASH_ISSUER_KEY).verifySignatures(DUMMY_KEY_1.public) }.missing
)
make(DUMMY_KEY_1).verifySignatures(DUMMY_KEY_2.public)
make(DUMMY_KEY_2).verifySignatures(DUMMY_KEY_1.public)
make(DUMMY_KEY_1, DUMMY_KEY_2).verifySignatures()
}
@Test
fun `transactions with no inputs can have any notary`() {
val baseOutState = TransactionState(DummyContract.SingleOwnerState(0, ALICE_PUBKEY), DUMMY_NOTARY)

View File

@ -5,7 +5,8 @@ import com.r3corda.core.crypto.SecureHash
import com.r3corda.core.seconds
import com.r3corda.core.transactions.TransactionBuilder
import com.r3corda.core.utilities.*
import com.r3corda.testing.*
import com.r3corda.testing.MINI_CORP
import com.r3corda.testing.generateStateRef
import org.junit.Before
import org.junit.Test
import java.security.PublicKey
@ -84,7 +85,7 @@ class TransactionSerializationTests {
val signedTX = tx.toSignedTransaction()
// Cannot construct with an empty sigs list.
assertFailsWith(IllegalStateException::class) {
assertFailsWith(IllegalArgumentException::class) {
signedTX.copy(sigs = emptyList())
}

View File

@ -5,7 +5,6 @@ import com.r3corda.core.contracts.DummyContract
import com.r3corda.core.contracts.TransactionType
import com.r3corda.core.utilities.DUMMY_NOTARY
import com.r3corda.core.utilities.DUMMY_NOTARY_KEY
import com.r3corda.testing.node.MockNetwork
import com.r3corda.node.services.network.NetworkMapService
import com.r3corda.node.services.transactions.ValidatingNotaryService
import com.r3corda.protocols.NotaryError
@ -13,6 +12,7 @@ import com.r3corda.protocols.NotaryException
import com.r3corda.protocols.NotaryProtocol
import com.r3corda.testing.MEGA_CORP_KEY
import com.r3corda.testing.MINI_CORP_KEY
import com.r3corda.testing.node.MockNetwork
import org.assertj.core.api.Assertions.assertThat
import org.junit.Before
import org.junit.Test
@ -73,6 +73,6 @@ class ValidatingNotaryServiceTests {
assertThat(notaryError).isInstanceOf(NotaryError.SignaturesMissing::class.java)
val missingKeys = (notaryError as NotaryError.SignaturesMissing).missingSigners
assertEquals(missingKeys, listOf(expectedMissingKey))
assertEquals(setOf(expectedMissingKey), missingKeys)
}
}