CORDA-3668: Prevent AttachmentURLConnection from assigning ALL_PERMISSIONS. (#6080)

* CORDA-3668: Prevent AttachmentURLConnection from assigning ALL_PERMISSIONS to classes inside an attachment.

* Strengthen the comment warning about AttachmentURLConnection.getPermission.
This commit is contained in:
Chris Rankin 2020-03-19 11:20:53 +00:00 committed by GitHub
parent 56067acd20
commit dd7852f2b8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 65 additions and 34 deletions

View File

@ -21,14 +21,18 @@ import net.corda.testing.internal.services.InternalMockAttachmentStorage
import net.corda.testing.node.internal.FINANCE_CONTRACTS_CORDAPP
import net.corda.testing.services.MockAttachmentStorage
import org.apache.commons.io.IOUtils
import org.assertj.core.api.Assertions.assertThat
import org.junit.Assert.assertArrayEquals
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNull
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Test
import java.io.ByteArrayOutputStream
import java.io.InputStream
import java.net.URL
import kotlin.test.assertFailsWith
import kotlin.test.fail
class AttachmentsClassLoaderTests {
companion object {
@ -84,14 +88,29 @@ class AttachmentsClassLoaderTests {
}
}
@Test(timeout=300_000)
fun `test contracts have no permissions for protection domain`() {
val isolatedId = importAttachment(ISOLATED_CONTRACTS_JAR_PATH.openStream(), "app", "isolated.jar")
assertNull(System.getSecurityManager())
createClassloader(isolatedId).use { classLoader ->
val contractClass = Class.forName(ISOLATED_CONTRACT_CLASS_NAME, true, classLoader)
val protectionDomain = contractClass.protectionDomain ?: fail("Protection Domain missing")
val permissions = protectionDomain.permissions ?: fail("Protection domain has no permissions")
assertThat(permissions.elements().toList()).isEmpty()
assertTrue(permissions.isReadOnly)
}
}
@Test(timeout=300_000)
fun `Dynamically load AnotherDummyContract from isolated contracts jar using the AttachmentsClassLoader`() {
val isolatedId = importAttachment(ISOLATED_CONTRACTS_JAR_PATH.openStream(), "app", "isolated.jar")
val classloader = createClassloader(isolatedId)
val contractClass = Class.forName(ISOLATED_CONTRACT_CLASS_NAME, true, classloader)
val contract = contractClass.getDeclaredConstructor().newInstance() as Contract
assertEquals("helloworld", contract.declaredField<Any?>("magicString").value)
createClassloader(isolatedId).use { classloader ->
val contractClass = Class.forName(ISOLATED_CONTRACT_CLASS_NAME, true, classloader)
val contract = contractClass.getDeclaredConstructor().newInstance() as Contract
assertEquals("helloworld", contract.declaredField<Any?>("magicString").value)
}
}
@Test(timeout=300_000)
@ -100,7 +119,7 @@ class AttachmentsClassLoaderTests {
val att2 = importAttachment(ISOLATED_CONTRACTS_JAR_PATH_V4.openStream(), "app", "isolated-4.0.jar")
assertFailsWith(TransactionVerificationException.OverlappingAttachmentsException::class) {
createClassloader(listOf(att1, att2))
createClassloader(listOf(att1, att2)).use {}
}
}
@ -111,7 +130,7 @@ class AttachmentsClassLoaderTests {
val isolatedSignedId = importAttachment(signedJar.first.toUri().toURL().openStream(), "app", "isolated-signed.jar")
// does not throw OverlappingAttachments exception
createClassloader(listOf(isolatedId, isolatedSignedId))
createClassloader(listOf(isolatedId, isolatedSignedId)).use {}
}
@Test(timeout=300_000)
@ -120,7 +139,7 @@ class AttachmentsClassLoaderTests {
val att2 = importAttachment(FINANCE_CONTRACTS_CORDAPP.jarFile.inputStream(), "app", "finance.jar")
// does not throw OverlappingAttachments exception
createClassloader(listOf(att1, att2))
createClassloader(listOf(att1, att2)).use {}
}
@Test(timeout=300_000)
@ -128,12 +147,13 @@ class AttachmentsClassLoaderTests {
val att1 = importAttachment(fakeAttachment("file1.txt", "some data").inputStream(), "app", "file1.jar")
val att2 = importAttachment(fakeAttachment("file2.txt", "some other data").inputStream(), "app", "file2.jar")
val cl = createClassloader(listOf(att1, att2))
val txt = IOUtils.toString(cl.getResourceAsStream("file1.txt"), Charsets.UTF_8.name())
assertEquals("some data", txt)
createClassloader(listOf(att1, att2)).use { cl ->
val txt = IOUtils.toString(cl.getResourceAsStream("file1.txt"), Charsets.UTF_8.name())
assertEquals("some data", txt)
val txt1 = IOUtils.toString(cl.getResourceAsStream("file2.txt"), Charsets.UTF_8.name())
assertEquals("some other data", txt1)
val txt1 = IOUtils.toString(cl.getResourceAsStream("file2.txt"), Charsets.UTF_8.name())
assertEquals("some other data", txt1)
}
}
@Test(timeout=300_000)
@ -141,9 +161,10 @@ class AttachmentsClassLoaderTests {
val att1 = importAttachment(fakeAttachment("file1.txt", "same data", "file2.txt", "same other data").inputStream(), "app", "file1.jar")
val att2 = importAttachment(fakeAttachment("file1.txt", "same data", "file3.txt", "same totally different").inputStream(), "app", "file2.jar")
val cl = createClassloader(listOf(att1, att2))
val txt = IOUtils.toString(cl.getResourceAsStream("file1.txt"), Charsets.UTF_8.name())
assertEquals("same data", txt)
createClassloader(listOf(att1, att2)).use { cl ->
val txt = IOUtils.toString(cl.getResourceAsStream("file1.txt"), Charsets.UTF_8.name())
assertEquals("same data", txt)
}
}
@Test(timeout=300_000)
@ -152,7 +173,7 @@ class AttachmentsClassLoaderTests {
val att1 = importAttachment(fakeAttachment(path, "some data").inputStream(), "app", "file1.jar")
val att2 = importAttachment(fakeAttachment(path, "some other data").inputStream(), "app", "file2.jar")
createClassloader(listOf(att1, att2))
createClassloader(listOf(att1, att2)).use {}
}
}
@ -161,7 +182,7 @@ class AttachmentsClassLoaderTests {
val att1 = importAttachment(fakeAttachment("meta-inf/services/net.corda.core.serialization.SerializationWhitelist", "some data").inputStream(), "app", "file1.jar")
val att2 = importAttachment(fakeAttachment("meta-inf/services/net.corda.core.serialization.SerializationWhitelist", "some other data").inputStream(), "app", "file2.jar")
createClassloader(listOf(att1, att2))
createClassloader(listOf(att1, att2)).use {}
}
@Test(timeout=300_000)
@ -170,7 +191,7 @@ class AttachmentsClassLoaderTests {
val att2 = importAttachment(fakeAttachment("meta-inf/services/com.example.something", "some other data").inputStream(), "app", "file2.jar")
assertFailsWith(TransactionVerificationException.OverlappingAttachmentsException::class) {
createClassloader(listOf(att1, att2))
createClassloader(listOf(att1, att2)).use {}
}
}
@ -180,7 +201,7 @@ class AttachmentsClassLoaderTests {
val att2 = storage.importAttachment(fakeAttachment("file1.txt", "some other data").inputStream(), "app", "file2.jar")
assertFailsWith(TransactionVerificationException.OverlappingAttachmentsException::class) {
createClassloader(listOf(att1, att2))
createClassloader(listOf(att1, att2)).use {}
}
}
@ -191,7 +212,7 @@ class AttachmentsClassLoaderTests {
val att1 = importAttachment(ISOLATED_CONTRACTS_JAR_PATH.openStream(), "app", ISOLATED_CONTRACTS_JAR_PATH.file)
val att2 = importAttachment(fakeAttachment("net/corda/finance/contracts/isolated/AnotherDummyContract\$State.class", "some attackdata").inputStream(), "app", "file2.jar")
assertFailsWith(TransactionVerificationException.OverlappingAttachmentsException::class) {
createClassloader(listOf(att1, att2))
createClassloader(listOf(att1, att2)).use {}
}
}
@ -220,10 +241,10 @@ class AttachmentsClassLoaderTests {
val untrustedClassJar = importAttachment(fakeAttachment("/com/example/something/MaliciousClass.class", "some malicious data").inputStream(), "untrusted", "file2.jar")
val trustedClassJar = importAttachment(fakeAttachment("/com/example/something/VirtuousClass.class", "some other data").inputStream(), "app", "file3.jar")
createClassloader(listOf(trustedResourceJar, untrustedResourceJar, trustedClassJar))
assertFailsWith(TransactionVerificationException.UntrustedAttachmentsException::class) {
createClassloader(listOf(trustedResourceJar, untrustedResourceJar, trustedClassJar, untrustedClassJar))
createClassloader(listOf(trustedResourceJar, untrustedResourceJar, trustedClassJar)).use {
assertFailsWith(TransactionVerificationException.UntrustedAttachmentsException::class) {
createClassloader(listOf(trustedResourceJar, untrustedResourceJar, trustedClassJar, untrustedClassJar)).use {}
}
}
}
@ -257,7 +278,7 @@ class AttachmentsClassLoaderTests {
signers = listOf(keyPairA.public, keyPairB.public)
)
createClassloader(untrustedAttachment)
createClassloader(untrustedAttachment).use {}
}
@Test(timeout=300_000)
@ -287,7 +308,7 @@ class AttachmentsClassLoaderTests {
signers = listOf(keyPairA.public, keyPairB.public)
)
createClassloader(untrustedAttachment)
createClassloader(untrustedAttachment).use {}
}
@Test(timeout=300_000)
@ -306,7 +327,7 @@ class AttachmentsClassLoaderTests {
)
assertFailsWith(TransactionVerificationException.UntrustedAttachmentsException::class) {
createClassloader(untrustedAttachment)
createClassloader(untrustedAttachment).use {}
}
}
@ -337,7 +358,7 @@ class AttachmentsClassLoaderTests {
)
assertFailsWith(TransactionVerificationException.UntrustedAttachmentsException::class) {
createClassloader(untrustedAttachment)
createClassloader(untrustedAttachment).use {}
}
}
@ -380,10 +401,10 @@ class AttachmentsClassLoaderTests {
)
// pass the inherited trust attachment through the classloader first to ensure it does not affect the next loaded attachment
createClassloader(inheritedTrustAttachment)
assertFailsWith(TransactionVerificationException.UntrustedAttachmentsException::class) {
createClassloader(untrustedAttachment)
createClassloader(inheritedTrustAttachment).use {
assertFailsWith(TransactionVerificationException.UntrustedAttachmentsException::class) {
createClassloader(untrustedAttachment).use {}
}
}
}
@ -421,7 +442,7 @@ class AttachmentsClassLoaderTests {
)
assertFailsWith(TransactionVerificationException.UntrustedAttachmentsException::class) {
createClassloader(untrustedAttachment)
createClassloader(untrustedAttachment).use {}
}
}
@ -446,6 +467,6 @@ class AttachmentsClassLoaderTests {
signers = listOf(keyPairA.public)
)
createClassloader(trustedAttachment)
createClassloader(trustedAttachment).use {}
}
}

View File

@ -18,6 +18,7 @@ import java.io.ByteArrayOutputStream
import java.io.IOException
import java.io.InputStream
import java.net.*
import java.security.Permission
import java.util.*
/**
@ -378,6 +379,15 @@ object AttachmentURLStreamHandlerFactory : URLStreamHandlerFactory {
private class AttachmentURLConnection(url: URL, private val attachment: Attachment) : URLConnection(url) {
override fun getContentLengthLong(): Long = attachment.size.toLong()
override fun getInputStream(): InputStream = attachment.open()
/**
* Define the permissions that [AttachmentsClassLoader] will need to
* use this [URL]. The attachment is stored in memory, and so we
* don't need any extra permissions here. But if we don't override
* [getPermission] then [AttachmentsClassLoader] will assign the
* default permission of ALL_PERMISSION to these classes'
* [java.security.ProtectionDomain]. This would be a security hole!
*/
override fun getPermission(): Permission? = null
override fun connect() {
connected = true
}