ENT-2763 - Change packageOwnership type (#4273)

* ENT-2763 - Change packageOwnership type

* ENT-2763 - Address code review comment.

* ENT-2673 Address code review comments.

* ENT-2673 Address code review comments.

* ENT-2673 Fix test

* ENT-2673 Address code review comments.
This commit is contained in:
Tudor Malene 2018-11-21 20:41:56 +00:00 committed by GitHub
parent 014f71d36a
commit 504f650022
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 71 additions and 90 deletions

View File

@ -516,4 +516,15 @@ fun <K, V> createSimpleCache(maxSize: Int, onEject: (MutableMap.MutableEntry<K,
}
}
fun <K,V> MutableMap<K,V>.toSynchronised(): MutableMap<K,V> = Collections.synchronizedMap(this)
fun <K, V> MutableMap<K, V>.toSynchronised(): MutableMap<K, V> = 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`." }
}

View File

@ -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<String, List<AttachmentId>>,
val eventHorizon: Duration,
@AutoAcceptable val packageOwnership: Map<JavaPackageName, PublicKey>
@AutoAcceptable val packageOwnership: Map<String, PublicKey>
) {
// 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<String>) = packages.all { currentPackage ->
packages.none { otherPackage -> otherPackage != currentPackage && otherPackage.startsWith("${currentPackage}.") }
}
private fun KProperty1<out NetworkParameters, Any?>.isAutoAcceptable(): Boolean {
return this.findAnnotation<AutoAcceptable>() != 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<String>): 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<JavaPackageName>) = packages.all { currentPackage ->
packages.none { otherPackage -> otherPackage != currentPackage && otherPackage.name.startsWith("${currentPackage.name}.") }
}
private fun KProperty1<out NetworkParameters, Any?>.isAutoAcceptable(): Boolean {
return this.findAnnotation<AutoAcceptable>() != null
}

View File

@ -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

View File

@ -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<JavaPackageName, PublicKey?> = emptyMap()) {
fun bootstrap(directory: Path, copyCordapps: Boolean, minimumPlatformVersion: Int, packageOwnership: Map<String, PublicKey?> = 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<JavaPackageName, PublicKey?> = emptyMap()
packageOwnership: Map<String, PublicKey?> = emptyMap()
) {
directory.createDirectories()
println("Bootstrapping local test network in $directory")
@ -385,7 +384,7 @@ internal constructor(private val initSerEnv: Boolean,
existingNetParams: NetworkParameters?,
nodeDirs: List<Path>,
minimumPlatformVersion: Int,
packageOwnership: Map<JavaPackageName, PublicKey?>
packageOwnership: Map<String, PublicKey?>
): NetworkParameters {
// TODO Add config for maxMessageSize and maxTransactionSize
val netParams = if (existingNetParams != null) {

View File

@ -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<JavaPackageName, PublicKey?> = emptyMap()) {
private fun bootstrap(copyCordapps: Boolean = true, packageOwnership : Map<String, PublicKey?> = 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<JavaPackageName, PublicKey>) {
private fun assertContainsPackageOwner(nodeDirName: String, packageOwners: Map<String, PublicKey>) {
val networkParams = (rootDir / nodeDirName).networkParameters
assertThat(networkParams.packageOwnership).isEqualTo(packageOwners)
}

View File

@ -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()))

View File

@ -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<PackageOwner> = 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<JavaPackageName> = mutableListOf()
var unregisterPackageOwnership: List<String> = 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<PackageOwner> {
override fun convert(packageOwner: String): PackageOwner {
@ -78,8 +77,11 @@ class PackageOwnerConverter : CommandLine.ITypeConverter<PackageOwner> {
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<PackageOwner> {
else throw IllegalArgumentException("Must specify package owner argument: 'java-package-namespace;keyStorePath;keyStorePassword;alias'")
}
}
/**
* Converter from String to JavaPackageName.
*/
class JavaPackageNameConverter : CommandLine.ITypeConverter<JavaPackageName> {
override fun convert(packageName: String): JavaPackageName {
return JavaPackageName(packageName)
}
}

View File

@ -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")
}
}