Generate distributed service certificate properly in keystore (#1529)

* * generate distributed service certificate properly in keystre instead of saving key in flat file when using `generateToDisk`
* move composite key loading hack to devSSL keystore generation process.
* fix and distributed service un-ignore test

* update comment to clarify the composite key certificate creation process.

* fixup after rebase

node/src/integration-test/kotlin/net/corda/node/services/BFTNotaryServiceTests.kt

@@ -37,7 +37,7 @@ import kotlin.test.assertTrue
 
 class BFTNotaryServiceTests {
     companion object {
-        private val clusterName = CordaX500Name(organisation = "BFT", locality = "Zurich", country = "CH")
+        private val clusterName = CordaX500Name(commonName = BFTNonValidatingNotaryService.type.id, organisation = "BFT", locality = "Zurich", country = "CH")
         private val serviceType = BFTNonValidatingNotaryService.type
     }
 
@@ -55,12 +55,11 @@ class BFTNotaryServiceTests {
                 replicaIds.map { mockNet.baseDirectory(mockNet.nextNodeId + it) },
                 serviceType.id,
                 clusterName)
-        val bftNotaryService = ServiceInfo(serviceType)
+        val bftNotaryService = ServiceInfo(serviceType, clusterName)
         val notaryClusterAddresses = replicaIds.map { NetworkHostAndPort("localhost", 11000 + it * 10) }
         replicaIds.forEach { replicaId ->
             mockNet.createNode(
                     node.network.myAddress,
-                    legalName = clusterName.copy(organisation = clusterName.organisation + replicaId),
                     advertisedServices = bftNotaryService,
                     configOverrides = {
                         whenever(it.bftSMaRt).thenReturn(BFTSMaRtConfiguration(replicaId, false, exposeRaces))

node/src/integration-test/kotlin/net/corda/node/services/DistributedServiceTests.kt

@@ -39,7 +39,7 @@ class DistributedServiceTests : DriverBasedTest() {
         )
         val aliceFuture = startNode(providedName = ALICE.name, rpcUsers = listOf(testUser))
         val notariesFuture = startNotaryCluster(
-                DUMMY_NOTARY.name,
+                DUMMY_NOTARY.name.copy(commonName = RaftValidatingNotaryService.type.id),
                 rpcUsers = listOf(testUser),
                 clusterSize = clusterSize,
                 type = RaftValidatingNotaryService.type

node/src/integration-test/kotlin/net/corda/node/services/RaftNotaryServiceTests.kt

@@ -12,22 +12,21 @@ import net.corda.core.internal.concurrent.transpose
 import net.corda.core.transactions.TransactionBuilder
 import net.corda.core.utilities.getOrThrow
 import net.corda.node.internal.StartedNode
+import net.corda.node.services.transactions.RaftValidatingNotaryService
 import net.corda.testing.DUMMY_BANK_A
+import net.corda.testing.chooseIdentity
 import net.corda.testing.contracts.DUMMY_PROGRAM_ID
 import net.corda.testing.contracts.DummyContract
 import net.corda.testing.dummyCommand
-import net.corda.testing.chooseIdentity
 import net.corda.testing.node.NodeBasedTest
-import org.junit.Ignore
 import org.junit.Test
 import java.util.*
 import kotlin.test.assertEquals
 import kotlin.test.assertFailsWith
 
 class RaftNotaryServiceTests : NodeBasedTest() {
-    private val notaryName = CordaX500Name(organisation = "RAFT Notary Service", locality = "London", country = "GB")
+    private val notaryName = CordaX500Name(commonName = RaftValidatingNotaryService.type.id, organisation = "RAFT Notary Service", locality = "London", country = "GB")
 
-    @Ignore
     @Test
     fun `detect double spend`() {
         val (bankA) = listOf(

node/src/integration-test/kotlin/net/corda/services/messaging/P2PMessagingTest.kt

@@ -31,7 +31,7 @@ import java.util.concurrent.atomic.AtomicInteger
 
 class P2PMessagingTest : NodeBasedTest() {
     private companion object {
-        val DISTRIBUTED_SERVICE_NAME = CordaX500Name(organisation = "DistributedService", locality = "London", country = "GB")
+        val DISTRIBUTED_SERVICE_NAME = CordaX500Name(commonName = RaftValidatingNotaryService.type.id, organisation = "DistributedService", locality = "London", country = "GB")
         val SERVICE_2_NAME = CordaX500Name(organisation = "Service 2", locality = "London", country = "GB")
     }
 
@@ -69,23 +69,23 @@ class P2PMessagingTest : NodeBasedTest() {
                 RaftValidatingNotaryService.type.id,
                 DISTRIBUTED_SERVICE_NAME)
 
+        val distributedService = ServiceInfo(RaftValidatingNotaryService.type, DISTRIBUTED_SERVICE_NAME)
         val notaryClusterAddress = freeLocalHostAndPort()
         startNetworkMapNode(
                 DUMMY_MAP.name,
-                advertisedServices = setOf(ServiceInfo(RaftValidatingNotaryService.type, DUMMY_MAP.name.copy(commonName = "DistributedService"))),
+                advertisedServices = setOf(distributedService),
                 configOverrides = mapOf("notaryNodeAddress" to notaryClusterAddress.toString()))
         val (serviceNode2, alice) = listOf(
                 startNode(
                         SERVICE_2_NAME,
-                        advertisedServices = setOf(ServiceInfo(RaftValidatingNotaryService.type, SERVICE_2_NAME.copy(commonName = "DistributedService"))),
+                        advertisedServices = setOf(distributedService),
                         configOverrides = mapOf(
                                 "notaryNodeAddress" to freeLocalHostAndPort().toString(),
                                 "notaryClusterAddresses" to listOf(notaryClusterAddress.toString()))),
                 startNode(ALICE.name)
         ).transpose().getOrThrow()
 
-        val serviceName = serviceNode2.info.legalIdentities[1].name
+        assertAllNodesAreUsed(listOf(networkMapNode, serviceNode2), DISTRIBUTED_SERVICE_NAME, alice)
-        assertAllNodesAreUsed(listOf(networkMapNode, serviceNode2), serviceName, alice)
     }
 
     @Ignore

node/src/main/kotlin/net/corda/node/internal/AbstractNode.kt

@@ -13,10 +13,12 @@ import net.corda.core.flows.ContractUpgradeFlow.Acceptor
 import net.corda.core.identity.CordaX500Name
 import net.corda.core.identity.Party
 import net.corda.core.identity.PartyAndCertificate
-import net.corda.core.internal.*
+import net.corda.core.internal.VisibleForTesting
+import net.corda.core.internal.cert
 import net.corda.core.internal.concurrent.doneFuture
 import net.corda.core.internal.concurrent.flatMap
 import net.corda.core.internal.concurrent.openFuture
+import net.corda.core.internal.toX509CertHolder
 import net.corda.core.messaging.CordaRPCOps
 import net.corda.core.messaging.RPCOps
 import net.corda.core.messaging.SingleMessageRecipient
@@ -36,8 +38,6 @@ import net.corda.node.internal.cordapp.CordappLoader
 import net.corda.node.internal.cordapp.CordappProvider
 import net.corda.node.services.NotaryChangeHandler
 import net.corda.node.services.NotifyTransactionHandler
-import net.corda.nodeapi.ServiceInfo
-import net.corda.nodeapi.ServiceType
 import net.corda.node.services.api.*
 import net.corda.node.services.config.NodeConfiguration
 import net.corda.node.services.config.configureWithDevSSLCertificate
@@ -67,13 +67,14 @@ import net.corda.node.services.vault.NodeVaultService
 import net.corda.node.services.vault.VaultSoftLockManager
 import net.corda.node.utilities.*
 import net.corda.node.utilities.AddOrRemove.ADD
+import net.corda.nodeapi.ServiceInfo
+import net.corda.nodeapi.ServiceType
 import net.corda.nodeapi.internal.serialization.DefaultWhitelist
 import org.apache.activemq.artemis.utils.ReusableLatch
 import org.slf4j.Logger
 import rx.Observable
 import java.io.IOException
 import java.lang.reflect.InvocationTargetException
-import java.nio.file.Path
 import java.security.KeyPair
 import java.security.KeyStoreException
 import java.security.PublicKey
@@ -431,7 +432,13 @@ abstract class AbstractNode(open val configuration: NodeConfiguration,
      * Used only for notary identities.
      */
     protected open fun getNotaryIdentity(): PartyAndCertificate? {
-        return advertisedServices.singleOrNull { it.type.isNotary() }?.let { obtainIdentity(it) }
+        return advertisedServices.singleOrNull { it.type.isNotary() }?.let {
+            it.name?.let {
+                require(it.commonName != null) {"Common name must not be null for notary service, use service type id as common name."}
+                require(ServiceType.parse(it.commonName!!).isNotary()) {"Common name for notary service must be the notary service type id."}
+            }
+            obtainIdentity(it)
+        }
     }
 
     @VisibleForTesting
@@ -630,43 +637,30 @@ abstract class AbstractNode(open val configuration: NodeConfiguration,
             // Create node identity if service info = null
             Pair("identity", myLegalName.copy(commonName = null))
         } else {
-            // Ensure that we always have notary in name and type of it. TODO It is temporary solution until we will have proper handling of NetworkParameters
+            val name = serviceInfo.name ?: myLegalName.copy(commonName = serviceInfo.type.id)
-            val baseName = serviceInfo.name ?: myLegalName
-            val name = if (baseName.commonName == null)
-                baseName.copy(commonName = serviceInfo.type.id)
-            else
-                baseName.copy(commonName = baseName.commonName + " " + serviceInfo.type.id)
             Pair(serviceInfo.type.id, name)
         }
 
         // TODO: Integrate with Key management service?
         val privateKeyAlias = "$id-private-key"
-        val compositeKeyAlias = "$id-composite-key"
 
         if (!keyStore.containsAlias(privateKeyAlias)) {
-            val privKeyFile = configuration.baseDirectory / privateKeyAlias
-            val pubIdentityFile = configuration.baseDirectory / "$id-public"
-            val compositeKeyFile = configuration.baseDirectory / compositeKeyAlias
             // TODO: Remove use of [ServiceIdentityGenerator.generateToDisk].
-            // Get keys from key file.
-            // TODO: this is here to smooth out the key storage transition, remove this migration in future release.
-            if (privKeyFile.exists()) {
-                migrateKeysFromFile(keyStore, name, pubIdentityFile, privKeyFile, compositeKeyFile, privateKeyAlias, compositeKeyAlias)
-            } else {
                 log.info("$privateKeyAlias not found in key store ${configuration.nodeKeystore}, generating fresh key!")
                 keyStore.signAndSaveNewKeyPair(name, privateKeyAlias, generateKeyPair())
-            }
         }
 
         val (x509Cert, keys) = keyStore.certificateAndKeyPair(privateKeyAlias)
 
         // TODO: Use configuration to indicate composite key should be used instead of public key for the identity.
+        val compositeKeyAlias = "$id-composite-key"
         val certificates = if (keyStore.containsAlias(compositeKeyAlias)) {
             // Use composite key instead if it exists
             val certificate = keyStore.getCertificate(compositeKeyAlias)
-            // We have to create the certificate chain for the composite key manually, this is because in order to store
+            // We have to create the certificate chain for the composite key manually, this is because we don't have a keystore
-            // the chain in key store we need a private key, however there is no corresponding private key for the composite key.
+            // provider that understand compositeKey-privateKey combo. The cert chain is created using the composite key certificate +
-            Lists.asList(certificate, keyStore.getCertificateChain(X509Utilities.CORDA_CLIENT_CA))
+            // the tail of the private key certificates, as they are both signed by the same certificate chain.
+            Lists.asList(certificate, keyStore.getCertificateChain(privateKeyAlias).drop(1).toTypedArray())
         } else {
             keyStore.getCertificateChain(privateKeyAlias).let {
                 check(it[0].toX509CertHolder() == x509Cert) { "Certificates from key store do not line up!" }
@@ -683,22 +677,6 @@ abstract class AbstractNode(open val configuration: NodeConfiguration,
         return PartyAndCertificate(CertificateFactory.getInstance("X509").generateCertPath(certificates))
     }
 
-    private fun migrateKeysFromFile(keyStore: KeyStoreWrapper, serviceName: CordaX500Name,
-                                    pubKeyFile: Path, privKeyFile: Path, compositeKeyFile:Path,
-                                    privateKeyAlias: String, compositeKeyAlias: String) {
-        log.info("Migrating $privateKeyAlias from file to key store...")
-        // Check that the identity in the config file matches the identity file we have stored to disk.
-        // Load the private key.
-        val publicKey = Crypto.decodePublicKey(pubKeyFile.readAll())
-        val privateKey = Crypto.decodePrivateKey(privKeyFile.readAll())
-        keyStore.signAndSaveNewKeyPair(serviceName, privateKeyAlias, KeyPair(publicKey, privateKey))
-        // Store composite key separately.
-        if (compositeKeyFile.exists()) {
-            keyStore.savePublicKey(serviceName, compositeKeyAlias, Crypto.decodePublicKey(compositeKeyFile.readAll()))
-        }
-        log.info("Finish migrating $privateKeyAlias from file to keystore.")
-    }
-
     protected open fun generateKeyPair() = cryptoGenerateKeyPair()
 
     private inner class ServiceHubInternalImpl : ServiceHubInternal, SingletonSerializeAsToken() {

node/src/main/kotlin/net/corda/node/services/config/ConfigUtilities.kt

@@ -56,6 +56,22 @@ fun SSLConfiguration.configureDevKeyAndTrustStores(myLegalName: CordaX500Name) {
     if (!sslKeystore.exists() || !nodeKeystore.exists()) {
         val caKeyStore = loadKeyStore(javaClass.classLoader.getResourceAsStream("net/corda/node/internal/certificates/cordadevcakeys.jks"), "cordacadevpass")
         createKeystoreForCordaNode(sslKeystore, nodeKeystore, keyStorePassword, keyStorePassword, caKeyStore, "cordacadevkeypass", myLegalName)
+
+        // Move distributed service composite key (generated by ServiceIdentityGenerator.generateToDisk) to keystore if exists.
+        val distributedServiceKeystore = certificatesDirectory / "distributedService.jks"
+        if (distributedServiceKeystore.exists()) {
+            val serviceKeystore = loadKeyStore(distributedServiceKeystore, "cordacadevpass")
+            val cordaNodeKeystore = loadKeyStore(nodeKeystore, keyStorePassword)
+
+            serviceKeystore.aliases().iterator().forEach {
+                if (serviceKeystore.isKeyEntry(it)) {
+                    cordaNodeKeystore.setKeyEntry(it, serviceKeystore.getKey(it, "cordacadevkeypass".toCharArray()), keyStorePassword.toCharArray(), serviceKeystore.getCertificateChain(it))
+                } else {
+                    cordaNodeKeystore.setCertificateEntry(it, serviceKeystore.getCertificate(it))
+                }
+            }
+            cordaNodeKeystore.save(nodeKeystore, keyStorePassword)
+        }
     }
 }
 

node/src/main/kotlin/net/corda/node/services/keys/PersistentKeyManagementService.kt

@@ -36,7 +36,7 @@ class PersistentKeyManagementService(val identityService: IdentityService,
     class PersistentKey(
 
             @Id
-            @Column(name = "public_key")
+            @Column(length = 6000, name = "public_key")
             var publicKey: String = "",
 
             @Lob

node/src/main/kotlin/net/corda/node/utilities/ServiceIdentityGenerator.kt

@@ -4,6 +4,8 @@ import net.corda.core.crypto.CompositeKey
 import net.corda.core.crypto.generateKeyPair
 import net.corda.core.identity.CordaX500Name
 import net.corda.core.identity.Party
+import net.corda.core.internal.cert
+import net.corda.core.internal.div
 import net.corda.core.utilities.loggerFor
 import net.corda.core.utilities.trace
 import java.nio.file.Files
@@ -30,15 +32,20 @@ object ServiceIdentityGenerator {
         log.trace { "Generating a group identity \"serviceName\" for nodes: ${dirs.joinToString()}" }
         val keyPairs = (1..dirs.size).map { generateKeyPair() }
         val notaryKey = CompositeKey.Builder().addKeys(keyPairs.map { it.public }).build(threshold)
-        // Avoid adding complexity! This class is a hack that needs to stay runnable in the gradle environment.
+
-        val privateKeyFile = "$serviceId-private-key"
+        val caKeyStore = loadKeyStore(javaClass.classLoader.getResourceAsStream("net/corda/node/internal/certificates/cordadevcakeys.jks"), "cordacadevpass")
-        val publicKeyFile = "$serviceId-public"
+        val issuer = caKeyStore.getCertificateAndKeyPair(X509Utilities.CORDA_INTERMEDIATE_CA, "cordacadevkeypass")
-        val compositeKeyFile = "$serviceId-composite-key"
+        val rootCert = caKeyStore.getCertificate(X509Utilities.CORDA_ROOT_CA)
+
         keyPairs.zip(dirs) { keyPair, dir ->
-            Files.createDirectories(dir)
+            val serviceKeyCert = X509Utilities.createCertificate(CertificateType.CLIENT_CA, issuer.certificate, issuer.keyPair, serviceName, keyPair.public)
-            Files.write(dir.resolve(compositeKeyFile), notaryKey.encoded)
+            val compositeKeyCert = X509Utilities.createCertificate(CertificateType.CLIENT_CA, issuer.certificate, issuer.keyPair, serviceName, notaryKey)
-            Files.write(dir.resolve(privateKeyFile), keyPair.private.encoded)
+            val certPath = Files.createDirectories(dir / "certificates") / "distributedService.jks"
-            Files.write(dir.resolve(publicKeyFile), keyPair.public.encoded)
+
+            val keystore = loadOrCreateKeyStore(certPath, "cordacadevpass")
+            keystore.setCertificateEntry("$serviceId-composite-key", compositeKeyCert.cert)
+            keystore.setKeyEntry("$serviceId-private-key", keyPair.private, "cordacadevkeypass".toCharArray(), arrayOf(serviceKeyCert.cert, issuer.certificate.cert, rootCert))
+            keystore.save(certPath, "cordacadevpass")
         }
         return Party(serviceName, notaryKey)
     }

node/src/test/kotlin/net/corda/node/services/statemachine/FlowFrameworkTests.kt

@@ -31,10 +31,10 @@ import net.corda.finance.flows.CashIssueFlow
 import net.corda.finance.flows.CashPaymentFlow
 import net.corda.node.internal.InitiatedFlowFactory
 import net.corda.node.internal.StartedNode
-import net.corda.nodeapi.ServiceInfo
 import net.corda.node.services.network.NetworkMapService
 import net.corda.node.services.persistence.checkpoints
 import net.corda.node.services.transactions.ValidatingNotaryService
+import net.corda.nodeapi.ServiceInfo
 import net.corda.testing.*
 import net.corda.testing.contracts.DUMMY_PROGRAM_ID
 import net.corda.testing.contracts.DummyState
@@ -85,7 +85,7 @@ class FlowFrameworkTests {
 
         // We intentionally create our own notary and ignore the one provided by the network
         val notaryKeyPair = generateKeyPair()
-        val notaryService = ServiceInfo(ValidatingNotaryService.type, CordaX500Name(organisation = "Notary service 2000", locality = "London", country = "GB"))
+        val notaryService = ServiceInfo(ValidatingNotaryService.type, CordaX500Name(commonName = ValidatingNotaryService.type.id, organisation = "Notary service 2000", locality = "London", country = "GB"))
         val overrideServices = mapOf(Pair(notaryService, notaryKeyPair))
         // Note that these notaries don't operate correctly as they don't share their state. They are only used for testing
         // service addressing.

testing/node-driver/src/main/kotlin/net/corda/testing/driver/Driver.kt

@@ -729,7 +729,7 @@ class DriverDSL(
         val nodeNames = (0 until clusterSize).map { CordaX500Name(organisation = "Notary Service $it", locality = "Zurich", country = "CH") }
         val paths = nodeNames.map { baseDirectory(it) }
         ServiceIdentityGenerator.generateToDisk(paths, type.id, notaryName)
-        val advertisedServices = setOf(ServiceInfo(type))
+        val advertisedServices = setOf(ServiceInfo(type, notaryName))
         val notaryClusterAddress = portAllocation.nextHostAndPort()
 
         // Start the first node that will bootstrap the cluster

testing/node-driver/src/main/kotlin/net/corda/testing/node/NodeBasedTest.kt

@@ -136,20 +136,19 @@ abstract class NodeBasedTest : TestDependencyInjectionBase() {
                 serviceType.id,
                 notaryName)
 
+        val serviceInfo = ServiceInfo(serviceType, notaryName)
         val nodeAddresses = getFreeLocalPorts("localhost", clusterSize).map { it.toString() }
 
-        val masterNode = CordaX500Name(organisation = "${notaryName.organisation}-0", locality = notaryName.locality, country = notaryName.country)
         val masterNodeFuture = startNode(
-                masterNode,
+                CordaX500Name(organisation = "${notaryName.organisation}-0", locality = notaryName.locality, country = notaryName.country),
-                advertisedServices = setOf(ServiceInfo(serviceType, masterNode.copy(commonName = serviceType.id))),
+                advertisedServices = setOf(serviceInfo),
                 configOverrides = mapOf("notaryNodeAddress" to nodeAddresses[0],
                         "database" to mapOf("serverNameTablePrefix" to if (clusterSize > 1) "${notaryName.organisation}0".replace(Regex("[^0-9A-Za-z]+"), "") else "")))
 
         val remainingNodesFutures = (1 until clusterSize).map {
-            val nodeName = CordaX500Name(organisation = "${notaryName.organisation}-$it", locality = notaryName.locality, country = notaryName.country)
             startNode(
-                    nodeName,
+                    CordaX500Name(organisation = "${notaryName.organisation}-$it", locality = notaryName.locality, country = notaryName.country),
-                    advertisedServices = setOf(ServiceInfo(serviceType, nodeName.copy(commonName = serviceType.id))),
+                    advertisedServices = setOf(serviceInfo),
                     configOverrides = mapOf(
                             "notaryNodeAddress" to nodeAddresses[it],
                             "notaryClusterAddresses" to listOf(nodeAddresses[0]),