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