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