diff --git a/core/src/main/kotlin/net/corda/core/internal/InternalUtils.kt b/core/src/main/kotlin/net/corda/core/internal/InternalUtils.kt index 1a0a1d5143..56ccac1646 100644 --- a/core/src/main/kotlin/net/corda/core/internal/InternalUtils.kt +++ b/core/src/main/kotlin/net/corda/core/internal/InternalUtils.kt @@ -516,4 +516,15 @@ fun createSimpleCache(maxSize: Int, onEject: (MutableMap.MutableEntry MutableMap.toSynchronised(): MutableMap = Collections.synchronizedMap(this) \ No newline at end of file +fun MutableMap.toSynchronised(): MutableMap = Collections.synchronizedMap(this) + +private fun isPackageValid(packageName: String): Boolean = packageName.isNotEmpty() && !packageName.endsWith(".") && packageName.split(".").all { token -> + Character.isJavaIdentifierStart(token[0]) && token.toCharArray().drop(1).all { Character.isJavaIdentifierPart(it) } +} + +/** + * Check if a string is a legal Java package name. + */ +fun requirePackageValid(name: String) { + require(isPackageValid(name)) { "Invalid Java package name: `$name`." } +} diff --git a/core/src/main/kotlin/net/corda/core/node/NetworkParameters.kt b/core/src/main/kotlin/net/corda/core/node/NetworkParameters.kt index 9a71d84f90..d43311b07f 100644 --- a/core/src/main/kotlin/net/corda/core/node/NetworkParameters.kt +++ b/core/src/main/kotlin/net/corda/core/node/NetworkParameters.kt @@ -4,6 +4,7 @@ import net.corda.core.CordaRuntimeException import net.corda.core.KeepForDJVM import net.corda.core.crypto.toStringShort import net.corda.core.identity.Party +import net.corda.core.internal.requirePackageValid import net.corda.core.node.services.AttachmentId import net.corda.core.serialization.CordaSerializable import net.corda.core.serialization.DeprecatedConstructorForDeserialization @@ -47,7 +48,7 @@ data class NetworkParameters( @AutoAcceptable val epoch: Int, @AutoAcceptable val whitelistedContractImplementations: Map>, val eventHorizon: Duration, - @AutoAcceptable val packageOwnership: Map + @AutoAcceptable val packageOwnership: Map ) { // DOCEND 1 @DeprecatedConstructorForDeserialization(1) @@ -92,9 +93,27 @@ data class NetworkParameters( companion object { private val memberPropertyPartition = NetworkParameters::class.declaredMemberProperties.asSequence() .partition { it.isAutoAcceptable() } - private val autoAcceptableNamesAndGetters = memberPropertyPartition.first.associateBy({it.name}, {it.javaGetter}) + private val autoAcceptableNamesAndGetters = memberPropertyPartition.first.associateBy({ it.name }, { it.javaGetter }) private val nonAutoAcceptableGetters = memberPropertyPartition.second.map { it.javaGetter } val autoAcceptablePropertyNames = autoAcceptableNamesAndGetters.keys + + /** + * Returns true if the [fullClassName] is in a subpackage of [packageName]. + * E.g.: "com.megacorp" owns "com.megacorp.tokens.MegaToken" + * + * Note: The ownership check is ignoring case to prevent people from just releasing a jar with: "com.megaCorp.megatoken" and pretend they are MegaCorp. + * By making the check case insensitive, the node will require that the jar is signed by MegaCorp, so the attack fails. + */ + private fun owns(packageName: String, fullClassName: String) = fullClassName.startsWith("$packageName.", ignoreCase = true) + + // Make sure that packages don't overlap so that ownership is clear. + private fun noOverlap(packages: Collection) = packages.all { currentPackage -> + packages.none { otherPackage -> otherPackage != currentPackage && otherPackage.startsWith("${currentPackage}.") } + } + + private fun KProperty1.isAutoAcceptable(): Boolean { + return this.findAnnotation() != null + } } init { @@ -104,6 +123,7 @@ data class NetworkParameters( require(maxMessageSize > 0) { "maxMessageSize must be at least 1" } require(maxTransactionSize > 0) { "maxTransactionSize must be at least 1" } require(!eventHorizon.isNegative) { "eventHorizon must be positive value" } + packageOwnership.keys.forEach(::requirePackageValid) require(noOverlap(packageOwnership.keys)) { "multiple packages added to the packageOwnership overlap." } } @@ -165,7 +185,7 @@ data class NetworkParameters( /** * Returns the public key of the package owner of the [contractClassName], or null if not owned. */ - fun getOwnerOf(contractClassName: String): PublicKey? = this.packageOwnership.filterKeys { it.owns(contractClassName) }.values.singleOrNull() + fun getOwnerOf(contractClassName: String): PublicKey? = this.packageOwnership.filterKeys { packageName -> owns(packageName, contractClassName) }.values.singleOrNull() /** * Returns true if the only properties changed in [newNetworkParameters] are [AutoAcceptable] and not @@ -173,7 +193,7 @@ data class NetworkParameters( */ fun canAutoAccept(newNetworkParameters: NetworkParameters, excludedParameterNames: Set): Boolean { return nonAutoAcceptableGetters.none { valueChanged(newNetworkParameters, it) } && - autoAcceptableNamesAndGetters.none { excludedParameterNames.contains(it.key) && valueChanged(newNetworkParameters, it.value) } + autoAcceptableNamesAndGetters.none { excludedParameterNames.contains(it.key) && valueChanged(newNetworkParameters, it.value) } } private fun valueChanged(newNetworkParameters: NetworkParameters, getter: Method?): Boolean { @@ -197,38 +217,3 @@ data class NotaryInfo(val identity: Party, val validating: Boolean) * version. */ class ZoneVersionTooLowException(message: String) : CordaRuntimeException(message) - -/** - * A wrapper for a legal java package. Used by the network parameters to store package ownership. - */ -@CordaSerializable -data class JavaPackageName(val name: String) { - init { - require(isPackageValid(name)) { "Invalid Java package name: $name" } - } - - /** - * Returns true if the [fullClassName] is in a subpackage of the current package. - * E.g.: "com.megacorp" owns "com.megacorp.tokens.MegaToken" - * - * Note: The ownership check is ignoring case to prevent people from just releasing a jar with: "com.megaCorp.megatoken" and pretend they are MegaCorp. - * By making the check case insensitive, the node will require that the jar is signed by MegaCorp, so the attack fails. - */ - fun owns(fullClassName: String) = fullClassName.startsWith("$name.", ignoreCase = true) - - override fun toString() = name -} - -// Check if a string is a legal Java package name. -private fun isPackageValid(packageName: String): Boolean = packageName.isNotEmpty() && !packageName.endsWith(".") && packageName.split(".").all { token -> - Character.isJavaIdentifierStart(token[0]) && token.toCharArray().drop(1).all { Character.isJavaIdentifierPart(it) } -} - -// Make sure that packages don't overlap so that ownership is clear. -private fun noOverlap(packages: Collection) = packages.all { currentPackage -> - packages.none { otherPackage -> otherPackage != currentPackage && otherPackage.name.startsWith("${currentPackage.name}.") } -} - -private fun KProperty1.isAutoAcceptable(): Boolean { - return this.findAnnotation() != null -} \ No newline at end of file diff --git a/core/src/test/kotlin/net/corda/core/contracts/PackageOwnershipVerificationTests.kt b/core/src/test/kotlin/net/corda/core/contracts/PackageOwnershipVerificationTests.kt index ecbb8f8374..f2da9f77ea 100644 --- a/core/src/test/kotlin/net/corda/core/contracts/PackageOwnershipVerificationTests.kt +++ b/core/src/test/kotlin/net/corda/core/contracts/PackageOwnershipVerificationTests.kt @@ -6,11 +6,7 @@ import net.corda.core.crypto.Crypto import net.corda.core.crypto.SecureHash import net.corda.core.identity.AbstractParty import net.corda.core.identity.CordaX500Name -import net.corda.core.node.JavaPackageName import net.corda.core.transactions.LedgerTransaction -import net.corda.finance.POUNDS -import net.corda.finance.`issued by` -import net.corda.finance.contracts.asset.Cash import net.corda.node.services.api.IdentityServiceInternal import net.corda.testing.common.internal.testNetworkParameters import net.corda.testing.core.DUMMY_NOTARY_NAME @@ -48,7 +44,7 @@ class PackageOwnershipVerificationTests { doReturn(BOB_PARTY).whenever(it).partyFromKey(BOB_PUBKEY) }, networkParameters = testNetworkParameters() - .copy(packageOwnership = mapOf(JavaPackageName("net.corda.core.contracts") to OWNER_KEY_PAIR.public)) + .copy(packageOwnership = mapOf("net.corda.core.contracts" to OWNER_KEY_PAIR.public)) ) @Test diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/network/NetworkBootstrapper.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/network/NetworkBootstrapper.kt index 36c567107c..a8dea65b33 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/network/NetworkBootstrapper.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/network/NetworkBootstrapper.kt @@ -9,7 +9,6 @@ import net.corda.core.identity.Party import net.corda.core.internal.* import net.corda.core.internal.concurrent.fork import net.corda.core.internal.concurrent.transpose -import net.corda.core.node.JavaPackageName import net.corda.core.node.NetworkParameters import net.corda.core.node.NodeInfo import net.corda.core.node.NotaryInfo @@ -190,7 +189,7 @@ internal constructor(private val initSerEnv: Boolean, } /** Entry point for the tool */ - fun bootstrap(directory: Path, copyCordapps: Boolean, minimumPlatformVersion: Int, packageOwnership: Map = emptyMap()) { + fun bootstrap(directory: Path, copyCordapps: Boolean, minimumPlatformVersion: Int, packageOwnership: Map = emptyMap()) { require(minimumPlatformVersion <= PLATFORM_VERSION) { "Minimum platform version cannot be greater than $PLATFORM_VERSION" } // Don't accidently include the bootstrapper jar as a CorDapp! val bootstrapperJar = javaClass.location.toPath() @@ -206,7 +205,7 @@ internal constructor(private val initSerEnv: Boolean, copyCordapps: Boolean, fromCordform: Boolean, minimumPlatformVersion: Int = PLATFORM_VERSION, - packageOwnership: Map = emptyMap() + packageOwnership: Map = emptyMap() ) { directory.createDirectories() println("Bootstrapping local test network in $directory") @@ -385,7 +384,7 @@ internal constructor(private val initSerEnv: Boolean, existingNetParams: NetworkParameters?, nodeDirs: List, minimumPlatformVersion: Int, - packageOwnership: Map + packageOwnership: Map ): NetworkParameters { // TODO Add config for maxMessageSize and maxTransactionSize val netParams = if (existingNetParams != null) { diff --git a/node-api/src/test/kotlin/net/corda/nodeapi/internal/network/NetworkBootstrapperTest.kt b/node-api/src/test/kotlin/net/corda/nodeapi/internal/network/NetworkBootstrapperTest.kt index 0c2345db05..cd691b53f4 100644 --- a/node-api/src/test/kotlin/net/corda/nodeapi/internal/network/NetworkBootstrapperTest.kt +++ b/node-api/src/test/kotlin/net/corda/nodeapi/internal/network/NetworkBootstrapperTest.kt @@ -5,7 +5,6 @@ import net.corda.core.crypto.secureRandomBytes import net.corda.core.crypto.sha256 import net.corda.core.identity.CordaX500Name import net.corda.core.internal.* -import net.corda.core.node.JavaPackageName import net.corda.core.node.NetworkParameters import net.corda.core.node.NodeInfo import net.corda.core.serialization.serialize @@ -214,8 +213,8 @@ class NetworkBootstrapperTest { private val ALICE = TestIdentity(ALICE_NAME, 70) private val BOB = TestIdentity(BOB_NAME, 80) - private val alicePackageName = JavaPackageName("com.example.alice") - private val bobPackageName = JavaPackageName("com.example.bob") + private val alicePackageName = "com.example.alice" + private val bobPackageName = "com.example.bob" @Test fun `register new package namespace in existing network`() { @@ -238,7 +237,7 @@ class NetworkBootstrapperTest { @Test fun `attempt to register overlapping namespaces in existing network`() { createNodeConfFile("alice", aliceConfig) - val greedyNamespace = JavaPackageName("com.example") + val greedyNamespace = "com.example" bootstrap(packageOwnership = mapOf(Pair(greedyNamespace, ALICE.publicKey))) assertContainsPackageOwner("alice", mapOf(Pair(greedyNamespace, ALICE.publicKey))) // register overlapping package name @@ -293,7 +292,7 @@ class NetworkBootstrapperTest { return bytes } - private fun bootstrap(copyCordapps: Boolean = true, packageOwnership : Map = emptyMap()) { + private fun bootstrap(copyCordapps: Boolean = true, packageOwnership : Map = emptyMap()) { providedCordaJar = (rootDir / "corda.jar").let { if (it.exists()) it.readAll() else null } bootstrapper.bootstrap(rootDir, copyCordapps, PLATFORM_VERSION, packageOwnership) } @@ -363,7 +362,7 @@ class NetworkBootstrapperTest { } } - private fun assertContainsPackageOwner(nodeDirName: String, packageOwners: Map) { + private fun assertContainsPackageOwner(nodeDirName: String, packageOwners: Map) { val networkParams = (rootDir / nodeDirName).networkParameters assertThat(networkParams.packageOwnership).isEqualTo(packageOwners) } diff --git a/node/src/test/kotlin/net/corda/node/internal/NetworkParametersTest.kt b/node/src/test/kotlin/net/corda/node/internal/NetworkParametersTest.kt index 4be3c41682..6546ab887c 100644 --- a/node/src/test/kotlin/net/corda/node/internal/NetworkParametersTest.kt +++ b/node/src/test/kotlin/net/corda/node/internal/NetworkParametersTest.kt @@ -1,7 +1,6 @@ package net.corda.node.internal import net.corda.core.crypto.generateKeyPair -import net.corda.core.node.JavaPackageName import net.corda.core.node.NetworkParameters import net.corda.core.node.NotaryInfo import net.corda.core.node.services.AttachmentId @@ -29,6 +28,7 @@ import org.junit.After import org.junit.Test import java.nio.file.Path import java.time.Instant +import kotlin.test.assertEquals import kotlin.test.assertFails class NetworkParametersTest { @@ -91,9 +91,7 @@ class NetworkParametersTest { 1, emptyMap(), Int.MAX_VALUE.days, - mapOf( - JavaPackageName("com.!example.stuff") to key2 - ) + mapOf("com.!example.stuff" to key2) ) }.withMessageContaining("Invalid Java package name") @@ -107,13 +105,13 @@ class NetworkParametersTest { emptyMap(), Int.MAX_VALUE.days, mapOf( - JavaPackageName("com.example") to key1, - JavaPackageName("com.example.stuff") to key2 + "com.example" to key1, + "com.example.stuff" to key2 ) ) }.withMessage("multiple packages added to the packageOwnership overlap.") - NetworkParameters(1, + val params = NetworkParameters(1, emptyList(), 2001, 2000, @@ -122,21 +120,22 @@ class NetworkParametersTest { emptyMap(), Int.MAX_VALUE.days, mapOf( - JavaPackageName("com.example") to key1, - JavaPackageName("com.examplestuff") to key2 + "com.example" to key1, + "com.examplestuff" to key2 ) ) - assert(JavaPackageName("com.example").owns("com.example.something.MyClass")) - assert(!JavaPackageName("com.example").owns("com.examplesomething.MyClass")) - assert(!JavaPackageName("com.exam").owns("com.example.something.MyClass")) + assertEquals(params.getOwnerOf("com.example.something.MyClass"), key1) + assertEquals(params.getOwnerOf("com.examplesomething.MyClass"), null) + assertEquals(params.getOwnerOf("com.examplestuff.something.MyClass"), key2) + assertEquals(params.getOwnerOf("com.exam.something.MyClass"), null) } @Test fun `auto acceptance checks are correct`() { val packageOwnership = mapOf( - JavaPackageName("com.example1") to generateKeyPair().public, - JavaPackageName("com.example2") to generateKeyPair().public) + "com.example1" to generateKeyPair().public, + "com.example2" to generateKeyPair().public) val whitelistedContractImplementations = mapOf( "example1" to listOf(AttachmentId.randomSHA256()), "example2" to listOf(AttachmentId.randomSHA256())) diff --git a/tools/bootstrapper/src/main/kotlin/net/corda/bootstrapper/Main.kt b/tools/bootstrapper/src/main/kotlin/net/corda/bootstrapper/Main.kt index ed21e8cb26..0ec24440fd 100644 --- a/tools/bootstrapper/src/main/kotlin/net/corda/bootstrapper/Main.kt +++ b/tools/bootstrapper/src/main/kotlin/net/corda/bootstrapper/Main.kt @@ -3,7 +3,7 @@ package net.corda.bootstrapper import net.corda.cliutils.CordaCliWrapper import net.corda.cliutils.start import net.corda.core.internal.PLATFORM_VERSION -import net.corda.core.node.JavaPackageName +import net.corda.core.internal.requirePackageValid import net.corda.nodeapi.internal.crypto.loadKeyStore import net.corda.nodeapi.internal.network.NetworkBootstrapper import picocli.CommandLine @@ -47,13 +47,12 @@ class NetworkBootstrapperRunner : CordaCliWrapper("bootstrapper", "Bootstrap a l var registerPackageOwnership: List = mutableListOf() @Option(names = ["--unregister-package-owner"], - converter = [JavaPackageNameConverter::class], description = [ "Unregister owner of Java package namespace in the network-parameters.", "Format: [java-package-namespace]", " `java-package-namespace` is case insensitive and cannot be a sub-package of an existing registered namespace" ]) - var unregisterPackageOwnership: List = mutableListOf() + var unregisterPackageOwnership: List = mutableListOf() override fun runProgram(): Int { NetworkBootstrapper().bootstrap(dir.toAbsolutePath().normalize(), @@ -67,10 +66,10 @@ class NetworkBootstrapperRunner : CordaCliWrapper("bootstrapper", "Bootstrap a l } -data class PackageOwner(val javaPackageName: JavaPackageName, val publicKey: PublicKey) +data class PackageOwner(val javaPackageName: String, val publicKey: PublicKey) /** - * Converter from String to PackageOwner (JavaPackageName and PublicKey) + * Converter from String to PackageOwner (String and PublicKey) */ class PackageOwnerConverter : CommandLine.ITypeConverter { override fun convert(packageOwner: String): PackageOwner { @@ -78,8 +77,11 @@ class PackageOwnerConverter : CommandLine.ITypeConverter { val packageOwnerSpec = packageOwner.split(";") if (packageOwnerSpec.size < 4) throw IllegalArgumentException("Package owner must specify 4 elements separated by semi-colon: 'java-package-namespace;keyStorePath;keyStorePassword;alias'") + // java package name validation - val javaPackageName = JavaPackageName(packageOwnerSpec[0]) + val javaPackageName = packageOwnerSpec[0] + requirePackageValid(javaPackageName) + // cater for passwords that include the argument delimiter field val keyStorePassword = if (packageOwnerSpec.size > 4) @@ -105,12 +107,3 @@ class PackageOwnerConverter : CommandLine.ITypeConverter { else throw IllegalArgumentException("Must specify package owner argument: 'java-package-namespace;keyStorePath;keyStorePassword;alias'") } } - -/** - * Converter from String to JavaPackageName. - */ -class JavaPackageNameConverter : CommandLine.ITypeConverter { - override fun convert(packageName: String): JavaPackageName { - return JavaPackageName(packageName) - } -} diff --git a/tools/bootstrapper/src/test/kotlin/net/corda/bootstrapper/PackageOwnerParsingTest.kt b/tools/bootstrapper/src/test/kotlin/net/corda/bootstrapper/PackageOwnerParsingTest.kt index cbc9b53cd4..b0df16c391 100644 --- a/tools/bootstrapper/src/test/kotlin/net/corda/bootstrapper/PackageOwnerParsingTest.kt +++ b/tools/bootstrapper/src/test/kotlin/net/corda/bootstrapper/PackageOwnerParsingTest.kt @@ -2,7 +2,6 @@ package net.corda.bootstrapper import net.corda.core.internal.deleteRecursively import net.corda.core.internal.div -import net.corda.core.node.JavaPackageName import net.corda.testing.core.ALICE_NAME import net.corda.testing.core.BOB_NAME import net.corda.testing.core.CHARLIE_NAME @@ -55,7 +54,7 @@ class PackageOwnerParsingTest { val aliceKeyStorePath = dirAlice / "_teststore" val args = arrayOf("--register-package-owner", "com.example.stuff;$aliceKeyStorePath;$ALICE_PASS;$ALICE") commandLine.parse(*args) - assertThat(networkBootstrapper.registerPackageOwnership[0].javaPackageName).isEqualTo(JavaPackageName("com.example.stuff")) + assertThat(networkBootstrapper.registerPackageOwnership[0].javaPackageName).isEqualTo("com.example.stuff") } @Test @@ -141,7 +140,7 @@ class PackageOwnerParsingTest { "net.something4;$aliceKeyStorePath5;\"\"passw;rd\"\";${ALICE}5") packageOwnerSpecs.forEachIndexed { i, packageOwnerSpec -> commandLine.parse(*arrayOf("--register-package-owner", packageOwnerSpec)) - assertThat(networkBootstrapper.registerPackageOwnership[0].javaPackageName).isEqualTo(JavaPackageName("net.something$i")) + assertThat(networkBootstrapper.registerPackageOwnership[0].javaPackageName).isEqualTo("net.something$i") } } @@ -149,7 +148,7 @@ class PackageOwnerParsingTest { fun `parse unregister request with single mapping`() { val args = arrayOf("--unregister-package-owner", "com.example.stuff") commandLine.parse(*args) - assertThat(networkBootstrapper.unregisterPackageOwnership).contains(JavaPackageName("com.example.stuff")) + assertThat(networkBootstrapper.unregisterPackageOwnership).contains("com.example.stuff") } @Test @@ -158,8 +157,8 @@ class PackageOwnerParsingTest { val args = arrayOf("--register-package-owner", "com.example.stuff;$aliceKeyStorePath;$ALICE_PASS;$ALICE", "--unregister-package-owner", "com.example.stuff2") commandLine.parse(*args) - assertThat(networkBootstrapper.registerPackageOwnership.map { it.javaPackageName }).contains(JavaPackageName("com.example.stuff")) - assertThat(networkBootstrapper.unregisterPackageOwnership).contains(JavaPackageName("com.example.stuff2")) + assertThat(networkBootstrapper.registerPackageOwnership.map { it.javaPackageName }).contains("com.example.stuff") + assertThat(networkBootstrapper.unregisterPackageOwnership).contains("com.example.stuff2") } }