Blob Blame Raw
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