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

Jule Anger janger at samba.org
Mon Aug 21 09:13:01 UTC 2023


The branch, v4-18-test has been updated
       via  794ce23b350 s4-rpc_server/drsupai: Avoid looping with Azure AD Connect by not incrementing temp_highest_usn for the NC root
       via  50bba4925e0 s4-rpc_server/drsuapi: Ensure logs show DN for replicated objects, not (null)
       via  9d3b0af9db7 s4-rpc_server/drsuapi: Update getnc_state to be != NULL
       via  cb83e9dbad8 s4-rpc_server/drsuapi: Rename ncRoot -> untrusted_ncRoot to avoid misuse
       via  21628e1f536 s4-rpc_server/drsuapi: Avoid modification to ncRoot input variable in GetNCChanges
       via  7da93e9a92f s4-rpc_server/drsuapi: Fix indentation in GetNCChanges()
       via  e43ea61cdd2 s4-rpc_server/drsuapi: Only keep and invalidate replication cycle state for normal replication
       via  dba337929d6 s4-torture/drs: Add test showing that if present in the set the NC root leads and tmp_highest_usn moves
       via  1fa63e6de9d s4-torture/drs: Add test demonstrating that a GetNCChanges REPL_OBJ will not reset the replication cookie
       via  69eac697606 s4-torture/drs: Add a test matching Azure AD Connect REPL_OBJ behaviour
       via  473cb476cdf s4-torture/drs: Use addCleanup() in getchanges.py for OU handling
       via  c6801832cb3 s4-torture/drs: Create temp OU with a unique name per test
       via  4b30611733e s4-torture/drs: Save the server dnsname on the DcConnection object
       via  958ae0038d6 s4-rpc_server/drsuapi: Remove rudundant check for valid and non-NULL ncRoot_dn
       via  2eae9fa2183 s4-dsdb: Improve logging for drs_ObjectIdentifier_to_dn_and_nc_root()
       via  e8fdc72b22e s4-rpc_server/drsuapi: Improve debug message for drs_ObjectIdentifier_to_dn_and_nc_root() failure
       via  251e3cd8c8d s4-rpc_server/drsuapi: Improve debugging of invalid DNs
       via  2fe39b167ac s4-rpc_server/drsuapi: Add tmp_highest_usn tracking to replication log
      from  0a044e409de 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-18-test


- Log -----------------------------------------------------------------
commit 794ce23b3503fa6c1b892854f86c4de9de42c31d
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-18-test): Jule Anger <janger at samba.org>
    Autobuild-Date(v4-18-test): Mon Aug 21 09:12:14 UTC 2023 on atb-devel-224

commit 50bba4925e0cd47fd0419c242e66ff9220d1ae7a
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 9d3b0af9db7ddf567d6fa420a0e58ae2fa932200
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 cb83e9dbad896961e9711d86916ca5247904dddc
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 21628e1f536702aa42875726f07d35a4115f76a8
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 7da93e9a92f87e9f6071f5cb2e107737d0a9f977
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 e43ea61cdd29779bf6202b5138d6af44451563d2
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 dba337929d6b668f1111c62af50e429072305087
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 1fa63e6de9d251d8acb270aa804aafe6b573cc84
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 69eac697606b5ab7c8d70062cb6e58a29235f5be
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 473cb476cdff88a780e332ea7cd1dd50e809b87b
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 c6801832cb3d59fba635c630a6450e905e942716
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 4b30611733ef9465c58593b073851b053737661f
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 958ae0038d62fbee811e425f2b8ea6f33ff8ee38
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 2eae9fa21831f0ef2e3e8962c6abefd7a15f4fe2
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 e8fdc72b22e27420d624609cbc312db6ffed0978
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 251e3cd8c8d5a0435344caf55a6d627ecc201e31
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 2fe39b167aca1cd08dfc04cde9297de85b664d29
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