[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

@@ -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.
+  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.
-  The soft fail mode allows the revocation check to succeed if the revocation status cannot be determined because of a network error.
+  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.
+  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
 

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 {
+                                                            override val maxMessageSize: Int,
-        constructor(config: MutualSslConfiguration, maxMessageSize: Int) : this(config.keyStore.get(), config.trustStore.get(), maxMessageSize)
+                                                            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

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,
+    private val bridgeManager: BridgeManager = AMQPBridgeManager(
-            artemisMessageClientFactory, bridgeMetricsService)
+            config,
+            maxMessageSize,
+            crlCheckSoftFail,
+            artemisMessageClientFactory,
+            bridgeMetricsService)
     private val validInboundQueues = mutableSetOf<String>()
     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()

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<ArtemisMessagingServer, ArtemisMessagingClient, BridgeManager> {
+    @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<ArtemisMessagingServer, ArtemisMessagingClient, BridgeManager> {
         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,

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"),
+        val amqpServer = createServer(serverPort, maxMessageSize = MAX_MESSAGE_SIZE, crlCheckSoftFail = false)
-                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",

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