From a8b10d7a4f1cad499fa1ba245dd73ca7beac4589 Mon Sep 17 00:00:00 2001
From: Mark Reynolds <mreynolds@redhat.com>
Date: Mon, 27 Feb 2017 07:59:30 -0500
Subject: [PATCH 71/71] Issue 49122 - Filtered nsrole that uses nsrole crashes
the server
Bug Description: When evaluating a filter role that uses "nsrole" in the filter
crashes the server due infinite loop that leads to a stack
overflow.
Fix Description: Virtual attributes are not allowed to be used in role filters.
We were already checking for COS attributes, but not nsrole.
Also did some minor code cleanup
https://pagure.io/389-ds-base/issue/49122
Reviewed by: nhosoi(Thanks!)
(cherry picked from commit a95889def41d3869692d7259a9213b1f9238f3c8)
(cherry picked from commit d589950cdd8ac9a0756b67cfe4ae3a33da094065)
---
dirsrvtests/tests/tickets/ticket49122_test.py | 73 ++++++++
ldap/servers/plugins/roles/roles_cache.c | 235 +++++++++++++++-----------
2 files changed, 205 insertions(+), 103 deletions(-)
create mode 100644 dirsrvtests/tests/tickets/ticket49122_test.py
diff --git a/dirsrvtests/tests/tickets/ticket49122_test.py b/dirsrvtests/tests/tickets/ticket49122_test.py
new file mode 100644
index 0000000..bd553f2
--- /dev/null
+++ b/dirsrvtests/tests/tickets/ticket49122_test.py
@@ -0,0 +1,73 @@
+import time
+import ldap
+import logging
+import pytest
+from lib389 import DirSrv, Entry, tools, tasks
+from lib389.tools import DirSrvTools
+from lib389._constants import *
+from lib389.properties import *
+from lib389.tasks import *
+from lib389.utils import *
+from lib389.topologies import topology_st as topo
+
+DEBUGGING = os.getenv("DEBUGGING", default=False)
+if DEBUGGING:
+ logging.getLogger(__name__).setLevel(logging.DEBUG)
+else:
+ logging.getLogger(__name__).setLevel(logging.INFO)
+log = logging.getLogger(__name__)
+
+USER_DN = 'uid=user,' + DEFAULT_SUFFIX
+ROLE_DN = 'cn=Filtered_Role_That_Includes_Empty_Role,' + DEFAULT_SUFFIX
+
+
+def test_ticket49122(topo):
+ """Search for non-existant role and make sure the server does not crash
+ """
+
+ # Enable roles plugin
+ topo.standalone.plugins.enable(name=PLUGIN_ROLES)
+ topo.standalone.restart()
+
+ # Add invalid role
+ try:
+ topo.standalone.add_s(Entry((
+ ROLE_DN, {'objectclass': ['top', 'ldapsubentry', 'nsroledefinition',
+ 'nscomplexroledefinition', 'nsfilteredroledefinition'],
+ 'cn': 'Filtered_Role_That_Includes_Empty_Role',
+ 'nsRoleFilter': '(!(nsrole=cn=This_Is_An_Empty_Managed_NsRoleDefinition,dc=example,dc=com))',
+ 'description': 'A filtered role with filter that will crash the server'})))
+ except ldap.LDAPError as e:
+ topo.standalone.log.fatal('Failed to add filtered role: error ' + e.message['desc'])
+ assert False
+
+ # Add test user
+ try:
+ topo.standalone.add_s(Entry((
+ USER_DN, {'objectclass': "top extensibleObject".split(),
+ 'uid': 'user'})))
+ except ldap.LDAPError as e:
+ topo.standalone.log.fatal('Failed to add test user: error ' + str(e))
+ assert False
+
+ if DEBUGGING:
+ # Add debugging steps(if any)...
+ print "Attach gdb"
+ time.sleep(20)
+
+ # Search for the role
+ try:
+ topo.standalone.search_s(USER_DN, ldap.SCOPE_SUBTREE, 'objectclass=*', ['nsrole'])
+ except ldap.LDAPError as e:
+ topo.standalone.log.fatal('Search failed: error ' + str(e))
+ assert False
+
+ topo.standalone.log.info('Test Passed')
+
+
+if __name__ == '__main__':
+ # Run isolated
+ # -s for DEBUG mode
+ CURRENT_FILE = os.path.realpath(__file__)
+ pytest.main("-s %s" % CURRENT_FILE)
+
diff --git a/ldap/servers/plugins/roles/roles_cache.c b/ldap/servers/plugins/roles/roles_cache.c
index 147d113..66c8553 100644
--- a/ldap/servers/plugins/roles/roles_cache.c
+++ b/ldap/servers/plugins/roles/roles_cache.c
@@ -1073,6 +1073,26 @@ static int roles_cache_create_role_under(roles_cache_def** roles_cache_suffix, S
return(rc);
}
+/*
+ * Check that we are not using nsrole in the filter
+ */
+static int roles_check_filter(Slapi_Filter *filter_list)
+{
+ Slapi_Filter *f;
+ char *type = NULL;
+
+ for ( f = slapi_filter_list_first( filter_list );
+ f != NULL;
+ f = slapi_filter_list_next( filter_list, f ) )
+ {
+ slapi_filter_get_attribute_type(f, &type);
+ if (strcasecmp(type, NSROLEATTR) == 0){
+ return -1;
+ }
+ }
+
+ return 0;
+}
/* roles_cache_create_object_from_entry
------------------------------------
@@ -1088,17 +1108,17 @@ static int roles_cache_create_object_from_entry(Slapi_Entry *role_entry, role_ob
int rc = 0;
int type = 0;
role_object *this_role = NULL;
- char *rolescopeDN = NULL;
+ char *rolescopeDN = NULL;
slapi_log_error(SLAPI_LOG_PLUGIN, ROLES_PLUGIN_SUBSYSTEM,
"--> roles_cache_create_object_from_entry\n");
- *result = NULL;
+ *result = NULL;
- /* Do not allow circular dependencies */
- if ( hint > MAX_NESTED_ROLES )
+ /* Do not allow circular dependencies */
+ if ( hint > MAX_NESTED_ROLES )
{
- char *ndn = NULL;
+ char *ndn = NULL;
ndn = slapi_entry_get_ndn( role_entry );
slapi_log_error(
@@ -1111,85 +1131,83 @@ static int roles_cache_create_object_from_entry(Slapi_Entry *role_entry, role_ob
return (0);
}
- /* Create the role cache definition */
- this_role = (role_object*)slapi_ch_calloc(1, sizeof(role_object));
- if (this_role == NULL )
+ /* Create the role cache definition */
+ this_role = (role_object*)slapi_ch_calloc(1, sizeof(role_object));
+ if (this_role == NULL )
{
- return ENOMEM;
- }
+ return ENOMEM;
+ }
- /* Check the entry is OK */
- /* Determine role type and assign to structure */
- /* We determine the role type by reading the objectclass */
+ /* Check the entry is OK */
+ /* Determine role type and assign to structure */
+ /* We determine the role type by reading the objectclass */
if ( roles_cache_is_role_entry(role_entry) == 0 )
{
- /* Bad type */
- slapi_ch_free((void**)&this_role);
- return SLAPI_ROLE_DEFINITION_ERROR;
- }
+ /* Bad type */
+ slapi_ch_free((void**)&this_role);
+ return SLAPI_ROLE_DEFINITION_ERROR;
+ }
- type = roles_cache_determine_class(role_entry);
+ type = roles_cache_determine_class(role_entry);
- if (type != 0)
+ if (type != 0)
{
- this_role->type = type;
- }
+ this_role->type = type;
+ }
else
{
- /* Bad type */
- slapi_ch_free((void**)&this_role);
- return SLAPI_ROLE_DEFINITION_ERROR;
- }
+ /* Bad type */
+ slapi_ch_free((void**)&this_role);
+ return SLAPI_ROLE_DEFINITION_ERROR;
+ }
this_role->dn = slapi_sdn_new();
slapi_sdn_copy(slapi_entry_get_sdn(role_entry),this_role->dn);
-
- rolescopeDN = slapi_entry_attr_get_charptr(role_entry, ROLE_SCOPE_DN);
- if (rolescopeDN) {
- Slapi_DN *rolescopeSDN;
- Slapi_DN *top_rolescopeSDN, *top_this_roleSDN;
-
- /* Before accepting to use this scope, first check if it belongs to the same suffix */
- rolescopeSDN = slapi_sdn_new_dn_byref(rolescopeDN);
- if ((strlen((char *) slapi_sdn_get_ndn(rolescopeSDN)) > 0) &&
- (slapi_dn_syntax_check(NULL, (char *) slapi_sdn_get_ndn(rolescopeSDN), 1) == 0)) {
- top_rolescopeSDN = roles_cache_get_top_suffix(rolescopeSDN);
- top_this_roleSDN = roles_cache_get_top_suffix(this_role->dn);
- if (slapi_sdn_compare(top_rolescopeSDN, top_this_roleSDN) == 0) {
- /* rolescopeDN belongs to the same suffix as the role, we can use this scope */
- this_role->rolescopedn = rolescopeSDN;
- } else {
- slapi_log_error(SLAPI_LOG_FATAL, ROLES_PLUGIN_SUBSYSTEM,
- "%s: invalid %s - %s not in the same suffix. Scope skipped.\n",
- (char*) slapi_sdn_get_dn(this_role->dn),
- ROLE_SCOPE_DN,
- rolescopeDN);
- slapi_sdn_free(&rolescopeSDN);
- }
- slapi_sdn_free(&top_rolescopeSDN);
- slapi_sdn_free(&top_this_roleSDN);
- } else {
- /* this is an invalid DN, just ignore this parameter*/
- slapi_log_error(SLAPI_LOG_FATAL, ROLES_PLUGIN_SUBSYSTEM,
- "%s: invalid %s - %s not a valid DN. Scope skipped.\n",
- (char*) slapi_sdn_get_dn(this_role->dn),
- ROLE_SCOPE_DN,
- rolescopeDN);
- slapi_sdn_free(&rolescopeSDN);
- }
- }
+
+ rolescopeDN = slapi_entry_attr_get_charptr(role_entry, ROLE_SCOPE_DN);
+ if (rolescopeDN) {
+ Slapi_DN *rolescopeSDN;
+ Slapi_DN *top_rolescopeSDN, *top_this_roleSDN;
+
+ /* Before accepting to use this scope, first check if it belongs to the same suffix */
+ rolescopeSDN = slapi_sdn_new_dn_byref(rolescopeDN);
+ if ((strlen((char *) slapi_sdn_get_ndn(rolescopeSDN)) > 0) &&
+ (slapi_dn_syntax_check(NULL, (char *) slapi_sdn_get_ndn(rolescopeSDN), 1) == 0)) {
+ top_rolescopeSDN = roles_cache_get_top_suffix(rolescopeSDN);
+ top_this_roleSDN = roles_cache_get_top_suffix(this_role->dn);
+ if (slapi_sdn_compare(top_rolescopeSDN, top_this_roleSDN) == 0) {
+ /* rolescopeDN belongs to the same suffix as the role, we can use this scope */
+ this_role->rolescopedn = rolescopeSDN;
+ } else {
+ slapi_log_error(SLAPI_LOG_FATAL, ROLES_PLUGIN_SUBSYSTEM,
+ "roles_cache_create_object_from_entry - %s: invalid %s - %s not in the same suffix. Scope skipped.\n",
+ (char*) slapi_sdn_get_dn(this_role->dn),
+ ROLE_SCOPE_DN,
+ rolescopeDN);
+ slapi_sdn_free(&rolescopeSDN);
+ }
+ slapi_sdn_free(&top_rolescopeSDN);
+ slapi_sdn_free(&top_this_roleSDN);
+ } else {
+ /* this is an invalid DN, just ignore this parameter*/
+ slapi_log_error(SLAPI_LOG_FATAL, ROLES_PLUGIN_SUBSYSTEM,
+ "roles_cache_create_object_from_entry - %s: invalid %s - %s not a valid DN. Scope skipped.\n",
+ (char*) slapi_sdn_get_dn(this_role->dn),
+ ROLE_SCOPE_DN,
+ rolescopeDN);
+ slapi_sdn_free(&rolescopeSDN);
+ }
+ }
- /* Depending upon role type, pull out the remaining information we need */
+ /* Depending upon role type, pull out the remaining information we need */
switch (this_role->type)
{
case ROLE_TYPE_MANAGED:
-
/* Nothing further needed */
break;
case ROLE_TYPE_FILTERED:
{
-
Slapi_Filter *filter = NULL;
char *filter_attr_value = NULL;
Slapi_PBlock *pb = NULL;
@@ -1203,6 +1221,7 @@ static int roles_cache_create_object_from_entry(Slapi_Entry *role_entry, role_ob
slapi_ch_free((void**)&this_role);
return SLAPI_ROLE_ERROR_NO_FILTER_SPECIFIED;
}
+
/* search (&(objectclass=costemplate)(filter_attr_value))*/
/* if found, reject it (returning SLAPI_ROLE_ERROR_FILTER_BAD) */
pb = slapi_pblock_new();
@@ -1211,33 +1230,33 @@ static int roles_cache_create_object_from_entry(Slapi_Entry *role_entry, role_ob
Slapi_Entry **cosentries = NULL;
char *costmpl_filter = NULL;
if ((*filter_attr_value == '(') &&
- (*(filter_attr_value+strlen(filter_attr_value)-1) == ')')) {
+ (*(filter_attr_value+strlen(filter_attr_value)-1) == ')')) {
costmpl_filter =
- slapi_ch_smprintf("(&(objectclass=costemplate)%s)",
- filter_attr_value);
+ slapi_ch_smprintf("(&(objectclass=costemplate)%s)",
+ filter_attr_value);
} else {
costmpl_filter =
- slapi_ch_smprintf("(&(objectclass=costemplate)(%s))",
- filter_attr_value);
+ slapi_ch_smprintf("(&(objectclass=costemplate)(%s))",
+ filter_attr_value);
}
slapi_search_internal_set_pb(pb, parent, LDAP_SCOPE_SUBTREE,
- costmpl_filter, NULL, 0, NULL,
- NULL, roles_get_plugin_identity(),
- 0);
+ costmpl_filter, NULL, 0, NULL,
+ NULL, roles_get_plugin_identity(),
+ 0);
slapi_search_internal_pb(pb);
slapi_pblock_get(pb, SLAPI_PLUGIN_INTOP_SEARCH_ENTRIES,
- &cosentries);
+ &cosentries);
slapi_ch_free_string(&costmpl_filter);
slapi_ch_free_string(&parent);
if (cosentries && *cosentries) {
slapi_free_search_results_internal(pb);
slapi_pblock_destroy(pb);
slapi_log_error(SLAPI_LOG_FATAL, ROLES_PLUGIN_SUBSYSTEM,
- "%s: not allowed to refer virtual attribute "
- "in the value of %s %s. The %s is disabled.\n",
- (char*)slapi_sdn_get_ndn(this_role->dn),
- ROLE_FILTER_ATTR_NAME, filter_attr_value,
- ROLE_FILTER_ATTR_NAME);
+ "roles_cache_create_object_from_entry - %s: not allowed to refer virtual attribute "
+ "in the value of %s %s. The %s is disabled.\n",
+ (char*)slapi_sdn_get_ndn(this_role->dn),
+ ROLE_FILTER_ATTR_NAME, filter_attr_value,
+ ROLE_FILTER_ATTR_NAME);
slapi_ch_free_string(&filter_attr_value);
slapi_ch_free((void**)&this_role);
return SLAPI_ROLE_ERROR_FILTER_BAD;
@@ -1248,16 +1267,27 @@ static int roles_cache_create_object_from_entry(Slapi_Entry *role_entry, role_ob
/* Turn it into a slapi filter object */
filter = slapi_str2filter(filter_attr_value);
- slapi_ch_free_string(&filter_attr_value);
-
- if ( filter == NULL )
+ if ( filter == NULL )
{
/* An error has occured */
slapi_ch_free((void**)&this_role);
+ slapi_ch_free_string(&filter_attr_value);
+ return SLAPI_ROLE_ERROR_FILTER_BAD;
+ }
+ if (roles_check_filter(filter)) {
+ slapi_log_error(SLAPI_LOG_FATAL, ROLES_PLUGIN_SUBSYSTEM,
+ "roles_cache_create_object_from_entry - \"%s\": not allowed to use \"nsrole\" "
+ "in the role filter \"%s\". %s is disabled.\n",
+ (char*)slapi_sdn_get_ndn(this_role->dn),
+ filter_attr_value,
+ ROLE_FILTER_ATTR_NAME);
+ slapi_ch_free((void**)&this_role);
+ slapi_ch_free_string(&filter_attr_value);
return SLAPI_ROLE_ERROR_FILTER_BAD;
}
/* Store on the object */
this_role->filter = filter;
+ slapi_ch_free_string(&filter_attr_value);
break;
}
@@ -1276,50 +1306,49 @@ static int roles_cache_create_object_from_entry(Slapi_Entry *role_entry, role_ob
int i = 0;
char *string = NULL;
Slapi_DN nested_role_dn;
- role_object_nested *nested_role_object = NULL;
+ role_object_nested *nested_role_object = NULL;
- for ( i = 0; va[i] != NULL; i++ )
+ for ( i = 0; va[i] != NULL; i++ )
{
- string = (char*)slapi_value_get_string(va[i]);
+ string = (char*)slapi_value_get_string(va[i]);
- /* Make a DN from the string */
- slapi_sdn_init_dn_byref(&nested_role_dn,string);
+ /* Make a DN from the string */
+ slapi_sdn_init_dn_byref(&nested_role_dn,string);
slapi_log_error(SLAPI_LOG_PLUGIN,
ROLES_PLUGIN_SUBSYSTEM, "roles_cache_create_object_from_entry: dn %s, nested %s\n",
(char*)slapi_sdn_get_ndn(this_role->dn),string);
- /* Make a role object nested from the DN */
- rc = roles_cache_object_nested_from_dn(&nested_role_dn,&nested_role_object);
+ /* Make a role object nested from the DN */
+ rc = roles_cache_object_nested_from_dn(&nested_role_dn,&nested_role_object);
- /* Insert it into the nested list */
- if ( (rc == 0) && nested_role_object)
+ /* Insert it into the nested list */
+ if ( (rc == 0) && nested_role_object)
{
/* Add to the tree where avl_data is a role_object_nested struct */
- rc = roles_cache_insert_object_nested(&(this_role->avl_tree),nested_role_object);
- }
- slapi_sdn_done(&nested_role_dn);
- }
- }
-
+ rc = roles_cache_insert_object_nested(&(this_role->avl_tree),nested_role_object);
+ }
+ slapi_sdn_done(&nested_role_dn);
+ }
+ }
+
break;
}
default:
- slapi_log_error(SLAPI_LOG_FATAL,
- ROLES_PLUGIN_SUBSYSTEM, "wrong role type\n");
+ slapi_log_error(SLAPI_LOG_FATAL, ROLES_PLUGIN_SUBSYSTEM,
+ "roles_cache_create_object_from_entry - wrong role type\n");
}
- if ( rc == 0 )
+ if ( rc == 0 )
{
- *result = this_role;
- }
+ *result = this_role;
+ }
slapi_log_error(SLAPI_LOG_PLUGIN, ROLES_PLUGIN_SUBSYSTEM,
- "<-- roles_cache_create_object_from_entry\n");
-
+ "<-- roles_cache_create_object_from_entry\n");
- return rc;
+ return rc;
}
/* roles_cache_determine_class:
--
2.7.4