From b3b184c93e8d066ac87918230ed7881be76c1004 Mon Sep 17 00:00:00 2001 From: JamesHR3 <45565019+JamesHR3@users.noreply.github.com> Date: Fri, 8 Mar 2019 16:23:07 +0000 Subject: [PATCH] [CORDA-2701] Ensure crlCheckSoftFail config option is respected (#4854) * Plumb through the crlCheckSoftFail configuration option to bridge manager * Add crlCheckSoftFail test to bridge manager and fix equivalent proton wrapper test * Update documentation and set the node configuration default to true * Revert default change and clarify consequences of setting option to false * Remove NodeConfiguration default to leave only AMQPConfiguration default --- docs/source/corda-configuration-file.rst | 8 +++--- .../internal/bridging/AMQPBridgeManager.kt | 16 +++++++---- .../bridging/BridgeControlListener.kt | 12 ++++++--- .../net/corda/node/amqp/AMQPBridgeTest.kt | 27 ++++++++++++++++--- .../net/corda/node/amqp/ProtonWrapperTests.kt | 6 +++-- .../kotlin/net/corda/node/internal/Node.kt | 6 ++++- 6 files changed, 58 insertions(+), 17 deletions(-) diff --git a/docs/source/corda-configuration-file.rst b/docs/source/corda-configuration-file.rst index 32032f9673..a0a669ad86 100644 --- a/docs/source/corda-configuration-file.rst +++ b/docs/source/corda-configuration-file.rst @@ -88,9 +88,11 @@ cordappSignerKeyFingerprintBlacklist *Default:* not defined crlCheckSoftFail - This is a boolean flag that when enabled (i.e. ``true`` value is set) causes the certificate revocation list (CRL) checking will use the soft fail mode. - The soft fail mode allows the revocation check to succeed if the revocation status cannot be determined because of a network error. - If this parameter is set to ``false`` rigorous CRL checking takes place, involving each certificate in the certificate path being checked needs to have the CRL distribution point extension set and pointing to a URL serving a valid CRL. + This is a boolean flag that when enabled (i.e. ``true`` value is set) causes certificate revocation list (CRL) checking to use soft fail mode. + Soft fail mode allows the revocation check to succeed if the revocation status cannot be determined because of a network error. + If this parameter is set to ``false`` rigorous CRL checking takes place. This involves each certificate in the certificate path being checked for a CRL distribution point extension, and that this extension points to a URL serving a valid CRL. + This means that if any CRL URL in the certificate path is inaccessible, the connection with the other party will fail and be marked as bad. + Additionally, if any certificate in the hierarchy, including the self-generated node SSL certificate, is missing a valid CRL URL, then the certificate path will be marked as invalid. *Default:* true diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/bridging/AMQPBridgeManager.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/bridging/AMQPBridgeManager.kt index d60a50bf76..7427a344e7 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/bridging/AMQPBridgeManager.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/bridging/AMQPBridgeManager.kt @@ -34,7 +34,9 @@ import kotlin.concurrent.withLock * The Netty thread pool used by the AMQPBridges is also shared and managed by the AMQPBridgeManager. */ @VisibleForTesting -class AMQPBridgeManager(config: MutualSslConfiguration, maxMessageSize: Int, +class AMQPBridgeManager(config: MutualSslConfiguration, + maxMessageSize: Int, + crlCheckSoftFail: Boolean, private val artemisMessageClientFactory: () -> ArtemisSessionProvider, private val bridgeMetricsService: BridgeMetricsService? = null) : BridgeManager { @@ -43,15 +45,19 @@ class AMQPBridgeManager(config: MutualSslConfiguration, maxMessageSize: Int, private class AMQPConfigurationImpl private constructor(override val keyStore: CertificateStore, override val trustStore: CertificateStore, - override val maxMessageSize: Int) : AMQPConfiguration { - constructor(config: MutualSslConfiguration, maxMessageSize: Int) : this(config.keyStore.get(), config.trustStore.get(), maxMessageSize) + override val maxMessageSize: Int, + override val crlCheckSoftFail: Boolean) : AMQPConfiguration { + constructor(config: MutualSslConfiguration, maxMessageSize: Int, crlCheckSoftFail: Boolean) : this(config.keyStore.get(), config.trustStore.get(), maxMessageSize, crlCheckSoftFail) } - private val amqpConfig: AMQPConfiguration = AMQPConfigurationImpl(config, maxMessageSize) + private val amqpConfig: AMQPConfiguration = AMQPConfigurationImpl(config, maxMessageSize, crlCheckSoftFail) private var sharedEventLoopGroup: EventLoopGroup? = null private var artemis: ArtemisSessionProvider? = null - constructor(config: MutualSslConfiguration, p2pAddress: NetworkHostAndPort, maxMessageSize: Int) : this(config, maxMessageSize, { ArtemisMessagingClient(config, p2pAddress, maxMessageSize) }) + constructor(config: MutualSslConfiguration, + p2pAddress: NetworkHostAndPort, + maxMessageSize: Int, + crlCheckSoftFail: Boolean) : this(config, maxMessageSize, crlCheckSoftFail, { ArtemisMessagingClient(config, p2pAddress, maxMessageSize) }) companion object { private const val NUM_BRIDGE_THREADS = 0 // Default sized pool diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/bridging/BridgeControlListener.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/bridging/BridgeControlListener.kt index c1fb05a20a..46e5062c66 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/bridging/BridgeControlListener.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/bridging/BridgeControlListener.kt @@ -20,18 +20,24 @@ import java.util.* class BridgeControlListener(val config: MutualSslConfiguration, maxMessageSize: Int, + crlCheckSoftFail: Boolean, private val artemisMessageClientFactory: () -> ArtemisSessionProvider, bridgeMetricsService: BridgeMetricsService? = null) : AutoCloseable { private val bridgeId: String = UUID.randomUUID().toString() - private val bridgeManager: BridgeManager = AMQPBridgeManager(config, maxMessageSize, - artemisMessageClientFactory, bridgeMetricsService) + private val bridgeManager: BridgeManager = AMQPBridgeManager( + config, + maxMessageSize, + crlCheckSoftFail, + artemisMessageClientFactory, + bridgeMetricsService) private val validInboundQueues = mutableSetOf() private var artemis: ArtemisSessionProvider? = null private var controlConsumer: ClientConsumer? = null constructor(config: MutualSslConfiguration, p2pAddress: NetworkHostAndPort, - maxMessageSize: Int) : this(config, maxMessageSize, { ArtemisMessagingClient(config, p2pAddress, maxMessageSize) }) + maxMessageSize: Int, + crlCheckSoftFail: Boolean) : this(config, maxMessageSize, crlCheckSoftFail, { ArtemisMessagingClient(config, p2pAddress, maxMessageSize) }) companion object { private val log = contextLogger() diff --git a/node/src/integration-test/kotlin/net/corda/node/amqp/AMQPBridgeTest.kt b/node/src/integration-test/kotlin/net/corda/node/amqp/AMQPBridgeTest.kt index 02350108f0..9b4f89cae7 100644 --- a/node/src/integration-test/kotlin/net/corda/node/amqp/AMQPBridgeTest.kt +++ b/node/src/integration-test/kotlin/net/corda/node/amqp/AMQPBridgeTest.kt @@ -4,6 +4,7 @@ import com.nhaarman.mockito_kotlin.doReturn import com.nhaarman.mockito_kotlin.whenever import net.corda.core.crypto.toStringShort import net.corda.core.internal.div +import net.corda.core.toFuture import net.corda.core.utilities.loggerFor import net.corda.node.services.config.NodeConfiguration import net.corda.node.services.config.configureWithDevSSLCertificate @@ -167,7 +168,25 @@ class AMQPBridgeTest { artemisServer.stop() } - private fun createArtemis(sourceQueueName: String?): Triple { + @Test + fun `bridge with strict CRL checking does not connect to server with invalid certificates`() { + // Note that the opposite of this test (that a connection is established if strict checking is disabled) is carried out by the + // ack/nack test above. "Strict CRL checking" means that soft fail mode is disabled. + val sourceQueueName = "internal.peers." + BOB.publicKey.toStringShort() + val (artemisServer, artemisClient, bridge) = createArtemis(sourceQueueName, crlCheckSoftFail = false) + + createAMQPServer().use { + val connectedFuture = it.onConnection.toFuture() + it.start() + val connectedResult = connectedFuture.get() + assertEquals(false, connectedResult.connected) + } + bridge.stop() + artemisClient.stop() + artemisServer.stop() + } + + private fun createArtemis(sourceQueueName: String?, crlCheckSoftFail: Boolean = true): Triple { val baseDir = temporaryFolder.root.toPath() / "artemis" val certificatesDirectory = baseDir / "certificates" val p2pSslConfiguration = CertificateStoreStubs.P2P.withCertificatesDirectory(certificatesDirectory) @@ -178,7 +197,7 @@ class AMQPBridgeTest { doReturn(certificatesDirectory).whenever(it).certificatesDirectory doReturn(signingCertificateStore).whenever(it).signingCertificateStore doReturn(p2pSslConfiguration).whenever(it).p2pSslOptions - doReturn(true).whenever(it).crlCheckSoftFail + doReturn(crlCheckSoftFail).whenever(it).crlCheckSoftFail doReturn(artemisAddress).whenever(it).p2pAddress doReturn(null).whenever(it).jmxMonitoringHttpPort } @@ -187,7 +206,7 @@ class AMQPBridgeTest { val artemisClient = ArtemisMessagingClient(artemisConfig.p2pSslOptions, artemisAddress, MAX_MESSAGE_SIZE) artemisServer.start() artemisClient.start() - val bridgeManager = AMQPBridgeManager(artemisConfig.p2pSslOptions, artemisAddress, MAX_MESSAGE_SIZE) + val bridgeManager = AMQPBridgeManager(artemisConfig.p2pSslOptions, artemisAddress, MAX_MESSAGE_SIZE, artemisConfig.crlCheckSoftFail) bridgeManager.start() val artemis = artemisClient.started!! if (sourceQueueName != null) { @@ -209,6 +228,7 @@ class AMQPBridgeTest { doReturn(certificatesDirectory).whenever(it).certificatesDirectory doReturn(signingCertificateStore).whenever(it).signingCertificateStore doReturn(p2pSslConfiguration).whenever(it).p2pSslOptions + doReturn(true).whenever(it).crlCheckSoftFail } serverConfig.configureWithDevSSLCertificate() @@ -218,6 +238,7 @@ class AMQPBridgeTest { override val trustStore = serverConfig.p2pSslOptions.trustStore.get() override val trace: Boolean = true override val maxMessageSize: Int = maxMessageSize + override val crlCheckSoftFail: Boolean = serverConfig.crlCheckSoftFail } return AMQPServer("0.0.0.0", amqpAddress.port, diff --git a/node/src/integration-test/kotlin/net/corda/node/amqp/ProtonWrapperTests.kt b/node/src/integration-test/kotlin/net/corda/node/amqp/ProtonWrapperTests.kt index 7eee3af489..f9336d6b19 100644 --- a/node/src/integration-test/kotlin/net/corda/node/amqp/ProtonWrapperTests.kt +++ b/node/src/integration-test/kotlin/net/corda/node/amqp/ProtonWrapperTests.kt @@ -91,8 +91,7 @@ class ProtonWrapperTests { @Test fun `AMPQ Client fails to connect when crl soft fail check is disabled`() { - val amqpServer = createServer(serverPort, CordaX500Name("Rogue 1", "London", "GB"), - maxMessageSize = MAX_MESSAGE_SIZE, crlCheckSoftFail = false) + val amqpServer = createServer(serverPort, maxMessageSize = MAX_MESSAGE_SIZE, crlCheckSoftFail = false) amqpServer.use { amqpServer.start() val amqpClient = createClient() @@ -448,6 +447,7 @@ class ProtonWrapperTests { override val trustStore = clientTruststore override val trace: Boolean = true override val maxMessageSize: Int = maxMessageSize + override val crlCheckSoftFail: Boolean = clientConfig.crlCheckSoftFail } return AMQPClient( listOf(NetworkHostAndPort("localhost", serverPort), @@ -479,6 +479,7 @@ class ProtonWrapperTests { override val trustStore = clientTruststore override val trace: Boolean = true override val maxMessageSize: Int = maxMessageSize + override val crlCheckSoftFail: Boolean = clientConfig.crlCheckSoftFail } return AMQPClient( listOf(NetworkHostAndPort("localhost", serverPort)), @@ -512,6 +513,7 @@ class ProtonWrapperTests { override val trustStore = serverTruststore override val trace: Boolean = true override val maxMessageSize: Int = maxMessageSize + override val crlCheckSoftFail: Boolean = serverConfig.crlCheckSoftFail } return AMQPServer( "0.0.0.0", diff --git a/node/src/main/kotlin/net/corda/node/internal/Node.kt b/node/src/main/kotlin/net/corda/node/internal/Node.kt index fa550bd233..d009c80738 100644 --- a/node/src/main/kotlin/net/corda/node/internal/Node.kt +++ b/node/src/main/kotlin/net/corda/node/internal/Node.kt @@ -256,7 +256,11 @@ open class Node(configuration: NodeConfiguration, startLocalRpcBroker(securityManager) } - val bridgeControlListener = BridgeControlListener(configuration.p2pSslOptions, network.serverAddress, networkParameters.maxMessageSize) + val bridgeControlListener = BridgeControlListener( + configuration.p2pSslOptions, + network.serverAddress, + networkParameters.maxMessageSize, + configuration.crlCheckSoftFail) printBasicNodeInfo("Advertised P2P messaging addresses", nodeInfo.addresses.joinToString()) val rpcServerConfiguration = RPCServerConfiguration.DEFAULT