ENT-8898: Replaced JDK cert revocation with custom plugable implementation (#7322)

node-api/build.gradle

@@ -54,6 +54,9 @@ dependencies {
     testRuntimeOnly "org.junit.vintage:junit-vintage-engine:${junit_vintage_version}"
     testRuntimeOnly "org.junit.jupiter:junit-jupiter-engine:${junit_jupiter_version}"
     testRuntimeOnly "org.junit.platform:junit-platform-launcher:${junit_platform_version}"
+
+    testCompile project(':node-driver')
+
     // Unit testing helpers.
     testCompile "org.assertj:assertj-core:$assertj_version"
     testCompile "org.jetbrains.kotlin:kotlin-test:$kotlin_version"

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

@@ -438,6 +438,8 @@ class X509CertificateFactory {
     fun generateCertPath(vararg certificates: X509Certificate): CertPath = generateCertPath(certificates.asList())
 
     fun generateCertPath(certificates: List<X509Certificate>): CertPath = delegate.generateCertPath(certificates)
+
+    fun generateCRL(input: InputStream): X509CRL = delegate.generateCRL(input) as X509CRL
 }
 
 enum class CertificateType(val keyUsage: KeyUsage, vararg val purposes: KeyPurposeId, val isCA: Boolean, val role: CertRole?) {

node-api/src/main/kotlin/net/corda/nodeapi/internal/protonwrapper/netty/AMQPChannelHandler.kt

@@ -28,7 +28,6 @@ import org.apache.qpid.proton.framing.TransportFrame
 import org.slf4j.MDC
 import java.net.InetSocketAddress
 import java.nio.channels.ClosedChannelException
-import java.security.cert.PKIXRevocationChecker
 import java.security.cert.X509Certificate
 import javax.net.ssl.ExtendedSSLSession
 import javax.net.ssl.SNIHostName
@@ -47,7 +46,6 @@ internal class AMQPChannelHandler(private val serverMode: Boolean,
                                   private val password: String?,
                                   private val trace: Boolean,
                                   private val suppressLogs: Boolean,
-                                  private val revocationChecker: PKIXRevocationChecker,
                                   private val onOpen: (SocketChannel, ConnectionChange) -> Unit,
                                   private val onClose: (SocketChannel, ConnectionChange) -> Unit,
                                   private val onReceive: (ReceivedMessage) -> Unit) : ChannelDuplexHandler() {
@@ -170,13 +168,6 @@ internal class AMQPChannelHandler(private val serverMode: Boolean,
                 } else {
                     handleFailedHandshake(ctx, evt)
                 }
-                if (log.isDebugEnabled) {
-                    withMDC {
-                        revocationChecker.softFailExceptions.forEachIndexed { index, e ->
-                            log.debug("Revocation soft fail exception (${index + 1}/${revocationChecker.softFailExceptions.size})", e)
-                        }
-                    }
-                }
             }
         }
     }

node-api/src/main/kotlin/net/corda/nodeapi/internal/protonwrapper/netty/AMQPClient.kt

@@ -26,7 +26,7 @@ import rx.Observable
 import rx.subjects.PublishSubject
 import java.lang.Long.min
 import java.net.InetSocketAddress
-import java.security.cert.PKIXRevocationChecker
+import java.security.cert.CertPathValidatorException
 import java.util.concurrent.TimeUnit
 import java.util.concurrent.locks.ReentrantLock
 import javax.net.ssl.KeyManagerFactory
@@ -83,6 +83,7 @@ class AMQPClient(private val targets: List<NetworkHostAndPort>,
     private var targetIndex = 0
     private var currentTarget: NetworkHostAndPort = targets.first()
     private var retryInterval = MIN_RETRY_INTERVAL
+    private val revocationChecker = configuration.revocationConfig.createPKIXRevocationChecker()
     private val badCertTargets = mutableSetOf<NetworkHostAndPort>()
     @Volatile
     private var amqpActive = false
@@ -145,15 +146,13 @@ class AMQPClient(private val targets: List<NetworkHostAndPort>,
     private class ClientChannelInitializer(val parent: AMQPClient) : ChannelInitializer<SocketChannel>() {
         private val keyManagerFactory = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm())
         private val trustManagerFactory = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm())
-        private val revocationChecker: PKIXRevocationChecker
         private val conf = parent.configuration
         @Volatile
         private lateinit var amqpChannelHandler: AMQPChannelHandler
 
         init {
             keyManagerFactory.init(conf.keyStore)
-            revocationChecker = createPKIXRevocationChecker(conf.revocationConfig)
+            trustManagerFactory.init(initialiseTrustStoreAndEnableCrlChecking(conf.trustStore, parent.revocationChecker))
-            trustManagerFactory.init(initialiseTrustStoreAndEnableCrlChecking(conf.trustStore, revocationChecker))
         }
 
         @Suppress("ComplexMethod")
@@ -211,7 +210,6 @@ class AMQPClient(private val targets: List<NetworkHostAndPort>,
                     conf.password,
                     conf.trace,
                     false,
-                    revocationChecker,
                     onOpen = { _, change -> onChannelOpen(change) },
                     onClose = { _, change -> onChannelClose(change, target) },
                     onReceive = parent._onReceive::onNext
@@ -330,4 +328,6 @@ class AMQPClient(private val targets: List<NetworkHostAndPort>,
     private val _onConnection = PublishSubject.create<ConnectionChange>().toSerialized()
     val onConnection: Observable<ConnectionChange>
         get() = _onConnection
+
+    val softFailExceptions: List<CertPathValidatorException> get() = revocationChecker.softFailExceptions
 }

node-api/src/main/kotlin/net/corda/nodeapi/internal/protonwrapper/netty/AMQPServer.kt

@@ -25,7 +25,7 @@ import rx.Observable
 import rx.subjects.PublishSubject
 import java.net.BindException
 import java.net.InetSocketAddress
-import java.security.cert.PKIXRevocationChecker
+import java.security.cert.CertPathValidatorException
 import java.util.concurrent.ConcurrentHashMap
 import java.util.concurrent.locks.ReentrantLock
 import javax.net.ssl.KeyManagerFactory
@@ -56,18 +56,17 @@ class AMQPServer(val hostName: String,
     private var bossGroup: EventLoopGroup? = null
     private var workerGroup: EventLoopGroup? = null
     private var serverChannel: Channel? = null
+    private val revocationChecker = configuration.revocationConfig.createPKIXRevocationChecker()
     private val clientChannels = ConcurrentHashMap<InetSocketAddress, SocketChannel>()
 
     private class ServerChannelInitializer(val parent: AMQPServer) : ChannelInitializer<SocketChannel>() {
         private val keyManagerFactory = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm())
         private val trustManagerFactory = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm())
-        private val revocationChecker: PKIXRevocationChecker
         private val conf = parent.configuration
 
         init {
             keyManagerFactory.init(conf.keyStore.value.internal, conf.keyStore.entryPassword.toCharArray())
-            revocationChecker = createPKIXRevocationChecker(conf.revocationConfig)
+            trustManagerFactory.init(initialiseTrustStoreAndEnableCrlChecking(conf.trustStore, parent.revocationChecker))
-            trustManagerFactory.init(initialiseTrustStoreAndEnableCrlChecking(conf.trustStore, revocationChecker))
         }
 
         override fun initChannel(ch: SocketChannel) {
@@ -88,7 +87,6 @@ class AMQPServer(val hostName: String,
                     conf.password,
                     conf.trace,
                     suppressLogs,
-                    revocationChecker,
                     onOpen = ::onChannelOpen,
                     onClose = ::onChannelClose,
                     onReceive = parent._onReceive::onNext
@@ -227,4 +225,6 @@ class AMQPServer(val hostName: String,
     private val _onConnection = PublishSubject.create<ConnectionChange>().toSerialized()
     val onConnection: Observable<ConnectionChange>
         get() = _onConnection
+    
+    val softFailExceptions: List<CertPathValidatorException> get() = revocationChecker.softFailExceptions
 }

node-api/src/main/kotlin/net/corda/nodeapi/internal/protonwrapper/netty/CrlSource.kt

@@ -3,7 +3,8 @@ package net.corda.nodeapi.internal.protonwrapper.netty
 import java.security.cert.X509CRL
 import java.security.cert.X509Certificate
 
-interface ExternalCrlSource {
+@FunctionalInterface
+interface CrlSource {
 
     /**
      * Given certificate provides a set of CRLs, potentially performing remote communication.

node-api/src/main/kotlin/net/corda/nodeapi/internal/protonwrapper/netty/RevocationConfig.kt

@@ -3,11 +3,15 @@ package net.corda.nodeapi.internal.protonwrapper.netty
 import com.typesafe.config.Config
 import net.corda.nodeapi.internal.config.ConfigParser
 import net.corda.nodeapi.internal.config.CustomConfigParser
+import net.corda.nodeapi.internal.revocation.CertDistPointCrlSource
+import net.corda.nodeapi.internal.revocation.CordaRevocationChecker
+import java.security.cert.PKIXRevocationChecker
 
 /**
  * Data structure for controlling the way how Certificate Revocation Lists are handled.
  */
 @CustomConfigParser(parser = RevocationConfigParser::class)
+// TODO This and RevocationConfigImpl should really be a single sealed data type
 interface RevocationConfig {
 
     enum class Mode {
@@ -26,7 +30,7 @@ interface RevocationConfig {
 
         /**
          * CRLs are obtained from external source
-         * @see ExternalCrlSource
+         * @see CrlSource
          */
         EXTERNAL_SOURCE,
 
@@ -39,14 +43,23 @@ interface RevocationConfig {
     val mode: Mode
 
     /**
-     * Optional `ExternalCrlSource` which only makes sense with `mode` = `EXTERNAL_SOURCE`
+     * Optional [CrlSource] which only makes sense with `mode` = `EXTERNAL_SOURCE`
      */
-    val externalCrlSource: ExternalCrlSource?
+    val externalCrlSource: CrlSource?
 
     /**
-     * Creates a copy of `RevocationConfig` with ExternalCrlSource enriched
+     * Creates a copy of [RevocationConfig] enriched by a [CrlSource].
      */
-    fun enrichExternalCrlSource(sourceFunc: (() -> ExternalCrlSource)?): RevocationConfig
+    fun enrichExternalCrlSource(sourceFunc: (() -> CrlSource)?): RevocationConfig
+
+    fun createPKIXRevocationChecker(): PKIXRevocationChecker {
+        return when (mode) {
+            Mode.OFF -> AllowAllRevocationChecker
+            Mode.EXTERNAL_SOURCE -> CordaRevocationChecker(externalCrlSource!!, softFail = true)
+            Mode.SOFT_FAIL -> CordaRevocationChecker(CertDistPointCrlSource(), softFail = true)
+            Mode.HARD_FAIL -> CordaRevocationChecker(CertDistPointCrlSource(), softFail = false)
+        }
+    }
 }
 
 /**
@@ -54,13 +67,21 @@ interface RevocationConfig {
  */
 fun Boolean.toRevocationConfig() = if(this) RevocationConfigImpl(RevocationConfig.Mode.SOFT_FAIL) else RevocationConfigImpl(RevocationConfig.Mode.HARD_FAIL)
 
-data class RevocationConfigImpl(override val mode: RevocationConfig.Mode, override val externalCrlSource: ExternalCrlSource? = null) : RevocationConfig {
+data class RevocationConfigImpl(override val mode: RevocationConfig.Mode, override val externalCrlSource: CrlSource? = null) : RevocationConfig {
-    override fun enrichExternalCrlSource(sourceFunc: (() -> ExternalCrlSource)?): RevocationConfig {
+    init {
+        if (mode == RevocationConfig.Mode.EXTERNAL_SOURCE) {
+            requireNotNull(externalCrlSource) { "externalCrlSource must not be null" }
+        }
+    }
+
+    // TODO This doesn't really need to be a member method. All it does is change externalCrlSource if applicable, which is the same as
+    //  just creating a new RevocationConfigImpl with that CrlSource.
+    override fun enrichExternalCrlSource(sourceFunc: (() -> CrlSource)?): RevocationConfig {
         return if (mode != RevocationConfig.Mode.EXTERNAL_SOURCE) {
             this
         } else {
-            assert(sourceFunc != null) { "There should be a way to obtain ExternalCrlSource" }
+            val func = requireNotNull(sourceFunc) { "There should be a way to obtain CrlSource" }
-            copy(externalCrlSource = sourceFunc!!())
+            copy(externalCrlSource = func())
         }
     }
 }

node-api/src/main/kotlin/net/corda/nodeapi/internal/protonwrapper/netty/SSLHelper.kt

@@ -13,15 +13,17 @@ import net.corda.core.identity.CordaX500Name
 import net.corda.core.internal.VisibleForTesting
 import net.corda.core.utilities.NetworkHostAndPort
 import net.corda.core.utilities.contextLogger
+import net.corda.core.utilities.debug
 import net.corda.core.utilities.toHex
 import net.corda.nodeapi.internal.ArtemisTcpTransport
 import net.corda.nodeapi.internal.config.CertificateStore
 import net.corda.nodeapi.internal.crypto.toBc
 import net.corda.nodeapi.internal.crypto.x509
-import net.corda.nodeapi.internal.protonwrapper.netty.revocation.ExternalSourceRevocationChecker
 import org.bouncycastle.asn1.ASN1InputStream
+import org.bouncycastle.asn1.ASN1Primitive
 import org.bouncycastle.asn1.DERIA5String
 import org.bouncycastle.asn1.DEROctetString
+import org.bouncycastle.asn1.x500.X500Name
 import org.bouncycastle.asn1.x509.AuthorityKeyIdentifier
 import org.bouncycastle.asn1.x509.CRLDistPoint
 import org.bouncycastle.asn1.x509.DistributionPointName
@@ -30,13 +32,15 @@ import org.bouncycastle.asn1.x509.GeneralName
 import org.bouncycastle.asn1.x509.GeneralNames
 import org.bouncycastle.asn1.x509.SubjectKeyIdentifier
 import org.slf4j.LoggerFactory
-import java.io.ByteArrayInputStream
 import java.net.Socket
+import java.net.URI
 import java.security.KeyStore
 import java.security.cert.*
 import java.util.*
 import java.util.concurrent.Executor
 import javax.net.ssl.*
+import javax.security.auth.x500.X500Principal
+import kotlin.collections.HashMap
 import kotlin.system.measureTimeMillis
 
 private const val HOSTNAME_FORMAT = "%s.corda.net"
@@ -46,40 +50,61 @@ internal const val DP_DEFAULT_ANSWER = "NO CRLDP ext"
 
 internal val logger = LoggerFactory.getLogger("net.corda.nodeapi.internal.protonwrapper.netty.SSLHelper")
 
-fun X509Certificate.distributionPoints(): Set<String> {
+/**
-    logger.debug("Checking CRLDPs for $subjectX500Principal")
+ * Returns all the CRL distribution points in the certificate as [URI]s along with the CRL issuer names, if any.
+ */
+@Suppress("ComplexMethod")
+fun X509Certificate.distributionPoints(): Map<URI, List<X500Principal>?> {
+    logger.debug { "Checking CRLDPs for $subjectX500Principal" }
 
     val crldpExtBytes = getExtensionValue(Extension.cRLDistributionPoints.id)
     if (crldpExtBytes == null) {
         logger.debug(DP_DEFAULT_ANSWER)
-        return emptySet()
+        return emptyMap()
     }
 
-    val derObjCrlDP = ASN1InputStream(ByteArrayInputStream(crldpExtBytes)).readObject()
+    val derObjCrlDP = crldpExtBytes.toAsn1Object()
     val dosCrlDP = derObjCrlDP as? DEROctetString
     if (dosCrlDP == null) {
         logger.error("Expected to have DEROctetString, actual type: ${derObjCrlDP.javaClass}")
-        return emptySet()
+        return emptyMap()
     }
-    val crldpExtOctetsBytes = dosCrlDP.octets
+    val dpObj = dosCrlDP.octets.toAsn1Object()
-    val dpObj = ASN1InputStream(ByteArrayInputStream(crldpExtOctetsBytes)).readObject()
+    val crlDistPoint = CRLDistPoint.getInstance(dpObj)
-    val distPoint = CRLDistPoint.getInstance(dpObj)
+    if (crlDistPoint == null) {
-    if (distPoint == null) {
         logger.error("Could not instantiate CRLDistPoint, from: $dpObj")
-        return emptySet()
+        return emptyMap()
     }
 
-    val dpNames = distPoint.distributionPoints.mapNotNull { it.distributionPoint }.filter { it.type == DistributionPointName.FULL_NAME }
+    val dpMap = HashMap<URI, List<X500Principal>?>()
-    val generalNames = dpNames.flatMap { GeneralNames.getInstance(it.name).names.asList() }
+    for (distributionPoint in crlDistPoint.distributionPoints) {
-    return generalNames.filter { it.tagNo == GeneralName.uniformResourceIdentifier}.map { DERIA5String.getInstance(it.name).string }.toSet()
+        val distributionPointName = distributionPoint.distributionPoint
+        if (distributionPointName?.type != DistributionPointName.FULL_NAME) continue
+        val issuerNames = distributionPoint.crlIssuer?.names?.mapNotNull {
+            if (it.tagNo == GeneralName.directoryName) {
+                X500Principal(X500Name.getInstance(it.name).encoded)
+            } else {
+                null
+            }
+        }
+        for (generalName in GeneralNames.getInstance(distributionPointName.name).names) {
+            if (generalName.tagNo == GeneralName.uniformResourceIdentifier) {
+                val uri = URI(DERIA5String.getInstance(generalName.name).string)
+                dpMap[uri] = issuerNames
+            }
+        }
+    }
+    return dpMap
 }
 
 fun X509Certificate.distributionPointsToString(): String {
-    return with(distributionPoints()) {
+    return with(distributionPoints().keys) {
         if (isEmpty()) DP_DEFAULT_ANSWER else sorted().joinToString()
     }
 }
 
+fun ByteArray.toAsn1Object(): ASN1Primitive = ASN1InputStream(this).readObject()
+
 fun certPathToString(certPath: Array<out X509Certificate>?): String {
     if (certPath == null) {
         return "<empty certpath>"
@@ -256,10 +281,9 @@ fun createAndInitSslContext(keyManagerFactory: KeyManagerFactory, trustManagerFa
     return sslContext
 }
 
-@VisibleForTesting
 fun initialiseTrustStoreAndEnableCrlChecking(trustStore: CertificateStore,
                                              revocationConfig: RevocationConfig): CertPathTrustManagerParameters {
-    return initialiseTrustStoreAndEnableCrlChecking(trustStore, createPKIXRevocationChecker(revocationConfig))
+    return initialiseTrustStoreAndEnableCrlChecking(trustStore, revocationConfig.createPKIXRevocationChecker())
 }
 
 fun initialiseTrustStoreAndEnableCrlChecking(trustStore: CertificateStore,
@@ -269,33 +293,6 @@ fun initialiseTrustStoreAndEnableCrlChecking(trustStore: CertificateStore,
     return CertPathTrustManagerParameters(pkixParams)
 }
 
-fun createPKIXRevocationChecker(revocationConfig: RevocationConfig): PKIXRevocationChecker {
-    return when (revocationConfig.mode) {
-        RevocationConfig.Mode.OFF -> AllowAllRevocationChecker  // Custom PKIXRevocationChecker skipping CRL check
-        RevocationConfig.Mode.EXTERNAL_SOURCE -> {
-            require(revocationConfig.externalCrlSource != null) { "externalCrlSource must not be null" }
-            ExternalSourceRevocationChecker(revocationConfig.externalCrlSource!!) { Date() } // Custom PKIXRevocationChecker which uses `externalCrlSource`
-        }
-        else -> {
-            val certPathBuilder = CertPathBuilder.getInstance("PKIX")
-            val pkixRevocationChecker = certPathBuilder.revocationChecker as PKIXRevocationChecker
-            val options = EnumSet.of(
-                    // Prefer CRL over OCSP
-                    PKIXRevocationChecker.Option.PREFER_CRLS,
-                    // Don't fall back to OCSP checking
-                    PKIXRevocationChecker.Option.NO_FALLBACK
-            )
-            if (revocationConfig.mode == RevocationConfig.Mode.SOFT_FAIL) {
-                // Allow revocation check to succeed if the revocation status cannot be determined for one of
-                // the following reasons: The CRL or OCSP response cannot be obtained because of a network error.
-                options += PKIXRevocationChecker.Option.SOFT_FAIL
-            }
-            pkixRevocationChecker.options = options
-            pkixRevocationChecker
-        }
-    }
-}
-
 /**
  * Creates a special SNI handler used only when openSSL is used for AMQPServer
  */

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

@@ -0,0 +1,84 @@
+package net.corda.nodeapi.internal.revocation
+
+import com.github.benmanes.caffeine.cache.Caffeine
+import com.github.benmanes.caffeine.cache.LoadingCache
+import net.corda.core.internal.readFully
+import net.corda.nodeapi.internal.crypto.X509CertificateFactory
+import net.corda.nodeapi.internal.protonwrapper.netty.CrlSource
+import net.corda.nodeapi.internal.protonwrapper.netty.distributionPoints
+import java.net.URI
+import java.security.cert.X509CRL
+import java.security.cert.X509Certificate
+import java.util.concurrent.TimeUnit
+import javax.security.auth.x500.X500Principal
+
+/**
+ * [CrlSource] which downloads CRLs from the distribution points in the X509 certificate.
+ */
+class CertDistPointCrlSource : CrlSource {
+    companion object {
+        // The default SSL handshake timeout is 60s (DEFAULT_SSL_HANDSHAKE_TIMEOUT). Considering there are 3 CRLs endpoints to check in a
+        // node handshake, we want to keep the total timeout within that.
+        private const val DEFAULT_CONNECT_TIMEOUT = 9_000
+        private const val DEFAULT_READ_TIMEOUT = 9_000
+        private const val DEFAULT_CACHE_SIZE = 185L  // Same default as the JDK (URICertStore)
+        private const val DEFAULT_CACHE_EXPIRY = 5 * 60 * 1000L
+
+        private val cache: LoadingCache<URI, X509CRL> = Caffeine.newBuilder()
+                .maximumSize(java.lang.Long.getLong("net.corda.dpcrl.cache.size", DEFAULT_CACHE_SIZE))
+                .expireAfterWrite(java.lang.Long.getLong("net.corda.dpcrl.cache.expiry", DEFAULT_CACHE_EXPIRY), TimeUnit.MILLISECONDS)
+                .build(::retrieveCRL)
+
+        private val connectTimeout = Integer.getInteger("net.corda.dpcrl.connect.timeout", DEFAULT_CONNECT_TIMEOUT)
+        private val readTimeout = Integer.getInteger("net.corda.dpcrl.read.timeout", DEFAULT_READ_TIMEOUT)
+
+        private fun retrieveCRL(uri: URI): X509CRL {
+            val bytes = run {
+                val conn = uri.toURL().openConnection()
+                conn.connectTimeout = connectTimeout
+                conn.readTimeout = readTimeout
+                // Read all bytes first and then pass them into the CertificateFactory. This may seem unnecessary when generateCRL already takes
+                // in an InputStream, but the JDK implementation (sun.security.provider.X509Factory.engineGenerateCRL) converts any IOException
+                // into CRLException and drops the cause chain.
+                conn.getInputStream().readFully()
+            }
+            return X509CertificateFactory().generateCRL(bytes.inputStream())
+        }
+    }
+
+    @Suppress("TooGenericExceptionCaught")
+    override fun fetch(certificate: X509Certificate): Set<X509CRL> {
+        val approvedCRLs = HashSet<X509CRL>()
+        var exception: Exception? = null
+        for ((distPointUri, issuerNames) in certificate.distributionPoints()) {
+            try {
+                val possibleCRL = getPossibleCRL(distPointUri)
+                if (verifyCRL(possibleCRL, certificate, issuerNames)) {
+                    approvedCRLs += possibleCRL
+                }
+            } catch (e: Exception) {
+                if (exception == null) {
+                    exception = e
+                } else {
+                    exception.addSuppressed(e)
+                }
+            }
+        }
+        // Only throw if no CRLs are retrieved
+        if (exception != null && approvedCRLs.isEmpty()) {
+            throw exception
+        } else {
+            return approvedCRLs
+        }
+    }
+
+    private fun getPossibleCRL(uri: URI): X509CRL {
+        return cache[uri]!!
+    }
+
+    // DistributionPointFetcher.verifyCRL
+    private fun verifyCRL(crl: X509CRL, certificate: X509Certificate, distPointIssuerNames: List<X500Principal>?): Boolean {
+        val crlIssuer = crl.issuerX500Principal
+        return distPointIssuerNames?.any { it == crlIssuer } ?: (certificate.issuerX500Principal == crlIssuer)
+    }
+}

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

@@ -1,30 +1,53 @@
-package net.corda.nodeapi.internal.protonwrapper.netty.revocation
+package net.corda.nodeapi.internal.revocation
 
 import net.corda.core.utilities.contextLogger
-import net.corda.nodeapi.internal.protonwrapper.netty.ExternalCrlSource
+import net.corda.nodeapi.internal.protonwrapper.netty.CrlSource
 import org.bouncycastle.asn1.x509.Extension
 import java.security.cert.CRLReason
 import java.security.cert.CertPathValidatorException
+import java.security.cert.CertPathValidatorException.BasicReason
 import java.security.cert.Certificate
 import java.security.cert.CertificateRevokedException
 import java.security.cert.PKIXRevocationChecker
 import java.security.cert.X509CRL
 import java.security.cert.X509Certificate
 import java.util.*
+import kotlin.collections.ArrayList
 
 /**
- * Implementation of [PKIXRevocationChecker] which determines whether certificate is revoked using [externalCrlSource] which knows how to
+ * Custom [PKIXRevocationChecker] which delegates to a plugable [CrlSource] to retrieve the CRLs for certificate revocation checks.
- * obtain a set of CRLs for a given certificate from an external source
  */
-class ExternalSourceRevocationChecker(private val externalCrlSource: ExternalCrlSource, private val dateSource: () -> Date) : PKIXRevocationChecker() {
+class CordaRevocationChecker(private val crlSource: CrlSource,
-
+                             private val softFail: Boolean,
+                             private val dateSource: () -> Date = ::Date) : PKIXRevocationChecker() {
     companion object {
         private val logger = contextLogger()
     }
 
+    private val softFailExceptions = ArrayList<CertPathValidatorException>()
+
     override fun check(cert: Certificate, unresolvedCritExts: MutableCollection<String>?) {
         val x509Certificate = cert as X509Certificate
-        checkApprovedCRLs(x509Certificate, externalCrlSource.fetch(x509Certificate))
+        checkApprovedCRLs(x509Certificate, getCRLs(x509Certificate))
+    }
+
+    @Suppress("TooGenericExceptionCaught")
+    private fun getCRLs(cert: X509Certificate): Set<X509CRL> {
+        val crls = try {
+            crlSource.fetch(cert)
+        } catch (e: Exception) {
+            if (softFail) {
+                addSoftFailException(e)
+                return emptySet()
+            } else {
+                throw undeterminedRevocationException("Unable to retrieve CRLs", e)
+            }
+        }
+        if (crls.isNotEmpty() || softFail) {
+            return crls
+        }
+        // Note, the JDK tries to find a valid CRL from a different signing key before giving up (RevocationChecker.verifyWithSeparateSigningKey)
+        throw undeterminedRevocationException("Could not find any valid CRLs", null)
     }
 
     /**
@@ -47,11 +70,11 @@ class ExternalSourceRevocationChecker(private val externalCrlSource: ExternalCrl
                  * 5.3 of RFC 5280).
                  */
                 val unresCritExts = entry.criticalExtensionOIDs
-                if (unresCritExts != null && !unresCritExts.isEmpty()) {
+                if (unresCritExts != null && unresCritExts.isNotEmpty()) {
                     /* remove any that we will process */
                     unresCritExts.remove(Extension.cRLDistributionPoints.id)
                     unresCritExts.remove(Extension.certificateIssuer.id)
-                    if (!unresCritExts.isEmpty()) {
+                    if (unresCritExts.isNotEmpty()) {
                         throw CertPathValidatorException(
                                 "Unrecognized critical extension(s) in revoked CRL entry: $unresCritExts")
                     }
@@ -64,14 +87,22 @@ class ExternalSourceRevocationChecker(private val externalCrlSource: ExternalCrl
                             revocationDate, reasonCode,
                             crl.issuerX500Principal, mutableMapOf())
                     throw CertPathValidatorException(
-                            t.message, t, null, -1, CertPathValidatorException.BasicReason.REVOKED)
+                            t.message, t, null, -1, BasicReason.REVOKED)
                 }
             }
         }
     }
 
+    /**
+     * This is set to false intentionally for security reasons.
+     * It ensures that certificates are provided in reverse direction (from most-trusted CA to target certificate)
+     * after the necessary validation checks have already been performed.
+     *
+     * If that wasn't the case, we could be reaching out to CRL endpoints for invalid certificates, which would open security holes
+     * e.g. systems that are not part of a Corda network could force a Corda firewall to initiate outbound requests to systems under their control.
+     */
     override fun isForwardCheckingSupported(): Boolean {
-        return true
+        return false
     }
 
     override fun getSupportedExtensions(): MutableSet<String>? {
@@ -79,10 +110,19 @@ class ExternalSourceRevocationChecker(private val externalCrlSource: ExternalCrl
     }
 
     override fun init(forward: Boolean) {
-        // Nothing to do
+        softFailExceptions.clear()
     }
 
     override fun getSoftFailExceptions(): MutableList<CertPathValidatorException> {
-        return LinkedList()
+        return Collections.unmodifiableList(softFailExceptions)
+    }
+
+    private fun addSoftFailException(e: Exception) {
+        logger.debug("Soft fail exception", e)
+        softFailExceptions += undeterminedRevocationException(e.message, e)
+    }
+
+    private fun undeterminedRevocationException(message: String?, cause: Throwable?): CertPathValidatorException {
+        return CertPathValidatorException(message, cause, null, -1, BasicReason.UNDETERMINED_REVOCATION_STATUS)
     }
 }

node-api/src/test/kotlin/net/corda/nodeapi/internal/revocation/CertDistPointCrlSourceTest.kt

@@ -0,0 +1,51 @@
+package net.corda.nodeapi.internal.revocation
+
+import net.corda.core.crypto.Crypto
+import net.corda.core.utilities.NetworkHostAndPort
+import net.corda.nodeapi.internal.createDevNodeCa
+import net.corda.testing.core.ALICE_NAME
+import net.corda.testing.node.internal.network.CrlServer
+import org.assertj.core.api.Assertions.assertThat
+import org.bouncycastle.jce.provider.BouncyCastleProvider
+import org.junit.After
+import org.junit.Before
+import org.junit.Test
+import java.math.BigInteger
+
+class CertDistPointCrlSourceTest {
+    private lateinit var crlServer: CrlServer
+
+    @Before
+    fun setUp() {
+        // Do not use Security.addProvider(BouncyCastleProvider()) to avoid EdDSA signature disruption in other tests.
+        Crypto.findProvider(BouncyCastleProvider.PROVIDER_NAME)
+        crlServer = CrlServer(NetworkHostAndPort("localhost", 0))
+        crlServer.start()
+    }
+
+    @After
+    fun tearDown() {
+        if (::crlServer.isInitialized) {
+            crlServer.close()
+        }
+    }
+
+    @Test(timeout=300_000)
+	fun `happy path`() {
+        val crlSource = CertDistPointCrlSource()
+
+        with(crlSource.fetch(crlServer.intermediateCa.certificate)) {
+            assertThat(size).isEqualTo(1)
+            assertThat(single().revokedCertificates).isNull()
+        }
+
+        val nodeCaCert = crlServer.replaceNodeCertDistPoint(createDevNodeCa(crlServer.intermediateCa, ALICE_NAME).certificate)
+
+        crlServer.revokedNodeCerts += listOf(BigInteger.ONE, BigInteger.TEN)
+        with(crlSource.fetch(nodeCaCert)) {  // Use a different cert to avoid the cache
+            assertThat(size).isEqualTo(1)
+            val revokedCertificates = single().revokedCertificates
+            assertThat(revokedCertificates.map { it.serialNumber }).containsExactlyInAnyOrder(BigInteger.ONE, BigInteger.TEN)
+        }
+    }
+}

node-api/src/test/kotlin/net/corda/nodeapi/internal/revocation/CordaRevocationCheckerTest.kt

@@ -1,26 +1,27 @@
-package net.corda.nodeapi.internal.protonwrapper.netty.revocation
+package net.corda.nodeapi.internal.revocation
 
 import net.corda.core.utilities.Try
 import net.corda.nodeapi.internal.DEV_CA_KEY_STORE_PASS
 import net.corda.nodeapi.internal.DEV_CA_PRIVATE_KEY_PASS
 import net.corda.nodeapi.internal.config.CertificateStore
 import net.corda.nodeapi.internal.crypto.X509Utilities
-import net.corda.nodeapi.internal.protonwrapper.netty.ExternalCrlSource
+import net.corda.nodeapi.internal.protonwrapper.netty.CrlSource
 import org.bouncycastle.jcajce.provider.asymmetric.x509.CertificateFactory
 import org.junit.Test
 import java.math.BigInteger
-
 import java.security.cert.X509CRL
 import java.security.cert.X509Certificate
-import java.sql.Date
+import java.time.LocalDate
+import java.time.ZoneOffset
+import java.util.*
 import kotlin.test.assertEquals
 import kotlin.test.assertTrue
 
-class ExternalSourceRevocationCheckerTest {
+class CordaRevocationCheckerTest {
 
     @Test(timeout=300_000)
 	fun checkRevoked() {
-        val checkResult = performCheckOnDate(Date.valueOf("2019-09-27"))
+        val checkResult = performCheckOnDate(LocalDate.of(2019, 9, 27))
         val failedChecks = checkResult.filterNot { it.second.isSuccess }
         assertEquals(1, failedChecks.size)
         assertEquals(BigInteger.valueOf(8310484079152632582), failedChecks.first().first.serialNumber)
@@ -28,11 +29,11 @@ class ExternalSourceRevocationCheckerTest {
 
     @Test(timeout=300_000)
 	fun checkTooEarly() {
-        val checkResult = performCheckOnDate(Date.valueOf("2019-08-27"))
+        val checkResult = performCheckOnDate(LocalDate.of(2019, 8, 27))
         assertTrue(checkResult.all { it.second.isSuccess })
     }
 
-    private fun performCheckOnDate(date: Date): List<Pair<X509Certificate, Try<Unit>>> {
+    private fun performCheckOnDate(date: LocalDate): List<Pair<X509Certificate, Try<Unit>>> {
         val certStore = CertificateStore.fromResource(
                 "net/corda/nodeapi/internal/protonwrapper/netty/sslkeystore_Revoked.jks",
                 DEV_CA_KEY_STORE_PASS, DEV_CA_PRIVATE_KEY_PASS)
@@ -40,16 +41,17 @@ class ExternalSourceRevocationCheckerTest {
         val resourceAsStream = javaClass.getResourceAsStream("/net/corda/nodeapi/internal/protonwrapper/netty/doorman.crl")
         val crl = CertificateFactory().engineGenerateCRL(resourceAsStream) as X509CRL
 
-        //val crlHolder = X509CRLHolder(resourceAsStream)
+        val crlSource = object : CrlSource {
-        //crlHolder.revokedCertificates as X509CRLEntryHolder
-
-        val instance = ExternalSourceRevocationChecker(object : ExternalCrlSource {
             override fun fetch(certificate: X509Certificate): Set<X509CRL> = setOf(crl)
-        }) { date }
+        }
+        val checker = CordaRevocationChecker(crlSource,
+                softFail = true,
+                dateSource = { Date.from(date.atStartOfDay().toInstant(ZoneOffset.UTC)) }
+        )
 
         return certStore.query {
             getCertificateChain(X509Utilities.CORDA_CLIENT_TLS).map {
-                Pair(it, Try.on { instance.check(it, mutableListOf()) })
+                Pair(it, Try.on { checker.check(it, mutableListOf()) })
             }
         }
     }

node/build.gradle

@@ -265,7 +265,7 @@ tasks.register('integrationTest', Test) {
     classpath = sourceSets.integrationTest.runtimeClasspath
     maxParallelForks = (System.env.CORDA_NODE_INT_TESTING_FORKS == null) ? 1 : "$System.env.CORDA_NODE_INT_TESTING_FORKS".toInteger()
     // CertificateRevocationListNodeTests
-    systemProperty 'com.sun.security.crl.timeout', '4'
+    systemProperty 'net.corda.dpcrl.connect.timeout', '4000'
 }
 
 tasks.register('slowIntegrationTest', Test) {

node/src/integration-test/kotlin/net/corda/node/amqp/CertificateRevocationListNodeTests.kt

@@ -5,6 +5,7 @@ import com.nhaarman.mockito_kotlin.whenever
 import net.corda.core.crypto.Crypto
 import net.corda.core.identity.CordaX500Name
 import net.corda.core.internal.div
+import net.corda.core.internal.rootCause
 import net.corda.core.internal.times
 import net.corda.core.toFuture
 import net.corda.core.utilities.NetworkHostAndPort
@@ -17,7 +18,7 @@ import net.corda.nodeapi.internal.ArtemisMessagingClient
 import net.corda.nodeapi.internal.ArtemisMessagingComponent.Companion.P2P_PREFIX
 import net.corda.nodeapi.internal.config.CertificateStoreSupplier
 import net.corda.nodeapi.internal.config.MutualSslConfiguration
-import net.corda.nodeapi.internal.crypto.*
+import net.corda.nodeapi.internal.crypto.X509Utilities
 import net.corda.nodeapi.internal.protonwrapper.messages.MessageStatus
 import net.corda.nodeapi.internal.protonwrapper.netty.AMQPClient
 import net.corda.nodeapi.internal.protonwrapper.netty.AMQPConfiguration
@@ -29,27 +30,25 @@ import net.corda.testing.core.CHARLIE_NAME
 import net.corda.testing.core.MAX_MESSAGE_SIZE
 import net.corda.testing.driver.internal.incrementalPortAllocation
 import net.corda.testing.node.internal.network.CrlServer
+import net.corda.testing.node.internal.network.CrlServer.Companion.EMPTY_CRL
 import net.corda.testing.node.internal.network.CrlServer.Companion.FORBIDDEN_CRL
+import net.corda.testing.node.internal.network.CrlServer.Companion.NODE_CRL
 import net.corda.testing.node.internal.network.CrlServer.Companion.withCrlDistPoint
 import org.apache.activemq.artemis.api.core.RoutingType
 import org.assertj.core.api.Assertions.assertThat
 import org.assertj.core.api.Assertions.assertThatIllegalArgumentException
-import org.bouncycastle.asn1.x500.X500Name
-import org.bouncycastle.asn1.x509.*
 import org.bouncycastle.jce.provider.BouncyCastleProvider
 import org.junit.After
 import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
 import org.junit.rules.TemporaryFolder
-import java.math.BigInteger
+import java.net.SocketTimeoutException
 import java.security.cert.X509Certificate
 import java.time.Duration
-import java.util.*
 import java.util.concurrent.atomic.AtomicInteger
 import kotlin.test.assertEquals
 
-@Suppress("LongParameterList")
 class CertificateRevocationListNodeTests {
     @Rule
     @JvmField
@@ -62,15 +61,12 @@ class CertificateRevocationListNodeTests {
     private lateinit var amqpServer: AMQPServer
     private lateinit var amqpClient: AMQPClient
 
-    private val revokedNodeCerts: MutableList<BigInteger> = mutableListOf()
-    private val revokedIntermediateCerts: MutableList<BigInteger> = mutableListOf()
-
     private abstract class AbstractNodeConfiguration : NodeConfiguration
 
     companion object {
         private val unreachableIpCounter = AtomicInteger(1)
 
-        private val crlTimeout = Duration.ofSeconds(System.getProperty("com.sun.security.crl.timeout").toLong())
+        private val crlConnectTimeout = Duration.ofMillis(System.getProperty("net.corda.dpcrl.connect.timeout").toLong())
 
         /**
          * Use this method to get a unqiue unreachable IP address. Subsequent uses of the same IP for connection timeout testing purposes
@@ -86,7 +82,7 @@ class CertificateRevocationListNodeTests {
     fun setUp() {
         // Do not use Security.addProvider(BouncyCastleProvider()) to avoid EdDSA signature disruption in other tests.
         Crypto.findProvider(BouncyCastleProvider.PROVIDER_NAME)
-        crlServer = CrlServer(NetworkHostAndPort("localhost", 0), revokedNodeCerts, revokedIntermediateCerts)
+        crlServer = CrlServer(NetworkHostAndPort("localhost", 0))
         crlServer.start()
     }
 
@@ -205,12 +201,13 @@ class CertificateRevocationListNodeTests {
         verifyAMQPConnection(
                 crlCheckSoftFail = true,
                 nodeCrlDistPoint = "http://${newUnreachableIpAddress()}/crl/unreachable.crl",
-                sslHandshakeTimeout = crlTimeout * 2,
+                sslHandshakeTimeout = crlConnectTimeout * 2,
                 expectedConnectStatus = true
         )
-        // We could use PKIXRevocationChecker.getSoftFailExceptions() to make sure timeout exceptions did actually occur, but the JDK seems
+        val timeoutExceptions = (amqpServer.softFailExceptions + amqpClient.softFailExceptions)
-        // to have a bug in the older 8 builds where this method returns an empty list. Newer builds don't have this issue, but we need to
+                .map { it.rootCause }
-        // be able to support that certain minimum build.
+                .filterIsInstance<SocketTimeoutException>()
+        assertThat(timeoutExceptions).isNotEmpty
     }
 
     @Test(timeout=300_000)
@@ -218,19 +215,17 @@ class CertificateRevocationListNodeTests {
         verifyAMQPConnection(
                 crlCheckSoftFail = true,
                 nodeCrlDistPoint = "http://${newUnreachableIpAddress()}/crl/unreachable.crl",
-                sslHandshakeTimeout = crlTimeout / 2,
+                sslHandshakeTimeout = crlConnectTimeout / 2,
                 expectedConnectStatus = false
         )
     }
 
     @Test(timeout=300_000)
 	fun `verify CRL algorithms`() {
-        val emptyCrl = "empty.crl"
-
         val crl = crlServer.createRevocationList(
                 "SHA256withECDSA",
                 crlServer.rootCa,
-                emptyCrl,
+                EMPTY_CRL,
                 true,
                 emptyList()
         )
@@ -242,7 +237,7 @@ class CertificateRevocationListNodeTests {
             crlServer.createRevocationList(
                     "EC",
                     crlServer.rootCa,
-                    emptyCrl,
+                    EMPTY_CRL,
                     true,
                     emptyList()
             )
@@ -284,11 +279,8 @@ class CertificateRevocationListNodeTests {
                 crlCheckArtemisServer = true,
                 expectedStatus = MessageStatus.Acknowledged,
                 nodeCrlDistPoint = "http://${newUnreachableIpAddress()}/crl/unreachable.crl",
-                sslHandshakeTimeout = crlTimeout * 3
+                sslHandshakeTimeout = crlConnectTimeout * 3
         )
-        // We could use PKIXRevocationChecker.getSoftFailExceptions() to make sure timeout exceptions did actually occur, but the JDK seems
-        // to have a bug in the older 8 builds where this method returns an empty list. Newer builds don't have this issue, but we need to
-        // be able to support that certain minimum build.
     }
 
     @Test(timeout = 300_000)
@@ -298,7 +290,7 @@ class CertificateRevocationListNodeTests {
                 crlCheckArtemisServer = true,
                 expectedConnected = false,
                 nodeCrlDistPoint = "http://${newUnreachableIpAddress()}/crl/unreachable.crl",
-                sslHandshakeTimeout = crlTimeout / 2
+                sslHandshakeTimeout = crlConnectTimeout / 2
         )
     }
 
@@ -334,8 +326,8 @@ class CertificateRevocationListNodeTests {
 
     private fun createAMQPClient(targetPort: Int,
                                  crlCheckSoftFail: Boolean,
-                                 nodeCrlDistPoint: String? = "http://${crlServer.hostAndPort}/crl/node.crl",
+                                 nodeCrlDistPoint: String? = "http://${crlServer.hostAndPort}/crl/$NODE_CRL",
-                                 tlsCrlDistPoint: String? = "http://${crlServer.hostAndPort}/crl/empty.crl",
+                                 tlsCrlDistPoint: String? = "http://${crlServer.hostAndPort}/crl/$EMPTY_CRL",
                                  maxMessageSize: Int = MAX_MESSAGE_SIZE): X509Certificate {
         val baseDirectory = temporaryFolder.root.toPath() / "client"
         val certificatesDirectory = baseDirectory / "certificates"
@@ -363,10 +355,12 @@ class CertificateRevocationListNodeTests {
         return nodeCert
     }
 
-    private fun createAMQPServer(port: Int, name: CordaX500Name = ALICE_NAME,
+    @Suppress("LongParameterList")
+    private fun createAMQPServer(port: Int,
+                                 name: CordaX500Name = ALICE_NAME,
                                  crlCheckSoftFail: Boolean,
-                                 nodeCrlDistPoint: String? = "http://${crlServer.hostAndPort}/crl/node.crl",
+                                 nodeCrlDistPoint: String? = "http://${crlServer.hostAndPort}/crl/$NODE_CRL",
-                                 tlsCrlDistPoint: String? = "http://${crlServer.hostAndPort}/crl/empty.crl",
+                                 tlsCrlDistPoint: String? = "http://${crlServer.hostAndPort}/crl/$EMPTY_CRL",
                                  maxMessageSize: Int = MAX_MESSAGE_SIZE,
                                  sslHandshakeTimeout: Duration? = null): X509Certificate {
         check(!::amqpServer.isInitialized)
@@ -401,7 +395,7 @@ class CertificateRevocationListNodeTests {
                                                  tlsCrlDistPoint: String?): X509Certificate {
         val nodeKeyStore = signingCertificateStore.get()
         val (nodeCert, nodeKeys) = nodeKeyStore.query { getCertificateAndKeyPair(X509Utilities.CORDA_CLIENT_CA, nodeKeyStore.entryPassword) }
-        val newNodeCert = nodeCert.withCrlDistPoint(crlServer.intermediateCa.keyPair, nodeCaCrlDistPoint)
+        val newNodeCert = crlServer.replaceNodeCertDistPoint(nodeCert, nodeCaCrlDistPoint)
         val nodeCertChain = listOf(newNodeCert, crlServer.intermediateCa.certificate) +
                 nodeKeyStore.query { getCertificateChain(X509Utilities.CORDA_CLIENT_CA) }.drop(2)
 
@@ -414,7 +408,7 @@ class CertificateRevocationListNodeTests {
 
         val sslKeyStore = p2pSslConfiguration.keyStore.get()
         val (tlsCert, tlsKeys) = sslKeyStore.query { getCertificateAndKeyPair(X509Utilities.CORDA_CLIENT_TLS, sslKeyStore.entryPassword) }
-        val newTlsCert = tlsCert.withCrlDistPoint(nodeKeys, tlsCrlDistPoint, X500Name.getInstance(crlServer.rootCa.certificate.subjectX500Principal.encoded))
+        val newTlsCert = tlsCert.withCrlDistPoint(nodeKeys, tlsCrlDistPoint, crlServer.rootCa.certificate.subjectX500Principal)
         val sslCertChain = listOf(newTlsCert, newNodeCert, crlServer.intermediateCa.certificate) +
                 sslKeyStore.query { getCertificateChain(X509Utilities.CORDA_CLIENT_TLS) }.drop(3)
 
@@ -427,8 +421,9 @@ class CertificateRevocationListNodeTests {
         return newNodeCert
     }
 
+    @Suppress("LongParameterList")
     private fun verifyAMQPConnection(crlCheckSoftFail: Boolean,
-                                     nodeCrlDistPoint: String? = "http://${crlServer.hostAndPort}/crl/node.crl",
+                                     nodeCrlDistPoint: String? = "http://${crlServer.hostAndPort}/crl/$NODE_CRL",
                                      revokeServerCert: Boolean = false,
                                      revokeClientCert: Boolean = false,
                                      sslHandshakeTimeout: Duration? = null,
@@ -440,7 +435,7 @@ class CertificateRevocationListNodeTests {
                 sslHandshakeTimeout = sslHandshakeTimeout
         )
         if (revokeServerCert) {
-            revokedNodeCerts.add(serverCert.serialNumber)
+            crlServer.revokedNodeCerts.add(serverCert.serialNumber)
         }
         amqpServer.start()
         amqpServer.onReceive.subscribe {
@@ -452,7 +447,7 @@ class CertificateRevocationListNodeTests {
                 nodeCrlDistPoint = nodeCrlDistPoint
         )
         if (revokeClientCert) {
-            revokedNodeCerts.add(clientCert.serialNumber)
+            crlServer.revokedNodeCerts.add(clientCert.serialNumber)
         }
         val serverConnected = amqpServer.onConnection.toFuture()
         amqpClient.start()
@@ -489,12 +484,13 @@ class CertificateRevocationListNodeTests {
         return server to client
     }
 
+    @Suppress("LongParameterList")
     private fun verifyArtemisConnection(crlCheckSoftFail: Boolean,
                                         crlCheckArtemisServer: Boolean,
                                         expectedConnected: Boolean = true,
                                         expectedStatus: MessageStatus? = null,
                                         revokedNodeCert: Boolean = false,
-                                        nodeCrlDistPoint: String = "http://${crlServer.hostAndPort}/crl/node.crl",
+                                        nodeCrlDistPoint: String = "http://${crlServer.hostAndPort}/crl/$NODE_CRL",
                                         sslHandshakeTimeout: Duration? = null) {
         val queueName = P2P_PREFIX + "Test"
         val (artemisServer, artemisClient) = createArtemisServerAndClient(crlCheckSoftFail, crlCheckArtemisServer, nodeCrlDistPoint, sslHandshakeTimeout)
@@ -503,7 +499,7 @@ class CertificateRevocationListNodeTests {
 
             val nodeCert = createAMQPClient(serverPort, true, nodeCrlDistPoint)
             if (revokedNodeCert) {
-                revokedNodeCerts.add(nodeCert.serialNumber)
+                crlServer.revokedNodeCerts.add(nodeCert.serialNumber)
             }
             val clientConnected = amqpClient.onConnection.toFuture()
             amqpClient.start()

node/src/main/kotlin/net/corda/node/internal/artemis/CertificateChainCheckPolicy.kt

@@ -5,15 +5,14 @@ import net.corda.core.utilities.contextLogger
 import net.corda.nodeapi.internal.crypto.X509CertificateFactory
 import net.corda.nodeapi.internal.crypto.X509Utilities
 import net.corda.nodeapi.internal.protonwrapper.netty.RevocationConfig
+import net.corda.nodeapi.internal.protonwrapper.netty.RevocationConfigImpl
 import net.corda.nodeapi.internal.protonwrapper.netty.certPathToString
 import java.security.KeyStore
 import java.security.cert.CertPathValidator
 import java.security.cert.CertPathValidatorException
 import java.security.cert.CertificateException
 import java.security.cert.PKIXBuilderParameters
-import java.security.cert.PKIXRevocationChecker
 import java.security.cert.X509CertSelector
-import java.util.EnumSet
 
 sealed class CertificateChainCheckPolicy {
     companion object {
@@ -22,7 +21,6 @@ sealed class CertificateChainCheckPolicy {
 
     @FunctionalInterface
     interface Check {
-        @Suppress("DEPRECATION")    // should use java.security.cert.X509Certificate
         fun checkCertificateChain(theirChain: Array<javax.security.cert.X509Certificate>)
     }
 
@@ -31,7 +29,6 @@ sealed class CertificateChainCheckPolicy {
     object Any : CertificateChainCheckPolicy() {
         override fun createCheck(keyStore: KeyStore, trustStore: KeyStore): Check {
             return object : Check {
-                @Suppress("DEPRECATION")    // should use java.security.cert.X509Certificate
                 override fun checkCertificateChain(theirChain: Array<javax.security.cert.X509Certificate>) {
                     // nothing to do here
                 }
@@ -44,7 +41,6 @@ sealed class CertificateChainCheckPolicy {
             val rootAliases = trustStore.aliases().asSequence().filter { it.startsWith(X509Utilities.CORDA_ROOT_CA) }
             val rootPublicKeys = rootAliases.map { trustStore.getCertificate(it).publicKey }.toSet()
             return object : Check {
-                @Suppress("DEPRECATION")    // should use java.security.cert.X509Certificate
                 override fun checkCertificateChain(theirChain: Array<javax.security.cert.X509Certificate>) {
                     val theirRoot = theirChain.last().publicKey
                     if (theirRoot !in rootPublicKeys) {
@@ -59,7 +55,6 @@ sealed class CertificateChainCheckPolicy {
         override fun createCheck(keyStore: KeyStore, trustStore: KeyStore): Check {
             val ourPublicKey = keyStore.getCertificate(X509Utilities.CORDA_CLIENT_TLS).publicKey
             return object : Check {
-                @Suppress("DEPRECATION")    // should use java.security.cert.X509Certificate
                 override fun checkCertificateChain(theirChain: Array<javax.security.cert.X509Certificate>) {
                     val theirLeaf = theirChain.first().publicKey
                     if (ourPublicKey != theirLeaf) {
@@ -74,7 +69,6 @@ sealed class CertificateChainCheckPolicy {
         override fun createCheck(keyStore: KeyStore, trustStore: KeyStore): Check {
             val trustedPublicKeys = trustedAliases.map { trustStore.getCertificate(it).publicKey }.toSet()
             return object : Check {
-                @Suppress("DEPRECATION")    // should use java.security.cert.X509Certificate
                 override fun checkCertificateChain(theirChain: Array<javax.security.cert.X509Certificate>) {
                     if (!theirChain.any { it.publicKey in trustedPublicKeys }) {
                         throw CertificateException("Their certificate chain contained none of the trusted ones")
@@ -92,7 +86,6 @@ sealed class CertificateChainCheckPolicy {
 
     class UsernameMustMatchCommonNameCheck : Check {
         lateinit var username: String
-        @Suppress("DEPRECATION")    // should use java.security.cert.X509Certificate
         override fun checkCertificateChain(theirChain: Array<javax.security.cert.X509Certificate>) {
             if (!theirChain.any { certificate -> CordaX500Name.parse(certificate.subjectDN.name).commonName == username }) {
                 throw CertificateException("Client certificate does not match login username.")
@@ -100,14 +93,12 @@ sealed class CertificateChainCheckPolicy {
         }
     }
 
-    class RevocationCheck(val revocationMode: RevocationConfig.Mode) : CertificateChainCheckPolicy() {
+    class RevocationCheck(val revocationConfig: RevocationConfig) : CertificateChainCheckPolicy() {
+        constructor(revocationMode: RevocationConfig.Mode) : this(RevocationConfigImpl(revocationMode))
+
         override fun createCheck(keyStore: KeyStore, trustStore: KeyStore): Check {
             return object : Check {
-                @Suppress("DEPRECATION")    // should use java.security.cert.X509Certificate
                 override fun checkCertificateChain(theirChain: Array<javax.security.cert.X509Certificate>) {
-                    if (revocationMode == RevocationConfig.Mode.OFF) {
-                        return
-                    }
                     // Convert javax.security.cert.X509Certificate to java.security.cert.X509Certificate.
                     val chain = theirChain.map { X509CertificateFactory().generateCertificate(it.encoded.inputStream()) }
                     log.info("Check Client Certpath:\r\n${certPathToString(chain.toTypedArray())}")
@@ -117,17 +108,7 @@ sealed class CertificateChainCheckPolicy {
                     // See PKIXValidator.engineValidate() for reference implementation.
                     val certPath = X509Utilities.buildCertPath(chain.dropLast(1))
                     val certPathValidator = CertPathValidator.getInstance("PKIX")
-                    val pkixRevocationChecker = certPathValidator.revocationChecker as PKIXRevocationChecker
+                    val pkixRevocationChecker = revocationConfig.createPKIXRevocationChecker()
-                    pkixRevocationChecker.options = EnumSet.of(
-                            // Prefer CRL over OCSP
-                            PKIXRevocationChecker.Option.PREFER_CRLS,
-                            // Don't fall back to OCSP checking
-                            PKIXRevocationChecker.Option.NO_FALLBACK)
-                    if (revocationMode == RevocationConfig.Mode.SOFT_FAIL) {
-                        // Allow revocation check to succeed if the revocation status cannot be determined for one of
-                        // the following reasons: The CRL or OCSP response cannot be obtained because of a network error.
-                        pkixRevocationChecker.options = pkixRevocationChecker.options + PKIXRevocationChecker.Option.SOFT_FAIL
-                    }
                     val params = PKIXBuilderParameters(trustStore, X509CertSelector())
                     params.addCertPathChecker(pkixRevocationChecker)
                     try {

testing/node-driver/src/main/kotlin/net/corda/testing/node/internal/network/CrlServer.kt

@@ -42,21 +42,23 @@ import java.security.KeyPair
 import java.security.cert.X509CRL
 import java.security.cert.X509Certificate
 import java.util.*
+import javax.security.auth.x500.X500Principal
 import javax.ws.rs.GET
 import javax.ws.rs.Path
 import javax.ws.rs.Produces
 import javax.ws.rs.core.Response
+import kotlin.collections.ArrayList
 
-class CrlServer(hostAndPort: NetworkHostAndPort,
+class CrlServer(hostAndPort: NetworkHostAndPort) : Closeable {
-                private val revokedNodeCerts: List<BigInteger>,
-                private val revokedIntermediateCerts: List<BigInteger>) : Closeable {
-
     companion object {
         private const val SIGNATURE_ALGORITHM = "SHA256withECDSA"
 
+        const val NODE_CRL = "node.crl"
         const val FORBIDDEN_CRL = "forbidden.crl"
+        const val INTERMEDIATE_CRL = "intermediate.crl"
+        const val EMPTY_CRL = "empty.crl"
 
-        fun X509Certificate.withCrlDistPoint(issuerKeyPair: KeyPair, crlDistPoint: String?, crlIssuer: X500Name? = null): X509Certificate {
+        fun X509Certificate.withCrlDistPoint(issuerKeyPair: KeyPair, crlDistPoint: String?, crlIssuer: X500Principal? = null): X509Certificate {
             val signatureScheme = Crypto.findSignatureScheme(issuerKeyPair.private)
             val provider = Crypto.findProvider(signatureScheme.providerName)
             val issuerSigner = ContentSignerBuilder.build(signatureScheme, issuerKeyPair.private, provider)
@@ -71,7 +73,7 @@ class CrlServer(hostAndPort: NetworkHostAndPort,
             )
             if (crlDistPoint != null) {
                 val distPointName = DistributionPointName(GeneralNames(GeneralName(GeneralName.uniformResourceIdentifier, crlDistPoint)))
-                val crlIssuerGeneralNames = crlIssuer?.let { GeneralNames(GeneralName(it)) }
+                val crlIssuerGeneralNames = crlIssuer?.let { GeneralNames(GeneralName(X500Name.getInstance(it.encoded))) }
                 val distPoint = DistributionPoint(distPointName, null, crlIssuerGeneralNames)
                 builder.addExtension(Extension.cRLDistributionPoints, false, CRLDistPoint(arrayOf(distPoint)))
             }
@@ -85,6 +87,9 @@ class CrlServer(hostAndPort: NetworkHostAndPort,
         }
     }
 
+    val revokedNodeCerts: MutableList<BigInteger> = ArrayList()
+    val revokedIntermediateCerts: MutableList<BigInteger> = ArrayList()
+
     val rootCa: CertificateAndKeyPair = DEV_ROOT_CA
 
     private lateinit var _intermediateCa: CertificateAndKeyPair
@@ -98,12 +103,18 @@ class CrlServer(hostAndPort: NetworkHostAndPort,
     fun start() {
         server.start()
         _intermediateCa = CertificateAndKeyPair(
-                DEV_INTERMEDIATE_CA.certificate.withCrlDistPoint(rootCa.keyPair, "http://$hostAndPort/crl/intermediate.crl"),
+                DEV_INTERMEDIATE_CA.certificate.withCrlDistPoint(rootCa.keyPair, "http://$hostAndPort/crl/$INTERMEDIATE_CRL"),
                 DEV_INTERMEDIATE_CA.keyPair
         )
         println("Network management web services started on $hostAndPort")
     }
 
+    fun replaceNodeCertDistPoint(nodeCaCert: X509Certificate,
+                                 nodeCaCrlDistPoint: String? = "http://$hostAndPort/crl/$NODE_CRL",
+                                 crlIssuer: X500Principal? = null): X509Certificate {
+        return nodeCaCert.withCrlDistPoint(intermediateCa.keyPair, nodeCaCrlDistPoint, crlIssuer)
+    }
+
     fun createRevocationList(signatureAlgorithm: String,
                              ca: CertificateAndKeyPair,
                              endpoint: String,
@@ -145,13 +156,13 @@ class CrlServer(hostAndPort: NetworkHostAndPort,
     @Path("crl")
     class CrlServlet(private val crlServer: CrlServer) {
         @GET
-        @Path("node.crl")
+        @Path(NODE_CRL)
         @Produces("application/pkcs7-crl")
         fun getNodeCRL(): Response {
             return Response.ok(crlServer.createRevocationList(
                     SIGNATURE_ALGORITHM,
                     crlServer.intermediateCa,
-                    "node.crl",
+                    NODE_CRL,
                     false,
                     crlServer.revokedNodeCerts
             ).encoded).build()
@@ -165,26 +176,26 @@ class CrlServer(hostAndPort: NetworkHostAndPort,
         }
 
         @GET
-        @Path("intermediate.crl")
+        @Path(INTERMEDIATE_CRL)
         @Produces("application/pkcs7-crl")
         fun getIntermediateCRL(): Response {
             return Response.ok(crlServer.createRevocationList(
                     SIGNATURE_ALGORITHM,
                     crlServer.rootCa,
-                    "intermediate.crl",
+                    INTERMEDIATE_CRL,
                     false,
                     crlServer.revokedIntermediateCerts
             ).encoded).build()
         }
 
         @GET
-        @Path("empty.crl")
+        @Path(EMPTY_CRL)
         @Produces("application/pkcs7-crl")
         fun getEmptyCRL(): Response {
             return Response.ok(crlServer.createRevocationList(
                     SIGNATURE_ALGORITHM,
                     crlServer.rootCa,
-                    "empty.crl",
+                    EMPTY_CRL,
                     true, emptyList()
             ).encoded).build()
         }