From e5cb97a16fa44e6944e234b9cf509ddb614559a3 Mon Sep 17 00:00:00 2001 From: Mark Reynolds Date: Mon, 9 Dec 2013 16:57:35 -0500 Subject: [PATCH 52/65] Ticket 47622 - Automember betxnpreoperation - transaction not aborted when group entry does not exist Bug Description: If the group defined in the automember plugin does not exist, than any add operation that should trigger an update, succeeds even though the automember update failed. Fix Description: Return an error if a automember post operation update fails - previously we always returned success. Updated plugin_call_func() to check the result of betxn postop plugins. Also added return text to the result message when a betxn plugin fails. This is useful for clients to explain why the operation failed. https://fedorahosted.org/389/ticket/47622 Jenkins: passed Valgrind: passed Coverity: passed Reviewed by: rmeggins(Thanks!) (cherry picked from commit 1214168a222a35627b2bb9964600fad0246558cd) (cherry picked from commit 6de4616f2506b4e093429cc1093e4ad21b22e6c9) --- ldap/servers/plugins/automember/automember.c | 151 ++++++++++++++++++++++----- ldap/servers/slapd/back-ldbm/ldbm_add.c | 4 +- ldap/servers/slapd/back-ldbm/ldbm_delete.c | 5 + ldap/servers/slapd/back-ldbm/ldbm_modify.c | 2 + ldap/servers/slapd/back-ldbm/ldbm_modrdn.c | 3 + ldap/servers/slapd/plugin.c | 3 +- 6 files changed, 137 insertions(+), 31 deletions(-) diff --git a/ldap/servers/plugins/automember/automember.c b/ldap/servers/plugins/automember/automember.c index c7168cb..3214ea1 100644 --- a/ldap/servers/plugins/automember/automember.c +++ b/ldap/servers/plugins/automember/automember.c @@ -103,8 +103,8 @@ static struct automemberRegexRule *automember_parse_regex_rule(char *rule_string static void automember_free_regex_rule(struct automemberRegexRule *rule); static int automember_parse_grouping_attr(char *value, char **grouping_attr, char **grouping_value); -static void automember_update_membership(struct configEntry *config, Slapi_Entry *e, PRFileDesc *ldif_fd); -static void automember_add_member_value(Slapi_Entry *member_e, const char *group_dn, +static int automember_update_membership(struct configEntry *config, Slapi_Entry *e, PRFileDesc *ldif_fd); +static int automember_add_member_value(Slapi_Entry *member_e, const char *group_dn, char *grouping_attr, char *grouping_value, PRFileDesc *ldif_fd); const char *fetch_attr(Slapi_Entry *e, const char *attrname, const char *default_val); @@ -1401,7 +1401,7 @@ automember_parse_grouping_attr(char *value, char **grouping_attr, char **groupin * Determines which target groups need to be updated according to * the rules in config, then performs the updates. */ -static void +static int automember_update_membership(struct configEntry *config, Slapi_Entry *e, PRFileDesc *ldif_fd) { PRCList *rule = NULL; @@ -1412,10 +1412,11 @@ automember_update_membership(struct configEntry *config, Slapi_Entry *e, PRFileD Slapi_DN *last = NULL; PRCList *curr_exclusion = NULL; char **vals = NULL; + int rc = 0; int i = 0; if (!config || !e) { - return; + return -1; } slapi_log_error(SLAPI_LOG_PLUGIN, AUTOMEMBER_PLUGIN_SUBSYSTEM, @@ -1555,15 +1556,23 @@ automember_update_membership(struct configEntry *config, Slapi_Entry *e, PRFileD if (PR_CLIST_IS_EMPTY(&targets)) { /* Add to each default group. */ for (i = 0; config->default_groups && config->default_groups[i]; i++) { - automember_add_member_value(e, config->default_groups[i], - config->grouping_attr, config->grouping_value, ldif_fd); + if(automember_add_member_value(e, config->default_groups[i], config->grouping_attr, + config->grouping_value, ldif_fd)) + { + rc = SLAPI_PLUGIN_FAILURE; + goto out; + } } } else { /* Update the target groups. */ dnitem = (struct automemberDNListItem *)PR_LIST_HEAD(&targets); while ((PRCList *)dnitem != &targets) { - automember_add_member_value(e, slapi_sdn_get_dn(dnitem->dn), - config->grouping_attr, config->grouping_value, ldif_fd); + if(automember_add_member_value(e, slapi_sdn_get_dn(dnitem->dn),config->grouping_attr, + config->grouping_value, ldif_fd)) + { + rc = SLAPI_PLUGIN_FAILURE; + goto out; + } dnitem = (struct automemberDNListItem *)PR_NEXT_LINK((PRCList *)dnitem); } } @@ -1582,6 +1591,9 @@ automember_update_membership(struct configEntry *config, Slapi_Entry *e, PRFileD slapi_ch_free((void**)&dnitem); } +out: + + return rc; } /* @@ -1589,7 +1601,7 @@ automember_update_membership(struct configEntry *config, Slapi_Entry *e, PRFileD * * Adds a member entry to a group. */ -static void +static int automember_add_member_value(Slapi_Entry *member_e, const char *group_dn, char *grouping_attr, char *grouping_value, PRFileDesc *ldif_fd) { @@ -1600,6 +1612,7 @@ automember_add_member_value(Slapi_Entry *member_e, const char *group_dn, char *g char *vals[2]; char *member_value = NULL; int freeit = 0; + int rc = 0; /* If grouping_value is dn, we need to fetch the dn instead. */ if (slapi_attr_type_cmp(grouping_value, "dn", SLAPI_TYPE_CMP_EXACT) == 0) { @@ -1649,6 +1662,7 @@ automember_add_member_value(Slapi_Entry *member_e, const char *group_dn, char *g "a \"%s\" value to group \"%s\" (%s).\n", member_value, grouping_attr, group_dn, ldap_err2string(result)); + rc = result; } } else { slapi_log_error(SLAPI_LOG_FATAL, AUTOMEMBER_PLUGIN_SUBSYSTEM, @@ -1662,8 +1676,9 @@ out: if (freeit) { slapi_ch_free_string(&member_value); } - slapi_pblock_destroy(mod_pb); + + return rc; } @@ -1833,6 +1848,7 @@ automember_add_post_op(Slapi_PBlock *pb) Slapi_DN *sdn = NULL; struct configEntry *config = NULL; PRCList *list = NULL; + int rc = SLAPI_PLUGIN_SUCCESS; slapi_log_error(SLAPI_LOG_TRACE, AUTOMEMBER_PLUGIN_SUBSYSTEM, "--> automember_add_post_op\n"); @@ -1848,8 +1864,9 @@ automember_add_post_op(Slapi_PBlock *pb) } } else { slapi_log_error(SLAPI_LOG_PLUGIN, AUTOMEMBER_PLUGIN_SUBSYSTEM, - "automember_add_post_op: Error " - "retrieving dn\n"); + "automember_add_post_op: Error retrieving dn\n"); + + rc = SLAPI_PLUGIN_FAILURE; goto bail; } @@ -1863,12 +1880,11 @@ automember_add_post_op(Slapi_PBlock *pb) if (e) { /* If the entry is a tombstone, just bail. */ - Slapi_Value *tombstone = - slapi_value_new_string(SLAPI_ATTR_VALUE_TOMBSTONE); - int rc = slapi_entry_attr_has_syntax_value(e, SLAPI_ATTR_OBJECTCLASS, - tombstone); + Slapi_Value *tombstone = slapi_value_new_string(SLAPI_ATTR_VALUE_TOMBSTONE); + int is_tombstone = slapi_entry_attr_has_syntax_value(e, SLAPI_ATTR_OBJECTCLASS, + tombstone); slapi_value_free(&tombstone); - if (rc) { + if (is_tombstone) { return SLAPI_PLUGIN_SUCCESS; } @@ -1891,7 +1907,10 @@ automember_add_post_op(Slapi_PBlock *pb) if (slapi_dn_issuffix(slapi_sdn_get_dn(sdn), config->scope) && (slapi_filter_test_simple(e, config->filter) == 0)) { /* Find out what membership changes are needed and make them. */ - automember_update_membership(config, e, NULL); + if(automember_update_membership(config, e, NULL)){ + rc = SLAPI_PLUGIN_FAILURE; + break; + } } list = PR_NEXT_LINK(list); @@ -1904,11 +1923,21 @@ automember_add_post_op(Slapi_PBlock *pb) "automember_add_post_op: Error " "retrieving post-op entry %s\n", slapi_sdn_get_dn(sdn)); } + bail: slapi_log_error(SLAPI_LOG_TRACE, AUTOMEMBER_PLUGIN_SUBSYSTEM, - "<-- automember_add_post_op\n"); + "<-- automember_add_post_op (%d)\n", rc); - return SLAPI_PLUGIN_SUCCESS; + if(rc){ + char errtxt[SLAPI_DSE_RETURNTEXT_SIZE]; + int result = LDAP_UNWILLING_TO_PERFORM; + + PR_snprintf(errtxt, SLAPI_DSE_RETURNTEXT_SIZE, "Automember Plugin update unexpectedly failed.\n"); + slapi_pblock_set(pb, SLAPI_RESULT_CODE, &result); + slapi_pblock_set(pb, SLAPI_PB_RESULT_TEXT, &errtxt); + } + + return rc; } /* @@ -2216,7 +2245,11 @@ void automember_rebuild_task_thread(void *arg){ if (slapi_dn_issuffix(slapi_entry_get_dn(entries[i]), config->scope) && (slapi_filter_test_simple(entries[i], config->filter) == 0)) { - automember_update_membership(config, entries[i], NULL); + if(automember_update_membership(config, entries[i], NULL)){ + result = SLAPI_PLUGIN_FAILURE; + automember_config_unlock(); + goto out; + } } list = PR_NEXT_LINK(list); } @@ -2416,7 +2449,7 @@ void automember_export_task_thread(void *arg){ /* make sure the plugin is still up, as this loop could run for awhile */ if (!g_plugin_started) { automember_config_unlock(); - result = -1; + result = SLAPI_DSE_CALLBACK_ERROR; goto out; } if (!PR_CLIST_IS_EMPTY(g_automember_config)) { @@ -2426,7 +2459,11 @@ void automember_export_task_thread(void *arg){ if (slapi_dn_issuffix(slapi_sdn_get_dn(td->base_dn), config->scope) && (slapi_filter_test_simple(entries[i], config->filter) == 0)) { - automember_update_membership(config, entries[i], ldif_fd); + if(automember_update_membership(config, entries[i], ldif_fd)){ + result = SLAPI_DSE_CALLBACK_ERROR; + automember_config_unlock(); + goto out; + } } list = PR_NEXT_LINK(list); } @@ -2624,7 +2661,13 @@ void automember_map_task_thread(void *arg){ if (slapi_dn_issuffix(slapi_entry_get_dn_const(e), config->scope) && (slapi_filter_test_simple(e, config->filter) == 0)) { - automember_update_membership(config, e, ldif_fd_out); + if(automember_update_membership(config, e, ldif_fd_out)){ + result = SLAPI_DSE_CALLBACK_ERROR; + slapi_entry_free(e); + slapi_ch_free_string(&entrystr); + automember_config_unlock(); + goto out; + } } list = PR_NEXT_LINK(list); } @@ -2635,7 +2678,7 @@ void automember_map_task_thread(void *arg){ slapi_task_log_notice(task, "Automember map task, skipping invalid entry."); slapi_task_log_status(task, "Automember map task, skipping invalid entry."); } - slapi_ch_free((void **)&entrystr); + slapi_ch_free_string(&entrystr); } automember_config_unlock(); @@ -2666,6 +2709,9 @@ automember_modrdn_post_op(Slapi_PBlock *pb) Slapi_DN *old_sdn = NULL; Slapi_DN *new_sdn = NULL; Slapi_Entry *post_e = NULL; + struct configEntry *config = NULL; + PRCList *list = NULL; + int rc = SLAPI_PLUGIN_SUCCESS; slapi_log_error(SLAPI_LOG_TRACE, AUTOMEMBER_PLUGIN_SUBSYSTEM, "--> automember_modrdn_post_op\n"); @@ -2684,7 +2730,7 @@ automember_modrdn_post_op(Slapi_PBlock *pb) slapi_log_error(SLAPI_LOG_PLUGIN, AUTOMEMBER_PLUGIN_SUBSYSTEM, "automember_modrdn_post_op: Error " "retrieving post-op entry\n"); - return 0; + return SLAPI_PLUGIN_FAILURE; } if ((old_sdn = automember_get_sdn(pb))) { @@ -2694,11 +2740,58 @@ automember_modrdn_post_op(Slapi_PBlock *pb) slapi_log_error(SLAPI_LOG_PLUGIN, AUTOMEMBER_PLUGIN_SUBSYSTEM, "automember_modrdn_post_op: Error " "retrieving dn\n"); + return SLAPI_PLUGIN_FAILURE; } - slapi_log_error(SLAPI_LOG_TRACE, AUTOMEMBER_PLUGIN_SUBSYSTEM, - "<-- automember_modrdn_post_op\n"); + /* If replication, just bail. */ + if (automember_isrepl(pb)) { + return SLAPI_PLUGIN_SUCCESS; + } - return 0; + /* + * Check if a config entry applies to the entry(post modrdn) + */ + automember_config_read_lock(); + + /* Bail out if the plug-in close function was just called. */ + if (!g_plugin_started) { + automember_config_unlock(); + return SLAPI_PLUGIN_SUCCESS; + } + + if (!PR_CLIST_IS_EMPTY(g_automember_config)) { + list = PR_LIST_HEAD(g_automember_config); + while (list != g_automember_config) { + config = (struct configEntry *)list; + + /* Does the entry meet scope and filter requirements? */ + if (slapi_dn_issuffix(slapi_sdn_get_dn(new_sdn), config->scope) && + (slapi_filter_test_simple(post_e, config->filter) == 0)) { + /* Find out what membership changes are needed and make them. */ + if(automember_update_membership(config, post_e, NULL)){ + rc = SLAPI_PLUGIN_FAILURE; + break; + } + } + + list = PR_NEXT_LINK(list); + } + } + + automember_config_unlock(); + + if(rc){ + char errtxt[SLAPI_DSE_RETURNTEXT_SIZE]; + int result = LDAP_UNWILLING_TO_PERFORM; + + PR_snprintf(errtxt, SLAPI_DSE_RETURNTEXT_SIZE, "Automember Plugin update unexpectedly failed. " + "Please see the server errors log for more information.\n"); + slapi_pblock_set(pb, SLAPI_RESULT_CODE, &result); + slapi_pblock_set(pb, SLAPI_PB_RESULT_TEXT, &errtxt); + } + + slapi_log_error(SLAPI_LOG_TRACE, AUTOMEMBER_PLUGIN_SUBSYSTEM, + "<-- automember_modrdn_post_op (%d)\n", rc); + return rc; } diff --git a/ldap/servers/slapd/back-ldbm/ldbm_add.c b/ldap/servers/slapd/back-ldbm/ldbm_add.c index fa1e9bc..e5b9eeb 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_add.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_add.c @@ -357,7 +357,7 @@ ldbm_back_add( Slapi_PBlock *pb ) /* make sure opreturn is set for the postop plugins */ slapi_pblock_set(pb, SLAPI_PLUGIN_OPRETURN, ldap_result_code ? &ldap_result_code : &rc); } - + slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message); goto error_return; } /* @@ -795,6 +795,7 @@ ldbm_back_add( Slapi_PBlock *pb ) if (!opreturn) { slapi_pblock_set(pb, SLAPI_PLUGIN_OPRETURN, ldap_result_code ? &ldap_result_code : &retval); } + slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message); goto error_return; } @@ -1046,6 +1047,7 @@ ldbm_back_add( Slapi_PBlock *pb ) if (!opreturn) { slapi_pblock_set(pb, SLAPI_PLUGIN_OPRETURN, ldap_result_code ? &ldap_result_code : &retval); } + slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message); goto error_return; } diff --git a/ldap/servers/slapd/back-ldbm/ldbm_delete.c b/ldap/servers/slapd/back-ldbm/ldbm_delete.c index 6725123..367ab99 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_delete.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_delete.c @@ -325,6 +325,7 @@ ldbm_back_delete( Slapi_PBlock *pb ) if (!opreturn) { slapi_pblock_set( pb, SLAPI_PLUGIN_OPRETURN, ldap_result_code ? &ldap_result_code : &rc ); } + slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message); goto error_return; } /* the flag could be set in a preop plugin (e.g., USN) */ @@ -354,6 +355,7 @@ ldbm_back_delete( Slapi_PBlock *pb ) ldap_result_code ? &ldap_result_code : &retval ); } + slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message); goto error_return; } @@ -603,6 +605,7 @@ ldbm_back_delete( Slapi_PBlock *pb ) ldap_result_code ? &ldap_result_code : &retval ); } + slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message); goto error_return; } } @@ -633,6 +636,7 @@ ldbm_back_delete( Slapi_PBlock *pb ) &ldap_result_code : &rc ); } /* retval is -1 */ + slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message); goto error_return; } slapi_pblock_set( pb, SLAPI_DELETE_BEPREOP_ENTRY, orig_entry ); @@ -1105,6 +1109,7 @@ ldbm_back_delete( Slapi_PBlock *pb ) if (!opreturn) { slapi_pblock_set( pb, SLAPI_PLUGIN_OPRETURN, &retval ); } + slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message); goto error_return; } diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modify.c b/ldap/servers/slapd/back-ldbm/ldbm_modify.c index b5bdb41..f3b099d 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_modify.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_modify.c @@ -582,6 +582,7 @@ ldbm_back_modify( Slapi_PBlock *pb ) if (!opreturn) { slapi_pblock_set(pb, SLAPI_PLUGIN_OPRETURN, ldap_result_code ? &ldap_result_code : &retval); } + slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message); goto error_return; } @@ -752,6 +753,7 @@ ldbm_back_modify( Slapi_PBlock *pb ) if (!opreturn) { slapi_pblock_set(pb, SLAPI_PLUGIN_OPRETURN, ldap_result_code ? &ldap_result_code : &retval); } + slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message); goto error_return; } diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c index 4908751..1162fdb 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c @@ -466,6 +466,7 @@ ldbm_back_modrdn( Slapi_PBlock *pb ) if (!opreturn) { slapi_pblock_set( pb, SLAPI_PLUGIN_OPRETURN, ldap_result_code ? &ldap_result_code : &rc ); } + slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message); goto error_return; } /* @@ -890,6 +891,7 @@ ldbm_back_modrdn( Slapi_PBlock *pb ) if (!opreturn) { slapi_pblock_set( pb, SLAPI_PLUGIN_OPRETURN, ldap_result_code ? &ldap_result_code : &retval ); } + slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message); goto error_return; } @@ -1130,6 +1132,7 @@ ldbm_back_modrdn( Slapi_PBlock *pb ) if (!opreturn) { slapi_pblock_set( pb, SLAPI_PLUGIN_OPRETURN, ldap_result_code ? &ldap_result_code : &retval ); } + slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message); goto error_return; } diff --git a/ldap/servers/slapd/plugin.c b/ldap/servers/slapd/plugin.c index 5f66ab2..1ca4dc5 100644 --- a/ldap/servers/slapd/plugin.c +++ b/ldap/servers/slapd/plugin.c @@ -1467,7 +1467,8 @@ plugin_call_func (struct slapdplugin *list, int operation, Slapi_PBlock *pb, int } else if (SLAPI_PLUGIN_BEPREOPERATION == list->plg_type || SLAPI_PLUGIN_BETXNPREOPERATION == list->plg_type || - SLAPI_PLUGIN_BEPOSTOPERATION == list->plg_type) + SLAPI_PLUGIN_BEPOSTOPERATION == list->plg_type || + SLAPI_PLUGIN_BETXNPOSTOPERATION == list->plg_type ) { /* * respect fatal error SLAPI_PLUGIN_FAILURE (-1); -- 1.8.1.4