Attachment 18057 Details for Bug 15401 – Patch in master backported to Samba 4.19

[patch] Patch in master backported to Samba 4.19

drs-spin-4.19.patch (text/plain), 59.01 KB, created by Andrew Bartlett on 2023-08-15 19:43:20 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andrew Bartlett

Created: 2023-08-15 19:43:20 UTC

Size: 59.01 KB

patch

obsolete

>From 973431387ead457572362840804fbc5996300425 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Mon, 24 Jul 2023 11:35:45 +1200
>Subject: [PATCH 01/18] s4-rpc_server/drsuapi: Add tmp_highest_usn tracking to
> replication log
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15401
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Stefan Metzmacher <metze@samba.org>
>(cherry picked from commit 0d9ea6c559317e19642662220c089e2d59ef3ecd)
>---
> source4/rpc_server/drsuapi/getncchanges.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
>diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c
>index 74b173c3965..9eb75c8502a 100644
>--- a/source4/rpc_server/drsuapi/getncchanges.c
>+++ b/source4/rpc_server/drsuapi/getncchanges.c
>@@ -3507,9 +3507,10 @@ allowed:
> 			getnc_state->last_dn = talloc_move(getnc_state, &msg->dn);
> 		}
> 
>-		DEBUG(8,(__location__ ": %s object %s\n",
>+		DEBUG(8,(__location__ ": %s object %s new tmp_highest_usn=%" PRIu64 "\n",
> 			 new_objs ? "replicating" : "skipping send of",
>-			 ldb_dn_get_linearized(msg->dn)));
>+			 ldb_dn_get_linearized(msg->dn),
>+			 r->out.ctr->ctr6.new_highwatermark.tmp_highest_usn));
> 
> 		getnc_state->total_links += (getnc_state->la_count - old_la_index);
> 
>-- 
>2.25.1
>
>
>From 564d2409e84cd43ec76e51ee8584f8dcef32a9b9 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Tue, 27 Jun 2023 12:18:24 +1200
>Subject: [PATCH 02/18] s4-rpc_server/drsuapi: Improve debugging of invalid DNs
>
>This is still unreachable, so but improve the logging
>to give more detail in this area anyway.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15401
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Stefan Metzmacher <metze@samba.org>
>(cherry picked from commit d0c1ce53add2fd3b3a4186581f4e214029cbcf1a)
>---
> source4/rpc_server/drsuapi/getncchanges.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
>diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c
>index 9eb75c8502a..d730a1d18f5 100644
>--- a/source4/rpc_server/drsuapi/getncchanges.c
>+++ b/source4/rpc_server/drsuapi/getncchanges.c
>@@ -2977,8 +2977,9 @@ allowed:
> 			 * This can't fail as we have done this above
> 			 * implicitly but not got the DN out
> 			 */
>-			DBG_ERR("Bad DN '%s'\n",
>-				drs_ObjectIdentifier_to_debug_string(mem_ctx, ncRoot));
>+			DBG_ERR("Bad DN '%s' as Naming Context for GetNCChanges: %s\n",
>+				drs_ObjectIdentifier_to_debug_string(mem_ctx, ncRoot),
>+				ldb_strerror(ret));
> 			return WERR_DS_DRA_INVALID_PARAMETER;
> 		}
> 		if (ldb_dn_compare(new_dn, getnc_state->ncRoot_dn) != 0) {
>-- 
>2.25.1
>
>
>From 882c1baf6c59cdbbf5256b13d676ce662fa14c04 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Tue, 27 Jun 2023 17:18:39 +1200
>Subject: [PATCH 03/18] s4-rpc_server/drsuapi: Improve debug message for
> drs_ObjectIdentifier_to_dn_and_nc_root() failure
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15401
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Stefan Metzmacher <metze@samba.org>
>(cherry picked from commit a12bcce89d26ae05bbaeed560cf8fcc7b5bcfdab)
>---
> source4/rpc_server/drsuapi/getncchanges.c | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c
>index d730a1d18f5..977a0b75069 100644
>--- a/source4/rpc_server/drsuapi/getncchanges.c
>+++ b/source4/rpc_server/drsuapi/getncchanges.c
>@@ -3022,6 +3022,9 @@ allowed:
> 							     &ncRoot_dn,
> 							     NULL);
> 		if (ret != LDB_SUCCESS) {
>+			DBG_ERR("Bad DN '%s' as Naming Context or EXOP DN for GetNCChanges: %s\n",
>+				drs_ObjectIdentifier_to_debug_string(mem_ctx, ncRoot),
>+				ldb_strerror(ret));
> 			return WERR_DS_DRA_BAD_DN;
> 		}
> 
>-- 
>2.25.1
>
>
>From cc3a1280fea1952752ed332f23443b1a2f670db1 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Tue, 27 Jun 2023 14:59:49 +1200
>Subject: [PATCH 04/18] s4-dsdb: Improve logging for
> drs_ObjectIdentifier_to_dn_and_nc_root()
>
>At this layer we can make a reasonable assumption about being able
>to read ldb_errstring() to print that for extra useful debugging.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15401
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Stefan Metzmacher <metze@samba.org>
>(cherry picked from commit 63843a22c8db73d459bee61e73bb1f0d31e3d427)
>---
> source4/dsdb/common/dsdb_dn.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
>diff --git a/source4/dsdb/common/dsdb_dn.c b/source4/dsdb/common/dsdb_dn.c
>index c86848fd697..b34d1e6052a 100644
>--- a/source4/dsdb/common/dsdb_dn.c
>+++ b/source4/dsdb/common/dsdb_dn.c
>@@ -554,6 +554,18 @@ int drs_ObjectIdentifier_to_dn_and_nc_root(TALLOC_CTX *mem_ctx,
> 						 new_dn,
> 						 normalised_dn,
> 						 nc_root);
>+	if (ret != LDB_SUCCESS) {
>+		/*
>+		 * dsdb_normalise_dn_and_find_nc_root() sets LDB error
>+		 * strings, and the functions it calls do also
>+		 */
>+		DBG_NOTICE("Failed to find DN \"%s\" -> \"%s\" for normalisation: %s (%s)\n",
>+			   drs_ObjectIdentifier_to_debug_string(mem_ctx, nc),
>+			   ldb_dn_get_extended_linearized(mem_ctx, new_dn, 1),
>+			   ldb_errstring(ldb),
>+			   ldb_strerror(ret));
>+	}
>+
> 	TALLOC_FREE(new_dn);
> 	return ret;
> }
>-- 
>2.25.1
>
>
>From 7d95934ced26df4b01c9c5dccfe7e64d4c926e63 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Tue, 27 Jun 2023 14:22:52 +1200
>Subject: [PATCH 05/18] s4-rpc_server/drsuapi: Remove rudundant check for valid
> and non-NULL ncRoot_dn
>
>This check was valuable before aee2039e63ceeb5e69a0461fb77e0f18278e4dc4
>but now only checks things we know to be true, as the value has come
>from Samba via drs_ObjectIdentifier_to_dn_and_nc_root() either on this
>or a previous call.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15401
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Stefan Metzmacher <metze@samba.org>
>(cherry picked from commit 0550e469eda4022659718ae9a56f5deaa9f9a307)
>---
> source4/rpc_server/drsuapi/getncchanges.c | 7 -------
> 1 file changed, 7 deletions(-)
>
>diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c
>index 977a0b75069..aaf8d8519ab 100644
>--- a/source4/rpc_server/drsuapi/getncchanges.c
>+++ b/source4/rpc_server/drsuapi/getncchanges.c
>@@ -3136,13 +3136,6 @@ allowed:
> 		TALLOC_FREE(res);
> 	}
> 
>-	if (!ldb_dn_validate(getnc_state->ncRoot_dn) ||
>-	    ldb_dn_is_null(getnc_state->ncRoot_dn)) {
>-		DEBUG(0,(__location__ ": Bad DN '%s'\n",
>-			 drs_ObjectIdentifier_to_debug_string(mem_ctx, ncRoot)));
>-		return WERR_DS_DRA_INVALID_PARAMETER;
>-	}
>-
> 	ncRoot->guid = getnc_state->ncRoot_guid;
> 
> 	/* we need the session key for encrypting password attributes */
>-- 
>2.25.1
>
>
>From dfbcb551d726deb343cd7cabcc97ab10d751f071 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Mon, 24 Jul 2023 11:40:46 +1200
>Subject: [PATCH 06/18] s4-torture/drs: Save the server dnsname on the
> DcConnection object
>
>This object is used to hold one of many possible connections and
>it is helpful for debugging and uniqueness to know which DC is being
>connected to.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15401
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Stefan Metzmacher <metze@samba.org>
>(cherry picked from commit c30bb8769ff2c4eba2d8f8a2bd3a56946b7d9d5e)
>---
> source4/torture/drs/python/getncchanges.py | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/source4/torture/drs/python/getncchanges.py b/source4/torture/drs/python/getncchanges.py
>index 1d25c58c51d..b101e9e2a08 100644
>--- a/source4/torture/drs/python/getncchanges.py
>+++ b/source4/torture/drs/python/getncchanges.py
>@@ -1229,3 +1229,4 @@ class DcConnection:
>         (self.drs, self.drs_handle) = drs_base._ds_bind(dnsname_dc)
>         (self.default_hwm, utdv) = drs_base._get_highest_hwm_utdv(ldb_dc)
>         self.default_utdv = utdv
>+        self.dnsname_dc = dnsname_dc
>-- 
>2.25.1
>
>
>From 35b6e1a896515ce5ffda66f1a56c561d86ec6173 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Mon, 24 Jul 2023 11:36:36 +1200
>Subject: [PATCH 07/18] s4-torture/drs: Create temp OU with a unique name per
> test
>
>It is always better to keep the testing OUs unique if possible.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15401
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Stefan Metzmacher <metze@samba.org>
>(cherry picked from commit 628eab11b3c2e82875bf602e363b781d3e5eb96d)
>---
> source4/torture/drs/python/getncchanges.py | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/source4/torture/drs/python/getncchanges.py b/source4/torture/drs/python/getncchanges.py
>index b101e9e2a08..2095bc61a77 100644
>--- a/source4/torture/drs/python/getncchanges.py
>+++ b/source4/torture/drs/python/getncchanges.py
>@@ -49,7 +49,8 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
>         self.set_test_ldb_dc(self.ldb_dc2)
> 
>         self.ou = str(samba.tests.create_test_ou(self.test_ldb_dc,
>-                                                 "getncchanges"))
>+                                                 "getncchanges." + self.id().rsplit(".", 1)[1]))
>+
>         self.base_dn = self.test_ldb_dc.get_default_basedn()
> 
>         self.default_conn = DcConnection(self, self.ldb_dc2, self.dnsname_dc2)
>-- 
>2.25.1
>
>
>From 6a7b947019e27d6e1f31dc30ade8c613ca75f5ff Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Mon, 24 Jul 2023 11:37:19 +1200
>Subject: [PATCH 08/18] s4-torture/drs: Use addCleanup() in getchanges.py for
> OU handling
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15401
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Stefan Metzmacher <metze@samba.org>
>(cherry picked from commit 40f831e67e1f312b1db52c74c119899245d03e32)
>---
> source4/torture/drs/python/getncchanges.py | 12 ++----------
> 1 file changed, 2 insertions(+), 10 deletions(-)
>
>diff --git a/source4/torture/drs/python/getncchanges.py b/source4/torture/drs/python/getncchanges.py
>index 2095bc61a77..46598311f79 100644
>--- a/source4/torture/drs/python/getncchanges.py
>+++ b/source4/torture/drs/python/getncchanges.py
>@@ -51,21 +51,13 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
>         self.ou = str(samba.tests.create_test_ou(self.test_ldb_dc,
>                                                  "getncchanges." + self.id().rsplit(".", 1)[1]))
> 
>+        self.addCleanup(self.ldb_dc2.delete, self.ou, ["tree_delete:1"])
>+
>         self.base_dn = self.test_ldb_dc.get_default_basedn()
> 
>         self.default_conn = DcConnection(self, self.ldb_dc2, self.dnsname_dc2)
>         self.set_dc_connection(self.default_conn)
> 
>-    def tearDown(self):
>-        super(DrsReplicaSyncIntegrityTestCase, self).tearDown()
>-        # tidyup groups and users
>-        try:
>-            self.ldb_dc2.delete(self.ou, ["tree_delete:1"])
>-        except ldb.LdbError as e:
>-            (enum, string) = e.args
>-            if enum == ldb.ERR_NO_SUCH_OBJECT:
>-                pass
>-
>     def init_test_state(self):
>         self.rxd_dn_list = []
>         self.rxd_links = []
>-- 
>2.25.1
>
>
>From 3aec483723efe8fa60b40da082e4cfae009fd16a Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Tue, 27 Jun 2023 12:20:32 +1200
>Subject: [PATCH 09/18] s4-torture/drs: Add a test matching Azure AD Connect
> REPL_OBJ behaviour
>
>Azure AD Connect will send a GUID but no DummyDN.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15401
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Stefan Metzmacher <metze@samba.org>
>(cherry picked from commit db16366b0bbefcdb91a0b36c903ed63456a081b8)
>---
> source4/torture/drs/python/getnc_exop.py | 25 +++++++++++++++++++++++-
> 1 file changed, 24 insertions(+), 1 deletion(-)
>
>diff --git a/source4/torture/drs/python/getnc_exop.py b/source4/torture/drs/python/getnc_exop.py
>index 7121b1ba35f..0f12d9b27d7 100644
>--- a/source4/torture/drs/python/getnc_exop.py
>+++ b/source4/torture/drs/python/getnc_exop.py
>@@ -295,6 +295,29 @@ class DrsReplicaSyncTestCase(drs_base.DrsBaseTestCase):
>             (enum, estr) = e1.args
>             self.assertEqual(enum, werror.WERR_DS_DRA_BAD_NC)
> 
>+    def test_valid_GUID_only_REPL_OBJ(self):
>+        dc_guid_1 = self.ldb_dc1.get_invocation_id()
>+        drs, drs_handle = self._ds_bind(self.dnsname_dc1, ip=self.url_dc1)
>+
>+        res = self.ldb_dc1.search(base=self.ou, scope=SCOPE_BASE,
>+                                  attrs=["objectGUID"])
>+
>+        guid = misc.GUID(res[0]["objectGUID"][0])
>+
>+        req8 = self._exop_req8(dest_dsa=None,
>+                               invocation_id=dc_guid_1,
>+                               nc_dn_str="",
>+                               nc_guid=guid,
>+                               exop=drsuapi.DRSUAPI_EXOP_REPL_OBJ)
>+
>+        try:
>+            (level, ctr) = drs.DsGetNCChanges(drs_handle, 8, req8)
>+        except WERRORError as e1:
>+            (enum, estr) = e1.args
>+            self.fail(f"Failed to call GetNCChanges with EXOP_REPL_OBJ and a GUID: {estr}")
>+
>+        self.assertEqual(ctr.first_object.object.identifier.guid, guid)
>+
>     def test_DummyDN_valid_GUID_REPL_OBJ(self):
>         dc_guid_1 = self.ldb_dc1.get_invocation_id()
>         drs, drs_handle = self._ds_bind(self.dnsname_dc1, ip=self.url_dc1)
>@@ -314,7 +337,7 @@ class DrsReplicaSyncTestCase(drs_base.DrsBaseTestCase):
>             (level, ctr) = drs.DsGetNCChanges(drs_handle, 8, req8)
>         except WERRORError as e1:
>             (enum, estr) = e1.args
>-            self.fail(f"Failed to call GetNCChanges with EXOP_REPL_OBJ and a GUID: {estr}")
>+            self.fail(f"Failed to call GetNCChanges with EXOP_REPL_OBJ, DummyDN and a GUID: {estr}")
> 
>         self.assertEqual(ctr.first_object.object.identifier.guid, guid)
> 
>-- 
>2.25.1
>
>
>From 6018c5e545e7b63d129679546b01422a74869985 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Mon, 26 Jun 2023 16:25:32 +1200
>Subject: [PATCH 10/18] s4-torture/drs: Add test demonstrating that a
> GetNCChanges REPL_OBJ will not reset the replication cookie
>
>This demonstrates the behaviour used by the "Azure AD Connect" cloud sync tool.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15401
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Stefan Metzmacher <metze@samba.org>
>(cherry picked from commit b323169d6ff8357f7c999ae346137166c98218ac)
>---
> selftest/knownfail.d/getncchanges          |  1 +
> source4/torture/drs/python/getncchanges.py | 55 ++++++++++++++++++++++
> 2 files changed, 56 insertions(+)
>
>diff --git a/selftest/knownfail.d/getncchanges b/selftest/knownfail.d/getncchanges
>index 5ef1bc98bef..b033b091c72 100644
>--- a/selftest/knownfail.d/getncchanges
>+++ b/selftest/knownfail.d/getncchanges
>@@ -4,3 +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\)
>+samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_do_full_repl_mix_no_overlap
>\ No newline at end of file
>diff --git a/source4/torture/drs/python/getncchanges.py b/source4/torture/drs/python/getncchanges.py
>index 46598311f79..5b402e81c88 100644
>--- a/source4/torture/drs/python/getncchanges.py
>+++ b/source4/torture/drs/python/getncchanges.py
>@@ -1213,6 +1213,61 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
>         # The NC should be the first object returned due to GET_ANC
>         self.assertEqual(ctr.first_object.object.identifier.guid, guid)
> 
>+    def _test_do_full_repl_no_overlap(self, mix=True, get_anc=False):
>+        self.default_hwm = drsuapi.DsReplicaHighWaterMark()
>+
>+        # We set get_anc=True so we can assert the BASE DN will be the
>+        # first object
>+        ctr6 = self._repl_send_request(get_anc=get_anc)
>+        guid_list_1 = self._get_ctr6_object_guids(ctr6)
>+
>+        if mix:
>+            dc_guid_1 = self.ldb_dc1.get_invocation_id()
>+
>+            req8 = self._exop_req8(dest_dsa=None,
>+                                   invocation_id=dc_guid_1,
>+                                   nc_dn_str=self.ldb_dc1.get_default_basedn(),
>+                                   exop=drsuapi.DRSUAPI_EXOP_REPL_OBJ)
>+
>+            (level, ctr_repl_obj) = self.drs.DsGetNCChanges(self.drs_handle, 8, req8)
>+
>+            self.assertEqual(ctr_repl_obj.extended_ret, drsuapi.DRSUAPI_EXOP_ERR_SUCCESS)
>+
>+            repl_obj_guid_list = self._get_ctr6_object_guids(ctr_repl_obj)
>+
>+            self.assertEqual(len(repl_obj_guid_list), 1)
>+
>+            # This should be the first object in the main replication due
>+            # to get_anc=True above in one case, and a rule that the NC must be first regardless otherwise
>+            self.assertEqual(repl_obj_guid_list[0], guid_list_1[0])
>+
>+        self.last_ctr = ctr6
>+        ctr6 = self._repl_send_request(get_anc=True)
>+        guid_list_2 = self._get_ctr6_object_guids(ctr6)
>+
>+        self.assertNotEqual(guid_list_1, guid_list_2)
>+
>+    def test_do_full_repl_no_overlap_get_anc(self):
>+        """
>+        Make sure that a full replication on an nc succeeds to the goal despite needing multiple passes
>+        """
>+        self._test_do_full_repl_no_overlap(mix=False, get_anc=True)
>+
>+    def test_do_full_repl_no_overlap(self):
>+        """
>+        Make sure that a full replication on an nc succeeds to the goal despite needing multiple passes
>+        """
>+        self._test_do_full_repl_no_overlap(mix=False)
>+
>+    def test_do_full_repl_mix_no_overlap(self):
>+        """
>+        Make sure that a full replication on an nc succeeds to the goal despite needing multiple passes
>+
>+        Assert this is true even if we do a REPL_OBJ in between the replications
>+
>+        """
>+        self._test_do_full_repl_no_overlap(mix=True)
>+
> 
> class DcConnection:
>     """Helper class to track a connection to another DC"""
>-- 
>2.25.1
>
>
>From d418a20d83747f937bac3cb63aae3fc7b144a446 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Mon, 24 Jul 2023 12:05:18 +1200
>Subject: [PATCH 11/18] s4-torture/drs: Add test showing that if present in the
> set the NC root leads and tmp_highest_usn moves
>
>The NC root, on any replication when it appears, is the first object to be
>replicated, including for all subsequent chunks in the replication.
>
>However the tmp_highest_usn is not updated by that USN, it must
>only be updated for the non-NC changes (to match Windows exactly),
>or at least only updated with the non-NC changes until it would
>naturally appear.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15401
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Stefan Metzmacher <metze@samba.org>
>(cherry picked from commit 87414955212143b8502b4c02aca150bc72cb8de5)
>---
> selftest/knownfail.d/getncchanges          |   7 +-
> source4/torture/drs/python/getncchanges.py | 147 +++++++++++++++++++++
> 2 files changed, 153 insertions(+), 1 deletion(-)
>
>diff --git a/selftest/knownfail.d/getncchanges b/selftest/knownfail.d/getncchanges
>index b033b091c72..e9a86ca2ad2 100644
>--- a/selftest/knownfail.d/getncchanges
>+++ b/selftest/knownfail.d/getncchanges
>@@ -4,4 +4,9 @@ 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\)
>-samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_do_full_repl_mix_no_overlap
>\ No newline at end of file
>+samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_do_full_repl_mix_no_overlap
>+samba4.drs.getncchanges.python\(.*\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_repl_nc_is_first_nc_change_only\(
>+samba4.drs.getncchanges.python\(.*\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_repl_nc_is_first\(
>+samba4.drs.getncchanges.python\(.*\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_repl_nc_is_first_mid\(
>+samba4.drs.getncchanges.python\(.*\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_repl_nc_is_first_start_zero\(
>+samba4.drs.getncchanges.python\(.*\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_repl_nc_is_first_start_zero_nc_change\(
>diff --git a/source4/torture/drs/python/getncchanges.py b/source4/torture/drs/python/getncchanges.py
>index 5b402e81c88..580d8cc66f3 100644
>--- a/source4/torture/drs/python/getncchanges.py
>+++ b/source4/torture/drs/python/getncchanges.py
>@@ -1268,6 +1268,153 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
>         """
>         self._test_do_full_repl_no_overlap(mix=True)
> 
>+    def nc_change(self):
>+        old_base_msg = self.default_conn.ldb_dc.search(base=self.base_dn,
>+                                                       scope=SCOPE_BASE,
>+                                                       attrs=["oEMInformation"])
>+        rec_cleanup = {"dn": self.base_dn,
>+                       "oEMInformation": old_base_msg[0]["oEMInformation"][0]}
>+        m_cleanup = ldb.Message.from_dict(self.default_conn.ldb_dc,
>+                                          rec_cleanup,
>+                                          ldb.FLAG_MOD_REPLACE)
>+
>+        self.addCleanup(self.default_conn.ldb_dc.modify, m_cleanup)
>+
>+        rec = {"dn": self.base_dn,
>+               "oEMInformation": f"Tortured by Samba's getncchanges.py {self.id()} against {self.default_conn.dnsname_dc}"}
>+        m = ldb.Message.from_dict(self.default_conn.ldb_dc, rec, ldb.FLAG_MOD_REPLACE)
>+        self.default_conn.ldb_dc.modify(m)
>+
>+    def _test_repl_nc_is_first(self, start_at_zero=True, nc_change=True, ou_change=True, mid_change=False):
>+        """Tests that the NC is always replicated first, but does not move the
>+           tmp_highest_usn at that point, just like 'early' GET_ANC objects.
>+        """
>+
>+        # create objects, twice more than the page size of 133
>+        objs = self.create_object_range(0, 300, prefix="obj")
>+
>+        if nc_change:
>+            self.nc_change()
>+
>+        if mid_change:
>+            # create even moire objects
>+            objs = self.create_object_range(301, 450, prefix="obj2")
>+
>+        base_msg = self.default_conn.ldb_dc.search(base=self.base_dn,
>+                                                   scope=SCOPE_BASE,
>+                                                   attrs=["uSNChanged",
>+                                                          "objectGUID"])
>+
>+        base_guid = misc.GUID(base_msg[0]["objectGUID"][0])
>+        base_usn = int(base_msg[0]["uSNChanged"][0])
>+
>+        if ou_change:
>+            # Make one more modification.  We want to assert we have
>+            # caught up to the base DN, but Windows both promotes the NC
>+            # to the front and skips including it in the tmp_highest_usn,
>+            # so we make a later modification that will be to show we get
>+            # this change.
>+            rec = {"dn": self.ou,
>+                   "postalCode": "0"}
>+            m = ldb.Message.from_dict(self.default_conn.ldb_dc, rec, ldb.FLAG_MOD_REPLACE)
>+            self.default_conn.ldb_dc.modify(m)
>+
>+        ou_msg = self.default_conn.ldb_dc.search(base=self.ou,
>+                                                   scope=SCOPE_BASE,
>+                                                   attrs=["uSNChanged",
>+                                                          "objectGUID"])
>+
>+        ou_guid = misc.GUID(ou_msg[0]["objectGUID"][0])
>+        ou_usn = int(ou_msg[0]["uSNChanged"][0])
>+
>+        # Check some predicates about USN ordering that the below tests will rely on
>+        if ou_change and nc_change:
>+            self.assertGreater(ou_usn, base_usn);
>+        elif not ou_change and nc_change:
>+            self.assertGreater(base_usn, ou_usn);
>+
>+        ctr6 = self.repl_get_next()
>+
>+        guid_list_1 = self._get_ctr6_object_guids(ctr6)
>+        if nc_change or start_at_zero:
>+            self.assertEqual(base_guid, misc.GUID(guid_list_1[0]))
>+            self.assertIn(str(base_guid), guid_list_1)
>+            self.assertNotIn(str(base_guid), guid_list_1[1:])
>+        else:
>+            self.assertNotEqual(base_guid, misc.GUID(guid_list_1[0]))
>+            self.assertNotIn(str(base_guid), guid_list_1)
>+
>+        self.assertTrue(ctr6.more_data)
>+
>+        if not ou_change and nc_change:
>+            self.assertLess(ctr6.new_highwatermark.tmp_highest_usn, base_usn)
>+
>+        i = 0
>+        while not self.replication_complete():
>+            i = i + 1
>+            last_tmp_highest_usn = ctr6.new_highwatermark.tmp_highest_usn
>+            ctr6 = self.repl_get_next()
>+            guid_list_2 = self._get_ctr6_object_guids(ctr6)
>+            if len(guid_list_2) > 0:
>+                self.assertNotEqual(last_tmp_highest_usn, ctr6.new_highwatermark.tmp_highest_usn)
>+
>+            if (nc_change or start_at_zero) and base_usn > last_tmp_highest_usn:
>+                self.assertEqual(base_guid, misc.GUID(guid_list_2[0]),
>+                                 f"pass={i} more_data={ctr6.more_data} base_usn={base_usn} tmp_highest_usn={ctr6.new_highwatermark.tmp_highest_usn} last_tmp_highest_usn={last_tmp_highest_usn}")
>+                self.assertIn(str(base_guid), guid_list_2,
>+                                 f"pass {i}Â·more_data={ctr6.more_data} base_usn={base_usn} tmp_highest_usn={ctr6.new_highwatermark.tmp_highest_usn} last_tmp_highest_usn={last_tmp_highest_usn}")
>+            else:
>+                self.assertNotIn(str(base_guid), guid_list_2,
>+                                 f"pass {i}Â·more_data={ctr6.more_data} base_usn={base_usn} tmp_highest_usn={ctr6.new_highwatermark.tmp_highest_usn} last_tmp_highest_usn={last_tmp_highest_usn}")
>+
>+        if ou_change:
>+            # The modification to the base OU should be in the final chunk
>+            self.assertIn(str(ou_guid), guid_list_2)
>+            self.assertGreaterEqual(ctr6.new_highwatermark.highest_usn,
>+                                    ou_usn)
>+        else:
>+            # Show that the NC root change does not show up in the
>+            # highest_usn.  We either get the change before or after
>+            # it.
>+            self.assertNotEqual(ctr6.new_highwatermark.highest_usn,
>+                                base_usn)
>+        self.assertEqual(ctr6.new_highwatermark.highest_usn,
>+                          ctr6.new_highwatermark.tmp_highest_usn)
>+
>+        self.assertFalse(ctr6.more_data)
>+
>+    def test_repl_nc_is_first_start_zero_nc_change(self):
>+        self.default_hwm = drsuapi.DsReplicaHighWaterMark()
>+        self._test_repl_nc_is_first(start_at_zero=True, nc_change=True, ou_change=True)
>+
>+    def test_repl_nc_is_first_start_zero(self):
>+        # Get the NC change in the middle of the replication stream, certainly not at the start or end
>+        self.nc_change()
>+        self.default_hwm = drsuapi.DsReplicaHighWaterMark()
>+        self._test_repl_nc_is_first(start_at_zero=True, nc_change=False, ou_change=False)
>+
>+    def test_repl_nc_is_first_mid(self):
>+        # This is a modification of the next test, that Samba
>+        # will pass as it will always include the NC in the
>+        # tmp_highest_usn at the point where it belongs
>+        self._test_repl_nc_is_first(start_at_zero=False,
>+                                    nc_change=True,
>+                                    ou_change=True,
>+                                    mid_change=True)
>+
>+    def test_repl_nc_is_first(self):
>+        # This is a modification of the next test, that Samba
>+        # will pass as it will always include the NC in the
>+        # tmp_highest_usn at the point where it belongs
>+        self._test_repl_nc_is_first(start_at_zero=False, nc_change=True, ou_change=True)
>+
>+    def test_repl_nc_is_first_nc_change_only(self):
>+        # This shows that the NC change is not reflected in the tmp_highest_usn
>+        self._test_repl_nc_is_first(start_at_zero=False, nc_change=True, ou_change=False)
>+
>+    def test_repl_nc_is_first_no_change(self):
>+        # The NC should not be present in this replication
>+        self._test_repl_nc_is_first(start_at_zero=False, nc_change=False, ou_change=False)
> 
> class DcConnection:
>     """Helper class to track a connection to another DC"""
>-- 
>2.25.1
>
>
>From 6518327aec153aabe4680d5fee4f86000971a120 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Mon, 26 Jun 2023 16:53:10 +1200
>Subject: [PATCH 12/18] s4-rpc_server/drsuapi: Only keep and invalidate
> replication cycle state for normal replication
>
>This changes the GetNCChanges server to use a per-call state for
>extended operations like RID_ALLOC or REPL_OBJ and only maintain
>and (more importantly) invalidate the state during normal replication.
>
>This allows REPL_OBJ to be called during a normal replication cycle
>that continues using after that call, continuing with the same
>highwatermark cookie.
>
>Azure AD will do a sequence of (roughly)
>
>* Normal replication (objects 1..100)
>* REPL_OBJ (of 1 object)
>* Normal replication (objects 101..200)
>
>However, if there are more than 100 (in this example) objects in the
>domain, and the second replication is required, the objects 1..100
>are sent, as the replication state was invalidated by the REPL_OBJ call.
>
>RN: Improve GetNChanges to address some (but not all "Azure AD Connect")
>syncronisation tool looping during the initial user sync phase.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15401
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Stefan Metzmacher <metze@samba.org>
>(cherry picked from commit 99579e706312192f46df33d55949db7f1475d0d0)
>---
> selftest/knownfail.d/getncchanges           |   1 -
> source4/rpc_server/drsuapi/dcesrv_drsuapi.h |   2 +-
> source4/rpc_server/drsuapi/getncchanges.c   | 103 ++++++++++++++++----
> 3 files changed, 84 insertions(+), 22 deletions(-)
>
>diff --git a/selftest/knownfail.d/getncchanges b/selftest/knownfail.d/getncchanges
>index e9a86ca2ad2..e92c86cb306 100644
>--- a/selftest/knownfail.d/getncchanges
>+++ b/selftest/knownfail.d/getncchanges
>@@ -4,7 +4,6 @@ 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\)
>-samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_do_full_repl_mix_no_overlap
> samba4.drs.getncchanges.python\(.*\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_repl_nc_is_first_nc_change_only\(
> samba4.drs.getncchanges.python\(.*\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_repl_nc_is_first\(
> samba4.drs.getncchanges.python\(.*\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_repl_nc_is_first_mid\(
>diff --git a/source4/rpc_server/drsuapi/dcesrv_drsuapi.h b/source4/rpc_server/drsuapi/dcesrv_drsuapi.h
>index 38375b5ce58..18ac997583c 100644
>--- a/source4/rpc_server/drsuapi/dcesrv_drsuapi.h
>+++ b/source4/rpc_server/drsuapi/dcesrv_drsuapi.h
>@@ -35,7 +35,7 @@ struct drsuapi_bind_state {
> 	struct GUID remote_bind_guid;
> 	struct drsuapi_DsBindInfoCtr *remote_info;
> 	struct drsuapi_DsBindInfoCtr *local_info;
>-	struct drsuapi_getncchanges_state *getncchanges_state;
>+	struct drsuapi_getncchanges_state *getncchanges_full_repl_state;
> };
> 
> 
>diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c
>index aaf8d8519ab..15e959f1639 100644
>--- a/source4/rpc_server/drsuapi/getncchanges.c
>+++ b/source4/rpc_server/drsuapi/getncchanges.c
>@@ -1711,6 +1711,7 @@ static const char *collect_objects_attrs[] = { "uSNChanged",
>  */
> static WERROR getncchanges_collect_objects(struct drsuapi_bind_state *b_state,
> 					   TALLOC_CTX *mem_ctx,
>+					   struct drsuapi_getncchanges_state *getnc_state,
> 					   struct drsuapi_DsGetNCChangesRequest10 *req10,
> 					   struct ldb_dn *search_dn,
> 					   const char *extra_filter,
>@@ -1719,7 +1720,6 @@ static WERROR getncchanges_collect_objects(struct drsuapi_bind_state *b_state,
> 	int ret;
> 	char* search_filter;
> 	enum ldb_scope scope = LDB_SCOPE_SUBTREE;
>-	struct drsuapi_getncchanges_state *getnc_state = b_state->getncchanges_state;
> 	bool critical_only = false;
> 
> 	if (req10->replica_flags & DRSUAPI_DRS_CRITICAL_ONLY) {
>@@ -1773,6 +1773,7 @@ static WERROR getncchanges_collect_objects(struct drsuapi_bind_state *b_state,
>  */
> static WERROR getncchanges_collect_objects_exop(struct drsuapi_bind_state *b_state,
> 						TALLOC_CTX *mem_ctx,
>+						struct drsuapi_getncchanges_state *getnc_state,
> 						struct drsuapi_DsGetNCChangesRequest10 *req10,
> 						struct drsuapi_DsGetNCChangesCtr6 *ctr6,
> 						struct ldb_dn *search_dn,
>@@ -1932,7 +1933,13 @@ static WERROR getncchanges_collect_objects_exop(struct drsuapi_bind_state *b_sta
> 		/* TODO: implement extended op specific collection
> 		 * of objects. Right now we just normal procedure
> 		 * for collecting objects */
>-		return getncchanges_collect_objects(b_state, mem_ctx, req10, search_dn, extra_filter, search_res);
>+		return getncchanges_collect_objects(b_state,
>+						    mem_ctx,
>+						    getnc_state,
>+						    req10,
>+						    search_dn,
>+						    extra_filter,
>+						    search_res);
> 	}
> }
> 
>@@ -2712,7 +2719,7 @@ WERROR dcesrv_drsuapi_DsGetNCChanges(struct dcesrv_call_state *dce_call, TALLOC_
> 	WERROR werr;
> 	struct dcesrv_handle *h;
> 	struct drsuapi_bind_state *b_state;
>-	struct drsuapi_getncchanges_state *getnc_state;
>+	struct drsuapi_getncchanges_state *getnc_state = NULL;
> 	struct drsuapi_DsGetNCChangesRequest10 *req10;
> 	uint32_t options;
> 	uint32_t link_count = 0;
>@@ -2962,7 +2969,31 @@ allowed:
> 		ZERO_STRUCT(req10->highwatermark);
> 	}
> 
>-	getnc_state = b_state->getncchanges_state;
>+	/*
>+	 * An extended operation is "special single-response cycle"
>+	 * per MS-DRSR 4.1.10.1.1 "Start and Finish" so we don't need
>+	 * to guess if this is a continuation of any long-term
>+	 * state.
>+	 *
>+	 * Otherwise, maintain (including marking as stale, which is
>+	 * what the below is for) the replication state.
>+	 *
>+	 * Note that point 5 "The server implementation MAY declare
>+	 * the supplied values ... as too stale to use."  would allow
>+	 * resetting the state at almost any point, Microsoft Azure AD
>+	 * Connect will switch back and forth between a REPL_OBJ and a
>+	 * full replication, so we must not reset the statue during
>+	 * extended operations.
>+	 */
>+	if (req10->extended_op == DRSUAPI_EXOP_NONE &&
>+	    b_state->getncchanges_full_repl_state != NULL) {
>+		/*
>+		 * Knowing that this is not an extended operation, we
>+		 * can access (and validate) the full replication
>+		 * state
>+		 */
>+		getnc_state = b_state->getncchanges_full_repl_state;
>+	}
> 
> 	/* see if a previous replication has been abandoned */
> 	if (getnc_state) {
>@@ -2975,7 +3006,9 @@ allowed:
> 		if (ret != LDB_SUCCESS) {
> 			/*
> 			 * This can't fail as we have done this above
>-			 * implicitly but not got the DN out
>+			 * implicitly but not got the DN out, but
>+			 * print a good error message regardless just
>+			 * in case.
> 			 */
> 			DBG_ERR("Bad DN '%s' as Naming Context for GetNCChanges: %s\n",
> 				drs_ObjectIdentifier_to_debug_string(mem_ctx, ncRoot),
>@@ -2988,7 +3021,7 @@ allowed:
> 				 ldb_dn_get_linearized(getnc_state->ncRoot_dn),
> 				 ldb_dn_get_linearized(getnc_state->last_dn)));
> 			TALLOC_FREE(getnc_state);
>-			b_state->getncchanges_state = NULL;
>+			b_state->getncchanges_full_repl_state = NULL;
> 		}
> 	}
> 
>@@ -3002,10 +3035,15 @@ allowed:
> 				 (ret > 0) ? "older" : "newer",
> 				 ldb_dn_get_linearized(getnc_state->last_dn)));
> 			TALLOC_FREE(getnc_state);
>-			b_state->getncchanges_state = NULL;
>+			b_state->getncchanges_full_repl_state = NULL;
> 		}
> 	}
> 
>+	 /*
>+	  * This is either a new replication cycle, or an extended
>+	  * operation.  A new cycle is triggered above by the
>+	  * TALLOC_FREE() which sets getnc_state to NULL.
>+	  */
> 	if (getnc_state == NULL) {
> 		struct ldb_result *res = NULL;
> 		const char *attrs[] = {
>@@ -3114,12 +3152,33 @@ allowed:
> 			return WERR_DS_DRA_NOT_SUPPORTED;
> 		}
> 
>-		/* Initialize the state we'll store over the replication cycle */
>-		getnc_state = talloc_zero(b_state, struct drsuapi_getncchanges_state);
>+		/*
>+		 * Initialize the state, initially for the remainder
>+		 * of this call (EXOPs)
>+		 *
>+		 * An extended operation is a "special single-response
>+		 * cycle" per MS-DRSR 4.1.10.1.1 "Start and Finish"
>+		 *
>+		 */
>+		getnc_state = talloc_zero(mem_ctx, struct drsuapi_getncchanges_state);
> 		if (getnc_state == NULL) {
> 			return WERR_NOT_ENOUGH_MEMORY;
> 		}
>-		b_state->getncchanges_state = getnc_state;
>+
>+		if (req10->extended_op == DRSUAPI_EXOP_NONE) {
>+			/*
>+			 * Promote the memory to being a store of
>+			 * long-term state that we will use over the
>+			 * replication cycle for full replication
>+			 * requests
>+			 *
>+			 * Store the state in a clearly named location
>+			 * for pulling back only during full
>+			 * replications
>+			 */
>+			b_state->getncchanges_full_repl_state
>+				= talloc_steal(b_state, getnc_state);
>+		}
> 
> 		getnc_state->ncRoot_dn = ncRoot_dn;
> 		talloc_steal(getnc_state, ncRoot_dn);
>@@ -3200,11 +3259,13 @@ allowed:
> 		}
> 
> 		if (req10->extended_op == DRSUAPI_EXOP_NONE) {
>-			werr = getncchanges_collect_objects(b_state, mem_ctx, req10,
>+			werr = getncchanges_collect_objects(b_state, mem_ctx,
>+							    getnc_state, req10,
> 							    search_dn, extra_filter,
> 							    &search_res);
> 		} else {
>-			werr = getncchanges_collect_objects_exop(b_state, mem_ctx, req10,
>+			werr = getncchanges_collect_objects_exop(b_state, mem_ctx,
>+								 getnc_state, req10,
> 								 &r->out.ctr->ctr6,
> 								 search_dn, extra_filter,
> 								 &search_res);
>@@ -3656,7 +3717,11 @@ allowed:
> 		r->out.ctr->ctr6.nc_linked_attributes_count = getnc_state->total_links;
> 	}
> 
>-	if (!r->out.ctr->ctr6.more_data) {
>+	if (req10->extended_op != DRSUAPI_EXOP_NONE) {
>+		r->out.ctr->ctr6.uptodateness_vector = NULL;
>+		r->out.ctr->ctr6.nc_object_count = 0;
>+		ZERO_STRUCT(r->out.ctr->ctr6.new_highwatermark);
>+	} else if (!r->out.ctr->ctr6.more_data) {
> 
> 		/* this is the last response in the replication cycle */
> 		r->out.ctr->ctr6.new_highwatermark = getnc_state->final_hwm;
>@@ -3668,9 +3733,13 @@ allowed:
> 		 * that the RPC message we're sending contains links stored in
> 		 * getnc_state. mem_ctx is local to this RPC call, so the memory
> 		 * will get freed after the RPC message is sent on the wire.
>+		 *
>+		 * We must not do this for an EXOP, as that should not
>+		 * end the replication state, which is why that is
>+		 * checked first above.
> 		 */
> 		talloc_steal(mem_ctx, getnc_state);
>-		b_state->getncchanges_state = NULL;
>+		b_state->getncchanges_full_repl_state = NULL;
> 	} else {
> 		ret = drsuapi_DsReplicaHighWaterMark_cmp(&r->out.ctr->ctr6.old_highwatermark,
> 							 &r->out.ctr->ctr6.new_highwatermark);
>@@ -3692,12 +3761,6 @@ allowed:
> 		getnc_state->last_hwm = r->out.ctr->ctr6.new_highwatermark;
> 	}
> 
>-	if (req10->extended_op != DRSUAPI_EXOP_NONE) {
>-		r->out.ctr->ctr6.uptodateness_vector = NULL;
>-		r->out.ctr->ctr6.nc_object_count = 0;
>-		ZERO_STRUCT(r->out.ctr->ctr6.new_highwatermark);
>-	}
>-
> 	TALLOC_FREE(repl_chunk);
> 
> 	DEBUG(r->out.ctr->ctr6.more_data?4:2,
>-- 
>2.25.1
>
>
>From b731853f6ea3db68242972fa0cc893f6bf8079cc Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Tue, 27 Jun 2023 17:06:13 +1200
>Subject: [PATCH 13/18] s4-rpc_server/drsuapi: Fix indentation in
> GetNCChanges()
>
>This avoids the indentation correction being in the previous patch.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15401
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Stefan Metzmacher <metze@samba.org>
>(cherry picked from commit fe7418e1765b79f60945b787536b4d84a548fe02)
>---
> source4/rpc_server/drsuapi/getncchanges.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c
>index 15e959f1639..92fa7ba7575 100644
>--- a/source4/rpc_server/drsuapi/getncchanges.c
>+++ b/source4/rpc_server/drsuapi/getncchanges.c
>@@ -3726,7 +3726,7 @@ allowed:
> 		/* this is the last response in the replication cycle */
> 		r->out.ctr->ctr6.new_highwatermark = getnc_state->final_hwm;
> 		r->out.ctr->ctr6.uptodateness_vector = talloc_move(mem_ctx,
>-							&getnc_state->final_udv);
>+								   &getnc_state->final_udv);
> 
> 		/*
> 		 * Free the state info stored for the replication cycle. Note
>-- 
>2.25.1
>
>
>From 4a7bf74fc28aa0da7a50c87301946bc1e54e4449 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Tue, 27 Jun 2023 14:39:18 +1200
>Subject: [PATCH 14/18] s4-rpc_server/drsuapi: Avoid modification to ncRoot
> input variable in GetNCChanges
>
>This tries to avoid it appearing that ncRoot is a value that can
>be trusted and used internally by not updating it and instead leaving
>it just as an input/echo-back value.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15401
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Stefan Metzmacher <metze@samba.org>
>(cherry picked from commit 548f141f11e89d335d8f9d74ab6925fa6b90fb84)
>---
> source4/rpc_server/drsuapi/getncchanges.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
>diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c
>index 92fa7ba7575..c3806f9e6de 100644
>--- a/source4/rpc_server/drsuapi/getncchanges.c
>+++ b/source4/rpc_server/drsuapi/getncchanges.c
>@@ -3185,7 +3185,6 @@ allowed:
> 
> 		getnc_state->ncRoot_guid = samdb_result_guid(res->msgs[0],
> 							     "objectGUID");
>-		ncRoot->guid = getnc_state->ncRoot_guid;
> 
> 		/* find out if we are to replicate Schema NC */
> 		ret = ldb_dn_compare_base(ldb_get_schema_basedn(sam_ctx),
>@@ -3195,8 +3194,6 @@ allowed:
> 		TALLOC_FREE(res);
> 	}
> 
>-	ncRoot->guid = getnc_state->ncRoot_guid;
>-
> 	/* we need the session key for encrypting password attributes */
> 	status = dcesrv_auth_session_key(dce_call, &session_key);
> 	if (!NT_STATUS_IS_OK(status)) {
>@@ -3378,11 +3375,19 @@ allowed:
> 	if (r->out.ctr->ctr6.naming_context == NULL) {
> 		return WERR_NOT_ENOUGH_MEMORY;
> 	}
>+
>+	/*
>+	 * Match Windows and echo back the original values from the request, even if
>+	 * they say DummyDN for the string NC
>+	 */
> 	*r->out.ctr->ctr6.naming_context = *ncRoot;
> 
> 	/* find the SID if there is one */
> 	dsdb_find_sid_by_dn(sam_ctx, getnc_state->ncRoot_dn, &r->out.ctr->ctr6.naming_context->sid);
> 
>+	/* Set GUID */
>+	r->out.ctr->ctr6.naming_context->guid = getnc_state->ncRoot_guid;
>+
> 	dsdb_get_oid_mappings_drsuapi(schema, true, mem_ctx, &ctr);
> 	r->out.ctr->ctr6.mapping_ctr = *ctr;
> 
>-- 
>2.25.1
>
>
>From 1dd5444cdb5cdc4c7fb9d516b0defa1e2132d8c8 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Tue, 27 Jun 2023 14:43:39 +1200
>Subject: [PATCH 15/18] s4-rpc_server/drsuapi: Rename ncRoot ->
> untrusted_ncRoot to avoid misuse
>
>Because of the requirement to echo back the original string, we can
>not force this to be a trustworthy value.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15401
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Stefan Metzmacher <metze@samba.org>
>(cherry picked from commit 2ed9815eeacfcf3a58871bafe0212398cc34c39e)
>---
> source4/rpc_server/drsuapi/getncchanges.c | 30 ++++++++++++++---------
> 1 file changed, 19 insertions(+), 11 deletions(-)
>
>diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c
>index c3806f9e6de..3e65c13f126 100644
>--- a/source4/rpc_server/drsuapi/getncchanges.c
>+++ b/source4/rpc_server/drsuapi/getncchanges.c
>@@ -2708,7 +2708,7 @@ WERROR dcesrv_drsuapi_DsGetNCChanges(struct dcesrv_call_state *dce_call, TALLOC_
> 		dcesrv_call_session_info(dce_call);
> 	struct imessaging_context *imsg_ctx =
> 		dcesrv_imessaging_context(dce_call->conn);
>-	struct drsuapi_DsReplicaObjectIdentifier *ncRoot;
>+	struct drsuapi_DsReplicaObjectIdentifier *untrusted_ncRoot;
> 	int ret;
> 	uint32_t i, k;
> 	struct dsdb_schema *schema;
>@@ -2830,8 +2830,8 @@ WERROR dcesrv_drsuapi_DsGetNCChanges(struct dcesrv_call_state *dce_call, TALLOC_
> 	}
> 
> 	/* Perform access checks. */
>-	ncRoot = req10->naming_context;
>-	if (ncRoot == NULL) {
>+	untrusted_ncRoot = req10->naming_context;
>+	if (untrusted_ncRoot == NULL) {
> 		DEBUG(0,(__location__ ": Request for DsGetNCChanges with no NC\n"));
> 		return WERR_DS_DRA_INVALID_PARAMETER;
> 	}
>@@ -3000,7 +3000,7 @@ allowed:
> 		struct ldb_dn *new_dn;
> 		ret = drs_ObjectIdentifier_to_dn_and_nc_root(getnc_state,
> 							     sam_ctx,
>-							     ncRoot,
>+							     untrusted_ncRoot,
> 							     &new_dn,
> 							     NULL);
> 		if (ret != LDB_SUCCESS) {
>@@ -3011,7 +3011,7 @@ allowed:
> 			 * in case.
> 			 */
> 			DBG_ERR("Bad DN '%s' as Naming Context for GetNCChanges: %s\n",
>-				drs_ObjectIdentifier_to_debug_string(mem_ctx, ncRoot),
>+				drs_ObjectIdentifier_to_debug_string(mem_ctx, untrusted_ncRoot),
> 				ldb_strerror(ret));
> 			return WERR_DS_DRA_INVALID_PARAMETER;
> 		}
>@@ -3056,12 +3056,12 @@ allowed:
> 
> 		ret = drs_ObjectIdentifier_to_dn_and_nc_root(mem_ctx,
> 							     sam_ctx,
>-							     ncRoot,
>+							     untrusted_ncRoot,
> 							     &ncRoot_dn,
> 							     NULL);
> 		if (ret != LDB_SUCCESS) {
> 			DBG_ERR("Bad DN '%s' as Naming Context or EXOP DN for GetNCChanges: %s\n",
>-				drs_ObjectIdentifier_to_debug_string(mem_ctx, ncRoot),
>+				drs_ObjectIdentifier_to_debug_string(mem_ctx, untrusted_ncRoot),
> 				ldb_strerror(ret));
> 			return WERR_DS_DRA_BAD_DN;
> 		}
>@@ -3380,7 +3380,7 @@ allowed:
> 	 * Match Windows and echo back the original values from the request, even if
> 	 * they say DummyDN for the string NC
> 	 */
>-	*r->out.ctr->ctr6.naming_context = *ncRoot;
>+	*r->out.ctr->ctr6.naming_context = *untrusted_ncRoot;
> 
> 	/* find the SID if there is one */
> 	dsdb_find_sid_by_dn(sam_ctx, getnc_state->ncRoot_dn, &r->out.ctr->ctr6.naming_context->sid);
>@@ -3615,7 +3615,15 @@ allowed:
> 		struct drsuapi_DsReplicaUpdateRefsRequest1 ureq;
> 		DEBUG(3,("UpdateRefs on getncchanges for %s\n",
> 			 GUID_string(mem_ctx, &req10->destination_dsa_guid)));
>-		ureq.naming_context = ncRoot;
>+
>+		/*
>+		 * We pass the pre-validation NC root here as
>+		 * drsuapi_UpdateRefs() has to check its own input
>+		 * values due to being called from
>+		 * dcesrv_drsuapi_DsReplicaUpdateRefs()
>+		 */
>+
>+		ureq.naming_context = untrusted_ncRoot;
> 		ureq.dest_dsa_dns_name = samdb_ntds_msdcs_dns_name(sam_ctx, mem_ctx,
> 								   &req10->destination_dsa_guid);
> 		if (!ureq.dest_dsa_dns_name) {
>@@ -3638,7 +3646,7 @@ allowed:
> 					  &ureq);
> 		if (!W_ERROR_IS_OK(werr)) {
> 			DEBUG(0,(__location__ ": Failed UpdateRefs on %s for %s in DsGetNCChanges - %s\n",
>-				 drs_ObjectIdentifier_to_debug_string(mem_ctx, ncRoot),
>+				 drs_ObjectIdentifier_to_debug_string(mem_ctx, untrusted_ncRoot),
> 				 ureq.dest_dsa_dns_name,
> 				 win_errstr(werr)));
> 		}
>@@ -3772,7 +3780,7 @@ allowed:
> 	      ("DsGetNCChanges with uSNChanged >= %llu flags 0x%08x on %s gave %u objects (done %u/%u) %u links (done %u/%u (as %s))\n",
> 	       (unsigned long long)(req10->highwatermark.highest_usn+1),
> 	       req10->replica_flags,
>-	       drs_ObjectIdentifier_to_debug_string(mem_ctx, ncRoot),
>+	       drs_ObjectIdentifier_to_debug_string(mem_ctx, untrusted_ncRoot),
> 	       r->out.ctr->ctr6.object_count,
> 	       i, r->out.ctr->ctr6.more_data?getnc_state->num_records:i,
> 	       r->out.ctr->ctr6.linked_attributes_count,
>-- 
>2.25.1
>
>
>From ad7beb4026f2ec8cc9a8e41f5c4d5f59a3fa4887 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Tue, 27 Jun 2023 17:01:28 +1200
>Subject: [PATCH 16/18] s4-rpc_server/drsuapi: Update getnc_state to be != NULL
>
>This is closer to our READDME.Coding style
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15401
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Stefan Metzmacher <metze@samba.org>
>(cherry picked from commit 2aba9e230ea62efcbd829f6f073894dfa3180c91)
>---
> source4/rpc_server/drsuapi/getncchanges.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c
>index 3e65c13f126..f932cb03131 100644
>--- a/source4/rpc_server/drsuapi/getncchanges.c
>+++ b/source4/rpc_server/drsuapi/getncchanges.c
>@@ -2996,7 +2996,7 @@ allowed:
> 	}
> 
> 	/* see if a previous replication has been abandoned */
>-	if (getnc_state) {
>+	if (getnc_state != NULL) {
> 		struct ldb_dn *new_dn;
> 		ret = drs_ObjectIdentifier_to_dn_and_nc_root(getnc_state,
> 							     sam_ctx,
>@@ -3025,7 +3025,7 @@ allowed:
> 		}
> 	}
> 
>-	if (getnc_state) {
>+	if (getnc_state != NULL) {
> 		ret = drsuapi_DsReplicaHighWaterMark_cmp(&getnc_state->last_hwm,
> 							 &req10->highwatermark);
> 		if (ret != 0) {
>-- 
>2.25.1
>
>
>From fae18923cd83667081df1415cd19c6ee0d1e9d25 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Wed, 28 Jun 2023 15:57:47 +1200
>Subject: [PATCH 17/18] s4-rpc_server/drsuapi: Ensure logs show DN for
> replicated objects, not (null)
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15407
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15401
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Stefan Metzmacher <metze@samba.org>
>(cherry picked from commit 17359afa627a3086ec8d6862f007a3479574a8b4)
>---
> source4/rpc_server/drsuapi/getncchanges.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
>diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c
>index f932cb03131..448217c03d6 100644
>--- a/source4/rpc_server/drsuapi/getncchanges.c
>+++ b/source4/rpc_server/drsuapi/getncchanges.c
>@@ -3567,7 +3567,15 @@ allowed:
> 			getncchanges_chunk_add_objects(repl_chunk, new_objs);
> 
> 			talloc_free(getnc_state->last_dn);
>-			getnc_state->last_dn = talloc_move(getnc_state, &msg->dn);
>+			/*
>+			 * talloc_steal() as we still need msg->dn to
>+			 * be a valid pointer for the log on the next
>+			 * line.
>+			 *
>+			 * msg only remains in scope for the next 25
>+			 * lines or so anyway.
>+			 */
>+			getnc_state->last_dn = talloc_steal(getnc_state, msg->dn);
> 		}
> 
> 		DEBUG(8,(__location__ ": %s object %s new tmp_highest_usn=%" PRIu64 "\n",
>-- 
>2.25.1
>
>
>From 348b3f89c39d9820267ed0216de99cd065e425e8 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Wed, 26 Jul 2023 14:27:16 +1200
>Subject: [PATCH 18/18] s4-rpc_server/drsupai: Avoid looping with Azure AD
> Connect by not incrementing temp_highest_usn for the NC root
>
>We send the NC root first, as a special case for every chunk
>that we send until the natural point where it belongs.
>
>We do not bump the tmp_highest_usn in the highwatermark that
>the client and server use (it is meant to be an opauqe cookie)
>until the 'natural' point where the object appears, similar
>to the cache for GET_ANC.
>
>The issue is that without this, because the NC root was sorted
>first in whatever chunk it appeared in but could have a 'high'
>highwatermark, Azure AD Connect will send back the same
>new_highwatermark->tmp_highest_usn, and due to a bug,
>a zero reserved_usn, which makes Samba discard it.
>
>The reserved_usn is now much less likely to ever be set because
>the tmp_higest_usn is now always advancing.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15401
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Stefan Metzmacher <metze@samba.org>
>(cherry picked from commit 79ca6ef28a6f94965cb030c4a7da8c1b9db7150b)
>---
> selftest/knownfail.d/getncchanges         |   5 +-
> source4/rpc_server/drsuapi/getncchanges.c | 117 ++++++++++++++++------
> 2 files changed, 87 insertions(+), 35 deletions(-)
>
>diff --git a/selftest/knownfail.d/getncchanges b/selftest/knownfail.d/getncchanges
>index e92c86cb306..bda9b31a1b1 100644
>--- a/selftest/knownfail.d/getncchanges
>+++ b/selftest/knownfail.d/getncchanges
>@@ -4,8 +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\)
>+# Samba chooses to always increment the USN for the NC root at the point where it would otherwise show up.
> samba4.drs.getncchanges.python\(.*\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_repl_nc_is_first_nc_change_only\(
>-samba4.drs.getncchanges.python\(.*\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_repl_nc_is_first\(
>-samba4.drs.getncchanges.python\(.*\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_repl_nc_is_first_mid\(
>-samba4.drs.getncchanges.python\(.*\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_repl_nc_is_first_start_zero\(
>-samba4.drs.getncchanges.python\(.*\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_repl_nc_is_first_start_zero_nc_change\(
>diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c
>index 448217c03d6..8864e79f10e 100644
>--- a/source4/rpc_server/drsuapi/getncchanges.c
>+++ b/source4/rpc_server/drsuapi/getncchanges.c
>@@ -62,6 +62,7 @@ struct drsuapi_getncchanges_state {
> 	bool is_get_anc;
> 	bool broken_samba_4_5_get_anc_emulation;
> 	bool is_get_tgt;
>+	bool send_nc_root_first;
> 	uint64_t min_usn;
> 	uint64_t max_usn;
> 	struct drsuapi_DsReplicaHighWaterMark last_hwm;
>@@ -1038,18 +1039,6 @@ static int site_res_cmp_usn_order(struct drsuapi_changed_objects *m1,
> 				  struct drsuapi_changed_objects *m2,
> 				  struct drsuapi_getncchanges_state *getnc_state)
> {
>-	int ret;
>-
>-	ret = ldb_dn_compare(getnc_state->ncRoot_dn, m1->dn);
>-	if (ret == 0) {
>-		return -1;
>-	}
>-
>-	ret = ldb_dn_compare(getnc_state->ncRoot_dn, m2->dn);
>-	if (ret == 0) {
>-		return 1;
>-	}
>-
> 	if (m1->usn == m2->usn) {
> 		return ldb_dn_compare(m2->dn, m1->dn);
> 	}
>@@ -2124,7 +2113,7 @@ static WERROR getncchanges_get_sorted_array(const struct drsuapi_DsReplicaLinked
>  * @param new_objs if parents are added, this gets updated to point to a chain
>  * of parent objects (with the parents first and the child last)
>  */
>-static WERROR getncchanges_add_ancestors(struct drsuapi_DsReplicaObjectListItemEx *child_obj,
>+static WERROR getncchanges_add_ancestors(const struct GUID *parent_object_guid,
> 					 struct ldb_dn *child_dn,
> 					 TALLOC_CTX *mem_ctx,
> 					 struct ldb_context *sam_ctx,
>@@ -2147,7 +2136,7 @@ static WERROR getncchanges_add_ancestors(struct drsuapi_DsReplicaObjectListItemE
> 					    DSDB_SECRET_ATTRIBUTES,
> 					    NULL };
> 
>-	next_anc_guid = child_obj->parent_object_guid;
>+	next_anc_guid = parent_object_guid;
> 
> 	while (next_anc_guid != NULL) {
> 		struct drsuapi_DsReplicaObjectListItemEx *anc_obj = NULL;
>@@ -2156,19 +2145,24 @@ static WERROR getncchanges_add_ancestors(struct drsuapi_DsReplicaObjectListItemE
> 		struct ldb_dn *anc_dn = NULL;
> 
> 		/*
>-		 * Don't send an object twice. (If we've sent the object, then
>-		 * we've also sent all its parents as well)
>+		 * For the GET_ANC case (but not the 'send NC root
>+		 * first' case), don't send an object twice.
>+		 *
>+		 * (If we've sent the object, then we've also sent all
>+		 * its parents as well)
> 		 */
>-		werr = dcesrv_drsuapi_obj_cache_exists(getnc_state->obj_cache,
>-						       next_anc_guid);
>-		if (W_ERROR_EQUAL(werr, WERR_OBJECT_NAME_EXISTS)) {
>-			return WERR_OK;
>-		}
>-		if (W_ERROR_IS_OK(werr)) {
>-			return WERR_INTERNAL_ERROR;
>-		}
>-		if (!W_ERROR_EQUAL(werr, WERR_OBJECT_NOT_FOUND)) {
>-			return werr;
>+		if (getnc_state->obj_cache) {
>+			werr = dcesrv_drsuapi_obj_cache_exists(getnc_state->obj_cache,
>+							       next_anc_guid);
>+			if (W_ERROR_EQUAL(werr, WERR_OBJECT_NAME_EXISTS)) {
>+				return WERR_OK;
>+			}
>+			if (W_ERROR_IS_OK(werr)) {
>+				return WERR_INTERNAL_ERROR;
>+			}
>+			if (!W_ERROR_EQUAL(werr, WERR_OBJECT_NOT_FOUND)) {
>+				return werr;
>+			}
> 		}
> 
> 		anc_obj = talloc_zero(mem_ctx,
>@@ -2222,11 +2216,18 @@ static WERROR getncchanges_add_ancestors(struct drsuapi_DsReplicaObjectListItemE
> 		/*
> 		 * Regardless of whether we actually use it or not,
> 		 * we add it to the cache so we don't look at it again
>+		 *
>+		 * The only time we are here without
>+		 * getnc_state->obj_cache is for the forced addition
>+		 * of the NC root to the start of the reply, this we
>+		 * want to add each and every call..
> 		 */
>-		werr = dcesrv_drsuapi_obj_cache_add(getnc_state->obj_cache,
>-						    next_anc_guid);
>-		if (!W_ERROR_IS_OK(werr)) {
>-			return werr;
>+		if (getnc_state->obj_cache) {
>+			werr = dcesrv_drsuapi_obj_cache_add(getnc_state->obj_cache,
>+							    next_anc_guid);
>+			if (!W_ERROR_IS_OK(werr)) {
>+				return werr;
>+			}
> 		}
> 
> 		/*
>@@ -2355,7 +2356,8 @@ static WERROR getncchanges_get_obj_to_send(const struct ldb_message *msg,
> 	 */
> 	if (getnc_state->is_get_anc
> 	    && !getnc_state->broken_samba_4_5_get_anc_emulation) {
>-		werr = getncchanges_add_ancestors(obj, msg->dn, mem_ctx,
>+		werr = getncchanges_add_ancestors(obj->parent_object_guid,
>+						  msg->dn, mem_ctx,
> 						  sam_ctx, getnc_state,
> 						  schema, session_key,
> 						  req10, local_pas,
>@@ -3287,6 +3289,12 @@ allowed:
> 			if (changes[i].usn > getnc_state->max_usn) {
> 				getnc_state->max_usn = changes[i].usn;
> 			}
>+
>+			if (req10->extended_op == DRSUAPI_EXOP_NONE &&
>+			    GUID_equal(&changes[i].guid, &getnc_state->ncRoot_guid))
>+			{
>+				getnc_state->send_nc_root_first = true;
>+			}
> 		}
> 
> 		if (req10->extended_op == DRSUAPI_EXOP_NONE) {
>@@ -3427,6 +3435,36 @@ allowed:
> 			       uint32_t_ptr_cmp);
> 	}
> 
>+	/*
>+	 * If we have the NC root in this replication, send it
>+	 * first regardless.  However, don't bump the USN now,
>+	 * treat it as if it was sent early due to GET_ANC
>+	 *
>+	 * This is triggered for each call, so every page of responses
>+	 * gets the NC root as the first object, up to the point where
>+	 * it naturally occurs in the replication.
>+	 */
>+
>+	if (getnc_state->send_nc_root_first) {
>+		struct drsuapi_DsReplicaObjectListItemEx *new_objs = NULL;
>+
>+		werr = getncchanges_add_ancestors(&getnc_state->ncRoot_guid,
>+						  NULL, mem_ctx,
>+						  sam_ctx, getnc_state,
>+						  schema, &session_key,
>+						  req10, local_pas,
>+						  machine_dn, &new_objs);
>+
>+		if (!W_ERROR_IS_OK(werr)) {
>+			return werr;
>+		}
>+
>+		getncchanges_chunk_add_objects(repl_chunk, new_objs);
>+
>+		DEBUG(8,(__location__ ": replicating NC root %s\n",
>+			 ldb_dn_get_linearized(getnc_state->ncRoot_dn)));
>+	}
>+
> 	/*
> 	 * Check in case we're still processing the links from an object in the
> 	 * previous chunk. We want to send the links (and any targets needed)
>@@ -3465,6 +3503,23 @@ allowed:
> 		TALLOC_CTX *tmp_ctx = talloc_new(mem_ctx);
> 		uint32_t old_la_index;
> 
>+		/*
>+		 * Once we get to the 'natural' place to send the NC
>+		 * root, stop sending it at the front of each reply
>+		 * and make sure to suppress sending it now
>+		 *
>+		 * We don't just 'continue' here as we must send links
>+		 * and unlike Windows we want to update the
>+		 * tmp_highest_usn
>+		 */
>+
>+		if (getnc_state->send_nc_root_first &&
>+		    GUID_equal(&getnc_state->guids[i], &getnc_state->ncRoot_guid))
>+		{
>+			getnc_state->send_nc_root_first = false;
>+			obj_already_sent = true;
>+		}
>+
> 		msg_dn = ldb_dn_new_fmt(tmp_ctx, sam_ctx, "<GUID=%s>",
> 					GUID_string(tmp_ctx, &getnc_state->guids[i]));
> 		W_ERROR_HAVE_NO_MEMORY(msg_dn);
>-- 
>2.25.1
>

Actions: View

Attachments on bug 15401: 18057 | 18063