Merge pull request #3054 from corda/pat/registration-tool-bugfix

Bug fix: registration tool doesn't register for service identity if keystore already contains node cert

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

@@ -14,7 +14,7 @@ import net.corda.node.services.config.shouldStartLocalShell
 import net.corda.node.services.config.shouldStartSSHDaemon
 import net.corda.node.services.transactions.bftSMaRtSerialFilter
 import net.corda.node.utilities.registration.HTTPNetworkRegistrationService
-import net.corda.node.utilities.registration.NetworkRegistrationHelper
+import net.corda.node.utilities.registration.NodeRegistrationHelper
 import net.corda.nodeapi.internal.addShutdownHook
 import net.corda.nodeapi.internal.config.UnknownConfigurationKeysException
 import net.corda.tools.shell.InteractiveShell
@@ -192,7 +192,7 @@ open class NodeStartup(val args: Array<String>) {
         println("*       Registering as a new participant with Corda network      *")
         println("*                                                                *")
         println("******************************************************************")
-        NetworkRegistrationHelper(conf, HTTPNetworkRegistrationService(compatibilityZoneURL), nodeRegistrationConfig).buildKeystore()
+        NodeRegistrationHelper(conf, HTTPNetworkRegistrationService(compatibilityZoneURL), nodeRegistrationConfig).buildKeystore()
     }
 
     open protected fun loadConfigFile(cmdlineOptions: CmdLineOptions): NodeConfiguration = cmdlineOptions.loadConfig()

node/src/main/kotlin/net/corda/node/utilities/registration/NetworkRegistrationHelper.kt

@@ -5,7 +5,6 @@ import net.corda.core.identity.CordaX500Name
 import net.corda.core.internal.*
 import net.corda.node.NodeRegistrationOption
 import net.corda.node.services.config.NodeConfiguration
-import net.corda.nodeapi.internal.DevIdentityGenerator
 import net.corda.nodeapi.internal.config.SSLConfiguration
 import net.corda.nodeapi.internal.crypto.CertificateType
 import net.corda.nodeapi.internal.crypto.X509KeyStore
@@ -25,19 +24,17 @@ import java.security.cert.X509Certificate
  * Helper for managing the node registration process, which checks for any existing certificates and requests them if
  * needed.
  */
-class NetworkRegistrationHelper(private val config: SSLConfiguration,
+// TODO: Use content signer instead of keypairs.
-                                private val myLegalName: CordaX500Name,
+open class NetworkRegistrationHelper(private val config: SSLConfiguration,
-                                private val emailAddress: String,
+                                     private val myLegalName: CordaX500Name,
-                                private val certService: NetworkRegistrationService,
+                                     private val emailAddress: String,
-                                private val networkRootTrustStorePath: Path,
+                                     private val certService: NetworkRegistrationService,
-                                networkRootTrustStorePassword: String,
+                                     private val networkRootTrustStorePath: Path,
-                                private val certRole: CertRole) {
+                                     networkRootTrustStorePassword: String,
+                                     private val keyAlias: String,
+                                     private val certRole: CertRole) {
 
-    // Constructor for corda node, cert role is restricted to [CertRole.NODE_CA].
+    companion object {
-    constructor(config: NodeConfiguration, certService: NetworkRegistrationService, regConfig: NodeRegistrationOption) :
-            this(config, config.myLegalName, config.emailAddress, certService, regConfig.networkRootTrustStorePath, regConfig.networkRootTrustStorePassword, CertRole.NODE_CA)
-
-    private companion object {
         const val SELF_SIGNED_PRIVATE_KEY = "Self Signed Private Key"
     }
 
@@ -70,22 +67,13 @@ class NetworkRegistrationHelper(private val config: SSLConfiguration,
     fun buildKeystore() {
         config.certificatesDirectory.createDirectories()
         val nodeKeyStore = config.loadNodeKeyStore(createNew = true)
-        if (CORDA_CLIENT_CA in nodeKeyStore) {
+        if (keyAlias in nodeKeyStore) {
             println("Certificate already exists, Corda node will now terminate...")
             return
         }
 
-        // Create or load self signed keypair from the key store.
+        val keyPair = nodeKeyStore.loadOrCreateKeyPair(SELF_SIGNED_PRIVATE_KEY)
-        // We use the self sign certificate to store the key temporarily in the keystore while waiting for the request approval.
-        if (SELF_SIGNED_PRIVATE_KEY !in nodeKeyStore) {
-            val keyPair = Crypto.generateKeyPair(X509Utilities.DEFAULT_TLS_SIGNATURE_SCHEME)
-            val selfSignCert = X509Utilities.createSelfSignedCACertificate(myLegalName.x500Principal, keyPair)
-            // Save to the key store.
-            nodeKeyStore.setPrivateKey(SELF_SIGNED_PRIVATE_KEY, keyPair.private, listOf(selfSignCert), keyPassword = privateKeyPassword)
-            nodeKeyStore.save()
-        }
 
-        val keyPair = nodeKeyStore.getCertificateAndKeyPair(SELF_SIGNED_PRIVATE_KEY, privateKeyPassword).keyPair
         val requestId = submitOrResumeCertificateSigningRequest(keyPair)
 
         val certificates = try {
@@ -97,11 +85,18 @@ class NetworkRegistrationHelper(private val config: SSLConfiguration,
             requestIdStore.deleteIfExists()
             throw certificateRequestException
         }
+        validateCertificates(certificates)
+        storePrivateKeyWithCertificates(nodeKeyStore, keyPair, certificates, keyAlias)
+        onSuccess(keyPair, certificates)
+        // All done, clean up temp files.
+        requestIdStore.deleteIfExists()
+    }
 
-        val certificate = certificates.first()
+    private fun validateCertificates(certificates: List<X509Certificate>) {
+        val nodeCACertificate = certificates.first()
 
         val nodeCaSubject = try {
-            CordaX500Name.build(certificate.subjectX500Principal)
+            CordaX500Name.build(nodeCACertificate.subjectX500Principal)
         } catch (e: IllegalArgumentException) {
             throw CertificateRequestException("Received node CA cert has invalid subject name: ${e.message}")
         }
@@ -110,56 +105,39 @@ class NetworkRegistrationHelper(private val config: SSLConfiguration,
         }
 
         val nodeCaCertRole = try {
-            CertRole.extract(certificate)
+            CertRole.extract(nodeCACertificate)
         } catch (e: IllegalArgumentException) {
             throw CertificateRequestException("Unable to extract cert role from received node CA cert: ${e.message}")
         }
 
+        if (certRole != nodeCaCertRole) {
+            throw CertificateRequestException("Received certificate contains invalid cert role, expected '$certRole', got '$nodeCaCertRole'.")
+        }
+
         // Validate certificate chain returned from the doorman with the root cert obtained via out-of-band process, to prevent MITM attack on doorman server.
         X509Utilities.validateCertificateChain(rootCert, certificates)
-
         println("Certificate signing request approved, storing private key with the certificate chain.")
+    }
 
-        when (nodeCaCertRole) {
+    private fun storePrivateKeyWithCertificates(nodeKeystore: X509KeyStore, keyPair: KeyPair, certificates: List<X509Certificate>, keyAlias: String) {
-            CertRole.NODE_CA -> {
+        // Save private key and certificate chain to the key store.
-                // Save private key and certificate chain to the key store.
+        nodeKeystore.setPrivateKey(keyAlias, keyPair.private, certificates, keyPassword = config.keyStorePassword)
-                nodeKeyStore.setPrivateKey(CORDA_CLIENT_CA, keyPair.private, certificates, keyPassword = privateKeyPassword)
+        nodeKeystore.internal.deleteEntry(SELF_SIGNED_PRIVATE_KEY)
-                nodeKeyStore.internal.deleteEntry(SELF_SIGNED_PRIVATE_KEY)
+        nodeKeystore.save()
-                nodeKeyStore.save()
+        println("Private key '$keyAlias' and certificate stored in ${config.nodeKeystore}.")
-                println("Node private key and certificate stored in ${config.nodeKeystore}.")
+    }
 
-                config.loadSslKeyStore(createNew = true).update {
+    private fun X509KeyStore.loadOrCreateKeyPair(alias: String): KeyPair {
-                    println("Generating SSL certificate for node messaging service.")
+        // Create or load self signed keypair from the key store.
-                    val sslKeyPair = Crypto.generateKeyPair(X509Utilities.DEFAULT_TLS_SIGNATURE_SCHEME)
+        // We use the self sign certificate to store the key temporarily in the keystore while waiting for the request approval.
-                    val sslCert = X509Utilities.createCertificate(
+        if (alias !in this) {
-                            CertificateType.TLS,
+            val keyPair = Crypto.generateKeyPair(X509Utilities.DEFAULT_TLS_SIGNATURE_SCHEME)
-                            certificate,
+            val selfSignCert = X509Utilities.createSelfSignedCACertificate(myLegalName.x500Principal, keyPair)
-                            keyPair,
+            // Save to the key store.
-                            myLegalName.x500Principal,
+            setPrivateKey(alias, keyPair.private, listOf(selfSignCert), keyPassword = privateKeyPassword)
-                            sslKeyPair.public)
+            save()
-                    setPrivateKey(CORDA_CLIENT_TLS, sslKeyPair.private, listOf(sslCert) + certificates)
-                }
-                println("SSL private key and certificate stored in ${config.sslKeystore}.")
-            }
-            // TODO: Fix this, this is not needed in corda node.
-            CertRole.SERVICE_IDENTITY -> {
-                // Only create keystore containing notary's key for service identity role.
-                nodeKeyStore.setPrivateKey("${DevIdentityGenerator.DISTRIBUTED_NOTARY_ALIAS_PREFIX}-private-key", keyPair.private, certificates, keyPassword = privateKeyPassword)
-                nodeKeyStore.internal.deleteEntry(SELF_SIGNED_PRIVATE_KEY)
-                nodeKeyStore.save()
-                println("Service identity private key and certificate stored in ${config.nodeKeystore}.")
-            }
-            else -> throw CertificateRequestException("Received node CA cert has invalid role: $nodeCaCertRole")
         }
-        // Save root certificates to trust store.
+        return getCertificateAndKeyPair(alias, privateKeyPassword).keyPair
-        config.loadTrustStore(createNew = true).update {
-            println("Generating trust store for corda node.")
-            // Assumes certificate chain always starts with client certificate and end with root certificate.
-            setCertificate(CORDA_ROOT_CA, certificates.last())
-        }
-        println("Node trust store stored in ${config.trustStoreFile}.")
-        // All done, clean up temp files.
-        requestIdStore.deleteIfExists()
     }
 
     /**
@@ -215,4 +193,47 @@ class NetworkRegistrationHelper(private val config: SSLConfiguration,
             requestId
         }
     }
+
+    protected open fun onSuccess(nodeCAKeyPair: KeyPair, certificates: List<X509Certificate>) {}
+}
+
+class NodeRegistrationHelper(private val config: NodeConfiguration, certService: NetworkRegistrationService, regConfig: NodeRegistrationOption) :
+        NetworkRegistrationHelper(config,
+                config.myLegalName,
+                config.emailAddress,
+                certService,
+                regConfig.networkRootTrustStorePath,
+                regConfig.networkRootTrustStorePassword,
+                CORDA_CLIENT_CA,
+                CertRole.NODE_CA) {
+
+    override fun onSuccess(nodeCAKeyPair: KeyPair, certificates: List<X509Certificate>) {
+        createSSLKeystore(nodeCAKeyPair, certificates)
+        createTruststore(certificates.last())
+    }
+
+    private fun createSSLKeystore(nodeCAKeyPair: KeyPair, certificates: List<X509Certificate>) {
+        config.loadSslKeyStore(createNew = true).update {
+            println("Generating SSL certificate for node messaging service.")
+            val sslKeyPair = Crypto.generateKeyPair(X509Utilities.DEFAULT_TLS_SIGNATURE_SCHEME)
+            val sslCert = X509Utilities.createCertificate(
+                    CertificateType.TLS,
+                    certificates.first(),
+                    nodeCAKeyPair,
+                    config.myLegalName.x500Principal,
+                    sslKeyPair.public)
+            setPrivateKey(CORDA_CLIENT_TLS, sslKeyPair.private, listOf(sslCert) + certificates)
+        }
+        println("SSL private key and certificate stored in ${config.sslKeystore}.")
+    }
+
+    private fun createTruststore(rootCertificate: X509Certificate) {
+        // Save root certificates to trust store.
+        config.loadTrustStore(createNew = true).update {
+            println("Generating trust store for corda node.")
+            // Assumes certificate chain always starts with client certificate and end with root certificate.
+            setCertificate(CORDA_ROOT_CA, rootCertificate)
+        }
+        println("Node trust store stored in ${config.trustStoreFile}.")
+    }
 }

node/src/test/kotlin/net/corda/node/utilities/registration/NetworkRegistrationHelperTest.kt

@@ -9,6 +9,7 @@ import com.nhaarman.mockito_kotlin.whenever
 import net.corda.core.crypto.Crypto
 import net.corda.core.crypto.SecureHash
 import net.corda.core.identity.CordaX500Name
+import net.corda.core.internal.CertRole
 import net.corda.core.internal.createDirectories
 import net.corda.core.internal.div
 import net.corda.core.internal.x500Name
@@ -152,11 +153,12 @@ class NetworkRegistrationHelperTest {
         val serviceIdentityCertPath = createServiceIdentityCertPath()
 
         saveNetworkTrustStore(serviceIdentityCertPath.last())
-        createRegistrationHelper(serviceIdentityCertPath).buildKeystore()
+        createRegistrationHelper(serviceIdentityCertPath, CertRole.SERVICE_IDENTITY).buildKeystore()
 
         val nodeKeystore = config.loadNodeKeyStore()
-        val trustStore = config.loadTrustStore()
+
         assertThat(config.sslKeystore).doesNotExist()
+        assertThat(config.trustStoreFile).doesNotExist()
 
         val serviceIdentityAlias = "${DevIdentityGenerator.DISTRIBUTED_NOTARY_ALIAS_PREFIX}-private-key"
 
@@ -167,12 +169,6 @@ class NetworkRegistrationHelperTest {
             assertFalse(contains(X509Utilities.CORDA_CLIENT_CA))
             assertThat(getCertificateChain(serviceIdentityAlias)).containsExactlyElementsOf(serviceIdentityCertPath)
         }
-
-        trustStore.run {
-            assertFalse(contains(X509Utilities.CORDA_CLIENT_CA))
-            assertFalse(contains(X509Utilities.CORDA_INTERMEDIATE_CA))
-            assertThat(getCertificate(X509Utilities.CORDA_ROOT_CA)).isEqualTo(serviceIdentityCertPath.last())
-        }
     }
 
     private fun createNodeCaCertPath(type: CertificateType = CertificateType.NODE_CA,
@@ -203,12 +199,25 @@ class NetworkRegistrationHelperTest {
         return listOf(serviceIdentityCert, intermediateCa.certificate, rootCa.certificate)
     }
 
-    private fun createRegistrationHelper(response: List<X509Certificate>): NetworkRegistrationHelper {
+    private fun createRegistrationHelper(response: List<X509Certificate>, certRole: CertRole = CertRole.NODE_CA): NetworkRegistrationHelper {
         val certService = rigorousMock<NetworkRegistrationService>().also {
             doReturn(requestId).whenever(it).submitRequest(any())
             doReturn(CertificateResponse(5.seconds, response)).whenever(it).retrieveCertificates(eq(requestId))
         }
-        return NetworkRegistrationHelper(config, certService, NodeRegistrationOption(config.certificatesDirectory / networkRootTrustStoreFileName, networkRootTrustStorePassword))
+
+        return when (certRole) {
+            CertRole.NODE_CA -> NodeRegistrationHelper(config, certService, NodeRegistrationOption(config.certificatesDirectory / networkRootTrustStoreFileName, networkRootTrustStorePassword))
+            CertRole.SERVICE_IDENTITY -> NetworkRegistrationHelper(
+                    config,
+                    config.myLegalName,
+                    config.emailAddress,
+                    certService,
+                    config.certificatesDirectory / networkRootTrustStoreFileName,
+                    networkRootTrustStorePassword,
+                    "${DevIdentityGenerator.DISTRIBUTED_NOTARY_ALIAS_PREFIX}-private-key",
+                    CertRole.SERVICE_IDENTITY)
+            else -> throw IllegalArgumentException("Unsupported cert role.")
+        }
     }
 
     private fun saveNetworkTrustStore(rootCert: X509Certificate) {

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

@@ -29,7 +29,7 @@ import net.corda.node.internal.StartedNode
 import net.corda.node.services.Permissions
 import net.corda.node.services.config.*
 import net.corda.node.utilities.registration.HTTPNetworkRegistrationService
-import net.corda.node.utilities.registration.NetworkRegistrationHelper
+import net.corda.node.utilities.registration.NodeRegistrationHelper
 import net.corda.nodeapi.internal.DevIdentityGenerator
 import net.corda.nodeapi.internal.SignedNodeInfo
 import net.corda.nodeapi.internal.addShutdownHook
@@ -247,7 +247,7 @@ class DriverDSLImpl(
 
         return if (startNodesInProcess) {
             executorService.fork {
-                NetworkRegistrationHelper(
+                NodeRegistrationHelper(
                         config.corda,
                         HTTPNetworkRegistrationService(compatibilityZoneURL),
                         NodeRegistrationOption(rootTruststorePath, rootTruststorePassword)
@@ -849,8 +849,7 @@ class DriverDSLImpl(
             val index = stackTrace.indexOfLast { it.className == "net.corda.testing.driver.Driver" }
             // In this case we're dealing with the the RPCDriver or one of it's cousins which are internal and we don't care about them
             if (index == -1) return emptyList()
-            val callerPackage = Class.forName(stackTrace[index + 1].className).`package` ?:
+            val callerPackage = Class.forName(stackTrace[index + 1].className).`package` ?: throw IllegalStateException("Function instantiating driver must be defined in a package.")
-                    throw IllegalStateException("Function instantiating driver must be defined in a package.")
             return listOf(callerPackage.name)
         }
 
@@ -898,16 +897,18 @@ private class NetworkVisibilityController {
             val (snapshot, updates) = rpc.networkMapFeed()
             visibleNodeCount = snapshot.size
             checkIfAllVisible()
-            subscription = updates.subscribe { when (it) {
+            subscription = updates.subscribe {
-                is NetworkMapCache.MapChange.Added -> {
+                when (it) {
-                    visibleNodeCount++
+                    is NetworkMapCache.MapChange.Added -> {
-                    checkIfAllVisible()
+                        visibleNodeCount++
+                        checkIfAllVisible()
+                    }
+                    is NetworkMapCache.MapChange.Removed -> {
+                        visibleNodeCount--
+                        checkIfAllVisible()
+                    }
                 }
-                is NetworkMapCache.MapChange.Removed -> {
+            }
-                    visibleNodeCount--
-                    checkIfAllVisible()
-                }
-            } }
             return future
         }