Attachment 15633 Details for Bug 12497 – initial patch for master (needs tests)

[patch] initial patch for master (needs tests)

bug-12497-master-01.patch (text/plain), 7.87 KB, created by Andrew Bartlett on 2019-11-26 05:07:29 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andrew Bartlett

Created: 2019-11-26 05:07:29 UTC

Size: 7.87 KB

patch

obsolete

>From 664d0e19b26f4f655be6b098bbb2dd7170bd4712 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Tue, 26 Nov 2019 15:44:32 +1300
>Subject: [PATCH 1/4] dsdb: Explain that descriptor_sd_propagation_recursive()
> is proctected by a transaction
>
>This means we can trust the DB did not change between the two search
>requests.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12497
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> source4/dsdb/samdb/ldb_modules/descriptor.c | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/descriptor.c b/source4/dsdb/samdb/ldb_modules/descriptor.c
>index 9018b750ab5..fb2854438e1 100644
>--- a/source4/dsdb/samdb/ldb_modules/descriptor.c
>+++ b/source4/dsdb/samdb/ldb_modules/descriptor.c
>@@ -1199,6 +1199,9 @@ static int descriptor_sd_propagation_recursive(struct ldb_module *module,
> 	 * LDB_SCOPE_SUBTREE searches are expensive.
> 	 *
> 	 * Note: that we do not search for deleted/recycled objects
>+	 *
>+	 * We know this is safe against a rename race as we are in the
>+	 * prepare_commit(), so must be in a transaction.
> 	 */
> 	ret = dsdb_module_search(module,
> 				 change,
>-- 
>2.17.1
>
>
>From b13c51a8a0b49b8c07a649c99a69c1aebe441e29 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Tue, 26 Nov 2019 15:50:35 +1300
>Subject: [PATCH 2/4] dsdb: Fix issue where inherited Security Descriptors were
> not replicated.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12497
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> .../dsdb/samdb/ldb_modules/repl_meta_data.c   | 25 +++++++++++++++++--
> 1 file changed, 23 insertions(+), 2 deletions(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>index c16ea7bb616..01ab441b37e 100644
>--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>@@ -5634,9 +5634,19 @@ static int replmd_replicated_apply_add(struct replmd_replicated_request *ar)
> 	replmd_ldb_message_sort(msg, ar->schema);
> 
> 	if (!remote_isDeleted) {
>+		/*
>+		 * Ensure any local ACL inheritence is applied from
>+		 * the parent object.
>+		 *
>+		 * This is needed because descriptor is above
>+		 * repl_meta_data in the module stack, so this will
>+		 * not be trigered 'naturally' by the flow of
>+		 * operations.
>+		 */
> 		ret = dsdb_module_schedule_sd_propagation(ar->module,
> 							  ar->objs->partition_dn,
>-							  msg->dn, true);
>+							  msg->dn,
>+							  true);
> 		if (ret != LDB_SUCCESS) {
> 			return replmd_replicated_request_error(ar, ret);
> 		}
>@@ -6320,9 +6330,20 @@ static int replmd_replicated_apply_merge(struct replmd_replicated_request *ar)
> 	}
> 
> 	if (sd_updated && !isDeleted) {
>+		/*
>+		 * This is an existing object, so there is no need to
>+		 * inherit from the parent, but we must inherit any
>+		 * incoming changes to our child objects.
>+		 *
>+		 * This is needed because descriptor is above
>+		 * repl_meta_data in the module stack, so this will
>+		 * not be trigered 'naturally' by the flow of
>+		 * operations.
>+		 */
> 		ret = dsdb_module_schedule_sd_propagation(ar->module,
> 							  ar->objs->partition_dn,
>-							  msg->dn, true);
>+							  msg->dn,
>+							  false);
> 		if (ret != LDB_SUCCESS) {
> 			return ldb_operr(ldb);
> 		}
>-- 
>2.17.1
>
>
>From 364195ec83c114a383cc03ebd83b71118450eb97 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Tue, 26 Nov 2019 16:17:32 +1300
>Subject: [PATCH 3/4] dsdb: Add comments explaining why SD propagation needs to
> be done here
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12497
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> source4/dsdb/samdb/ldb_modules/descriptor.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/descriptor.c b/source4/dsdb/samdb/ldb_modules/descriptor.c
>index fb2854438e1..1179251a68a 100644
>--- a/source4/dsdb/samdb/ldb_modules/descriptor.c
>+++ b/source4/dsdb/samdb/ldb_modules/descriptor.c
>@@ -966,6 +966,10 @@ static int descriptor_rename(struct ldb_module *module, struct ldb_request *req)
> 			return ldb_oom(ldb);
> 		}
> 
>+		/*
>+		 * Force SD propagation on this record (get a new
>+		 * inherited SD from the potentially new parent
>+		 */
> 		ret = dsdb_module_schedule_sd_propagation(module, nc_root,
> 							  newdn, true);
> 		if (ret != LDB_SUCCESS) {
>-- 
>2.17.1
>
>
>From ac5e85d2dacb7bda61718befe71fa5d8e1b59cf8 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Tue, 26 Nov 2019 17:07:16 +1300
>Subject: [PATCH 4/4] dsdb: Also trigger SD propegation when a DN is renamed
> over DRS in repl_meta_data
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12497
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> source4/dsdb/samdb/ldb_modules/descriptor.c   |  3 ++
> .../dsdb/samdb/ldb_modules/repl_meta_data.c   | 45 ++++++++++++++++++-
> 2 files changed, 47 insertions(+), 1 deletion(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/descriptor.c b/source4/dsdb/samdb/ldb_modules/descriptor.c
>index 1179251a68a..7070affa645 100644
>--- a/source4/dsdb/samdb/ldb_modules/descriptor.c
>+++ b/source4/dsdb/samdb/ldb_modules/descriptor.c
>@@ -876,6 +876,9 @@ static int descriptor_modify(struct ldb_module *module, struct ldb_request *req)
> 			return ldb_oom(ldb);
> 		}
> 
>+		/*
>+		 * Force SD propagation on children of this record
>+		 */
> 		ret = dsdb_module_schedule_sd_propagation(module, nc_root,
> 							  dn, false);
> 		if (ret != LDB_SUCCESS) {
>diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>index 01ab441b37e..85a39cd07d0 100644
>--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>@@ -5908,6 +5908,22 @@ static int replmd_replicated_handle_rename(struct replmd_replicated_request *ar,
> 	if (ret == LDB_SUCCESS) {
> 		talloc_free(tmp_ctx);
> 		*renamed = true;
>+		/*
>+		 * Force SD propagation onto this record from the
>+		 * parent (which might have something to inherit)
>+		 */
>+		ret = dsdb_module_schedule_sd_propagation(ar->module,
>+							  ar->objs->partition_dn,
>+							  msg->dn,
>+							  true);
>+		if (ret != LDB_SUCCESS) {
>+			ldb_asprintf_errstring(ldb_module_get_ctx(ar->module),
>+					       "Failed to schedule SD propogation "
>+					       "for incoming dn '%s' to '%s': %s",
>+					       ldb_dn_get_linearized(ar->search_msg->dn),
>+					       ldb_dn_get_linearized(msg->dn),
>+					       ldb_errstring(ldb_module_get_ctx(ar->module)));
>+		}
> 		return ret;
> 	}
> 
>@@ -5946,7 +5962,29 @@ static int replmd_replicated_handle_rename(struct replmd_replicated_request *ar,
> 
> 		ret = dsdb_module_rename(ar->module, ar->search_msg->dn, new_dn,
> 					 DSDB_FLAG_NEXT_MODULE, ar->req);
>-		if (ret != LDB_SUCCESS) {
>+		if (ret == LDB_SUCCESS) {
>+			/*
>+			 * Force SD propagation onto this record from
>+			 * the parent (which might have something to
>+			 * inherit)
>+			 */
>+			ret = dsdb_module_schedule_sd_propagation(ar->module,
>+								  ar->objs->partition_dn,
>+								  new_dn,
>+								  true);
>+			if (ret != LDB_SUCCESS) {
>+				ldb_asprintf_errstring(ldb_module_get_ctx(ar->module),
>+						       "Failed to schedule SD propogation "
>+						       "for incoming conflicting dn '%s' "
>+						       "(was '%s) to %s: %s",
>+						       ldb_dn_get_linearized(conflict_dn),
>+						       ldb_dn_get_linearized(ar->search_msg->dn),
>+						       ldb_dn_get_linearized(new_dn),
>+						       ldb_errstring(ldb_module_get_ctx(ar->module)));
>+				talloc_free(tmp_ctx);
>+				return ret;
>+			}
>+		} else {
> 			ldb_asprintf_errstring(ldb_module_get_ctx(ar->module),
> 					       "Failed to rename incoming conflicting dn '%s' (was '%s') to '%s' - %s\n",
> 					       ldb_dn_get_linearized(conflict_dn),
>@@ -6018,6 +6056,11 @@ static int replmd_replicated_handle_rename(struct replmd_replicated_request *ar,
> 			goto failed;
> 	}
> 
>+	/*
>+	 * We do not schedule SD propogation as this record has not
>+	 * changed parent
>+	 */
>+
> 	talloc_free(tmp_ctx);
> 	return ret;
> failed:
>-- 
>2.17.1
>

Actions: View

Attachments on bug 12497: 15633 | 15670 | 15678 | 15679 | 15680 | 15681 | 15685 | 15686 | 15687 | 15688 | 15689 | 15706 | 15708 | 15738