From d84e9aab7b7550bad078dc5f44f833e074f670bd Mon Sep 17 00:00:00 2001 From: Richard Green Date: Tue, 14 Nov 2017 16:57:59 +0000 Subject: [PATCH] Added exception if same attachment uploaded. Added test --- .../core/node/services/AttachmentStorage.kt | 14 +++ .../internal/cordapp/CordappProviderImpl.kt | 12 ++- .../persistence/NodeAttachmentService.kt | 102 +++++++++++------- .../net/corda/node/CordaRPCOpsImplTest.kt | 12 +++ .../testing/node/MockAttachmentStorage.kt | 35 ++++-- 5 files changed, 124 insertions(+), 51 deletions(-) diff --git a/core/src/main/kotlin/net/corda/core/node/services/AttachmentStorage.kt b/core/src/main/kotlin/net/corda/core/node/services/AttachmentStorage.kt index e7da510f23..9332587f7a 100644 --- a/core/src/main/kotlin/net/corda/core/node/services/AttachmentStorage.kt +++ b/core/src/main/kotlin/net/corda/core/node/services/AttachmentStorage.kt @@ -45,6 +45,13 @@ interface AttachmentStorage { @Throws(FileAlreadyExistsException::class, IOException::class) fun importAttachment(jar: InputStream, uploader: String, filename: String): AttachmentId + /** + * Inserts or returns Attachment Id of attachment. Does not throw an exception if already uploaded. + * @param jar [InputStream] of Jar file + * @return [AttachmentId] of uploaded attachment + */ + fun importOrGetAttachment(jar: InputStream): AttachmentId + /** * Searches attachment using given criteria and optional sort rules * @param criteria Query criteria to use as a filter @@ -53,5 +60,12 @@ interface AttachmentStorage { * @return List of AttachmentId of attachment matching criteria, sorted according to given sorting parameter */ fun queryAttachments(criteria: AttachmentQueryCriteria, sorting: AttachmentSort? = null): List + + /** + * Searches for an attachment already in the store + * @param attachmentId The attachment Id + * @return true if it's in there + */ + fun hasAttachment(attachmentId: AttachmentId): Boolean } diff --git a/node/src/main/kotlin/net/corda/node/internal/cordapp/CordappProviderImpl.kt b/node/src/main/kotlin/net/corda/node/internal/cordapp/CordappProviderImpl.kt index ba305283f2..cfed6d0cd2 100644 --- a/node/src/main/kotlin/net/corda/node/internal/cordapp/CordappProviderImpl.kt +++ b/node/src/main/kotlin/net/corda/node/internal/cordapp/CordappProviderImpl.kt @@ -2,18 +2,24 @@ package net.corda.node.internal.cordapp import com.google.common.collect.HashBiMap import net.corda.core.contracts.ContractClassName -import net.corda.core.crypto.SecureHash -import net.corda.core.node.services.AttachmentStorage import net.corda.core.cordapp.Cordapp import net.corda.core.cordapp.CordappContext +import net.corda.core.crypto.SecureHash import net.corda.core.node.services.AttachmentId +import net.corda.core.node.services.AttachmentStorage import net.corda.core.serialization.SingletonSerializeAsToken +import net.corda.core.utilities.loggerFor import java.net.URL /** * Cordapp provider and store. For querying CorDapps for their attachment and vice versa. */ open class CordappProviderImpl(private val cordappLoader: CordappLoader, attachmentStorage: AttachmentStorage) : SingletonSerializeAsToken(), CordappProviderInternal { + + companion object { + private val log = loggerFor() + } + override fun getAppContext(): CordappContext { // TODO: Use better supported APIs in Java 9 Exception().stackTrace.forEach { stackFrame -> @@ -45,7 +51,7 @@ open class CordappProviderImpl(private val cordappLoader: CordappLoader, attachm private fun loadContractsIntoAttachmentStore(attachmentStorage: AttachmentStorage): Map { val cordappsWithAttachments = cordapps.filter { !it.contractClassNames.isEmpty() }.map { it.jarPath } - val attachmentIds = cordappsWithAttachments.map { it.openStream().use { attachmentStorage.importAttachment(it) } } + val attachmentIds = cordappsWithAttachments.map { it.openStream().use { attachmentStorage.importOrGetAttachment(it) }} return attachmentIds.zip(cordappsWithAttachments).toMap() } diff --git a/node/src/main/kotlin/net/corda/node/services/persistence/NodeAttachmentService.kt b/node/src/main/kotlin/net/corda/node/services/persistence/NodeAttachmentService.kt index 26f251b73e..6c4d4a9e75 100644 --- a/node/src/main/kotlin/net/corda/node/services/persistence/NodeAttachmentService.kt +++ b/node/src/main/kotlin/net/corda/node/services/persistence/NodeAttachmentService.kt @@ -1,18 +1,19 @@ package net.corda.node.services.persistence import com.codahale.metrics.MetricRegistry -import net.corda.core.internal.VisibleForTesting import com.google.common.hash.HashCode import com.google.common.hash.Hashing import com.google.common.hash.HashingInputStream import com.google.common.io.CountingInputStream import net.corda.core.CordaRuntimeException -import net.corda.core.contracts.* -import net.corda.core.internal.AbstractAttachment +import net.corda.core.contracts.Attachment import net.corda.core.crypto.SecureHash +import net.corda.core.internal.AbstractAttachment +import net.corda.core.internal.VisibleForTesting import net.corda.core.node.services.AttachmentId import net.corda.core.node.services.AttachmentStorage -import net.corda.core.node.services.vault.* +import net.corda.core.node.services.vault.AttachmentQueryCriteria +import net.corda.core.node.services.vault.AttachmentSort import net.corda.core.serialization.* import net.corda.core.utilities.contextLogger import net.corda.node.services.vault.HibernateAttachmentQueryCriteriaParser @@ -25,7 +26,6 @@ import java.time.Instant import java.util.jar.JarInputStream import javax.annotation.concurrent.ThreadSafe import javax.persistence.* -import javax.persistence.Column /** * Stores attachments using Hibernate to database. @@ -33,6 +33,29 @@ import javax.persistence.Column @ThreadSafe class NodeAttachmentService(metrics: MetricRegistry) : AttachmentStorage, SingletonSerializeAsToken() { + companion object { + private val log = contextLogger() + + // Just iterate over the entries with verification enabled: should be good enough to catch mistakes. + // Note that JarInputStream won't throw any kind of error at all if the file stream is in fact not + // a ZIP! It'll just pretend it's an empty archive, which is kind of stupid but that's how it works. + // So we have to check to ensure we found at least one item. + private fun checkIsAValidJAR(stream: InputStream) { + val jar = JarInputStream(stream, true) + var count = 0 + while (true) { + val cursor = jar.nextJarEntry ?: break + val entryPath = Paths.get(cursor.name) + // Security check to stop zips trying to escape their rightful place. + require(!entryPath.isAbsolute) { "Path $entryPath is absolute" } + require(entryPath.normalize() == entryPath) { "Path $entryPath is not normalised" } + require(!('\\' in cursor.name || cursor.name == "." || cursor.name == "..")) { "Bad character in $entryPath" } + count++ + } + require(count > 0) { "Stream is either empty or not a JAR/ZIP" } + } + } + @Entity @Table(name = "${NODE_DATABASE_PREFIX}attachments", indexes = arrayOf(Index(name = "att_id_idx", columnList = "att_id"))) @@ -55,10 +78,6 @@ class NodeAttachmentService(metrics: MetricRegistry) : AttachmentStorage, Single var filename: String? = null ) : Serializable - companion object { - private val log = contextLogger() - } - @VisibleForTesting var checkAttachmentsOnLoad = true @@ -170,6 +189,24 @@ class NodeAttachmentService(metrics: MetricRegistry) : AttachmentStorage, Single return import(jar, uploader, filename) } + fun getAttachmentIdAndBytes(jar: InputStream): Pair { + val hs = HashingInputStream(Hashing.sha256(), jar) + val bytes = hs.readBytes() + checkIsAValidJAR(ByteArrayInputStream(bytes)) + val id = SecureHash.SHA256(hs.hash().asBytes()) + return Pair(id, bytes) + } + + override fun hasAttachment(attachmentId: AttachmentId): Boolean { + val session = currentDBSession() + val criteriaBuilder = session.criteriaBuilder + val criteriaQuery = criteriaBuilder.createQuery(Long::class.java) + val attachments = criteriaQuery.from(NodeAttachmentService.DBAttachment::class.java) + criteriaQuery.select(criteriaBuilder.count(criteriaQuery.from(NodeAttachmentService.DBAttachment::class.java))) + criteriaQuery.where(criteriaBuilder.equal(attachments.get(DBAttachment::attId.name), attachmentId.toString())) + return (session.createQuery(criteriaQuery).singleResult > 0) + } + // TODO: PLT-147: The attachment should be randomised to prevent brute force guessing and thus privacy leaks. private fun import(jar: InputStream, uploader: String?, filename: String?): AttachmentId { require(jar !is JarInputStream) @@ -179,27 +216,28 @@ class NodeAttachmentService(metrics: MetricRegistry) : AttachmentStorage, Single // To do this we must pipe stream into the database without knowing its hash, which we will learn only once // the insert/upload is complete. We can then query to see if it's a duplicate and if so, erase, and if not // set the hash field of the new attachment record. - val hs = HashingInputStream(Hashing.sha256(), jar) - val bytes = hs.readBytes() - checkIsAValidJAR(ByteArrayInputStream(bytes)) - val id = SecureHash.SHA256(hs.hash().asBytes()) - val session = currentDBSession() - val criteriaBuilder = session.criteriaBuilder - val criteriaQuery = criteriaBuilder.createQuery(Long::class.java) - val attachments = criteriaQuery.from(NodeAttachmentService.DBAttachment::class.java) - criteriaQuery.select(criteriaBuilder.count(criteriaQuery.from(NodeAttachmentService.DBAttachment::class.java))) - criteriaQuery.where(criteriaBuilder.equal(attachments.get(DBAttachment::attId.name), id.toString())) - val count = session.createQuery(criteriaQuery).singleResult - if (count == 0L) { + val (id, bytes) = getAttachmentIdAndBytes(jar) + if (!hasAttachment(id)) { + checkIsAValidJAR(ByteArrayInputStream(bytes)) + val session = currentDBSession() val attachment = NodeAttachmentService.DBAttachment(attId = id.toString(), content = bytes, uploader = uploader, filename = filename) session.save(attachment) - attachmentCount.inc() log.info("Stored new attachment $id") + return id + } else { + throw java.nio.file.FileAlreadyExistsException(id.toString()) } + } - return id + override fun importOrGetAttachment(jar: InputStream): AttachmentId { + try { + return importAttachment(jar) + } + catch (faee: java.nio.file.FileAlreadyExistsException) { + return AttachmentId.parse(faee.message!!) + } } override fun queryAttachments(criteria: AttachmentQueryCriteria, sorting: AttachmentSort?): List { @@ -226,22 +264,4 @@ class NodeAttachmentService(metrics: MetricRegistry) : AttachmentStorage, Single } - private fun checkIsAValidJAR(stream: InputStream) { - // Just iterate over the entries with verification enabled: should be good enough to catch mistakes. - // Note that JarInputStream won't throw any kind of error at all if the file stream is in fact not - // a ZIP! It'll just pretend it's an empty archive, which is kind of stupid but that's how it works. - // So we have to check to ensure we found at least one item. - val jar = JarInputStream(stream, true) - var count = 0 - while (true) { - val cursor = jar.nextJarEntry ?: break - val entryPath = Paths.get(cursor.name) - // Security check to stop zips trying to escape their rightful place. - require(!entryPath.isAbsolute) { "Path $entryPath is absolute" } - require(entryPath.normalize() == entryPath) { "Path $entryPath is not normalised" } - require(!('\\' in cursor.name || cursor.name == "." || cursor.name == "..")) { "Bad character in $entryPath" } - count++ - } - require(count > 0) { "Stream is either empty or not a JAR/ZIP" } - } } diff --git a/node/src/test/kotlin/net/corda/node/CordaRPCOpsImplTest.kt b/node/src/test/kotlin/net/corda/node/CordaRPCOpsImplTest.kt index 02e2f80dc4..5ddff0c2d5 100644 --- a/node/src/test/kotlin/net/corda/node/CordaRPCOpsImplTest.kt +++ b/node/src/test/kotlin/net/corda/node/CordaRPCOpsImplTest.kt @@ -241,6 +241,18 @@ class CordaRPCOpsImplTest { } } + @Test + fun `can't upload the same attachment`() { + withPermissions(invokeRpc(CordaRPCOps::uploadAttachment), invokeRpc(CordaRPCOps::attachmentExists)) { + assertThatExceptionOfType(java.nio.file.FileAlreadyExistsException::class.java).isThrownBy { + val inputJar1 = Thread.currentThread().contextClassLoader.getResourceAsStream(testJar) + val inputJar2 = Thread.currentThread().contextClassLoader.getResourceAsStream(testJar) + val secureHash1 = rpc.uploadAttachment(inputJar1) + val secureHash2 = rpc.uploadAttachment(inputJar2) + } + } + } + @Test fun `can download an uploaded attachment`() { withPermissions(invokeRpc(CordaRPCOps::uploadAttachment), invokeRpc(CordaRPCOps::openAttachment)) { diff --git a/testing/test-utils/src/main/kotlin/net/corda/testing/node/MockAttachmentStorage.kt b/testing/test-utils/src/main/kotlin/net/corda/testing/node/MockAttachmentStorage.kt index 0550e1722e..c12e202b33 100644 --- a/testing/test-utils/src/main/kotlin/net/corda/testing/node/MockAttachmentStorage.kt +++ b/testing/test-utils/src/main/kotlin/net/corda/testing/node/MockAttachmentStorage.kt @@ -11,21 +11,25 @@ import net.corda.core.node.services.vault.AttachmentSort import net.corda.core.serialization.SingletonSerializeAsToken import java.io.ByteArrayOutputStream import java.io.InputStream -import java.util.HashMap +import java.util.* import java.util.jar.JarInputStream class MockAttachmentStorage : AttachmentStorage, SingletonSerializeAsToken() { - - override fun importAttachment(jar: InputStream): AttachmentId { - // JIS makes read()/readBytes() return bytes of the current file, but we want to hash the entire container here. - require(jar !is JarInputStream) - - val bytes = run { + companion object { + fun getBytes(jar: InputStream) = run { val s = ByteArrayOutputStream() jar.copyTo(s) s.close() s.toByteArray() } + } + + override fun importAttachment(jar: InputStream): AttachmentId { + // JIS makes read()/readBytes() return bytes of the current file, but we want to hash the entire container here. + require(jar !is JarInputStream) + + val bytes = getBytes(jar) + val sha256 = bytes.sha256() if (!files.containsKey(sha256)) { files[sha256] = bytes @@ -49,4 +53,21 @@ class MockAttachmentStorage : AttachmentStorage, SingletonSerializeAsToken() { override fun queryAttachments(criteria: AttachmentQueryCriteria, sorting: AttachmentSort?): List { throw NotImplementedError("Querying for attachments not implemented") } + + override fun hasAttachment(attachmentId: AttachmentId) = files.containsKey(attachmentId) + + fun getAttachmentIdAndBytes(jar: InputStream): Pair { + val bytes = getBytes(jar) + return Pair(bytes.sha256(), bytes) + } + + override fun importOrGetAttachment(jar: InputStream): AttachmentId { + try { + return importAttachment(jar) + } + catch (faee: java.nio.file.FileAlreadyExistsException) { + return AttachmentId.parse(faee.message!!) + } + } + }