From 664d0e19b26f4f655be6b098bbb2dd7170bd4712 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett 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 --- 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 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 --- .../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 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 --- 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 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 --- 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