From 060f7cc3cb7063d85f55e658ac349ae7e68c3137 Mon Sep 17 00:00:00 2001 From: Shams Asari Date: Fri, 16 Mar 2018 18:45:28 +0000 Subject: [PATCH] Using CordaX500Name in the doorman database entity objects (ENT-1524) (#573) certificate_signing_request.legal_name is nullable - invalid names from CSRs are not stored in the db. --- .../CertificateSigningRequestStorage.kt | 3 +- ...tentCertificateRevocationRequestStorage.kt | 17 +++++---- ...sistentCertificateSigningRequestStorage.kt | 38 ++++++++----------- .../CertificateRevocationRequestEntity.kt | 6 ++- .../entity/CertificateSigningRequestEntity.kt | 34 +++++++++-------- .../common/persistence/entity/Converters.kt | 15 ++++++++ .../network-manager.changelog-init.xml | 4 +- .../com/r3/corda/networkmanage/TestBase.kt | 3 +- .../doorman/signer/JiraCsrHandlerTest.kt | 8 ++-- 9 files changed, 73 insertions(+), 55 deletions(-) create mode 100644 network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/entity/Converters.kt diff --git a/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/CertificateSigningRequestStorage.kt b/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/CertificateSigningRequestStorage.kt index fb830b0d7f..04cbe1d187 100644 --- a/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/CertificateSigningRequestStorage.kt +++ b/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/CertificateSigningRequestStorage.kt @@ -11,6 +11,7 @@ package com.r3.corda.networkmanage.common.persistence import net.corda.core.crypto.SecureHash +import net.corda.core.identity.CordaX500Name import net.corda.core.serialization.CordaSerializable import org.bouncycastle.pkcs.PKCS10CertificationRequest import java.security.cert.CertPath @@ -18,7 +19,7 @@ import java.security.cert.CertPath data class CertificateData(val certStatus: CertificateStatus, val certPath: CertPath) data class CertificateSigningRequest(val requestId: String, - val legalName: String, + val legalName: CordaX500Name?, val publicKeyHash: SecureHash, val status: RequestStatus, val request: PKCS10CertificationRequest, diff --git a/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/PersistentCertificateRevocationRequestStorage.kt b/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/PersistentCertificateRevocationRequestStorage.kt index 808d8f2858..302a176efe 100644 --- a/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/PersistentCertificateRevocationRequestStorage.kt +++ b/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/PersistentCertificateRevocationRequestStorage.kt @@ -37,7 +37,7 @@ class PersistentCertificateRevocationRequestStorage(private val database: CordaP modifiedBy = request.reporter, certificateData = certificateData, reporter = request.reporter, - legalName = certificateData.legalName() + legalName = certificateData.legalName )) requestId } @@ -84,7 +84,7 @@ class PersistentCertificateRevocationRequestStorage(private val database: CordaP legalName: CordaX500Name?, result: CertificateSigningRequestEntity): CertificateSigningRequestEntity { val certData = result.certificateData!! - require(legalName == null || result.legalName == legalName.toString()) { + require(legalName == null || result.legalName == legalName) { "The legal name does not match." } require(csrRequestId == null || result.requestId == csrRequestId) { @@ -98,7 +98,7 @@ class PersistentCertificateRevocationRequestStorage(private val database: CordaP override fun getRevocationRequest(requestId: String): CertificateRevocationRequestData? { return database.transaction { - getRevocationRequestEntity(requestId)?.toCertificateRevocationRequest() + getRevocationRequestEntity(requestId)?.toCertificateRevocationRequestData() } } @@ -110,7 +110,7 @@ class PersistentCertificateRevocationRequestStorage(private val database: CordaP where(builder.equal(get(CertificateRevocationRequestEntity::status.name), revocationStatus)) } } - session.createQuery(query).resultList.map { it.toCertificateRevocationRequest() } + session.createQuery(query).resultList.map { it.toCertificateRevocationRequestData() } } } @@ -153,14 +153,15 @@ class PersistentCertificateRevocationRequestStorage(private val database: CordaP } } - private fun CertificateRevocationRequestEntity.toCertificateRevocationRequest(): CertificateRevocationRequestData { + private fun CertificateRevocationRequestEntity.toCertificateRevocationRequestData(): CertificateRevocationRequestData { return CertificateRevocationRequestData( requestId, certificateSerialNumber, modifiedAt, - CordaX500Name.parse(legalName), + legalName, status, revocationReason, - reporter) + reporter + ) } -} \ No newline at end of file +} diff --git a/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/PersistentCertificateSigningRequestStorage.kt b/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/PersistentCertificateSigningRequestStorage.kt index 46ea5f9a3d..4a0df5cdb2 100644 --- a/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/PersistentCertificateSigningRequestStorage.kt +++ b/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/PersistentCertificateSigningRequestStorage.kt @@ -61,27 +61,21 @@ class PersistentCertificateSigningRequestStorage(private val database: CordaPers override fun saveRequest(request: PKCS10CertificationRequest): String { val requestId = SecureHash.randomSHA256().toString() database.transaction(TransactionIsolationLevel.SERIALIZABLE) { - val requestEntity = try { - val legalName = validateRequestAndParseLegalName(request) - CertificateSigningRequestEntity( - requestId = requestId, - legalName = legalName, - publicKeyHash = toSupportedPublicKey(request.subjectPublicKeyInfo).hashString(), - requestBytes = request.encoded, - modifiedBy = CertificateSigningRequestStorage.DOORMAN_SIGNATURE, - status = RequestStatus.NEW - ) + val legalNameOrRejectMessage = try { + validateRequestAndParseLegalName(request) } catch (e: RequestValidationException) { - CertificateSigningRequestEntity( + e.rejectMessage + } + + val requestEntity = CertificateSigningRequestEntity( requestId = requestId, - legalName = e.parsedLegalName, + legalName = legalNameOrRejectMessage as? CordaX500Name, publicKeyHash = toSupportedPublicKey(request.subjectPublicKeyInfo).hashString(), requestBytes = request.encoded, - remark = e.rejectMessage, + remark = legalNameOrRejectMessage as? String, modifiedBy = CertificateSigningRequestStorage.DOORMAN_SIGNATURE, - status = RequestStatus.REJECTED - ) - } + status = if (legalNameOrRejectMessage is CordaX500Name) RequestStatus.NEW else RequestStatus.REJECTED + ) session.save(requestEntity) } return requestId @@ -155,7 +149,7 @@ class PersistentCertificateSigningRequestStorage(private val database: CordaPers } } - private fun DatabaseTransaction.validateRequestAndParseLegalName(request: PKCS10CertificationRequest): String { + private fun DatabaseTransaction.validateRequestAndParseLegalName(request: PKCS10CertificationRequest): CordaX500Name { // It's important that we always use the toString() output of CordaX500Name as it standardises the string format // to make querying possible. val legalName = try { @@ -169,21 +163,21 @@ class PersistentCertificateSigningRequestStorage(private val database: CordaPers // 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. - nonRejectedRequestExists(CertificateSigningRequestEntity::legalName.name, legalName.toString()) -> throw RequestValidationException(legalName.toString(), "Duplicate legal name") + nonRejectedRequestExists(CertificateSigningRequestEntity::legalName.name, legalName) -> throw RequestValidationException(legalName.toString(), "Duplicate legal name") //TODO Consider following scenario: There is a CSR that is signed but the certificate itself has expired or was revoked nonRejectedRequestExists(CertificateSigningRequestEntity::publicKeyHash.name, toSupportedPublicKey(request.subjectPublicKeyInfo).hashString()) -> throw RequestValidationException(legalName.toString(), "Duplicate public key") - else -> legalName.toString() + else -> legalName } } /** * Check if "non-rejected" request exists with provided column and value. */ - private fun DatabaseTransaction.nonRejectedRequestExists(columnName: String, value: String): Boolean { + private fun DatabaseTransaction.nonRejectedRequestExists(columnName: String, value: Any): Boolean { val query = session.criteriaBuilder.run { val criteriaQuery = createQuery(CertificateSigningRequestEntity::class.java) criteriaQuery.from(CertificateSigningRequestEntity::class.java).run { - val valueQuery = equal(get(columnName), value) + val valueQuery = equal(get(columnName), value) val statusQuery = notEqual(get(CertificateSigningRequestEntity::status.name), RequestStatus.REJECTED) criteriaQuery.where(and(valueQuery, statusQuery)) } @@ -191,5 +185,5 @@ class PersistentCertificateSigningRequestStorage(private val database: CordaPers return session.createQuery(query).setMaxResults(1).resultList.isNotEmpty() } - private class RequestValidationException(val parsedLegalName: String, val rejectMessage: String) : Exception("Validation failed for $parsedLegalName. $rejectMessage.") + private class RequestValidationException(subjectName: String, val rejectMessage: String) : Exception("Validation failed for $subjectName. $rejectMessage.") } \ No newline at end of file diff --git a/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/entity/CertificateRevocationRequestEntity.kt b/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/entity/CertificateRevocationRequestEntity.kt index 0d522b16e6..3319d1f7a0 100644 --- a/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/entity/CertificateRevocationRequestEntity.kt +++ b/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/entity/CertificateRevocationRequestEntity.kt @@ -1,6 +1,7 @@ package com.r3.corda.networkmanage.common.persistence.entity import com.r3.corda.networkmanage.common.persistence.RequestStatus +import net.corda.core.identity.CordaX500Name import org.hibernate.envers.Audited import java.math.BigInteger import java.security.cert.CRLReason @@ -25,7 +26,8 @@ class CertificateRevocationRequestEntity( val certificateSerialNumber: BigInteger, @Column(name = "legal_name", length = 256, nullable = false) - val legalName: String, + @Convert(converter = CordaX500NameAttributeConverter::class) + val legalName: CordaX500Name, @Audited @Column(name = "status", nullable = false) @@ -56,7 +58,7 @@ class CertificateRevocationRequestEntity( certificateData: CertificateDataEntity = this.certificateData, certificateSerialNumber: BigInteger = this.certificateSerialNumber, status: RequestStatus = this.status, - legalName: String = this.legalName, + legalName: CordaX500Name = this.legalName, reporter: String = this.reporter, modifiedBy: String = this.modifiedBy, modifiedAt: Instant = this.modifiedAt, diff --git a/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/entity/CertificateSigningRequestEntity.kt b/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/entity/CertificateSigningRequestEntity.kt index f8f226d30f..2a4a41b477 100644 --- a/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/entity/CertificateSigningRequestEntity.kt +++ b/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/entity/CertificateSigningRequestEntity.kt @@ -16,11 +16,12 @@ import com.r3.corda.networkmanage.common.persistence.CertificateStatus import com.r3.corda.networkmanage.common.persistence.RequestStatus import com.r3.corda.networkmanage.common.utils.buildCertPath import net.corda.core.crypto.SecureHash +import net.corda.core.identity.CordaX500Name +import net.corda.nodeapi.internal.crypto.x509Certificates import org.bouncycastle.pkcs.PKCS10CertificationRequest import org.hibernate.envers.Audited import java.math.BigInteger import java.security.cert.CertPath -import java.security.cert.X509Certificate import java.time.Instant import javax.persistence.* @@ -32,8 +33,9 @@ data class CertificateSigningRequestEntity( val requestId: String, // TODO: Store X500Name with a proper schema. - @Column(name = "legal_name", length = 256, nullable = false) - val legalName: String, + @Column(name = "legal_name", length = 256) + @Convert(converter = CordaX500NameAttributeConverter::class) + val legalName: CordaX500Name?, @Column(name = "public_key_hash", length = 64) val publicKeyHash: String, @@ -66,16 +68,18 @@ data class CertificateSigningRequestEntity( @JoinColumn(name = "private_network", foreignKey = ForeignKey(name = "FK_CSR_PN")) val privateNetwork: PrivateNetworkEntity? = null ) { - fun toCertificateSigningRequest() = CertificateSigningRequest( - requestId = requestId, - legalName = legalName, - publicKeyHash = SecureHash.parse(publicKeyHash), - status = status, - request = request(), - remark = remark, - modifiedBy = modifiedBy, - certData = certificateData?.toCertificateData() - ) + fun toCertificateSigningRequest(): CertificateSigningRequest { + return CertificateSigningRequest( + requestId = requestId, + legalName = legalName, + publicKeyHash = SecureHash.parse(publicKeyHash), + status = status, + request = request(), + remark = remark, + modifiedBy = modifiedBy, + certData = certificateData?.toCertificateData() + ) + } private fun request() = PKCS10CertificationRequest(requestBytes) } @@ -108,8 +112,8 @@ data class CertificateDataEntity( ) } - fun legalName(): String { - return (toCertificatePath().certificates.first() as X509Certificate).subjectX500Principal.name + val legalName: CordaX500Name get() { + return CordaX500Name.build(toCertificatePath().x509Certificates[0].subjectX500Principal) } fun copy(certificateStatus: CertificateStatus = this.certificateStatus, diff --git a/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/entity/Converters.kt b/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/entity/Converters.kt new file mode 100644 index 0000000000..a315c632b3 --- /dev/null +++ b/network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/entity/Converters.kt @@ -0,0 +1,15 @@ +package com.r3.corda.networkmanage.common.persistence.entity + +import net.corda.core.identity.CordaX500Name +import javax.persistence.AttributeConverter + +class CordaX500NameAttributeConverter : AttributeConverter { + override fun convertToDatabaseColumn(attribute: CordaX500Name?): String? = attribute?.toString() + override fun convertToEntityAttribute(dbData: String?): CordaX500Name? = dbData?.let { CordaX500Name.parse(it) } +} + +// TODO Use SecureHash in entities +//class SecureHashAttributeConverter : AttributeConverter { +// override fun convertToDatabaseColumn(attribute: SecureHash?): String? = attribute?.toString() +// override fun convertToEntityAttribute(dbData: String?): SecureHash? = dbData?.let { SecureHash.parse(it) } +//} diff --git a/network-management/src/main/resources/migration/network-manager.changelog-init.xml b/network-management/src/main/resources/migration/network-manager.changelog-init.xml index 6eadb6102a..f0474d7d3c 100644 --- a/network-management/src/main/resources/migration/network-manager.changelog-init.xml +++ b/network-management/src/main/resources/migration/network-manager.changelog-init.xml @@ -42,9 +42,7 @@ - - - + diff --git a/network-management/src/test/kotlin/com/r3/corda/networkmanage/TestBase.kt b/network-management/src/test/kotlin/com/r3/corda/networkmanage/TestBase.kt index 3a53b3da31..9b96affffd 100644 --- a/network-management/src/test/kotlin/com/r3/corda/networkmanage/TestBase.kt +++ b/network-management/src/test/kotlin/com/r3/corda/networkmanage/TestBase.kt @@ -17,6 +17,7 @@ import net.corda.core.identity.CordaX500Name import net.corda.core.internal.CertRole import net.corda.nodeapi.internal.crypto.X509Utilities import net.corda.nodeapi.internal.crypto.x509Certificates +import net.corda.testing.core.ALICE_NAME import net.corda.testing.core.SerializationEnvironmentRule import net.corda.testing.internal.createDevNodeCaCertPath import org.bouncycastle.pkcs.PKCS10CertificationRequest @@ -35,7 +36,7 @@ abstract class TestBase { protected fun certificateSigningRequest( requestId: String = SecureHash.randomSHA256().toString(), status: RequestStatus = RequestStatus.NEW, - legalName: String = "TestLegalName", + legalName: CordaX500Name = ALICE_NAME, publicKeyHash: SecureHash = SecureHash.randomSHA256(), remark: String = "Test remark", request: PKCS10CertificationRequest = mock(), diff --git a/network-management/src/test/kotlin/com/r3/corda/networkmanage/doorman/signer/JiraCsrHandlerTest.kt b/network-management/src/test/kotlin/com/r3/corda/networkmanage/doorman/signer/JiraCsrHandlerTest.kt index 1f3768b976..c9f118544c 100644 --- a/network-management/src/test/kotlin/com/r3/corda/networkmanage/doorman/signer/JiraCsrHandlerTest.kt +++ b/network-management/src/test/kotlin/com/r3/corda/networkmanage/doorman/signer/JiraCsrHandlerTest.kt @@ -20,6 +20,8 @@ import net.corda.core.crypto.Crypto import net.corda.core.crypto.SecureHash import net.corda.core.identity.CordaX500Name import net.corda.nodeapi.internal.crypto.X509Utilities +import net.corda.testing.core.ALICE_NAME +import net.corda.testing.core.BOB_NAME import org.junit.Before import org.junit.Rule import org.junit.Test @@ -89,7 +91,7 @@ class JiraCsrHandlerTest : TestBase() { fun `create tickets`() { val csr = certificateSigningRequest( requestId = requestId, - legalName = "name", + legalName = ALICE_NAME, status = RequestStatus.NEW, request = pkcS10CertificationRequest) whenever(certificationRequestStorage.getRequests(RequestStatus.NEW)).thenReturn(listOf(csr)) @@ -105,8 +107,8 @@ class JiraCsrHandlerTest : TestBase() { fun `sync tickets status`() { val id1 = SecureHash.randomSHA256().toString() val id2 = SecureHash.randomSHA256().toString() - val csr1 = CertificateSigningRequest(id1, "name1", SecureHash.randomSHA256(), RequestStatus.NEW, pkcS10CertificationRequest, null, "Test", null) - val csr2 = CertificateSigningRequest(id2, "name2", SecureHash.randomSHA256(), RequestStatus.NEW, pkcS10CertificationRequest, null, "Test", null) + val csr1 = CertificateSigningRequest(id1, ALICE_NAME, SecureHash.randomSHA256(), RequestStatus.NEW, pkcS10CertificationRequest, null, "Test", null) + val csr2 = CertificateSigningRequest(id2, BOB_NAME, SecureHash.randomSHA256(), RequestStatus.NEW, pkcS10CertificationRequest, null, "Test", null) val requests = mutableMapOf(id1 to csr1, id2 to csr2)