From 00b570df29112d6f1a09873493f06b2507da88f0 Mon Sep 17 00:00:00 2001 From: igor nitto Date: Fri, 5 Jan 2018 11:50:21 +0000 Subject: [PATCH 1/2] Improve RPC security test coverage [CORDA-827] (#2320) * Added test cases covering encrypted password usage * Renamed UserAuthServiceTests as AuthDBTests: the integration tests checking user credentials loaded from external database (still limited to H2 in-memory for now). * Some internal renamings --- ...UserAuthServiceTests.kt => AuthDBTests.kt} | 203 ++++++++++-------- .../node/services/config/NodeConfiguration.kt | 25 ++- .../node/services/RPCSecurityManagerTest.kt | 15 +- 3 files changed, 136 insertions(+), 107 deletions(-) rename node/src/integration-test/kotlin/net/corda/node/{services/UserAuthServiceTests.kt => AuthDBTests.kt} (61%) diff --git a/node/src/integration-test/kotlin/net/corda/node/services/UserAuthServiceTests.kt b/node/src/integration-test/kotlin/net/corda/node/AuthDBTests.kt similarity index 61% rename from node/src/integration-test/kotlin/net/corda/node/services/UserAuthServiceTests.kt rename to node/src/integration-test/kotlin/net/corda/node/AuthDBTests.kt index 8805144b56..4015265da8 100644 --- a/node/src/integration-test/kotlin/net/corda/node/services/UserAuthServiceTests.kt +++ b/node/src/integration-test/kotlin/net/corda/node/AuthDBTests.kt @@ -1,4 +1,4 @@ -package net.corda.node.services +package net.corda.node import co.paralleluniverse.fibers.Suspendable import net.corda.client.rpc.CordaRPCClient @@ -11,26 +11,89 @@ import net.corda.core.messaging.startFlow import net.corda.finance.flows.CashIssueFlow import net.corda.node.internal.Node import net.corda.node.internal.StartedNode +import net.corda.node.services.Permissions import net.corda.node.services.config.PasswordEncryption -import net.corda.node.services.config.SecurityConfiguration -import net.corda.node.services.config.AuthDataSourceType -import net.corda.nodeapi.internal.config.User -import net.corda.nodeapi.internal.config.toConfig import net.corda.testing.node.internal.NodeBasedTest import net.corda.testing.* import org.apache.activemq.artemis.api.core.ActiveMQSecurityException +import org.apache.shiro.authc.credential.DefaultPasswordService import org.junit.After import org.junit.Before import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.Parameterized import java.sql.DriverManager import java.sql.Statement -import java.util.* import kotlin.test.assertFailsWith -abstract class UserAuthServiceTest : NodeBasedTest() { +/* + * Starts Node's instance configured to load clients credentials and permissions from an external DB, then + * check authentication/authorization of RPC connections. + */ +@RunWith(Parameterized::class) +class AuthDBTests : NodeBasedTest() { - protected lateinit var node: StartedNode - protected lateinit var client: CordaRPCClient + private lateinit var node: StartedNode + private lateinit var client: CordaRPCClient + private lateinit var db: UsersDB + + companion object { + private val cacheExpireAfterSecs: Long = 1 + + @JvmStatic + @Parameterized.Parameters(name = "password encryption format = {0}") + fun encFormats() = arrayOf(PasswordEncryption.NONE, PasswordEncryption.SHIRO_1_CRYPT) + } + + @Parameterized.Parameter + lateinit var passwordEncryption: PasswordEncryption + + @Before + fun setup() { + db = UsersDB( + name = "SecurityDataSourceTestDB", + users = listOf(UserAndRoles(username = "user", + password = encodePassword("foo", passwordEncryption), + roles = listOf("default"))), + roleAndPermissions = listOf( + RoleAndPermissions( + role = "default", + permissions = listOf( + Permissions.startFlow(), + Permissions.invokeRpc("vaultQueryBy"), + Permissions.invokeRpc(CordaRPCOps::stateMachinesFeed), + Permissions.invokeRpc("vaultQueryByCriteria"))), + RoleAndPermissions( + role = "admin", + permissions = listOf("ALL") + ))) + + val securityConfig = mapOf( + "security" to mapOf( + "authService" to mapOf( + "dataSource" to mapOf( + "type" to "DB", + "passwordEncryption" to passwordEncryption.toString(), + "connection" to mapOf( + "jdbcUrl" to db.jdbcUrl, + "username" to "", + "password" to "", + "driverClassName" to "org.h2.Driver" + ) + ) + ), + "options" to mapOf( + "cache" to mapOf( + "expireAfterSecs" to cacheExpireAfterSecs, + "maxEntries" to 50 + ) + ) + ) + ) + + node = startNode(ALICE_NAME, rpcUsers = emptyList(), configOverrides = securityConfig) + client = CordaRPCClient(node.internals.configuration.rpcAddress!!) + } @Test fun `login with correct credentials`() { @@ -58,7 +121,7 @@ abstract class UserAuthServiceTest : NodeBasedTest() { val proxy = it.proxy proxy.startFlowDynamic(DummyFlow::class.java) proxy.startTrackedFlowDynamic(DummyFlow::class.java) - proxy.startFlow(::DummyFlow) + proxy.startFlow(AuthDBTests::DummyFlow) assertFailsWith( PermissionException::class, "This user should not be authorized to start flow `CashIssueFlow`") { @@ -85,77 +148,8 @@ abstract class UserAuthServiceTest : NodeBasedTest() { } } - @StartableByRPC - @InitiatingFlow - class DummyFlow : FlowLogic() { - @Suspendable - override fun call() = Unit - } -} - -class UserAuthServiceEmbedded : UserAuthServiceTest() { - - private val rpcUser = User("user", "foo", permissions = setOf( - Permissions.startFlow(), - Permissions.invokeRpc("vaultQueryBy"), - Permissions.invokeRpc(CordaRPCOps::stateMachinesFeed), - Permissions.invokeRpc("vaultQueryByCriteria"))) - - @Before - fun setup() { - val securityConfig = SecurityConfiguration( - authService = SecurityConfiguration.AuthService.fromUsers(listOf(rpcUser))) - - val configOverrides = mapOf("security" to securityConfig.toConfig().root().unwrapped()) - node = startNode(ALICE_NAME, rpcUsers = emptyList(), configOverrides = configOverrides) - client = CordaRPCClient(node.internals.configuration.rpcAddress!!) - } -} - -class UserAuthServiceTestsJDBC : UserAuthServiceTest() { - - private val db = UsersDB( - name = "SecurityDataSourceTestDB", - users = listOf(UserAndRoles(username = "user", - password = "foo", - roles = listOf("default"))), - roleAndPermissions = listOf( - RoleAndPermissions( - role = "default", - permissions = listOf( - Permissions.startFlow(), - Permissions.invokeRpc("vaultQueryBy"), - Permissions.invokeRpc(CordaRPCOps::stateMachinesFeed), - Permissions.invokeRpc("vaultQueryByCriteria"))), - RoleAndPermissions( - role = "admin", - permissions = listOf("ALL") - ))) - - @Before - fun setup() { - val securityConfig = SecurityConfiguration( - authService = SecurityConfiguration.AuthService( - dataSource = SecurityConfiguration.AuthService.DataSource( - type = AuthDataSourceType.DB, - passwordEncryption = PasswordEncryption.NONE, - connection = Properties().apply { - setProperty("jdbcUrl", db.jdbcUrl) - setProperty("username", "") - setProperty("password", "") - setProperty("driverClassName", "org.h2.Driver") - } - ) - ) - ) - - val configOverrides = mapOf("security" to securityConfig.toConfig().root().unwrapped()) - node = startNode(ALICE_NAME, rpcUsers = emptyList(), configOverrides = configOverrides) - client = CordaRPCClient(node.internals.configuration.rpcAddress!!) - } - @Test - fun `Add new users on-the-fly`() { + fun `Add new users dynamically`() { assertFailsWith( ActiveMQSecurityException::class, "Login with incorrect password should fail") { @@ -164,7 +158,7 @@ class UserAuthServiceTestsJDBC : UserAuthServiceTest() { db.insert(UserAndRoles( username = "user2", - password = "bar", + password = encodePassword("bar"), roles = listOf("default"))) client.start("user2", "bar") @@ -174,10 +168,9 @@ class UserAuthServiceTestsJDBC : UserAuthServiceTest() { fun `Modify user permissions during RPC session`() { db.insert(UserAndRoles( username = "user3", - password = "bar", + password = encodePassword("bar"), roles = emptyList())) - client.start("user3", "bar").use { val proxy = it.proxy assertFailsWith( @@ -186,6 +179,7 @@ class UserAuthServiceTestsJDBC : UserAuthServiceTest() { proxy.stateMachinesFeed() } db.addRoleToUser("user3", "default") + Thread.sleep(1500) proxy.stateMachinesFeed() } } @@ -194,13 +188,14 @@ class UserAuthServiceTestsJDBC : UserAuthServiceTest() { fun `Revoke user permissions during RPC session`() { db.insert(UserAndRoles( username = "user4", - password = "test", + password = encodePassword("test"), roles = listOf("default"))) client.start("user4", "test").use { val proxy = it.proxy proxy.stateMachinesFeed() db.deleteUser("user4") + Thread.sleep(1500) assertFailsWith( PermissionException::class, "This user should not be authorized to call 'nodeInfo'") { @@ -209,15 +204,27 @@ class UserAuthServiceTestsJDBC : UserAuthServiceTest() { } } + @StartableByRPC + @InitiatingFlow + class DummyFlow : FlowLogic() { + @Suspendable + override fun call() = Unit + } + @After fun tearDown() { - db.close() + db.close() } + + private fun encodePassword(s: String) = encodePassword(s, passwordEncryption) } private data class UserAndRoles(val username: String, val password: String, val roles: List) private data class RoleAndPermissions(val role: String, val permissions: List) +/* + * Manage in-memory DB mocking a users database with the schema expected by Node's security manager + */ private class UsersDB : AutoCloseable { val jdbcUrl: String @@ -254,12 +261,6 @@ private class UsersDB : AutoCloseable { } } - fun deleteRole(role: String) { - session { - it.execute("DELETE FROM role_permissions WHERE role_name = '$role'") - } - } - fun deleteUser(username: String) { session { it.execute("DELETE FROM users WHERE username = '$username'") @@ -300,4 +301,22 @@ private class UsersDB : AutoCloseable { } } } -} \ No newline at end of file +} + +/* + * Sample of hardcoded hashes to watch for format backward compatibility + */ +private val hashedPasswords = mapOf( + PasswordEncryption.SHIRO_1_CRYPT to mapOf( + "foo" to "\$shiro1\$SHA-256$500000\$WSiEVj6q8d02sFcCk1dkoA==\$MBkU/ghdD9ovoDerdzNfkXdP9Bdhmok7tidvVIqGzcA=", + "bar" to "\$shiro1\$SHA-256$500000\$Q6dmdY1uVMm0LYAWaOHtCA==\$u7NbFaj9tHf2RTW54jedLPiOiGjJv0RVEPIjVquJuYY=", + "test" to "\$shiro1\$SHA-256$500000\$F6CWSFDDxGTlzvREwih8Gw==\$DQhyAPoUw3RdvNYJ1aubCnzEIXm+szGQ3HplaG+euz8=")) + +/* + * A functional object for producing password encoded according to the given scheme. + */ +private fun encodePassword(s: String, format: PasswordEncryption) = when (format) { + PasswordEncryption.NONE -> s + PasswordEncryption.SHIRO_1_CRYPT -> hashedPasswords[format]!![s] ?: + DefaultPasswordService().encryptPassword(s.toCharArray()) + } \ No newline at end of file diff --git a/node/src/main/kotlin/net/corda/node/services/config/NodeConfiguration.kt b/node/src/main/kotlin/net/corda/node/services/config/NodeConfiguration.kt index 60acff95d1..f549aa8bf9 100644 --- a/node/src/main/kotlin/net/corda/node/services/config/NodeConfiguration.kt +++ b/node/src/main/kotlin/net/corda/node/services/config/NodeConfiguration.kt @@ -198,8 +198,16 @@ data class SecurityConfiguration(val authService: SecurityConfiguration.AuthServ data class Options(val cache: Options.Cache?) { // Cache parameters - data class Cache(val expireAfterSecs: Long, val maxEntries: Long) - + data class Cache(val expireAfterSecs: Long, val maxEntries: Long) { + init { + require(expireAfterSecs >= 0) { + "Expected positive value for 'cache.expireAfterSecs'" + } + require(maxEntries > 0) { + "Expected positive value for 'cache.maxEntries'" + } + } + } } // Provider of users credentials and permissions data @@ -223,12 +231,13 @@ data class SecurityConfiguration(val authService: SecurityConfiguration.AuthServ AuthDataSourceType.DB -> AuthServiceId("REMOTE_DATABASE") } - fun fromUsers(users: List) = AuthService( - dataSource = DataSource( - type = AuthDataSourceType.INMEMORY, - users = users, - passwordEncryption = PasswordEncryption.NONE), - id = AuthServiceId("NODE_CONFIG")) + fun fromUsers(users: List, encryption: PasswordEncryption = PasswordEncryption.NONE) = + AuthService( + dataSource = DataSource( + type = AuthDataSourceType.INMEMORY, + users = users, + passwordEncryption = encryption), + id = AuthServiceId("NODE_CONFIG")) } } } \ No newline at end of file diff --git a/node/src/test/kotlin/net/corda/node/services/RPCSecurityManagerTest.kt b/node/src/test/kotlin/net/corda/node/services/RPCSecurityManagerTest.kt index be0d40ae8e..230170f0a7 100644 --- a/node/src/test/kotlin/net/corda/node/services/RPCSecurityManagerTest.kt +++ b/node/src/test/kotlin/net/corda/node/services/RPCSecurityManagerTest.kt @@ -7,6 +7,7 @@ import net.corda.node.internal.security.Password import net.corda.node.internal.security.RPCSecurityManagerImpl import net.corda.node.internal.security.tryAuthenticate import net.corda.nodeapi.internal.config.User +import net.corda.node.services.config.SecurityConfiguration import org.assertj.core.api.Assertions.assertThatThrownBy import org.junit.Test import javax.security.auth.login.FailedLoginException @@ -26,7 +27,7 @@ class RPCSecurityManagerTest { @Test fun `Generic RPC call authorization`() { - checkUserPermissions( + checkUserActions( permitted = setOf(arrayListOf("nodeInfo"), arrayListOf("notaryIdentities")), permissions = setOf( Permissions.invokeRpc(CordaRPCOps::nodeInfo), @@ -35,7 +36,7 @@ class RPCSecurityManagerTest { @Test fun `Flow invocation authorization`() { - checkUserPermissions( + checkUserActions( permissions = setOf(Permissions.startFlow()), permitted = setOf( arrayListOf("startTrackedFlowDynamic", "net.corda.node.services.RPCSecurityManagerTest\$DummyFlow"), @@ -44,21 +45,21 @@ class RPCSecurityManagerTest { @Test fun `Check startFlow RPC permission implies startFlowDynamic`() { - checkUserPermissions( + checkUserActions( permissions = setOf(Permissions.invokeRpc("startFlow")), permitted = setOf(arrayListOf("startFlow"), arrayListOf("startFlowDynamic"))) } @Test fun `Check startTrackedFlow RPC permission implies startTrackedFlowDynamic`() { - checkUserPermissions( + checkUserActions( permitted = setOf(arrayListOf("startTrackedFlow"), arrayListOf("startTrackedFlowDynamic")), permissions = setOf(Permissions.invokeRpc("startTrackedFlow"))) } @Test fun `Admin authorization`() { - checkUserPermissions( + checkUserActions( permissions = setOf("all"), permitted = allActions.map { arrayListOf(it) }.toSet()) } @@ -118,9 +119,9 @@ class RPCSecurityManagerTest { users = listOf(User(username, "password", setOf())), id = AuthServiceId("TEST")) } - private fun checkUserPermissions(permissions: Set, permitted: Set>) { + private fun checkUserActions(permissions: Set, permitted: Set>) { val user = User(username = "user", password = "password", permissions = permissions) - val userRealms = RPCSecurityManagerImpl.fromUserList(users = listOf(user), id = AuthServiceId("TEST")) + val userRealms = RPCSecurityManagerImpl(SecurityConfiguration.AuthService.fromUsers(listOf(user))) val disabled = allActions.filter { !permitted.contains(listOf(it)) } for (subject in listOf( userRealms.authenticate("user", Password("password")), From 694721e8ba693d83419feaad2582d3cf6b17d517 Mon Sep 17 00:00:00 2001 From: Katelyn Baker Date: Fri, 5 Jan 2018 14:07:55 +0000 Subject: [PATCH 2/2] TEMP removal of SSH tests till they're made determinsitic (#2325) --- .../integration-test/kotlin/net/corda/node/SSHServerTest.kt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/node/src/integration-test/kotlin/net/corda/node/SSHServerTest.kt b/node/src/integration-test/kotlin/net/corda/node/SSHServerTest.kt index 4f597a016c..78e80c4913 100644 --- a/node/src/integration-test/kotlin/net/corda/node/SSHServerTest.kt +++ b/node/src/integration-test/kotlin/net/corda/node/SSHServerTest.kt @@ -19,10 +19,12 @@ import java.net.ConnectException import kotlin.test.assertTrue import kotlin.test.fail import org.assertj.core.api.Assertions.assertThat +import org.junit.Ignore import java.util.regex.Pattern class SSHServerTest { + @Ignore("Test has undeterministic capacity to hang, ignore till fixed") @Test() fun `ssh server does not start be default`() { val user = User("u", "p", setOf()) @@ -44,6 +46,7 @@ class SSHServerTest { } } + @Ignore("Test has undeterministic capacity to hang, ignore till fixed") @Test fun `ssh server starts when configured`() { val user = User("u", "p", setOf()) @@ -64,6 +67,7 @@ class SSHServerTest { } + @Ignore("Test has undeterministic capacity to hang, ignore till fixed") @Test fun `ssh server verify credentials`() { val user = User("u", "p", setOf()) @@ -87,6 +91,7 @@ class SSHServerTest { } } + @Ignore("Test has undeterministic capacity to hang, ignore till fixed") @Test fun `ssh respects permissions`() { val user = User("u", "p", setOf(startFlow())) @@ -117,6 +122,7 @@ class SSHServerTest { } } + @Ignore("Test has undeterministic capacity to hang, ignore till fixed") @Test fun `ssh runs flows`() { val user = User("u", "p", setOf(startFlow()))