From 7793b7f764c9a60dc8a67289c690c78835fc337b Mon Sep 17 00:00:00 2001 From: Mark Reynolds Date: Tue, 20 Dec 2016 14:59:02 -0500 Subject: [PATCH 419/425] Ticket 49072 - validate memberof fixup task args Bug Description: If an invalid base dn, or invalid filter was provided in the task there was no way to tell thathte task actually failed. Fix Description: Log an error, and properly update the task status/exit code when an error occurs. Added CI test (also fixed some issues in the dynamic plugins test suite). https://fedorahosted.org/389/ticket/49072 Reviewed by: nhosoi(Thanks!) (cherry picked from commit a79ae70df6b20cd288fca511f784c414e8c52df4) (cherry picked from commit b0020b73d34bdd630fb5b1a3e4fcebbb4b81f9c9) (cherry picked from commit 8efcc704c42fb11e0950b12d7abaf65e77050070) --- ldap/servers/plugins/memberof/memberof.c | 48 ++++++++++++++++++++++---------- ldap/servers/slapd/plugin_internal_op.c | 27 ++++++++---------- 2 files changed, 46 insertions(+), 29 deletions(-) diff --git a/ldap/servers/plugins/memberof/memberof.c b/ldap/servers/plugins/memberof/memberof.c index 7cb0e27..fd080da 100644 --- a/ldap/servers/plugins/memberof/memberof.c +++ b/ldap/servers/plugins/memberof/memberof.c @@ -92,6 +92,13 @@ typedef struct _memberof_get_groups_data Slapi_ValueSet **groupvals; } memberof_get_groups_data; +typedef struct _task_data +{ + char *dn; + char *bind_dn; + char *filter_str; +} task_data; + /*** function prototypes ***/ /* exported functions */ @@ -169,7 +176,7 @@ static void memberof_task_destructor(Slapi_Task *task); static const char *fetch_attr(Slapi_Entry *e, const char *attrname, const char *default_val); static void memberof_fixup_task_thread(void *arg); -static int memberof_fix_memberof(MemberOfConfig *config, char *dn, char *filter_str); +static int memberof_fix_memberof(MemberOfConfig *config, Slapi_Task *task, task_data *td); static int memberof_fix_memberof_callback(Slapi_Entry *e, void *callback_data); @@ -2271,13 +2278,6 @@ void memberof_unlock() PR_ExitMonitor(memberof_operation_lock); } -typedef struct _task_data -{ - char *dn; - char *bind_dn; - char *filter_str; -} task_data; - void memberof_fixup_task_thread(void *arg) { MemberOfConfig configCopy = {0, 0, 0, 0}; @@ -2285,6 +2285,11 @@ void memberof_fixup_task_thread(void *arg) task_data *td = NULL; int rc = 0; + + if (!task) { + return; /* no task */ + } + /* Fetch our task data from the task */ td = (task_data *)slapi_task_get_data(task); @@ -2292,8 +2297,10 @@ void memberof_fixup_task_thread(void *arg) slapi_td_set_dn(slapi_ch_strdup(td->bind_dn)); slapi_task_begin(task, 1); - slapi_task_log_notice(task, "Memberof task starts (arg: %s) ...\n", - td->filter_str); + slapi_task_log_notice(task, "Memberof task starts (filter: %s) ...\n", + td->filter_str); + slapi_log_error(SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM, + "Memberof task starts (filter: \"%s\") ...\n", td->filter_str); /* We need to get the config lock first. Trying to get the * config lock after we already hold the op lock can cause @@ -2310,7 +2317,7 @@ void memberof_fixup_task_thread(void *arg) memberof_lock(); /* do real work */ - rc = memberof_fix_memberof(&configCopy, td->dn, td->filter_str); + rc = memberof_fix_memberof(&configCopy, task, td); /* release the memberOf operation lock */ memberof_unlock(); @@ -2320,6 +2327,9 @@ void memberof_fixup_task_thread(void *arg) slapi_task_log_notice(task, "Memberof task finished."); slapi_task_log_status(task, "Memberof task finished."); slapi_task_inc_progress(task); + slapi_log_error(SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM, + "Memberof task finished (filter: %s) result: %d\n", + td->filter_str, rc); /* this will queue the destruction of the task */ slapi_task_finish(task, rc); @@ -2434,13 +2444,13 @@ memberof_task_destructor(Slapi_Task *task) } } -int memberof_fix_memberof(MemberOfConfig *config, char *dn, char *filter_str) +int memberof_fix_memberof(MemberOfConfig *config, Slapi_Task *task, task_data *td) { int rc = 0; Slapi_PBlock *search_pb = slapi_pblock_new(); - slapi_search_internal_set_pb(search_pb, dn, - LDAP_SCOPE_SUBTREE, filter_str, 0, 0, + slapi_search_internal_set_pb(search_pb, td->dn, + LDAP_SCOPE_SUBTREE, td->filter_str, 0, 0, 0, 0, memberof_get_plugin_id(), 0); @@ -2449,6 +2459,16 @@ int memberof_fix_memberof(MemberOfConfig *config, char *dn, char *filter_str) config, 0, memberof_fix_memberof_callback, 0); + if (rc){ + char *errmsg; + int result; + + slapi_pblock_get(search_pb, SLAPI_PLUGIN_INTOP_RESULT, &result); + errmsg = ldap_err2string(result); + slapi_log_err(SLAPI_LOG_ERR, MEMBEROF_PLUGIN_SUBSYSTEM, + "memberof_fix_memberof - Failed (%s)\n", errmsg ); + slapi_task_log_notice(task, "Memberof task failed (%s)\n", errmsg ); + } slapi_pblock_destroy(search_pb); diff --git a/ldap/servers/slapd/plugin_internal_op.c b/ldap/servers/slapd/plugin_internal_op.c index 4c7462d..f916caf 100644 --- a/ldap/servers/slapd/plugin_internal_op.c +++ b/ldap/servers/slapd/plugin_internal_op.c @@ -757,7 +757,7 @@ search_internal_callback_pb (Slapi_PBlock *pb, void *callback_data, if (ifstr == NULL || (scope != LDAP_SCOPE_BASE && scope != LDAP_SCOPE_ONELEVEL && scope != LDAP_SCOPE_SUBTREE)) { - opresult = LDAP_PARAM_ERROR; + opresult = LDAP_PARAM_ERROR; slapi_pblock_set(pb, SLAPI_PLUGIN_INTOP_RESULT, &opresult); return -1; } @@ -774,19 +774,19 @@ search_internal_callback_pb (Slapi_PBlock *pb, void *callback_data, op->o_search_referral_handler = internal_ref_entry_callback; filter = slapi_str2filter((fstr = slapi_ch_strdup(ifstr))); - if(scope == LDAP_SCOPE_BASE) { - filter->f_flags |= (SLAPI_FILTER_LDAPSUBENTRY | - SLAPI_FILTER_TOMBSTONE | SLAPI_FILTER_RUV); + if (NULL == filter) { + int result = LDAP_FILTER_ERROR; + send_ldap_result(pb, result, NULL, NULL, 0, NULL); + slapi_pblock_set(pb, SLAPI_PLUGIN_INTOP_RESULT, &result); + rc = -1; + goto done; } - if (NULL == filter) - { - send_ldap_result(pb, LDAP_FILTER_ERROR, NULL, NULL, 0, NULL); - rc = -1; - goto done; + if (scope == LDAP_SCOPE_BASE) { + filter->f_flags |= (SLAPI_FILTER_LDAPSUBENTRY | + SLAPI_FILTER_TOMBSTONE | SLAPI_FILTER_RUV); } filter_normalize(filter); - slapi_pblock_set(pb, SLAPI_SEARCH_FILTER, filter); slapi_pblock_set(pb, SLAPI_REQCONTROLS, controls); @@ -814,11 +814,8 @@ search_internal_callback_pb (Slapi_PBlock *pb, void *callback_data, slapi_pblock_get(pb, SLAPI_SEARCH_FILTER, &filter); done: - slapi_ch_free((void **) & fstr); - if (filter != NULL) - { - slapi_filter_free(filter, 1 /* recurse */); - } + slapi_ch_free_string(&fstr); + slapi_filter_free(filter, 1 /* recurse */); slapi_pblock_get(pb, SLAPI_SEARCH_ATTRS, &tmp_attrs); slapi_ch_array_free(tmp_attrs); slapi_pblock_set(pb, SLAPI_SEARCH_ATTRS, NULL); -- 2.9.3