Adding public key constraint (#319)

* ENT-1356 Adding public key constraint

* Addressing review comments

* Removing SERIALIZABLE from transaction

* Adding stashed changes

* Removing SERIALIZABLE from node info storage

* Addressing review comments

* Addressing possible certificate inconsitency (design gap) + clearing whole database for new liquibase changeset

* Addressing review comments

network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/CertificationRequestStorage.kt

@@ -1,12 +1,14 @@
 package com.r3.corda.networkmanage.common.persistence
 
+import net.corda.core.crypto.SecureHash
 import org.bouncycastle.pkcs.PKCS10CertificationRequest
 import java.security.cert.CertPath
 
-data class CertificateData(val publicKeyHash: String, val certStatus: CertificateStatus, val certPath: CertPath)
+data class CertificateData(val certStatus: CertificateStatus, val certPath: CertPath)
 
 data class CertificateSigningRequest(val requestId: String,
                                      val legalName: String,
+                                     val publicKeyHash: SecureHash,
                                      val status: RequestStatus,
                                      val request: PKCS10CertificationRequest,
                                      val remark: String?,

network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/PersistentCertificateRequestStorage.kt

@@ -3,6 +3,7 @@ package com.r3.corda.networkmanage.common.persistence
 import com.r3.corda.networkmanage.common.persistence.entity.CertificateDataEntity
 import com.r3.corda.networkmanage.common.persistence.entity.CertificateSigningRequestEntity
 import com.r3.corda.networkmanage.common.utils.hashString
+import net.corda.core.crypto.Crypto.toSupportedPublicKey
 import net.corda.core.crypto.SecureHash
 import net.corda.core.identity.CordaX500Name
 import net.corda.nodeapi.internal.persistence.CordaPersistence
@@ -26,14 +27,12 @@ class PersistentCertificateRequestStorage(private val database: CordaPersistence
                 builder.and(requestIdEq, statusEq)
             }
             request ?: throw IllegalArgumentException("Cannot retrieve 'APPROVED' certificate signing request for request id: $requestId")
-            val publicKeyHash = certificates.certificates.first().publicKey.hashString()
             val certificateSigningRequest = request.copy(
                     modifiedBy = signedBy,
                     modifiedAt = Instant.now(),
                     status = RequestStatus.SIGNED)
             session.merge(certificateSigningRequest)
             val certificateDataEntity = CertificateDataEntity(
-                    publicKeyHash = publicKeyHash,
                     certificateStatus = CertificateStatus.VALID,
                     certificatePathBytes = certificates.encoded,
                     certificateSigningRequest = certificateSigningRequest)
@@ -48,6 +47,7 @@ class PersistentCertificateRequestStorage(private val database: CordaPersistence
             session.save(CertificateSigningRequestEntity(
                     requestId = requestId,
                     legalName = legalName,
+                    publicKeyHash = toSupportedPublicKey(request.subjectPublicKeyInfo).hashString(),
                     requestBytes = request.encoded,
                     remark = rejectReason,
                     modifiedBy = emptyList(),
@@ -134,7 +134,7 @@ class PersistentCertificateRequestStorage(private val database: CordaPersistence
             return Pair(request.subject.toString(), "Name validation failed: ${e.message}")
         }
 
-        val query = session.criteriaBuilder.run {
+        val duplicateNameQuery = session.criteriaBuilder.run {
             val criteriaQuery = createQuery(CertificateSigningRequestEntity::class.java)
             criteriaQuery.from(CertificateSigningRequestEntity::class.java).run {
                 criteriaQuery.where(equal(get<String>(CertificateSigningRequestEntity::legalName.name), legalName))
@@ -144,10 +144,27 @@ class PersistentCertificateRequestStorage(private val database: CordaPersistence
         // TODO consider scenario: There is a CSR that is signed but the certificate itself has expired or was revoked
         // Also, at the moment we assume that once the CSR is approved it cannot be rejected.
         // What if we approved something by mistake.
-        val duplicates = session.createQuery(query).resultList.filter {
+        val nameDuplicates = session.createQuery(duplicateNameQuery).resultList.filter {
             it.status != RequestStatus.REJECTED
         }
 
-        return Pair(legalName, if (duplicates.isEmpty()) null else "Duplicate legal name")
+        if (nameDuplicates.isNotEmpty()) {
+            return Pair(legalName, "Duplicate legal name")
+        }
+
+        val publicKey = toSupportedPublicKey(request.subjectPublicKeyInfo).hashString()
+        val duplicatePkQuery = session.criteriaBuilder.run {
+            val criteriaQuery = createQuery(CertificateSigningRequestEntity::class.java)
+            criteriaQuery.from(CertificateSigningRequestEntity::class.java).run {
+                criteriaQuery.where(equal(get<String>(CertificateSigningRequestEntity::publicKeyHash.name), publicKey))
+            }
+        }
+
+        //TODO Consider following scenario: There is a CSR that is signed but the certificate itself has expired or was revoked
+        val pkDuplicates = session.createQuery(duplicatePkQuery).resultList.filter {
+            it.status != RequestStatus.REJECTED
+        }
+
+        return Pair(legalName, if (pkDuplicates.isEmpty()) null else "Duplicate public key")
     }
 }

network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/PersistentNodeInfoStorage.kt

@@ -1,6 +1,5 @@
 package com.r3.corda.networkmanage.common.persistence
 
-import com.r3.corda.networkmanage.common.persistence.entity.CertificateDataEntity
 import com.r3.corda.networkmanage.common.persistence.entity.CertificateSigningRequestEntity
 import com.r3.corda.networkmanage.common.persistence.entity.NodeInfoEntity
 import com.r3.corda.networkmanage.common.utils.buildCertPath
@@ -10,7 +9,7 @@ import net.corda.core.internal.CertRole
 import net.corda.core.serialization.serialize
 import net.corda.nodeapi.internal.SignedNodeInfo
 import net.corda.nodeapi.internal.persistence.CordaPersistence
-import net.corda.nodeapi.internal.persistence.TransactionIsolationLevel
+import net.corda.nodeapi.internal.persistence.DatabaseTransaction
 import java.security.cert.CertPath
 
 /**
@@ -21,16 +20,13 @@ class PersistentNodeInfoStorage(private val database: CordaPersistence) : NodeIn
         val nodeInfo = nodeInfoWithSigned.nodeInfo
         val signedNodeInfo = nodeInfoWithSigned.signedNodeInfo
         val nodeCaCert = nodeInfo.legalIdentitiesAndCerts[0].certPath.certificates.find { CertRole.extract(it) == CertRole.NODE_CA }
-        return database.transaction(TransactionIsolationLevel.SERIALIZABLE) {
+        return database.transaction {
             // TODO Move these checks out of data access layer
             val request = nodeCaCert?.let {
-                singleRequestWhere(CertificateDataEntity::class.java) { builder, path ->
+                getSignedRequestByPublicHash(it.publicKey.encoded.sha256(), this)
-                    val certPublicKeyHashEq = builder.equal(path.get<String>(CertificateDataEntity::publicKeyHash.name), it.publicKey.encoded.sha256().toString())
-                    val certStatusValid = builder.equal(path.get<CertificateStatus>(CertificateDataEntity::certificateStatus.name), CertificateStatus.VALID)
-                    builder.and(certPublicKeyHashEq, certStatusValid)
-                }
             }
             request ?: throw IllegalArgumentException("Unknown node info, this public key is not registered with the network management service.")
+            require(request.certificateData!!.certificateStatus == CertificateStatus.VALID) { "Certificate is no longer valid" }
 
             /*
              * Delete any previous [NodeInfoEntity] instance for this CSR
@@ -40,13 +36,13 @@ class PersistentNodeInfoStorage(private val database: CordaPersistence) : NodeIn
              * but it has been confirmed that this fact has been acknowledged at the design time and we are fine with it.
              */
             deleteRequest(NodeInfoEntity::class.java) { builder, path ->
-                builder.equal(path.get<CertificateSigningRequestEntity>(NodeInfoEntity::certificateSigningRequest.name), request.certificateSigningRequest)
+                builder.equal(path.get<CertificateSigningRequestEntity>(NodeInfoEntity::certificateSigningRequest.name), request)
             }
             val hash = signedNodeInfo.raw.hash
 
             val hashedNodeInfo = NodeInfoEntity(
                     nodeInfoHash = hash.toString(),
-                    certificateSigningRequest = request.certificateSigningRequest,
+                    certificateSigningRequest = request,
                     signedNodeInfoBytes = signedNodeInfo.serialize().bytes)
             session.save(hashedNodeInfo)
             hash
@@ -61,16 +57,16 @@ class PersistentNodeInfoStorage(private val database: CordaPersistence) : NodeIn
 
     override fun getCertificatePath(publicKeyHash: SecureHash): CertPath? {
         return database.transaction {
-            val builder = session.criteriaBuilder
+            val request = getSignedRequestByPublicHash(publicKeyHash, this)
-            val query = builder.createQuery(ByteArray::class.java).run {
+            request?.let { buildCertPath(it.certificateData!!.certificatePathBytes) }
-                from(CertificateSigningRequestEntity::class.java).run {
+        }
-                    select(get<CertificateDataEntity>(CertificateSigningRequestEntity::certificateData.name)
+    }
-                            .get<ByteArray>(CertificateDataEntity::certificatePathBytes.name))
+
-                    where(builder.equal(get<CertificateDataEntity>(CertificateSigningRequestEntity::certificateData.name)
+    private fun getSignedRequestByPublicHash(publicKeyHash: SecureHash, transaction: DatabaseTransaction): CertificateSigningRequestEntity? {
-                            .get<String>(CertificateDataEntity::publicKeyHash.name), publicKeyHash.toString()))
+        return transaction.singleRequestWhere(CertificateSigningRequestEntity::class.java) { builder, path ->
-                }
+            val publicKeyEq = builder.equal(path.get<String>(CertificateSigningRequestEntity::publicKeyHash.name), publicKeyHash.toString())
-            }
+            val statusEq = builder.equal(path.get<RequestStatus>(CertificateSigningRequestEntity::status.name), RequestStatus.SIGNED)
-            session.createQuery(query).uniqueResultOptional().orElseGet { null }?.let { buildCertPath(it) }
+            builder.and(publicKeyEq, statusEq)
         }
     }
 }

network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/entity/CertificateSigningRequestEntity.kt

@@ -4,6 +4,7 @@ import com.r3.corda.networkmanage.common.persistence.CertificateData
 import com.r3.corda.networkmanage.common.persistence.CertificateSigningRequest
 import com.r3.corda.networkmanage.common.persistence.CertificateStatus
 import com.r3.corda.networkmanage.common.persistence.RequestStatus
+import net.corda.core.crypto.SecureHash
 import org.bouncycastle.pkcs.PKCS10CertificationRequest
 import org.hibernate.envers.Audited
 import java.security.cert.CertPath
@@ -12,7 +13,7 @@ import java.time.Instant
 import javax.persistence.*
 
 @Entity
-@Table(name = "certificate_signing_request")
+@Table(name = "certificate_signing_request", indexes = arrayOf(Index(name = "IDX_PUB_KEY_HASH", columnList = "public_key_hash")))
 class CertificateSigningRequestEntity(
         @Id
         @Column(name = "request_id", length = 64)
@@ -22,6 +23,9 @@ class CertificateSigningRequestEntity(
         @Column(name = "legal_name", length = 256, nullable = false)
         val legalName: String,
 
+        @Column(name = "public_key_hash", length = 64)
+        val publicKeyHash: String,
+
         @Audited
         @Column(name = "status", nullable = false)
         @Enumerated(EnumType.STRING)
@@ -50,6 +54,7 @@ class CertificateSigningRequestEntity(
     fun toCertificateSigningRequest() = CertificateSigningRequest(
             requestId = requestId,
             legalName = legalName,
+            publicKeyHash = SecureHash.parse(publicKeyHash),
             status = status,
             request = request(),
             remark = remark,
@@ -59,6 +64,7 @@ class CertificateSigningRequestEntity(
 
     fun copy(requestId: String = this.requestId,
              legalName: String = this.legalName,
+             publicKeyHash: String = this.publicKeyHash,
              status: RequestStatus = this.status,
              modifiedBy: List<String> = this.modifiedBy,
              modifiedAt: Instant = this.modifiedAt,
@@ -69,6 +75,7 @@ class CertificateSigningRequestEntity(
         return CertificateSigningRequestEntity(
                 requestId = requestId,
                 legalName = legalName,
+                publicKeyHash = publicKeyHash,
                 status = status,
                 modifiedAt = modifiedAt,
                 modifiedBy = modifiedBy,
@@ -82,16 +89,13 @@ class CertificateSigningRequestEntity(
 }
 
 @Entity
-@Table(name = "certificate_data", indexes = arrayOf(Index(name = "IDX_PUB_KEY_HASH", columnList = "public_key_hash")))
+@Table(name = "certificate_data")
 class CertificateDataEntity(
 
         @Id
         @GeneratedValue(strategy = GenerationType.SEQUENCE)
         val id: Long? = null,
 
-        @Column(name = "public_key_hash", length = 64)
-        val publicKeyHash: String,
-
         @Column(name = "certificate_status")
         val certificateStatus: CertificateStatus,
 
@@ -105,7 +109,6 @@ class CertificateDataEntity(
 ) {
     fun toCertificateData(): CertificateData {
         return CertificateData(
-                publicKeyHash = publicKeyHash,
                 certStatus = certificateStatus,
                 certPath = toCertificatePath()
         )

network-management/src/main/resources/migration/network-manager.changelog-master.xml

@@ -3,5 +3,6 @@
 
     <include file="migration/network-manager.changelog-init.xml"/>
     <include file="migration/network-manager.changelog-signing-network-params.xml"/>
+    <include file="migration/network-manager.changelog-pub-key-move.xml"/>
 
 </databaseChangeLog>

network-management/src/main/resources/migration/network-manager.changelog-pub-key-move.xml

@@ -0,0 +1,22 @@
+<?xml version="1.1" encoding="UTF-8" standalone="no"?>
+<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog" xmlns:ext="http://www.liquibase.org/xml/ns/dbchangelog-ext" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog-ext http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-ext.xsd http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-3.5.xsd">
+    <changeSet author="R3.Corda" id="Move public key hash">
+        <delete tableName="certificate_signing_request"/>
+        <delete tableName="certificate_data"/>
+        <addColumn tableName="certificate_signing_request">
+            <column name="public_key_hash" type="NVARCHAR(64)"/>
+        </addColumn>
+        <dropIndex indexName="IDX_PUB_KEY_HASH" tableName="certificate_data"/>
+        <createIndex indexName="IDX_PUB_KEY_HASH" tableName="certificate_signing_request">
+            <column name="public_key_hash"/>
+        </createIndex>
+        <dropColumn columnName="public_key_hash" tableName="certificate_data"/>
+        <delete tableName="CertificateSigningRequestEntity_modifiedBy"/>
+        <delete tableName="CertificateSigningRequestEntity_modifiedBy_AUD"/>
+        <delete tableName="certificate_signing_request_AUD"/>
+        <delete tableName="network_map"/>
+        <delete tableName="network_parameters"/>
+        <delete tableName="node_info"/>
+        <delete tableName="REVINFO"/>
+    </changeSet>
+</databaseChangeLog>

network-management/src/test/kotlin/com/r3/corda/networkmanage/TestBase.kt

@@ -20,6 +20,7 @@ abstract class TestBase {
             requestId: String = SecureHash.randomSHA256().toString(),
             status: RequestStatus = RequestStatus.NEW,
             legalName: String = "TestLegalName",
+            publicKeyHash: SecureHash = SecureHash.randomSHA256(),
             remark: String = "Test remark",
             request: PKCS10CertificationRequest = mock(),
             certData: CertificateData = mock(),
@@ -29,6 +30,7 @@ abstract class TestBase {
                 requestId = requestId,
                 status = status,
                 legalName = legalName,
+                publicKeyHash = publicKeyHash,
                 remark = remark,
                 certData = certData,
                 request = request,
@@ -36,11 +38,9 @@ abstract class TestBase {
         )
     }
 
-    protected fun certificateData(publicKeyHash: String = SecureHash.randomSHA256().toString(),
+    protected fun certificateData(certStatus: CertificateStatus = CertificateStatus.VALID,
-                                  certStatus: CertificateStatus = CertificateStatus.VALID,
                                   certPath: CertPath = mock()): CertificateData {
         return CertificateData(
-                publicKeyHash = publicKeyHash,
                 certStatus = certStatus,
                 certPath = certPath
         )

network-management/src/test/kotlin/com/r3/corda/networkmanage/common/persistence/PersistentCertificateRequestStorageTest.kt

@@ -19,6 +19,7 @@ import org.junit.After
 import org.junit.Before
 import org.junit.Test
 import java.security.KeyPair
+import java.security.cert.CertPath
 import java.util.*
 import javax.security.auth.x500.X500Principal
 import kotlin.test.*
@@ -69,7 +70,7 @@ class PersistentCertificateRequestStorageTest : TestBase() {
 
     @Test
     fun `sign request`() {
-        val (csr, _) = createRequest("LegalName")
+        val (csr, nodeKeyPair) = createRequest("LegalName")
         // Add request to DB.
         val requestId = storage.saveRequest(csr)
         // New request should equals to 1.
@@ -86,11 +87,7 @@ class PersistentCertificateRequestStorageTest : TestBase() {
         // Sign certificate
         storage.putCertificatePath(
                 requestId,
-                JcaPKCS10CertificationRequest(csr).run {
+                generateSignedCertPath(csr, nodeKeyPair),
-                    // TODO We need a utility in InternalUtils for converting X500Name -> CordaX500Name
-                    val (rootCa, intermediateCa, nodeCa) = createDevNodeCaCertPath(CordaX500Name.build(X500Principal(subject.encoded)))
-                    buildCertPath(nodeCa.certificate, intermediateCa.certificate, rootCa.certificate)
-                },
                 listOf(DOORMAN_SIGNATURE)
         )
         // Check request is ready
@@ -99,33 +96,51 @@ class PersistentCertificateRequestStorageTest : TestBase() {
 
     @Test
     fun `sign request ignores subsequent sign requests`() {
-        val (csr, _) = createRequest("LegalName")
+        val (csr, nodeKeyPair) = createRequest("LegalName")
         // Add request to DB.
         val requestId = storage.saveRequest(csr)
         // Store certificate to DB.
         storage.markRequestTicketCreated(requestId)
         storage.approveRequest(requestId, DOORMAN_SIGNATURE)
+        // Sign certificate
         storage.putCertificatePath(
                 requestId,
-                JcaPKCS10CertificationRequest(csr).run {
+                generateSignedCertPath(csr, nodeKeyPair),
-                    val (rootCa, intermediateCa, nodeCa) = createDevNodeCaCertPath(CordaX500Name.build(X500Principal(subject.encoded)))
-                    buildCertPath(nodeCa.certificate, intermediateCa.certificate, rootCa.certificate)
-                },
                 listOf(DOORMAN_SIGNATURE)
         )
-        // Sign certificate
         // When subsequent signature requested
         assertFailsWith(IllegalArgumentException::class) {
             storage.putCertificatePath(
                     requestId,
-                    JcaPKCS10CertificationRequest(csr).run {
+                    generateSignedCertPath(csr, nodeKeyPair),
-                        val (rootCa, intermediateCa, nodeCa) = createDevNodeCaCertPath(CordaX500Name.build(X500Principal(subject.encoded)))
-                        buildCertPath(nodeCa.certificate, intermediateCa.certificate, rootCa.certificate)
-                    },
                     listOf(DOORMAN_SIGNATURE))
         }
     }
 
+    @Test
+    fun `sign request rejects requests with the same public key`() {
+        val (csr, nodeKeyPair) = createRequest("LegalName")
+        // Add request to DB.
+        val requestId = storage.saveRequest(csr)
+        // Store certificate to DB.
+        storage.markRequestTicketCreated(requestId)
+        storage.approveRequest(requestId, DOORMAN_SIGNATURE)
+        // Sign certificate
+        storage.putCertificatePath(
+                requestId,
+                generateSignedCertPath(csr, nodeKeyPair),
+                listOf(DOORMAN_SIGNATURE)
+        )
+        // Sign certificate
+        // When request with the same public key is requested
+        val (newCsr, _) = createRequest("NewLegalName", nodeKeyPair)
+        val duplicateRequestId = storage.saveRequest(newCsr)
+        assertThat(storage.getRequests(RequestStatus.NEW)).isEmpty()
+        val duplicateRequest = storage.getRequest(duplicateRequestId)
+        assertThat(duplicateRequest!!.status).isEqualTo(RequestStatus.REJECTED)
+        assertThat(duplicateRequest.remark).isEqualTo("Duplicate public key")
+    }
+
     @Test
     fun `reject request`() {
         val requestId = storage.saveRequest(createRequest("BankA").first)
@@ -159,18 +174,14 @@ class PersistentCertificateRequestStorageTest : TestBase() {
 
     @Test
     fun `request with the same legal name as a previously signed request`() {
-        val csr = createRequest("BankA").first
+        val (csr, nodeKeyPair) = createRequest("BankA")
         val requestId = storage.saveRequest(csr)
         storage.markRequestTicketCreated(requestId)
         storage.approveRequest(requestId, DOORMAN_SIGNATURE)
         // Sign certificate
         storage.putCertificatePath(
                 requestId,
-                JcaPKCS10CertificationRequest(csr).run {
+                generateSignedCertPath(csr, nodeKeyPair),
-                    // TODO We need a utility in InternalUtils for converting X500Name -> CordaX500Name
-                    val (rootCa, intermediateCa, nodeCa) = createDevNodeCaCertPath(CordaX500Name.build(X500Principal(subject.encoded)))
-                    buildCertPath(nodeCa.certificate, intermediateCa.certificate, rootCa.certificate)
-                },
                 listOf(DOORMAN_SIGNATURE)
         )
         val rejectedRequestId = storage.saveRequest(createRequest("BankA").first)
@@ -215,6 +226,14 @@ class PersistentCertificateRequestStorageTest : TestBase() {
         }
     }
 
+    private fun generateSignedCertPath(csr: PKCS10CertificationRequest, keyPair: KeyPair): CertPath {
+        return JcaPKCS10CertificationRequest(csr).run {
+            // TODO We need a utility in InternalUtils for converting X500Name -> CordaX500Name
+            val (rootCa, intermediateCa, nodeCa) = createDevNodeCaCertPath(CordaX500Name.build(X500Principal(subject.encoded)), keyPair)
+            buildCertPath(nodeCa.certificate, intermediateCa.certificate, rootCa.certificate)
+        }
+    }
+
     private fun makeTestDataSourceProperties(nodeName: String = SecureHash.randomSHA256().toString()): Properties {
         val props = Properties()
         props.setProperty("dataSourceClassName", "org.h2.jdbcx.JdbcDataSource")
@@ -225,8 +244,7 @@ class PersistentCertificateRequestStorageTest : TestBase() {
     }
 }
 
-internal fun createRequest(organisation: String): Pair<PKCS10CertificationRequest, KeyPair> {
+internal fun createRequest(organisation: String, keyPair: KeyPair = Crypto.generateKeyPair(X509Utilities.DEFAULT_TLS_SIGNATURE_SCHEME)): Pair<PKCS10CertificationRequest, KeyPair> {
-    val keyPair = Crypto.generateKeyPair(X509Utilities.DEFAULT_TLS_SIGNATURE_SCHEME)
     val request = X509Utilities.createCertificateSigningRequest(X500Principal("O=$organisation,L=London,C=GB"), "my@mail.com", keyPair)
     return Pair(request, keyPair)
-}
+}

network-management/src/test/kotlin/com/r3/corda/networkmanage/common/persistence/PersistentNetworkMapStorageTest.kt

@@ -5,13 +5,11 @@ import net.corda.core.identity.CordaX500Name
 import net.corda.core.internal.signWithCert
 import net.corda.nodeapi.internal.createDevNetworkMapCa
 import net.corda.nodeapi.internal.crypto.CertificateAndKeyPair
-import net.corda.nodeapi.internal.crypto.X509CertificateFactory
 import net.corda.nodeapi.internal.network.NetworkMap
 import net.corda.nodeapi.internal.network.verifiedNetworkMapCert
 import net.corda.nodeapi.internal.persistence.CordaPersistence
 import net.corda.nodeapi.internal.persistence.DatabaseConfig
 import net.corda.testing.common.internal.testNetworkParameters
-import net.corda.testing.internal.TestNodeInfoBuilder
 import net.corda.testing.internal.createDevIntermediateCaCertPath
 import net.corda.testing.node.MockServices.Companion.makeTestDataSourceProperties
 import org.assertj.core.api.Assertions.assertThat
@@ -50,7 +48,7 @@ class PersistentNetworkMapStorageTest : TestBase() {
     fun `saveNetworkMap and saveNetworkParameters create current network map and parameters`() {
         // given
         // Create node info.
-        val signedNodeInfo = createValidSignedNodeInfo("Test")
+        val (signedNodeInfo) = createValidSignedNodeInfo("Test", requestStorage)
         val nodeInfoHash = nodeInfoStorage.putNodeInfo(signedNodeInfo)
 
         val networkParameters = testNetworkParameters(emptyList())
@@ -117,8 +115,8 @@ class PersistentNetworkMapStorageTest : TestBase() {
     fun `getValidNodeInfoHashes returns only valid and signed node info hashes`() {
         // given
         // Create node infos.
-        val signedNodeInfoA = createValidSignedNodeInfo("TestA")
+        val (signedNodeInfoA) = createValidSignedNodeInfo("TestA", requestStorage)
-        val signedNodeInfoB = createValidSignedNodeInfo("TestB")
+        val (signedNodeInfoB) = createValidSignedNodeInfo("TestB", requestStorage)
 
         // Put signed node info data
         val nodeInfoHashA = nodeInfoStorage.putNodeInfo(signedNodeInfoA)
@@ -139,15 +137,4 @@ class PersistentNetworkMapStorageTest : TestBase() {
         // then
         assertThat(validNodeInfoHash).containsOnly(nodeInfoHashA, nodeInfoHashB)
     }
-
-    private fun createValidSignedNodeInfo(organisation: String): NodeInfoWithSigned {
-        val nodeInfoBuilder = TestNodeInfoBuilder()
-        val requestId = requestStorage.saveRequest(createRequest(organisation).first)
-        requestStorage.markRequestTicketCreated(requestId)
-        requestStorage.approveRequest(requestId, "TestUser")
-        val (identity) = nodeInfoBuilder.addIdentity(CordaX500Name(organisation, "London", "GB"))
-        val nodeCaCertPath = X509CertificateFactory().generateCertPath(identity.certPath.certificates.drop(1))
-        requestStorage.putCertificatePath(requestId, nodeCaCertPath, emptyList())
-        return NodeInfoWithSigned(nodeInfoBuilder.buildWithSigned().second)
-    }
 }

network-management/src/test/kotlin/com/r3/corda/networkmanage/common/persistence/PersistentNodeInfoStorageTest.kt

@@ -9,7 +9,6 @@ import net.corda.core.identity.CordaX500Name
 import net.corda.core.serialization.serialize
 import net.corda.nodeapi.internal.crypto.CertificateAndKeyPair
 import net.corda.nodeapi.internal.crypto.CertificateType
-import net.corda.nodeapi.internal.crypto.X509CertificateFactory
 import net.corda.nodeapi.internal.crypto.X509Utilities
 import net.corda.nodeapi.internal.persistence.CordaPersistence
 import net.corda.nodeapi.internal.persistence.DatabaseConfig
@@ -23,6 +22,7 @@ import org.junit.Before
 import org.junit.Test
 import java.security.PrivateKey
 import java.security.cert.X509Certificate
+import javax.security.auth.x500.X500Principal
 import kotlin.test.assertEquals
 import kotlin.test.assertNotNull
 import kotlin.test.assertNull
@@ -83,8 +83,8 @@ class PersistentNodeInfoStorageTest : TestBase() {
     @Test
     fun `getNodeInfo returns persisted SignedNodeInfo using the hash of just the NodeInfo`() {
         // given
-        val (nodeA) = createValidSignedNodeInfo("TestA")
+        val (nodeA) = createValidSignedNodeInfo("TestA", requestStorage)
-        val (nodeB) = createValidSignedNodeInfo("TestB")
+        val (nodeB) = createValidSignedNodeInfo("TestB", requestStorage)
 
         // Put signed node info data
         nodeInfoStorage.putNodeInfo(nodeA)
@@ -102,7 +102,7 @@ class PersistentNodeInfoStorageTest : TestBase() {
     @Test
     fun `same public key with different node info`() {
         // Create node info.
-        val (node1, key) = createValidSignedNodeInfo("Test", serial = 1)
+        val (node1, key) = createValidSignedNodeInfo("Test", requestStorage)
         val nodeInfo2 = node1.nodeInfo.copy(serial = 2)
         val node2 = NodeInfoWithSigned(nodeInfo2.signWith(listOf(key)))
 
@@ -120,7 +120,7 @@ class PersistentNodeInfoStorageTest : TestBase() {
     @Test
     fun `putNodeInfo persists SignedNodeInfo with its signature`() {
         // given
-        val (nodeInfoWithSigned) = createValidSignedNodeInfo("Test")
+        val (nodeInfoWithSigned) = createValidSignedNodeInfo("Test", requestStorage)
 
         // when
         val nodeInfoHash = nodeInfoStorage.putNodeInfo(nodeInfoWithSigned)
@@ -129,16 +129,17 @@ class PersistentNodeInfoStorageTest : TestBase() {
         val persistedSignedNodeInfo = nodeInfoStorage.getNodeInfo(nodeInfoHash)
         assertThat(persistedSignedNodeInfo?.signatures).isEqualTo(nodeInfoWithSigned.signedNodeInfo.signatures)
     }
+}
 
-    private fun createValidSignedNodeInfo(organisation: String, serial: Long = 1): Pair<NodeInfoWithSigned, PrivateKey> {
+internal fun createValidSignedNodeInfo(organisation: String,
-        val nodeInfoBuilder = TestNodeInfoBuilder()
+                                       storage: CertificationRequestStorage): Pair<NodeInfoWithSigned, PrivateKey> {
-        val requestId = requestStorage.saveRequest(createRequest(organisation).first)
+    val (csr, nodeKeyPair) = createRequest(organisation)
-        requestStorage.markRequestTicketCreated(requestId)
+    val requestId = storage.saveRequest(csr)
-        requestStorage.approveRequest(requestId, "TestUser")
+    storage.markRequestTicketCreated(requestId)
-        val (identity, key) = nodeInfoBuilder.addIdentity(CordaX500Name(organisation, "London", "GB"))
+    storage.approveRequest(requestId, "TestUser")
-        val nodeCaCertPath = X509CertificateFactory().generateCertPath(identity.certPath.certificates.drop(1))
+    val nodeInfoBuilder = TestNodeInfoBuilder()
-        requestStorage.putCertificatePath(requestId, nodeCaCertPath, emptyList())
+    val (identity, key) = nodeInfoBuilder.addIdentity(CordaX500Name.build(X500Principal(csr.subject.encoded)), nodeKeyPair)
-        val (_, signedNodeInfo) = nodeInfoBuilder.buildWithSigned(serial)
+    storage.putCertificatePath(requestId, identity.certPath, listOf("Test"))
-        return Pair(NodeInfoWithSigned(signedNodeInfo), key)
+    val (_, signedNodeInfo) = nodeInfoBuilder.buildWithSigned(1)
-    }
+    return Pair(NodeInfoWithSigned(signedNodeInfo), key)
 }

network-management/src/test/kotlin/com/r3/corda/networkmanage/doorman/signer/DefaultCsrHandlerTest.kt

@@ -28,7 +28,7 @@ class DefaultCsrHandlerTest : TestBase() {
 
         val requestStorage: CertificationRequestStorage = mock {
             on { getRequest("New") }.thenReturn(certificateSigningRequest())
-            on { getRequest("Signed") }.thenReturn(certificateSigningRequest(status = RequestStatus.SIGNED, certData = certificateData("", CertificateStatus.VALID, buildCertPath(cert))))
+            on { getRequest("Signed") }.thenReturn(certificateSigningRequest(status = RequestStatus.SIGNED, certData = certificateData(CertificateStatus.VALID, buildCertPath(cert))))
             on { getRequest("Rejected") }.thenReturn(certificateSigningRequest(status = RequestStatus.REJECTED, remark = "Random reason"))
         }
         val requestProcessor = DefaultCsrHandler(requestStorage, null)

network-management/src/test/kotlin/com/r3/corda/networkmanage/doorman/signer/JiraCsrHandlerTest.kt

@@ -1,6 +1,7 @@
 package com.r3.corda.networkmanage.doorman.signer
 
 import com.nhaarman.mockito_kotlin.*
+import com.r3.corda.networkmanage.TestBase
 import com.r3.corda.networkmanage.common.persistence.*
 import com.r3.corda.networkmanage.doorman.ApprovedRequest
 import com.r3.corda.networkmanage.doorman.JiraClient
@@ -18,7 +19,7 @@ import org.mockito.junit.MockitoRule
 import java.security.cert.CertPath
 import kotlin.test.assertEquals
 
-class JiraCsrHandlerTest {
+class JiraCsrHandlerTest : TestBase() {
     @Rule
     @JvmField
     val mockitoRule: MockitoRule = MockitoJUnit.rule()
@@ -76,7 +77,11 @@ class JiraCsrHandlerTest {
 
     @Test
     fun `create tickets`() {
-        val csr = CertificateSigningRequest(requestId, "name", RequestStatus.NEW, pkcS10CertificationRequest, null, emptyList(), null)
+        val csr = certificateSigningRequest(
+                requestId = requestId,
+                legalName = "name",
+                status = RequestStatus.NEW,
+                request = pkcS10CertificationRequest)
         whenever(certificationRequestStorage.getRequests(RequestStatus.NEW)).thenReturn(listOf(csr))
 
         // Test
@@ -90,8 +95,8 @@ class JiraCsrHandlerTest {
     fun `sync tickets status`() {
         val id1 = SecureHash.randomSHA256().toString()
         val id2 = SecureHash.randomSHA256().toString()
-        val csr1 = CertificateSigningRequest(id1, "name1", RequestStatus.NEW, pkcS10CertificationRequest, null, emptyList(), null)
+        val csr1 = CertificateSigningRequest(id1, "name1", SecureHash.randomSHA256(), RequestStatus.NEW, pkcS10CertificationRequest, null, emptyList(), null)
-        val csr2 = CertificateSigningRequest(id2, "name2", RequestStatus.NEW, pkcS10CertificationRequest, null, emptyList(), null)
+        val csr2 = CertificateSigningRequest(id2, "name2", SecureHash.randomSHA256(), RequestStatus.NEW, pkcS10CertificationRequest, null, emptyList(), null)
 
         val requests = mutableMapOf(id1 to csr1, id2 to csr2)
 
@@ -134,7 +139,7 @@ class JiraCsrHandlerTest {
 
         // Sign request 1
         val certPath = mock<CertPath>()
-        val certData = CertificateData("", CertificateStatus.VALID, certPath)
+        val certData = CertificateData(CertificateStatus.VALID, certPath)
         requests[id1] = requests[id1]!!.copy(status = RequestStatus.SIGNED, certData = certData)
 
         // Process request again.

node-api/src/main/kotlin/net/corda/nodeapi/internal/KeyStoreConfigHelpers.kt

@@ -1,6 +1,7 @@
 package net.corda.nodeapi.internal
 
 import net.corda.core.crypto.Crypto
+import net.corda.core.crypto.Crypto.generateKeyPair
 import net.corda.core.identity.CordaX500Name
 import net.corda.core.internal.x500Name
 import net.corda.nodeapi.internal.config.SSLConfiguration
@@ -8,6 +9,7 @@ import net.corda.nodeapi.internal.crypto.*
 import org.bouncycastle.asn1.x509.GeneralName
 import org.bouncycastle.asn1.x509.GeneralSubtree
 import org.bouncycastle.asn1.x509.NameConstraints
+import java.security.KeyPair
 import java.security.cert.X509Certificate
 import javax.security.auth.x500.X500Principal
 
@@ -31,7 +33,7 @@ fun SSLConfiguration.createDevKeyStores(legalName: CordaX500Name,
         save(nodeKeystore, keyStorePassword)
     }
 
-    val tlsKeyPair = Crypto.generateKeyPair(X509Utilities.DEFAULT_TLS_SIGNATURE_SCHEME)
+    val tlsKeyPair = generateKeyPair(X509Utilities.DEFAULT_TLS_SIGNATURE_SCHEME)
     val tlsCert = X509Utilities.createCertificate(CertificateType.TLS, nodeCaCert, nodeCaKeyPair, legalName.x500Principal, tlsKeyPair.public)
 
     loadOrCreateKeyStore(sslKeystore, keyStorePassword).apply {
@@ -59,17 +61,18 @@ fun createDevNetworkMapCa(rootCa: CertificateAndKeyPair = DEV_ROOT_CA): Certific
  * Create a dev node CA cert, as a sub-cert of the given [intermediateCa], and matching key pair using the given
  * [CordaX500Name] as the cert subject.
  */
-fun createDevNodeCa(intermediateCa: CertificateAndKeyPair, legalName: CordaX500Name): CertificateAndKeyPair {
+fun createDevNodeCa(intermediateCa: CertificateAndKeyPair,
-    val keyPair = Crypto.generateKeyPair(X509Utilities.DEFAULT_TLS_SIGNATURE_SCHEME)
+                    legalName: CordaX500Name,
+                    nodeKeyPair: KeyPair = generateKeyPair(X509Utilities.DEFAULT_TLS_SIGNATURE_SCHEME)): CertificateAndKeyPair {
     val nameConstraints = NameConstraints(arrayOf(GeneralSubtree(GeneralName(GeneralName.directoryName, legalName.x500Name))), arrayOf())
     val cert = X509Utilities.createCertificate(
             CertificateType.NODE_CA,
             intermediateCa.certificate,
             intermediateCa.keyPair,
             legalName.x500Principal,
-            keyPair.public,
+            nodeKeyPair.public,
             nameConstraints = nameConstraints)
-    return CertificateAndKeyPair(cert, keyPair)
+    return CertificateAndKeyPair(cert, nodeKeyPair)
 }
 
 val DEV_INTERMEDIATE_CA: CertificateAndKeyPair get() = DevCaHelper.loadDevCa(X509Utilities.CORDA_INTERMEDIATE_CA)

testing/test-utils/src/main/kotlin/net/corda/testing/internal/InternalTestUtils.kt

@@ -2,6 +2,7 @@ package net.corda.testing.internal
 
 import com.nhaarman.mockito_kotlin.doAnswer
 import net.corda.core.crypto.Crypto
+import net.corda.core.crypto.Crypto.generateKeyPair
 import net.corda.core.identity.CordaX500Name
 import net.corda.core.utilities.loggerFor
 import net.corda.node.services.config.configureDevKeyAndTrustStores
@@ -15,6 +16,7 @@ import org.mockito.Mockito
 import org.mockito.internal.stubbing.answers.ThrowsException
 import java.lang.reflect.Modifier
 import java.nio.file.Files
+import java.security.KeyPair
 import java.util.*
 import javax.security.auth.x500.X500Principal
 
@@ -102,11 +104,12 @@ fun createDevIntermediateCaCertPath(
  */
 fun createDevNodeCaCertPath(
         legalName: CordaX500Name,
+        nodeKeyPair: KeyPair = generateKeyPair(X509Utilities.DEFAULT_TLS_SIGNATURE_SCHEME),
         rootCaName: X500Principal = defaultRootCaName,
         intermediateCaName: X500Principal = defaultIntermediateCaName
 ): Triple<CertificateAndKeyPair, CertificateAndKeyPair, CertificateAndKeyPair> {
     val (rootCa, intermediateCa) = createDevIntermediateCaCertPath(rootCaName, intermediateCaName)
-    val nodeCa = createDevNodeCa(intermediateCa, legalName)
+    val nodeCa = createDevNodeCa(intermediateCa, legalName, nodeKeyPair)
     return Triple(rootCa, intermediateCa, nodeCa)
 }
 

testing/test-utils/src/main/kotlin/net/corda/testing/internal/TestNodeInfoBuilder.kt

@@ -8,16 +8,35 @@ import net.corda.core.node.NodeInfo
 import net.corda.core.serialization.serialize
 import net.corda.core.utilities.NetworkHostAndPort
 import net.corda.nodeapi.internal.SignedNodeInfo
-import net.corda.testing.getTestPartyAndCertificate
+import net.corda.nodeapi.internal.createDevNodeCa
+import net.corda.nodeapi.internal.crypto.CertificateAndKeyPair
+import net.corda.nodeapi.internal.crypto.CertificateType
+import net.corda.nodeapi.internal.crypto.X509CertificateFactory
+import net.corda.nodeapi.internal.crypto.X509Utilities
+import net.corda.testing.DEV_INTERMEDIATE_CA
+import net.corda.testing.DEV_ROOT_CA
+import java.security.KeyPair
 import java.security.PrivateKey
+import java.security.cert.X509Certificate
 
-class TestNodeInfoBuilder {
+class TestNodeInfoBuilder(private val intermediateAndRoot: Pair<CertificateAndKeyPair, X509Certificate> = DEV_INTERMEDIATE_CA to DEV_ROOT_CA.certificate) {
     private val identitiesAndPrivateKeys = ArrayList<Pair<PartyAndCertificate, PrivateKey>>()
 
-    fun addIdentity(name: CordaX500Name): Pair<PartyAndCertificate, PrivateKey> {
+    fun addIdentity(name: CordaX500Name, nodeKeyPair: KeyPair = Crypto.generateKeyPair(X509Utilities.DEFAULT_TLS_SIGNATURE_SCHEME)): Pair<PartyAndCertificate, PrivateKey> {
+        val nodeCertificateAndKeyPair = createDevNodeCa(intermediateAndRoot.first, name, nodeKeyPair)
         val identityKeyPair = Crypto.generateKeyPair()
-        val identity = getTestPartyAndCertificate(name, identityKeyPair.public)
+        val identityCert = X509Utilities.createCertificate(
-        return Pair(identity, identityKeyPair.private).also {
+                CertificateType.LEGAL_IDENTITY,
+                nodeCertificateAndKeyPair.certificate,
+                nodeCertificateAndKeyPair.keyPair,
+                nodeCertificateAndKeyPair.certificate.subjectX500Principal,
+                identityKeyPair.public)
+        val certPath = X509CertificateFactory()
+                .generateCertPath(identityCert,
+                        nodeCertificateAndKeyPair.certificate,
+                        intermediateAndRoot.first.certificate,
+                        intermediateAndRoot.second)
+        return Pair(PartyAndCertificate(certPath), identityKeyPair.private).also {
             identitiesAndPrivateKeys += it
         }
     }