From e47e64776f32a0bb593d1b9b3441e080f87f36f4 Mon Sep 17 00:00:00 2001 From: Mark Reynolds 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