[SCM] Samba Shared Repository - branch v4-19-test updated

Jule Anger janger at samba.org
Fri Aug 18 10:34:02 UTC 2023


The branch, v4-19-test has been updated
       via  fd2fdecdec0 s4-rpc_server/drsupai: Avoid looping with Azure AD Connect by not incrementing temp_highest_usn for the NC root
       via  7310afa34df s4-rpc_server/drsuapi: Ensure logs show DN for replicated objects, not (null)
       via  e5dc7e82a5a s4-rpc_server/drsuapi: Update getnc_state to be != NULL
       via  de10a323c6f s4-rpc_server/drsuapi: Rename ncRoot -> untrusted_ncRoot to avoid misuse
       via  a33689ad82c s4-rpc_server/drsuapi: Avoid modification to ncRoot input variable in GetNCChanges
       via  a596e4cdb9f s4-rpc_server/drsuapi: Fix indentation in GetNCChanges()
       via  91c7c77af14 s4-rpc_server/drsuapi: Only keep and invalidate replication cycle state for normal replication
       via  bd4ce525648 s4-torture/drs: Add test showing that if present in the set the NC root leads and tmp_highest_usn moves
       via  5243f55ee1c s4-torture/drs: Add test demonstrating that a GetNCChanges REPL_OBJ will not reset the replication cookie
       via  75197f528ff s4-torture/drs: Add a test matching Azure AD Connect REPL_OBJ behaviour
       via  c2b69e42780 s4-torture/drs: Use addCleanup() in getchanges.py for OU handling
       via  eeda4c3b5cd s4-torture/drs: Create temp OU with a unique name per test
       via  d1cdcf27571 s4-torture/drs: Save the server dnsname on the DcConnection object
       via  f8defe00366 s4-rpc_server/drsuapi: Remove rudundant check for valid and non-NULL ncRoot_dn
       via  f23c0d54a9b s4-dsdb: Improve logging for drs_ObjectIdentifier_to_dn_and_nc_root()
       via  2ecb53d5079 s4-rpc_server/drsuapi: Improve debug message for drs_ObjectIdentifier_to_dn_and_nc_root() failure
       via  85abc2852c4 s4-rpc_server/drsuapi: Improve debugging of invalid DNs
       via  0bd2f592217 s4-rpc_server/drsuapi: Add tmp_highest_usn tracking to replication log
      from  6a4622c4e8d s3: smbd: Ensure init_smb1_request() zeros out what the incoming pointer points to.

https://git.samba.org/?p=samba.git;a=shortlog;h=v4-19-test


- Log -----------------------------------------------------------------
commit fd2fdecdec0b0eef03ac67fcb05faa675566beb6
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Wed Jul 26 14:27:16 2023 +1200

    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 at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>
    (cherry picked from commit 79ca6ef28a6f94965cb030c4a7da8c1b9db7150b)
    
    Autobuild-User(v4-19-test): Jule Anger <janger at samba.org>
    Autobuild-Date(v4-19-test): Fri Aug 18 10:33:44 UTC 2023 on atb-devel-224

commit 7310afa34df397736c900aed04996f84b752dd1f
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Wed Jun 28 15:57:47 2023 +1200

    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 at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>
    (cherry picked from commit 17359afa627a3086ec8d6862f007a3479574a8b4)

commit e5dc7e82a5a33114b5cdffc2738bed8673bd65e0
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Tue Jun 27 17:01:28 2023 +1200

    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 at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>
    (cherry picked from commit 2aba9e230ea62efcbd829f6f073894dfa3180c91)

commit de10a323c6ff4ab75cd532c7252db3c723093434
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Tue Jun 27 14:43:39 2023 +1200

    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 at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>
    (cherry picked from commit 2ed9815eeacfcf3a58871bafe0212398cc34c39e)

commit a33689ad82c87500a550a003cc9b938c4111c147
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Tue Jun 27 14:39:18 2023 +1200

    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 at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>
    (cherry picked from commit 548f141f11e89d335d8f9d74ab6925fa6b90fb84)

commit a596e4cdb9f4e83a740befe1e7bb81cd00d38da7
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Tue Jun 27 17:06:13 2023 +1200

    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 at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>
    (cherry picked from commit fe7418e1765b79f60945b787536b4d84a548fe02)

commit 91c7c77af143cfe39c08ecb7bc73a1a842a71a24
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Mon Jun 26 16:53:10 2023 +1200

    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 at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>
    (cherry picked from commit 99579e706312192f46df33d55949db7f1475d0d0)

commit bd4ce52564846b686b515a8a40952d4aff37c333
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Mon Jul 24 12:05:18 2023 +1200

    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 at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>
    (cherry picked from commit 87414955212143b8502b4c02aca150bc72cb8de5)

commit 5243f55ee1c7fecf9aad309ddb38ac417e2a3d4c
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Mon Jun 26 16:25:32 2023 +1200

    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 at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>
    (cherry picked from commit b323169d6ff8357f7c999ae346137166c98218ac)

commit 75197f528ff1f5bd78ecd2ca0e0d94a00d80cfc3
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Tue Jun 27 12:20:32 2023 +1200

    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 at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>
    (cherry picked from commit db16366b0bbefcdb91a0b36c903ed63456a081b8)

commit c2b69e4278016c04bc472686f2dc5fd881d4b952
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Mon Jul 24 11:37:19 2023 +1200

    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 at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>
    (cherry picked from commit 40f831e67e1f312b1db52c74c119899245d03e32)

commit eeda4c3b5cda5a5876789b03a357a2cef56b9f3a
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Mon Jul 24 11:36:36 2023 +1200

    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 at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>
    (cherry picked from commit 628eab11b3c2e82875bf602e363b781d3e5eb96d)

commit d1cdcf27571db5a4f1f183da2205cb531405fab0
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Mon Jul 24 11:40:46 2023 +1200

    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 at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>
    (cherry picked from commit c30bb8769ff2c4eba2d8f8a2bd3a56946b7d9d5e)

commit f8defe00366c96746290fcfe36623cba3f4d167b
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Tue Jun 27 14:22:52 2023 +1200

    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 at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>
    (cherry picked from commit 0550e469eda4022659718ae9a56f5deaa9f9a307)

commit f23c0d54a9b07b9a184b055ad32ae6011a0fbcf0
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Tue Jun 27 14:59:49 2023 +1200

    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 at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>
    (cherry picked from commit 63843a22c8db73d459bee61e73bb1f0d31e3d427)

commit 2ecb53d5079b1504e948f8baa8d87d5d92a80bdd
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Tue Jun 27 17:18:39 2023 +1200

    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 at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>
    (cherry picked from commit a12bcce89d26ae05bbaeed560cf8fcc7b5bcfdab)

commit 85abc2852c47f4b7b5c5721bfdf350bccd74aced
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Tue Jun 27 12:18:24 2023 +1200

    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 at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>
    (cherry picked from commit d0c1ce53add2fd3b3a4186581f4e214029cbcf1a)

commit 0bd2f592217fe643571d1fdf3d4f8928b4bc758a
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Mon Jul 24 11:35:45 2023 +1200

    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 at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>
    (cherry picked from commit 0d9ea6c559317e19642662220c089e2d59ef3ecd)

-----------------------------------------------------------------------

Summary of changes:
 selftest/knownfail.d/getncchanges           |   2 +
 source4/dsdb/common/dsdb_dn.c               |  12 ++
 source4/rpc_server/drsuapi/dcesrv_drsuapi.h |   2 +-
 source4/rpc_server/drsuapi/getncchanges.c   | 293 ++++++++++++++++++++--------
 source4/torture/drs/python/getnc_exop.py    |  25 ++-
 source4/torture/drs/python/getncchanges.py  | 218 +++++++++++++++++++--
 6 files changed, 461 insertions(+), 91 deletions(-)


Changeset truncated at 500 lines:

diff --git a/selftest/knownfail.d/getncchanges b/selftest/knownfail.d/getncchanges
index 5ef1bc98bef..bda9b31a1b1 100644
--- a/selftest/knownfail.d/getncchanges
+++ b/selftest/knownfail.d/getncchanges
@@ -4,3 +4,5 @@ samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegri
 samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_repl_get_tgt_chain\(promoted_dc\)
 samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_repl_get_tgt_and_anc\(promoted_dc\)
 samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_repl_get_tgt_multivalued_links\(promoted_dc\)
+# 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\(
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;
 }
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 74b173c3965..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);
 	}
@@ -1711,6 +1700,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 +1709,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 +1762,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 +1922,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);
 	}
 }
 
@@ -2117,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,
@@ -2140,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;
@@ -2149,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,
@@ -2215,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;
+			}
 		}
 
 		/*
@@ -2348,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,
@@ -2701,7 +2710,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;
@@ -2712,7 +2721,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;
@@ -2823,8 +2832,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;
 	}
@@ -2962,23 +2971,50 @@ 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) {
+	if (getnc_state != NULL) {
 		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) {
 			/*
 			 * 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'\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, untrusted_ncRoot),
+				ldb_strerror(ret));
 			return WERR_DS_DRA_INVALID_PARAMETER;
 		}
 		if (ldb_dn_compare(new_dn, getnc_state->ncRoot_dn) != 0) {
@@ -2987,11 +3023,11 @@ 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;
 		}
 	}
 
-	if (getnc_state) {
+	if (getnc_state != NULL) {
 		ret = drsuapi_DsReplicaHighWaterMark_cmp(&getnc_state->last_hwm,
 							 &req10->highwatermark);
 		if (ret != 0) {
@@ -3001,10 +3037,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[] = {
@@ -3017,10 +3058,13 @@ 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, untrusted_ncRoot),
+				ldb_strerror(ret));
 			return WERR_DS_DRA_BAD_DN;
 		}
 
@@ -3110,19 +3154,39 @@ 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);
 
 		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),
@@ -3132,15 +3196,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 */
 	status = dcesrv_auth_session_key(dce_call, &session_key);
 	if (!NT_STATUS_IS_OK(status)) {
@@ -3203,11 +3258,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);
@@ -3232,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) {
@@ -3320,11 +3383,19 @@ allowed:
 	if (r->out.ctr->ctr6.naming_context == NULL) {
 		return WERR_NOT_ENOUGH_MEMORY;
 	}
-	*r->out.ctr->ctr6.naming_context = *ncRoot;
+
+	/*
+	 * 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 = *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);
 
+	/* 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;
 
@@ -3364,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)
@@ -3402,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))
+		{


-- 
Samba Shared Repository



More information about the samba-cvs mailing list