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 <abartlet@samba.org>
Reviewed-by: Tim Beale <timbeale@catalyst.net.nz>
Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
Autobuild-Date(master): Tue Oct 30 10:32:51 CET 2018 on sn-devel-144

diff --git a/source4/dsdb/samdb/ldb_modules/linked_attributes.c b/source4/dsdb/samdb/ldb_modules/linked_attributes.c
index 57d25076a7550ac14bd57c64ebc632e3ce5ba83f..2568d4d17900b5ae0b3de20cab61e3b73b5293d3 100644 (file)
--- 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 231390949ac5a5002297d49edc1c1d49d3f91eda..b1f1523ffa242619511ba23c926fba450a5f0165 100644 (file)
--- 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 c2d803e1d15cedd4bfbf25cabd78e1e2ccdc0883..0fbc287114ad40b87b0cd91de82637dd06f64749 100755 (executable)
--- 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`