CORDA-2740 - Remove RPC exception obfuscation (#5455)

core/src/main/kotlin/net/corda/core/ClientRelevantError.kt

@@ -7,4 +7,5 @@ import net.corda.core.serialization.CordaSerializable
  */
 @CordaSerializable
 @KeepForDJVM
+@Deprecated("This is no longer used as the exception obfuscation feature is no longer available.")
 interface ClientRelevantError

docs/source/changelog.rst

@@ -7,6 +7,9 @@ release, see :doc:`app-upgrade-notes`.
 Unreleased
 ----------
 
+* Removed the RPC exception privacy feature. Previously, in production mode, the exceptions thrown on the node were stripped of all content
+  when rethrown on the RPC client.
+
 * Introduced a new parameter ``externalIds: List<UUID>`` to ``VaultQueryCriteria`` which allows CorDapp developers to constrain queries
   to a specified set of external IDs.
 

docs/source/clientrpc.rst

@@ -327,13 +327,9 @@ If something goes wrong with the RPC infrastructure itself, an ``RPCException``
 requires a higher version of the protocol than the server supports, ``UnsupportedOperationException`` is thrown.
 Otherwise the behaviour depends on the ``devMode`` node configuration option.
 
-In ``devMode``, if the server implementation throws an exception, that exception is serialised and rethrown on the client
+If the server implementation throws an exception, that exception is serialised and rethrown on the client
 side as if it was thrown from inside the called RPC method. These exceptions can be caught as normal.
 
-When not in ``devMode``, the server will mask exceptions not meant for clients and return an ``InternalNodeException`` instead.
-This does not expose internal information to clients, strengthening privacy and security. CorDapps can have exceptions implement
-``ClientRelevantError`` to allow them to reach RPC clients.
-
 Reconnecting RPC clients
 ------------------------
 

node-api/src/main/kotlin/net/corda/nodeapi/exceptions/RpcExceptions.kt

@@ -16,27 +16,6 @@ class DuplicateAttachmentException(attachmentHash: String) : java.nio.file.FileA
  */
 class NonRpcFlowException(logicType: Class<*>) : IllegalArgumentException("${logicType.name} was not designed for RPC"), ClientRelevantError
 
-/**
- * An [Exception] to signal RPC clients that something went wrong within a Corda node.
- * The message is generic on purpose, as this prevents internal information from reaching RPC clients.
- * Leaking internal information outside can compromise privacy e.g., party names and security e.g., passwords, stacktraces, etc.
- *
- * @param errorIdentifier an optional identifier for tracing problems across parties.
- */
-class InternalNodeException(private val errorIdentifier: Long? = null) : CordaRuntimeException(message), ClientRelevantError, IdentifiableException {
-
-    companion object {
-        /**
-         * Message for the exception.
-         */
-        const val message = "Something went wrong within the Corda node."
-    }
-
-    override fun getErrorId(): Long? {
-        return errorIdentifier
-    }
-}
-
 class OutdatedNetworkParameterHashException(old: SecureHash, new: SecureHash) : CordaRuntimeException(TEMPLATE.format(old, new)), ClientRelevantError {
 
     private companion object {

node/src/integration-test/kotlin/net/corda/ClientRelevantException.kt

@@ -1,6 +0,0 @@
-package net.corda
-
-import net.corda.core.CordaRuntimeException
-import net.corda.core.ClientRelevantError
-
-class ClientRelevantException(message: String?, cause: Throwable?) : CordaRuntimeException(message, cause), ClientRelevantError

node/src/integration-test/kotlin/net/corda/node/BootTests.kt

@@ -10,7 +10,6 @@ import net.corda.core.messaging.startFlow
 import net.corda.core.utilities.getOrThrow
 import net.corda.node.internal.NodeStartup
 import net.corda.node.services.Permissions.Companion.startFlow
-import net.corda.nodeapi.exceptions.InternalNodeException
 import net.corda.nodeapi.internal.crypto.X509Utilities.NODE_IDENTITY_ALIAS_PREFIX
 import net.corda.nodeapi.internal.installDevNodeCaCertPath
 import net.corda.testing.core.ALICE_NAME
@@ -49,7 +48,7 @@ class BootTests {
             val node = startNode(ALICE_NAME, devMode = false, parameters = params).getOrThrow()
 
             assertThatThrownBy { devModeNode.attemptJavaDeserialization() }.isInstanceOf(CordaRuntimeException::class.java)
-            assertThatThrownBy { node.attemptJavaDeserialization() }.isInstanceOf(InternalNodeException::class.java)
+            assertThatThrownBy { node.attemptJavaDeserialization() }.isInstanceOf(CordaRuntimeException::class.java)
         }
     }
 

node/src/integration-test/kotlin/net/corda/node/logging/ErrorCodeLoggingTests.kt

@@ -1,5 +1,6 @@
 package net.corda.node.logging
 
+import net.corda.core.flows.FlowException
 import net.corda.core.flows.FlowLogic
 import net.corda.core.flows.InitiatingFlow
 import net.corda.core.flows.StartableByRPC
@@ -36,19 +37,8 @@ class ErrorCodeLoggingTests {
             val node = startNode(startInSameProcess = false, logLevelOverride = "ERROR").getOrThrow()
             node.rpc.startFlow(::MyFlow).waitForCompletion()
             val logFile = node.logFile()
-            assertThat(logFile.length()).isGreaterThan(0)
+            // An exception thrown in a flow will log at the "INFO" level.
-
+            assertThat(logFile.length()).isEqualTo(0)
-            val linesWithoutError = logFile.useLines { lines ->
-                lines.filterNot {
-                    it.contains("[ERROR")
-                }.filter {
-                    it.contains("[INFO")
-                            .or(it.contains("[WARN"))
-                            .or(it.contains("[DEBUG"))
-                            .or(it.contains("[TRACE"))
-                }.toList()
-            }
-            assertThat(linesWithoutError.isEmpty()).isTrue()
         }
     }
 

node/src/integration-test/kotlin/net/corda/node/services/rpc/RpcExceptionHandlingTest.kt

@@ -1,7 +1,6 @@
 package net.corda.node.services.rpc
 
 import co.paralleluniverse.fibers.Suspendable
-import net.corda.ClientRelevantException
 import net.corda.core.CordaRuntimeException
 import net.corda.core.flows.*
 import net.corda.core.identity.CordaX500Name
@@ -10,7 +9,6 @@ import net.corda.core.messaging.startFlow
 import net.corda.core.utilities.getOrThrow
 import net.corda.core.utilities.unwrap
 import net.corda.node.services.Permissions
-import net.corda.nodeapi.exceptions.InternalNodeException
 import net.corda.testing.core.*
 import net.corda.testing.driver.*
 import net.corda.testing.node.User
@@ -27,7 +25,7 @@ class RpcExceptionHandlingTest {
     private val users = listOf(user)
 
     @Test
-    fun `rpc client receives wrapped exceptions in devMode with no stacktraces`() {
+    fun `rpc client receives relevant exceptions`() {
         val params = NodeParameters(rpcUsers = users)
         val clientRelevantMessage = "This is for the players!"
 
@@ -37,17 +35,15 @@ class RpcExceptionHandlingTest {
 
         driver(DriverParameters(startNodesInProcess = true, notarySpecs = emptyList())) {
             val devModeNode = startNode(params, BOB_NAME).getOrThrow()
-            assertThatThrownBy { devModeNode.throwExceptionFromFlow() }.isInstanceOfSatisfying(ClientRelevantException::class.java) { exception ->
+            assertThatThrownBy { devModeNode.throwExceptionFromFlow() }.isInstanceOfSatisfying(CordaRuntimeException::class.java) { exception ->
                 assertEquals((exception.cause as CordaRuntimeException).originalExceptionClassName, SQLException::class.qualifiedName)
-                assertThat(exception.stackTrace).isEmpty()
+                assertThat(exception.originalMessage).isEqualTo(clientRelevantMessage)
-                assertThat((exception.cause as CordaRuntimeException).stackTrace).isEmpty()
-                assertThat(exception.message).isEqualTo(clientRelevantMessage)
             }
         }
     }
 
     @Test
-    fun `rpc client receives client-relevant message regardless of devMode`() {
+    fun `rpc client receives client-relevant message`() {
         val params = NodeParameters(rpcUsers = users)
         val clientRelevantMessage = "This is for the players!"
 
@@ -56,8 +52,8 @@ class RpcExceptionHandlingTest {
         }
 
         fun assertThatThrownExceptionIsReceivedUnwrapped(node: NodeHandle) {
-            assertThatThrownBy { node.throwExceptionFromFlow() }.isInstanceOfSatisfying(ClientRelevantException::class.java) { exception ->
+            assertThatThrownBy { node.throwExceptionFromFlow() }.isInstanceOfSatisfying(CordaRuntimeException::class.java) { exception ->
-                assertThat(exception.message).isEqualTo(clientRelevantMessage)
+                assertThat(exception.originalMessage).isEqualTo(clientRelevantMessage)
             }
         }
 
@@ -71,26 +67,7 @@ class RpcExceptionHandlingTest {
     }
 
     @Test
-    fun `rpc client receives no specific information in non devMode`() {
+    fun `FlowException is received by the RPC client`() {
-        val params = NodeParameters(rpcUsers = users)
-        val clientRelevantMessage = "This is for the players!"
-
-        fun NodeHandle.throwExceptionFromFlow() {
-            rpc.startFlow(::ClientRelevantErrorFlow, clientRelevantMessage).returnValue.getOrThrow()
-        }
-
-        driver(DriverParameters(startNodesInProcess = true, notarySpecs = emptyList())) {
-            val node = startNode(ALICE_NAME, devMode = false, parameters = params).getOrThrow()
-            assertThatThrownBy { node.throwExceptionFromFlow() }.isInstanceOfSatisfying(ClientRelevantException::class.java) { exception ->
-                assertThat(exception).hasNoCause()
-                assertThat(exception.stackTrace).isEmpty()
-                assertThat(exception.message).isEqualTo(clientRelevantMessage)
-            }
-        }
-    }
-
-    @Test
-    fun `FlowException is received by the RPC client only if in devMode`() {
         val params = NodeParameters(rpcUsers = users)
         val expectedMessage = "Flow error!"
         val expectedErrorId = 123L
@@ -104,17 +81,16 @@ class RpcExceptionHandlingTest {
             val node = startNode(ALICE_NAME, devMode = false, parameters = params).getOrThrow()
 
             assertThatThrownBy { devModeNode.throwExceptionFromFlow() }.isInstanceOfSatisfying(FlowException::class.java) { exception ->
-
                 assertThat(exception).hasNoCause()
                 assertThat(exception.stackTrace).isEmpty()
                 assertThat(exception.message).isEqualTo(expectedMessage)
                 assertThat(exception.errorId).isEqualTo(expectedErrorId)
             }
-            assertThatThrownBy { node.throwExceptionFromFlow() }.isInstanceOfSatisfying(InternalNodeException::class.java) { exception ->
 
+            assertThatThrownBy { node.throwExceptionFromFlow() }.isInstanceOfSatisfying(FlowException::class.java) { exception ->
                 assertThat(exception).hasNoCause()
                 assertThat(exception.stackTrace).isEmpty()
-                assertThat(exception.message).isEqualTo(InternalNodeException.message)
+                assertThat(exception.message).isEqualTo(expectedMessage)
                 assertThat(exception.errorId).isEqualTo(expectedErrorId)
             }
         }
@@ -143,12 +119,8 @@ class RpcExceptionHandlingTest {
             assertThatThrownBy { scenario(
                     DUMMY_BANK_A_NAME,
                     DUMMY_BANK_B_NAME,
-                    false) }.isInstanceOfSatisfying(InternalNodeException::class.java) { exception ->
+                    false)
-
+            }.isInstanceOf(UnexpectedFlowEndException::class.java)
-                assertThat(exception).hasNoCause()
-                assertThat(exception.stackTrace).isEmpty()
-                assertThat(exception.message).isEqualTo(InternalNodeException.message)
-            }
         }
     }
 }
@@ -175,7 +147,7 @@ class InitiatedFlow(private val initiatingSession: FlowSession) : FlowLogic<Unit
 @StartableByRPC
 class ClientRelevantErrorFlow(private val message: String) : FlowLogic<String>() {
     @Suspendable
-    override fun call(): String = throw ClientRelevantException(message, SQLException("Oops!"))
+    override fun call(): String = throw Exception(message, SQLException("Oops!"))
 }
 
 @StartableByRPC

node/src/main/kotlin/net/corda/node/internal/AbstractNode.kt

@@ -39,8 +39,6 @@ import net.corda.node.VersionInfo
 import net.corda.node.internal.classloading.requireAnnotation
 import net.corda.node.internal.cordapp.*
 import net.corda.node.internal.rpc.proxies.AuthenticatedRpcOpsProxy
-import net.corda.node.internal.rpc.proxies.ExceptionMaskingRpcOpsProxy
-import net.corda.node.internal.rpc.proxies.ExceptionSerialisingRpcOpsProxy
 import net.corda.node.internal.rpc.proxies.ThreadContextAdjustingRpcOpsProxy
 import net.corda.node.services.ContractUpgradeHandler
 import net.corda.node.services.FinalityHandler
@@ -284,10 +282,6 @@ abstract class AbstractNode<S>(val configuration: NodeConfiguration,
         val proxies = mutableListOf<(InternalCordaRPCOps) -> InternalCordaRPCOps>()
         // Mind that order is relevant here.
         proxies += ::AuthenticatedRpcOpsProxy
-        if (!configuration.devMode) {
-            proxies += { ExceptionMaskingRpcOpsProxy(it, true) }
-        }
-        proxies += { ExceptionSerialisingRpcOpsProxy(it, configuration.devMode) }
         proxies += { ThreadContextAdjustingRpcOpsProxy(it, cordappLoader.appClassLoader) }
         return proxies.fold(ops) { delegate, decorate -> decorate(delegate) }
     }

node/src/main/kotlin/net/corda/node/internal/rpc/proxies/ExceptionMaskingRpcOpsProxy.kt

@@ -1,117 +0,0 @@
-package net.corda.node.internal.rpc.proxies
-
-import net.corda.core.*
-import net.corda.core.concurrent.CordaFuture
-import net.corda.core.contracts.TransactionVerificationException
-import net.corda.core.flows.IdentifiableException
-import net.corda.core.internal.concurrent.doOnError
-import net.corda.core.internal.concurrent.mapError
-import net.corda.core.internal.declaredField
-import net.corda.core.internal.messaging.InternalCordaRPCOps
-import net.corda.core.messaging.*
-import net.corda.core.utilities.loggerFor
-import net.corda.node.internal.InvocationHandlerTemplate
-import net.corda.nodeapi.exceptions.InternalNodeException
-import rx.Observable
-import java.lang.reflect.Method
-import java.lang.reflect.Proxy.newProxyInstance
-import kotlin.reflect.KClass
-
-internal class ExceptionMaskingRpcOpsProxy(private val delegate: InternalCordaRPCOps, doLog: Boolean) : InternalCordaRPCOps by proxy(delegate, doLog) {
-    private companion object {
-        private val logger = loggerFor<ExceptionMaskingRpcOpsProxy>()
-
-        private val whitelist = setOf(
-                ClientRelevantError::class,
-                TransactionVerificationException::class
-        )
-
-        private fun proxy(delegate: InternalCordaRPCOps, doLog: Boolean): InternalCordaRPCOps {
-            val handler = ErrorObfuscatingInvocationHandler(delegate, whitelist, doLog)
-            return newProxyInstance(delegate::class.java.classLoader, arrayOf(InternalCordaRPCOps::class.java), handler) as InternalCordaRPCOps
-        }
-    }
-
-    private class ErrorObfuscatingInvocationHandler(override val delegate: InternalCordaRPCOps, private val whitelist: Set<KClass<*>>, private val doLog: Boolean) : InvocationHandlerTemplate {
-        override fun invoke(proxy: Any, method: Method, arguments: Array<out Any?>?): Any? {
-            try {
-                val result = super.invoke(proxy, method, arguments)
-                return result?.let { obfuscateResult(it) }
-            } catch (exception: Exception) {
-                // In this special case logging and re-throwing is the right approach.
-                log(exception)
-                throw obfuscate(exception)
-            }
-        }
-
-        private fun <RESULT : Any> obfuscateResult(result: RESULT): Any {
-            return when (result) {
-                is CordaFuture<*> -> wrapFuture(result)
-                is DataFeed<*, *> -> wrapFeed(result)
-                is FlowProgressHandle<*> -> wrapFlowProgressHandle(result)
-                is FlowHandle<*> -> wrapFlowHandle(result)
-                is Observable<*> -> wrapObservable(result)
-                else -> result
-            }
-        }
-
-        private fun wrapFlowProgressHandle(handle: FlowProgressHandle<*>): FlowProgressHandle<*> {
-            val returnValue = wrapFuture(handle.returnValue)
-            val progress = wrapObservable(handle.progress)
-            val stepsTreeIndexFeed = handle.stepsTreeIndexFeed?.let { wrapFeed(it) }
-            val stepsTreeFeed = handle.stepsTreeFeed?.let { wrapFeed(it) }
-
-            return FlowProgressHandleImpl(handle.id, returnValue, progress, stepsTreeIndexFeed, stepsTreeFeed)
-        }
-
-        private fun wrapFlowHandle(handle: FlowHandle<*>): FlowHandle<*> {
-            return FlowHandleImpl(handle.id, wrapFuture(handle.returnValue))
-        }
-
-        private fun <ELEMENT> wrapObservable(observable: Observable<ELEMENT>): Observable<ELEMENT> {
-            return observable.doOnError(::log).mapErrors(::obfuscate)
-        }
-
-        private fun <SNAPSHOT, ELEMENT> wrapFeed(feed: DataFeed<SNAPSHOT, ELEMENT>): DataFeed<SNAPSHOT, ELEMENT> {
-            return feed.doOnError(::log).mapErrors(::obfuscate)
-        }
-
-        private fun <RESULT> wrapFuture(future: CordaFuture<RESULT>): CordaFuture<RESULT> {
-            return future.doOnError(::log).mapError(::obfuscate)
-        }
-
-        private fun log(error: Throwable) {
-            if (doLog) {
-                logger.error("Error during RPC invocation", error)
-            }
-        }
-
-        private fun obfuscate(error: Throwable): Throwable {
-            val exposed = if (error.isWhitelisted()) error else InternalNodeException((error as? IdentifiableException)?.errorId)
-            removeDetails(exposed)
-            return exposed
-        }
-
-        private fun removeDetails(error: Throwable) {
-            error.stackTrace = arrayOf<StackTraceElement>()
-            error.declaredField<Any?>("cause").value = null
-            error.declaredField<Any?>("suppressedExceptions").value = null
-            when (error) {
-                is CordaException -> error.setCause(null)
-                is CordaRuntimeException -> error.setCause(null)
-            }
-        }
-
-        private fun Throwable.isWhitelisted(): Boolean {
-            return whitelist.any { it.isInstance(this) }
-        }
-
-        override fun toString(): String {
-            return "ErrorObfuscatingInvocationHandler(whitelist=$whitelist)"
-        }
-    }
-
-    override fun toString(): String {
-        return "ExceptionMaskingRpcOpsProxy"
-    }
-}

node/src/main/kotlin/net/corda/node/internal/rpc/proxies/ExceptionSerialisingRpcOpsProxy.kt

@@ -1,107 +0,0 @@
-package net.corda.node.internal.rpc.proxies
-
-import net.corda.core.CordaRuntimeException
-import net.corda.core.concurrent.CordaFuture
-import net.corda.core.doOnError
-import net.corda.core.internal.concurrent.doOnError
-import net.corda.core.internal.concurrent.mapError
-import net.corda.core.internal.messaging.InternalCordaRPCOps
-import net.corda.core.mapErrors
-import net.corda.core.messaging.*
-import net.corda.core.serialization.CordaSerializable
-import net.corda.core.utilities.loggerFor
-import net.corda.node.internal.InvocationHandlerTemplate
-import rx.Observable
-import java.lang.reflect.Method
-import java.lang.reflect.Proxy.newProxyInstance
-
-internal class ExceptionSerialisingRpcOpsProxy(private val delegate: InternalCordaRPCOps, doLog: Boolean) : InternalCordaRPCOps by proxy(delegate, doLog) {
-    private companion object {
-        private val logger = loggerFor<ExceptionSerialisingRpcOpsProxy>()
-
-        private fun proxy(delegate: InternalCordaRPCOps, doLog: Boolean): InternalCordaRPCOps {
-            val handler = ErrorSerialisingInvocationHandler(delegate, doLog)
-            return newProxyInstance(delegate::class.java.classLoader, arrayOf(InternalCordaRPCOps::class.java), handler) as InternalCordaRPCOps
-        }
-    }
-
-    private class ErrorSerialisingInvocationHandler(override val delegate: InternalCordaRPCOps, private val doLog: Boolean) : InvocationHandlerTemplate {
-        override fun invoke(proxy: Any, method: Method, arguments: Array<out Any?>?): Any? {
-            try {
-                val result = super.invoke(proxy, method, arguments)
-                return result?.let { ensureSerialisable(it) }
-            } catch (exception: Exception) {
-                throw ensureSerialisable(exception)
-            }
-        }
-
-        private fun <RESULT : Any> ensureSerialisable(result: RESULT): Any {
-            return when (result) {
-                is CordaFuture<*> -> wrapFuture(result)
-                is DataFeed<*, *> -> wrapFeed(result)
-                is FlowProgressHandle<*> -> wrapFlowProgressHandle(result)
-                is FlowHandle<*> -> wrapFlowHandle(result)
-                is Observable<*> -> wrapObservable(result)
-                else -> result
-            }
-        }
-
-        private fun wrapFlowProgressHandle(handle: FlowProgressHandle<*>): FlowProgressHandle<*> {
-            val returnValue = wrapFuture(handle.returnValue)
-            val progress = wrapObservable(handle.progress)
-            val stepsTreeIndexFeed = handle.stepsTreeIndexFeed?.let { wrapFeed(it) }
-            val stepsTreeFeed = handle.stepsTreeFeed?.let { wrapFeed(it) }
-
-            return FlowProgressHandleImpl(handle.id, returnValue, progress, stepsTreeIndexFeed, stepsTreeFeed)
-        }
-
-        private fun wrapFlowHandle(handle: FlowHandle<*>): FlowHandle<*> {
-            return FlowHandleImpl(handle.id, wrapFuture(handle.returnValue))
-        }
-
-        private fun <ELEMENT> wrapObservable(observable: Observable<ELEMENT>): Observable<ELEMENT> {
-            return observable.doOnError(::log).mapErrors(::ensureSerialisable)
-        }
-
-        private fun <SNAPSHOT, ELEMENT> wrapFeed(feed: DataFeed<SNAPSHOT, ELEMENT>): DataFeed<SNAPSHOT, ELEMENT> {
-            return feed.doOnError(::log).mapErrors(::ensureSerialisable)
-        }
-
-        private fun <RESULT> wrapFuture(future: CordaFuture<RESULT>): CordaFuture<RESULT> {
-            return future.doOnError(::log).mapError(::ensureSerialisable)
-        }
-
-        private fun ensureSerialisable(error: Throwable): Throwable {
-            val serialisable = (superclasses(error::class.java) + error::class.java).any { it.isAnnotationPresent(CordaSerializable::class.java) || it.interfaces.any { it.isAnnotationPresent(CordaSerializable::class.java) } }
-            val result = if (serialisable) {
-                error
-            } else {
-                log(error)
-                CordaRuntimeException(error.message, error)
-            }
-            return result
-        }
-
-        private fun log(error: Throwable) {
-            if (doLog) {
-                logger.error("Error during RPC invocation", error)
-            }
-        }
-
-        private fun superclasses(clazz: Class<*>): List<Class<*>> {
-            val superclasses = mutableListOf<Class<*>>()
-            var current: Class<*>?
-            var superclass = clazz.superclass
-            while (superclass != null) {
-                superclasses += superclass
-                current = superclass
-                superclass = current.superclass
-            }
-            return superclasses
-        }
-    }
-
-    override fun toString(): String {
-        return "ExceptionSerialisingRpcOpsProxy"
-    }
-}