From 00a22c91df3d88ebb173f4aa43530c85aa0bfa9b Mon Sep 17 00:00:00 2001 From: Katarzyna Streich Date: Sat, 7 Apr 2018 15:28:57 +0100 Subject: [PATCH] ENT-1684: Remove nodes that didn't accept update from network map (#692) On flag day all nodes that didn't accept new parameters update will be kicked out of the network map. --- .../doorman/NetworkParametersUpdateTest.kt | 3 ++ .../common/persistence/NetworkMapStorage.kt | 8 +++ .../common/persistence/NodeInfoStorage.kt | 5 +- .../PersistentNetworkMapStorage.kt | 9 ++++ .../persistence/PersistentNodeInfoStorage.kt | 9 ++-- .../common/signer/NetworkMapSigner.kt | 8 +-- .../webservice/NetworkMapWebService.kt | 2 +- .../PersistentNetworkMapStorageTest.kt | 53 +++++++++++++++++++ .../PersistentNodeInfoStorageTest.kt | 22 +++++++- .../webservice/NetworkMapWebServiceTest.kt | 2 +- 10 files changed, 107 insertions(+), 14 deletions(-) diff --git a/network-management/src/integration-test/kotlin/com/r3/corda/networkmanage/doorman/NetworkParametersUpdateTest.kt b/network-management/src/integration-test/kotlin/com/r3/corda/networkmanage/doorman/NetworkParametersUpdateTest.kt index 2dad7a1acc..f49ffa27c1 100644 --- a/network-management/src/integration-test/kotlin/com/r3/corda/networkmanage/doorman/NetworkParametersUpdateTest.kt +++ b/network-management/src/integration-test/kotlin/com/r3/corda/networkmanage/doorman/NetworkParametersUpdateTest.kt @@ -94,6 +94,7 @@ class NetworkParametersUpdateTest : IntegrationTest() { ) { var (alice) = listOf( startNode(providedName = ALICE_NAME), + startNode(providedName = BOB_NAME), defaultNotaryNode ).transpose().getOrThrow() alice as NodeHandleInternal @@ -137,6 +138,8 @@ class NetworkParametersUpdateTest : IntegrationTest() { .readObject().verified() assertEquals(networkParameters, paramUpdateInfo.parameters) assertThat(alice.rpc.networkParametersFeed().snapshot).isNull() // Check that NMS doesn't advertise updates anymore. + // Check that Bob is no longer on the network as it didn't accept the new parameteres. + assertThat(alice.rpc.networkMapSnapshot().map { it.legalIdentities[0].name }).doesNotContain(BOB_NAME) } } diff --git a/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/NetworkMapStorage.kt b/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/NetworkMapStorage.kt index fcf228d398..c9c73d33bb 100644 --- a/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/NetworkMapStorage.kt +++ b/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/NetworkMapStorage.kt @@ -36,6 +36,7 @@ interface NetworkMapStorage { /** * Retrieves node info hashes where [NodeInfoEntity.isCurrent] is true and the certificate status is [CertificateStatus.VALID] + * Nodes should have declared that they are using correct set of parameters. */ // TODO "Active" is the wrong word here fun getActiveNodeInfoHashes(): List @@ -70,4 +71,11 @@ interface NetworkMapStorage { fun getCurrentParametersUpdate(): ParametersUpdateEntity? fun setParametersUpdateStatus(update: ParametersUpdateEntity, newStatus: UpdateStatus) + + /** + * Perform the switch of parameters on the flagDay. + * 1. Change status of ParametersUpdateEntity to [UpdateStatus.APPLIED] + * 2. Mark all the node infos that didn't accept the update as not current (so they won't be advertised in the network map) + */ + fun switchFlagDay(update: ParametersUpdateEntity) } diff --git a/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/NodeInfoStorage.kt b/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/NodeInfoStorage.kt index 1368be7b67..9fb43c2d17 100644 --- a/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/NodeInfoStorage.kt +++ b/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/NodeInfoStorage.kt @@ -15,6 +15,7 @@ import net.corda.core.crypto.SecureHash import net.corda.core.node.NodeInfo import net.corda.nodeapi.internal.NodeInfoAndSigned import net.corda.nodeapi.internal.SignedNodeInfo +import java.security.PublicKey import java.security.cert.CertPath /** @@ -48,8 +49,8 @@ interface NodeInfoStorage { /** * Store information about latest accepted [NetworkParameters] hash. - * @param publicKeyHash Hash of public key that accepted network parameters. This public key should belong to [NodeInfo] + * @param publicKey Public key that accepted network parameters. This public key should belong to [NodeInfo] * @param acceptedParametersHash Hash of latest accepted network parameters. */ - fun ackNodeInfoParametersUpdate(publicKeyHash: SecureHash, acceptedParametersHash: SecureHash) + fun ackNodeInfoParametersUpdate(publicKey: PublicKey, acceptedParametersHash: SecureHash) } \ No newline at end of file diff --git a/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/PersistentNetworkMapStorage.kt b/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/PersistentNetworkMapStorage.kt index c1f69692f4..3d61fc789c 100644 --- a/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/PersistentNetworkMapStorage.kt +++ b/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/PersistentNetworkMapStorage.kt @@ -156,6 +156,15 @@ class PersistentNetworkMapStorage(private val database: CordaPersistence) : Netw session.merge(update.copy(status = newStatus)) } } + + override fun switchFlagDay(update: ParametersUpdateEntity) { + database.transaction { + setParametersUpdateStatus(update, UpdateStatus.APPLIED) + session.createQuery("update ${NodeInfoEntity::class.java.name} n set n.isCurrent = false " + + "where (n.acceptedParametersUpdate != :acceptedParamUp or n.acceptedParametersUpdate is null) and n.isCurrent = true") + .setParameter("acceptedParamUp", update).executeUpdate() + } + } } internal fun DatabaseTransaction.getNetworkParametersEntity(hash: SecureHash): NetworkParametersEntity? { 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 29ad2ab8a9..24b00ccc8e 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 @@ -25,6 +25,7 @@ import net.corda.nodeapi.internal.SignedNodeInfo import net.corda.nodeapi.internal.crypto.x509Certificates import net.corda.nodeapi.internal.persistence.CordaPersistence import net.corda.nodeapi.internal.persistence.DatabaseTransaction +import java.security.PublicKey import java.security.cert.CertPath /** @@ -39,7 +40,7 @@ class PersistentNodeInfoStorage(private val database: CordaPersistence) : NodeIn database.transaction { val count = session.createQuery( - "select count(*) from ${NodeInfoEntity::class.java.name} where nodeInfoHash = :nodeInfoHash", java.lang.Long::class.java) + "select count(*) from ${NodeInfoEntity::class.java.name} where nodeInfoHash = :nodeInfoHash and isCurrent = true", java.lang.Long::class.java) .setParameter("nodeInfoHash", nodeInfoHash.toString()) .singleResult .toLong() @@ -64,7 +65,7 @@ class PersistentNodeInfoStorage(private val database: CordaPersistence) : NodeIn // Update any [NodeInfoEntity] instance for this CSR as not current. existingNodeInfos.forEach { session.merge(it.copy(isCurrent = false)) } - session.save(NodeInfoEntity( + session.saveOrUpdate(NodeInfoEntity( nodeInfoHash = nodeInfoHash.toString(), publicKeyHash = nodeInfo.legalIdentities[0].owningKey.hashString(), certificateSigningRequest = request, @@ -96,11 +97,11 @@ class PersistentNodeInfoStorage(private val database: CordaPersistence) : NodeIn } } - override fun ackNodeInfoParametersUpdate(publicKeyHash: SecureHash, acceptedParametersHash: SecureHash) { + override fun ackNodeInfoParametersUpdate(publicKey: PublicKey, acceptedParametersHash: SecureHash) { return database.transaction { val nodeInfoEntity = session.fromQuery( "n where n.publicKeyHash = :publicKeyHash and isCurrent = true") - .setParameter("publicKeyHash", publicKeyHash.toString()) + .setParameter("publicKeyHash", publicKey.hashString()) .singleResult val parametersUpdateEntity = session.fromQuery( "u where u.networkParameters.hash = :acceptedParametersHash"). diff --git a/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/signer/NetworkMapSigner.kt b/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/signer/NetworkMapSigner.kt index 1840aa8799..d680842844 100644 --- a/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/signer/NetworkMapSigner.kt +++ b/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/signer/NetworkMapSigner.kt @@ -51,9 +51,6 @@ class NetworkMapSigner(private val networkMapStorage: NetworkMapStorage, private val activeNetworkParameters = activeNetworkMap?.networkParameters logger.debug { "Current network map parameters: ${activeNetworkParameters?.networkParameters}" } - val nodeInfoHashes = networkMapStorage.getActiveNodeInfoHashes() - logger.debug { "Retrieved node info hashes:\n${nodeInfoHashes.joinToString("\n")}" } - // We persist signed parameters only if they were not persisted before (they are not in currentSignedNetworkMap as // normal parameters or as an update) if (!latestNetworkParameters.isSigned) { @@ -63,12 +60,15 @@ class NetworkMapSigner(private val networkMapStorage: NetworkMapStorage, private } val parametersToNetworkMap = if (parametersUpdate?.status == FLAG_DAY || activeNetworkParameters == null) { - parametersUpdate?.let { networkMapStorage.setParametersUpdateStatus(it, APPLIED) } + parametersUpdate?.let { networkMapStorage.switchFlagDay(it) } latestNetworkParameters } else { activeNetworkParameters } + val nodeInfoHashes = networkMapStorage.getActiveNodeInfoHashes() + logger.debug { "Retrieved node info hashes:\n${nodeInfoHashes.joinToString("\n")}" } + val newNetworkMap = NetworkMap( nodeInfoHashes, SecureHash.parse(parametersToNetworkMap.hash), diff --git a/network-management/src/main/kotlin/com/r3/corda/networkmanage/doorman/webservice/NetworkMapWebService.kt b/network-management/src/main/kotlin/com/r3/corda/networkmanage/doorman/webservice/NetworkMapWebService.kt index 6fa65e424a..d3e4eac3e7 100644 --- a/network-management/src/main/kotlin/com/r3/corda/networkmanage/doorman/webservice/NetworkMapWebService.kt +++ b/network-management/src/main/kotlin/com/r3/corda/networkmanage/doorman/webservice/NetworkMapWebService.kt @@ -115,7 +115,7 @@ class NetworkMapWebService(private val nodeInfoStorage: NodeInfoStorage, val hash = signedParametersHash.verified() networkMapStorage.getSignedNetworkParameters(hash) ?: throw IllegalArgumentException("No network parameters with hash $hash") logger.debug { "Received ack-parameters with $hash from ${signedParametersHash.sig.by}" } - nodeInfoStorage.ackNodeInfoParametersUpdate(signedParametersHash.sig.by.encoded.sha256(), hash) + nodeInfoStorage.ackNodeInfoParametersUpdate(signedParametersHash.sig.by, hash) ok() } catch (e: SignatureException) { status(Response.Status.FORBIDDEN).entity(e.message) diff --git a/network-management/src/test/kotlin/com/r3/corda/networkmanage/common/persistence/PersistentNetworkMapStorageTest.kt b/network-management/src/test/kotlin/com/r3/corda/networkmanage/common/persistence/PersistentNetworkMapStorageTest.kt index 8425aae758..5a3676bb16 100644 --- a/network-management/src/test/kotlin/com/r3/corda/networkmanage/common/persistence/PersistentNetworkMapStorageTest.kt +++ b/network-management/src/test/kotlin/com/r3/corda/networkmanage/common/persistence/PersistentNetworkMapStorageTest.kt @@ -14,7 +14,10 @@ import com.r3.corda.networkmanage.TestBase import com.r3.corda.networkmanage.common.persistence.entity.NodeInfoEntity import com.r3.corda.networkmanage.common.persistence.entity.ParametersUpdateEntity import com.r3.corda.networkmanage.common.persistence.entity.UpdateStatus +import com.r3.corda.networkmanage.common.utils.hashString import net.corda.core.crypto.SecureHash +import net.corda.core.crypto.sha256 +import net.corda.core.serialization.serialize import net.corda.core.utilities.days import net.corda.nodeapi.internal.createDevNetworkMapCa import net.corda.nodeapi.internal.crypto.CertificateAndKeyPair @@ -149,4 +152,54 @@ class PersistentNetworkMapStorageTest : TestBase() { assertThat(firstUpdate.status).isEqualTo(UpdateStatus.CANCELLED) assertThat(networkMapStorage.getCurrentParametersUpdate()?.description).isEqualTo("Update of update") } + + @Test + fun `switch ParametersUpdate on flag day`() { + // Update + val networkParameters1 = testNetworkParameters() + val updateDeadline = Instant.now() + 10.days + networkMapStorage.saveNewParametersUpdate(networkParameters1, "Update 1", updateDeadline) + // given + val (signedNodeInfoA) = createValidSignedNodeInfo("TestA", requestStorage) // null as acceptedParametersUpdate + val (signedNodeInfoB) = createValidSignedNodeInfo("TestB", requestStorage) // accepts update + + // Put signed node info data + nodeInfoStorage.putNodeInfo(signedNodeInfoA) + val nodeInfoHashB = nodeInfoStorage.putNodeInfo(signedNodeInfoB) + + nodeInfoStorage.ackNodeInfoParametersUpdate(signedNodeInfoB.nodeInfo.legalIdentities[0].owningKey, networkParameters1.serialize().hash) + val parameterUpdate = networkMapStorage.getCurrentParametersUpdate()!! + networkMapStorage.switchFlagDay(parameterUpdate) + // when + val validNodeInfoHashes = networkMapStorage.getActiveNodeInfoHashes() + // then + assertThat(validNodeInfoHashes).containsOnly(nodeInfoHashB) + } + + @Test + fun `accept second set of parameters and switch on flag day`() { + // Update 1 + val networkParameters1 = testNetworkParameters() + val updateDeadline = Instant.now() + 10.days + networkMapStorage.saveNewParametersUpdate(networkParameters1, "Update 1", updateDeadline) + // given + val (signedNodeInfoA) = createValidSignedNodeInfo("TestA", requestStorage) // Update 1 as acceptedParametersUpdate + val (signedNodeInfoB) = createValidSignedNodeInfo("TestB", requestStorage) // Update 2 as acceptedParametersUpdate + + // Put signed node info data + nodeInfoStorage.putNodeInfo(signedNodeInfoA) + val nodeInfoHashB = nodeInfoStorage.putNodeInfo(signedNodeInfoB) + + nodeInfoStorage.ackNodeInfoParametersUpdate(signedNodeInfoA.nodeInfo.legalIdentities[0].owningKey, networkParameters1.serialize().hash) + // Update 2 + val networkParameters2 = testNetworkParameters(epoch = 2) + networkMapStorage.saveNewParametersUpdate(networkParameters2, "Update 2", updateDeadline + 10.days) + nodeInfoStorage.ackNodeInfoParametersUpdate(signedNodeInfoB.nodeInfo.legalIdentities[0].owningKey, networkParameters2.serialize().hash) + val parameterUpdate = networkMapStorage.getCurrentParametersUpdate()!! + networkMapStorage.switchFlagDay(parameterUpdate) + // when + val validNodeInfoHashes = networkMapStorage.getActiveNodeInfoHashes() + // then + assertThat(validNodeInfoHashes).containsOnly(nodeInfoHashB) + } } 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 cae4623552..dafe94172d 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 @@ -164,6 +164,24 @@ class PersistentNodeInfoStorageTest : TestBase() { assertThat(singleNodeInfo().isCurrent).isTrue() } + @Test + fun `publish same node info twice after isCurrent change`() { + fun singleNodeInfo() = persistence.transaction { session.fromQuery("").singleResult } + + val (nodeInfoAndSigned) = createValidSignedNodeInfo("Test", requestStorage) + nodeInfoStorage.putNodeInfo(nodeInfoAndSigned) + // Change isCurrent to false (that happens on flagDay change) + persistence.transaction { + val ni = singleNodeInfo() + session.merge(ni.copy(isCurrent = false)) + } + assertThat(singleNodeInfo().isCurrent).isFalse() + val nodeInfo = singleNodeInfo() + nodeInfoStorage.putNodeInfo(nodeInfoAndSigned) + assertThat(nodeInfo.publishedAt).isBeforeOrEqualTo(singleNodeInfo().publishedAt) + assertThat(singleNodeInfo().isCurrent).isTrue() + } + @Test fun `accept parameters updates node info correctly`() { // given @@ -174,7 +192,7 @@ class PersistentNodeInfoStorageTest : TestBase() { val netParamsHash = networkParameters.serialize().hash networkMapStorage.saveNewParametersUpdate(networkParameters, "Update", Instant.now() + 1.days) val nodeInfoHash = nodeInfoStorage.putNodeInfo(nodeInfoAndSigned) - nodeInfoStorage.ackNodeInfoParametersUpdate(nodeInfoAndSigned.nodeInfo.legalIdentities[0].owningKey.encoded.sha256(), netParamsHash) + nodeInfoStorage.ackNodeInfoParametersUpdate(nodeInfoAndSigned.nodeInfo.legalIdentities[0].owningKey, netParamsHash) // then val acceptedUpdate = nodeInfoStorage.getAcceptedParametersUpdate(nodeInfoHash) @@ -190,7 +208,7 @@ class PersistentNodeInfoStorageTest : TestBase() { val (nodeInfoAndSigned, privateKey) = createValidSignedNodeInfo("Test", requestStorage) nodeInfoStorage.putNodeInfo(nodeInfoAndSigned) - nodeInfoStorage.ackNodeInfoParametersUpdate(nodeInfoAndSigned.nodeInfo.legalIdentities[0].owningKey.encoded.sha256(), netParamsHash) + nodeInfoStorage.ackNodeInfoParametersUpdate(nodeInfoAndSigned.nodeInfo.legalIdentities[0].owningKey, netParamsHash) val nodeInfo2 = nodeInfoAndSigned.nodeInfo.copy(serial = 2) val nodeInfoAndSigned2 = NodeInfoAndSigned(nodeInfo2.signWith(listOf(privateKey))) diff --git a/network-management/src/test/kotlin/com/r3/corda/networkmanage/doorman/webservice/NetworkMapWebServiceTest.kt b/network-management/src/test/kotlin/com/r3/corda/networkmanage/doorman/webservice/NetworkMapWebServiceTest.kt index 54d8476e3e..9be8ea2c25 100644 --- a/network-management/src/test/kotlin/com/r3/corda/networkmanage/doorman/webservice/NetworkMapWebServiceTest.kt +++ b/network-management/src/test/kotlin/com/r3/corda/networkmanage/doorman/webservice/NetworkMapWebServiceTest.kt @@ -217,7 +217,7 @@ class NetworkMapWebServiceTest { val keyPair = Crypto.generateKeyPair() val signedHash = hash.serialize().sign { keyPair.sign(it) } it.doPost("ack-parameters", signedHash.serialize()) - verify(nodeInfoStorage).ackNodeInfoParametersUpdate(keyPair.public.encoded.sha256(), hash) + verify(nodeInfoStorage).ackNodeInfoParametersUpdate(keyPair.public, hash) val randomSigned = SecureHash.randomSHA256().serialize().sign { keyPair.sign(it) } assertThatThrownBy { it.doPost("ack-parameters", randomSigned.serialize()) } .hasMessageContaining("HTTP ERROR 500")