This is an automated email from the git hooks/post-receive script.

mreynolds pushed a commit to branch master in repository 389-ds-base.

commit a95889def41d3869692d7259a9213b1f9238f3c8 Author: Mark Reynolds mreynolds@redhat.com Date: Mon Feb 27 07:59:30 2017 -0500

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!) --- dirsrvtests/tests/tickets/ticket49122_test.py | 73 ++++++++ ldap/servers/plugins/roles/roles_cache.c | 233 +++++++++++++++----------- 2 files changed, 204 insertions(+), 102 deletions(-)

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 b7303de..4f27c4c 100644 --- a/ldap/servers/plugins/roles/roles_cache.c +++ b/ldap/servers/plugins/roles/roles_cache.c @@ -1072,6 +1072,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 ------------------------------------ @@ -1087,17 +1107,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_err(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_err(SLAPI_LOG_ERR, ROLES_PLUGIN_SUBSYSTEM, @@ -1108,85 +1128,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_err(SLAPI_LOG_ERR, 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_err(SLAPI_LOG_ERR, 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); - } - } + + 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_err(SLAPI_LOG_ERR, 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_err(SLAPI_LOG_ERR, 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; @@ -1200,6 +1218,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(); @@ -1208,33 +1227,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_err(SLAPI_LOG_ERR, ROLES_PLUGIN_SUBSYSTEM, - "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); + "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; @@ -1245,16 +1264,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_err(SLAPI_LOG_ERR, 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; } @@ -1273,50 +1303,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_err(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_err(SLAPI_LOG_ERR, ROLES_PLUGIN_SUBSYSTEM, - "roles_cache_create_object_from_entry - wrong role type\n"); + "roles_cache_create_object_from_entry - wrong role type\n"); }

- if ( rc == 0 ) + if ( rc == 0 ) { - *result = this_role; - } + *result = this_role; + }

slapi_log_err(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: