From f9ed55b731a9e9a5801e6ad7f6beaa8bb1dc1846 Mon Sep 17 00:00:00 2001 From: Patrick Kuo Date: Thu, 12 Apr 2018 13:05:15 +0100 Subject: [PATCH] ENT-1323 Network map service to check all identities in submitted node info (#499) * ENT-1323 Network map service to check all identities in submitted node info * fixup after rebase * address PR issues, refactored createValidNodeInfo * address PR issues --- .../persistence/PersistentNodeInfoStorage.kt | 23 ++++-- .../PersistentNodeInfoStorageTest.kt | 74 ++++++++++++++++--- .../nodeapi/internal/SignedNodeInfoTest.kt | 18 ++--- .../services/network/NetworkMapClientTest.kt | 4 +- .../testing/internal/TestNodeInfoBuilder.kt | 35 +++++++-- 5 files changed, 118 insertions(+), 36 deletions(-) diff --git a/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/PersistentNodeInfoStorage.kt b/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/PersistentNodeInfoStorage.kt index df4cc7c7cc..e2a229c4bb 100644 --- a/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/PersistentNodeInfoStorage.kt +++ b/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/PersistentNodeInfoStorage.kt @@ -34,10 +34,11 @@ import java.security.cert.CertPath class PersistentNodeInfoStorage(private val database: CordaPersistence) : NodeInfoStorage { override fun putNodeInfo(nodeInfoAndSigned: NodeInfoAndSigned): SecureHash { val (nodeInfo, signedNodeInfo) = nodeInfoAndSigned - val nodeCaCert = nodeInfo.legalIdentitiesAndCerts[0].certPath.x509Certificates.find { CertRole.extract(it) == NODE_CA } - nodeCaCert ?: throw IllegalArgumentException("Missing Node CA") val nodeInfoHash = signedNodeInfo.raw.hash + // Extract identities issued by the intermediate CAs (doorman). + val registeredIdentities = nodeInfo.legalIdentitiesAndCerts.map { it.certPath.x509Certificates.single { CertRole.extract(it) in setOf(CertRole.SERVICE_IDENTITY, NODE_CA) } } + database.transaction { val count = session.createQuery( "select count(*) from ${NodeInfoEntity::class.java.name} where nodeInfoHash = :nodeInfoHash and isCurrent = true", java.lang.Long::class.java) @@ -50,13 +51,21 @@ class PersistentNodeInfoStorage(private val database: CordaPersistence) : NodeIn } // TODO Move these checks out of data access layer - val request = requireNotNull(getSignedRequestByPublicHash(nodeCaCert.publicKey.encoded.sha256())) { - "Node-info not registered with us" - } - request.certificateData?.certificateStatus.let { - require(it == CertificateStatus.VALID) { "Certificate is no longer valid: $it" } + // For each identity known by the doorman, validate against it's CSR. + val requests = registeredIdentities.map { + val request = requireNotNull(getSignedRequestByPublicHash(it.publicKey.hash)) { + "Node-info not registered with us" + } + request.certificateData?.certificateStatus.let { + require(it == CertificateStatus.VALID) { "Certificate is no longer valid: $it" } + } + CertRole.extract(it) to request } + // Ensure only 1 NodeCA identity. + // TODO: make this support multiple node identities. + val (_, request) = requireNotNull(requests.singleOrNull { it.first == CertRole.NODE_CA }) { "Require exactly 1 Node CA identity in the node-info." } + val existingNodeInfos = session.fromQuery( "n where n.certificateSigningRequest = :csr and n.isCurrent = true order by n.publishedAt desc") .setParameter("csr", request) diff --git a/network-management/src/test/kotlin/com/r3/corda/networkmanage/common/persistence/PersistentNodeInfoStorageTest.kt b/network-management/src/test/kotlin/com/r3/corda/networkmanage/common/persistence/PersistentNodeInfoStorageTest.kt index 143b9216c5..0f87e44d76 100644 --- a/network-management/src/test/kotlin/com/r3/corda/networkmanage/common/persistence/PersistentNodeInfoStorageTest.kt +++ b/network-management/src/test/kotlin/com/r3/corda/networkmanage/common/persistence/PersistentNodeInfoStorageTest.kt @@ -13,10 +13,10 @@ package com.r3.corda.networkmanage.common.persistence import com.r3.corda.networkmanage.TestBase import com.r3.corda.networkmanage.common.persistence.entity.NodeInfoEntity import net.corda.core.crypto.Crypto -import net.corda.core.crypto.sha256 import net.corda.core.identity.CordaX500Name import net.corda.core.internal.CertRole import net.corda.core.internal.hash +import net.corda.core.node.NodeInfo import net.corda.core.serialization.serialize import net.corda.core.utilities.days import net.corda.nodeapi.internal.NodeInfoAndSigned @@ -31,6 +31,7 @@ import net.corda.testing.internal.createDevIntermediateCaCertPath import net.corda.testing.internal.signWith import net.corda.testing.node.MockServices import org.assertj.core.api.Assertions.assertThat +import org.assertj.core.api.Assertions.assertThatThrownBy import org.junit.After import org.junit.Before import org.junit.Test @@ -216,17 +217,66 @@ class PersistentNodeInfoStorageTest : TestBase() { val acceptedUpdate = nodeInfoStorage.getAcceptedParametersUpdate(nodeInfoHash2) assertThat(acceptedUpdate?.networkParameters?.hash).isEqualTo(netParamsHash.toString()) } + + @Test + fun `persist node info with multiple node CA identities`() { + val (nodeInfo1, nodeKeyPair1) = createValidNodeInfo("Alice", requestStorage) + val (nodeInfo2, nodeKeyPair2) = createValidNodeInfo("Bob", requestStorage) + + val multiIdentityNodeInfo = nodeInfo1.copy(legalIdentitiesAndCerts = nodeInfo1.legalIdentitiesAndCerts + nodeInfo2.legalIdentitiesAndCerts) + val signedNodeInfo = multiIdentityNodeInfo.signWith(listOf(nodeKeyPair1, nodeKeyPair2)) + + assertThatThrownBy { nodeInfoStorage.putNodeInfo(NodeInfoAndSigned(signedNodeInfo)) } + .isInstanceOf(IllegalArgumentException::class.java) + .hasMessageContaining("Require exactly 1 Node CA identity in the node-info.") + } + + @Test + fun `persist node info with service identity`() { + val (nodeInfo, nodeKeyPairs) = createValidNodeInfo(requestStorage, CertRole.NODE_CA to "Alice", CertRole.SERVICE_IDENTITY to "Alice Notary") + val signedNodeInfo = nodeInfo.signWith(nodeKeyPairs) + nodeInfoStorage.putNodeInfo(NodeInfoAndSigned(signedNodeInfo)) + } + + @Test + fun `persist node info with unregistered service identity`() { + val (nodeInfo1, nodeKeyPair1) = createValidNodeInfo("Alice", requestStorage) + // Create a unregistered cert path with valid intermediate cert. + val (identity, key) = TestNodeInfoBuilder().addServiceIdentity(CordaX500Name("Test", "London", "GB"), Crypto.generateKeyPair()) + + val multiIdentityNodeInfo = nodeInfo1.copy(legalIdentitiesAndCerts = nodeInfo1.legalIdentitiesAndCerts + identity) + val signedNodeInfo = multiIdentityNodeInfo.signWith(listOf(nodeKeyPair1, key)) + + assertThatThrownBy { nodeInfoStorage.putNodeInfo(NodeInfoAndSigned(signedNodeInfo)) } + .isInstanceOf(IllegalArgumentException::class.java) + .hasMessageContaining("Node-info not registered with us") + } } -internal fun createValidSignedNodeInfo(organisation: String, - storage: CertificateSigningRequestStorage): Pair { - val (csr, nodeKeyPair) = createRequest(organisation, certRole = CertRole.NODE_CA) - val requestId = storage.saveRequest(csr) - storage.markRequestTicketCreated(requestId) - storage.approveRequest(requestId, "TestUser") +private fun createValidNodeInfo(organisation: String, storage: CertificateSigningRequestStorage): Pair { + val (nodeInfo, keys) = createValidNodeInfo(storage, CertRole.NODE_CA to organisation) + return Pair(nodeInfo, keys.single()) +} + +private fun createValidNodeInfo(storage: CertificateSigningRequestStorage, vararg identities: Pair): Pair> { val nodeInfoBuilder = TestNodeInfoBuilder() - val (identity, key) = nodeInfoBuilder.addIdentity(CordaX500Name.build(X500Principal(csr.subject.encoded)), nodeKeyPair) - storage.putCertificatePath(requestId, identity.certPath, "Test") - val (_, signedNodeInfo) = nodeInfoBuilder.buildWithSigned(1) - return Pair(NodeInfoAndSigned(signedNodeInfo), key) -} \ No newline at end of file + val keys = identities.map { (certRole, name) -> + val (csr, nodeKeyPair) = createRequest(name, certRole = certRole) + val requestId = storage.saveRequest(csr) + storage.markRequestTicketCreated(requestId) + storage.approveRequest(requestId, "TestUser") + val (identity, key) = when (certRole) { + CertRole.NODE_CA -> nodeInfoBuilder.addLegalIdentity(CordaX500Name.build(X500Principal(csr.subject.encoded)), nodeKeyPair) + CertRole.SERVICE_IDENTITY -> nodeInfoBuilder.addServiceIdentity(CordaX500Name.build(X500Principal(csr.subject.encoded)), nodeKeyPair) + else -> throw IllegalArgumentException("Unsupported cert role $certRole.") + } + storage.putCertificatePath(requestId, identity.certPath, "Test") + key + } + return Pair(nodeInfoBuilder.build(), keys) +} + +internal fun createValidSignedNodeInfo(organisation: String, storage: CertificateSigningRequestStorage): Pair { + val (nodeInfo, key) = createValidNodeInfo(organisation, storage) + return Pair(NodeInfoAndSigned(nodeInfo.signWith(listOf(key))), key) +} diff --git a/node-api/src/test/kotlin/net/corda/nodeapi/internal/SignedNodeInfoTest.kt b/node-api/src/test/kotlin/net/corda/nodeapi/internal/SignedNodeInfoTest.kt index 8f1ae50216..d81a14e1fc 100644 --- a/node-api/src/test/kotlin/net/corda/nodeapi/internal/SignedNodeInfoTest.kt +++ b/node-api/src/test/kotlin/net/corda/nodeapi/internal/SignedNodeInfoTest.kt @@ -41,23 +41,23 @@ class SignedNodeInfoTest { @Test fun `verifying single identity`() { - nodeInfoBuilder.addIdentity(ALICE_NAME) + nodeInfoBuilder.addLegalIdentity(ALICE_NAME) val (nodeInfo, signedNodeInfo) = nodeInfoBuilder.buildWithSigned() assertThat(signedNodeInfo.verified()).isEqualTo(nodeInfo) } @Test fun `verifying multiple identities`() { - nodeInfoBuilder.addIdentity(ALICE_NAME) - nodeInfoBuilder.addIdentity(BOB_NAME) + nodeInfoBuilder.addLegalIdentity(ALICE_NAME) + nodeInfoBuilder.addLegalIdentity(BOB_NAME) val (nodeInfo, signedNodeInfo) = nodeInfoBuilder.buildWithSigned() assertThat(signedNodeInfo.verified()).isEqualTo(nodeInfo) } @Test fun `verifying missing signature`() { - val (_, aliceKey) = nodeInfoBuilder.addIdentity(ALICE_NAME) - nodeInfoBuilder.addIdentity(BOB_NAME) + val (_, aliceKey) = nodeInfoBuilder.addLegalIdentity(ALICE_NAME) + nodeInfoBuilder.addLegalIdentity(BOB_NAME) val nodeInfo = nodeInfoBuilder.build() val signedNodeInfo = nodeInfo.signWith(listOf(aliceKey)) assertThatThrownBy { signedNodeInfo.verified() } @@ -80,7 +80,7 @@ class SignedNodeInfoTest { @Test fun `verifying extra signature`() { - val (_, aliceKey) = nodeInfoBuilder.addIdentity(ALICE_NAME) + val (_, aliceKey) = nodeInfoBuilder.addLegalIdentity(ALICE_NAME) val nodeInfo = nodeInfoBuilder.build() val signedNodeInfo = nodeInfo.signWith(listOf(aliceKey, generateKeyPair().private)) assertThatThrownBy { signedNodeInfo.verified() } @@ -90,7 +90,7 @@ class SignedNodeInfoTest { @Test fun `verifying incorrect signature`() { - nodeInfoBuilder.addIdentity(ALICE_NAME) + nodeInfoBuilder.addLegalIdentity(ALICE_NAME) val nodeInfo = nodeInfoBuilder.build() val signedNodeInfo = nodeInfo.signWith(listOf(generateKeyPair().private)) assertThatThrownBy { signedNodeInfo.verified() } @@ -100,8 +100,8 @@ class SignedNodeInfoTest { @Test fun `verifying with signatures in wrong order`() { - val (_, aliceKey) = nodeInfoBuilder.addIdentity(ALICE_NAME) - val (_, bobKey) = nodeInfoBuilder.addIdentity(BOB_NAME) + val (_, aliceKey) = nodeInfoBuilder.addLegalIdentity(ALICE_NAME) + val (_, bobKey) = nodeInfoBuilder.addLegalIdentity(BOB_NAME) val nodeInfo = nodeInfoBuilder.build() val signedNodeInfo = nodeInfo.signWith(listOf(bobKey, aliceKey)) assertThatThrownBy { signedNodeInfo.verified() } diff --git a/node/src/test/kotlin/net/corda/node/services/network/NetworkMapClientTest.kt b/node/src/test/kotlin/net/corda/node/services/network/NetworkMapClientTest.kt index b506b93d06..2315b369c7 100644 --- a/node/src/test/kotlin/net/corda/node/services/network/NetworkMapClientTest.kt +++ b/node/src/test/kotlin/net/corda/node/services/network/NetworkMapClientTest.kt @@ -83,8 +83,8 @@ class NetworkMapClientTest { @Test fun `errors return a meaningful error message`() { val nodeInfoBuilder = TestNodeInfoBuilder() - val (_, aliceKey) = nodeInfoBuilder.addIdentity(ALICE_NAME) - nodeInfoBuilder.addIdentity(BOB_NAME) + val (_, aliceKey) = nodeInfoBuilder.addLegalIdentity(ALICE_NAME) + nodeInfoBuilder.addLegalIdentity(BOB_NAME) val nodeInfo3 = nodeInfoBuilder.build() val signedNodeInfo3 = nodeInfo3.signWith(listOf(aliceKey)) diff --git a/testing/test-utils/src/main/kotlin/net/corda/testing/internal/TestNodeInfoBuilder.kt b/testing/test-utils/src/main/kotlin/net/corda/testing/internal/TestNodeInfoBuilder.kt index 9f52ee4dd9..fa28b21313 100644 --- a/testing/test-utils/src/main/kotlin/net/corda/testing/internal/TestNodeInfoBuilder.kt +++ b/testing/test-utils/src/main/kotlin/net/corda/testing/internal/TestNodeInfoBuilder.kt @@ -30,7 +30,7 @@ import java.security.cert.X509Certificate class TestNodeInfoBuilder(private val intermediateAndRoot: Pair = DEV_INTERMEDIATE_CA to DEV_ROOT_CA.certificate) { private val identitiesAndPrivateKeys = ArrayList>() - fun addIdentity(name: CordaX500Name, nodeKeyPair: KeyPair = Crypto.generateKeyPair(X509Utilities.DEFAULT_TLS_SIGNATURE_SCHEME)): Pair { + fun addLegalIdentity(name: CordaX500Name, nodeKeyPair: KeyPair = Crypto.generateKeyPair(X509Utilities.DEFAULT_TLS_SIGNATURE_SCHEME)): Pair { val nodeCertificateAndKeyPair = createDevNodeCa(intermediateAndRoot.first, name, nodeKeyPair) val identityKeyPair = Crypto.generateKeyPair() val identityCert = X509Utilities.createCertificate( @@ -39,12 +39,35 @@ class TestNodeInfoBuilder(private val intermediateAndRoot: Pair { + val serviceCert = X509Utilities.createCertificate( + CertificateType.SERVICE_IDENTITY, + intermediateAndRoot.first.certificate, + intermediateAndRoot.first.keyPair, + name.x500Principal, + nodeKeyPair.public) + + val certs = arrayOf(serviceCert) + val key = nodeKeyPair.private + + val certPath = X509Utilities.buildCertPath(*certs, + intermediateAndRoot.first.certificate, + intermediateAndRoot.second) + + return Pair(PartyAndCertificate(certPath), key).also { identitiesAndPrivateKeys += it } } @@ -72,7 +95,7 @@ class TestNodeInfoBuilder(private val intermediateAndRoot: Pair