Attachment 15281 Details for Bug 14022 – Backport for v4.9

[patch] Backport for v4.9

v4-9-patch.txt (text/plain), 9.24 KB, created by Tim Beale on 2019-07-03 05:42:03 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tim Beale

Created: 2019-07-03 05:42:03 UTC

Size: 9.24 KB

patch

obsolete

>From 4e16281c9fc431ee8c473ab413646be08da3e643 Mon Sep 17 00:00:00 2001
>From: Tim Beale <timbeale@catalyst.net.nz>
>Date: Mon, 1 Jul 2019 14:16:13 +1200
>Subject: [PATCH 1/2] tests: Add getncchanges test for cross-partition links +
> TGT
>
>This adds a test-case to highlight a bug in the client side GetNCChanges
>handling.
>
>These tests mostly exercise the server-side behaviour of sending the
>GetNCChanges, however, there's a bug in the client-side code when we try
>to handle a missing cross-partition link target *in combination* with
>the GET_TGT flag already having been set.
>
>The test is exercising the client-side code by using the 'samba-tool drs
>replicate' command. By adding a one-way link to a deleted target object,
>we force the client code to retry with the GET_TGT flag set.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14022
>
>Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit dba9987bf500f82fbbcda1cd78c543a87f90cec5)
>[conflict in knownfail.d/getncchanges file, merged manually]
>---
> selftest/knownfail.d/getncchanges          |  2 ++
> source4/torture/drs/python/getncchanges.py | 28 +++++++++++++++++++++++++++-
> 2 files changed, 29 insertions(+), 1 deletion(-)
>
>diff --git a/selftest/knownfail.d/getncchanges b/selftest/knownfail.d/getncchanges
>index 5ef1bc9..525b6df 100644
>--- a/selftest/knownfail.d/getncchanges
>+++ b/selftest/knownfail.d/getncchanges
>@@ -4,3 +4,5 @@ samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegri
> samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_repl_get_tgt_chain\(promoted_dc\)
> samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_repl_get_tgt_and_anc\(promoted_dc\)
> samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_repl_get_tgt_multivalued_links\(promoted_dc\)
>+# this fails due to a bug
>+samba4.drs.getncchanges.python.*.getncchanges.DrsReplicaSyncIntegrityTestCase.test_repl_integrity_cross_partition_links_with_tgt
>diff --git a/source4/torture/drs/python/getncchanges.py b/source4/torture/drs/python/getncchanges.py
>index 9136d07..614f650 100644
>--- a/source4/torture/drs/python/getncchanges.py
>+++ b/source4/torture/drs/python/getncchanges.py
>@@ -966,7 +966,7 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
>         self.assert_DCs_replication_is_consistent(peer_conn, all_objects,
>                                                   expected_links)
> 
>-    def test_repl_integrity_cross_partition_links(self):
>+    def _test_repl_integrity_cross_partition_links(self, get_tgt=False):
>         """
>         Checks that a cross-partition link to an unknown target object does
>         not result in missing links.
>@@ -979,6 +979,18 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
>         # stop replication so the peer gets the following objects in one go
>         self._disable_all_repl(self.dnsname_dc2)
> 
>+        # optionally force the client-side to use GET_TGT locally, by adding a
>+        # one-way link to a missing/deleted target object
>+        if get_tgt:
>+            missing_target = "OU=missing_tgt,%s" % self.ou
>+            self.add_object(missing_target)
>+            get_tgt_source = "CN=get_tgt_src,%s" % self.ou
>+            self.add_object(get_tgt_source,
>+                            objectclass="msExchConfigurationContainer")
>+            self.modify_object(get_tgt_source, "addressBookRoots2",
>+                               missing_target)
>+            self.test_ldb_dc.delete(missing_target)
>+
>         # create a link source object in the main NC
>         la_source = "OU=cross_nc_src,%s" % self.ou
>         self.add_object(la_source)
>@@ -1007,6 +1019,14 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
>             self.repl_get_next(get_tgt=True)
> 
>         self.set_dc_connection(self.default_conn)
>+
>+        # delete the GET_TGT test object. We're not interested in asserting its
>+        # links - it was just there to make the client use GET_TGT (and it
>+        # creates an inconsistency because one DC correctly ignores the link,
>+        # because it points to a deleted object)
>+        if get_tgt:
>+            self.test_ldb_dc.delete(get_tgt_source)
>+
>         self.init_test_state()
> 
>         # Now sync across the partition containing the link target object
>@@ -1063,6 +1083,12 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
>         self.test_ldb_dc.delete(la_target)
>         self._enable_all_repl(self.dnsname_dc2)
> 
>+    def test_repl_integrity_cross_partition_links(self):
>+        self._test_repl_integrity_cross_partition_links(get_tgt=False)
>+
>+    def test_repl_integrity_cross_partition_links_with_tgt(self):
>+        self._test_repl_integrity_cross_partition_links(get_tgt=True)
>+
>     def test_repl_get_tgt_multivalued_links(self):
>         """Tests replication with multi-valued link attributes."""
> 
>-- 
>2.7.4
>
>
>From f84f7d1b1e583765ef63617c7920d0d1aefe219e Mon Sep 17 00:00:00 2001
>From: Tim Beale <timbeale@catalyst.net.nz>
>Date: Mon, 1 Jul 2019 13:41:14 +1200
>Subject: [PATCH 2/2] repl_md: Avoid dropping cross-partition links
>
>Cross-partition links could still be dropped if GET_TGT was already
>previously set for the replication.
>
>This was due to a slight error in the order of logic. We never want to
>ignore cross-partition links (regardless of whether the TARGETS_UPTODATE
>/GET_TGT flag is set). We should only be returning early in the
>GET_TGT case if the objects are both in the same partition.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14022
>RN: When the AD domain contained a linked attribute that spanned
>partitions, DRS replication could drop the link. This dropped link could
>then result in subtle differences in behaviour between DCs, as some DCs
>would have the link and others wouldn't. When this issue occurred, the
>dropped link would be logged in a warning message:
> "<target-dn> is Unknown but up to date. Ignoring link from <source-dn>"
>This issue would not always occur - it depended a lot on the database
>contents. Typically, it would only potentially occur when joining a new
>DC to the domain (doing an ldapcmp after the join would also highlight
>the problem, if it occurred). This issue has now been resolved.
>
>Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 98848142cde51d4b280a6fb5cd95dc4bd2471e17)
>[conflict in knownfail.d/getncchanges file, merged manually]
>---
> selftest/knownfail.d/getncchanges               |  3 +--
> source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 31 +++++++++++++------------
> 2 files changed, 17 insertions(+), 17 deletions(-)
>
>diff --git a/selftest/knownfail.d/getncchanges b/selftest/knownfail.d/getncchanges
>index 525b6df..39452cd 100644
>--- a/selftest/knownfail.d/getncchanges
>+++ b/selftest/knownfail.d/getncchanges
>@@ -4,5 +4,4 @@ samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegri
> samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_repl_get_tgt_chain\(promoted_dc\)
> samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_repl_get_tgt_and_anc\(promoted_dc\)
> samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_repl_get_tgt_multivalued_links\(promoted_dc\)
>-# this fails due to a bug
>-samba4.drs.getncchanges.python.*.getncchanges.DrsReplicaSyncIntegrityTestCase.test_repl_integrity_cross_partition_links_with_tgt
>+
>diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>index 04a51ec..648b7f0 100644
>--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>@@ -7230,27 +7230,28 @@ static int replmd_allow_missing_target(struct ldb_module *module,
> 		return LDB_SUCCESS;
> 	}
> 
>-	if (dsdb_repl_flags & DSDB_REPL_FLAG_TARGETS_UPTODATE) {
>+	is_in_same_nc = dsdb_objects_have_same_nc(ldb,
>+						  mem_ctx,
>+						  source_dn,
>+						  target_dn);
>+	if (is_in_same_nc) {
> 
> 		/*
>-		 * target should already be up-to-date so there's no point in
>+		 * if the target is already be up-to-date there's no point in
> 		 * retrying. This could be due to bad timing, or if a target
> 		 * on a one-way link was deleted. We ignore the link rather
> 		 * than failing the replication cycle completely
> 		 */
>-		*ignore_link = true;
>-		DBG_WARNING("%s is %s but up to date. Ignoring link from %s\n",
>-			    ldb_dn_get_linearized(target_dn), missing_str,
>-			    ldb_dn_get_linearized(source_dn));
>-		return LDB_SUCCESS;
>-	}
>-	
>-	is_in_same_nc = dsdb_objects_have_same_nc(ldb,
>-						  mem_ctx,
>-						  source_dn,
>-						  target_dn);
>-	if (is_in_same_nc) {
>-		/* fail the replication and retry with GET_TGT */
>+		if (dsdb_repl_flags & DSDB_REPL_FLAG_TARGETS_UPTODATE) {
>+			*ignore_link = true;
>+			DBG_WARNING("%s is %s "
>+				    "but up to date. Ignoring link from %s\n",
>+				    ldb_dn_get_linearized(target_dn), missing_str,
>+				    ldb_dn_get_linearized(source_dn));
>+			return LDB_SUCCESS;
>+		}
>+
>+		/* otherwise fail the replication and retry with GET_TGT */
> 		ldb_asprintf_errstring(ldb, "%s target %s GUID %s linked from %s\n",
> 				       missing_str,
> 				       ldb_dn_get_linearized(target_dn),
>-- 
>2.7.4
>

Flags:

timbeale: review? (abartlet)

Actions: View

Attachments on bug 14022: 15280 | 15281