[Freeipa-devel] [PATCH 0196] baseldap: Fix MidairCollision instantiation during entry modification

Alexander Bokovoy abokovoy at redhat.com
Tue Jul 26 15:22:19 UTC 2016


On Tue, 26 Jul 2016, Martin Babinsky wrote:
>Fix for https://fedorahosted.org/freeipa/ticket/6097
>
>Since this issue was found during investigation of other ticket[1], 
>you can test it by performing steps to reproduce #6041, but instead of 
>internal error you should see the MidairCollision raised as public 
>error with the right error message.
>
>[1] https://fedorahosted.org/freeipa/ticket/6041
I have a preliminary patch for slapi-nis to fix 6041 (attached).

As for this fix -- ACK.

>
>-- 
>Martin^3 Babinsky

>From 8f0d6dab08f61fe2fd1ad64a63f7ab91fc5227d4 Mon Sep 17 00:00:00 2001
>From: Martin Babinsky <mbabinsk at redhat.com>
>Date: Mon, 25 Jul 2016 14:05:08 +0200
>Subject: [PATCH] baseldap: Fix MidairCollision instantiation during entry
> modification
>
>https://fedorahosted.org/freeipa/ticket/6097
>---
> ipaserver/plugins/baseldap.py | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/ipaserver/plugins/baseldap.py b/ipaserver/plugins/baseldap.py
>index 6107e43a6ee17d9b9a63d9dc109664d8b232069f..f7844e3e7c59c259b9c8367d135b2dbefc3f0016 100644
>--- a/ipaserver/plugins/baseldap.py
>+++ b/ipaserver/plugins/baseldap.py
>@@ -1466,7 +1466,7 @@ class LDAPUpdate(LDAPQuery, crud.Update):
>                 entry_attrs.dn, attrs_list)
>         except errors.NotFound:
>             raise errors.MidairCollision(
>-                format=_('the entry was deleted while being modified')
>+                message=_('the entry was deleted while being modified')
>             )
> 
>         self.obj.get_indirect_members(entry_attrs, attrs_list)
>@@ -2344,7 +2344,7 @@ class BaseLDAPModAttribute(LDAPQuery):
>                 entry_attrs.dn, attrs_list)
>         except errors.NotFound:
>             raise errors.MidairCollision(
>-                format=_('the entry was deleted while being modified')
>+                message=_('the entry was deleted while being modified')
>             )
> 
>         for callback in self.get_callbacks('post'):
>-- 
>2.7.4
>

>-- 
>Manage your subscription for the Freeipa-devel mailing list:
>https://www.redhat.com/mailman/listinfo/freeipa-devel
>Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


-- 
/ Alexander Bokovoy
-------------- next part --------------
From 987472fbe8c3564c0bb70c0dd8eebbc430117e0a Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <abokovoy at redhat.com>
Date: Tue, 26 Jul 2016 18:11:53 +0300
Subject: [PATCH] back-sch: do not clobber target of the pblock for idview

When extracting idview all we care is the DN of new target.
We don't really use the rewritten target as a string anymore,
so there is no need to rewrite the string in the pblock.

This fixes a bug when running with 389-ds 1.3.5.10+ which is more
strict about modification of the values in pblock.

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1360245
---
 src/back-sch.c | 43 ++++++++++++++++++++++---------------------
 src/back-sch.h |  2 +-
 2 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/src/back-sch.c b/src/back-sch.c
index 0745329..e15988f 100644
--- a/src/back-sch.c
+++ b/src/back-sch.c
@@ -1652,6 +1652,8 @@ backend_search_cb(Slapi_PBlock *pb)
 	struct backend_search_cbdata cbdata;
 	struct backend_staged_search *staged, *next;
 	int i, isroot, ret;
+	char *original_target = NULL;
+	char *target = NULL;
 
 	if (wrap_get_call_level() > 0) {
 		return 0;
@@ -1676,7 +1678,7 @@ backend_search_cb(Slapi_PBlock *pb)
 		return 0;
 	}
 
-	slapi_pblock_get(pb, SLAPI_SEARCH_TARGET, &cbdata.target);
+	slapi_pblock_get(pb, SLAPI_SEARCH_TARGET, &original_target);
 	slapi_pblock_get(pb, SLAPI_SEARCH_SCOPE, &cbdata.scope);
 	slapi_pblock_get(pb, SLAPI_SEARCH_SIZELIMIT, &cbdata.sizelimit);
 	slapi_pblock_get(pb, SLAPI_SEARCH_TIMELIMIT, &cbdata.timelimit);
@@ -1697,15 +1699,15 @@ backend_search_cb(Slapi_PBlock *pb)
 	/* Okay, we can search. */
 	slapi_log_error(SLAPI_LOG_PLUGIN, cbdata.state->plugin_desc->spd_id,
 			"searching from \"%s\" for \"%s\" with scope %d%s\n",
-			cbdata.target, cbdata.strfilter, cbdata.scope,
+			original_target, cbdata.strfilter, cbdata.scope,
 			backend_sch_scope_as_string(cbdata.scope));
-	cbdata.target_dn = slapi_sdn_new_dn_byval(cbdata.target);
+	cbdata.target_dn = slapi_sdn_new_dn_byval(original_target);
 	/* Check if there's a backend handling this search. */
 	if (!slapi_be_exist(cbdata.target_dn)) {
 		slapi_log_error(SLAPI_LOG_PLUGIN,
 				cbdata.state->plugin_desc->spd_id,
 				"slapi_be_exists(\"%s\") = 0, "
-				"ignoring search\n", cbdata.target);
+				"ignoring search\n", original_target);
 		slapi_sdn_free(&cbdata.target_dn);
 		return 0;
 	}
@@ -1716,22 +1718,23 @@ backend_search_cb(Slapi_PBlock *pb)
 	 * detect the ID view use. Unless the ID view is within the set we control, don't consider the override */
 	map_data_foreach_domain(cbdata.state, backend_search_find_set_dn_cb, &cbdata);
 	if (cbdata.answer == FALSE) {
-		idview_replace_target_dn(&cbdata.target, &cbdata.idview);
+		target = slapi_ch_strdup(original_target);
+		idview_replace_target_dn(&target, &cbdata.idview);
 		if (cbdata.idview != NULL) {
 			slapi_sdn_free(&cbdata.target_dn);
 			/* Perform another check, now for rewritten DN */
-			cbdata.target_dn = slapi_sdn_new_dn_byval(cbdata.target);
+			cbdata.target_dn = slapi_sdn_new_dn_byval(target);
 			map_data_foreach_domain(cbdata.state, backend_search_find_set_dn_cb, &cbdata);
 			/* Rewritten DN might still be outside of our trees */
 			if (cbdata.answer == TRUE) {
 				slapi_log_error(SLAPI_LOG_PLUGIN, cbdata.state->plugin_desc->spd_id,
 						"Use of ID view '%s' is detected, searching from \"%s\" "
 						"for \"%s\" with scope %d%s. Filter may get overridden later.\n",
-						cbdata.idview, cbdata.target, cbdata.strfilter, cbdata.scope,
+						cbdata.idview, target, cbdata.strfilter, cbdata.scope,
 						backend_sch_scope_as_string(cbdata.scope));
 			} else {
 				slapi_sdn_free(&cbdata.target_dn);
-				slapi_ch_free_string(&cbdata.target);
+				slapi_ch_free_string(&target);
 				slapi_ch_free_string(&cbdata.idview);
 				slapi_log_error(SLAPI_LOG_PLUGIN,
 						cbdata.state->plugin_desc->spd_id,
@@ -1739,6 +1742,8 @@ backend_search_cb(Slapi_PBlock *pb)
 						"ignoring search\n");
 				return 0;
 			}
+		} else {
+			slapi_ch_free_string(&target);
 		}
 	}
 	cbdata.answer = FALSE;
@@ -1890,7 +1895,7 @@ backend_search_cb(Slapi_PBlock *pb)
 	}
 	slapi_sdn_free(&cbdata.target_dn);
 	if (cbdata.idview != NULL) {
-		slapi_ch_free_string(&cbdata.target);
+		slapi_ch_free_string(&target);
 	}
 	slapi_ch_free_string(&cbdata.idview);
 #ifdef USE_IPA_IDVIEWS
@@ -1904,7 +1909,6 @@ backend_search_cb(Slapi_PBlock *pb)
 /* Locate the entry for a given DN. */
 struct backend_locate_cbdata {
 	struct plugin_state *state;
-	char *target;
 	Slapi_DN *target_dn;
 
 	struct backend_entry_data *entry_data;
@@ -1953,6 +1957,7 @@ static void
 backend_locate(Slapi_PBlock *pb, struct backend_entry_data **data, const char **group, const char**set)
 {
 	struct backend_locate_cbdata cbdata;
+	char *original_target = NULL;
 
 	slapi_pblock_get(pb, SLAPI_PLUGIN_PRIVATE, &cbdata.state);
 	if (cbdata.state->plugin_base == NULL) {
@@ -1960,9 +1965,9 @@ backend_locate(Slapi_PBlock *pb, struct backend_entry_data **data, const char **
 		*data = NULL;
 		return;
 	}
-	slapi_pblock_get(pb, SLAPI_TARGET_DN, &cbdata.target);
+	slapi_pblock_get(pb, SLAPI_TARGET_DN, &original_target);
 
-	cbdata.target_dn = slapi_sdn_new_dn_byval(cbdata.target);
+	cbdata.target_dn = slapi_sdn_new_dn_byval(original_target);
 	cbdata.entry_data = NULL;
 	cbdata.entry_group = NULL;
 	cbdata.entry_set = NULL;
@@ -1972,12 +1977,9 @@ backend_locate(Slapi_PBlock *pb, struct backend_entry_data **data, const char **
 	 * rebuild the target's RDN to use original attribute's value */
 	if (cbdata.entry_data == NULL) {
 		char *idview = NULL;
-		char *target, *original_target;
-		target = original_target = slapi_ch_strdup(cbdata.target);
+		char *target = NULL;
+		target = slapi_ch_strdup(original_target);
 		idview_replace_target_dn(&target, &idview);
-		if (target != original_target) {
-			slapi_ch_free_string(&original_target);
-		}
 		if (idview != NULL) {
 			char *rdnstr;
 			char *val;
@@ -1992,7 +1994,6 @@ backend_locate(Slapi_PBlock *pb, struct backend_entry_data **data, const char **
 					bval.bv_val = slapi_ch_strdup(val);
 					memset(&scbdata, 0, sizeof(scbdata));
 					scbdata.idview = idview;
-					scbdata.target = target;
 					scbdata.pb = pb;
 					scbdata.state = cbdata.state;
 					scbdata.target_dn = slapi_sdn_new_dn_byval(target);
@@ -2025,7 +2026,6 @@ backend_locate(Slapi_PBlock *pb, struct backend_entry_data **data, const char **
  * insufficient-access error. */
 struct backend_group_check_scope_cbdata {
 	struct plugin_state *state;
-	const char *target;
 	Slapi_DN *target_dn;
 	bool_t ours;
 };
@@ -2050,14 +2050,15 @@ static bool_t
 backend_check_scope_pb(Slapi_PBlock *pb)
 {
 	struct backend_group_check_scope_cbdata cbdata;
+	char *original_target = NULL;
 
 	slapi_pblock_get(pb, SLAPI_PLUGIN_PRIVATE, &cbdata.state);
 	if (cbdata.state->plugin_base == NULL) {
 		/* The plugin was not actually started. */
 		return FALSE;
 	}
-	slapi_pblock_get(pb, SLAPI_TARGET_DN, &cbdata.target);
-	cbdata.target_dn = slapi_sdn_new_dn_byval(cbdata.target);
+	slapi_pblock_get(pb, SLAPI_TARGET_DN, &original_target);
+	cbdata.target_dn = slapi_sdn_new_dn_byval(original_target);
 	cbdata.ours = FALSE;
 	map_data_foreach_domain(cbdata.state, backend_group_check_scope_cb,
 				&cbdata);
diff --git a/src/back-sch.h b/src/back-sch.h
index 1258ae0..9a9abc7 100644
--- a/src/back-sch.h
+++ b/src/back-sch.h
@@ -88,7 +88,7 @@ struct entries_to_send {
 struct backend_search_cbdata {
 	Slapi_PBlock *pb;
 	struct plugin_state *state;
-	char *target, *strfilter, **attrs;
+	char *strfilter, **attrs;
 	char *idview;
 	Slapi_Entry **overrides;
 	int scope, sizelimit, timelimit, attrsonly;
-- 
2.7.4



More information about the Freeipa-devel mailing list