CORDA-819 Add checks to ensure TLS and Client CA cert chains to the same trusted root (#2149)

* testnet bad node info bug fix

* address PR issues

* fix PR issues

* remove TODO for checking validation logic
This commit is contained in:
Patrick Kuo 2017-12-08 14:35:49 +00:00 committed by GitHub
parent 7c5a328cc1
commit 9b097aa988
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 92 additions and 11 deletions

View File

@ -0,0 +1,63 @@
package net.corda.node
import net.corda.core.crypto.Crypto
import net.corda.core.identity.CordaX500Name
import net.corda.core.internal.cert
import net.corda.core.internal.div
import net.corda.core.utilities.getOrThrow
import net.corda.node.services.config.configureDevKeyAndTrustStores
import net.corda.nodeapi.config.SSLConfiguration
import net.corda.nodeapi.internal.crypto.*
import net.corda.testing.ALICE_NAME
import net.corda.testing.driver.driver
import org.junit.Test
import java.nio.file.Path
import kotlin.test.assertEquals
import kotlin.test.assertFailsWith
import kotlin.test.assertTrue
class NodeKeystoreCheckTest {
@Test
fun `node should throw exception if cert path doesn't chain to the trust root`() {
driver(startNodesInProcess = true) {
// This will fail because there are no keystore configured.
assertFailsWith(IllegalArgumentException::class) {
startNode(customOverrides = mapOf("devMode" to false)).getOrThrow()
}.apply {
assertTrue(message?.startsWith("Identity certificate not found. ") ?: false)
}
// Create keystores
val keystorePassword = "password"
val config = object : SSLConfiguration {
override val keyStorePassword: String = keystorePassword
override val trustStorePassword: String = keystorePassword
override val certificatesDirectory: Path = baseDirectory(ALICE_NAME.toString()) / "certificates"
}
config.configureDevKeyAndTrustStores(ALICE_NAME)
// This should pass with correct keystore.
val node = startNode(providedName = ALICE_NAME, customOverrides = mapOf("devMode" to false,
"keyStorePassword" to keystorePassword,
"trustStorePassword" to keystorePassword)).get()
node.stop()
// Fiddle with node keystore.
val keystore = loadKeyStore(config.nodeKeystore, config.keyStorePassword)
// Self signed root
val badRootKeyPair = Crypto.generateKeyPair()
val badRoot = X509Utilities.createSelfSignedCACertificate(CordaX500Name("Bad Root", "Lodnon", "GB"), badRootKeyPair)
val nodeCA = keystore.getCertificateAndKeyPair(X509Utilities.CORDA_CLIENT_CA, config.keyStorePassword)
val badNodeCACert = X509Utilities.createCertificate(CertificateType.CLIENT_CA, badRoot, badRootKeyPair, ALICE_NAME, nodeCA.keyPair.public)
keystore.setKeyEntry(X509Utilities.CORDA_CLIENT_CA, nodeCA.keyPair.private, config.keyStorePassword.toCharArray(), arrayOf(badNodeCACert.cert, badRoot.cert))
keystore.save(config.nodeKeystore, config.keyStorePassword)
assertFailsWith(IllegalArgumentException::class) {
startNode(providedName = ALICE_NAME, customOverrides = mapOf("devMode" to false)).getOrThrow()
}.apply {
assertEquals("Client CA certificate must chain to the trusted root.", message)
}
}
}
}

View File

@ -567,6 +567,17 @@ abstract class AbstractNode(val configuration: NodeConfiguration,
"or if you don't have one yet, fill out the config file and run corda.jar --initial-registration. " + "or if you don't have one yet, fill out the config file and run corda.jar --initial-registration. " +
"Read more at: https://docs.corda.net/permissioning.html" "Read more at: https://docs.corda.net/permissioning.html"
} }
// Check all cert path chain to the trusted root
val sslKeystore = loadKeyStore(configuration.sslKeystore, configuration.keyStorePassword)
val identitiesKeystore = loadKeyStore(configuration.nodeKeystore, configuration.keyStorePassword)
val trustStore = loadKeyStore(configuration.trustStoreFile, configuration.trustStorePassword)
val sslRoot = sslKeystore.getCertificateChain(X509Utilities.CORDA_CLIENT_TLS).last()
val clientCARoot = identitiesKeystore.getCertificateChain(X509Utilities.CORDA_CLIENT_CA).last()
val trustRoot = trustStore.getCertificate(X509Utilities.CORDA_ROOT_CA)
require(sslRoot == trustRoot) { "TLS certificate must chain to the trusted root." }
require(clientCARoot == trustRoot) { "Client CA certificate must chain to the trusted root." }
} }
// Specific class so that MockNode can catch it. // Specific class so that MockNode can catch it.

View File

@ -50,7 +50,7 @@ fun NodeConfiguration.configureWithDevSSLCertificate() = configureDevKeyAndTrust
fun SSLConfiguration.configureDevKeyAndTrustStores(myLegalName: CordaX500Name) { fun SSLConfiguration.configureDevKeyAndTrustStores(myLegalName: CordaX500Name) {
certificatesDirectory.createDirectories() certificatesDirectory.createDirectories()
if (!trustStoreFile.exists()) { if (!trustStoreFile.exists()) {
javaClass.classLoader.getResourceAsStream("net/corda/node/internal/certificates/cordatruststore.jks").copyTo(trustStoreFile) loadKeyStore(javaClass.classLoader.getResourceAsStream("net/corda/node/internal/certificates/cordatruststore.jks"), "trustpass").save(trustStoreFile, trustStorePassword)
} }
if (!sslKeystore.exists() || !nodeKeystore.exists()) { if (!sslKeystore.exists() || !nodeKeystore.exists()) {
val caKeyStore = loadKeyStore(javaClass.classLoader.getResourceAsStream("net/corda/node/internal/certificates/cordadevcakeys.jks"), "cordacadevpass") val caKeyStore = loadKeyStore(javaClass.classLoader.getResourceAsStream("net/corda/node/internal/certificates/cordadevcakeys.jks"), "cordacadevpass")

View File

@ -45,18 +45,17 @@ class InMemoryIdentityService(identities: Iterable<PartyAndCertificate>,
principalToParties.putAll(identities.associateBy { it.name }) principalToParties.putAll(identities.associateBy { it.name })
} }
// TODO: Check the certificate validation logic
@Throws(CertificateExpiredException::class, CertificateNotYetValidException::class, InvalidAlgorithmParameterException::class) @Throws(CertificateExpiredException::class, CertificateNotYetValidException::class, InvalidAlgorithmParameterException::class)
override fun verifyAndRegisterIdentity(identity: PartyAndCertificate): PartyAndCertificate? { override fun verifyAndRegisterIdentity(identity: PartyAndCertificate): PartyAndCertificate? {
// Validate the chain first, before we do anything clever with it // Validate the chain first, before we do anything clever with it
try { try {
identity.verify(trustAnchor) identity.verify(trustAnchor)
} catch (e: CertPathValidatorException) { } catch (e: CertPathValidatorException) {
log.error("Certificate validation failed for ${identity.name} against trusted root ${trustAnchor.trustedCert.subjectX500Principal}.") log.warn("Certificate validation failed for ${identity.name} against trusted root ${trustAnchor.trustedCert.subjectX500Principal}.")
log.error("Certificate path :") log.warn("Certificate path :")
identity.certPath.certificates.reversed().forEachIndexed { index, certificate -> identity.certPath.certificates.reversed().forEachIndexed { index, certificate ->
val space = (0 until index).joinToString("") { " " } val space = (0 until index).joinToString("") { " " }
log.error("$space${certificate.toX509CertHolder().subject}") log.warn("$space${certificate.toX509CertHolder().subject}")
} }
throw e throw e
} }

View File

@ -110,17 +110,16 @@ class PersistentIdentityService(override val trustRoot: X509Certificate,
} }
} }
// TODO: Check the certificate validation logic
@Throws(CertificateExpiredException::class, CertificateNotYetValidException::class, InvalidAlgorithmParameterException::class) @Throws(CertificateExpiredException::class, CertificateNotYetValidException::class, InvalidAlgorithmParameterException::class)
override fun verifyAndRegisterIdentity(identity: PartyAndCertificate): PartyAndCertificate? { override fun verifyAndRegisterIdentity(identity: PartyAndCertificate): PartyAndCertificate? {
// Validate the chain first, before we do anything clever with it // Validate the chain first, before we do anything clever with it
try { try {
identity.verify(trustAnchor) identity.verify(trustAnchor)
} catch (e: CertPathValidatorException) { } catch (e: CertPathValidatorException) {
log.error(e.localizedMessage) log.warn(e.localizedMessage)
log.error("Path = ") log.warn("Path = ")
identity.certPath.certificates.reversed().forEach { identity.certPath.certificates.reversed().forEach {
log.error(it.toX509CertHolder().subject.toString()) log.warn(it.toX509CertHolder().subject.toString())
} }
throw e throw e
} }

View File

@ -20,6 +20,7 @@ import net.corda.core.serialization.SingletonSerializeAsToken
import net.corda.core.serialization.serialize import net.corda.core.serialization.serialize
import net.corda.core.utilities.NetworkHostAndPort import net.corda.core.utilities.NetworkHostAndPort
import net.corda.core.utilities.contextLogger import net.corda.core.utilities.contextLogger
import net.corda.core.utilities.loggerFor
import net.corda.node.services.api.NetworkMapCacheBaseInternal import net.corda.node.services.api.NetworkMapCacheBaseInternal
import net.corda.node.services.api.NetworkMapCacheInternal import net.corda.node.services.api.NetworkMapCacheInternal
import net.corda.nodeapi.internal.persistence.CordaPersistence import net.corda.nodeapi.internal.persistence.CordaPersistence
@ -37,13 +38,22 @@ class NetworkMapCacheImpl(
networkMapCacheBase: NetworkMapCacheBaseInternal, networkMapCacheBase: NetworkMapCacheBaseInternal,
private val identityService: IdentityService private val identityService: IdentityService
) : NetworkMapCacheBaseInternal by networkMapCacheBase, NetworkMapCacheInternal { ) : NetworkMapCacheBaseInternal by networkMapCacheBase, NetworkMapCacheInternal {
companion object {
private val logger = loggerFor<NetworkMapCacheImpl>()
}
init { init {
networkMapCacheBase.allNodes.forEach { it.legalIdentitiesAndCerts.forEach { identityService.verifyAndRegisterIdentity(it) } } networkMapCacheBase.allNodes.forEach { it.legalIdentitiesAndCerts.forEach { identityService.verifyAndRegisterIdentity(it) } }
networkMapCacheBase.changed.subscribe { mapChange -> networkMapCacheBase.changed.subscribe { mapChange ->
// TODO how should we handle network map removal // TODO how should we handle network map removal
if (mapChange is MapChange.Added) { if (mapChange is MapChange.Added) {
mapChange.node.legalIdentitiesAndCerts.forEach { mapChange.node.legalIdentitiesAndCerts.forEach {
identityService.verifyAndRegisterIdentity(it) try {
identityService.verifyAndRegisterIdentity(it)
} catch (ignore: Exception) {
// Log a warning to indicate node info is not added to the network map cache.
logger.warn("Node info for :'${it.name}' is not added to the network map due to verification error.")
}
} }
} }
} }

View File

@ -3,7 +3,6 @@ package net.corda.node.services.network
import net.corda.core.node.services.NetworkMapCache import net.corda.core.node.services.NetworkMapCache
import net.corda.testing.ALICE_NAME import net.corda.testing.ALICE_NAME
import net.corda.testing.BOB_NAME import net.corda.testing.BOB_NAME
import net.corda.testing.DUMMY_NOTARY
import net.corda.testing.node.MockNetwork import net.corda.testing.node.MockNetwork
import net.corda.testing.node.MockNodeParameters import net.corda.testing.node.MockNodeParameters
import net.corda.testing.singleIdentity import net.corda.testing.singleIdentity