From 1c1fe6af5db9f156eb3f9abb5441cde86f5195b0 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 6fe70c105a8050daa521a80e8a333d856eb7422a 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 229d55897ba6ec1eb15bf02fe73fc1dc378ff427 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 57b6f6ac2eaa0655dd618c044f4d156d5fa8543e 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 3e098c149784541cb8e31d29e0738d6d78354762 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 8d93be91b0308babee8877452fed5c75faa3228e 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 beef540b06e0a08e51f1722021270478bda995a2 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 dc928a3f48da7d7fdae04dd35578766f6439f6f8 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 db1fb48b261fffcab9d2e6cc5ab0b01dbe223083 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 e5eae1b2440fe436ca5f3a220b808fca1d7bde81 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 b795d74db101313d94102c0c5b14c320dfca9b3d 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 d98524528699681dfbf45c5a23d9274d1d66008f 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 728bc773261a295d8e2338f41e846992d0d45ec5 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 6e856365525c4c76ce14b0b734794538790f8392 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 18ae38009e73042d3d2aeec1aad753620c2a4f70 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 17993c4c036a5a936dff4b482aadff2366084287 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 a75214fb8cdca98d32588a7a0da49014d3921d9a 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 2d7a2dd1b44c63f287ae0667613d175b582422c9 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.

RN: Avoid infinite loop in initial user sync with Azure AD Connect
 when synchronising a large Samba AD domain.

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