From 7793b7f764c9a60dc8a67289c690c78835fc337b Mon Sep 17 00:00:00 2001
From: Mark Reynolds <mreynolds@redhat.com>
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