diff --git a/confidential-identities/src/main/kotlin/net/corda/confidential/SwapIdentitiesFlow.kt b/confidential-identities/src/main/kotlin/net/corda/confidential/SwapIdentitiesFlow.kt index b3da56637a..50f21923cc 100644 --- a/confidential-identities/src/main/kotlin/net/corda/confidential/SwapIdentitiesFlow.kt +++ b/confidential-identities/src/main/kotlin/net/corda/confidential/SwapIdentitiesFlow.kt @@ -1,118 +1,106 @@ package net.corda.confidential import co.paralleluniverse.fibers.Suspendable +import net.corda.core.CordaInternal import net.corda.core.crypto.DigitalSignature +import net.corda.core.crypto.verify import net.corda.core.flows.FlowException import net.corda.core.flows.FlowLogic -import net.corda.core.flows.InitiatingFlow -import net.corda.core.flows.StartableByRPC +import net.corda.core.flows.FlowSession import net.corda.core.identity.AnonymousParty import net.corda.core.identity.CordaX500Name import net.corda.core.identity.Party import net.corda.core.identity.PartyAndCertificate -import net.corda.core.node.services.IdentityService +import net.corda.core.internal.VisibleForTesting +import net.corda.core.node.ServiceHub import net.corda.core.serialization.CordaSerializable -import net.corda.core.serialization.SerializedBytes -import net.corda.core.serialization.deserialize import net.corda.core.serialization.serialize import net.corda.core.utilities.ProgressTracker import net.corda.core.utilities.unwrap import java.security.PublicKey import java.security.SignatureException -import java.util.* /** * Very basic flow which generates new confidential identities for parties in a transaction and exchanges the transaction * key and certificate paths between the parties. This is intended for use as a sub-flow of another flow which builds a - * transaction. + * transaction. The flow running on the other side must also call this flow at the correct location. */ -@StartableByRPC -@InitiatingFlow -// TODO Make this non-initiating as otherwise any CorDapp using confidential identities will cause its node to have an -// open door where any counterparty will be able to swap identities at will. Instead SwapIdentitiesFlow and its counterpart, -// SwapIdentitiesHandler, should be in-lined and called by CorDapp specfic-flows. -class SwapIdentitiesFlow(private val otherParty: Party, - private val revocationEnabled: Boolean, - override val progressTracker: ProgressTracker) : FlowLogic>() { - constructor(otherParty: Party) : this(otherParty, false, tracker()) - +class SwapIdentitiesFlow @JvmOverloads constructor(private val otherSideSession: FlowSession, + override val progressTracker: ProgressTracker = tracker()) : FlowLogic() { companion object { - object AWAITING_KEY : ProgressTracker.Step("Awaiting key") + object GENERATING_IDENTITY : ProgressTracker.Step("Generating our anonymous identity") + object SIGNING_IDENTITY : ProgressTracker.Step("Signing our anonymous identity") + object AWAITING_IDENTITY : ProgressTracker.Step("Awaiting counterparty's anonymous identity") + object VERIFYING_IDENTITY : ProgressTracker.Step("Verifying counterparty's anonymous identity") + + @JvmStatic + fun tracker(): ProgressTracker = ProgressTracker(GENERATING_IDENTITY, SIGNING_IDENTITY, AWAITING_IDENTITY, VERIFYING_IDENTITY) - fun tracker() = ProgressTracker(AWAITING_KEY) /** * Generate the deterministic data blob the confidential identity's key holder signs to indicate they want to * represent the subject named in the X.509 certificate. Note that this is never actually sent between nodes, * but only the signature is sent. The blob is built independently on each node and the received signature * verified against the expected blob, rather than exchanging the blob. */ - fun buildDataToSign(confidentialIdentity: PartyAndCertificate): ByteArray { - val certOwnerAssert = CertificateOwnershipAssertion(confidentialIdentity.name, confidentialIdentity.owningKey) - return certOwnerAssert.serialize().bytes + @CordaInternal + @VisibleForTesting + internal fun buildDataToSign(identity: PartyAndCertificate): ByteArray { + return CertificateOwnershipAssertion(identity.name, identity.owningKey).serialize().bytes } - @Throws(SwapIdentitiesException::class) - fun validateAndRegisterIdentity(identityService: IdentityService, - otherSide: Party, - anonymousOtherSideBytes: PartyAndCertificate, - sigBytes: DigitalSignature): PartyAndCertificate { - val anonymousOtherSide: PartyAndCertificate = anonymousOtherSideBytes - if (anonymousOtherSide.name != otherSide.name) { + @CordaInternal + @VisibleForTesting + internal fun validateAndRegisterIdentity(serviceHub: ServiceHub, + otherSide: Party, + theirAnonymousIdentity: PartyAndCertificate, + signature: DigitalSignature): PartyAndCertificate { + if (theirAnonymousIdentity.name != otherSide.name) { throw SwapIdentitiesException("Certificate subject must match counterparty's well known identity.") } - val signature = DigitalSignature.WithKey(anonymousOtherSide.owningKey, sigBytes.bytes) try { - signature.verify(buildDataToSign(anonymousOtherSideBytes)) - } catch(ex: SignatureException) { + theirAnonymousIdentity.owningKey.verify(buildDataToSign(theirAnonymousIdentity), signature) + } catch (ex: SignatureException) { throw SwapIdentitiesException("Signature does not match the expected identity ownership assertion.", ex) } - // Validate then store their identity so that we can prove the key in the transaction is owned by the - // counterparty. - identityService.verifyAndRegisterIdentity(anonymousOtherSide) - return anonymousOtherSide + // Validate then store their identity so that we can prove the key in the transaction is owned by the counterparty. + serviceHub.identityService.verifyAndRegisterIdentity(theirAnonymousIdentity) + return theirAnonymousIdentity } } @Suspendable - override fun call(): LinkedHashMap { - progressTracker.currentStep = AWAITING_KEY - val legalIdentityAnonymous = serviceHub.keyManagementService.freshKeyAndCert(ourIdentityAndCert, revocationEnabled) - val serializedIdentity = SerializedBytes(legalIdentityAnonymous.serialize().bytes) - - // Special case that if we're both parties, a single identity is generated. - // TODO: for increased privacy, we should create one anonymous key per output state. - val identities = LinkedHashMap() - if (serviceHub.myInfo.isLegalIdentity(otherParty)) { - identities[otherParty] = legalIdentityAnonymous.party.anonymise() - } else { - val otherSession = initiateFlow(otherParty) - val data = buildDataToSign(legalIdentityAnonymous) - val ourSig: DigitalSignature.WithKey = serviceHub.keyManagementService.sign(data, legalIdentityAnonymous.owningKey) - val ourIdentWithSig = IdentityWithSignature(serializedIdentity, ourSig.withoutKey()) - val anonymousOtherSide = otherSession.sendAndReceive(ourIdentWithSig) - .unwrap { (confidentialIdentityBytes, theirSigBytes) -> - val confidentialIdentity: PartyAndCertificate = confidentialIdentityBytes.bytes.deserialize() - validateAndRegisterIdentity(serviceHub.identityService, otherParty, confidentialIdentity, theirSigBytes) - } - identities[ourIdentity] = legalIdentityAnonymous.party.anonymise() - identities[otherParty] = anonymousOtherSide.party.anonymise() + override fun call(): AnonymousResult { + progressTracker.currentStep = GENERATING_IDENTITY + val ourAnonymousIdentity = serviceHub.keyManagementService.freshKeyAndCert(ourIdentityAndCert, false) + val data = buildDataToSign(ourAnonymousIdentity) + progressTracker.currentStep = SIGNING_IDENTITY + val signature = serviceHub.keyManagementService.sign(data, ourAnonymousIdentity.owningKey).withoutKey() + val ourIdentWithSig = IdentityWithSignature(ourAnonymousIdentity, signature) + progressTracker.currentStep = AWAITING_IDENTITY + val theirAnonymousIdentity = otherSideSession.sendAndReceive(ourIdentWithSig).unwrap { theirIdentWithSig -> + progressTracker.currentStep = VERIFYING_IDENTITY + validateAndRegisterIdentity(serviceHub, otherSideSession.counterparty, theirIdentWithSig.identity, theirIdentWithSig.signature) } - return identities + return AnonymousResult(ourAnonymousIdentity.party.anonymise(), theirAnonymousIdentity.party.anonymise()) } + /** + * Result class containing the caller's anonymous identity ([ourIdentity]) and the counterparty's ([theirIdentity]). + */ @CordaSerializable - data class IdentityWithSignature(val identity: SerializedBytes, val signature: DigitalSignature) + data class AnonymousResult(val ourIdentity: AnonymousParty, val theirIdentity: AnonymousParty) + + @CordaSerializable + private data class IdentityWithSignature(val identity: PartyAndCertificate, val signature: DigitalSignature) + + /** + * Data class used only in the context of asserting that the owner of the private key for the listed key wants to use it + * to represent the named entity. This is paired with an X.509 certificate (which asserts the signing identity says + * the key represents the named entity) and protects against a malicious party incorrectly claiming others' + * keys. + */ + @CordaSerializable + private data class CertificateOwnershipAssertion(val name: CordaX500Name, val owningKey: PublicKey) } -/** - * Data class used only in the context of asserting that the owner of the private key for the listed key wants to use it - * to represent the named entity. This is paired with an X.509 certificate (which asserts the signing identity says - * the key represents the named entity) and protects against a malicious party incorrectly claiming others' - * keys. - */ -@CordaSerializable -data class CertificateOwnershipAssertion(val x500Name: CordaX500Name, - val publicKey: PublicKey) - -open class SwapIdentitiesException @JvmOverloads constructor(message: String, cause: Throwable? = null) - : FlowException(message, cause) \ No newline at end of file +open class SwapIdentitiesException @JvmOverloads constructor(message: String, cause: Throwable? = null) : FlowException(message, cause) diff --git a/confidential-identities/src/main/kotlin/net/corda/confidential/SwapIdentitiesHandler.kt b/confidential-identities/src/main/kotlin/net/corda/confidential/SwapIdentitiesHandler.kt deleted file mode 100644 index a5a17ccdf8..0000000000 --- a/confidential-identities/src/main/kotlin/net/corda/confidential/SwapIdentitiesHandler.kt +++ /dev/null @@ -1,36 +0,0 @@ -package net.corda.confidential - -import co.paralleluniverse.fibers.Suspendable -import net.corda.core.flows.FlowLogic -import net.corda.core.flows.FlowSession -import net.corda.core.identity.PartyAndCertificate -import net.corda.core.serialization.SerializedBytes -import net.corda.core.serialization.deserialize -import net.corda.core.serialization.serialize -import net.corda.core.utilities.ProgressTracker -import net.corda.core.utilities.unwrap - -class SwapIdentitiesHandler(val otherSideSession: FlowSession, val revocationEnabled: Boolean) : FlowLogic() { - constructor(otherSideSession: FlowSession) : this(otherSideSession, false) - - companion object { - object SENDING_KEY : ProgressTracker.Step("Sending key") - } - - override val progressTracker: ProgressTracker = ProgressTracker(SENDING_KEY) - - @Suspendable - override fun call() { - val revocationEnabled = false - progressTracker.currentStep = SENDING_KEY - val ourConfidentialIdentity = serviceHub.keyManagementService.freshKeyAndCert(ourIdentityAndCert, revocationEnabled) - val serializedIdentity = SerializedBytes(ourConfidentialIdentity.serialize().bytes) - val data = SwapIdentitiesFlow.buildDataToSign(ourConfidentialIdentity) - val ourSig = serviceHub.keyManagementService.sign(data, ourConfidentialIdentity.owningKey) - otherSideSession.sendAndReceive(SwapIdentitiesFlow.IdentityWithSignature(serializedIdentity, ourSig.withoutKey())) - .unwrap { (theirConfidentialIdentityBytes, theirSigBytes) -> - val theirConfidentialIdentity = theirConfidentialIdentityBytes.deserialize() - SwapIdentitiesFlow.validateAndRegisterIdentity(serviceHub.identityService, otherSideSession.counterparty, theirConfidentialIdentity, theirSigBytes) - } - } -} \ No newline at end of file diff --git a/confidential-identities/src/test/kotlin/net/corda/confidential/SwapIdentitiesFlowTests.kt b/confidential-identities/src/test/kotlin/net/corda/confidential/SwapIdentitiesFlowTests.kt index 7023242a07..a4ccae45a9 100644 --- a/confidential-identities/src/test/kotlin/net/corda/confidential/SwapIdentitiesFlowTests.kt +++ b/confidential-identities/src/test/kotlin/net/corda/confidential/SwapIdentitiesFlowTests.kt @@ -1,26 +1,40 @@ package net.corda.confidential +import co.paralleluniverse.fibers.Suspendable import com.natpryce.hamkrest.MatchResult import com.natpryce.hamkrest.Matcher +import com.natpryce.hamkrest.assertion.assert import com.natpryce.hamkrest.equalTo -import net.corda.core.identity.* +import net.corda.core.crypto.DigitalSignature +import net.corda.core.flows.FlowLogic +import net.corda.core.flows.FlowSession +import net.corda.core.flows.InitiatedBy +import net.corda.core.flows.InitiatingFlow +import net.corda.core.identity.AbstractParty +import net.corda.core.identity.AnonymousParty +import net.corda.core.identity.Party +import net.corda.core.identity.PartyAndCertificate +import net.corda.core.internal.packageName import net.corda.testing.core.* import net.corda.testing.internal.matchers.allOf import net.corda.testing.internal.matchers.flow.willReturn -import net.corda.testing.node.internal.InternalMockNetwork -import net.corda.testing.node.internal.startFlow -import org.junit.Test -import kotlin.test.* -import com.natpryce.hamkrest.assertion.assert -import net.corda.core.crypto.DigitalSignature import net.corda.testing.internal.matchers.hasOnlyEntries +import net.corda.testing.node.internal.InternalMockNetwork import net.corda.testing.node.internal.TestStartedNode +import net.corda.testing.node.internal.cordappsForPackages +import net.corda.testing.node.internal.startFlow import org.junit.AfterClass +import org.junit.Test import java.security.PublicKey +import kotlin.test.assertFailsWith class SwapIdentitiesFlowTests { companion object { - private val mockNet = InternalMockNetwork(networkSendManuallyPumped = false, threadPerNode = true) + private val mockNet = InternalMockNetwork( + networkSendManuallyPumped = false, + threadPerNode = true, + cordappsForAllNodes = cordappsForPackages(this::class.packageName) + ) @AfterClass @JvmStatic @@ -36,7 +50,7 @@ class SwapIdentitiesFlowTests { @Test fun `issue key`() { assert.that( - aliceNode.services.startFlow(SwapIdentitiesFlow(bob)), + aliceNode.services.startFlow(SwapIdentitiesInitiator(bob)), willReturn( hasOnlyEntries( alice to allOf( @@ -102,21 +116,20 @@ class SwapIdentitiesFlowTests { services.keyManagementService.freshKeyAndCert(services.myInfo.singleIdentityAndCert(), false) } - private fun TestStartedNode.signSwapIdentitiesFlowData(party: PartyAndCertificate, owningKey: PublicKey) = - services.keyManagementService.sign( - SwapIdentitiesFlow.buildDataToSign(party), - owningKey) + private fun TestStartedNode.signSwapIdentitiesFlowData(party: PartyAndCertificate, owningKey: PublicKey): DigitalSignature.WithKey { + return services.keyManagementService.sign(SwapIdentitiesFlow.buildDataToSign(party), owningKey) + } - private fun TestStartedNode.validateSwapIdentitiesFlow( - party: Party, - counterparty: PartyAndCertificate, - signature: DigitalSignature.WithKey) = - SwapIdentitiesFlow.validateAndRegisterIdentity( - services.identityService, - party, - counterparty, - signature.withoutKey() - ) + private fun TestStartedNode.validateSwapIdentitiesFlow(party: Party, + counterparty: PartyAndCertificate, + signature: DigitalSignature.WithKey): PartyAndCertificate { + return SwapIdentitiesFlow.validateAndRegisterIdentity( + services, + party, + counterparty, + signature.withoutKey() + ) + } //endregion //region Matchers @@ -141,21 +154,36 @@ class SwapIdentitiesFlowTests { override val description = "has an owning key which is ${sayNotIf(negated)}held by ${node.info.singleIdentity().name}" - override fun invoke(actual: AnonymousParty) = - if (negated != actual.owningKey in node.services.keyManagementService.keys) { - MatchResult.Match - } else { - MatchResult.Mismatch(""" - had an owning key which was ${sayNotIf(!negated)}held by ${node.info.singleIdentity().name} - """.trimIndent()) - } - - override fun not(): Matcher { - return copy(negated=!negated) + override fun invoke(actual: AnonymousParty): MatchResult { + return if (negated != actual.owningKey in node.services.keyManagementService.keys) { + MatchResult.Match + } else { + MatchResult.Mismatch(""" + had an owning key which was ${sayNotIf(!negated)}held by ${node.info.singleIdentity().name} + """.trimIndent()) + } } + + override fun not(): Matcher = copy(negated=!negated) } private fun TestStartedNode.holdsOwningKey() = HoldsOwningKeyMatcher(this) //endregion - +} + +@InitiatingFlow +private class SwapIdentitiesInitiator(private val otherSide: Party) : FlowLogic>() { + @Suspendable + override fun call(): Map { + val (anonymousUs, anonymousThem) = subFlow(SwapIdentitiesFlow(initiateFlow(otherSide))) + return mapOf(ourIdentity to anonymousUs, otherSide to anonymousThem) + } +} + +@InitiatedBy(SwapIdentitiesInitiator::class) +private class SwapIdentitiesResponder(private val otherSide: FlowSession) : FlowLogic() { + @Suspendable + override fun call() { + subFlow(SwapIdentitiesFlow(otherSide)) + } } diff --git a/core/src/main/kotlin/net/corda/core/contracts/TransactionVerificationException.kt b/core/src/main/kotlin/net/corda/core/contracts/TransactionVerificationException.kt index 4df42e63c9..3b57766508 100644 --- a/core/src/main/kotlin/net/corda/core/contracts/TransactionVerificationException.kt +++ b/core/src/main/kotlin/net/corda/core/contracts/TransactionVerificationException.kt @@ -201,4 +201,11 @@ abstract class TransactionVerificationException(val txId: SecureHash, message: S """The Contract attachment JAR: $attachmentHash containing the contract: $contractClass is not signed by the owner specified in the network parameters. Please check the source of this attachment and if it is malicious contact your zone operator to report this incident. For details see: https://docs.corda.net/network-map.html#network-parameters""".trimIndent(), null) + + /** + * Thrown when multiple attachments provide the same file when building the AttachmentsClassloader for a transaction. + */ + @CordaSerializable + @KeepForDJVM + class OverlappingAttachmentsException(path: String) : Exception("Multiple attachments define a file at path `$path`.") } diff --git a/core/src/main/kotlin/net/corda/core/crypto/DigitalSignature.kt b/core/src/main/kotlin/net/corda/core/crypto/DigitalSignature.kt index 24e4ce1587..da48067fb1 100644 --- a/core/src/main/kotlin/net/corda/core/crypto/DigitalSignature.kt +++ b/core/src/main/kotlin/net/corda/core/crypto/DigitalSignature.kt @@ -7,9 +7,6 @@ import java.security.InvalidKeyException import java.security.PublicKey import java.security.SignatureException -// TODO: Is there a use-case for bare DigitalSignature, or is everything a DigitalSignature.WithKey? If there's no -// actual use-case, we should merge the with key version into the parent class. In that case CompositeSignatureWithKeys -// should be renamed to match. /** A wrapper around a digital signature. */ @CordaSerializable @KeepForDJVM diff --git a/core/src/main/kotlin/net/corda/core/crypto/SignedData.kt b/core/src/main/kotlin/net/corda/core/crypto/SignedData.kt index 9a914405e6..c33ac597fd 100644 --- a/core/src/main/kotlin/net/corda/core/crypto/SignedData.kt +++ b/core/src/main/kotlin/net/corda/core/crypto/SignedData.kt @@ -25,7 +25,7 @@ open class SignedData(val raw: SerializedBytes, val sig: DigitalSign */ @Throws(SignatureException::class) fun verified(): T { - sig.by.verify(raw.bytes, sig) + sig.verify(raw) val data: T = uncheckedCast(raw.deserialize()) verifyData(data) return data diff --git a/core/src/main/kotlin/net/corda/core/serialization/internal/AttachmentsClassLoader.kt b/core/src/main/kotlin/net/corda/core/serialization/internal/AttachmentsClassLoader.kt index 1a8d25437c..39a1847bf5 100644 --- a/core/src/main/kotlin/net/corda/core/serialization/internal/AttachmentsClassLoader.kt +++ b/core/src/main/kotlin/net/corda/core/serialization/internal/AttachmentsClassLoader.kt @@ -2,17 +2,18 @@ package net.corda.core.serialization.internal import net.corda.core.contracts.Attachment import net.corda.core.contracts.ContractAttachment +import net.corda.core.contracts.TransactionVerificationException import net.corda.core.crypto.SecureHash import net.corda.core.internal.VisibleForTesting +import net.corda.core.internal.createSimpleCache import net.corda.core.internal.isUploaderTrusted -import net.corda.core.serialization.CordaSerializable +import net.corda.core.internal.toSynchronised import net.corda.core.serialization.SerializationFactory import net.corda.core.serialization.internal.AttachmentURLStreamHandlerFactory.toUrl -import net.corda.core.internal.createSimpleCache -import net.corda.core.internal.toSynchronised import java.io.IOException import java.io.InputStream import java.net.* +import java.util.jar.Manifest /** * A custom ClassLoader that knows how to load classes from a set of attachments. The attachments themselves only @@ -30,33 +31,24 @@ class AttachmentsClassLoader(attachments: List, parent: ClassLoader URL.setURLStreamHandlerFactory(AttachmentURLStreamHandlerFactory) } - private const val `META-INF` = "meta-inf" - private val excludeFromNoOverlapCheck = setOf( - "manifest.mf", - "license", - "license.txt", - "notice", - "notice.txt", - "index.list" - ) + // Jolokia and Json-simple are dependencies that were bundled by mistake within contract jars. + // In the AttachmentsClassLoader we just ignore any class in those 2 packages. + private val ignoreDirectories = listOf("org/jolokia/", "org/json/simple/") + private val ignorePackages = ignoreDirectories.map { it.replace("/", ".") } - private fun shouldCheckForNoOverlap(path: String): Boolean { - if (!path.startsWith(`META-INF`)) return true - val p = path.substring(`META-INF`.length + 1) - if (p in excludeFromNoOverlapCheck) return false - if (p.endsWith(".sf") || p.endsWith(".dsa")) return false - return true - } - - @CordaSerializable - class OverlappingAttachments(val path: String) : Exception() { - override fun toString() = "Multiple attachments define a file at path $path" + private fun shouldCheckForNoOverlap(path: String, targetPlatformVersion: Int) = when { + path.endsWith("/") -> false // Directories (packages) can overlap. + targetPlatformVersion < 4 && ignoreDirectories.any { path.startsWith(it) } -> false // Ignore jolokia and json-simple for old cordapps. + path.endsWith(".class") -> true // All class files need to be unique. + !path.startsWith("meta-inf") -> true // All files outside of Meta-inf need to be unique. + else -> false // This allows overlaps over any non-class files in "Meta-inf". } private fun requireNoDuplicates(attachments: List) { val classLoaderEntries = mutableSetOf() for (attachment in attachments) { attachment.openAsJAR().use { jar -> + val targetPlatformVersion = jar.manifest?.targetPlatformVersion ?: 1 while (true) { val entry = jar.nextJarEntry ?: break @@ -67,16 +59,24 @@ class AttachmentsClassLoader(attachments: List, parent: ClassLoader // filesystem tries to be case insensitive. This may break developers who attempt to use ProGuard. // // Also convert to Unix path separators as all resource/class lookups will expect this. - // If 2 entries have the same CRC, it means the same file is present in both attachments, so that is ok. TODO - Mike, wdyt? val path = entry.name.toLowerCase().replace('\\', '/') - if (shouldCheckForNoOverlap(path)) { - if (path in classLoaderEntries) throw OverlappingAttachments(path) + // TODO - If 2 entries are identical, it means the same file is present in both attachments, so that should be ok. + if (shouldCheckForNoOverlap(path, targetPlatformVersion)) { + if (path in classLoaderEntries) throw TransactionVerificationException.OverlappingAttachmentsException(path) classLoaderEntries.add(path) } } } } } + + // This was reused from: https://github.com/corda/corda/pull/4240. + // TODO - Once that is merged it should be extracted to a utility. + private val Manifest.targetPlatformVersion: Int + get() { + val minPlatformVersion = mainAttributes.getValue("Min-Platform-Version")?.toInt() ?: 1 + return mainAttributes.getValue("Target-Platform-Version")?.toInt() ?: minPlatformVersion + } } init { @@ -86,6 +86,17 @@ class AttachmentsClassLoader(attachments: List, parent: ClassLoader requireNoDuplicates(attachments) } + + /** + * Required to prevent classes that were excluded from the no-overlap check from being loaded by contract code. + * As it can lead to non-determinism. + */ + override fun loadClass(name: String?): Class<*> { + if (ignorePackages.any { name!!.startsWith(it) }) { + throw ClassNotFoundException(name) + } + return super.loadClass(name) + } } /** diff --git a/core/src/test/kotlin/net/corda/core/transactions/AttachmentsClassLoaderTests.kt b/core/src/test/kotlin/net/corda/core/transactions/AttachmentsClassLoaderTests.kt index 0fe3727df8..8f79ff44a2 100644 --- a/core/src/test/kotlin/net/corda/core/transactions/AttachmentsClassLoaderTests.kt +++ b/core/src/test/kotlin/net/corda/core/transactions/AttachmentsClassLoaderTests.kt @@ -2,6 +2,7 @@ package net.corda.core.transactions import net.corda.core.contracts.Attachment import net.corda.core.contracts.Contract +import net.corda.core.contracts.TransactionVerificationException import net.corda.core.internal.declaredField import net.corda.core.serialization.internal.AttachmentsClassLoader import net.corda.testing.internal.fakeAttachment @@ -65,7 +66,7 @@ class AttachmentsClassLoaderTests { val att1 = storage.importAttachment(fakeAttachment("file1.txt", "some data").inputStream(), "app", "file1.jar") val att2 = storage.importAttachment(fakeAttachment("file1.txt", "some other data").inputStream(), "app", "file2.jar") - assertFailsWith(AttachmentsClassLoader.Companion.OverlappingAttachments::class) { + assertFailsWith(TransactionVerificationException.OverlappingAttachmentsException::class) { AttachmentsClassLoader(arrayOf(att1, att2).map { storage.openAttachment(it)!! }) } } diff --git a/docs/source/changelog.rst b/docs/source/changelog.rst index ada5a497c0..8a6fba6be6 100644 --- a/docs/source/changelog.rst +++ b/docs/source/changelog.rst @@ -7,6 +7,19 @@ release, see :doc:`upgrade-notes`. Unreleased ---------- +* ``SwapIdentitiesFlow``, from the experimental confidential-identities module, is now an inlined flow. Instead of passing in a ``Party`` with + whom to exchange the anonymous identity, a ``FlowSession`` to that party is required instead. The flow running on the other side must + also call ``SwapIdentitiesFlow``. This change was required as the previous API allowed any counterparty to generate anonoymous identities + with a node at will with no checks. + + The result type has changed to a simple wrapper class, instead of a Map, to make extracting the identities easier. Also, the wire protocol + of the flow has slightly changed. + + .. note:: V3 and V4 of confidential-identities are not compatible and confidential-identities V3 will not work with a V4 Corda node. CorDapps + in such scenarios using confidential-identities must be updated. + + The ``confidential-identities`` dependency in your CorDapp must now be ``compile`` and not ``cordaCompile``. + * Marked the ``Attachment`` interface as ``@DoNotImplement`` because it is not meant to be extended by CorDapp developers. If you have already done so, please get in contact on the usual communication channels. @@ -33,10 +46,10 @@ Unreleased un-acknowledged in the message broker. This enables the recovery scenerio whereby any missing CorDapp can be installed and retried on node restart. As a consequence the initiating flow will be blocked until the receiving node has resolved the issue. -* ``FinalityFlow`` is now an inlined flow and no longer requires a handler flow in the counterparty. This is to fix the - security problem with the handler flow as it accepts any transaction it receives without any checks. Existing CorDapp - binaries relying on this old behaviour will continue to function as previously. However, it is strongly recommended that - CorDapps switch to this new API. See :doc:`upgrade-notes` for further details. +* ``FinalityFlow`` is now an inlined flow and requires ``FlowSession`` s to each party intended to receive the transaction. This is to fix the + security problem with the old API that required every node to accept any transaction it received without any checks. Existing CorDapp + binaries relying on this old behaviour will continue to function as previously. However, it is strongly recommended that CorDapps switch to + this new API. See :doc:`upgrade-notes` for further details. * Introduced new optional network bootstrapper command line option (--minimum-platform-version) to set as a network parameter diff --git a/finance/build.gradle b/finance/build.gradle index e3ceca1403..9e4a433754 100644 --- a/finance/build.gradle +++ b/finance/build.gradle @@ -27,10 +27,7 @@ dependencies { // Note the :finance module is a CorDapp in its own right // and CorDapps using :finance features should use 'cordapp' not 'compile' linkage. cordaCompile project(':core') - cordaCompile project(':confidential-identities') - - // TODO Remove this once we have app configs - compile "com.typesafe:config:$typesafe_config_version" + compile project(':confidential-identities') // For JSON compile "com.fasterxml.jackson.core:jackson-databind:${jackson_version}" diff --git a/finance/src/main/kotlin/net/corda/finance/flows/CashPaymentFlow.kt b/finance/src/main/kotlin/net/corda/finance/flows/CashPaymentFlow.kt index 9eca7ca3df..bed2aaadd1 100644 --- a/finance/src/main/kotlin/net/corda/finance/flows/CashPaymentFlow.kt +++ b/finance/src/main/kotlin/net/corda/finance/flows/CashPaymentFlow.kt @@ -5,11 +5,11 @@ import net.corda.confidential.SwapIdentitiesFlow import net.corda.core.contracts.Amount import net.corda.core.contracts.InsufficientBalanceException import net.corda.core.flows.* -import net.corda.core.identity.AnonymousParty import net.corda.core.identity.Party import net.corda.core.serialization.CordaSerializable import net.corda.core.transactions.TransactionBuilder import net.corda.core.utilities.ProgressTracker +import net.corda.core.utilities.unwrap import net.corda.finance.contracts.asset.Cash import net.corda.finance.flows.AbstractCashFlow.Companion.FINALISING_TX import net.corda.finance.flows.AbstractCashFlow.Companion.GENERATING_ID @@ -49,23 +49,26 @@ open class CashPaymentFlow( @Suspendable override fun call(): AbstractCashFlow.Result { progressTracker.currentStep = GENERATING_ID - val txIdentities = if (anonymous) { - subFlow(SwapIdentitiesFlow(recipient)) + val recipientSession = initiateFlow(recipient) + recipientSession.send(anonymous) + val anonymousRecipient = if (anonymous) { + subFlow(SwapIdentitiesFlow(recipientSession)).theirIdentity } else { - emptyMap() + recipient } - val anonymousRecipient = txIdentities[recipient] ?: recipient progressTracker.currentStep = GENERATING_TX val builder = TransactionBuilder(notary = notary ?: serviceHub.networkMapCache.notaryIdentities.first()) logger.info("Generating spend for: ${builder.lockId}") // TODO: Have some way of restricting this to states the caller controls val (spendTX, keysForSigning) = try { - Cash.generateSpend(serviceHub, + Cash.generateSpend( + serviceHub, builder, amount, ourIdentityAndCert, anonymousRecipient, - issuerConstraint) + issuerConstraint + ) } catch (e: InsufficientBalanceException) { throw CashException("Insufficient cash for spend: ${e.message}", e) } @@ -76,8 +79,8 @@ open class CashPaymentFlow( progressTracker.currentStep = FINALISING_TX logger.info("Finalising transaction for: ${tx.id}") - val sessions = if (serviceHub.myInfo.isLegalIdentity(recipient)) emptyList() else listOf(initiateFlow(recipient)) - val notarised = finaliseTx(tx, sessions, "Unable to notarise spend") + val sessionsForFinality = if (serviceHub.myInfo.isLegalIdentity(recipient)) emptyList() else listOf(recipientSession) + val notarised = finaliseTx(tx, sessionsForFinality, "Unable to notarise spend") logger.info("Finalised transaction for: ${notarised.id}") return Result(notarised, anonymousRecipient) } @@ -91,9 +94,16 @@ open class CashPaymentFlow( } @InitiatedBy(CashPaymentFlow::class) -class CashPaymentResponderFlow(private val otherSide: FlowSession) : FlowLogic() { +class CashPaymentReceiverFlow(private val otherSide: FlowSession) : FlowLogic() { @Suspendable override fun call() { - subFlow(ReceiveFinalityFlow(otherSide)) + val anonymous = otherSide.receive().unwrap { it } + if (anonymous) { + subFlow(SwapIdentitiesFlow(otherSide)) + } + // Not ideal that we have to do this check, but we must as FinalityFlow does not send locally + if (!serviceHub.myInfo.isLegalIdentity(otherSide.counterparty)) { + subFlow(ReceiveFinalityFlow(otherSide)) + } } } diff --git a/finance/src/main/kotlin/net/corda/finance/flows/TwoPartyDealFlow.kt b/finance/src/main/kotlin/net/corda/finance/flows/TwoPartyDealFlow.kt index 413937aaa9..3bfff54f01 100644 --- a/finance/src/main/kotlin/net/corda/finance/flows/TwoPartyDealFlow.kt +++ b/finance/src/main/kotlin/net/corda/finance/flows/TwoPartyDealFlow.kt @@ -55,9 +55,7 @@ object TwoPartyDealFlow { @Suspendable override fun call(): SignedTransaction { progressTracker.currentStep = GENERATING_ID - val txIdentities = subFlow(SwapIdentitiesFlow(otherSideSession.counterparty)) - val anonymousMe = txIdentities[ourIdentity] ?: ourIdentity.anonymise() - val anonymousCounterparty = txIdentities[otherSideSession.counterparty] ?: otherSideSession.counterparty.anonymise() + val (anonymousMe, anonymousCounterparty) = subFlow(SwapIdentitiesFlow(otherSideSession)) // DOCEND 2 progressTracker.currentStep = SENDING_PROPOSAL // Make the first message we'll send to kick off the flow. @@ -131,6 +129,7 @@ object TwoPartyDealFlow { @Suspendable private fun receiveAndValidateHandshake(): Handshake { + subFlow(SwapIdentitiesFlow(otherSideSession)) progressTracker.currentStep = RECEIVING // Wait for a trade request to come in on our pre-provided session ID. val handshake = otherSideSession.receive>() diff --git a/node/build.gradle b/node/build.gradle index afd01a45c7..e602e11b49 100644 --- a/node/build.gradle +++ b/node/build.gradle @@ -68,7 +68,6 @@ processTestResources { dependencies { compile project(':node-api') - compile project(":confidential-identities") compile project(':client:rpc') compile project(':tools:shell') compile project(':tools:cliutils') diff --git a/node/src/integration-test/kotlin/net/corda/node/services/distributed/DistributedServiceTests.kt b/node/src/integration-test/kotlin/net/corda/node/services/distributed/DistributedServiceTests.kt index ec3b92f392..6454f4dde7 100644 --- a/node/src/integration-test/kotlin/net/corda/node/services/distributed/DistributedServiceTests.kt +++ b/node/src/integration-test/kotlin/net/corda/node/services/distributed/DistributedServiceTests.kt @@ -52,13 +52,12 @@ class DistributedServiceTests : IntegrationTest() { invokeRpc(CordaRPCOps::stateMachinesFeed)) ) driver(DriverParameters( - extraCordappPackagesToScan = listOf("net.corda.finance.contracts", "net.corda.finance.schemas", "net.corda.notary.raft"), - notarySpecs = listOf( - NotarySpec( - DUMMY_NOTARY_NAME, - rpcUsers = listOf(testUser), - cluster = DummyClusterSpec(clusterSize = 3, compositeServiceIdentity = compositeIdentity)) - ) + extraCordappPackagesToScan = listOf("net.corda.finance", "net.corda.notary.raft"), + notarySpecs = listOf(NotarySpec( + DUMMY_NOTARY_NAME, + rpcUsers = listOf(testUser), + cluster = DummyClusterSpec(clusterSize = 3, compositeServiceIdentity = compositeIdentity) + )) )) { alice = startNode(providedName = ALICE_NAME, rpcUsers = listOf(testUser)).getOrThrow() raftNotaryIdentity = defaultNotaryIdentity diff --git a/node/src/main/kotlin/net/corda/node/internal/AbstractNode.kt b/node/src/main/kotlin/net/corda/node/internal/AbstractNode.kt index e613b6dfb4..5e9f2d4a9a 100644 --- a/node/src/main/kotlin/net/corda/node/internal/AbstractNode.kt +++ b/node/src/main/kotlin/net/corda/node/internal/AbstractNode.kt @@ -4,8 +4,6 @@ import com.codahale.metrics.MetricRegistry import com.google.common.collect.MutableClassToInstanceMap import com.google.common.util.concurrent.MoreExecutors import com.zaxxer.hikari.pool.HikariPool -import net.corda.confidential.SwapIdentitiesFlow -import net.corda.confidential.SwapIdentitiesHandler import net.corda.core.CordaException import net.corda.core.concurrent.CordaFuture import net.corda.core.context.InvocationContext @@ -672,8 +670,6 @@ abstract class AbstractNode(val configuration: NodeConfiguration, installFinalityHandler() flowManager.registerInitiatedCoreFlowFactory(NotaryChangeFlow::class, NotaryChangeHandler::class, ::NotaryChangeHandler) flowManager.registerInitiatedCoreFlowFactory(ContractUpgradeFlow.Initiate::class, NotaryChangeHandler::class, ::ContractUpgradeHandler) - // TODO Make this an inlined flow (and remove this flow mapping!), which should be possible now that FinalityFlow is also inlined - flowManager.registerInitiatedCoreFlowFactory(SwapIdentitiesFlow::class, SwapIdentitiesHandler::class, ::SwapIdentitiesHandler) } // The FinalityHandler is insecure as it blindly accepts any and all transactions into the node's local vault without doing any checks. diff --git a/node/src/test/kotlin/net/corda/node/CordaRPCOpsImplTest.kt b/node/src/test/kotlin/net/corda/node/CordaRPCOpsImplTest.kt index 42836c56e7..f338dc60b7 100644 --- a/node/src/test/kotlin/net/corda/node/CordaRPCOpsImplTest.kt +++ b/node/src/test/kotlin/net/corda/node/CordaRPCOpsImplTest.kt @@ -14,7 +14,6 @@ import net.corda.core.flows.FlowLogic import net.corda.core.flows.StartableByRPC import net.corda.core.flows.StateMachineRunId import net.corda.core.identity.Party -import net.corda.core.internal.extractFile import net.corda.core.internal.uncheckedCast import net.corda.core.messaging.* import net.corda.core.node.services.Vault @@ -31,6 +30,7 @@ import net.corda.finance.GBP import net.corda.finance.contracts.asset.Cash import net.corda.finance.flows.CashIssueFlow import net.corda.finance.flows.CashPaymentFlow +import net.corda.node.internal.security.AuthorizingSubject import net.corda.node.internal.security.RPCSecurityManagerImpl import net.corda.node.services.Permissions.Companion.invokeRpc import net.corda.node.services.Permissions.Companion.startFlow @@ -56,20 +56,22 @@ import org.junit.Before import org.junit.Test import rx.Observable import java.io.ByteArrayOutputStream -import java.util.jar.JarInputStream import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertNull import kotlin.test.assertTrue // Mock an AuthorizingSubject instance sticking to a fixed set of permissions -private fun buildSubject(principal: String, permissionStrings: Set) = - RPCSecurityManagerImpl.fromUserList( - id = AuthServiceId("TEST"), - users = listOf(User(username = principal, - password = "", - permissions = permissionStrings))) - .buildSubject(principal) +private fun buildSubject(principal: String, permissionStrings: Set): AuthorizingSubject { + return RPCSecurityManagerImpl.fromUserList( + id = AuthServiceId("TEST"), + users = listOf(User( + username = principal, + password = "", + permissions = permissionStrings + )) + ).buildSubject(principal) +} class CordaRPCOpsImplTest { private companion object { @@ -87,7 +89,7 @@ class CordaRPCOpsImplTest { @Before fun setup() { - mockNet = InternalMockNetwork(cordappsForAllNodes = cordappsForPackages("net.corda.finance.contracts.asset", "net.corda.finance.schemas")) + mockNet = InternalMockNetwork(cordappsForAllNodes = cordappsForPackages("net.corda.finance")) aliceNode = mockNet.createNode(InternalMockNodeParameters(legalName = ALICE_NAME)) rpc = aliceNode.rpcOps CURRENT_RPC_CONTEXT.set(RpcAuthContext(InvocationContext.rpc(testActor()), buildSubject("TEST_USER", emptySet()))) @@ -163,11 +165,13 @@ class CordaRPCOpsImplTest { @Test fun `issue and move`() { @Suppress("DEPRECATION") - withPermissions(invokeRpc(CordaRPCOps::stateMachinesFeed), + withPermissions( + invokeRpc(CordaRPCOps::stateMachinesFeed), invokeRpc(CordaRPCOps::internalVerifiedTransactionsFeed), invokeRpc("vaultTrackBy"), startFlow(), - startFlow()) { + startFlow() + ) { aliceNode.database.transaction { stateMachineUpdates = rpc.stateMachinesFeed().updates transactions = rpc.internalVerifiedTransactionsFeed().updates @@ -183,7 +187,8 @@ class CordaRPCOpsImplTest { mockNet.runNetwork() var issueSmId: StateMachineRunId? = null - var moveSmId: StateMachineRunId? = null + var paymentSmId: StateMachineRunId? = null + var paymentRecSmId: StateMachineRunId? = null stateMachineUpdates.expectEvents { sequence( // ISSUE @@ -191,14 +196,20 @@ class CordaRPCOpsImplTest { issueSmId = add.id }, expect { remove: StateMachineUpdate.Removed -> - require(remove.id == issueSmId) + assertThat(remove.id).isEqualTo(issueSmId) }, - // MOVE + // PAYMENT expect { add: StateMachineUpdate.Added -> - moveSmId = add.id + paymentSmId = add.id + }, + expect { add: StateMachineUpdate.Added -> + paymentRecSmId = add.id }, expect { remove: StateMachineUpdate.Removed -> - require(remove.id == moveSmId) + assertThat(remove.id).isEqualTo(paymentRecSmId) + }, + expect { remove: StateMachineUpdate.Removed -> + assertThat(remove.id).isEqualTo(paymentSmId) } ) } diff --git a/serialization/src/main/kotlin/net/corda/serialization/internal/model/LocalTypeInformation.kt b/serialization/src/main/kotlin/net/corda/serialization/internal/model/LocalTypeInformation.kt index c7473623d4..9c815fe84f 100644 --- a/serialization/src/main/kotlin/net/corda/serialization/internal/model/LocalTypeInformation.kt +++ b/serialization/src/main/kotlin/net/corda/serialization/internal/model/LocalTypeInformation.kt @@ -54,11 +54,12 @@ sealed class LocalTypeInformation { * types beginning the with provided [Type] and construct a complete set of [LocalTypeInformation] for that type. * * @param type The [Type] to obtain [LocalTypeInformation] for. + * @param typeIdentifier The [TypeIdentifier] for the [Type] to obtain [LocalTypeInformation] for. * @param lookup The [LocalTypeLookup] to use to find previously-constructed [LocalTypeInformation]. */ - fun forType(type: Type, lookup: LocalTypeLookup): LocalTypeInformation { + fun forType(type: Type, typeIdentifier: TypeIdentifier, lookup: LocalTypeLookup): LocalTypeInformation { val builder = LocalTypeInformationBuilder(lookup) - val result = builder.build(type, TypeIdentifier.forGenericType(type)) + val result = builder.build(type, typeIdentifier) // Patch every cyclic reference with a `follow` property pointing to the type information it refers to. builder.cycles.forEach { cycle -> diff --git a/serialization/src/main/kotlin/net/corda/serialization/internal/model/LocalTypeModel.kt b/serialization/src/main/kotlin/net/corda/serialization/internal/model/LocalTypeModel.kt index 9acdacb71e..45bdc8794f 100644 --- a/serialization/src/main/kotlin/net/corda/serialization/internal/model/LocalTypeModel.kt +++ b/serialization/src/main/kotlin/net/corda/serialization/internal/model/LocalTypeModel.kt @@ -41,12 +41,6 @@ interface LocalTypeModel { * @param type The [Type] to get [LocalTypeInformation] for. */ fun inspect(type: Type): LocalTypeInformation - - /** - * Get [LocalTypeInformation] directly from the registry by [TypeIdentifier], returning null if no type information - * is registered for that identifier. - */ - operator fun get(typeIdentifier: TypeIdentifier): LocalTypeInformation? } /** @@ -55,20 +49,51 @@ interface LocalTypeModel { * * @param typeModelConfiguration Configuration controlling the behaviour of the [LocalTypeModel]'s type inspection. */ -class ConfigurableLocalTypeModel(private val typeModelConfiguration: LocalTypeModelConfiguration): LocalTypeModel, LocalTypeLookup { +class ConfigurableLocalTypeModel(private val typeModelConfiguration: LocalTypeModelConfiguration): LocalTypeModel { private val typeInformationCache = DefaultCacheProvider.createCache() - override fun isExcluded(type: Type): Boolean = typeModelConfiguration.isExcluded(type) + /** + * We need to provide the [TypeInformationBuilder] with a temporary local cache, so that it doesn't leak + * [LocalTypeInformation] with unpatched cycles into the global cache where other threads can access them + * before we've patched the cycles up. + */ + private class BuilderLookup( + private val globalCache: MutableMap, + private val typeModelConfiguration: LocalTypeModelConfiguration) : LocalTypeLookup { - override fun inspect(type: Type): LocalTypeInformation = LocalTypeInformation.forType(type, this) + private val localCache: MutableMap = mutableMapOf() - override fun findOrBuild(type: Type, typeIdentifier: TypeIdentifier, builder: (Boolean) -> LocalTypeInformation): LocalTypeInformation = - this[typeIdentifier] ?: builder(typeModelConfiguration.isOpaque(type)).apply { - typeInformationCache.putIfAbsent(typeIdentifier, this) + /** + * Read from the global cache (which contains only cycle-resolved type information), falling through + * to the local cache if the type isn't there yet. + */ + override fun findOrBuild(type: Type, typeIdentifier: TypeIdentifier, builder: (Boolean) -> LocalTypeInformation): LocalTypeInformation = + globalCache[typeIdentifier] ?: + localCache.getOrPut(typeIdentifier) { builder(typeModelConfiguration.isOpaque(type)) } + + override fun isExcluded(type: Type): Boolean = typeModelConfiguration.isExcluded(type) + + /** + * Merge the local cache back into the global cache, once we've finished traversal (and patched all cycles). + */ + fun merge() { + localCache.forEach { (identifier, information) -> + globalCache.putIfAbsent(identifier, information) } + } + } - override operator fun get(typeIdentifier: TypeIdentifier): LocalTypeInformation? = typeInformationCache[typeIdentifier] + override fun inspect(type: Type): LocalTypeInformation { + val typeIdentifier = TypeIdentifier.forGenericType(type) + + return typeInformationCache.getOrPut(typeIdentifier) { + val lookup = BuilderLookup(typeInformationCache, typeModelConfiguration) + val result = LocalTypeInformation.forType(type, typeIdentifier, lookup) + lookup.merge() + result + } + } } /**