CORDA-2413 Improve exception handling and recovery for untrusted contract attachments (#4543)

* Improve exception handling and recovery for untrusted contract attachments.

* Fix broken JUnit.

* Fixed incorrect Exception description.

* Additional clarification on flow processing.

* Reasoning and future deterministic JVM clarification.

* Note::

* UntrustedAttachmentException.

core/src/main/kotlin/net/corda/core/serialization/internal/AttachmentsClassLoader.kt

@@ -1,16 +1,15 @@
 package net.corda.core.serialization.internal
 
+import net.corda.core.CordaException
+import net.corda.core.KeepForDJVM
 import net.corda.core.contracts.Attachment
 import net.corda.core.contracts.ContractAttachment
 import net.corda.core.contracts.TransactionVerificationException.OverlappingAttachmentsException
 import net.corda.core.crypto.SecureHash
 import net.corda.core.crypto.sha256
-import net.corda.core.internal.VisibleForTesting
+import net.corda.core.internal.*
 import net.corda.core.internal.cordapp.targetPlatformVersion
-import net.corda.core.internal.createSimpleCache
-import net.corda.core.internal.isUploaderTrusted
-import net.corda.core.internal.toSynchronised
-import net.corda.core.serialization.MissingAttachmentsException
+import net.corda.core.serialization.CordaSerializable
 import net.corda.core.serialization.SerializationFactory
 import net.corda.core.serialization.internal.AttachmentURLStreamHandlerFactory.toUrl
 import net.corda.core.utilities.contextLogger
@@ -19,8 +18,6 @@ import java.io.ByteArrayOutputStream
 import java.io.IOException
 import java.io.InputStream
 import java.net.*
-import java.lang.reflect.AccessibleObject.setAccessible
-import java.net.URLStreamHandlerFactory
 
 /**
  * A custom ClassLoader that knows how to load classes from a set of attachments. The attachments themselves only
@@ -34,7 +31,7 @@ class AttachmentsClassLoader(attachments: List<Attachment>, parent: ClassLoader
     init {
         val untrusted = attachments.mapNotNull { it as? ContractAttachment }.filterNot { isUploaderTrusted(it.uploader) }.map(ContractAttachment::id)
         if(untrusted.isNotEmpty()) {
-            throw MissingAttachmentsException(untrusted, "Attempting to load Contract Attachments downloaded from the network")
+            throw UntrustedAttachmentsException(untrusted)
         }
         requireNoDuplicates(attachments)
     }
@@ -248,3 +245,11 @@ object AttachmentURLStreamHandlerFactory : URLStreamHandlerFactory {
     }
 }
 
+/** Thrown during classloading upon encountering an untrusted attachment (eg. not in the [TRUSTED_UPLOADERS] list) */
+@KeepForDJVM
+@CordaSerializable
+class UntrustedAttachmentsException(val ids: List<SecureHash>) :
+        CordaException("Attempting to load untrusted Contract Attachments: $ids" +
+                "These may have been received over the p2p network from a remote node." +
+                "Please follow the operational steps outlined in https://docs.corda.net/cordapp-build-systems.html#cordapp-contract-attachments to continue."
+        )

docs/source/clientrpc.rst

@@ -21,6 +21,8 @@ you.
    `CordaRPCClient`_ class. You can find an example of how to do this using the popular Spring Boot server
    `here <https://github.com/corda/spring-webserver>`_.
 
+.. _clientrpc_connect_ref:
+
 Connecting to a node via RPC
 ----------------------------
 To use `CordaRPCClient`_, you must add ``net.corda:corda-rpc:$corda_release_version`` as a ``cordaCompile`` dependency

docs/source/cordapp-build-systems.rst

@@ -346,6 +346,9 @@ This is typically done by appending the version string to the CorDapp's name. Th
 once the JAR has been deployed on a node. If it does, make sure no one is relying on ``FlowContext.appName`` in their
 flows (see :doc:`versioning`).
 
+
+.. _cordapp_install_ref:
+
 Installing the CorDapp JAR
 --------------------------
 
@@ -471,4 +474,36 @@ For a CorDapp that contains flows and/or services we specify the `workflow` tag:
             }
         }
 
-.. note:: It is possible, but *not recommended*, to include everything in a single CorDapp jar and use both the ``contract`` and ``workflow`` Gradle plugin tags.
+.. note:: It is possible, but *not recommended*, to include everything in a single CorDapp jar and use both the ``contract`` and ``workflow`` Gradle plugin tags.
+
+.. _cordapp_contract_attachments_ref:
+
+CorDapp Contract Attachments
+----------------------------
+
+As of Corda 4, CorDapp Contract JARs must be installed on a node by a trusted uploader, either by
+
+- installing manually as per :ref:`Installing the CorDapp JAR <cordapp_install_ref>` and re-starting the node.
+
+- uploading the attachment JAR to the node via RPC, either programmatically (see :ref:`Connecting to a node via RPC <clientrpc_connect_ref>`)
+  or via the :doc:`shell` by issuing the following command:
+
+``>>> run uploadAttachment jar: path/to/the/file.jar``
+
+Contract attachments that are received from a peer over the p2p network are considered **untrusted** and will throw a `UntrustedAttachmentsException` exception
+when processed by a listening flow that cannot resolve that attachment from its local attachment storage. The flow will be aborted and sent to the nodes flow hospital for recovery and retry.
+The untrusted attachment JAR will be stored in the nodes local attachment store for review by a node operator. It can be downloaded for viewing using the following CRaSH shell command:
+
+``>>> run openAttachment id: <hash of untrusted attachment given by `UntrustedAttachmentsException` exception``
+
+Should the node operator deem the attachment trustworthy, they may then issue the following CRaSH shell command to reload it as trusted:
+
+``>>> run uploadAttachment jar: path/to/the/trusted-file.jar``
+
+and subsequently retry the failed flow (currently this requires a node re-start).
+
+.. note:: this behaviour is to protect the node from executing contract code that was not vetted. It is a temporary precaution until the
+   Deterministic JVM is integrated into Corda whereby execution takes place in a sandboxed environment which protects the node from malicious code.
+
+
+

serialization/src/test/kotlin/net/corda/serialization/internal/CordaClassResolverTests.kt

@@ -11,11 +11,11 @@ import com.nhaarman.mockito_kotlin.verify
 import com.nhaarman.mockito_kotlin.whenever
 import net.corda.core.internal.DEPLOYED_CORDAPP_UPLOADER
 import net.corda.core.node.services.AttachmentStorage
-import net.corda.core.serialization.internal.CheckpointSerializationContext
 import net.corda.core.serialization.ClassWhitelist
 import net.corda.core.serialization.CordaSerializable
-import net.corda.core.serialization.MissingAttachmentsException
 import net.corda.core.serialization.internal.AttachmentsClassLoader
+import net.corda.core.serialization.internal.CheckpointSerializationContext
+import net.corda.core.serialization.internal.UntrustedAttachmentsException
 import net.corda.node.serialization.kryo.CordaClassResolver
 import net.corda.node.serialization.kryo.CordaKryo
 import net.corda.testing.internal.rigorousMock
@@ -23,7 +23,6 @@ import net.corda.testing.services.MockAttachmentStorage
 import org.junit.Rule
 import org.junit.Test
 import org.junit.rules.ExpectedException
-import java.lang.IllegalStateException
 import java.net.URL
 import java.sql.Connection
 import java.util.*
@@ -217,8 +216,8 @@ class CordaClassResolverTests {
         CordaClassResolver(emptyWhitelistContext).getRegistration(attachedClass)
     }
 
-    @Test(expected = MissingAttachmentsException::class)
-    fun `Attempt to load contract attachment with the incorrect uploader should fails with MissingAttachmentsException`() {
+    @Test(expected = UntrustedAttachmentsException::class)
+    fun `Attempt to load contract attachment with untrusted uploader should fail with UntrustedAttachmentsException`() {
         val storage = MockAttachmentStorage()
         val attachmentHash = importJar(storage, "some_uploader")
         val classLoader = AttachmentsClassLoader(arrayOf(attachmentHash).map { storage.openAttachment(it)!! })