From dfa6291b2f1e62db95815ab9dfbd39d70126ae6c Mon Sep 17 00:00:00 2001 From: Nick Dunstone <49945179+nickdunstone13@users.noreply.github.com> Date: Fri, 4 Oct 2019 11:15:31 +0100 Subject: [PATCH] CORDA-3235: O/S version of fix for slow running in 4.3 (#5531) * O/S version of fix for slow running in 4.3 * Removal of IdentityServiceInternal from test classes * Code review comments * O/S version of fix for slow running in 4.3 * Removal of IdentityServiceInternal from test classes * Code review comments * Re-baselined Detekt * Fixed warning --- detekt-baseline.xml | 14 +--- .../services/api/IdentityServiceInternal.kt | 22 +++++ .../identity/InMemoryIdentityService.kt | 14 +++- .../identity/PersistentIdentityService.kt | 81 ++++++++++++------- .../net/corda/node/services/keys/KMSUtils.kt | 8 +- .../node-core.changelog-v14-table.xml | 2 + 6 files changed, 95 insertions(+), 46 deletions(-) create mode 100644 node/src/main/kotlin/net/corda/node/services/api/IdentityServiceInternal.kt diff --git a/detekt-baseline.xml b/detekt-baseline.xml index 07d0b5cf0c..85a4547a98 100644 --- a/detekt-baseline.xml +++ b/detekt-baseline.xml @@ -3021,18 +3021,6 @@ MaxLineLength:PersistentIdentityMigrationNewTable.kt$PersistentIdentityMigrationNewTable$throw PersistentIdentitiesMigrationException("Cannot migrate persistent identities as liquibase failed to provide a suitable database connection") MaxLineLength:PersistentIdentityMigrationNewTableTest.kt$PersistentIdentityMigrationNewTableTest$session.save(PersistentIdentityService.PersistentPublicKeyHashToCertificate(it.owningKey.hash.toString(), it.certPath.encoded)) MaxLineLength:PersistentIdentityMigrationNewTableTest.kt$PersistentIdentityMigrationNewTableTest$val identityService = makeTestIdentityService(PersistentIdentityMigrationNewTableTest.dummyNotary.identity, BOB_IDENTITY, ALICE_IDENTITY) - MaxLineLength:PersistentIdentityService.kt$PersistentIdentityService$ @Throws(CertificateExpiredException::class, CertificateNotYetValidException::class, InvalidAlgorithmParameterException::class) private fun verifyAndRegisterIdentity(trustAnchor: TrustAnchor, identity: PartyAndCertificate): PartyAndCertificate? - MaxLineLength:PersistentIdentityService.kt$PersistentIdentityService$// Allows us to eliminate keys we know belong to others by using the cache contents that might have been seen during other identity activity. // Concentrating activity on the identity cache works better than spreading checking across identity and key management, because we cache misses too. fun stripNotOurKeys(keys: Iterable<PublicKey>): Iterable<PublicKey> - MaxLineLength:PersistentIdentityService.kt$PersistentIdentityService$@Throws(UnknownAnonymousPartyException::class) override - MaxLineLength:PersistentIdentityService.kt$PersistentIdentityService$fun loadIdentities(identities: Collection<PartyAndCertificate> = emptySet(), confidentialIdentities: Collection<PartyAndCertificate> = emptySet()) - MaxLineLength:PersistentIdentityService.kt$PersistentIdentityService$log.warn("Certificate validation failed for ${identity.name} against trusted root ${trustAnchor.trustedCert.subjectX500Principal}.") - MaxLineLength:PersistentIdentityService.kt$PersistentIdentityService${ // If there is no entry in the legal keyToPartyAndCert table then the party must be a confidential identity so we perform // a lookup in the keyToName table. If an entry for that public key exists, then we attempt look up the associated node's // PartyAndCertificate. val name = keyToName[party.owningKey.toStringShort()] if (name != null) { // This should never return null as this node would not be able to communicate with the node providing a confidential // identity unless its NodeInfo/PartyAndCertificate were available. wellKnownPartyFromX500Name(name) } else { null } } - MaxLineLength:PersistentIdentityService.kt$PersistentIdentityService${ // This should never return null as this node would not be able to communicate with the node providing a confidential // identity unless its NodeInfo/PartyAndCertificate were available. wellKnownPartyFromX500Name(name) } - MaxLineLength:PersistentIdentityService.kt$PersistentIdentityService${ // Update the three tables as necessary. We definitely store the public key and map it to a party and we optionally update // the public key to external ID mapping table. This block will only ever be reached when registering keys generated on // other because when a node generates its own keys "registerKeyToParty" is automatically called by KeyManagementService.freshKey. registerKeyToParty(publicKey, party) hashToKey[publicKeyHash] = publicKey if (externalId != null) { registerKeyToExternalId(publicKey, externalId) } } - MaxLineLength:PersistentIdentityService.kt$PersistentIdentityService.Companion$fun createHashToKeyMap(cacheFactory: NamedCacheFactory): AppendOnlyPersistentMap<String, PublicKey, PersistentHashToPublicKey, String> - MaxLineLength:PersistentIdentityService.kt$PersistentIdentityService.Companion$fun createKeyToPartyAndCertMap(cacheFactory: NamedCacheFactory): AppendOnlyPersistentMap<String, PartyAndCertificate, PersistentPublicKeyHashToCertificate, String> - MaxLineLength:PersistentIdentityService.kt$PersistentIdentityService.Companion$fun createKeyToX500Map(cacheFactory: NamedCacheFactory): AppendOnlyPersistentMap<String, CordaX500Name, PersistentPublicKeyHashToParty, String> - MaxLineLength:PersistentIdentityService.kt$PersistentIdentityService.Companion$fun createX500ToKeyMap(cacheFactory: NamedCacheFactory): AppendOnlyPersistentMap<CordaX500Name, String, PersistentPartyToPublicKeyHash, String> MaxLineLength:PersistentIdentityServiceTests.kt$PersistentIdentityServiceTests$listOf("Organisation A", "Organisation B", "Organisation C") .map { getTestPartyAndCertificate(CordaX500Name(organisation = it, locality = "London", country = "GB"), generateKeyPair().public) } MaxLineLength:PersistentIdentityServiceTests.kt$PersistentIdentityServiceTests$val alicente = getTestPartyAndCertificate(CordaX500Name(organisation = "Alicente Worldwide", locality = "London", country = "GB"), generateKeyPair().public) MaxLineLength:PersistentMap.kt$PersistentMap$ExplicitRemoval<K, V, E, EK> : RemovalListener @@ -4393,7 +4381,7 @@ TooManyFunctions:P2PMessagingClient.kt$P2PMessagingClient : SingletonSerializeAsTokenMessagingServiceAddressToArtemisQueueResolver TooManyFunctions:PathUtils.kt$net.corda.core.internal.PathUtils.kt TooManyFunctions:Perceivable.kt$net.corda.finance.contracts.universal.Perceivable.kt - TooManyFunctions:PersistentIdentityService.kt$PersistentIdentityService : SingletonSerializeAsTokenIdentityService + TooManyFunctions:PersistentIdentityService.kt$PersistentIdentityService : SingletonSerializeAsTokenIdentityServiceInternal TooManyFunctions:PersistentNetworkMapCache.kt$PersistentNetworkMapCache : NetworkMapCacheInternalSingletonSerializeAsToken TooManyFunctions:PortfolioApi.kt$PortfolioApi TooManyFunctions:PropertyDescriptor.kt$net.corda.serialization.internal.amqp.PropertyDescriptor.kt diff --git a/node/src/main/kotlin/net/corda/node/services/api/IdentityServiceInternal.kt b/node/src/main/kotlin/net/corda/node/services/api/IdentityServiceInternal.kt new file mode 100644 index 0000000000..2e64fbbfad --- /dev/null +++ b/node/src/main/kotlin/net/corda/node/services/api/IdentityServiceInternal.kt @@ -0,0 +1,22 @@ +package net.corda.node.services.api + +import net.corda.core.identity.PartyAndCertificate +import net.corda.core.node.services.IdentityService +import net.corda.core.utilities.contextLogger +import java.security.InvalidAlgorithmParameterException +import java.security.cert.CertificateExpiredException +import java.security.cert.CertificateNotYetValidException + +interface IdentityServiceInternal : IdentityService { + private companion object { + val log = contextLogger() + } + + /** This method exists so it can be mocked with doNothing, rather than having to make up a possibly invalid return value. */ + fun justVerifyAndRegisterIdentity(identity: PartyAndCertificate, isNewRandomIdentity: Boolean = false) { + verifyAndRegisterIdentity(identity, isNewRandomIdentity) + } + + @Throws(CertificateExpiredException::class, CertificateNotYetValidException::class, InvalidAlgorithmParameterException::class) + fun verifyAndRegisterIdentity(identity: PartyAndCertificate, isNewRandomIdentity: Boolean): PartyAndCertificate? +} \ No newline at end of file diff --git a/node/src/main/kotlin/net/corda/node/services/identity/InMemoryIdentityService.kt b/node/src/main/kotlin/net/corda/node/services/identity/InMemoryIdentityService.kt index 080e4e8c98..b1cf9d3e9f 100644 --- a/node/src/main/kotlin/net/corda/node/services/identity/InMemoryIdentityService.kt +++ b/node/src/main/kotlin/net/corda/node/services/identity/InMemoryIdentityService.kt @@ -11,6 +11,7 @@ import net.corda.core.node.services.IdentityService import net.corda.core.serialization.SingletonSerializeAsToken import net.corda.core.utilities.contextLogger import net.corda.core.utilities.trace +import net.corda.node.services.api.IdentityServiceInternal import net.corda.node.services.persistence.WritablePublicKeyToOwningIdentityCache import net.corda.nodeapi.internal.crypto.X509Utilities import net.corda.nodeapi.internal.crypto.x509Certificates @@ -32,7 +33,7 @@ import kotlin.collections.LinkedHashSet class InMemoryIdentityService( identities: List = emptyList(), override val trustRoot: X509Certificate -) : SingletonSerializeAsToken(), IdentityService { +) : SingletonSerializeAsToken(), IdentityServiceInternal { companion object { private val log = contextLogger() } @@ -60,6 +61,11 @@ class InMemoryIdentityService( return verifyAndRegisterIdentity(trustAnchor, identity) } + @Throws(CertificateExpiredException::class, CertificateNotYetValidException::class, InvalidAlgorithmParameterException::class) + override fun verifyAndRegisterIdentity(identity: PartyAndCertificate, isNewRandomIdentity: Boolean): PartyAndCertificate? { + return verifyAndRegisterIdentity(trustAnchor, identity) + } + @Throws(CertificateExpiredException::class, CertificateNotYetValidException::class, InvalidAlgorithmParameterException::class) private fun verifyAndRegisterIdentity(trustAnchor: TrustAnchor, identity: PartyAndCertificate): PartyAndCertificate? { // Validate the chain first, before we do anything clever with it @@ -82,12 +88,12 @@ class InMemoryIdentityService( val firstPath = X509Utilities.buildCertPath(identityCertChain.slice(idx until identityCertChain.size)) verifyAndRegisterIdentity(trustAnchor, PartyAndCertificate(firstPath)) } - return registerIdentity(identity) + return registerIdentity(identity, false) } - private fun registerIdentity(identity: PartyAndCertificate): PartyAndCertificate? { + private fun registerIdentity(identity: PartyAndCertificate, isNewRandomIdentity: Boolean): PartyAndCertificate? { val identityCertChain = identity.certPath.x509Certificates - log.trace { "Registering identity $identity" } + log.trace { "Registering identity $identity isNewRandomIdentity=${isNewRandomIdentity}" } keyToPartyAndCerts[identity.owningKey] = identity // Always keep the first party we registered, as that's the well known identity nameToKey.computeIfAbsent(identity.name) {identity.owningKey} diff --git a/node/src/main/kotlin/net/corda/node/services/identity/PersistentIdentityService.kt b/node/src/main/kotlin/net/corda/node/services/identity/PersistentIdentityService.kt index 4fab6894b7..6dd70ea852 100644 --- a/node/src/main/kotlin/net/corda/node/services/identity/PersistentIdentityService.kt +++ b/node/src/main/kotlin/net/corda/node/services/identity/PersistentIdentityService.kt @@ -13,6 +13,7 @@ import net.corda.core.serialization.SingletonSerializeAsToken import net.corda.core.utilities.MAX_HASH_HEX_SIZE import net.corda.core.utilities.contextLogger import net.corda.core.utilities.debug +import net.corda.node.services.api.IdentityServiceInternal import net.corda.node.services.keys.BasicHSMKeyManagementService import net.corda.node.services.persistence.PublicKeyHashToExternalId import net.corda.node.services.persistence.WritablePublicKeyToOwningIdentityCache @@ -43,7 +44,7 @@ import kotlin.streams.toList * cached for efficient lookup. */ @ThreadSafe -class PersistentIdentityService(cacheFactory: NamedCacheFactory) : SingletonSerializeAsToken(), IdentityService { +class PersistentIdentityService(cacheFactory: NamedCacheFactory) : SingletonSerializeAsToken(), IdentityServiceInternal { companion object { private val log = contextLogger() @@ -56,7 +57,8 @@ class PersistentIdentityService(cacheFactory: NamedCacheFactory) : SingletonSeri const val IDENTITY_COLUMN_NAME = "identity_value" const val NAME_COLUMN_NAME = "name" - fun createKeyToPartyAndCertMap(cacheFactory: NamedCacheFactory): AppendOnlyPersistentMap { + fun createKeyToPartyAndCertMap(cacheFactory: NamedCacheFactory): AppendOnlyPersistentMap { return AppendOnlyPersistentMap( cacheFactory = cacheFactory, name = "PersistentIdentityService_keyToPartyAndCert", @@ -74,7 +76,8 @@ class PersistentIdentityService(cacheFactory: NamedCacheFactory) : SingletonSeri ) } - fun createX500ToKeyMap(cacheFactory: NamedCacheFactory): AppendOnlyPersistentMap { + fun createX500ToKeyMap(cacheFactory: NamedCacheFactory): AppendOnlyPersistentMap { return AppendOnlyPersistentMap( cacheFactory = cacheFactory, name = "PersistentIdentityService_nameToKey", @@ -89,7 +92,8 @@ class PersistentIdentityService(cacheFactory: NamedCacheFactory) : SingletonSeri ) } - fun createKeyToX500Map(cacheFactory: NamedCacheFactory): AppendOnlyPersistentMap { + fun createKeyToX500Map(cacheFactory: NamedCacheFactory): AppendOnlyPersistentMap { return AppendOnlyPersistentMap( cacheFactory = cacheFactory, name = "PersistentIdentityService_keyToName", @@ -106,7 +110,8 @@ class PersistentIdentityService(cacheFactory: NamedCacheFactory) : SingletonSeri persistentEntityClass = PersistentPublicKeyHashToParty::class.java) } - fun createHashToKeyMap(cacheFactory: NamedCacheFactory): AppendOnlyPersistentMap { + fun createHashToKeyMap(cacheFactory: NamedCacheFactory): AppendOnlyPersistentMap { return AppendOnlyPersistentMap( cacheFactory = cacheFactory, name = "PersistentIdentityService_hashToKey", @@ -207,7 +212,8 @@ class PersistentIdentityService(cacheFactory: NamedCacheFactory) : SingletonSeri notaryIdentityCache.addAll(notaryIdentities) } - fun loadIdentities(identities: Collection = emptySet(), confidentialIdentities: Collection = emptySet()) { + fun loadIdentities(identities: Collection = emptySet(), confidentialIdentities: Collection = + emptySet()) { identities.forEach { val key = mapToKey(it) keyToPartyAndCert.addWithDuplicatesAllowed(key, it, false) @@ -222,7 +228,14 @@ class PersistentIdentityService(cacheFactory: NamedCacheFactory) : SingletonSeri @Throws(CertificateExpiredException::class, CertificateNotYetValidException::class, InvalidAlgorithmParameterException::class) override fun verifyAndRegisterIdentity(identity: PartyAndCertificate): PartyAndCertificate? { - return verifyAndRegisterIdentity(trustAnchor, identity) + return verifyAndRegisterIdentity(identity, false) + } + + @Throws(CertificateExpiredException::class, CertificateNotYetValidException::class, InvalidAlgorithmParameterException::class) + override fun verifyAndRegisterIdentity(identity: PartyAndCertificate, isNewRandomIdentity: Boolean): PartyAndCertificate? { + return database.transaction { + verifyAndRegisterIdentity(trustAnchor, identity, isNewRandomIdentity) + } } /** @@ -230,16 +243,17 @@ class PersistentIdentityService(cacheFactory: NamedCacheFactory) : SingletonSeri * * @param trustAnchor The trust anchor that will verify the identity's validity * @param identity The identity to verify - * @param isNewRandomIdentity true if the identity will not have been registered before (e.g. because it is randomly generated by ourselves). + * @param isNewRandomIdentity true if identity will not have been registered before (e.g. because it is randomly generated by us) */ @Throws(CertificateExpiredException::class, CertificateNotYetValidException::class, InvalidAlgorithmParameterException::class) - private fun verifyAndRegisterIdentity(trustAnchor: TrustAnchor, identity: PartyAndCertificate): PartyAndCertificate? { - // Validate the chain first, before we do anything clever with it + private fun verifyAndRegisterIdentity(trustAnchor: TrustAnchor, identity: PartyAndCertificate, isNewRandomIdentity: Boolean = false): + PartyAndCertificate? { + // Validate the chain first, before we do anything clever with it val identityCertChain = identity.certPath.x509Certificates try { identity.verify(trustAnchor) } catch (e: CertPathValidatorException) { - log.warn("Certificate validation failed for ${identity.name} against trusted root ${trustAnchor.trustedCert.subjectX500Principal}.") + log.warn("Certificate validation failed for ${identity.name} against trusted root ${trustAnchor.trustedCert.subjectX500Principal}.") log.warn("Certificate path :") identityCertChain.reversed().forEachIndexed { index, certificate -> val space = (0 until index).joinToString("") { " " } @@ -249,24 +263,32 @@ class PersistentIdentityService(cacheFactory: NamedCacheFactory) : SingletonSeri } // Ensure we record the first identity of the same name, first val wellKnownCert = identityCertChain.single { CertRole.extract(it)?.isWellKnown ?: false } - if (wellKnownCert != identity.certificate) { + if (wellKnownCert != identity.certificate && !isNewRandomIdentity) { val idx = identityCertChain.lastIndexOf(wellKnownCert) val firstPath = X509Utilities.buildCertPath(identityCertChain.slice(idx until identityCertChain.size)) verifyAndRegisterIdentity(trustAnchor, PartyAndCertificate(firstPath)) } - return registerIdentity(identity) + return registerIdentity(identity, isNewRandomIdentity) } - private fun registerIdentity(identity: PartyAndCertificate): PartyAndCertificate? { + private fun registerIdentity(identity: PartyAndCertificate, isNewRandomIdentity: Boolean): PartyAndCertificate? { log.debug { "Registering identity $identity" } val identityCertChain = identity.certPath.x509Certificates val key = mapToKey(identity) - return database.transaction { - keyToPartyAndCert.addWithDuplicatesAllowed(key, identity, false) - nameToKey.addWithDuplicatesAllowed(identity.name, key, false) - keyToName.addWithDuplicatesAllowed(key, identity.name, false) + + if (isNewRandomIdentity) { + // Because this is supposed to be new and random, there's no way we have it in the database already, so skip the this check + keyToPartyAndCert[key] = identity val parentId = identityCertChain[1].publicKey.toStringShort() - keyToPartyAndCert[parentId] + return keyToPartyAndCert[parentId] + } else { + return database.transaction { + keyToPartyAndCert.addWithDuplicatesAllowed(key, identity, false) + nameToKey.addWithDuplicatesAllowed(identity.name, key, false) + keyToName.addWithDuplicatesAllowed(key, identity.name, false) + val parentId = identityCertChain[1].publicKey.toStringShort() + keyToPartyAndCert[parentId] + } } } @@ -305,13 +327,13 @@ class PersistentIdentityService(cacheFactory: NamedCacheFactory) : SingletonSeri // If we cannot find it then we perform a lookup on the public key to X500 name table val legalIdentity = super.wellKnownPartyFromAnonymous(party) if (legalIdentity == null) { - // If there is no entry in the legal keyToPartyAndCert table then the party must be a confidential identity so we perform - // a lookup in the keyToName table. If an entry for that public key exists, then we attempt look up the associated node's - // PartyAndCertificate. + // If there is no entry in the legal keyToPartyAndCert table then the party must be a confidential identity so we + // perform a lookup in the keyToName table. If an entry for that public key exists, then we attempt look up the + // associated node's PartyAndCertificate. val name = keyToName[party.owningKey.toStringShort()] if (name != null) { - // This should never return null as this node would not be able to communicate with the node providing a confidential - // identity unless its NodeInfo/PartyAndCertificate were available. + // This should never return null as this node would not be able to communicate with the node providing a + // confidential identity unless its NodeInfo/PartyAndCertificate were available. wellKnownPartyFromX500Name(name) } else { null @@ -332,12 +354,14 @@ class PersistentIdentityService(cacheFactory: NamedCacheFactory) : SingletonSeri } @Throws(UnknownAnonymousPartyException::class) - override fun assertOwnership(party: Party, anonymousParty: AnonymousParty) = database.transaction { super.assertOwnership(party, anonymousParty) } + override fun assertOwnership(party: Party, anonymousParty: AnonymousParty) = database.transaction { super.assertOwnership(party, + anonymousParty) } lateinit var ourNames: Set - // Allows us to eliminate keys we know belong to others by using the cache contents that might have been seen during other identity activity. - // Concentrating activity on the identity cache works better than spreading checking across identity and key management, because we cache misses too. + // Allows us to eliminate keys we know belong to others by using the cache contents that might have been seen during other identity + // activity. Concentrating activity on the identity cache works better than spreading checking across identity and key management, + // because we cache misses too. fun stripNotOurKeys(keys: Iterable): Iterable { return keys.filter { (@Suppress("DEPRECATION") certificateFromKey(it))?.name in ourNames } } @@ -351,7 +375,8 @@ class PersistentIdentityService(cacheFactory: NamedCacheFactory) : SingletonSeri if (existingEntryForKey == null) { // Update the three tables as necessary. We definitely store the public key and map it to a party and we optionally update // the public key to external ID mapping table. This block will only ever be reached when registering keys generated on - // other because when a node generates its own keys "registerKeyToParty" is automatically called by KeyManagementService.freshKey. + // other because when a node generates its own keys "registerKeyToParty" is automatically called by + // KeyManagementService.freshKey. registerKeyToParty(publicKey, party) hashToKey[publicKeyHash] = publicKey if (externalId != null) { diff --git a/node/src/main/kotlin/net/corda/node/services/keys/KMSUtils.kt b/node/src/main/kotlin/net/corda/node/services/keys/KMSUtils.kt index d2aaa2b3c9..68ade403b9 100644 --- a/node/src/main/kotlin/net/corda/node/services/keys/KMSUtils.kt +++ b/node/src/main/kotlin/net/corda/node/services/keys/KMSUtils.kt @@ -5,6 +5,7 @@ import net.corda.core.identity.PartyAndCertificate import net.corda.core.internal.CertRole import net.corda.core.node.services.IdentityService import net.corda.core.utilities.days +import net.corda.node.services.api.IdentityServiceInternal import net.corda.nodeapi.internal.crypto.CertificateType import net.corda.nodeapi.internal.crypto.ContentSignerBuilder import net.corda.nodeapi.internal.crypto.X509Utilities @@ -44,7 +45,12 @@ fun freshCertificate(identityService: IdentityService, window) val ourCertPath = X509Utilities.buildCertPath(ourCertificate, issuer.certPath.x509Certificates) val anonymisedIdentity = PartyAndCertificate(ourCertPath) - identityService.verifyAndRegisterIdentity(anonymisedIdentity) + if (identityService is IdentityServiceInternal) { + identityService.justVerifyAndRegisterIdentity(anonymisedIdentity, true) + } else { + identityService.verifyAndRegisterIdentity(anonymisedIdentity) + } + return anonymisedIdentity } diff --git a/node/src/main/resources/migration/node-core.changelog-v14-table.xml b/node/src/main/resources/migration/node-core.changelog-v14-table.xml index b510d6196f..f4f03a1f30 100644 --- a/node/src/main/resources/migration/node-core.changelog-v14-table.xml +++ b/node/src/main/resources/migration/node-core.changelog-v14-table.xml @@ -13,6 +13,8 @@ + \ No newline at end of file