From 05b83f123551c930aa6d807963e4035e6e73252a Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Sun, 23 Jun 2013 19:47:35 +1000 Subject: [PATCH 01/12] dsdb-descriptor: Do not do a subtree search unless we have child entries This avoids a subtree search here in most cases where an object is deleted. Andrew Bartlett Signed-off-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher (cherry picked from commit 03b44d26fd17761675fe33ab29e8f325f59d8a5c) --- source4/dsdb/samdb/ldb_modules/descriptor.c | 33 ++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/source4/dsdb/samdb/ldb_modules/descriptor.c b/source4/dsdb/samdb/ldb_modules/descriptor.c index 7743baa..ceac8db 100644 --- a/source4/dsdb/samdb/ldb_modules/descriptor.c +++ b/source4/dsdb/samdb/ldb_modules/descriptor.c @@ -1186,16 +1186,47 @@ static int descriptor_sd_propagation_recursive(struct ldb_module *module, const char * const no_attrs[] = { "@__NONE__", NULL }; struct descriptor_changes *c; struct descriptor_changes *stopped_stack = NULL; + enum ldb_scope scope; int ret; /* + * First confirm this object has children, or exists (depending on change->force_self) + * + * LDB_SCOPE_SUBTREE searches are expensive. + * + * Note: that we do not search for deleted/recycled objects + */ + ret = dsdb_module_search(module, + change, + &res, + change->dn, + LDB_SCOPE_ONELEVEL, + no_attrs, + DSDB_FLAG_NEXT_MODULE | + DSDB_FLAG_AS_SYSTEM, + NULL, /* parent_req */ + "(objectClass=*)"); + if (ret != LDB_SUCCESS) { + return ret; + } + + if (res->count == 0 && !change->force_self) { + TALLOC_FREE(res); + return LDB_SUCCESS; + } else if (res->count == 0 && change->force_self) { + scope = LDB_SCOPE_BASE; + } else { + scope = LDB_SCOPE_SUBTREE; + } + + /* * Note: that we do not search for deleted/recycled objects */ ret = dsdb_module_search(module, change, &res, change->dn, - LDB_SCOPE_SUBTREE, + scope, no_attrs, DSDB_FLAG_NEXT_MODULE | DSDB_FLAG_AS_SYSTEM, -- 1.7.10.4 From 498d9b3d687f91baf7f3ab1e92cc77cc9da0c82c Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Sun, 23 Jun 2013 21:38:40 +1000 Subject: [PATCH 02/12] dsdb: Rework subtree_rename module to use recursive LDB_SCOPE_ONELEVEL searches This should be more efficient, particularly in the leaf node case when renaming and deleting entries on large databases. Andrew Bartlett Signed-off-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher (cherry picked from commit 31fb7f9c1b93b0f2114dec5096e43616ed317720) --- source4/dsdb/samdb/ldb_modules/subtree_rename.c | 201 ++++++++++---------- .../dsdb/samdb/ldb_modules/wscript_build_server | 2 +- 2 files changed, 99 insertions(+), 104 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/subtree_rename.c b/source4/dsdb/samdb/ldb_modules/subtree_rename.c index ee787d1..d26dabe 100644 --- a/source4/dsdb/samdb/ldb_modules/subtree_rename.c +++ b/source4/dsdb/samdb/ldb_modules/subtree_rename.c @@ -34,19 +34,11 @@ #include #include "libds/common/flags.h" #include "dsdb/samdb/samdb.h" - -struct subren_msg_store { - struct subren_msg_store *next; - struct ldb_dn *olddn; - struct ldb_dn *newdn; -}; +#include "dsdb/samdb/ldb_modules/util.h" struct subtree_rename_context { struct ldb_module *module; struct ldb_request *req; - - struct subren_msg_store *list; - struct subren_msg_store *current; }; static struct subtree_rename_context *subren_ctx_init(struct ldb_module *module, @@ -66,14 +58,11 @@ static struct subtree_rename_context *subren_ctx_init(struct ldb_module *module, return ac; } -static int subtree_rename_next_request(struct subtree_rename_context *ac); - static int subtree_rename_callback(struct ldb_request *req, struct ldb_reply *ares) { struct ldb_context *ldb; struct subtree_rename_context *ac; - int ret; ac = talloc_get_type(req->context, struct subtree_rename_context); ldb = ldb_module_get_ctx(ac->module); @@ -98,47 +87,8 @@ static int subtree_rename_callback(struct ldb_request *req, LDB_ERR_OPERATIONS_ERROR); } - if (ac->current == NULL) { - /* this was the last one */ - return ldb_module_done(ac->req, ares->controls, - ares->response, LDB_SUCCESS); - } - - ret = subtree_rename_next_request(ac); - if (ret != LDB_SUCCESS) { - return ldb_module_done(ac->req, NULL, NULL, ret); - } - talloc_free(ares); - return LDB_SUCCESS; -} - -static int subtree_rename_next_request(struct subtree_rename_context *ac) -{ - struct ldb_context *ldb; - struct ldb_request *req; - int ret; - - ldb = ldb_module_get_ctx(ac->module); - - if (ac->current == NULL) { - return ldb_operr(ldb); - } - - ret = ldb_build_rename_req(&req, ldb, ac->current, - ac->current->olddn, - ac->current->newdn, - ac->req->controls, - ac, subtree_rename_callback, - ac->req); - LDB_REQ_SET_LOCATION(req); - if (ret != LDB_SUCCESS) { - return ret; - } - - ac->current = ac->current->next; - - return ldb_next_request(ac->module, req); + return ldb_module_done(ac->req, NULL, NULL, LDB_SUCCESS); } static int check_constraints(struct ldb_message *msg, @@ -297,16 +247,15 @@ static int check_constraints(struct ldb_message *msg, return LDB_SUCCESS; } -static int subtree_rename_search_callback(struct ldb_request *req, - struct ldb_reply *ares) +static int subtree_rename_search_onelevel_callback(struct ldb_request *req, + struct ldb_reply *ares) { - struct subren_msg_store *store; struct subtree_rename_context *ac; int ret; ac = talloc_get_type(req->context, struct subtree_rename_context); - if (!ares || !ac->current) { + if (!ares) { return ldb_module_done(ac->req, NULL, NULL, LDB_ERR_OPERATIONS_ERROR); } @@ -317,46 +266,89 @@ static int subtree_rename_search_callback(struct ldb_request *req, switch (ares->type) { case LDB_REPLY_ENTRY: - if (ldb_dn_compare(ares->message->dn, ac->list->olddn) == 0) { - /* - * This is the root entry of the originating move - * respectively rename request. It has been already - * stored in the list using "subtree_rename_search()". - * Only this one is subject to constraint checking. - */ - ret = check_constraints(ares->message, ac, - ac->list->olddn, - ac->list->newdn); - if (ret != LDB_SUCCESS) { - return ldb_module_done(ac->req, NULL, NULL, - ret); - } - - talloc_free(ares); - return LDB_SUCCESS; + { + struct ldb_dn *old_dn = ares->message->dn; + struct ldb_dn *new_dn = ldb_dn_copy(ares, old_dn); + if (!new_dn) { + return ldb_module_oom(ac->module); } - store = talloc_zero(ac, struct subren_msg_store); - if (store == NULL) { + if ( ! ldb_dn_remove_base_components(new_dn, + ldb_dn_get_comp_num(ac->req->op.rename.olddn))) { return ldb_module_done(ac->req, NULL, NULL, LDB_ERR_OPERATIONS_ERROR); } - ac->current->next = store; - ac->current = store; - - /* the first list element contains the base for the rename */ - store->olddn = talloc_steal(store, ares->message->dn); - store->newdn = ldb_dn_copy(store, store->olddn); - if ( ! ldb_dn_remove_base_components(store->newdn, - ldb_dn_get_comp_num(ac->list->olddn))) { + if ( ! ldb_dn_add_base(new_dn, ac->req->op.rename.newdn)) { return ldb_module_done(ac->req, NULL, NULL, LDB_ERR_OPERATIONS_ERROR); } + ret = dsdb_module_rename(ac->module, old_dn, new_dn, DSDB_FLAG_OWN_MODULE, req); + if (ret != LDB_SUCCESS) { + return ret; + } + + talloc_free(ares); - if ( ! ldb_dn_add_base(store->newdn, ac->list->newdn)) { + return LDB_SUCCESS; + } + case LDB_REPLY_REFERRAL: + /* ignore */ + break; + + case LDB_REPLY_DONE: + + ret = ldb_build_rename_req(&req, ldb_module_get_ctx(ac->module), ac, + ac->req->op.rename.olddn, + ac->req->op.rename.newdn, + ac->req->controls, + ac, subtree_rename_callback, + ac->req); + LDB_REQ_SET_LOCATION(req); + if (ret != LDB_SUCCESS) { + return ret; + } + + talloc_free(ares); + return ldb_next_request(ac->module, req); + } + + return LDB_SUCCESS; +} + +static int subtree_rename_search_base_callback(struct ldb_request *req, + struct ldb_reply *ares) +{ + struct ldb_request *search_req; + struct subtree_rename_context *ac; + static const char * const no_attrs[] = {NULL}; + int ret; + + ac = talloc_get_type(req->context, struct subtree_rename_context); + + if (!ares) { + return ldb_module_done(ac->req, NULL, NULL, + LDB_ERR_OPERATIONS_ERROR); + } + if (ares->error != LDB_SUCCESS) { + return ldb_module_done(ac->req, ares->controls, + ares->response, ares->error); + } + + switch (ares->type) { + case LDB_REPLY_ENTRY: + /* + * This is the root entry of the originating move + * respectively rename request. It has been already + * stored in the list using "subtree_rename_search()". + * Only this one is subject to constraint checking. + */ + ret = check_constraints(ares->message, ac, + ac->req->op.rename.olddn, + ac->req->op.rename.newdn); + if (ret != LDB_SUCCESS) { return ldb_module_done(ac->req, NULL, NULL, - LDB_ERR_OPERATIONS_ERROR); + ret); } break; @@ -366,16 +358,28 @@ static int subtree_rename_search_callback(struct ldb_request *req, case LDB_REPLY_DONE: - /* rewind ac->current */ - ac->current = ac->list; - - /* All dns set up, start with the first one */ - ret = subtree_rename_next_request(ac); + ret = ldb_build_search_req(&search_req, ldb_module_get_ctx(ac->module), ac, + ac->req->op.rename.olddn, + LDB_SCOPE_ONELEVEL, + "(objectClass=*)", + no_attrs, + NULL, + ac, + subtree_rename_search_onelevel_callback, + req); + LDB_REQ_SET_LOCATION(search_req); + if (ret != LDB_SUCCESS) { + return ret; + } + ret = ldb_request_add_control(search_req, LDB_CONTROL_SHOW_RECYCLED_OID, + true, NULL); if (ret != LDB_SUCCESS) { - return ldb_module_done(ac->req, NULL, NULL, ret); + return ret; } - break; + + talloc_free(ares); + return ldb_next_request(ac->module, search_req); } talloc_free(ares); @@ -412,23 +416,14 @@ static int subtree_rename(struct ldb_module *module, struct ldb_request *req) return ldb_oom(ldb); } - /* add this entry as the first to do */ - ac->current = talloc_zero(ac, struct subren_msg_store); - if (ac->current == NULL) { - return ldb_oom(ldb); - } - ac->current->olddn = req->op.rename.olddn; - ac->current->newdn = req->op.rename.newdn; - ac->list = ac->current; - ret = ldb_build_search_req(&search_req, ldb, ac, req->op.rename.olddn, - LDB_SCOPE_SUBTREE, + LDB_SCOPE_BASE, "(objectClass=*)", attrs, NULL, ac, - subtree_rename_search_callback, + subtree_rename_search_base_callback, req); LDB_REQ_SET_LOCATION(search_req); if (ret != LDB_SUCCESS) { diff --git a/source4/dsdb/samdb/ldb_modules/wscript_build_server b/source4/dsdb/samdb/ldb_modules/wscript_build_server index c23ad16..41eb0f3 100755 --- a/source4/dsdb/samdb/ldb_modules/wscript_build_server +++ b/source4/dsdb/samdb/ldb_modules/wscript_build_server @@ -235,7 +235,7 @@ bld.SAMBA_MODULE('ldb_subtree_rename', init_function='ldb_subtree_rename_module_init', module_init_name='ldb_init_module', internal_module=False, - deps='talloc samba-util ldb samdb-common' + deps='talloc samba-util ldb samdb-common DSDB_MODULE_HELPERS' ) -- 1.7.10.4 From 16f59484da8b04054ddb759babf6ad24f492448c Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Wed, 19 Jun 2013 10:30:48 +1000 Subject: [PATCH 03/12] dsdb-ridalloc: Rework ridalloc to return error strings where RID allocation fails We now also only poke the RID manager once per request. This may help track down why RID allocation can fail, as while we never wait for the RID set to be created/updated, it may be the only clue the admin gets as to why the async allocations were failing. Andrew Bartlett Pair-Programmed-With: Stefan Metzmacher Signed-off-by: Andrew Bartlett Signed-off-by: Stefan Metzmacher Reviewed-by: Stefan Metzmacher (cherry picked from commit db9c3c62c89e1328872e3fdedde22b78770728a9) --- source4/dsdb/samdb/ldb_modules/ridalloc.c | 56 +++++++++++++++++++++-------- 1 file changed, 41 insertions(+), 15 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/ridalloc.c b/source4/dsdb/samdb/ldb_modules/ridalloc.c index c0859d3..f6738f9 100644 --- a/source4/dsdb/samdb/ldb_modules/ridalloc.c +++ b/source4/dsdb/samdb/ldb_modules/ridalloc.c @@ -64,7 +64,7 @@ returns immediately. It should be called well before we completely run out of RIDs */ -static void ridalloc_poke_rid_manager(struct ldb_module *module) +static int ridalloc_poke_rid_manager(struct ldb_module *module) { struct imessaging_context *msg; struct server_id *server; @@ -72,26 +72,42 @@ static void ridalloc_poke_rid_manager(struct ldb_module *module) struct loadparm_context *lp_ctx = (struct loadparm_context *)ldb_get_opaque(ldb, "loadparm"); TALLOC_CTX *tmp_ctx = talloc_new(module); + NTSTATUS status; msg = imessaging_client_init(tmp_ctx, lp_ctx, ldb_get_event_context(ldb)); if (!msg) { + ldb_asprintf_errstring(ldb_module_get_ctx(module), + "Failed to send MSG_DREPL_ALLOCATE_RID, " + "unable init client messaging context"); DEBUG(3,(__location__ ": Failed to create messaging context\n")); talloc_free(tmp_ctx); - return; + return LDB_ERR_UNWILLING_TO_PERFORM; } server = irpc_servers_byname(msg, msg, "dreplsrv"); if (!server) { + ldb_asprintf_errstring(ldb_module_get_ctx(module), + "Failed to send MSG_DREPL_ALLOCATE_RID, " + "unable to locate dreplsrv"); /* this means the drepl service is not running */ talloc_free(tmp_ctx); - return; + return LDB_ERR_UNWILLING_TO_PERFORM; } - imessaging_send(msg, server[0], MSG_DREPL_ALLOCATE_RID, NULL); + status = imessaging_send(msg, server[0], MSG_DREPL_ALLOCATE_RID, NULL); + + /* Only error out if an error happened, not on STATUS_MORE_ENTRIES, ie a delayed message */ + if (NT_STATUS_IS_ERR(status)) { + ldb_asprintf_errstring(ldb_module_get_ctx(module), + "Failed to send MSG_DREPL_ALLOCATE_RID to dreplsrv at %s: %s", + server_id_str(tmp_ctx, server), nt_errstr(status)); + talloc_free(tmp_ctx); + return LDB_ERR_UNWILLING_TO_PERFORM; + } - /* we don't care if the message got through */ talloc_free(tmp_ctx); + return LDB_SUCCESS; } @@ -423,8 +439,16 @@ static int ridalloc_create_own_rid_set(struct ldb_module *module, TALLOC_CTX *me } if (!GUID_equal(&fsmo_role_guid, our_ntds_guid)) { - ridalloc_poke_rid_manager(module); - ldb_asprintf_errstring(ldb, "Remote RID Set allocation needs refresh"); + ret = ridalloc_poke_rid_manager(module); + if (ret != LDB_SUCCESS) { + ldb_asprintf_errstring(ldb, + "Request for remote creation of " + "RID Set for this DC failed: %s", + ldb_errstring(ldb)); + } else { + ldb_asprintf_errstring(ldb, + "Remote RID Set creation needed"); + } talloc_free(tmp_ctx); return LDB_ERR_UNWILLING_TO_PERFORM; } @@ -473,8 +497,13 @@ static int ridalloc_new_own_pool(struct ldb_module *module, uint64_t *new_pool, } if (!is_us) { - ridalloc_poke_rid_manager(module); - ldb_asprintf_errstring(ldb, "Remote RID Set allocation needs refresh"); + ret = ridalloc_poke_rid_manager(module); + if (ret != LDB_SUCCESS) { + ldb_asprintf_errstring(ldb, "Request for remote refresh of RID Set allocation failed: %s", + ldb_errstring(ldb)); + } else { + ldb_asprintf_errstring(ldb, "Remote RID Set refresh needed"); + } talloc_free(tmp_ctx); return LDB_ERR_UNWILLING_TO_PERFORM; } @@ -568,12 +597,9 @@ int ridalloc_allocate_rid(struct ldb_module *module, uint32_t *rid, struct ldb_r * ask async for a new pool. */ ret = ridalloc_new_own_pool(module, &nridset.alloc_pool, parent); - if (ret == LDB_ERR_UNWILLING_TO_PERFORM) { - ridalloc_poke_rid_manager(module); - talloc_free(tmp_ctx); - return ret; - } if (ret != LDB_SUCCESS) { + ldb_asprintf_errstring(ldb, "NO RID values available: %s", + ldb_errstring(ldb)); talloc_free(tmp_ctx); return ret; } @@ -616,7 +642,7 @@ int ridalloc_allocate_rid(struct ldb_module *module, uint32_t *rid, struct ldb_r */ ret = ridalloc_new_own_pool(module, &nridset.alloc_pool, parent); if (ret == LDB_ERR_UNWILLING_TO_PERFORM) { - ridalloc_poke_rid_manager(module); + ldb_reset_err_string(ldb); ret = LDB_SUCCESS; } if (ret != LDB_SUCCESS) { -- 1.7.10.4 From 81418d3d230653a0392927eb1bfc736813788154 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Wed, 19 Jun 2013 11:33:36 +1000 Subject: [PATCH 04/12] selftest: Ensure the DC has started and and got a RID set before we proceed This avoids errors when a busy DC has not yet fetched a RID set, showing up as flapping tests when users are created, such as the samr.large-dc test. Andrew Bartlett Signed-off-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher (cherry picked from commit ae0ba6bd833f71c4337ae3b6621bf797cb3c48c2) --- selftest/target/Samba4.pm | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm index e279beb..e574b48 100644 --- a/selftest/target/Samba4.pm +++ b/selftest/target/Samba4.pm @@ -141,6 +141,7 @@ sub check_or_start($$$) sub wait_for_start($$) { my ($self, $testenv_vars) = @_; + my $ret; # give time for nbt server to register its names print "delaying for nbt name registration\n"; sleep 2; @@ -161,7 +162,25 @@ sub wait_for_start($$) system("$nmblookup $testenv_vars->{CONFIGURATION} $testenv_vars->{NETBIOSNAME}"); system("$nmblookup $testenv_vars->{CONFIGURATION} -U $testenv_vars->{SERVER_IP} $testenv_vars->{NETBIOSNAME}"); + # Ensure we have the first RID Set before we start tests. This makes the tests more reliable. + if ($testenv_vars->{SERVER_ROLE} eq "domain controller" and not ($testenv_vars->{NETBIOS_NAME} eq "rodc")) { + print "waiting for working LDAP and a RID Set to be allocated\n"; + my $ldbsearch = Samba::bindir_path($self, "ldbsearch"); + my $count = 0; + my $base_dn = "DC=".join(",DC=", split(/\./, $testenv_vars->{REALM})); + my $rid_set_dn = "cn=RID Set,cn=$testenv_vars->{NETBIOSNAME},ou=domain controllers,$base_dn"; + while (system("$ldbsearch -H ldap://$testenv_vars->{SERVER} -U$testenv_vars->{USERNAME}%$testenv_vars->{PASSWORD} -s base -b \"$rid_set_dn\" rIDAllocationPool > /dev/null") != 0) { + $count++; + if ($count > 40) { + $ret = 1; + last; + } + sleep(1); + } + } print $self->getlog_env($testenv_vars); + + return $ret } sub write_ldb_file($$$) @@ -692,7 +711,8 @@ nogroup:x:65534:nobody NSS_WRAPPER_WINBIND_SO_PATH => Samba::nss_wrapper_winbind_so_path($self), LOCAL_PATH => $ctx->{share}, UID_RFC2307TEST => $uid_rfc2307test, - GID_RFC2307TEST => $gid_rfc2307test + GID_RFC2307TEST => $gid_rfc2307test, + SERVER_ROLE => $ctx->{server_role} }; return $ret; -- 1.7.10.4 From 0ff54029ef8be4d0905e724bd81c12148e400165 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Fri, 28 Jun 2013 09:15:16 +1000 Subject: [PATCH 05/12] dsdb: Add assert in drepl_take_FSMO_role Pair-Programmed-With: Stefan Metzmacher Signed-off-by: Andrew Bartlett Signed-off-by: Stefan Metzmacher (cherry picked from commit e9faf50ee123a8d1d647ebffa39107ca0dce756c) --- source4/dsdb/repl/drepl_fsmo.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/source4/dsdb/repl/drepl_fsmo.c b/source4/dsdb/repl/drepl_fsmo.c index 37fb684..7a107da 100644 --- a/source4/dsdb/repl/drepl_fsmo.c +++ b/source4/dsdb/repl/drepl_fsmo.c @@ -91,11 +91,10 @@ NTSTATUS drepl_take_FSMO_role(struct irpc_message *msg, extended_op = DRSUAPI_EXOP_FSMO_REQ_PDC; break; default: - DEBUG(2,("Unknown role %u in role transfer\n", + DEBUG(0,("Unknown role %u in role transfer\n", (unsigned)role)); - r->out.result = WERR_DS_DRA_INTERNAL_ERROR; - talloc_free(tmp_ctx); - return NT_STATUS_OK; + /* IRPC messages are trusted, so this really should not happen */ + smb_panic("Unknown role despite dsdb_get_fsmo_role_info success"); } ret = samdb_dn_is_our_ntdsa(service->samdb, role_owner_dn, &is_us); -- 1.7.10.4 From 120e4e6eb997f75de2ef66b008a5def7ac527f61 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Fri, 28 Jun 2013 09:19:48 +1000 Subject: [PATCH 06/12] rpc_server-drsuapi: Improve comments and DEBUG lines Signed-off-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher (cherry picked from commit 5e1f2795f28b0a213b4529e046edec68caa3bd41) --- source4/rpc_server/drsuapi/getncchanges.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c index 5ee87cb..9a405fd 100644 --- a/source4/rpc_server/drsuapi/getncchanges.c +++ b/source4/rpc_server/drsuapi/getncchanges.c @@ -1128,8 +1128,7 @@ static WERROR getncchanges_change_master(struct drsuapi_bind_state *b_state, return WERR_OK; } - /* retrieve the current role owner */ - /* find the DN of the RID Manager */ + /* find the DN of the current role owner */ ret = samdb_reference_dn_is_our_ntdsa(ldb, req_dn, "fSMORoleOwner", &is_us); if (ret != LDB_SUCCESS) { DEBUG(0,("Failed to find fSMORoleOwner in RID Manager object\n")); @@ -1138,8 +1137,8 @@ static WERROR getncchanges_change_master(struct drsuapi_bind_state *b_state, } if (!is_us) { - /* we're not the RID Manager - go away */ - DEBUG(0,(__location__ ": RID Alloc request when not RID Manager\n")); + /* we're not the RID Manager or role owner - go away */ + DEBUG(0,(__location__ ": FSMO role or RID manager transfer owner request when not role owner\n")); ctr6->extended_ret = DRSUAPI_EXOP_ERR_FSMO_NOT_OWNER; return WERR_OK; } -- 1.7.10.4 From f4007afd2a909c9c8ec0e4b603d21625af130441 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Sat, 13 Jul 2013 19:34:45 +1000 Subject: [PATCH 07/12] selftest: ensure samba4.rpc.samr.large-dc.two.samr.many is always tested This test should now be more reliable with the over-allocation of RID values now fixed. Andrew Bartlett Signed-off-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher (cherry picked from commit 93b83151c9563f4c1f47b925fed079d275f8ec43) --- selftest/flapping | 1 - 1 file changed, 1 deletion(-) diff --git a/selftest/flapping b/selftest/flapping index 170bf7b..afeae65 100644 --- a/selftest/flapping +++ b/selftest/flapping @@ -25,4 +25,3 @@ ^samba3.raw.samba3checkfsp.samba3checkfsp\(plugin_s4_dc\) # Seems to flap - succeeds on sn-devel, fails on Fedora 16 ^samba3.raw.samba3closeerr.samba3closeerr\(plugin_s4_dc\) # Seems to flap - succeeds on sn-devel, fails on Fedora 16 ^samba4.nss.test.*using.*winbind # fails sometimes on sn-devel -^samba4.rpc.samr.large-dc.two.samr.many.*\(vampire_dc\) # often fails on sn-devel-104, rid allocation? -- 1.7.10.4 From c7fe33b658d73a2c978d61ba768e2f38a0d9df8a Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 10 Jun 2013 14:00:01 +0200 Subject: [PATCH 08/12] dsdb/samdb: use RECYCLED it implies DELETED... Signed-off-by: Stefan Metzmacher (cherry picked from commit 63c05e820f1449b2dfa6e4f096d8270284a60bbb) --- source4/dsdb/samdb/cracknames.c | 2 +- source4/dsdb/samdb/ldb_modules/linked_attributes.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/source4/dsdb/samdb/cracknames.c b/source4/dsdb/samdb/cracknames.c index 15463a7..0c4cdfc 100644 --- a/source4/dsdb/samdb/cracknames.c +++ b/source4/dsdb/samdb/cracknames.c @@ -945,7 +945,7 @@ static WERROR DsCrackNameOneFilter(struct ldb_context *sam_ctx, TALLOC_CTX *mem_ real_search_dn = NULL; } if (format_desired == DRSUAPI_DS_NAME_FORMAT_GUID){ - dsdb_flags = dsdb_flags| DSDB_SEARCH_SHOW_DELETED; + dsdb_flags |= DSDB_SEARCH_SHOW_RECYCLED; } /* search with the 'phantom root' flag */ diff --git a/source4/dsdb/samdb/ldb_modules/linked_attributes.c b/source4/dsdb/samdb/ldb_modules/linked_attributes.c index eb57f91..63ccbde 100644 --- a/source4/dsdb/samdb/ldb_modules/linked_attributes.c +++ b/source4/dsdb/samdb/ldb_modules/linked_attributes.c @@ -642,7 +642,7 @@ static int linked_attributes_modify(struct ldb_module *module, struct ldb_reques /* We need to figure out our own extended DN, to fill in as the backlink target */ if (ret == LDB_SUCCESS) { ret = dsdb_request_add_controls(search_req, - DSDB_SEARCH_SHOW_DELETED | + DSDB_SEARCH_SHOW_RECYCLED | DSDB_SEARCH_SHOW_EXTENDED_DN); } if (ret == LDB_SUCCESS) { @@ -1000,7 +1000,7 @@ static int la_add_callback(struct ldb_request *req, struct ldb_reply *ares) if (ret == LDB_SUCCESS) { ret = dsdb_request_add_controls(search_req, - DSDB_SEARCH_SHOW_DELETED | + DSDB_SEARCH_SHOW_RECYCLED | DSDB_SEARCH_SHOW_EXTENDED_DN); } if (ret != LDB_SUCCESS) { -- 1.7.10.4 From 0096270819b11cbc24c071507803ada58340296e Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Mon, 17 Jun 2013 22:37:54 +1000 Subject: [PATCH 09/12] torture/drs: Expand an error message to aid debugging Reviewed-by: Stefan Metzmacher Autobuild-User(master): Stefan Metzmacher Autobuild-Date(master): Thu Jul 25 13:51:44 CEST 2013 on sn-devel-104 (cherry picked from commit a74c7d780cb6a1e8a5a63ebbbcf36fd7cf717ea1) --- source4/torture/drs/python/delete_object.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source4/torture/drs/python/delete_object.py b/source4/torture/drs/python/delete_object.py index 0f56fa8..f36232e 100644 --- a/source4/torture/drs/python/delete_object.py +++ b/source4/torture/drs/python/delete_object.py @@ -74,7 +74,7 @@ class DrsDeleteObjectTestCase(drs_base.DrsBaseTestCase): self.assertTrue(not("objectCategory" in user_cur)) self.assertTrue(not("sAMAccountType" in user_cur)) self.assertTrue(dodn in str(user_cur["dn"]), - "User %s is deleted but it is not located under %s!" % (name_orig, dodn)) + "User %s is deleted but it is not located under %s (found at %s)!" % (name_orig, dodn, user_cur["dn"])) self.assertEquals(name_cur, name_orig + "\nDEL:" + guid_str) else: self.assertTrue(not("isDeleted" in user_cur)) -- 1.7.10.4 From 43fd2fe405288a17e144f7ee9d706ed5ea3a46cb Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Fri, 31 May 2013 20:01:17 +1000 Subject: [PATCH 10/12] dsdb: Prune deleted objects of links and extra attributes of replicated deletes When an object is deleted, the links to be removed are not propogated, you have to watch out for them manually! We do this by calling back into the originating update delete code (ie what is called if you ldb_delete() locally) so that any extra attribute found locally and not on the remote server becomes removed remotely too. We currently do the same with links, but that isn't strictly correct, but for now our getNCChanges server code filters these out, so only the usn is bumped. Andrew Bartlett Signed-off-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher (cherry picked from commit d3aad891c5759f66bd891cb47866d908a0562a8a) --- source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 270 +++++++++++++++++------ 1 file changed, 199 insertions(+), 71 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c index b6ff7ff..813438d 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -2,10 +2,10 @@ ldb database library Copyright (C) Simo Sorce 2004-2008 - Copyright (C) Andrew Bartlett 2005 - Copyright (C) Andrew Tridgell 2005 + Copyright (C) Andrew Bartlett 2005-2013 + Copyright (C) Andrew Tridgell 2005-2009 Copyright (C) Stefan Metzmacher 2007 - Copyright (C) Matthieu Patou 2010 + Copyright (C) Matthieu Patou 2010-2011 This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -92,9 +92,12 @@ struct replmd_replicated_request { uint64_t seq_num; bool is_urgent; + + bool isDeleted; }; static int replmd_replicated_apply_merge(struct replmd_replicated_request *ar); +static int replmd_delete_internals(struct ldb_module *module, struct ldb_request *req, bool re_delete); enum urgent_situation { REPL_URGENT_ON_CREATE = 1, @@ -154,7 +157,7 @@ static bool replmd_check_urgent_attribute(const struct ldb_message_element *el) } -static int replmd_replicated_apply_next(struct replmd_replicated_request *ar); +static int replmd_replicated_apply_isDeleted(struct replmd_replicated_request *ar); /* initialise the module @@ -456,10 +459,7 @@ static int replmd_op_callback(struct ldb_request *req, struct ldb_reply *ares) } if (ac->apply_mode) { - talloc_free(ares); - ac->index_current++; - - ret = replmd_replicated_apply_next(ac); + ret = replmd_replicated_apply_isDeleted(ac); if (ret != LDB_SUCCESS) { return ldb_module_done(ac->req, NULL, NULL, ret); } @@ -2735,8 +2735,11 @@ static int replmd_rename_callback(struct ldb_request *req, struct ldb_reply *are } /* - remove links from objects that point at this object when an object - is deleted + * remove links from objects that point at this object when an object + * is deleted. We remove it from the NEXT module per MS-DRSR 5.160 + * RemoveObj which states that link removal due to the object being + * deleted is NOT an originating update - they just go away! + * */ static int replmd_delete_remove_link(struct ldb_module *module, const struct dsdb_schema *schema, @@ -2817,8 +2820,13 @@ static int replmd_delete_remove_link(struct ldb_module *module, This also handles the mapping of delete to a rename operation to allow deletes to be replicated. + + It also handles the incoming deleted objects, to ensure they are + fully deleted here. In that case re_delete is true, and we do not + use this as a signal to change the deleted state, just reinforce it. + */ -static int replmd_delete(struct ldb_module *module, struct ldb_request *req) +static int replmd_delete_internals(struct ldb_module *module, struct ldb_request *req, bool re_delete) { int ret = LDB_ERR_OTHER; bool retb, disallow_move_on_delete; @@ -2861,6 +2869,7 @@ static int replmd_delete(struct ldb_module *module, struct ldb_request *req) schema = dsdb_get_schema(ldb, tmp_ctx); if (!schema) { + talloc_free(tmp_ctx); return LDB_ERR_OPERATIONS_ERROR; } @@ -2874,6 +2883,11 @@ static int replmd_delete(struct ldb_module *module, struct ldb_request *req) DSDB_SEARCH_REVEAL_INTERNALS | DSDB_SEARCH_SHOW_DN_IN_STORAGE_FORMAT, req); if (ret != LDB_SUCCESS) { + ldb_asprintf_errstring(ldb_module_get_ctx(module), + "repmd_delete: Failed to %s %s, because we failed to find it: %s", + re_delete ? "re-delete" : "delete", + ldb_dn_get_linearized(old_dn), + ldb_errstring(ldb_module_get_ctx(module))); talloc_free(tmp_ctx); return ret; } @@ -2882,8 +2896,7 @@ static int replmd_delete(struct ldb_module *module, struct ldb_request *req) ret = dsdb_recyclebin_enabled(module, &enabled); if (ret != LDB_SUCCESS) { - talloc_free(tmp_ctx); - return ret; + enabled = false; } if (ldb_msg_check_string_attribute(old_msg, "isDeleted", "TRUE")) { @@ -2897,7 +2910,13 @@ static int replmd_delete(struct ldb_module *module, struct ldb_request *req) deletion_state = OBJECT_DELETED; next_deletion_state = OBJECT_RECYCLED; } + + /* This supports us noticing an incoming isDeleted and acting on it */ + if (re_delete) { + next_deletion_state = deletion_state; + } } else { + SMB_ASSERT(!re_delete); deletion_state = OBJECT_NOT_DELETED; if (enabled) { next_deletion_state = OBJECT_DELETED; @@ -3006,58 +3025,48 @@ static int replmd_delete(struct ldb_module *module, struct ldb_request *req) see MS-ADTS "Tombstone Requirements" section 3.1.1.5.5.1.1 */ - /* we need the storage form of the parent GUID */ - ret = dsdb_module_search_dn(module, tmp_ctx, &parent_res, - ldb_dn_get_parent(tmp_ctx, old_dn), NULL, - DSDB_FLAG_NEXT_MODULE | - DSDB_SEARCH_SHOW_DN_IN_STORAGE_FORMAT | - DSDB_SEARCH_REVEAL_INTERNALS| - DSDB_SEARCH_SHOW_RECYCLED, req); - if (ret != LDB_SUCCESS) { - talloc_free(tmp_ctx); - return ret; - } - - if (deletion_state == OBJECT_NOT_DELETED){ - ret = ldb_msg_add_steal_string(msg, "lastKnownParent", - ldb_dn_get_extended_linearized(tmp_ctx, parent_res->msgs[0]->dn, 1)); + if (deletion_state == OBJECT_NOT_DELETED) { + /* we need the storage form of the parent GUID */ + ret = dsdb_module_search_dn(module, tmp_ctx, &parent_res, + ldb_dn_get_parent(tmp_ctx, old_dn), NULL, + DSDB_FLAG_NEXT_MODULE | + DSDB_SEARCH_SHOW_DN_IN_STORAGE_FORMAT | + DSDB_SEARCH_REVEAL_INTERNALS| + DSDB_SEARCH_SHOW_RECYCLED, req); if (ret != LDB_SUCCESS) { - DEBUG(0,(__location__ ": Failed to add lastKnownParent string to the msg\n")); - ldb_module_oom(module); + ldb_asprintf_errstring(ldb_module_get_ctx(module), + "repmd_delete: Failed to %s %s, because we failed to find it's parent (%s): %s", + re_delete ? "re-delete" : "delete", + ldb_dn_get_linearized(old_dn), + ldb_dn_get_linearized(ldb_dn_get_parent(tmp_ctx, old_dn)), + ldb_errstring(ldb_module_get_ctx(module))); talloc_free(tmp_ctx); return ret; } - msg->elements[el_count++].flags = LDB_FLAG_MOD_REPLACE; - } - - switch (next_deletion_state){ - case OBJECT_DELETED: - - ret = ldb_msg_add_value(msg, "msDS-LastKnownRDN", rdn_value, NULL); + ret = ldb_msg_add_steal_string(msg, "lastKnownParent", + ldb_dn_get_extended_linearized(tmp_ctx, parent_res->msgs[0]->dn, 1)); if (ret != LDB_SUCCESS) { - DEBUG(0,(__location__ ": Failed to add msDS-LastKnownRDN string to the msg\n")); + DEBUG(0,(__location__ ": Failed to add lastKnownParent string to the msg\n")); ldb_module_oom(module); talloc_free(tmp_ctx); return ret; } - msg->elements[el_count++].flags = LDB_FLAG_MOD_ADD; - - ret = ldb_msg_add_empty(msg, "objectCategory", LDB_FLAG_MOD_REPLACE, NULL); - if (ret != LDB_SUCCESS) { - talloc_free(tmp_ctx); - ldb_module_oom(module); - return ret; - } + msg->elements[el_count++].flags = LDB_FLAG_MOD_REPLACE; - ret = ldb_msg_add_empty(msg, "sAMAccountType", LDB_FLAG_MOD_REPLACE, NULL); - if (ret != LDB_SUCCESS) { - talloc_free(tmp_ctx); - ldb_module_oom(module); - return ret; + if (next_deletion_state == OBJECT_DELETED) { + ret = ldb_msg_add_value(msg, "msDS-LastKnownRDN", rdn_value, NULL); + if (ret != LDB_SUCCESS) { + DEBUG(0,(__location__ ": Failed to add msDS-LastKnownRDN string to the msg\n")); + ldb_module_oom(module); + talloc_free(tmp_ctx); + return ret; + } + msg->elements[el_count++].flags = LDB_FLAG_MOD_ADD; } + } - break; + switch (next_deletion_state) { case OBJECT_RECYCLED: case OBJECT_TOMBSTONE: @@ -3078,7 +3087,7 @@ static int replmd_delete(struct ldb_module *module, struct ldb_request *req) talloc_free(tmp_ctx); return ret; } - msg->elements[el_count++].flags = LDB_FLAG_MOD_ADD; + msg->elements[el_count++].flags = LDB_FLAG_MOD_REPLACE; } /* work out which of the old attributes we will be removing */ @@ -3124,6 +3133,36 @@ static int replmd_delete(struct ldb_module *module, struct ldb_request *req) return ret; } } + + /* Duplicate with the below - we remove the + * samAccountType as an originating update, in case it + * somehow came back. The objectCategory will have + * gone in the above */ + ret = ldb_msg_add_empty(msg, "sAMAccountType", LDB_FLAG_MOD_REPLACE, NULL); + if (ret != LDB_SUCCESS) { + talloc_free(tmp_ctx); + ldb_module_oom(module); + return ret; + } + + break; + + case OBJECT_DELETED: + + ret = ldb_msg_add_empty(msg, "objectCategory", LDB_FLAG_MOD_REPLACE, NULL); + if (ret != LDB_SUCCESS) { + talloc_free(tmp_ctx); + ldb_module_oom(module); + return ret; + } + + ret = ldb_msg_add_empty(msg, "sAMAccountType", LDB_FLAG_MOD_REPLACE, NULL); + if (ret != LDB_SUCCESS) { + talloc_free(tmp_ctx); + ldb_module_oom(module); + return ret; + } + break; default: @@ -3166,6 +3205,11 @@ static int replmd_delete(struct ldb_module *module, struct ldb_request *req) } } + /* + * TODO: Per MS-DRSR 5.160 RemoveObj we should remove links directly, not as an originating update! + * + */ + ret = dsdb_module_modify(module, msg, DSDB_FLAG_OWN_MODULE, req); if (ret != LDB_SUCCESS) { ldb_asprintf_errstring(ldb, "replmd_delete: Failed to modify object %s in delete - %s", @@ -3192,6 +3236,10 @@ static int replmd_delete(struct ldb_module *module, struct ldb_request *req) return ldb_module_done(req, NULL, NULL, LDB_SUCCESS); } +static int replmd_delete(struct ldb_module *module, struct ldb_request *req) +{ + return replmd_delete_internals(module, req, false); +} static int replmd_replicated_request_error(struct replmd_replicated_request *ar, int ret) @@ -3543,7 +3591,6 @@ static int replmd_op_possible_conflict_callback(struct ldb_request *req, struct if (rename_incoming_record) { struct GUID guid; struct ldb_dn *new_dn; - struct ldb_message *new_msg; /* * We want to run the original callback here, which @@ -3580,14 +3627,7 @@ static int replmd_op_possible_conflict_callback(struct ldb_request *req, struct /* re-submit the request, but with a different callback, so we don't loop forever. */ - new_msg = ldb_msg_copy_shallow(req, msg); - if (!new_msg) { - goto failed; - DEBUG(0,(__location__ ": Failed to copy conflict DN message for %s\n", - ldb_dn_get_linearized(conflict_dn))); - } - new_msg->dn = new_dn; - req->op.add.message = new_msg; + msg->dn = new_dn; req->callback = replmd_op_name_modify_callback; return ldb_next_request(ar->module, req); @@ -3761,6 +3801,8 @@ static int replmd_replicated_apply_add(struct replmd_replicated_request *ar) } } + ar->isDeleted = remote_isDeleted; + if (DEBUGLVL(4)) { char *s = ldb_ldif_message_string(ldb, ar, LDB_CHANGETYPE_ADD, msg); DEBUG(4, ("DRS replication add message:\n%s\n", s)); @@ -3907,7 +3949,10 @@ static int replmd_replicated_apply_search_for_parent_callback(struct ldb_request } /* - * Look for the parent object, so we put the new object in the right place + * Look for the parent object, so we put the new object in the right + * place This is akin to NameObject in MS-DRSR - this routine and the + * callbacks find the right parent name, and correct name for this + * object */ static int replmd_replicated_apply_search_for_parent(struct replmd_replicated_request *ar) @@ -4246,25 +4291,32 @@ static int replmd_replicated_apply_merge(struct replmd_replicated_request *ar) } /* + * Work out if this object is deleted, so we can prune any extra attributes. See MS-DRSR 4.1.10.6.9 + * UpdateObject. + * + * This also controls SD propagation below + */ + if (take_remote_isDeleted) { + isDeleted = remote_isDeleted; + } else { + isDeleted = local_isDeleted; + } + + ar->isDeleted = isDeleted; + + /* * check if some replicated attributes left, otherwise skip the ldb_modify() call */ if (msg->num_elements == 0) { ldb_debug(ldb, LDB_DEBUG_TRACE, "replmd_replicated_apply_merge[%u]: skip replace\n", ar->index_current); - ar->index_current++; - return replmd_replicated_apply_next(ar); + return replmd_replicated_apply_isDeleted(ar); } ldb_debug(ldb, LDB_DEBUG_TRACE, "replmd_replicated_apply_merge[%u]: replace %u attributes\n", ar->index_current, msg->num_elements); - if (take_remote_isDeleted) { - isDeleted = remote_isDeleted; - } else { - isDeleted = local_isDeleted; - } - if (renamed) { sd_updated = true; } @@ -4470,6 +4522,7 @@ static int replmd_replicated_apply_next(struct replmd_replicated_request *ar) ldb = ldb_module_get_ctx(ar->module); ar->search_msg = NULL; + ar->isDeleted = false; tmp_str = ldb_binary_encode(ar, ar->objs->objects[ar->index_current].guid_value); if (!tmp_str) return replmd_replicated_request_werror(ar, WERR_NOMEM); @@ -4500,6 +4553,81 @@ static int replmd_replicated_apply_next(struct replmd_replicated_request *ar) return ldb_next_request(ar->module, search_req); } +/* + * This is essentially a wrapper for replmd_replicated_apply_next() + * + * This is needed to ensure that both codepaths call this handler. + */ +static int replmd_replicated_apply_isDeleted(struct replmd_replicated_request *ar) +{ + if (ar->isDeleted) { + /* + * Do a delete here again, so that if there is + * anything local that conflicts with this + * object being deleted, it is removed. This + * includes links. See MS-DRSR 4.1.10.6.9 + * UpdateObject. + * + * If the object is already deleted, and there + * is no more work required, it doesn't do + * anything. + */ + + /* This has been updated to point to the DN we eventually did the modify on */ + struct ldb_message *msg = ar->objs->objects[ar->index_current].msg; + + struct ldb_request *del_req; + struct ldb_result *res; + int ret; + + TALLOC_CTX *tmp_ctx = talloc_new(ar); + if (!tmp_ctx) { + ret = ldb_oom(ldb_module_get_ctx(ar->module)); + return ret; + } + + res = talloc_zero(tmp_ctx, struct ldb_result); + if (!res) { + ret = ldb_oom(ldb_module_get_ctx(ar->module)); + talloc_free(tmp_ctx); + return ret; + } + + /* Build a delete request, which hopefully will artually turn into nothing */ + ret = ldb_build_del_req(&del_req, ldb_module_get_ctx(ar->module), tmp_ctx, + msg->dn, + NULL, + res, + ldb_modify_default_callback, + ar->req); + LDB_REQ_SET_LOCATION(del_req); + if (ret != LDB_SUCCESS) { + talloc_free(tmp_ctx); + return ret; + } + + /* + * This is the guts of the call, call bark + * into our delete code, but setting the + * re_delete flag so we delete anything that + * shouldn't be there on a deleted or recycled + * object + */ + ret = replmd_delete_internals(ar->module, del_req, true); + if (ret == LDB_SUCCESS) { + ret = ldb_wait(del_req->handle, LDB_WAIT_ALL); + } + + talloc_free(tmp_ctx); + if (ret != LDB_SUCCESS) { + return ret; + } + } + + ar->index_current++; + return replmd_replicated_apply_next(ar); +} + static int replmd_replicated_uptodate_modify_callback(struct ldb_request *req, struct ldb_reply *ares) { -- 1.7.10.4 From e5cdaa2932bf3741b0c6668d81a932173c72bada Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 5 Jun 2013 09:35:42 +0200 Subject: [PATCH 11/12] dsdb/repl_meta_data: split out replmd_deletion_state() Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit a796cad90f1028ccc54a3539e34dc0728b990a96) --- source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 102 ++++++++++++++++------- 1 file changed, 71 insertions(+), 31 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c index 813438d..5f15ece 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -105,6 +105,70 @@ enum urgent_situation { REPL_URGENT_ON_DELETE = 4 }; +enum deletion_state { + OBJECT_NOT_DELETED=1, + OBJECT_DELETED=2, + OBJECT_RECYCLED=3, + OBJECT_TOMBSTONE=4, + OBJECT_REMOVED=5 +}; + +static void replmd_deletion_state(struct ldb_module *module, + const struct ldb_message *msg, + enum deletion_state *current_state, + enum deletion_state *next_state) +{ + int ret; + bool enabled = false; + + if (msg == NULL) { + *current_state = OBJECT_REMOVED; + if (next_state != NULL) { + *next_state = OBJECT_REMOVED; + } + return; + } + + ret = dsdb_recyclebin_enabled(module, &enabled); + if (ret != LDB_SUCCESS) { + enabled = false; + } + + if (ldb_msg_check_string_attribute(msg, "isDeleted", "TRUE")) { + if (!enabled) { + *current_state = OBJECT_TOMBSTONE; + if (next_state != NULL) { + *next_state = OBJECT_REMOVED; + } + return; + } + + if (ldb_msg_check_string_attribute(msg, "isRecycled", "TRUE")) { + *current_state = OBJECT_RECYCLED; + if (next_state != NULL) { + *next_state = OBJECT_REMOVED; + } + return; + } + + *current_state = OBJECT_DELETED; + if (next_state != NULL) { + *next_state = OBJECT_RECYCLED; + } + return; + } + + *current_state = OBJECT_NOT_DELETED; + if (next_state == NULL) { + return; + } + + if (enabled) { + *next_state = OBJECT_DELETED; + } else { + *next_state = OBJECT_TOMBSTONE; + } +} static const struct { const char *update_name; @@ -2852,8 +2916,6 @@ static int replmd_delete_internals(struct ldb_module *module, struct ldb_request "trustType", "trustAttributes", "userAccountControl", "uSNChanged", "uSNCreated", "whenCreated", "whenChanged", NULL}; unsigned int i, el_count = 0; - enum deletion_state { OBJECT_NOT_DELETED=1, OBJECT_DELETED=2, OBJECT_RECYCLED=3, - OBJECT_TOMBSTONE=4, OBJECT_REMOVED=5 }; enum deletion_state deletion_state, next_deletion_state; bool enabled; @@ -2893,36 +2955,14 @@ static int replmd_delete_internals(struct ldb_module *module, struct ldb_request } old_msg = res->msgs[0]; + replmd_deletion_state(module, old_msg, + &deletion_state, + &next_deletion_state); - ret = dsdb_recyclebin_enabled(module, &enabled); - if (ret != LDB_SUCCESS) { - enabled = false; - } - - if (ldb_msg_check_string_attribute(old_msg, "isDeleted", "TRUE")) { - if (!enabled) { - deletion_state = OBJECT_TOMBSTONE; - next_deletion_state = OBJECT_REMOVED; - } else if (ldb_msg_check_string_attribute(old_msg, "isRecycled", "TRUE")) { - deletion_state = OBJECT_RECYCLED; - next_deletion_state = OBJECT_REMOVED; - } else { - deletion_state = OBJECT_DELETED; - next_deletion_state = OBJECT_RECYCLED; - } - - /* This supports us noticing an incoming isDeleted and acting on it */ - if (re_delete) { - next_deletion_state = deletion_state; - } - } else { - SMB_ASSERT(!re_delete); - deletion_state = OBJECT_NOT_DELETED; - if (enabled) { - next_deletion_state = OBJECT_DELETED; - } else { - next_deletion_state = OBJECT_TOMBSTONE; - } + /* This supports us noticing an incoming isDeleted and acting on it */ + if (re_delete) { + SMB_ASSERT(deletion_state > OBJECT_NOT_DELETED); + next_deletion_state = deletion_state; } if (next_deletion_state == OBJECT_REMOVED) { -- 1.7.10.4 From 3faa86fe2dd424c8eb3bd466e84d9961a96d9342 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Mon, 17 Jun 2013 22:37:20 +1000 Subject: [PATCH 12/12] dsdb: Ensure we always force deleted objects back under the deleted objects DN Signed-off-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher (cherry picked from commit 32955a1dec3a97ab4550869dbeb5034247f3b1bc) --- source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 91 ++++++++++++++++------- 1 file changed, 65 insertions(+), 26 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c index 5f15ece..591f071 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -2917,7 +2917,6 @@ static int replmd_delete_internals(struct ldb_module *module, struct ldb_request "whenChanged", NULL}; unsigned int i, el_count = 0; enum deletion_state deletion_state, next_deletion_state; - bool enabled; if (ldb_dn_is_special(req->op.del.dn)) { return ldb_next_request(module, req); @@ -2995,45 +2994,57 @@ static int replmd_delete_internals(struct ldb_module *module, struct ldb_request msg->dn = old_dn; - if (deletion_state == OBJECT_NOT_DELETED){ - /* consider the SYSTEM_FLAG_DISALLOW_MOVE_ON_DELETE flag */ - disallow_move_on_delete = - (ldb_msg_find_attr_as_int(old_msg, "systemFlags", 0) - & SYSTEM_FLAG_DISALLOW_MOVE_ON_DELETE); + /* consider the SYSTEM_FLAG_DISALLOW_MOVE_ON_DELETE flag */ + disallow_move_on_delete = + (ldb_msg_find_attr_as_int(old_msg, "systemFlags", 0) + & SYSTEM_FLAG_DISALLOW_MOVE_ON_DELETE); - /* work out where we will be renaming this object to */ - if (!disallow_move_on_delete) { - ret = dsdb_get_deleted_objects_dn(ldb, tmp_ctx, old_dn, - &new_dn); - if (ret != LDB_SUCCESS) { - /* this is probably an attempted delete on a partition - * that doesn't allow delete operations, such as the - * schema partition */ - ldb_asprintf_errstring(ldb, "No Deleted Objects container for DN %s", - ldb_dn_get_linearized(old_dn)); - talloc_free(tmp_ctx); - return LDB_ERR_UNWILLING_TO_PERFORM; - } - } else { + /* work out where we will be renaming this object to */ + if (!disallow_move_on_delete) { + ret = dsdb_get_deleted_objects_dn(ldb, tmp_ctx, old_dn, + &new_dn); + /* + * Deleted Objects itself appears to be deleted, but + * should also not be moved, and we should not move + * objects if we can't find the deleted objects DN + */ + if (re_delete && (ret != LDB_SUCCESS || ldb_dn_compare(old_dn, new_dn) == 0)) { new_dn = ldb_dn_get_parent(tmp_ctx, old_dn); if (new_dn == NULL) { ldb_module_oom(module); talloc_free(tmp_ctx); return LDB_ERR_OPERATIONS_ERROR; } + } else if (ret != LDB_SUCCESS) { + /* this is probably an attempted delete on a partition + * that doesn't allow delete operations, such as the + * schema partition */ + ldb_asprintf_errstring(ldb, "No Deleted Objects container for DN %s", + ldb_dn_get_linearized(old_dn)); + talloc_free(tmp_ctx); + return LDB_ERR_UNWILLING_TO_PERFORM; } + } else { + new_dn = ldb_dn_get_parent(tmp_ctx, old_dn); + if (new_dn == NULL) { + ldb_module_oom(module); + talloc_free(tmp_ctx); + return LDB_ERR_OPERATIONS_ERROR; + } + } + if (deletion_state == OBJECT_NOT_DELETED) { /* get the objects GUID from the search we just did */ guid = samdb_result_guid(old_msg, "objectGUID"); /* Add a formatted child */ retb = ldb_dn_add_child_fmt(new_dn, "%s=%s\\0ADEL:%s", - rdn_name, - ldb_dn_escape_value(tmp_ctx, *rdn_value), - GUID_string(tmp_ctx, &guid)); + rdn_name, + ldb_dn_escape_value(tmp_ctx, *rdn_value), + GUID_string(tmp_ctx, &guid)); if (!retb) { DEBUG(0,(__location__ ": Unable to add a formatted child to dn: %s", - ldb_dn_get_linearized(new_dn))); + ldb_dn_get_linearized(new_dn))); talloc_free(tmp_ctx); return LDB_ERR_OPERATIONS_ERROR; } @@ -3046,6 +3057,30 @@ static int replmd_delete_internals(struct ldb_module *module, struct ldb_request return ret; } msg->elements[el_count++].flags = LDB_FLAG_MOD_REPLACE; + } else { + /* + * No matter what has happened with other renames etc, try again to + * get this to be under the deleted DN. See MS-DRSR 5.160 RemoveObj + */ + + struct ldb_dn *rdn = ldb_dn_copy(tmp_ctx, old_dn); + retb = ldb_dn_remove_base_components(rdn, ldb_dn_get_comp_num(rdn) - 1); + if (!retb) { + DEBUG(0,(__location__ ": Unable to add a prepare rdn of %s", + ldb_dn_get_linearized(rdn))); + talloc_free(tmp_ctx); + return LDB_ERR_OPERATIONS_ERROR; + } + SMB_ASSERT(ldb_dn_get_comp_num(rdn) == 1); + + retb = ldb_dn_add_child(new_dn, rdn); + if (!retb) { + DEBUG(0,(__location__ ": Unable to add rdn %s to base dn: %s", + ldb_dn_get_linearized(rdn), + ldb_dn_get_linearized(new_dn))); + talloc_free(tmp_ctx); + return LDB_ERR_OPERATIONS_ERROR; + } } /* @@ -3258,7 +3293,11 @@ static int replmd_delete_internals(struct ldb_module *module, struct ldb_request return ret; } - if (deletion_state == OBJECT_NOT_DELETED) { + /* + * No matter what has happned with other renames, try again to + * get this to be under the deleted DN. + */ + if (strcmp(ldb_dn_get_linearized(old_dn), ldb_dn_get_linearized(new_dn)) != 0) { /* now rename onto the new DN */ ret = dsdb_module_rename(module, old_dn, new_dn, DSDB_FLAG_NEXT_MODULE, req); if (ret != LDB_SUCCESS){ @@ -4647,7 +4686,7 @@ static int replmd_replicated_apply_isDeleted(struct replmd_replicated_request *a } /* - * This is the guts of the call, call bark + * This is the guts of the call, call back * into our delete code, but setting the * re_delete flag so we delete anything that * shouldn't be there on a deleted or recycled -- 1.7.10.4