From e47e64776f32a0bb593d1b9b3441e080f87f36f4 Mon Sep 17 00:00:00 2001

From: Mark Reynolds <mreynolds@redhat.com>

Date: Wed, 3 Dec 2014 16:47:59 -0500

Subject: [PATCH 53/53] Ticket 47525 - Crash if setting invalid plugin config

 area for MemberOf Plugin

Bug Description:  Setting the nsslapd-pluginconfigarea to an entry that

                  does not have the required config attributes causes a

                  crash.

Fix Description:  The plugin entry was being accidentally freed instead

                  of the config area entry.

                  The shared config area validation was being performed

                  in postop - this has now been moved into the preop stage.

                  Also, set the returntext when an error occurs.

https://fedorahosted.org/389/ticket/47525

Reviewed by: rmeggins(Thanks!)

(cherry picked from commit 42f935ab7406802d522f357048db1e68d729d5e5)

(cherry picked from commit d06b39743ef3b016ac25a242821a5dfc1ec67cb4)

---

 ldap/servers/plugins/memberof/memberof.c        |   1 -

 ldap/servers/plugins/memberof/memberof_config.c | 193 ++++++++++++++++--------

 2 files changed, 130 insertions(+), 64 deletions(-)

diff --git a/ldap/servers/plugins/memberof/memberof.c b/ldap/servers/plugins/memberof/memberof.c

index bfa733a..7e3e308 100644

--- a/ldap/servers/plugins/memberof/memberof.c

+++ b/ldap/servers/plugins/memberof/memberof.c

@@ -408,7 +408,6 @@ int memberof_postop_start(Slapi_PBlock *pb)

 		}

 	}

-	memberof_set_plugin_area(slapi_entry_get_sdn(config_e));

 	memberof_set_config_area(slapi_entry_get_sdn(config_e));

 	if (( rc = memberof_config( config_e, pb )) != LDAP_SUCCESS ) {

 		slapi_log_error( SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM,

diff --git a/ldap/servers/plugins/memberof/memberof_config.c b/ldap/servers/plugins/memberof/memberof_config.c

index df8ddcb..eb83bd0 100644

--- a/ldap/servers/plugins/memberof/memberof_config.c

+++ b/ldap/servers/plugins/memberof/memberof_config.c

@@ -342,56 +342,40 @@ memberof_apply_config (Slapi_PBlock *pb, Slapi_Entry* entryBefore, Slapi_Entry*

 	*returncode = LDAP_SUCCESS;

 	/*

-	 * Apply the config settings from the shared config entry

+	 * Check if this is a shared config entry

 	 */

 	sharedcfg = slapi_entry_attr_get_charptr(e, SLAPI_PLUGIN_SHARED_CONFIG_AREA);

 	if(sharedcfg){

-		int rc = 0;

-

-		rc = slapi_dn_syntax_check(pb, sharedcfg, 1);

-		if (rc) { /* syntax check failed */

-			slapi_log_error( SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM,"memberof_apply_config: "

-					"%s does not contain a valid DN (%s)\n",

-					SLAPI_PLUGIN_SHARED_CONFIG_AREA, sharedcfg);

-			*returncode = LDAP_INVALID_DN_SYNTAX;

-			goto done;

-		}

 		if((config_sdn = slapi_sdn_new_dn_byval(sharedcfg))){

 			slapi_search_internal_get_entry(config_sdn, NULL, &config_entry, memberof_get_plugin_id());

 			if(config_entry){

-				char errtext[SLAPI_DSE_RETURNTEXT_SIZE];

-				int err = 0;

-				/*

-				 * If we got here, we are updating the shared config area, so we need to

-				 * validate and apply the settings from that config area.

-				 */

-				if ( SLAPI_DSE_CALLBACK_ERROR == memberof_validate_config (pb, NULL, config_entry, &err, errtext,0))

-				{

-					slapi_log_error( SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM,

-									"%s", errtext);

-					*returncode = LDAP_UNWILLING_TO_PERFORM;

-					goto done;

-

-				}

+				/* Set the entry to be the shared config entry.  Validation was done in preop */

 				e = config_entry;

 			} else {

-				/* this should of been checked in preop validation */

-				slapi_log_error( SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_apply_config: "

-						"Failed to locate shared config entry (%s)\n",sharedcfg);

+				/* This should of been checked in preop validation */

+				PR_snprintf(returntext, SLAPI_DSE_RETURNTEXT_SIZE,

+					"memberof_apply_config: Failed to locate shared config entry (%s)",

+					sharedcfg);

+				slapi_log_error( SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM,"%s\n",returntext);

 				*returncode = LDAP_UNWILLING_TO_PERFORM;

 				goto done;

 			}

 		}

 	}

+	/*

+	 * Apply the config settings

+	 */

 	groupattrs = slapi_entry_attr_get_charray(e, MEMBEROF_GROUP_ATTR);

 	memberof_attr = slapi_entry_attr_get_charptr(e, MEMBEROF_ATTR);

 	allBackends = slapi_entry_attr_get_charptr(e, MEMBEROF_BACKEND_ATTR);

 	entryScope = slapi_entry_attr_get_charptr(e, MEMBEROF_ENTRY_SCOPE_ATTR);

         entryScopeExcludeSubtree = slapi_entry_attr_get_charptr(e, MEMBEROF_ENTRY_SCOPE_EXCLUDE_SUBTREE);

-	/* We want to be sure we don't change the config in the middle of

-	 * a memberOf operation, so we obtain an exclusive lock here */

+	/*

+	 * We want to be sure we don't change the config in the middle of

+	 * a memberOf operation, so we obtain an exclusive lock here

+	 */

 	memberof_wlock_config();

 	if (groupattrs)

@@ -402,8 +386,10 @@ memberof_apply_config (Slapi_PBlock *pb, Slapi_Entry* entryBefore, Slapi_Entry*

 		theConfig.groupattrs = groupattrs;

 		groupattrs = NULL; /* config now owns memory */

-		/* We allocate a list of Slapi_Attr using the groupattrs for

-		 * convenience in our memberOf comparison functions */

+		/*

+		 * We allocate a list of Slapi_Attr using the groupattrs for

+		 * convenience in our memberOf comparison functions

+		 */

 		for (i = 0; theConfig.group_slapiattrs && theConfig.group_slapiattrs[i]; i++)

 		{

 			slapi_attr_free(&theConfig.group_slapiattrs[i]);

@@ -412,8 +398,10 @@ memberof_apply_config (Slapi_PBlock *pb, Slapi_Entry* entryBefore, Slapi_Entry*

 		/* Count the number of groupattrs. */

 		for (num_groupattrs = 0; theConfig.groupattrs && theConfig.groupattrs[num_groupattrs]; num_groupattrs++)

 		{

-			/* Add up the total length of all attribute names.  We need

-			 * to know this for building the group check filter later. */

+			/*

+			 * Add up the total length of all attribute names.  We need

+			 * to know this for building the group check filter later.

+			 */

 			groupattr_name_len += strlen(theConfig.groupattrs[num_groupattrs]);

 		}

@@ -434,8 +422,7 @@ memberof_apply_config (Slapi_PBlock *pb, Slapi_Entry* entryBefore, Slapi_Entry*

 		/* Terminate the list. */

 		theConfig.group_slapiattrs[i] = NULL;

-		/* The filter is based off of the groupattr, so we

-		 * update it here too. */

+		/* The filter is based off of the groupattr, so we update it here too. */

 		slapi_filter_free(theConfig.group_filter, 1);

 		if (num_groupattrs > 1)

@@ -463,11 +450,13 @@ memberof_apply_config (Slapi_PBlock *pb, Slapi_Entry* entryBefore, Slapi_Entry*

 			filter_str = slapi_ch_smprintf("(%s=*)", theConfig.groupattrs[0]);

 		}

-		/* Log an error if we were unable to build the group filter for some

+		/*

+		 * Log an error if we were unable to build the group filter for some

 		 * reason.  If this happens, the memberOf plugin will not be able to

 		 * check if an entry is a group, causing it to not catch changes.  This

 		 * shouldn't happen, but there may be some garbage configuration that

-		 * could trigger this. */

+		 * could trigger this.

+		 */

 		if ((theConfig.group_filter = slapi_str2filter(filter_str)) == NULL)

 		{

 			slapi_log_error( SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM,

@@ -547,13 +536,10 @@ memberof_apply_config (Slapi_PBlock *pb, Slapi_Entry* entryBefore, Slapi_Entry*

 	memberof_unlock_config();

 done:

-	slapi_ch_free_string(&sharedcfg);

 	slapi_sdn_free(&config_sdn);

-	if(config_entry){

-		/* we switched the entry pointer to the shared config entry - which needs to be freed */

-		slapi_entry_free(e);

-	}

+	slapi_entry_free(config_entry);

 	slapi_ch_array_free(groupattrs);

+	slapi_ch_free_string(&sharedcfg);

 	slapi_ch_free_string(&memberof_attr);

 	slapi_ch_free_string(&allBackends);

@@ -745,6 +731,7 @@ memberof_config_get_entry_scope_exclude_subtree()

 	return entry_exclude_subtree;

 }

+

 /*

  * Check if we are modifying the config, or changing the shared config entry

  */

@@ -753,53 +740,133 @@ memberof_shared_config_validate(Slapi_PBlock *pb)

 {

 	Slapi_Entry *e = 0;

 	Slapi_Entry *resulting_e = 0;

-	Slapi_DN *sdn = 0;

+	Slapi_Entry *config_entry = NULL;

+	Slapi_DN *sdn = NULL;

+	Slapi_DN *config_sdn = NULL;

 	Slapi_Mods *smods = 0;

+	Slapi_Mod *smod = NULL, *nextmod = NULL;

 	LDAPMod **mods = NULL;

 	char returntext[SLAPI_DSE_RETURNTEXT_SIZE];

+	char *configarea_dn = NULL;

 	int ret = SLAPI_PLUGIN_SUCCESS;

 	slapi_pblock_get(pb, SLAPI_TARGET_SDN, &sdn;;

-   	if (slapi_sdn_issuffix(sdn, memberof_get_config_area()) &&

-   	    slapi_sdn_compare(sdn, memberof_get_config_area()) == 0)

-   	{

-   		/*

-   		 * This is the shared config entry.  Apply the mods and set/validate

-   		 * the config

-   		 */

-		int result = 0;

-

+	if (slapi_sdn_compare(sdn, memberof_get_plugin_area()) == 0 ||

+	    slapi_sdn_compare(sdn, memberof_get_config_area()) == 0)

+	{

 		slapi_pblock_get(pb, SLAPI_ENTRY_PRE_OP, &e);

 		if(e){

+			/*

+			 * Create a copy of the entry and apply the

+			 * mods to create the resulting entry.

+			 */

 			slapi_pblock_get(pb, SLAPI_MODIFY_MODS, &mods;;

 			smods = slapi_mods_new();

 			slapi_mods_init_byref(smods, mods);

-

-			/* Create a copy of the entry and apply the

-			 * mods to create the resulting entry. */

 			resulting_e = slapi_entry_dup(e);

 			if (mods && (slapi_entry_apply_mods(resulting_e, mods) != LDAP_SUCCESS)) {

 				/* we don't care about this, the update is invalid and will be caught later */

 				goto bail;

 			}

-			if ( SLAPI_DSE_CALLBACK_ERROR == memberof_validate_config (pb, NULL, resulting_e, &ret, returntext,0)) {

-				slapi_log_error( SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM,

-								"%s", returntext);

-				ret = LDAP_UNWILLING_TO_PERFORM;

+			if (slapi_sdn_compare(sdn, memberof_get_plugin_area())){

+				/*

+				 * This entry is a plugin config area entry, validate it.

+				 */

+				if( SLAPI_DSE_CALLBACK_ERROR == memberof_validate_config (pb, NULL, resulting_e, &ret, returntext,0)) {

+					ret = LDAP_UNWILLING_TO_PERFORM;

+				}

+			} else {

+				/*

+				 * This is the memberOf plugin entry, check if we are adding/replacing the

+				 * plugin config area.

+				 */

+				nextmod = slapi_mod_new();

+				for (smod = slapi_mods_get_first_smod(smods, nextmod);

+					 smod != NULL;

+					 smod = slapi_mods_get_next_smod(smods, nextmod) )

+				{

+					if ( PL_strcasecmp(SLAPI_PLUGIN_SHARED_CONFIG_AREA, slapi_mod_get_type(smod)) == 0 )

+					{

+						/*

+						 * Okay, we are modifying the plugin config area, we only care about

+						 * adds and replaces.

+						 */

+						if(SLAPI_IS_MOD_REPLACE(slapi_mod_get_operation(smod)) ||

+						   SLAPI_IS_MOD_ADD(slapi_mod_get_operation(smod)))

+						{

+						    struct berval *bv = NULL;

+						    int rc = 0;

+

+							bv = slapi_mod_get_first_value(smod);

+							configarea_dn = slapi_ch_strdup(bv->bv_val);

+							if(configarea_dn){

+								/* Check the DN syntax */

+								rc = slapi_dn_syntax_check(pb, configarea_dn, 1);

+								if (rc) { /* syntax check failed */

+									PR_snprintf(returntext, SLAPI_DSE_RETURNTEXT_SIZE,

+										"%s does not contain a valid DN (%s)",

+										SLAPI_PLUGIN_SHARED_CONFIG_AREA, configarea_dn);

+									ret = LDAP_UNWILLING_TO_PERFORM;

+									goto bail;

+								}

+

+								/* Check if the plugin config area entry exists */

+								if((config_sdn = slapi_sdn_new_dn_byval(configarea_dn))){

+									rc = slapi_search_internal_get_entry(config_sdn, NULL, &config_entry,

+										memberof_get_plugin_id());

+									if(config_entry){

+										int err = 0;

+										/*

+										 * Validate the settings from the new config area.

+										 */

+										if ( memberof_validate_config(pb, NULL, config_entry, &err, returntext,0)

+											 == SLAPI_DSE_CALLBACK_ERROR )

+										{

+											ret = LDAP_UNWILLING_TO_PERFORM;

+											goto bail;

+

+										}

+									} else {

+										/* The config area does not exist */

+										PR_snprintf(returntext, SLAPI_DSE_RETURNTEXT_SIZE,

+											"Unable to locate shared config entry (%s) error %d",

+											slapi_sdn_get_dn(memberof_get_config_area()), rc);

+										ret = LDAP_UNWILLING_TO_PERFORM;

+										goto bail;

+									}

+								}

+							}

+							slapi_ch_free_string(&configarea_dn);

+							slapi_sdn_free(&config_sdn);

+							slapi_entry_free(config_entry);

+						}

+					}

+				}

 			}

 		} else {

-			slapi_log_error( SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_shared_config_validate: "

-											"Unable to locate shared config entry (%s) error %d\n",

-											slapi_sdn_get_dn(memberof_get_config_area()), result);

+			PR_snprintf(returntext, SLAPI_DSE_RETURNTEXT_SIZE,"Unable to locate shared config entry (%s)",

+				slapi_sdn_get_dn(memberof_get_config_area()));

 			ret = LDAP_UNWILLING_TO_PERFORM;

 		}

    	}

 bail:

+

+	if (ret){

+		slapi_pblock_set(pb, SLAPI_RESULT_CODE, &ret;;

+		slapi_pblock_set(pb, SLAPI_PB_RESULT_TEXT, returntext);

+		slapi_log_error( SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_shared_config_validate: %s/n",

+			returntext);

+	}

+	slapi_sdn_free(&config_sdn);

+	if(nextmod)

+		slapi_mod_free(&nextmod);

 	slapi_mods_free(&smods);

 	slapi_entry_free(resulting_e);

+	slapi_entry_free(config_entry);

+	slapi_ch_free_string(&configarea_dn);

 	return ret;

 }

-- 

1.9.3