From 5720697b0df6f00ca8520847c7da81eaac3af698 Mon Sep 17 00:00:00 2001 From: igor nitto Date: Wed, 13 Dec 2017 17:09:09 +0000 Subject: [PATCH] [CORDA-827] Improved unit tests coverage and documentation (#2229) * Extend unit test on RPCSecurityManager * Fix corner cases in permission parsing and bug in tryAuthenticate * Rework docsite page * Add missing ChangeLog entry --- .../corda/client/rpc/RPCPermissionsTests.kt | 3 + docs/source/changelog.rst | 2 + docs/source/clientrpc.rst | 135 +++++++++++++++- docs/source/corda-configuration-file.rst | 3 + docs/source/corda-nodes-index.rst | 1 - docs/source/node-auth-config.rst | 136 ---------------- .../internal/security/RPCSecurityManager.kt | 2 +- .../security/RPCSecurityManagerImpl.kt | 27 ++-- .../node/services/config/NodeConfiguration.kt | 2 +- .../node/services/RPCSecurityManagerTest.kt | 153 +++++++++++++++++- 10 files changed, 309 insertions(+), 155 deletions(-) delete mode 100644 docs/source/node-auth-config.rst diff --git a/client/rpc/src/test/kotlin/net/corda/client/rpc/RPCPermissionsTests.kt b/client/rpc/src/test/kotlin/net/corda/client/rpc/RPCPermissionsTests.kt index 9d57aa401e..fae5fbc6d5 100644 --- a/client/rpc/src/test/kotlin/net/corda/client/rpc/RPCPermissionsTests.kt +++ b/client/rpc/src/test/kotlin/net/corda/client/rpc/RPCPermissionsTests.kt @@ -1,6 +1,9 @@ package net.corda.client.rpc +import net.corda.core.flows.FlowLogic +import net.corda.core.messaging.CordaRPCOps import net.corda.core.messaging.RPCOps +import net.corda.node.services.Permissions import net.corda.node.services.messaging.rpcContext import net.corda.nodeapi.internal.config.User import net.corda.testing.node.internal.RPCDriverDSL diff --git a/docs/source/changelog.rst b/docs/source/changelog.rst index 01270b15a3..13f5f3d440 100644 --- a/docs/source/changelog.rst +++ b/docs/source/changelog.rst @@ -6,6 +6,8 @@ from the previous milestone release. UNRELEASED ---------- +* Support for external user credentials data source and password encryption [CORDA-827]. + * Exporting additional JMX metrics (artemis, hibernate statistics) and loading Jolokia agent at JVM startup when using DriverDSL and/or cordformation node runner. diff --git a/docs/source/clientrpc.rst b/docs/source/clientrpc.rst index 14d29ba9b9..7f8322bbfb 100644 --- a/docs/source/clientrpc.rst +++ b/docs/source/clientrpc.rst @@ -26,7 +26,7 @@ permissions that RPC can use for fine-grain access control. These users are added to the node's ``node.conf`` file. -The syntax for adding an RPC user is: +The simplest way of adding an RPC user is to include it in the ``rpcUsers`` list: .. container:: codeset @@ -59,9 +59,6 @@ Users need permissions to invoke any RPC call. By default, nothing is allowed. T ... ] -.. note:: Currently, the node's web server has super-user access, meaning that it can run any RPC operation without - logging in. This will be changed in a future release. - Permissions Syntax ^^^^^^^^^^^^^^^^^^ @@ -71,6 +68,136 @@ Fine grained permissions allow a user to invoke a specific RPC operation, or to - to invoke a RPC operation: ``InvokeRpc.`` e.g., ``InvokeRpc.nodeInfo``. .. note:: Permission ``InvokeRpc.startFlow`` allows a user to initiate all flows. +RPC security management +----------------------- + +Hard coding user accounts in the ``rpcUsers`` field provides a quick way of allowing node's RPC to be accessed by a fixed +set of authenticated users but has some obvious shortcomings. To support use cases aiming for higher security and flexibility, +Corda RPC security system offers additional features such as: + + * Fetching users credentials and permissions from external data source (e.g.: a remote RDBMS), with optional caching + in node memory. In particular, this allows user credentials and permissions externally to be updated externally without + requiring node's restart. + * Password stored in hash-encrypted form. This is regarded as must-have when security is a concern. Corda currently supports + a flexible password hash format conforming to the Modular Crypt Format and defined by the `Apache Shiro framework `_ + +These features are controlled by a set of options nested in the ``security`` field of a node configuration. + +.. warning:: The ``rpcUsers`` field is now deprecated in favour of the set the ``security`` config structure. A node + configuration specifying both ``rpcUsers`` and ``security`` fields will trigger an exception during node startup. + +The following example configuration points the node to a remote RDBMS storing hash-encrypted passwords and enable caching +of user data in node's memory: + +.. container:: codeset + + .. sourcecode:: groovy + + security = { + authService = { + dataSource = { + type = "DB", + passwordEncryption = "SHIRO_1_CRYPT", + connection = { + jdbcUrl = "" + username = "" + password = "" + driverClassName = "" + } + } + options = { + cache = { + expireAfterSecs = 120 + maxEntries = 10000 + } + } + } + } + +Moreover, for practical reasons, we can still have an hard-coded static list of users embedded in the ``security`` +structure like in the old ``rpcUsers`` format, by specifying a ``dataSource`` of ``INMEMORY`` type: + +.. container:: codeset + + .. sourcecode:: groovy + + security = { + authService = { + dataSource = { + type = "INMEMORY", + users = [ + { + username = "", + password = "", + permissions = ["", "", ...] + }, + ... + ] + } + } + } + +Authentication/authorisation data +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +The ``dataSource`` field defines the data provider supplying credentials and permissions for users. It currently exists +in two forms, identified by the subfield ``type``: + + :INMEMORY: A list of user credentials and permissions hard-coded in configuration in the ``users`` field (see example above) + + :DB: An external RDBMS accessed via the JDBC connection described by ``connection``. The current implementation + expect the database to store data according to the following schema: + + - Table ``users`` containing columns ``username`` and ``password``. The ``username`` column *must have unique values*. + - Table ``user_roles`` containing columns ``username`` and ``role_name`` associating a user to a set of *roles* + - Table ``roles_permissions`` containing columns ``role_name`` and ``permission`` associating a role to a set of + permission strings + + Unlike the ``INMEMORY`` case, in the user database permissions are assigned to *roles* rather than individual users. + + .. note:: There is no prescription on the SQL type of the columns (although our tests were conducted on ``username`` and + ``role_name`` declared of SQL type ``VARCHAR`` and ``password`` of ``TEXT`` type). It is also possible to have extra columns + in each table alongside the expected ones. + +Password encryption +^^^^^^^^^^^^^^^^^^^ + +Storing passwords in plain text is discouraged in production environment where security is critical. Passwords are assumed +to be in plain format by default, unless a different format is specified ny the ``passwordEncryption`` field, like: + +.. container:: codeset + + .. sourcecode:: groovy + + passwordEncryption = SHIRO_1_CRYPT + +``SHIRO_1_CRYPT`` identifies the `Apache Shiro fully reversible +Modular Crypt Format `_, +currently the only non-plain password hash-encryption format supported by Corda. Passwords can be hash-encrypted in this +format using the `Apache Shiro Hasher command line tool `_. + +Caching users data +^^^^^^^^^^^^^^^^^^ + +A cache layer on top of the external data source of users credentials and permissions can significantly benefit +performances in some cases, with the disadvantage of causing a (controllable) delay in picking up updates to the underlying data. +Caching is disabled by default, it can be enabled by defining the ``options.cache`` field in ``security.authService``, +for example: + +.. container:: codeset + + .. sourcecode:: groovy + + options = { + cache = { + expireAfterSecs = 120 + maxEntries = 10000 + } + } + +This will enable a non-persistent cache contained in the node's memory with maximum number of entries set to ``maxEntries`` +with entries expiring and refreshed after ``expireAfterSecs`` number of seconds. + Observables ----------- The RPC system handles observables in a special way. When a method returns an observable, whether directly or diff --git a/docs/source/corda-configuration-file.rst b/docs/source/corda-configuration-file.rst index b8361203e4..566e3d5e64 100644 --- a/docs/source/corda-configuration-file.rst +++ b/docs/source/corda-configuration-file.rst @@ -90,6 +90,9 @@ path to the node's base directory. :rpcAddress: The address of the RPC system on which RPC requests can be made to the node. If not provided then the node will run without RPC. +:security: Contains various nested fields controlling user authentication/authorization, in particular for RPC accesses. See + :doc:`clientrpc` for details. + :webAddress: The host and port on which the webserver will listen if it is started. This is not used by the node itself. .. note:: If HTTPS is enabled then the browser security checks will require that the accessing url host name is one diff --git a/docs/source/corda-nodes-index.rst b/docs/source/corda-nodes-index.rst index 061902b1aa..c1dfa0b508 100644 --- a/docs/source/corda-nodes-index.rst +++ b/docs/source/corda-nodes-index.rst @@ -10,7 +10,6 @@ Corda nodes corda-configuration-file clientrpc shell - node-auth-config node-database node-administration out-of-process-verification \ No newline at end of file diff --git a/docs/source/node-auth-config.rst b/docs/source/node-auth-config.rst deleted file mode 100644 index 5f9c54c3e4..0000000000 --- a/docs/source/node-auth-config.rst +++ /dev/null @@ -1,136 +0,0 @@ -Access security settings -======================== - -Access to node functionalities via SSH or RPC is protected by an authentication and authorisation policy. - -The field ``security`` in ``node.conf`` exposes various sub-fields related to authentication/authorisation specifying: - - * The data source providing credentials and permissions for users (e.g.: a remote RDBMS) - * An optional password encryption method. - * An optional caching of users data from Node side. - -.. warning:: Specifying both ``rpcUsers`` and ``security`` fields in ``node.conf`` is considered an illegal setting and - rejected by the node at startup since ``rpcUsers`` is effectively deprecated in favour of ``security.authService``. - -**Example 1:** connect to remote RDBMS for credentials/permissions, with encrypted user passwords and -caching on node-side: - -.. container:: codeset - - .. sourcecode:: groovy - - security = { - authService = { - dataSource = { - type = "DB", - passwordEncryption = "SHIRO_1_CRYPT", - connection = { - jdbcUrl = "" - username = "" - password = "" - driverClassName = "" - } - } - options = { - cache = { - expiryTimeSecs = 120 - capacity = 10000 - } - } - } - } - -**Example 2:** list of user credentials and permissions hard-coded in ``node.conf`` - -.. container:: codeset - - .. sourcecode:: groovy - - security = { - authService = { - dataSource = { - type = "INMEMORY", - users =[ - { - username = "user1" - password = "password" - permissions = [ - "StartFlow.net.corda.flows.ExampleFlow1", - "StartFlow.net.corda.flows.ExampleFlow2", - ... - ] - }, - ... - ] - } - } - } - -Let us look in more details at the structure of ``security.authService``: - -Authentication/authorisation data ---------------------------------- - -The ``dataSource`` field defines the data provider supplying credentials and permissions for users. The ``type`` -subfield identify the type of data provider, currently supported one are: - - * **INMEMORY:** a list of user credentials and permissions hard-coded in configuration in the ``users`` field - (see example 2 above) - - * **DB:** An external RDBMS accessed via the JDBC connection described by ``connection``. The current implementation - expect the database to store data according to the following schema: - - - Table ``users`` containing columns ``username`` and ``password``. - The ``username`` column *must have unique values*. - - Table ``user_roles`` containing columns ``username`` and ``role_name`` associating a user to a set of *roles* - - Table ``roles_permissions`` containing columns ``role_name`` and ``permission`` associating a role to a set of - permission strings - - Note in particular how in the DB case permissions are assigned to _roles_ rather than individual users. - Also, there is no prescription on the SQL type of the columns (although in our tests we defined ``username`` and - ``role_name`` of SQL type ``VARCHAR`` and ``password`` of ``TEXT`` type) and it is allowed to put additional columns - besides the one expected by the implementation. - -Password encryption -------------------- - -Storing passwords in plain text is discouraged in production systems aiming for high security requirements. We support -reading passwords stored using the Apache Shiro fully reversible Modular Crypt Format, specified in the documentation -of ``org.apache.shiro.crypto.hash.format.Shiro1CryptFormat``. - -Password are assumed in plain format by default. To specify an encryption it is necessary to use the field: - -.. container:: codeset - - .. sourcecode:: groovy - - passwordEncryption = SHIRO_1_CRYPT - -Hash encrypted password based on the Shiro1CryptFormat can be produced with the `Apache Shiro Hasher tool `_ - -Cache ------ - -Adding a cache layer on top of an external provider of users credentials and permissions can significantly benefit -performances in some cases, with the disadvantage of introducing a latency in the propagation of changes to the data. - -Caching of users data is disabled by default, it can be enabled by defining the ``options.cache`` field, like seen in -the examples above: - -.. container:: codeset - - .. sourcecode:: groovy - - options = { - cache = { - expiryTimeSecs = 120 - capacity = 10000 - } - } - -This will enable an in-memory cache with maximum capacity (number of entries) and maximum life time of entries given by -respectively the values set by the ``capacity`` and ``expiryTimeSecs`` fields. - - - - diff --git a/node/src/main/kotlin/net/corda/node/internal/security/RPCSecurityManager.kt b/node/src/main/kotlin/net/corda/node/internal/security/RPCSecurityManager.kt index dafa069833..4307df44e0 100644 --- a/node/src/main/kotlin/net/corda/node/internal/security/RPCSecurityManager.kt +++ b/node/src/main/kotlin/net/corda/node/internal/security/RPCSecurityManager.kt @@ -34,7 +34,7 @@ fun RPCSecurityManager.tryAuthenticate(principal: String, password: Password): A password.use { return try { authenticate(principal, password) - } catch (e: AuthenticationException) { + } catch (e: FailedLoginException) { null } } diff --git a/node/src/main/kotlin/net/corda/node/internal/security/RPCSecurityManagerImpl.kt b/node/src/main/kotlin/net/corda/node/internal/security/RPCSecurityManagerImpl.kt index add690620e..e667d673b7 100644 --- a/node/src/main/kotlin/net/corda/node/internal/security/RPCSecurityManagerImpl.kt +++ b/node/src/main/kotlin/net/corda/node/internal/security/RPCSecurityManagerImpl.kt @@ -95,8 +95,8 @@ class RPCSecurityManagerImpl(config: AuthServiceConfig) : RPCSecurityManager { // Setup optional cache layer if configured it.cacheManager = config.options?.cache?.let { GuavaCacheManager( - timeToLiveSeconds = it.expiryTimeInSecs, - maxSize = it.capacity) + timeToLiveSeconds = it.expireAfterSecs, + maxSize = it.maxEntries) } } } @@ -149,22 +149,29 @@ private object RPCPermissionResolver : PermissionResolver { private val ACTION_START_FLOW = "startflow" private val ACTION_INVOKE_RPC = "invokerpc" private val ACTION_ALL = "all" - - private val FLOW_RPC_CALLS = setOf("startFlowDynamic", "startTrackedFlowDynamic") + private val FLOW_RPC_CALLS = setOf( + "startFlowDynamic", + "startTrackedFlowDynamic", + "startFlow", + "startTrackedFlow") override fun resolvePermission(representation: String): Permission { - - val action = representation.substringBefore(SEPARATOR).toLowerCase() + val action = representation.substringBefore(SEPARATOR).toLowerCase() when (action) { ACTION_INVOKE_RPC -> { - val rpcCall = representation.substringAfter(SEPARATOR) - require(representation.count { it == SEPARATOR } == 1) { + val rpcCall = representation.substringAfter(SEPARATOR, "") + require(representation.count { it == SEPARATOR } == 1 && !rpcCall.isEmpty()) { "Malformed permission string" } - return RPCPermission(setOf(rpcCall)) + val permitted = when(rpcCall) { + "startFlow" -> setOf("startFlowDynamic", rpcCall) + "startTrackedFlow" -> setOf("startTrackedFlowDynamic", rpcCall) + else -> setOf(rpcCall) + } + return RPCPermission(permitted) } ACTION_START_FLOW -> { - val targetFlow = representation.substringAfter(SEPARATOR) + val targetFlow = representation.substringAfter(SEPARATOR, "") require(targetFlow.isNotEmpty()) { "Missing target flow after StartFlow" } 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 7472c067bc..7d47ea60cb 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 @@ -196,7 +196,7 @@ data class SecurityConfiguration(val authService: SecurityConfiguration.AuthServ data class Options(val cache: Options.Cache?) { // Cache parameters - data class Cache(val expiryTimeInSecs: Long, val capacity: Long) + data class Cache(val expireAfterSecs: Long, val maxEntries: Long) } 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 2866d41dc4..be0d40ae8e 100644 --- a/node/src/test/kotlin/net/corda/node/services/RPCSecurityManagerTest.kt +++ b/node/src/test/kotlin/net/corda/node/services/RPCSecurityManagerTest.kt @@ -1,10 +1,19 @@ package net.corda.node.services import net.corda.core.context.AuthServiceId +import net.corda.core.flows.FlowLogic +import net.corda.core.messaging.CordaRPCOps +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 org.assertj.core.api.Assertions.assertThatThrownBy import org.junit.Test +import javax.security.auth.login.FailedLoginException +import kotlin.reflect.KFunction +import kotlin.test.assertFails +import kotlin.test.assertFailsWith +import kotlin.test.assertNull class RPCSecurityManagerTest { @@ -15,7 +24,147 @@ class RPCSecurityManagerTest { assertThatThrownBy { configWithRPCUsername("user#1") }.hasMessageContaining("#") } - private fun configWithRPCUsername(username: String) { - RPCSecurityManagerImpl.fromUserList(users = listOf(User(username, "password", setOf())), id = AuthServiceId("TEST")) + @Test + fun `Generic RPC call authorization`() { + checkUserPermissions( + permitted = setOf(arrayListOf("nodeInfo"), arrayListOf("notaryIdentities")), + permissions = setOf( + Permissions.invokeRpc(CordaRPCOps::nodeInfo), + Permissions.invokeRpc(CordaRPCOps::notaryIdentities))) } + + @Test + fun `Flow invocation authorization`() { + checkUserPermissions( + permissions = setOf(Permissions.startFlow()), + permitted = setOf( + arrayListOf("startTrackedFlowDynamic", "net.corda.node.services.RPCSecurityManagerTest\$DummyFlow"), + arrayListOf("startFlowDynamic", "net.corda.node.services.RPCSecurityManagerTest\$DummyFlow"))) + } + + @Test + fun `Check startFlow RPC permission implies startFlowDynamic`() { + checkUserPermissions( + permissions = setOf(Permissions.invokeRpc("startFlow")), + permitted = setOf(arrayListOf("startFlow"), arrayListOf("startFlowDynamic"))) + } + + @Test + fun `Check startTrackedFlow RPC permission implies startTrackedFlowDynamic`() { + checkUserPermissions( + permitted = setOf(arrayListOf("startTrackedFlow"), arrayListOf("startTrackedFlowDynamic")), + permissions = setOf(Permissions.invokeRpc("startTrackedFlow"))) + } + + @Test + fun `Admin authorization`() { + checkUserPermissions( + permissions = setOf("all"), + permitted = allActions.map { arrayListOf(it) }.toSet()) + } + + @Test + fun `Malformed permission strings`() { + assertMalformedPermission("bar") + assertMalformedPermission("InvokeRpc.nodeInfo.XXX") + assertMalformedPermission("") + assertMalformedPermission(".") + assertMalformedPermission("..") + assertMalformedPermission("startFlow") + assertMalformedPermission("startFlow.") + } + + @Test + fun `Login with unknown user`() { + val userRealm = RPCSecurityManagerImpl.fromUserList( + users = listOf(User("user", "xxxx", emptySet())), + id = AuthServiceId("TEST")) + userRealm.authenticate("user", Password("xxxx")) + assertFailsWith(FailedLoginException::class, "Login with wrong password should fail") { + userRealm.authenticate("foo", Password("xxxx")) + } + assertNull(userRealm.tryAuthenticate("foo", Password("wrong")), + "Login with wrong password should fail") + } + + @Test + fun `Login with wrong credentials`() { + val userRealm = RPCSecurityManagerImpl.fromUserList( + users = listOf(User("user", "password", emptySet())), + id = AuthServiceId("TEST")) + userRealm.authenticate("user", Password("password")) + assertFailsWith(FailedLoginException::class, "Login with wrong password should fail") { + userRealm.authenticate("user", Password("wrong")) + } + assertNull(userRealm.tryAuthenticate("user", Password("wrong")), + "Login with wrong password should fail") + } + + @Test + fun `Build invalid subject`() { + val userRealm = RPCSecurityManagerImpl.fromUserList( + users = listOf(User("user", "password", emptySet())), + id = AuthServiceId("TEST")) + val subject = userRealm.buildSubject("foo") + for (action in allActions) { + assert(!subject.isPermitted(action)) { + "Invalid subject should not be allowed to call $action" + } + } + } + + private fun configWithRPCUsername(username: String) { + RPCSecurityManagerImpl.fromUserList( + users = listOf(User(username, "password", setOf())), id = AuthServiceId("TEST")) + } + + private fun checkUserPermissions(permissions: Set, permitted: Set>) { + val user = User(username = "user", password = "password", permissions = permissions) + val userRealms = RPCSecurityManagerImpl.fromUserList(users = listOf(user), id = AuthServiceId("TEST")) + val disabled = allActions.filter { !permitted.contains(listOf(it)) } + for (subject in listOf( + userRealms.authenticate("user", Password("password")), + userRealms.tryAuthenticate("user", Password("password"))!!, + userRealms.buildSubject("user"))) { + for (request in permitted) { + val call = request.first() + val args = request.drop(1).toTypedArray() + assert(subject.isPermitted(request.first(), *args)) { + "User ${subject.principal} should be permitted ${call} with target '${request.toList()}'" + } + if (args.isEmpty()) { + assert(subject.isPermitted(request.first(), "XXX")) { + "User ${subject.principal} should be permitted ${call} with any target" + } + } + } + + disabled.forEach { + assert(!subject.isPermitted(it)) { + "Permissions $permissions should not allow to call $it" + } + } + + disabled.filter { !permitted.contains(listOf(it, "foo")) }.forEach { + assert(!subject.isPermitted(it, "foo")) { + "Permissions $permissions should not allow to call $it with argument 'foo'" + } + } + } + } + + private fun assertMalformedPermission(permission: String) { + assertFails { + RPCSecurityManagerImpl.fromUserList( + users = listOf(User("x", "x", setOf(permission))), + id = AuthServiceId("TEST")) + } + } + + companion object { + private val allActions = CordaRPCOps::class.members.filterIsInstance>().map { it.name }.toSet() + + setOf("startFlow", "startTrackedFlow") + } + + private abstract class DummyFlow : FlowLogic() } \ No newline at end of file