This is an automated email from the git hooks/post-receive script.
tbordaz pushed a commit to branch master
in repository 389-ds-base.
commit 74045251c748d3a6d15cb37e920d6ba6ddbfc922
Author: Thierry Bordaz <tbordaz(a)redhat.com>
Date: Thu Apr 13 15:21:49 2017 +0200
Ticket 49184 - Overflow in memberof
Bug Description:
The function memberof_call_foreach_dn can be used to retrieve ancestors of a
given entry. (ancestors are groups owning directly or indirectly a given entry).
With the use of group cache in memberof, at the entrance of
memberof_call_foreach_dn
there is an attempt to get the entry ancestors from the cache.
Before doing so it needs to test if the cache is safe. In fact in case of
circular groups the use of the cache is disabled and lookup in the cache should
not
happend.
To know if the cache is safe it needs to access a flag (use_cache) in
callback_data.
The callback_data structure is opaque at this level. So accessing it
while its structure is unknown is dangerous.
The bug is that we may read an 'int' at an offset that overflow the actual
structure.
This is just a test and should not trigger a crash.
Fix Description:
Add a flag to call memberof_call_foreach_dn so that, that indicates if
it is valid to use the group cache.
https://pagure.io/389-ds-base/issue/49184
Reviewed by: William Brown and Mark Reynolds (thanks to you !!)
Platforms tested: F23
Flag Day: no
Doc impact: no
---
dirsrvtests/tests/tickets/ticket49184_test.py | 146 ++++++++++++++++++++++++++
ldap/servers/plugins/memberof/memberof.c | 36 ++++---
2 files changed, 166 insertions(+), 16 deletions(-)
diff --git a/dirsrvtests/tests/tickets/ticket49184_test.py
b/dirsrvtests/tests/tickets/ticket49184_test.py
new file mode 100644
index 0000000..20edfde
--- /dev/null
+++ b/dirsrvtests/tests/tickets/ticket49184_test.py
@@ -0,0 +1,146 @@
+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)
+GROUP_DN_1 = ("cn=group1," + DEFAULT_SUFFIX)
+GROUP_DN_2 = ("cn=group2," + DEFAULT_SUFFIX)
+SUPER_GRP1 = ("cn=super_grp1," + DEFAULT_SUFFIX)
+SUPER_GRP2 = ("cn=super_grp2," + DEFAULT_SUFFIX)
+SUPER_GRP3 = ("cn=super_grp3," + DEFAULT_SUFFIX)
+
+if DEBUGGING:
+ logging.getLogger(__name__).setLevel(logging.DEBUG)
+else:
+ logging.getLogger(__name__).setLevel(logging.INFO)
+log = logging.getLogger(__name__)
+
+def _add_group_with_members(topo, group_dn):
+ # Create group
+ try:
+ topo.standalone.add_s(Entry((group_dn,
+ {'objectclass': 'top groupofnames
extensibleObject'.split(),
+ 'cn': 'group'})))
+ except ldap.LDAPError as e:
+ log.fatal('Failed to add group: error ' + e.message['desc'])
+ assert False
+
+ # Add members to the group - set timeout
+ log.info('Adding members to the group...')
+ for idx in range(1, 5):
+ try:
+ MEMBER_VAL = ("uid=member%d,%s" % (idx, DEFAULT_SUFFIX))
+ topo.standalone.modify_s(group_dn,
+ [(ldap.MOD_ADD,
+ 'member',
+ MEMBER_VAL)])
+ except ldap.LDAPError as e:
+ log.fatal('Failed to update group: member (%s) - error: %s' %
+ (MEMBER_VAL, e.message['desc']))
+ assert False
+
+def _check_memberof(topo, member=None, memberof=True, group_dn=None):
+ # Check that members have memberof attribute on M1
+ for idx in range(1, 5):
+ try:
+ USER_DN = ("uid=member%d,%s" % (idx, DEFAULT_SUFFIX))
+ ent = topo.standalone.getEntry(USER_DN, ldap.SCOPE_BASE,
"(objectclass=*)")
+ if presence_flag:
+ assert ent.hasAttr('memberof') and
ent.getValue('memberof') == group_dn
+ else:
+ assert not ent.hasAttr('memberof')
+ except ldap.LDAPError as e:
+ log.fatal('Failed to retrieve user (%s): error %s' % (USER_DN,
e.message['desc']))
+ assert False
+
+def _check_memberof(topo, member=None, memberof=True, group_dn=None):
+ ent = topo.standalone.getEntry(member, ldap.SCOPE_BASE, "(objectclass=*)")
+ if memberof:
+ assert group_dn
+ assert ent.hasAttr('memberof') and group_dn in
ent.getValues('memberof')
+ else:
+ if ent.hasAttr('memberof'):
+ assert group_dn not in ent.getValues('memberof')
+
+
+def test_ticket49184(topo):
+ """Write your testcase here...
+
+ Also, if you need any testcase initialization,
+ please, write additional fixture for that(include finalizer).
+ """
+
+ topo.standalone.plugins.enable(name=PLUGIN_MEMBER_OF)
+ topo.standalone.restart(timeout=10)
+
+ #
+ # create some users and a group
+ #
+ log.info('create users and group...')
+ for idx in range(1, 5):
+ try:
+ USER_DN = ("uid=member%d,%s" % (idx, DEFAULT_SUFFIX))
+ topo.standalone.add_s(Entry((USER_DN,
+ {'objectclass': 'top
extensibleObject'.split(),
+ 'uid': 'member%d' % (idx)})))
+ except ldap.LDAPError as e:
+ log.fatal('Failed to add user (%s): error %s' % (USER_DN,
e.message['desc']))
+ assert False
+
+ # add all users in GROUP_DN_1 and checks each users is memberof GROUP_DN_1
+ _add_group_with_members(topo, GROUP_DN_1)
+ for idx in range(1, 5):
+ USER_DN = ("uid=member%d,%s" % (idx, DEFAULT_SUFFIX))
+ _check_memberof(topo, member=USER_DN, memberof=True, group_dn=GROUP_DN_1 )
+
+ # add all users in GROUP_DN_2 and checks each users is memberof GROUP_DN_2
+ _add_group_with_members(topo, GROUP_DN_2)
+ for idx in range(1, 5):
+ USER_DN = ("uid=member%d,%s" % (idx, DEFAULT_SUFFIX))
+ _check_memberof(topo, member=USER_DN, memberof=True, group_dn=GROUP_DN_2 )
+
+ # add the level 2, 3 and 4 group
+ for super_grp in (SUPER_GRP1, SUPER_GRP2, SUPER_GRP3):
+ topo.standalone.add_s(Entry((super_grp,
+ {'objectclass': 'top groupofnames
extensibleObject'.split(),
+ 'cn': 'super_grp'})))
+ topo.standalone.modify_s(SUPER_GRP1,
+ [(ldap.MOD_ADD,
+ 'member',
+ GROUP_DN_1),
+ (ldap.MOD_ADD,
+ 'member',
+ GROUP_DN_2)])
+ topo.standalone.modify_s(SUPER_GRP2,
+ [(ldap.MOD_ADD,
+ 'member',
+ GROUP_DN_1),
+ (ldap.MOD_ADD,
+ 'member',
+ GROUP_DN_2)])
+ return
+ topo.standalone.delete_s(GROUP_DN_2)
+ for idx in range(1, 5):
+ USER_DN = ("uid=member%d,%s" % (idx, DEFAULT_SUFFIX))
+ _check_memberof(topo, member=USER_DN, memberof=True, group_dn=GROUP_DN_1 )
+ _check_memberof(topo, member=USER_DN, memberof=False, group_dn=GROUP_DN_2 )
+
+ if DEBUGGING:
+ # Add debugging steps(if any)...
+ pass
+
+
+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/memberof/memberof.c
b/ldap/servers/plugins/memberof/memberof.c
index 899358d..8d36b80 100644
--- a/ldap/servers/plugins/memberof/memberof.c
+++ b/ldap/servers/plugins/memberof/memberof.c
@@ -159,7 +159,7 @@ static int memberof_qsort_compare(const void *a, const void *b);
static void memberof_load_array(Slapi_Value **array, Slapi_Attr *attr);
static int memberof_del_dn_from_groups(Slapi_PBlock *pb, MemberOfConfig *config, Slapi_DN
*sdn);
static int memberof_call_foreach_dn(Slapi_PBlock *pb, Slapi_DN *sdn, MemberOfConfig
*config,
- char **types, plugin_search_entry_callback callback, void *callback_data, int
*cached);
+ char **types, plugin_search_entry_callback callback, void *callback_data, int *cached,
PRBool use_grp_cache);
static int memberof_is_direct_member(MemberOfConfig *config, Slapi_Value *groupdn,
Slapi_Value *memberdn);
static int memberof_is_grouping_attr(char *type, MemberOfConfig *config);
@@ -659,7 +659,7 @@ memberof_del_dn_from_groups(Slapi_PBlock *pb, MemberOfConfig *config,
Slapi_DN *
slapi_log_err(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM,
"memberof_del_dn_from_groups: Ancestors of %s\n", slapi_sdn_get_dn(sdn));
rc = memberof_call_foreach_dn(pb, sdn, config, groupattrs,
- memberof_del_dn_type_callback, &data, &cached);
+ memberof_del_dn_type_callback, &data, &cached,
PR_FALSE);
}
return rc;
@@ -777,7 +777,7 @@ add_ancestors_cbdata(memberof_cached_value *ancestors, void
*callback_data)
*/
int
memberof_call_foreach_dn(Slapi_PBlock *pb __attribute__((unused)), Slapi_DN *sdn,
- MemberOfConfig *config, char **types, plugin_search_entry_callback callback, void
*callback_data, int *cached)
+ MemberOfConfig *config, char **types, plugin_search_entry_callback callback, void
*callback_data, int *cached, PRBool use_grp_cache)
{
Slapi_PBlock *search_pb = NULL;
Slapi_DN *base_sdn = NULL;
@@ -792,9 +792,6 @@ memberof_call_foreach_dn(Slapi_PBlock *pb __attribute__((unused)),
Slapi_DN *sdn
int free_it = 0;
int rc = 0;
int i = 0;
- memberof_cached_value *ht_grp = NULL;
- memberof_get_groups_data *data = (memberof_get_groups_data*) callback_data;
- const char *ndn = slapi_sdn_get_ndn(sdn);
*cached = 0;
@@ -802,17 +799,24 @@ memberof_call_foreach_dn(Slapi_PBlock *pb __attribute__((unused)),
Slapi_DN *sdn
return (rc);
}
- /* Here we will retrieve the ancestor of sdn.
- * The key access is the normalized sdn
- * This is done through recursive internal searches of parents
- * If the ancestors of sdn are already cached, just use
- * this value
+ /* This flags indicates memberof_call_foreach_dn is called to retrieve ancestors
(groups).
+ * To improve performance, it can use a cache. (it will not in case of circular groups)
+ * When this flag is true it means no circular group are detected (so far) so we can use
the cache
*/
- if (data && data->use_cache) {
+ if (use_grp_cache) {
+ /* Here we will retrieve the ancestor of sdn.
+ * The key access is the normalized sdn
+ * This is done through recursive internal searches of parents
+ * If the ancestors of sdn are already cached, just use
+ * this value
+ */
+ memberof_cached_value *ht_grp = NULL;
+ const char *ndn = slapi_sdn_get_ndn(sdn);
+
ht_grp = ancestors_cache_lookup((const void *) ndn);
if (ht_grp) {
#if MEMBEROF_CACHE_DEBUG
- slapi_log_err(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM,
"memberof_call_foreach_dn: Ancestors of %s already cached (%x)\n", ndn,
ht_grp);
+ slapi_log_err(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM,
"memberof_call_foreach_dn: Ancestors of %s already cached (%x)\n", ndn,
ht_grp);
#endif
add_ancestors_cbdata(ht_grp, callback_data);
*cached = 1;
@@ -1106,7 +1110,7 @@ memberof_replace_dn_from_groups(Slapi_PBlock *pb, MemberOfConfig
*config,
slapi_log_err(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM,
"memberof_replace_dn_from_groups: Ancestors of %s\n",
slapi_sdn_get_dn(post_sdn));
if((ret = memberof_call_foreach_dn(pb, pre_sdn, config, groupattrs,
memberof_replace_dn_type_callback,
- &data, &cached)))
+ &data, &cached, PR_FALSE)))
{
break;
}
@@ -2383,7 +2387,7 @@ memberof_get_groups_r(MemberOfConfig *config, Slapi_DN *member_sdn,
slapi_log_err(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_get_groups_r:
Ancestors of %s\n", slapi_sdn_get_dn(member_sdn));
#endif
rc = memberof_call_foreach_dn(NULL, member_sdn, config, config->groupattrs,
- memberof_get_groups_callback, &member_data, &cached);
+ memberof_get_groups_callback, &member_data, &cached, member_data.use_cache);
merge_ancestors(&member_ndn_val, &member_data, data);
if (!cached && member_data.use_cache)
@@ -2578,7 +2582,7 @@ memberof_test_membership(Slapi_PBlock *pb, MemberOfConfig *config,
int cached = 0;
return memberof_call_foreach_dn(pb, group_sdn, config, attrs,
- memberof_test_membership_callback, config, &cached);
+ memberof_test_membership_callback, config, &cached, PR_FALSE);
}
/*
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.