From 053fb02f73220be53d1fb93511684a6f7aa3226f Mon Sep 17 00:00:00 2001 From: Thierry Bordaz Date: Thu, 10 Nov 2022 16:54:40 +0100 Subject: [PATCH 3/5] Issue 5440 - memberof is slow on update/fixup if there are several 'groupattr' (#5455) Bug description: When there are several groupattr (attr_1, attr_2,..) in memberof config To fixup entry 'e1', memberof does an internal search "(|(attr_1=e1)(attr_2=e1)...(attr_n=e1))" This is not valid regarding membership relation and in addition it prevents the server to bypass the filter evaluation. Fix description: To fixup an entry iterate several internal searches "(attr_1=e1)" , then "(attr_2=e1)", then "(attr_n=e1)" relates: #5440 Reviewed by: Pierre Rogier, Mark Reynolds, Simon Pichugin (Thanks) --- ldap/servers/plugins/memberof/memberof.c | 155 +++++++++++------------ 1 file changed, 73 insertions(+), 82 deletions(-) diff --git a/ldap/servers/plugins/memberof/memberof.c b/ldap/servers/plugins/memberof/memberof.c index b54eb3977..541b27250 100644 --- a/ldap/servers/plugins/memberof/memberof.c +++ b/ldap/servers/plugins/memberof/memberof.c @@ -704,8 +704,6 @@ memberof_call_foreach_dn(Slapi_PBlock *pb __attribute__((unused)), Slapi_DN *sdn char *filter_str = NULL; char *cookie = NULL; int all_backends = config->allBackends; - int types_name_len = 0; - int num_types = 0; int dn_len = slapi_sdn_get_ndn_len(sdn); int free_it = 0; int rc = 0; @@ -744,107 +742,100 @@ memberof_call_foreach_dn(Slapi_PBlock *pb __attribute__((unused)), Slapi_DN *sdn #if MEMBEROF_CACHE_DEBUG slapi_log_err(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_call_foreach_dn: Ancestors of %s not cached\n", ndn); #endif - /* Count the number of types. */ - for (num_types = 0; types && types[num_types]; num_types++) { - /* Add up the total length of all attribute names. - * We need to know this for building the filter. */ - types_name_len += strlen(types[num_types]); - } /* Escape the dn, and build the search filter. */ escaped_filter_val = slapi_escape_filter_value((char *)slapi_sdn_get_dn(sdn), dn_len); if (escaped_filter_val) { - dn_len = strlen(escaped_filter_val); free_it = 1; } else { escaped_filter_val = (char *)slapi_sdn_get_dn(sdn); } - if (num_types > 1) { - int bytes_out = 0; - int filter_str_len = types_name_len + (num_types * (3 + dn_len)) + 4; - - /* Allocate enough space for the filter */ - filter_str = slapi_ch_malloc(filter_str_len); - - /* Add beginning of filter. */ - bytes_out = snprintf(filter_str, filter_str_len - bytes_out, "(|"); - - /* Add filter section for each type. */ - for (i = 0; types[i]; i++) { - bytes_out += snprintf(filter_str + bytes_out, filter_str_len - bytes_out, - "(%s=%s)", types[i], escaped_filter_val); - } - - /* Add end of filter. */ - snprintf(filter_str + bytes_out, filter_str_len - bytes_out, ")"); - } else if (num_types == 1) { - filter_str = slapi_ch_smprintf("(%s=%s)", types[0], escaped_filter_val); - } - - if (free_it) - slapi_ch_free_string(&escaped_filter_val); + for (i = 0; types[i]; i++) { + /* Triggers one internal search per membership attribute. + * Assuming the attribute is indexed (eq), the search will + * bypass the evaluation of the filter (nsslapd-search-bypass-filter-test) + * against the candidates. This is important to bypass the filter + * because on large valueset (static group) it is very expensive + */ + filter_str = slapi_ch_smprintf("(%s=%s)", types[i], escaped_filter_val); - if (filter_str == NULL) { - return rc; - } + be = slapi_get_first_backend(&cookie); + while (be) { + PRBool do_suffix_search = PR_TRUE; - search_pb = slapi_pblock_new(); - be = slapi_get_first_backend(&cookie); - while (be) { - Slapi_DN *scope_sdn = NULL; + if (!all_backends) { + be = slapi_be_select(sdn); + if (be == NULL) { + break; + } + } + if ((base_sdn = (Slapi_DN *) slapi_be_getsuffix(be, 0)) == NULL) { + if (!all_backends) { + break; + } else { + /* its ok, goto the next backend */ + be = slapi_get_next_backend(cookie); + continue; + } + } - if (!all_backends) { - be = slapi_be_select(sdn); - if (be == NULL) { - break; + search_pb = slapi_pblock_new(); + if (config->entryScopes || config->entryScopeExcludeSubtrees) { + if (memberof_entry_in_scope(config, base_sdn)) { + /* do nothing, entry scope is spanning + * multiple suffixes, start at suffix */ + } else if (config->entryScopes) { + for (size_t i = 0; config->entryScopes[i]; i++) { + if (slapi_sdn_issuffix(config->entryScopes[i], base_sdn)) { + /* Search each include scope */ + slapi_search_internal_set_pb(search_pb, slapi_sdn_get_dn(config->entryScopes[i]), + LDAP_SCOPE_SUBTREE, filter_str, 0, 0, 0, 0, + memberof_get_plugin_id(), 0); + slapi_search_internal_callback_pb(search_pb, callback_data, 0, callback, 0); + /* We already did the search for this backend, don't + * do it again when we fall through */ + do_suffix_search = PR_FALSE; + } + } + } else if (!all_backends) { + slapi_pblock_destroy(search_pb); + break; + } else { + /* its ok, goto the next backend */ + be = slapi_get_next_backend(cookie); + slapi_pblock_destroy(search_pb); + continue; + } } - } - if ((base_sdn = (Slapi_DN *)slapi_be_getsuffix(be, 0)) == NULL) { - if (!all_backends) { - break; - } else { - /* its ok, goto the next backend */ - be = slapi_get_next_backend(cookie); - continue; + + if (do_suffix_search) { + slapi_search_internal_set_pb(search_pb, slapi_sdn_get_dn(base_sdn), + LDAP_SCOPE_SUBTREE, filter_str, 0, 0, 0, 0, + memberof_get_plugin_id(), 0); + slapi_search_internal_callback_pb(search_pb, callback_data, 0, callback, 0); + slapi_pblock_get(search_pb, SLAPI_PLUGIN_INTOP_RESULT, &rc); + if (rc != LDAP_SUCCESS) { + slapi_pblock_destroy(search_pb); + break; + } } - } - if (config->entryScopes || config->entryScopeExcludeSubtrees) { - if (memberof_entry_in_scope(config, base_sdn)) { - /* do nothing, entry scope is spanning - * multiple suffixes, start at suffix */ - } else if ((scope_sdn = memberof_scope_is_child_of_dn(config, base_sdn))) { - /* scope is below suffix, set search base */ - base_sdn = scope_sdn; - } else if (!all_backends) { + if (!all_backends) { + slapi_pblock_destroy(search_pb); break; - } else { - /* its ok, goto the next backend */ - be = slapi_get_next_backend(cookie); - continue; } - } - - slapi_search_internal_set_pb(search_pb, slapi_sdn_get_dn(base_sdn), - LDAP_SCOPE_SUBTREE, filter_str, 0, 0, 0, 0, memberof_get_plugin_id(), 0); - slapi_search_internal_callback_pb(search_pb, callback_data, 0, callback, 0); - slapi_pblock_get(search_pb, SLAPI_PLUGIN_INTOP_RESULT, &rc); - if (rc != LDAP_SUCCESS) { - break; - } - if (!all_backends) { - break; + be = slapi_get_next_backend(cookie); + slapi_pblock_destroy(search_pb); } - slapi_pblock_init(search_pb); - be = slapi_get_next_backend(cookie); + slapi_ch_free((void **)&cookie); + slapi_ch_free_string(&filter_str); } - slapi_pblock_destroy(search_pb); - slapi_ch_free((void **)&cookie); - slapi_ch_free_string(&filter_str); - + if (free_it) { + slapi_ch_free_string(&escaped_filter_val); + } return rc; } -- 2.39.1