From 053fb02f73220be53d1fb93511684a6f7aa3226f Mon Sep 17 00:00:00 2001
From: Thierry Bordaz <tbordaz@redhat.com>
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