[SCM] Samba Shared Repository - branch master updated

Andrew Bartlett abartlet at samba.org
Sun Aug 13 22:55:01 UTC 2023


The branch, master has been updated
       via  58260e1e4fe python/samba/netcmd/domain/schemaupgrade.py: fix missing newline
       via  79ca6ef28a6 s4-rpc_server/drsupai: Avoid looping with Azure AD Connect by not incrementing temp_highest_usn for the NC root
       via  17359afa627 s4-rpc_server/drsuapi: Ensure logs show DN for replicated objects, not (null)
       via  2aba9e230ea s4-rpc_server/drsuapi: Update getnc_state to be != NULL
       via  2ed9815eeac s4-rpc_server/drsuapi: Rename ncRoot -> untrusted_ncRoot to avoid misuse
       via  548f141f11e s4-rpc_server/drsuapi: Avoid modification to ncRoot input variable in GetNCChanges
       via  fe7418e1765 s4-rpc_server/drsuapi: Fix indentation in GetNCChanges()
       via  99579e70631 s4-rpc_server/drsuapi: Only keep and invalidate replication cycle state for normal replication
       via  87414955212 s4-torture/drs: Add test showing that if present in the set the NC root leads and tmp_highest_usn moves
       via  b323169d6ff s4-torture/drs: Add test demonstrating that a GetNCChanges REPL_OBJ will not reset the replication cookie
       via  db16366b0bb s4-torture/drs: Add a test matching Azure AD Connect REPL_OBJ behaviour
       via  40f831e67e1 s4-torture/drs: Use addCleanup() in getchanges.py for OU handling
       via  628eab11b3c s4-torture/drs: Create temp OU with a unique name per test
       via  c30bb8769ff s4-torture/drs: Save the server dnsname on the DcConnection object
       via  0550e469eda s4-rpc_server/drsuapi: Remove rudundant check for valid and non-NULL ncRoot_dn
       via  63843a22c8d s4-dsdb: Improve logging for drs_ObjectIdentifier_to_dn_and_nc_root()
       via  a12bcce89d2 s4-rpc_server/drsuapi: Improve debug message for drs_ObjectIdentifier_to_dn_and_nc_root() failure
       via  d0c1ce53add s4-rpc_server/drsuapi: Improve debugging of invalid DNs
       via  0d9ea6c5593 s4-rpc_server/drsuapi: Add tmp_highest_usn tracking to replication log
      from  5ec660160e4 smbclient3: Get all reparse data for allinfo

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 58260e1e4fe9af44bb0c93df969a07466d65d2ca
Author: Michael Tokarev <mjt at tls.msk.ru>
Date:   Fri Aug 4 07:40:02 2023 +0300

    python/samba/netcmd/domain/schemaupgrade.py: fix missing newline
    
    Signed-off-by: Michael Tokarev <mjt at tls.msk.ru>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>
    
    Autobuild-User(master): Andrew Bartlett <abartlet at samba.org>
    Autobuild-Date(master): Sun Aug 13 22:54:55 UTC 2023 on atb-devel-224

commit 79ca6ef28a6f94965cb030c4a7da8c1b9db7150b
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.
    
    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>

commit 17359afa627a3086ec8d6862f007a3479574a8b4
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>

commit 2aba9e230ea62efcbd829f6f073894dfa3180c91
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>

commit 2ed9815eeacfcf3a58871bafe0212398cc34c39e
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>

commit 548f141f11e89d335d8f9d74ab6925fa6b90fb84
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>

commit fe7418e1765b79f60945b787536b4d84a548fe02
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>

commit 99579e706312192f46df33d55949db7f1475d0d0
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>

commit 87414955212143b8502b4c02aca150bc72cb8de5
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>

commit b323169d6ff8357f7c999ae346137166c98218ac
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>

commit db16366b0bbefcdb91a0b36c903ed63456a081b8
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>

commit 40f831e67e1f312b1db52c74c119899245d03e32
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>

commit 628eab11b3c2e82875bf602e363b781d3e5eb96d
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>

commit c30bb8769ff2c4eba2d8f8a2bd3a56946b7d9d5e
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>

commit 0550e469eda4022659718ae9a56f5deaa9f9a307
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>

commit 63843a22c8db73d459bee61e73bb1f0d31e3d427
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>

commit a12bcce89d26ae05bbaeed560cf8fcc7b5bcfdab
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>

commit d0c1ce53add2fd3b3a4186581f4e214029cbcf1a
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>

commit 0d9ea6c559317e19642662220c089e2d59ef3ecd
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>

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

Summary of changes:
 python/samba/netcmd/domain/schemaupgrade.py |   2 +-
 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 +++++++++++++++++++--
 7 files changed, 462 insertions(+), 92 deletions(-)


Changeset truncated at 500 lines:

diff --git a/python/samba/netcmd/domain/schemaupgrade.py b/python/samba/netcmd/domain/schemaupgrade.py
index 1d67ab58c15..ff00a771b20 100644
--- a/python/samba/netcmd/domain/schemaupgrade.py
+++ b/python/samba/netcmd/domain/schemaupgrade.py
@@ -228,7 +228,7 @@ class cmd_domain_schema_upgrade(Command):
         try:
             from samba.ms_schema_markdown import read_ms_markdown
         except ImportError as e:
-            self.outf.write("Exception in importing markdown: %s" % e)
+            self.outf.write("Exception in importing markdown: %s\n" % e)
             raise CommandError('Failed to import module markdown')
         from samba.schema import Schema
 
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 d40750a0376..63a8628d6d3 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 7a1c8af72e4..3ce2a896950 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;
 


-- 
Samba Shared Repository



More information about the samba-cvs mailing list