From 852e1db12b0afa04a738c03bb2609c084fe96a7f Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 30 Oct 2018 15:56:43 +1300 Subject: [PATCH] dsdb: Add comments explaining the limitations of our current backlink behaviour BUG: https://bugzilla.samba.org/show_bug.cgi?id=13418 Signed-off-by: Andrew Bartlett Reviewed-by: Tim Beale Autobuild-User(master): Andrew Bartlett Autobuild-Date(master): Tue Oct 30 10:32:51 CET 2018 on sn-devel-144 --- .../samdb/ldb_modules/linked_attributes.c | 18 +++++++++++++- .../dsdb/samdb/ldb_modules/repl_meta_data.c | 24 ++++++++++++++----- testprogs/blackbox/test_primary_group.sh | 6 ++++- 3 files changed, 40 insertions(+), 8 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/linked_attributes.c b/source4/dsdb/samdb/ldb_modules/linked_attributes.c index 57d25076a75..2568d4d1790 100644 --- a/source4/dsdb/samdb/ldb_modules/linked_attributes.c +++ b/source4/dsdb/samdb/ldb_modules/linked_attributes.c @@ -25,7 +25,23 @@ * * Component: ldb linked_attributes module * - * Description: Module to ensure linked attribute pairs remain in sync + * Description: Module to ensure linked attribute pairs (i.e. forward-links + * and backlinks) remain in sync. + * + * Backlinks are 'plain' links (without extra metadata). When the link target + * object is modified (e.g. renamed), we use the backlinks to keep the link + * source object updated. Note there are some cases where we can't do this: + * - one-way links, which don't have a corresponding backlink + * - two-way deactivated links, i.e. when a user is removed from a group, + * the forward 'member' link still exists (but is inactive), however, the + * 'memberOf' backlink is deleted. + * In these cases, we can end up with a dangling forward link which is + * incorrect (i.e. the target has been renamed or deleted). We have dbcheck + * rules to detect and fix this, and cope otherwise by filtering at runtime + * (i.e. in the extended_dn module). + * + * See also repl_meta_data.c, which handles updating links for deleted + * objects, as well as link changes received from another DC. * * Author: Andrew Bartlett */ diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c index 231390949ac..b1f1523ffa2 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -4378,6 +4378,10 @@ static int replmd_delete_internals(struct ldb_module *module, struct ldb_request - preserved if in above list, or is rDN - remove all linked attribs from this object - remove all links from other objects to this object + (note we use the backlinks to do this, so we won't find one-way + links that still point to this object, or deactivated two-way + links, i.e. 'member' after the user has been removed from the + group) - add lastKnownParent - update replPropertyMetaData? @@ -4515,12 +4519,12 @@ static int replmd_delete_internals(struct ldb_module *module, struct ldb_request if (sa->linkID & 1) { /* - we have a backlink in this object - that needs to be removed. We're not - allowed to remove it directly - however, so we instead setup a - modify to delete the corresponding - forward link + * we have a backlink in this object + * that needs to be removed. We're not + * allowed to remove it directly + * however, so we instead setup a + * modify to delete the corresponding + * forward link */ ret = replmd_delete_remove_link(module, schema, replmd_private, @@ -7670,6 +7674,14 @@ static int replmd_delete_link_value(struct ldb_module *module, /* if the existing link is active, remove its backlink */ if (is_active) { + /* + * NOTE WELL: After this we will never (at runtime) be + * able to find this forward link (for instant + * removal) if/when the link target is deleted. + * + * We have dbcheck rules to cover this and cope otherwise + * by filtering at runtime (i.e. in the extended_dn module). + */ ret = replmd_add_backlink(module, replmd_private, schema, src_obj_dn, target_guid, false, attr, NULL); diff --git a/testprogs/blackbox/test_primary_group.sh b/testprogs/blackbox/test_primary_group.sh index c2d803e1d15..0fbc287114a 100755 --- a/testprogs/blackbox/test_primary_group.sh +++ b/testprogs/blackbox/test_primary_group.sh @@ -75,11 +75,15 @@ testit "delete '$testgroup'" $VALGRIND $PYTHON $BINDIR/samba-tool group delete " # # As we don't support phantom objects and virtual backlinks -# the deletion of the user and group cause dangling links, +# the deletion of the user prior to the group causes dangling links, # which are detected like this: # # WARNING: target DN is deleted for member in object # +# Specifically, this happens because after the member link is +# deactivated the memberOf is gone, and so there is no way to find the +# now redundant forward link to clean it up. +# testit_expect_failure "dbcheck run3" $VALGRIND $PYTHON $BINDIR/samba-tool dbcheck --attrs=member --fix --yes || failed=`expr $failed + 1` testit "dbcheck run4" $VALGRIND $PYTHON $BINDIR/samba-tool dbcheck --attrs=member || failed=`expr $failed + 1` -- 2.34.1