Skip to content

Commit

Permalink
CORDA-2114: SwapIdentitiesFlow is now inlined (corda#4260)
Browse files Browse the repository at this point in the history
This is to fix the security issue whereby any counterparty is able to generate anonymous identities with a node at will without checks.
  • Loading branch information
shamsasari authored Nov 26, 2018
1 parent b01f278 commit 3b8a74f
Show file tree
Hide file tree
Showing 12 changed files with 196 additions and 194 deletions.
Original file line number Diff line number Diff line change
@@ -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<LinkedHashMap<Party, AnonymousParty>>() {
constructor(otherParty: Party) : this(otherParty, false, tracker())

class SwapIdentitiesFlow @JvmOverloads constructor(private val otherSideSession: FlowSession,
override val progressTracker: ProgressTracker = tracker()) : FlowLogic<SwapIdentitiesFlow.AnonymousResult>() {
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<Party, AnonymousParty> {
progressTracker.currentStep = AWAITING_KEY
val legalIdentityAnonymous = serviceHub.keyManagementService.freshKeyAndCert(ourIdentityAndCert, revocationEnabled)
val serializedIdentity = SerializedBytes<PartyAndCertificate>(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<Party, AnonymousParty>()
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<IdentityWithSignature>(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<IdentityWithSignature>(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<PartyAndCertificate>, val signature: DigitalSignature)
}
data class AnonymousResult(val ourIdentity: AnonymousParty, val theirIdentity: AnonymousParty)

/**
* 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)
@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)
}

open class SwapIdentitiesException @JvmOverloads constructor(message: String, cause: Throwable? = null)
: FlowException(message, cause)
open class SwapIdentitiesException @JvmOverloads constructor(message: String, cause: Throwable? = null) : FlowException(message, cause)

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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(
Expand Down Expand Up @@ -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.validateSwapIdentitiesFlow(
party: Party,
counterparty: PartyAndCertificate,
signature: DigitalSignature.WithKey) =
SwapIdentitiesFlow.validateAndRegisterIdentity(
services.identityService,
party,
counterparty,
signature.withoutKey()
)
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): PartyAndCertificate {
return SwapIdentitiesFlow.validateAndRegisterIdentity(
services,
party,
counterparty,
signature.withoutKey()
)
}
//endregion

//region Matchers
Expand All @@ -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<AnonymousParty> {
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<AnonymousParty> = copy(negated=!negated)
}

private fun TestStartedNode.holdsOwningKey() = HoldsOwningKeyMatcher(this)
//endregion
}

@InitiatingFlow
private class SwapIdentitiesInitiator(private val otherSide: Party) : FlowLogic<Map<Party, AnonymousParty>>() {
@Suspendable
override fun call(): Map<Party, AnonymousParty> {
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<Unit>() {
@Suspendable
override fun call() {
subFlow(SwapIdentitiesFlow(otherSide))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/kotlin/net/corda/core/crypto/SignedData.kt
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ open class SignedData<T : Any>(val raw: SerializedBytes<T>, val sig: DigitalSign
*/
@Throws(SignatureException::class)
fun verified(): T {
sig.by.verify(raw.bytes, sig)
sig.verify(raw)
val data: T = uncheckedCast(raw.deserialize<Any>())
verifyData(data)
return data
Expand Down
Loading

0 comments on commit 3b8a74f

Please sign in to comment.