Attachment 13581 Details for Bug 12972 – Patches for v4-7-test

[patch] Patches for v4-7-test

tmp47.diff.txt (text/plain), 135.42 KB, created by Stefan Metzmacher on 2017-09-12 10:20:14 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Metzmacher

Created: 2017-09-12 10:20:14 UTC

Size: 135.42 KB

patch

obsolete

>From 8add91de238774607677e16b7876b43e3e37d7f1 Mon Sep 17 00:00:00 2001
>From: Tim Beale <timbeale@catalyst.net.nz>
>Date: Mon, 29 May 2017 17:06:55 +1200
>Subject: [PATCH 01/20] drs: support sync-forced for 'samba-tool drs replicate
> --local'
>
>The sync-forced option wasn't being passed into the replication request
>when the --local option was used. This meant if outbound replication
>were disabled on the target DC, then the replicate --local command would
>fail.
>
>Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 314b96e18371e92cde8172c3f3739b52970ee2d7)
>---
> python/samba/drs_utils.py  | 6 +++++-
> python/samba/netcmd/drs.py | 9 ++++++---
> 2 files changed, 11 insertions(+), 4 deletions(-)
>
>diff --git a/python/samba/drs_utils.py b/python/samba/drs_utils.py
>index 8624f3f..1719af6 100644
>--- a/python/samba/drs_utils.py
>+++ b/python/samba/drs_utils.py
>@@ -200,7 +200,7 @@ class drs_Replicate(object):
> 
>     def replicate(self, dn, source_dsa_invocation_id, destination_dsa_guid,
>                   schema=False, exop=drsuapi.DRSUAPI_EXOP_NONE, rodc=False,
>-                  replica_flags=None, full_sync=True):
>+                  replica_flags=None, full_sync=True, sync_forced=False):
>         '''replicate a single DN'''
> 
>         # setup for a GetNCChanges call
>@@ -262,6 +262,10 @@ class drs_Replicate(object):
>                     drsuapi.DRSUAPI_DRS_SPECIAL_SECRET_PROCESSING)
>             else:
>                 req8.replica_flags |= drsuapi.DRSUAPI_DRS_WRIT_REP
>+
>+        if sync_forced:
>+            req8.replica_flags |= drsuapi.DRSUAPI_DRS_SYNC_FORCED
>+
>         req8.max_object_count = 402
>         req8.max_ndr_size = 402116
>         req8.extended_op = exop
>diff --git a/python/samba/netcmd/drs.py b/python/samba/netcmd/drs.py
>index b9b876a..e1886b9 100644
>--- a/python/samba/netcmd/drs.py
>+++ b/python/samba/netcmd/drs.py
>@@ -241,7 +241,8 @@ class cmd_drs_kcc(Command):
> 
> 
> 
>-def drs_local_replicate(self, SOURCE_DC, NC, full_sync=False, single_object=False):
>+def drs_local_replicate(self, SOURCE_DC, NC, full_sync=False, single_object=False,
>+                        sync_forced=False):
>     '''replicate from a source DC to the local SAM'''
> 
>     self.server = SOURCE_DC
>@@ -284,7 +285,7 @@ def drs_local_replicate(self, SOURCE_DC, NC, full_sync=False, single_object=Fals
>         (num_objects, num_links) = repl.replicate(NC,
>                                                   source_dsa_invocation_id, destination_dsa_guid,
>                                                   rodc=rodc, full_sync=full_sync,
>-                                                  exop=exop)
>+                                                  exop=exop, sync_forced=sync_forced)
>     except Exception, e:
>         raise CommandError("Error replicating DN %s" % NC, e)
>     self.samdb.transaction_commit()
>@@ -332,7 +333,9 @@ class cmd_drs_replicate(Command):
>         self.creds = credopts.get_credentials(self.lp, fallback_machine=True)
> 
>         if local:
>-            drs_local_replicate(self, SOURCE_DC, NC, full_sync=full_sync, single_object=single_object)
>+            drs_local_replicate(self, SOURCE_DC, NC, full_sync=full_sync,
>+                                single_object=single_object,
>+                                sync_forced=sync_forced)
>             return
> 
>         if local_online:
>-- 
>1.9.1
>
>
>From 74fcf311f6bf990f8d607c80557e8a6527dd5483 Mon Sep 17 00:00:00 2001
>From: Tim Beale <timbeale@catalyst.net.nz>
>Date: Wed, 7 Jun 2017 16:56:18 +1200
>Subject: [PATCH 02/20] drs_utils: HWM in 'samba-tool drs replicate --local'
> always zero
>
>The code to check for the 'repsFrom' highwatermark didn't have any
>effect because the hwm variable was overwritten (initialized to all
>zeroes) further down.
>
>Using a zero HWM probably wouldn't have impacted functionality because
>we were still correctly using the uptodatenessvector, which should
>avoid a full replication.
>
>This was introduced in commit e2ba17d26af42974e5d, presumably by
>accident.
>
>Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 4bd8467018eee181588c13490edbea26b488ee36)
>---
> python/samba/drs_utils.py | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
>diff --git a/python/samba/drs_utils.py b/python/samba/drs_utils.py
>index 1719af6..b9ed059 100644
>--- a/python/samba/drs_utils.py
>+++ b/python/samba/drs_utils.py
>@@ -211,7 +211,13 @@ class drs_Replicate(object):
>         req8.naming_context = drsuapi.DsReplicaObjectIdentifier()
>         req8.naming_context.dn = dn
> 
>+        # Default to a full replication if we don't find an upToDatenessVector
>         udv = None
>+        hwm = drsuapi.DsReplicaHighWaterMark()
>+        hwm.tmp_highest_usn = 0
>+        hwm.reserved_usn = 0
>+        hwm.highest_usn = 0
>+
>         if not full_sync:
>             res = self.samdb.search(base=dn, scope=ldb.SCOPE_BASE,
>                                     attrs=["repsFrom"])
>@@ -238,12 +244,6 @@ class drs_Replicate(object):
>             udv.cursors = cursors_v1
>             udv.count = len(cursors_v1)
> 
>-        # If we can't find an upToDateVector, or where told not to, replicate fully
>-        hwm = drsuapi.DsReplicaHighWaterMark()
>-        hwm.tmp_highest_usn = 0
>-        hwm.reserved_usn = 0
>-        hwm.highest_usn = 0
>-
>         req8.highwatermark = hwm
>         req8.uptodateness_vector = udv
> 
>-- 
>1.9.1
>
>
>From 43e430defa4e3deb628df730f9de73a6cc4dd146 Mon Sep 17 00:00:00 2001
>From: Tim Beale <timbeale@catalyst.net.nz>
>Date: Mon, 19 Jun 2017 10:26:48 +1200
>Subject: [PATCH 03/20] libnet: Initialize req_level in become_dc tests
>
>The net.api.become.dc tests would always pass the request into
>libnet_vampire_cb_store_chunk() with req_level=0, which meant that
>storing the chunk didn't use the correct replica_flags/exop.
>
>I noticed this problem when working on client-side support for GET_TGT.
>My changes relied on the critical-only request flag being passed down
>into replmd, but because the request flags weren't passed correctly, my
>changes caused the become_dc tests to fail.
>
>Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 475a3206461f5458059f8f530b45f0b1ae636739)
>---
> source4/libnet/libnet_become_dc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source4/libnet/libnet_become_dc.c b/source4/libnet/libnet_become_dc.c
>index 43a3209..e9153a0 100644
>--- a/source4/libnet/libnet_become_dc.c
>+++ b/source4/libnet/libnet_become_dc.c
>@@ -2673,7 +2673,7 @@ static WERROR becomeDC_drsuapi_pull_partition_recv(struct libnet_BecomeDC_state
> 						   struct libnet_BecomeDC_Partition *partition,
> 						   struct drsuapi_DsGetNCChanges *r)
> {
>-	uint32_t req_level = 0;
>+	uint32_t req_level = r->in.level;
> 	struct drsuapi_DsGetNCChangesRequest5 *req5 = NULL;
> 	struct drsuapi_DsGetNCChangesRequest8 *req8 = NULL;
> 	struct drsuapi_DsGetNCChangesRequest10 *req10 = NULL;
>-- 
>1.9.1
>
>
>From 9a6c9c3f165929ae58c13c082d2fa88c386f137c Mon Sep 17 00:00:00 2001
>From: Tim Beale <timbeale@catalyst.net.nz>
>Date: Wed, 7 Jun 2017 11:13:52 +1200
>Subject: [PATCH 04/20] getnc_exop.py: Fix typo in function name
>
>This drove me crazy when I tried to search for it.
>
>Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit dddcf80660dd883ee24a1dfa13fd5cfe4cd49e62)
>---
> source4/torture/drs/python/drs_base.py   |  2 +-
> source4/torture/drs/python/getnc_exop.py | 14 +++++++-------
> 2 files changed, 8 insertions(+), 8 deletions(-)
>
>diff --git a/source4/torture/drs/python/drs_base.py b/source4/torture/drs/python/drs_base.py
>index f07592d..f19efca 100644
>--- a/source4/torture/drs/python/drs_base.py
>+++ b/source4/torture/drs/python/drs_base.py
>@@ -166,7 +166,7 @@ class DrsBaseTestCase(SambaToolCmdTest):
>         utdv.cursors = cursors
>         return (hwm, utdv)
> 
>-    def _get_indentifier(self, ldb_conn, dn):
>+    def _get_identifier(self, ldb_conn, dn):
>         res = ldb_conn.search(dn, scope=ldb.SCOPE_BASE,
>                 attrs=["objectGUID", "objectSid"])
>         id = drsuapi.DsReplicaObjectIdentifier()
>diff --git a/source4/torture/drs/python/getnc_exop.py b/source4/torture/drs/python/getnc_exop.py
>index 26786be..caa7826 100644
>--- a/source4/torture/drs/python/getnc_exop.py
>+++ b/source4/torture/drs/python/getnc_exop.py
>@@ -146,20 +146,20 @@ class DrsReplicaSyncTestCase(drs_base.DrsBaseTestCase):
>             "dn": ou1,
>             "objectclass": "organizationalUnit"
>             })
>-        ou1_id = self._get_indentifier(self.ldb_dc1, ou1)
>+        ou1_id = self._get_identifier(self.ldb_dc1, ou1)
>         ou2 = "OU=get_anc2,%s" % ou1
>         self.ldb_dc1.add({
>             "dn": ou2,
>             "objectclass": "organizationalUnit"
>             })
>-        ou2_id = self._get_indentifier(self.ldb_dc1, ou2)
>+        ou2_id = self._get_identifier(self.ldb_dc1, ou2)
>         dc3 = "CN=test_anc_dc_%u,%s" % (random.randint(0, 4294967295), ou2)
>         self.ldb_dc1.add({
>             "dn": dc3,
>             "objectclass": "computer",
>             "userAccountControl": "%d" % (samba.dsdb.UF_ACCOUNTDISABLE | samba.dsdb.UF_SERVER_TRUST_ACCOUNT)
>             })
>-        dc3_id = self._get_indentifier(self.ldb_dc1, dc3)
>+        dc3_id = self._get_identifier(self.ldb_dc1, dc3)
> 
>         req8 = self._exop_req8(dest_dsa=None,
>                                invocation_id=self.ldb_dc1.get_invocation_id(),
>@@ -201,20 +201,20 @@ class DrsReplicaSyncTestCase(drs_base.DrsBaseTestCase):
>             "dn": ou1,
>             "objectclass": "organizationalUnit"
>             })
>-        ou1_id = self._get_indentifier(self.ldb_dc1, ou1)
>+        ou1_id = self._get_identifier(self.ldb_dc1, ou1)
>         ou2 = "OU=get_anc2,%s" % ou1
>         self.ldb_dc1.add({
>             "dn": ou2,
>             "objectclass": "organizationalUnit"
>             })
>-        ou2_id = self._get_indentifier(self.ldb_dc1, ou2)
>+        ou2_id = self._get_identifier(self.ldb_dc1, ou2)
>         dc3 = "CN=test_anc_dc_%u,%s" % (random.randint(0, 4294967295), ou2)
>         self.ldb_dc1.add({
>             "dn": dc3,
>             "objectclass": "computer",
>             "userAccountControl": "%d" % (samba.dsdb.UF_ACCOUNTDISABLE | samba.dsdb.UF_SERVER_TRUST_ACCOUNT)
>             })
>-        dc3_id = self._get_indentifier(self.ldb_dc1, dc3)
>+        dc3_id = self._get_identifier(self.ldb_dc1, dc3)
> 
>         (hwm1, utdv1) = self._check_replication([ou1,ou2,dc3],
>                                                 drsuapi.DRSUAPI_DRS_WRIT_REP)
>@@ -325,7 +325,7 @@ class DrsReplicaSyncTestCase(drs_base.DrsBaseTestCase):
>             "dn": cn3,
>             "objectclass": "container",
>             })
>-        cn3_id = self._get_indentifier(self.ldb_dc1, cn3)
>+        cn3_id = self._get_identifier(self.ldb_dc1, cn3)
> 
>         (hwm5, utdv5) = self._check_replication([dc3,ou1,ou2,self.ou,cn3],
>                             drsuapi.DRSUAPI_DRS_WRIT_REP)
>-- 
>1.9.1
>
>
>From 82d6bfe06beb887708d6898633cc37eed713d5f3 Mon Sep 17 00:00:00 2001
>From: Tim Beale <timbeale@catalyst.net.nz>
>Date: Mon, 12 Jun 2017 11:20:54 +1200
>Subject: [PATCH 05/20] getncchanges.c: Remove unused null_scope variable
>
>This was added in 4cc6b5a69b1f94d96a73ac1 but the very next commit
>(f1c6bab60e52624f5f3) removed where it was set, which meant the variable
>was always false and seemingly pointless.
>
>Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 50b638d15cceafc4f3e78d1fab8c7ed0a6e44a1f)
>---
> source4/rpc_server/drsuapi/getncchanges.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
>diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c
>index 32580bd..77d881f 100644
>--- a/source4/rpc_server/drsuapi/getncchanges.c
>+++ b/source4/rpc_server/drsuapi/getncchanges.c
>@@ -2022,7 +2022,7 @@ WERROR dcesrv_drsuapi_DsGetNCChanges(struct dcesrv_call_state *dce_call, TALLOC_
> 	uint32_t link_total = 0;
> 	uint32_t link_given = 0;
> 	struct ldb_dn *search_dn = NULL;
>-	bool am_rodc, null_scope=false;
>+	bool am_rodc;
> 	enum security_user_level security_level;
> 	struct ldb_context *sam_ctx;
> 	struct dom_sid *user_sid;
>@@ -2553,7 +2553,6 @@ allowed:
> 
> 	for (i=getnc_state->num_processed;
> 	     i<getnc_state->num_records &&
>-		     !null_scope &&
> 		     (r->out.ctr->ctr6.object_count < max_objects)
> 		     && !max_wait_reached;
> 	    i++) {
>-- 
>1.9.1
>
>
>From 7554c814c46b06b47d89a1a383db5d82301fcae4 Mon Sep 17 00:00:00 2001
>From: Tim Beale <timbeale@catalyst.net.nz>
>Date: Tue, 20 Jun 2017 13:14:43 +1200
>Subject: [PATCH 06/20] repl: Remove old TODO
>
>This TODO was added in 2007 before we supported linked attributes.
>It's no longer relevant.
>
>Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 69f593ec5a89411fb3e2cbf62f2092c3073fa0f7)
>---
> source4/dsdb/repl/replicated_objects.c | 2 --
> 1 file changed, 2 deletions(-)
>
>diff --git a/source4/dsdb/repl/replicated_objects.c b/source4/dsdb/repl/replicated_objects.c
>index e5fec1f..862fafa 100644
>--- a/source4/dsdb/repl/replicated_objects.c
>+++ b/source4/dsdb/repl/replicated_objects.c
>@@ -835,8 +835,6 @@ WERROR dsdb_replicated_objects_commit(struct ldb_context *ldb,
> 		return WERR_NOT_ENOUGH_MEMORY;
> 	}
> 
>-	/* TODO: handle linked attributes */
>-
> 	/* wrap the extended operation in a transaction 
> 	   See [MS-DRSR] 3.3.2 Transactions
> 	 */
>-- 
>1.9.1
>
>
>From de0e72d533f83a07a0b8f1aea901ee802231b0a6 Mon Sep 17 00:00:00 2001
>From: Tim Beale <timbeale@catalyst.net.nz>
>Date: Tue, 30 May 2017 15:08:44 +1200
>Subject: [PATCH 07/20] werror: Add WERR_DS_DRA_RECYCLED_TARGET
>
>When the DRS client encounters a linked attribute with an unknown target
>object, it should return a RECYCLED_TARGET error, which should result in
>the client resending the GETNCChanges request with the GET_TGT flag set.
>
>This error code is currently documented by Microsoft under System Error
>Codes (8200-8999). I contacted them and they will also add it to the
>MS-ERREF doc in future.
>
>Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
>Reviewed-by: Garming Sam <garming@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12972
>(cherry picked from commit d13e7b92f85e5623c5cf1309f13d59141dc7d888)
>---
> libcli/util/werror.h | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/libcli/util/werror.h b/libcli/util/werror.h
>index d3d3327..0370a06 100644
>--- a/libcli/util/werror.h
>+++ b/libcli/util/werror.h
>@@ -100,6 +100,7 @@ typedef uint32_t WERROR;
> #define WERR_INVALID_PRIMARY_GROUP      W_ERROR(0x0000051C)
> 
> #define WERR_DS_DRA_SECRETS_DENIED			W_ERROR(0x000021B6)
>+#define WERR_DS_DRA_RECYCLED_TARGET			W_ERROR(0x000021BF)
> 
> #define WERR_DNS_ERROR_KEYMASTER_REQUIRED               W_ERROR(0x0000238D)
> #define WERR_DNS_ERROR_NOT_ALLOWED_ON_SIGNED_ZONE       W_ERROR(0x0000238E)
>-- 
>1.9.1
>
>
>From 24ee8e6b3dd23fd8759bd9e867e81cc8867e7333 Mon Sep 17 00:00:00 2001
>From: Tim Beale <timbeale@catalyst.net.nz>
>Date: Wed, 14 Jun 2017 10:51:59 +1200
>Subject: [PATCH 08/20] replmd: Split checking link attr target into new
> function
>
>We want to re-use this code to check that the linked attribute's target
>object exists *before* we try to commit the transaction. This will allow
>us to re-request the block with the GET_TGT flag set.
>
>This splits checking the target object exists into a separate function.
>
>Minor changes of note:
>- the 'parent' argument was passed to replmd_process_linked_attribute()
>  as NULL, so I've just replaced where it was used in the refactored code
>  with NULL.
>- I've tweaked the "Failed to find GUID" error message slightly to display
>  the attribute ID rather than the attribute name (saves repeating
>  lookups and/or passing extra arguments).
>- Tweaked the replmd_deletion_state() logic - it only made sense to call
>  it in the code block where we actually found the target
>
>Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
>Reviewed-by: Garming Sam <garming@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12972
>(cherry picked from commit 04ce638a1f7166b3fe75b52efd0a522248196fd4)
>---
> source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 171 ++++++++++++++----------
> 1 file changed, 102 insertions(+), 69 deletions(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>index 0ed24cb..53e2c12 100644
>--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>@@ -6627,6 +6627,105 @@ static int replmd_extended_replicated_objects(struct ldb_module *module, struct
> 	return replmd_replicated_apply_next(ar);
> }
> 
>+/**
>+ * Checks that the target object for a linked attribute exists.
>+ * If it exists, its GUID and deletion-state are returned
>+ */
>+static int replmd_check_target_exists(struct ldb_module *module,
>+				      struct dsdb_dn *dsdb_dn,
>+				      struct la_entry *la_entry,
>+				      struct ldb_dn *source_dn,
>+				      struct GUID *guid,
>+				      enum deletion_state *target_deletion_state)
>+{
>+	struct drsuapi_DsReplicaLinkedAttribute *la = la_entry->la;
>+	struct ldb_context *ldb = ldb_module_get_ctx(module);
>+	struct ldb_result *target_res;
>+	TALLOC_CTX *tmp_ctx = talloc_new(la_entry);
>+	const char *attrs[] = { "isDeleted", "isRecycled", NULL };
>+	NTSTATUS ntstatus;
>+	int ret;
>+	bool active = (la->flags & DRSUAPI_DS_LINKED_ATTRIBUTE_FLAG_ACTIVE) ? true : false;
>+
>+	*target_deletion_state = OBJECT_REMOVED;
>+
>+	ntstatus = dsdb_get_extended_dn_guid(dsdb_dn->dn, guid, "GUID");
>+
>+	if (!NT_STATUS_IS_OK(ntstatus) && !active) {
>+
>+		/*
>+		 * This strange behaviour (allowing a NULL/missing
>+		 * GUID) originally comes from:
>+		 *
>+		 * commit e3054ce0fe0f8f62d2f5b2a77893e7a1479128bd
>+		 * Author: Andrew Tridgell <tridge@samba.org>
>+		 * Date:   Mon Dec 21 21:21:55 2009 +1100
>+		 *
>+		 *  s4-drs: cope better with NULL GUIDS from DRS
>+		 *
>+		 *  It is valid to get a NULL GUID over DRS for a deleted forward link. We
>+		 *  need to match by DN if possible when seeing if we should update an
>+		 *  existing link.
>+		 *
>+		 *  Pair-Programmed-With: Andrew Bartlett <abartlet@samba.org>
>+		 */
>+		ret = dsdb_module_search_dn(module, tmp_ctx, &target_res,
>+					    dsdb_dn->dn, attrs,
>+					    DSDB_FLAG_NEXT_MODULE |
>+					    DSDB_SEARCH_SHOW_RECYCLED |
>+					    DSDB_SEARCH_SEARCH_ALL_PARTITIONS |
>+					    DSDB_SEARCH_SHOW_DN_IN_STORAGE_FORMAT,
>+					    NULL);
>+	} else if (!NT_STATUS_IS_OK(ntstatus)) {
>+		ldb_asprintf_errstring(ldb, "Failed to find GUID in linked attribute 0x%x blob for %s from %s",
>+				       la->attid,
>+				       ldb_dn_get_linearized(dsdb_dn->dn),
>+				       ldb_dn_get_linearized(source_dn));
>+		talloc_free(tmp_ctx);
>+		return LDB_ERR_OPERATIONS_ERROR;
>+	} else {
>+		ret = dsdb_module_search(module, tmp_ctx, &target_res,
>+					 NULL, LDB_SCOPE_SUBTREE,
>+					 attrs,
>+					 DSDB_FLAG_NEXT_MODULE |
>+					 DSDB_SEARCH_SHOW_RECYCLED |
>+					 DSDB_SEARCH_SEARCH_ALL_PARTITIONS |
>+					 DSDB_SEARCH_SHOW_DN_IN_STORAGE_FORMAT,
>+					 NULL,
>+					 "objectGUID=%s",
>+					 GUID_string(tmp_ctx, guid));
>+	}
>+
>+	if (ret != LDB_SUCCESS) {
>+		ldb_asprintf_errstring(ldb, "Failed to re-resolve GUID %s: %s\n",
>+				       GUID_string(tmp_ctx, guid),
>+				       ldb_errstring(ldb));
>+		talloc_free(tmp_ctx);
>+		return ret;
>+	}
>+
>+	if (target_res->count == 0) {
>+		DEBUG(2,(__location__ ": WARNING: Failed to re-resolve GUID %s - using %s\n",
>+			 GUID_string(tmp_ctx, guid),
>+			 ldb_dn_get_linearized(dsdb_dn->dn)));
>+	} else if (target_res->count != 1) {
>+		ldb_asprintf_errstring(ldb, "More than one object found matching objectGUID %s\n",
>+				       GUID_string(tmp_ctx, guid));
>+		ret = LDB_ERR_OPERATIONS_ERROR;
>+	} else {
>+		struct ldb_message *target_msg = target_res->msgs[0];
>+
>+		dsdb_dn->dn = talloc_steal(dsdb_dn, target_msg->dn);
>+
>+		/* Get the object's state (i.e. Not Deleted, Tombstone, etc) */
>+		replmd_deletion_state(module, target_msg,
>+				      target_deletion_state, NULL);
>+	}
>+
>+	talloc_free(tmp_ctx);
>+	return ret;
>+}
>+
> /*
>   process one linked attribute structure
>  */
>@@ -6638,7 +6737,6 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
> 	struct drsuapi_DsReplicaLinkedAttribute *la = la_entry->la;
> 	struct ldb_context *ldb = ldb_module_get_ctx(module);
> 	struct ldb_message *msg;
>-	struct ldb_message *target_msg = NULL;
> 	TALLOC_CTX *tmp_ctx = talloc_new(la_entry);
> 	const struct dsdb_schema *schema = dsdb_get_schema(ldb, tmp_ctx);
> 	int ret;
>@@ -6649,17 +6747,15 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
> 	WERROR status;
> 	time_t t = time(NULL);
> 	struct ldb_result *res;
>-	struct ldb_result *target_res;
> 	const char *attrs[4];
>-	const char *attrs2[] = { "isDeleted", "isRecycled", NULL };
> 	struct parsed_dn *pdn_list, *pdn, *next;
> 	struct GUID guid = GUID_zero();
>-	NTSTATUS ntstatus;
> 	bool active = (la->flags & DRSUAPI_DS_LINKED_ATTRIBUTE_FLAG_ACTIVE)?true:false;
> 
> 	enum deletion_state deletion_state = OBJECT_NOT_DELETED;
> 	enum deletion_state target_deletion_state = OBJECT_NOT_DELETED;
> 
>+
> /*
> linked_attributes[0]:
>      &objs->linked_attributes[i]: struct drsuapi_DsReplicaLinkedAttribute
>@@ -6779,74 +6875,14 @@ linked_attributes[0]:
> 		return LDB_ERR_OPERATIONS_ERROR;
> 	}
> 
>-	ntstatus = dsdb_get_extended_dn_guid(dsdb_dn->dn, &guid, "GUID");
>-	if (!NT_STATUS_IS_OK(ntstatus) && !active) {
>-		/*
>-		 * This strange behaviour (allowing a NULL/missing
>-		 * GUID) originally comes from:
>-		 *
>-		 * commit e3054ce0fe0f8f62d2f5b2a77893e7a1479128bd
>-		 * Author: Andrew Tridgell <tridge@samba.org>
>-		 * Date:   Mon Dec 21 21:21:55 2009 +1100
>-		 *
>-		 *  s4-drs: cope better with NULL GUIDS from DRS
>-		 *
>-		 *  It is valid to get a NULL GUID over DRS for a deleted forward link. We
>-		 *  need to match by DN if possible when seeing if we should update an
>-		 *  existing link.
>-		 *
>-		 *  Pair-Programmed-With: Andrew Bartlett <abartlet@samba.org>
>-		 */
>-
>-		ret = dsdb_module_search_dn(module, tmp_ctx, &target_res,
>-					    dsdb_dn->dn, attrs2,
>-					    DSDB_FLAG_NEXT_MODULE |
>-					    DSDB_SEARCH_SHOW_RECYCLED |
>-					    DSDB_SEARCH_SEARCH_ALL_PARTITIONS |
>-					    DSDB_SEARCH_SHOW_DN_IN_STORAGE_FORMAT,
>-					    parent);
>-	} else if (!NT_STATUS_IS_OK(ntstatus)) {
>-		ldb_asprintf_errstring(ldb, "Failed to find GUID in linked attribute blob for %s on %s from %s",
>-				       old_el->name,
>-				       ldb_dn_get_linearized(dsdb_dn->dn),
>-				       ldb_dn_get_linearized(msg->dn));
>-		talloc_free(tmp_ctx);
>-		return LDB_ERR_OPERATIONS_ERROR;
>-	} else {
>-		ret = dsdb_module_search(module, tmp_ctx, &target_res,
>-					 NULL, LDB_SCOPE_SUBTREE,
>-					 attrs2,
>-					 DSDB_FLAG_NEXT_MODULE |
>-					 DSDB_SEARCH_SHOW_RECYCLED |
>-					 DSDB_SEARCH_SEARCH_ALL_PARTITIONS |
>-					 DSDB_SEARCH_SHOW_DN_IN_STORAGE_FORMAT,
>-					 parent,
>-					 "objectGUID=%s",
>-					 GUID_string(tmp_ctx, &guid));
>-	}
>+	ret = replmd_check_target_exists(module, dsdb_dn, la_entry, msg->dn,
>+					 &guid, &target_deletion_state);
> 
> 	if (ret != LDB_SUCCESS) {
>-		ldb_asprintf_errstring(ldb_module_get_ctx(module), "Failed to re-resolve GUID %s: %s\n",
>-				       GUID_string(tmp_ctx, &guid),
>-				       ldb_errstring(ldb_module_get_ctx(module)));
> 		talloc_free(tmp_ctx);
> 		return ret;
> 	}
> 
>-	if (target_res->count == 0) {
>-		DEBUG(2,(__location__ ": WARNING: Failed to re-resolve GUID %s - using %s\n",
>-			 GUID_string(tmp_ctx, &guid),
>-			 ldb_dn_get_linearized(dsdb_dn->dn)));
>-	} else if (target_res->count != 1) {
>-		ldb_asprintf_errstring(ldb_module_get_ctx(module), "More than one object found matching objectGUID %s\n",
>-				       GUID_string(tmp_ctx, &guid));
>-		talloc_free(tmp_ctx);
>-		return LDB_ERR_OPERATIONS_ERROR;
>-	} else {
>-		target_msg = target_res->msgs[0];
>-		dsdb_dn->dn = talloc_steal(dsdb_dn, target_msg->dn);
>-	}
>-
> 	/*
> 	 * Check for deleted objects per MS-DRSR 4.1.10.6.13
> 	 * ProcessLinkValue, because link updates are not applied to
>@@ -6854,9 +6890,6 @@ linked_attributes[0]:
> 	 * any existing link, that should have happened when the
> 	 * object deletion was replicated or initiated.
> 	 */
>-	replmd_deletion_state(module, target_msg,
>-			      &target_deletion_state, NULL);
>-
> 	if (target_deletion_state >= OBJECT_RECYCLED) {
> 		talloc_free(tmp_ctx);
> 		return LDB_SUCCESS;
>-- 
>1.9.1
>
>
>From 456bc3bc9ced09b98483a2f535d7daeb9f859b7e Mon Sep 17 00:00:00 2001
>From: Tim Beale <timbeale@catalyst.net.nz>
>Date: Wed, 14 Jun 2017 11:35:36 +1200
>Subject: [PATCH 09/20] drs: Fail replication transaction instead of dropping
> links
>
>If the DRS client received a linked attribute that it couldn't resolve
>the target for, then it would just ignore that link and keep going. That
>link would then be lost forever (although a full-sync would resolve
>this). Instead of silently ignoring the link, fail the transaction.
>
>This *can* happen on Samba, but it is unusual. The target object and
>linked-attribute would need to be added while a replication is still in
>progress. It can also happen fairly easily when talking to a Windows DC.
>
>There are two import exceptions to this:
>
>1). Linked attributes that span partitions. We can never guarantee that
>we will have received the target object, because it may be in a partition
>we haven't replicated yet. Samba doesn't have a great way of handling
>this currently, but we shouldn't fail the replication (because that breaks
>basic join tests). Just skip that linked attribute and hope that a
>subsequent full-sync will fix it.
>(I queried Microsoft and they said resolving cross-partition linked
>attributes is a implementation-specific problem to solve. GET_TGT won't
>resolve it)
>
>2). When the replication involves a subset of objects, e.g.
>critical-only. In these cases, we don't increase the highwater-mark, so
>it is probably not such a dire problem if we don't add the link. In the
>case of critical-only, we will do a subsequent full sync which will then
>add the links.
>
>Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
>Reviewed-by: Garming Sam <garming@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12972
>(cherry picked from commit f69596cd21d8106afdf494f39d1fcebea2b0f5dd)
>---
> source4/dsdb/repl/drepl_out_helpers.c           |  4 ++
> source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 92 ++++++++++++++++++++++++-
> source4/dsdb/samdb/samdb.h                      |  2 +-
> source4/libnet/libnet_vampire.c                 |  1 +
> 4 files changed, 95 insertions(+), 4 deletions(-)
>
>diff --git a/source4/dsdb/repl/drepl_out_helpers.c b/source4/dsdb/repl/drepl_out_helpers.c
>index 079edc8..ad4801a 100644
>--- a/source4/dsdb/repl/drepl_out_helpers.c
>+++ b/source4/dsdb/repl/drepl_out_helpers.c
>@@ -795,6 +795,10 @@ static void dreplsrv_op_pull_source_apply_changes_trigger(struct tevent_req *req
> 	if (state->op->options & DRSUAPI_DRS_SPECIAL_SECRET_PROCESSING) {
> 		dsdb_repl_flags |= DSDB_REPL_FLAG_EXPECT_NO_SECRETS;
> 	}
>+	if (state->op->options & DRSUAPI_DRS_CRITICAL_ONLY ||
>+	    state->op->extended_op != DRSUAPI_EXOP_NONE) {
>+		dsdb_repl_flags |= DSDB_REPL_FLAG_OBJECT_SUBSET;
>+	}
> 
> 	if (state->op->extended_op != DRSUAPI_EXOP_NONE) {
> 		ret = dsdb_find_nc_root(service->samdb, partition,
>diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>index 53e2c12..dc16674 100644
>--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>@@ -74,6 +74,7 @@ struct replmd_private {
> struct la_entry {
> 	struct la_entry *next, *prev;
> 	struct drsuapi_DsReplicaLinkedAttribute *la;
>+	bool incomplete_replica;
> };
> 
> struct replmd_replicated_request {
>@@ -6535,6 +6536,7 @@ static int replmd_extended_replicated_objects(struct ldb_module *module, struct
> 	struct ldb_control **ctrls;
> 	int ret;
> 	uint32_t i;
>+	bool incomplete_subset;
> 	struct replmd_private *replmd_private =
> 		talloc_get_type(ldb_module_get_private(module), struct replmd_private);
> 
>@@ -6592,6 +6594,7 @@ static int replmd_extended_replicated_objects(struct ldb_module *module, struct
> 
> 	ar->controls = req->controls;
> 	req->controls = ctrls;
>+	incomplete_subset = (ar->objs->dsdb_repl_flags & DSDB_REPL_FLAG_OBJECT_SUBSET);
> 
> 	DEBUG(4,("linked_attributes_count=%u\n", objs->linked_attributes_count));
> 
>@@ -6616,6 +6619,13 @@ static int replmd_extended_replicated_objects(struct ldb_module *module, struct
> 		}
> 		*la_entry->la = ar->objs->linked_attributes[i];
> 
>+		/*
>+		 * we may not be able to resolve link targets properly when
>+		 * dealing with subsets of objects, e.g. the source is a
>+		 * critical object and the target isn't
>+		 */
>+		la_entry->incomplete_replica = incomplete_subset;
>+
> 		/* we need to steal the non-scalars so they stay
> 		   around until the end of the transaction */
> 		talloc_steal(la_entry->la, la_entry->la->identifier);
>@@ -6628,6 +6638,46 @@ static int replmd_extended_replicated_objects(struct ldb_module *module, struct
> }
> 
> /**
>+ * Returns True if the source and target DNs both have the same naming context,
>+ * i.e. they're both in the same partition.
>+ */
>+static bool replmd_objects_have_same_nc(struct ldb_context *ldb,
>+					TALLOC_CTX *mem_ctx,
>+					struct ldb_dn *source_dn,
>+					struct ldb_dn *target_dn)
>+{
>+	TALLOC_CTX *tmp_ctx;
>+	struct ldb_dn *source_nc;
>+	struct ldb_dn *target_nc;
>+	int ret;
>+	bool same_nc = true;
>+
>+	tmp_ctx = talloc_new(mem_ctx);
>+
>+	ret = dsdb_find_nc_root(ldb, tmp_ctx, source_dn, &source_nc);
>+	if (ret != LDB_SUCCESS) {
>+		DBG_ERR("Failed to find base DN for source %s\n",
>+			ldb_dn_get_linearized(source_dn));
>+		talloc_free(tmp_ctx);
>+		return true;
>+	}
>+
>+	ret = dsdb_find_nc_root(ldb, tmp_ctx, target_dn, &target_nc);
>+	if (ret != LDB_SUCCESS) {
>+		DBG_ERR("Failed to find base DN for target %s\n",
>+			ldb_dn_get_linearized(target_dn));
>+		talloc_free(tmp_ctx);
>+		return true;
>+	}
>+
>+	same_nc = (ldb_dn_compare(source_nc, target_nc) == 0);
>+
>+	talloc_free(tmp_ctx);
>+
>+	return same_nc;
>+}
>+
>+/**
>  * Checks that the target object for a linked attribute exists.
>  * If it exists, its GUID and deletion-state are returned
>  */
>@@ -6705,9 +6755,45 @@ static int replmd_check_target_exists(struct ldb_module *module,
> 	}
> 
> 	if (target_res->count == 0) {
>-		DEBUG(2,(__location__ ": WARNING: Failed to re-resolve GUID %s - using %s\n",
>-			 GUID_string(tmp_ctx, guid),
>-			 ldb_dn_get_linearized(dsdb_dn->dn)));
>+
>+		/*
>+		 * TODO:
>+		 * When we implement Trusted Domains we need to consider
>+		 * whether they get treated as an incomplete replica here or not
>+		 */
>+		if (la_entry->incomplete_replica) {
>+
>+			/*
>+			 * If we're only replicating a subset of objects (e.g.
>+			 * critical-only, single-object), then an unknown target
>+			 * is probably not a critical problem. We don't increase
>+			 * the highwater-mark so subsequent replications should
>+			 * resolve any missing links
>+			 */
>+			DEBUG(2,(__location__
>+				 ": Failed to find target %s linked from %s\n",
>+				 ldb_dn_get_linearized(dsdb_dn->dn),
>+				 ldb_dn_get_linearized(source_dn)));
>+
>+		} else if (replmd_objects_have_same_nc(ldb, tmp_ctx, source_dn,
>+						       dsdb_dn->dn)) {
>+			ldb_asprintf_errstring(ldb, "Unknown target %s GUID %s linked from %s\n",
>+					       ldb_dn_get_linearized(dsdb_dn->dn),
>+					       GUID_string(tmp_ctx, guid),
>+					       ldb_dn_get_linearized(source_dn));
>+			ret = LDB_ERR_NO_SUCH_OBJECT;
>+		} else {
>+
>+			/*
>+			 * TODO:
>+			 * We don't handle cross-partition links well here (we
>+			 * could potentially lose them), but don't fail the
>+			 * replication.
>+			 */
>+			DEBUG(2,("Failed to resolve cross-partition link between %s and %s\n",
>+				 ldb_dn_get_linearized(source_dn),
>+				 ldb_dn_get_linearized(dsdb_dn->dn)));
>+		}
> 	} else if (target_res->count != 1) {
> 		ldb_asprintf_errstring(ldb, "More than one object found matching objectGUID %s\n",
> 				       GUID_string(tmp_ctx, guid));
>diff --git a/source4/dsdb/samdb/samdb.h b/source4/dsdb/samdb/samdb.h
>index c8658dc..2abaf4a 100644
>--- a/source4/dsdb/samdb/samdb.h
>+++ b/source4/dsdb/samdb/samdb.h
>@@ -64,7 +64,7 @@ struct dsdb_control_current_partition {
> #define DSDB_REPL_FLAG_PARTIAL_REPLICA     2
> #define DSDB_REPL_FLAG_ADD_NCNAME	   4
> #define DSDB_REPL_FLAG_EXPECT_NO_SECRETS   8
>-
>+#define DSDB_REPL_FLAG_OBJECT_SUBSET       16
> 
> #define DSDB_CONTROL_REPLICATED_UPDATE_OID "1.3.6.1.4.1.7165.4.3.3"
> 
>diff --git a/source4/libnet/libnet_vampire.c b/source4/libnet/libnet_vampire.c
>index 7f25a3a..d89256b 100644
>--- a/source4/libnet/libnet_vampire.c
>+++ b/source4/libnet/libnet_vampire.c
>@@ -660,6 +660,7 @@ WERROR libnet_vampire_cb_store_chunk(void *private_data,
> 		 */
> 		ZERO_STRUCT(s_dsa->highwatermark);
> 		uptodateness_vector = NULL;
>+		dsdb_repl_flags |= DSDB_REPL_FLAG_OBJECT_SUBSET;
> 	}
> 
> 	/* TODO: avoid hardcoded flags */
>-- 
>1.9.1
>
>
>From 103e7d9e695a1d82afea21ce5b406f72e0340ffc Mon Sep 17 00:00:00 2001
>From: Tim Beale <timbeale@catalyst.net.nz>
>Date: Thu, 15 Jun 2017 14:09:27 +1200
>Subject: [PATCH 10/20] drs: Check target object is known after applying
> objects
>
>Currently we only check that the target object is known at the end of
>the transaction (i.e. the .prepare_commit hook). It's too late at this
>point to resend the request with GET_TGT. Move this processing earlier
>on, after we've applied all the objects (i.e. off the .extended hook).
>
>In reality, we need to perform the checks at both points. I've
>split the common code that gets the source/target details out of the
>la_entry into a helper function. It's not the greatest function ever,
>but seemed to make more sense than duplicating the code.
>
>Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
>Reviewed-by: Garming Sam <garming@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12972
>(cherry picked from commit 67617d470028b45c722c598944a17591cab1e6ba)
>---
> source4/dsdb/repl/replicated_objects.c          |  11 +-
> source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 178 ++++++++++++++++++------
> 2 files changed, 145 insertions(+), 44 deletions(-)
>
>diff --git a/source4/dsdb/repl/replicated_objects.c b/source4/dsdb/repl/replicated_objects.c
>index 862fafa..dd84570 100644
>--- a/source4/dsdb/repl/replicated_objects.c
>+++ b/source4/dsdb/repl/replicated_objects.c
>@@ -884,12 +884,15 @@ WERROR dsdb_replicated_objects_commit(struct ldb_context *ldb,
> 			dsdb_reference_schema(ldb, cur_schema, false);
> 		}
> 
>-		if (!W_ERROR_EQUAL(objects->error, WERR_DS_DRA_MISSING_PARENT)) {
>-			DEBUG(1,("Failed to apply records: %s: %s\n",
>-				 ldb_errstring(ldb), ldb_strerror(ret)));
>-		} else {
>+		if (W_ERROR_EQUAL(objects->error, WERR_DS_DRA_RECYCLED_TARGET)) {
>+			DEBUG(3,("Missing target while attempting to apply records: %s\n",
>+				 ldb_errstring(ldb)));
>+		} else if (W_ERROR_EQUAL(objects->error, WERR_DS_DRA_MISSING_PARENT)) {
> 			DEBUG(3,("Missing parent while attempting to apply records: %s\n",
> 				 ldb_errstring(ldb)));
>+		} else {
>+			DEBUG(1,("Failed to apply records: %s: %s\n",
>+				 ldb_errstring(ldb), ldb_strerror(ret)));
> 		}
> 		ldb_transaction_cancel(ldb);
> 		TALLOC_FREE(tmp_ctx);
>diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>index dc16674..c04358f 100644
>--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>@@ -114,6 +114,7 @@ static int replmd_check_upgrade_links(struct ldb_context *ldb,
> 				      struct parsed_dn *dns, uint32_t count,
> 				      struct ldb_message_element *el,
> 				      const char *ldap_oid);
>+static int replmd_verify_linked_attributes(struct replmd_replicated_request *ar);
> 
> enum urgent_situation {
> 	REPL_URGENT_ON_CREATE = 1,
>@@ -6057,7 +6058,18 @@ static int replmd_replicated_apply_next(struct replmd_replicated_request *ar)
> 	struct GUID_txt_buf guid_str_buf;
> 
> 	if (ar->index_current >= ar->objs->num_objects) {
>-		/* done with it, go to next stage */
>+
>+		/*
>+		 * Now that we've applied all the objects, check that the new
>+		 * linked attributes only reference objects we know about
>+		 */
>+		ret = replmd_verify_linked_attributes(ar);
>+
>+		if (ret != LDB_SUCCESS) {
>+			return ret;
>+		}
>+
>+		/* done applying objects, move on to the next stage */
> 		return replmd_replicated_uptodate_vector(ar);
> 	}
> 
>@@ -6812,35 +6824,29 @@ static int replmd_check_target_exists(struct ldb_module *module,
> 	return ret;
> }
> 
>-/*
>-  process one linked attribute structure
>+/**
>+ * Extracts the key details about the source/target object for a
>+ * linked-attribute entry.
>+ * This returns the following details:
>+ * @param ret_attr the schema details for the linked attribute
>+ * @param source_msg the search result for the source object
>+ * @param target_dsdb_dn the unpacked DN info for the target object
>  */
>-static int replmd_process_linked_attribute(struct ldb_module *module,
>-					   struct replmd_private *replmd_private,
>+static int replmd_extract_la_entry_details(struct ldb_module *module,
> 					   struct la_entry *la_entry,
>-					   struct ldb_request *parent)
>+					   TALLOC_CTX *mem_ctx,
>+					   const struct dsdb_attribute **ret_attr,
>+					   struct ldb_message **source_msg,
>+					   struct dsdb_dn **target_dsdb_dn)
> {
> 	struct drsuapi_DsReplicaLinkedAttribute *la = la_entry->la;
> 	struct ldb_context *ldb = ldb_module_get_ctx(module);
>-	struct ldb_message *msg;
>-	TALLOC_CTX *tmp_ctx = talloc_new(la_entry);
>-	const struct dsdb_schema *schema = dsdb_get_schema(ldb, tmp_ctx);
>+	const struct dsdb_schema *schema = dsdb_get_schema(ldb, mem_ctx);
> 	int ret;
> 	const struct dsdb_attribute *attr;
>-	struct dsdb_dn *dsdb_dn;
>-	uint64_t seq_num = 0;
>-	struct ldb_message_element *old_el;
> 	WERROR status;
>-	time_t t = time(NULL);
> 	struct ldb_result *res;
> 	const char *attrs[4];
>-	struct parsed_dn *pdn_list, *pdn, *next;
>-	struct GUID guid = GUID_zero();
>-	bool active = (la->flags & DRSUAPI_DS_LINKED_ATTRIBUTE_FLAG_ACTIVE)?true:false;
>-
>-	enum deletion_state deletion_state = OBJECT_NOT_DELETED;
>-	enum deletion_state target_deletion_state = OBJECT_NOT_DELETED;
>-
> 
> /*
> linked_attributes[0]:
>@@ -6885,7 +6891,6 @@ linked_attributes[0]:
> 				       la->attid,
> 				       GUID_buf_string(&la->identifier->guid,
> 						       &guid_str));
>-		talloc_free(tmp_ctx);
> 		return LDB_ERR_OPERATIONS_ERROR;
> 	}
> 
>@@ -6894,28 +6899,130 @@ linked_attributes[0]:
> 	attrs[2] = "isRecycled";
> 	attrs[3] = NULL;
> 
>-	/* get the existing message from the db for the object with
>-	   this GUID, returning attribute being modified. We will then
>-	   use this msg as the basis for a modify call */
>-	ret = dsdb_module_search(module, tmp_ctx, &res, NULL, LDB_SCOPE_SUBTREE, attrs,
>+	/*
>+	 * get the existing message from the db for the object with
>+	 * this GUID, returning attribute being modified. We will then
>+	 * use this msg as the basis for a modify call
>+	 */
>+	ret = dsdb_module_search(module, mem_ctx, &res, NULL, LDB_SCOPE_SUBTREE, attrs,
> 	                         DSDB_FLAG_NEXT_MODULE |
> 				 DSDB_SEARCH_SEARCH_ALL_PARTITIONS |
> 				 DSDB_SEARCH_SHOW_RECYCLED |
> 				 DSDB_SEARCH_SHOW_DN_IN_STORAGE_FORMAT |
> 				 DSDB_SEARCH_REVEAL_INTERNALS,
>-				 parent,
>-				 "objectGUID=%s", GUID_string(tmp_ctx, &la->identifier->guid));
>+				 NULL,
>+				 "objectGUID=%s", GUID_string(mem_ctx, &la->identifier->guid));
> 	if (ret != LDB_SUCCESS) {
>-		talloc_free(tmp_ctx);
> 		return ret;
> 	}
> 	if (res->count != 1) {
> 		ldb_asprintf_errstring(ldb, "DRS linked attribute for GUID %s - DN not found",
>-				       GUID_string(tmp_ctx, &la->identifier->guid));
>-		talloc_free(tmp_ctx);
>+				       GUID_string(mem_ctx, &la->identifier->guid));
> 		return LDB_ERR_NO_SUCH_OBJECT;
> 	}
>-	msg = res->msgs[0];
>+
>+	*source_msg = res->msgs[0];
>+
>+	/* the value blob for the attribute holds the target object DN */
>+	status = dsdb_dn_la_from_blob(ldb, attr, schema, mem_ctx, la->value.blob, target_dsdb_dn);
>+	if (!W_ERROR_IS_OK(status)) {
>+		ldb_asprintf_errstring(ldb, "Failed to parsed linked attribute blob for %s on %s - %s\n",
>+				       attr->lDAPDisplayName,
>+				       ldb_dn_get_linearized(res->msgs[0]->dn),
>+				       win_errstr(status));
>+		return LDB_ERR_OPERATIONS_ERROR;
>+	}
>+
>+	*ret_attr = attr;
>+
>+	return LDB_SUCCESS;
>+}
>+
>+/**
>+ * Verifies that the source and target objects are known for each linked
>+ * attribute in the current transaction.
>+ */
>+static int replmd_verify_linked_attributes(struct replmd_replicated_request *ar)
>+{
>+	int ret = LDB_SUCCESS;
>+	struct la_entry *la;
>+	struct ldb_module *module = ar->module;
>+	TALLOC_CTX *tmp_ctx = talloc_new(ar);
>+	struct replmd_private *replmd_private =
>+		talloc_get_type(ldb_module_get_private(module), struct replmd_private);
>+
>+	for (la = DLIST_TAIL(replmd_private->la_list); la; la = DLIST_PREV(la)) {
>+		struct ldb_message *src_msg;
>+		const struct dsdb_attribute *attr;
>+		struct dsdb_dn *tgt_dsdb_dn;
>+		enum deletion_state del_state = OBJECT_NOT_DELETED;
>+		struct GUID guid = GUID_zero();
>+
>+		ret = replmd_extract_la_entry_details(module, la, tmp_ctx, &attr,
>+						      &src_msg, &tgt_dsdb_dn);
>+
>+		if (ret != LDB_SUCCESS) {
>+			break;
>+		}
>+
>+		ret = replmd_check_target_exists(module, tgt_dsdb_dn, la,
>+						 src_msg->dn, &guid, &del_state);
>+
>+		/*
>+		 * When we fail to find the target object, the error code we pass
>+		 * back here is really important. It flags back to the callers to
>+		 * retry this request with DRSUAPI_DRS_GET_TGT
>+		 */
>+		if (ret == LDB_ERR_NO_SUCH_OBJECT) {
>+			ret = replmd_replicated_request_werror(ar, WERR_DS_DRA_RECYCLED_TARGET);
>+		}
>+
>+		if (ret != LDB_SUCCESS) {
>+			break;
>+		}
>+	}
>+
>+	talloc_free(tmp_ctx);
>+	return ret;
>+}
>+
>+/*
>+  process one linked attribute structure
>+ */
>+static int replmd_process_linked_attribute(struct ldb_module *module,
>+					   struct replmd_private *replmd_private,
>+					   struct la_entry *la_entry,
>+					   struct ldb_request *parent)
>+{
>+	struct drsuapi_DsReplicaLinkedAttribute *la = la_entry->la;
>+	struct ldb_context *ldb = ldb_module_get_ctx(module);
>+	struct ldb_message *msg;
>+	TALLOC_CTX *tmp_ctx = talloc_new(la_entry);
>+	const struct dsdb_schema *schema = dsdb_get_schema(ldb, tmp_ctx);
>+	int ret;
>+	const struct dsdb_attribute *attr;
>+	struct dsdb_dn *dsdb_dn;
>+	uint64_t seq_num = 0;
>+	struct ldb_message_element *old_el;
>+	time_t t = time(NULL);
>+	struct parsed_dn *pdn_list, *pdn, *next;
>+	struct GUID guid = GUID_zero();
>+	bool active = (la->flags & DRSUAPI_DS_LINKED_ATTRIBUTE_FLAG_ACTIVE)?true:false;
>+
>+	enum deletion_state deletion_state = OBJECT_NOT_DELETED;
>+	enum deletion_state target_deletion_state = OBJECT_NOT_DELETED;
>+
>+	/*
>+	 * get the attribute being modified, the search result for the source object,
>+	 * and the target object's DN details
>+	 */
>+	ret = replmd_extract_la_entry_details(module, la_entry, tmp_ctx, &attr,
>+					      &msg, &dsdb_dn);
>+
>+	if (ret != LDB_SUCCESS) {
>+		talloc_free(tmp_ctx);
>+		return ret;
>+	}
> 
> 	/*
> 	 * Check for deleted objects per MS-DRSR 4.1.10.6.13
>@@ -6924,7 +7031,6 @@ linked_attributes[0]:
> 	 * any existing link, that should have happened when the
> 	 * object deletion was replicated or initiated.
> 	 */
>-
> 	replmd_deletion_state(module, msg, &deletion_state, NULL);
> 
> 	if (deletion_state >= OBJECT_RECYCLED) {
>@@ -6953,14 +7059,6 @@ linked_attributes[0]:
> 		return ret;
> 	}
> 
>-	status = dsdb_dn_la_from_blob(ldb, attr, schema, tmp_ctx, la->value.blob, &dsdb_dn);
>-	if (!W_ERROR_IS_OK(status)) {
>-		ldb_asprintf_errstring(ldb, "Failed to parsed linked attribute blob for %s on %s - %s\n",
>-				       old_el->name, ldb_dn_get_linearized(msg->dn), win_errstr(status));
>-		talloc_free(tmp_ctx);
>-		return LDB_ERR_OPERATIONS_ERROR;
>-	}
>-
> 	ret = replmd_check_target_exists(module, dsdb_dn, la_entry, msg->dn,
> 					 &guid, &target_deletion_state);
> 
>-- 
>1.9.1
>
>
>From dedce122c8aa1f62f6f359ef5b49443b450cb2a7 Mon Sep 17 00:00:00 2001
>From: Tim Beale <timbeale@catalyst.net.nz>
>Date: Thu, 15 Jun 2017 16:52:33 +1200
>Subject: [PATCH 11/20] drepl: Support GET_TGT on periodic replication client
>
>- Update IDL comments to include Microsoft reference doc
>- Add support for sending v10 GetNCChanges request (needed for the
>  GET_TGT flag, which is in the new 'more_flags' field)
>- Update to also set the GET_TGT flag in the same place we were setting
>  GET_ANC (I split this logic out into a separate function).
>- The state struct now needs to hold a 'more_flags' field as well (this
>  flag is different to the GET_ANC replica flag)
>
>Note that using the GET_TGT when replicating from a Windows DC could be
>highly inefficient. Because Samba keeps the GET_TGT flag set throughout
>the replication cycle, it will basically receive a repeated object from
>Windows for every single linked attribute that it receives.
>
>I believe Windows behaviour only expects the client to set the GET_TGT
>flag when it actually needs to (i.e. when it receives a target object it
>doesn't know about), rather than throughout the replication cycle.
>However, this approach won't work with Samba-to-Samba replication,
>because when the server receives the GET_TGT flag it restarts the
>replication cycle from scratch. So if we only set the GET_TGT flag when
>the client encountered an unknown target then Samba-to-Samba could
>potentially get into an endless replication loop.
>
>Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
>Reviewed-by: Garming Sam <garming@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12972
>(cherry picked from commit cc201c2c4f5d2dfd18f68143d001c5ba02c00b32)
>---
> librpc/idl/drsuapi.idl                |  4 +-
> source4/dsdb/repl/drepl_out_helpers.c | 97 +++++++++++++++++++++++++++++------
> source4/dsdb/repl/drepl_out_pull.c    |  1 +
> source4/dsdb/repl/drepl_service.h     |  2 +
> 4 files changed, 86 insertions(+), 18 deletions(-)
>
>diff --git a/librpc/idl/drsuapi.idl b/librpc/idl/drsuapi.idl
>index 2eb1d65..51ef567 100644
>--- a/librpc/idl/drsuapi.idl
>+++ b/librpc/idl/drsuapi.idl
>@@ -70,7 +70,9 @@ interface drsuapi
>         } drsuapi_DrsUpdate;
> 
> 	/*****************/
>-        /* Function 0x00 */
>+        /* Function 0x00 drsuapi_DsBind() */
>+
>+        /* MS-DRSR 5.39 DRS_EXTENSIONS_INT */
>         typedef [bitmap32bit] bitmap {
> 		DRSUAPI_SUPPORTED_EXTENSION_BASE			= 0x00000001,
> 		DRSUAPI_SUPPORTED_EXTENSION_ASYNC_REPLICATION		= 0x00000002,
>diff --git a/source4/dsdb/repl/drepl_out_helpers.c b/source4/dsdb/repl/drepl_out_helpers.c
>index ad4801a..6e24e7f 100644
>--- a/source4/dsdb/repl/drepl_out_helpers.c
>+++ b/source4/dsdb/repl/drepl_out_helpers.c
>@@ -555,7 +555,28 @@ static void dreplsrv_op_pull_source_get_changes_trigger(struct tevent_req *req)
> 	}
> 
> 	r->in.bind_handle	= &drsuapi->bind_handle;
>-	if (drsuapi->remote_info28.supported_extensions & DRSUAPI_SUPPORTED_EXTENSION_GETCHGREQ_V8) {
>+
>+	if (drsuapi->remote_info28.supported_extensions & DRSUAPI_SUPPORTED_EXTENSION_GETCHGREQ_V10) {
>+		r->in.level				= 10;
>+		r->in.req->req10.destination_dsa_guid	= service->ntds_guid;
>+		r->in.req->req10.source_dsa_invocation_id= rf1->source_dsa_invocation_id;
>+		r->in.req->req10.naming_context		= &partition->nc;
>+		r->in.req->req10.highwatermark		= highwatermark;
>+		r->in.req->req10.uptodateness_vector	= uptodateness_vector;
>+		r->in.req->req10.replica_flags		= replica_flags;
>+		r->in.req->req10.max_object_count	= 133;
>+		r->in.req->req10.max_ndr_size		= 1336811;
>+		r->in.req->req10.extended_op		= state->op->extended_op;
>+		r->in.req->req10.fsmo_info		= state->op->fsmo_info;
>+		r->in.req->req10.partial_attribute_set	= pas;
>+		r->in.req->req10.partial_attribute_set_ex= NULL;
>+		r->in.req->req10.mapping_ctr.num_mappings= mappings == NULL ? 0 : mappings->num_mappings;
>+		r->in.req->req10.mapping_ctr.mappings	= mappings == NULL ? NULL : mappings->mappings;
>+
>+		/* the only difference to v8 is the more_flags */
>+		r->in.req->req10.more_flags = state->op->more_flags;
>+
>+	} else if (drsuapi->remote_info28.supported_extensions & DRSUAPI_SUPPORTED_EXTENSION_GETCHGREQ_V8) {
> 		r->in.level				= 8;
> 		r->in.req->req8.destination_dsa_guid	= service->ntds_guid;
> 		r->in.req->req8.source_dsa_invocation_id= rf1->source_dsa_invocation_id;
>@@ -693,6 +714,53 @@ static void dreplsrv_op_pull_source_get_changes_done(struct tevent_req *subreq)
> 	dreplsrv_op_pull_source_apply_changes_trigger(req, r, ctr_level, ctr1, ctr6);
> }
> 
>+/**
>+ * If processing a chunk of replication data fails, check if it is due to a
>+ * problem that can be fixed by setting extra flags in the GetNCChanges request,
>+ * i.e. GET_ANC or GET_TGT.
>+ * @returns NT_STATUS_OK if the request was retried, and an error code if not
>+ */
>+static NTSTATUS dreplsrv_op_pull_retry_with_flags(struct tevent_req *req,
>+						  WERROR error_code)
>+{
>+	struct dreplsrv_op_pull_source_state *state;
>+	NTSTATUS nt_status = NT_STATUS_OK;
>+
>+	state = tevent_req_data(req, struct dreplsrv_op_pull_source_state);
>+
>+	/*
>+	 * Check if we failed to apply the records due to a missing parent or
>+	 * target object. If so, try again and ask for any mising parent/target
>+	 * objects to be included this time.
>+	 */
>+	if (W_ERROR_EQUAL(error_code, WERR_DS_DRA_RECYCLED_TARGET)) {
>+
>+		if (state->op->more_flags & DRSUAPI_DRS_GET_TGT) {
>+			DEBUG(1,("Missing target object despite setting DRSUAPI_DRS_GET_TGT flag\n"));
>+			nt_status = NT_STATUS_INVALID_NETWORK_RESPONSE;
>+		} else {
>+			state->op->more_flags |= DRSUAPI_DRS_GET_TGT;
>+			DEBUG(1,("Missing target object when we didn't set the DRSUAPI_DRS_GET_TGT flag, retrying\n"));
>+			dreplsrv_op_pull_source_get_changes_trigger(req);
>+		}
>+	} else if (W_ERROR_EQUAL(error_code, WERR_DS_DRA_MISSING_PARENT)) {
>+
>+		if (state->op->options & DRSUAPI_DRS_GET_ANC) {
>+			DEBUG(1,("Missing parent object despite setting DRSUAPI_DRS_GET_ANC flag\n"));
>+			nt_status = NT_STATUS_INVALID_NETWORK_RESPONSE;
>+		} else {
>+			state->op->options |= DRSUAPI_DRS_GET_ANC;
>+			DEBUG(4,("Missing parent object when we didn't set the DRSUAPI_DRS_GET_ANC flag, retrying\n"));
>+			dreplsrv_op_pull_source_get_changes_trigger(req);
>+		}
>+	} else {
>+		nt_status = werror_to_ntstatus(WERR_BAD_NET_RESP);
>+	}
>+
>+	return nt_status;
>+}
>+
>+
> static void dreplsrv_update_refs_trigger(struct tevent_req *req);
> 
> static void dreplsrv_op_pull_source_apply_changes_trigger(struct tevent_req *req,
>@@ -937,25 +1005,20 @@ static void dreplsrv_op_pull_source_apply_changes_trigger(struct tevent_req *req
> 	if (!W_ERROR_IS_OK(status)) {
> 
> 		/*
>-		 * If we failed to apply the records due to a missing
>-		 * parent, try again after asking for the parent
>-		 * records first.  Because we don't update the
>-		 * highwatermark, we start this part of the cycle
>-		 * again.
>+		 * Check if this error can be fixed by resending the GetNCChanges
>+		 * request with extra flags set (i.e. GET_ANC/GET_TGT)
> 		 */
>-		if (((state->op->options & DRSUAPI_DRS_GET_ANC) == 0)
>-		    && W_ERROR_EQUAL(status, WERR_DS_DRA_MISSING_PARENT)) {
>-			state->op->options |= DRSUAPI_DRS_GET_ANC;
>-			DEBUG(4,("Missing parent object when we didn't set the DRSUAPI_DRS_GET_ANC flag, retrying\n"));
>-			dreplsrv_op_pull_source_get_changes_trigger(req);
>+		nt_status = dreplsrv_op_pull_retry_with_flags(req, status);
>+
>+		if (NT_STATUS_IS_OK(nt_status)) {
>+
>+			/*
>+			 * We resent the request. Don't update the highwatermark,
>+			 * we'll start this part of the cycle again.
>+			 */
> 			return;
>-		} else if (((state->op->options & DRSUAPI_DRS_GET_ANC))
>-			   && W_ERROR_EQUAL(status, WERR_DS_DRA_MISSING_PARENT)) {
>-			DEBUG(1,("Missing parent object despite setting DRSUAPI_DRS_GET_ANC flag\n"));
>-			nt_status = NT_STATUS_INVALID_NETWORK_RESPONSE;
>-		} else {
>-			nt_status = werror_to_ntstatus(WERR_BAD_NET_RESP);
> 		}
>+
> 		DEBUG(0,("Failed to commit objects: %s/%s\n",
> 			  win_errstr(status), nt_errstr(nt_status)));
> 		tevent_req_nterror(req, nt_status);
>diff --git a/source4/dsdb/repl/drepl_out_pull.c b/source4/dsdb/repl/drepl_out_pull.c
>index 8af6412..78442a9 100644
>--- a/source4/dsdb/repl/drepl_out_pull.c
>+++ b/source4/dsdb/repl/drepl_out_pull.c
>@@ -126,6 +126,7 @@ WERROR dreplsrv_schedule_partition_pull_source(struct dreplsrv_service *s,
> 	op->callback    = callback;
> 	op->cb_data	= cb_data;
> 	op->schedule_time = time(NULL);
>+	op->more_flags	= 0;
> 
> 	DLIST_ADD_END(s->ops.pending, op);
> 
>diff --git a/source4/dsdb/repl/drepl_service.h b/source4/dsdb/repl/drepl_service.h
>index 6b85ad6..5fc894f 100644
>--- a/source4/dsdb/repl/drepl_service.h
>+++ b/source4/dsdb/repl/drepl_service.h
>@@ -130,6 +130,8 @@ struct dreplsrv_out_operation {
> 	enum drsuapi_DsExtendedError extended_ret;
> 	dreplsrv_extended_callback_t callback;
> 	void *cb_data;
>+	/* more replication flags - used by DsReplicaSync GET_TGT */
>+	uint32_t more_flags;
> };
> 
> struct dreplsrv_notify_operation {
>-- 
>1.9.1
>
>
>From 29e76ff39ed215cc58ec89f72e502c2087bf2bcc Mon Sep 17 00:00:00 2001
>From: Tim Beale <timbeale@catalyst.net.nz>
>Date: Fri, 16 Jun 2017 09:49:16 +1200
>Subject: [PATCH 12/20] replmd: Set GET_ANC if Windows sends a link with
> unknown source object
>
>Windows replication can send the linked attribute before it sends the
>source object. The MS-DRSR spec says that in this case the client should
>resend the GetNCChanges request with the GET_ANC flag set. In my testing
>this resolves the problem - Windows will include the source object for the
>linked attribute in the same replication chunk.
>
>This problem doesn't happen with Samba-to-Samba replication, because the
>source object for the linked attribute is guaranteed to have already been
>sent.
>
>Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
>Reviewed-by: Garming Sam <garming@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12972
>(cherry picked from commit f87332eb35638cc38f83c580d4623ab978088601)
>---
> source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>index c04358f..fde9b72 100644
>--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>@@ -6961,6 +6961,17 @@ static int replmd_verify_linked_attributes(struct replmd_replicated_request *ar)
> 		ret = replmd_extract_la_entry_details(module, la, tmp_ctx, &attr,
> 						      &src_msg, &tgt_dsdb_dn);
> 
>+		/*
>+		 * When we fail to find the source object, the error code we pass
>+		 * back here is really important. It flags back to the callers to
>+		 * retry this request with DRSUAPI_DRS_GET_ANC. This case should
>+		 * never happen if we're replicating from a Samba DC, but it is
>+		 * needed to talk to a Windows DC
>+		 */
>+		if (ret == LDB_ERR_NO_SUCH_OBJECT) {
>+			ret = replmd_replicated_request_werror(ar, WERR_DS_DRA_MISSING_PARENT);
>+		}
>+
> 		if (ret != LDB_SUCCESS) {
> 			break;
> 		}
>-- 
>1.9.1
>
>
>From 5214d264df575e40a4a6626f5ab532cc3b6dfd05 Mon Sep 17 00:00:00 2001
>From: Tim Beale <timbeale@catalyst.net.nz>
>Date: Fri, 16 Jun 2017 12:54:32 +1200
>Subject: [PATCH 13/20] drs_utils: Add GET_TGT support to 'samba-tool drs
> replicate --local'
>
>Update drs_Replicate.replicate() so it handles being passed the GET_TGT
>flag (more_flags). To do this, we need to always use a v10 GetNCChanges
>request (v8 and v10 are essentially the same except for the more_flags).
>
>If the replicate_chunk() call into the C bindings throws an error, check
>to see whether the error could be fixed by setting the GET_TGT flag, and
>re-send the request if so.
>
>Unfortunately because WERR_DS_DRA_RECYCLED_TARGET isn't documented with
>the other AD error codes, I've left it hardcoded for now (Microsoft
>should be fixing up their Docs).
>
>Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
>Reviewed-by: Garming Sam <garming@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12972
>(cherry picked from commit f812c29d4000ef35fa5d7f6007606ca53c5ed37a)
>---
> python/samba/drs_utils.py | 101 ++++++++++++++++++++++++++++++----------------
> 1 file changed, 66 insertions(+), 35 deletions(-)
>
>diff --git a/python/samba/drs_utils.py b/python/samba/drs_utils.py
>index b9ed059..bed8817 100644
>--- a/python/samba/drs_utils.py
>+++ b/python/samba/drs_utils.py
>@@ -21,6 +21,8 @@ from samba.dcerpc import drsuapi, misc, drsblobs
> from samba.net import Net
> from samba.ndr import ndr_unpack
> from samba import dsdb
>+from samba import werror
>+from samba import WERRORError
> import samba, ldb
> 
> 
>@@ -198,18 +200,37 @@ class drs_Replicate(object):
>             raise RuntimeError("Must not set GUID 00000000-0000-0000-0000-000000000000 as invocation_id")
>         self.replication_state = self.net.replicate_init(self.samdb, lp, self.drs, invocation_id)
> 
>+    def _should_retry_with_get_tgt(self, error_code, req):
>+
>+        # If the error indicates we fail to resolve a target object for a
>+        # linked attribute, then we should retry the request with GET_TGT
>+        # (if we support it and haven't already tried that)
>+
>+        # TODO fix up the below line when we next update werror_err_table.txt
>+        # and pull in the new error-code
>+        # return (error_code == werror.WERR_DS_DRA_RECYCLED_TARGET and
>+        return (error_code == 0x21bf and
>+                (req.more_flags & drsuapi.DRSUAPI_DRS_GET_TGT) == 0 and
>+                self.supported_extensions & drsuapi.DRSUAPI_SUPPORTED_EXTENSION_GETCHGREQ_V10)
>+
>     def replicate(self, dn, source_dsa_invocation_id, destination_dsa_guid,
>                   schema=False, exop=drsuapi.DRSUAPI_EXOP_NONE, rodc=False,
>-                  replica_flags=None, full_sync=True, sync_forced=False):
>+                  replica_flags=None, full_sync=True, sync_forced=False, more_flags=0):
>         '''replicate a single DN'''
> 
>         # setup for a GetNCChanges call
>-        req8 = drsuapi.DsGetNCChangesRequest8()
>+        if self.supported_extensions & drsuapi.DRSUAPI_SUPPORTED_EXTENSION_GETCHGREQ_V10:
>+            req = drsuapi.DsGetNCChangesRequest10()
>+            req.more_flags = more_flags
>+            req_level = 10
>+        else:
>+            req_level = 8
>+            req = drsuapi.DsGetNCChangesRequest8()
> 
>-        req8.destination_dsa_guid = destination_dsa_guid
>-        req8.source_dsa_invocation_id = source_dsa_invocation_id
>-        req8.naming_context = drsuapi.DsReplicaObjectIdentifier()
>-        req8.naming_context.dn = dn
>+        req.destination_dsa_guid = destination_dsa_guid
>+        req.source_dsa_invocation_id = source_dsa_invocation_id
>+        req.naming_context = drsuapi.DsReplicaObjectIdentifier()
>+        req.naming_context.dn = dn
> 
>         # Default to a full replication if we don't find an upToDatenessVector
>         udv = None
>@@ -244,49 +265,46 @@ class drs_Replicate(object):
>             udv.cursors = cursors_v1
>             udv.count = len(cursors_v1)
> 
>-        req8.highwatermark = hwm
>-        req8.uptodateness_vector = udv
>+        req.highwatermark = hwm
>+        req.uptodateness_vector = udv
> 
>         if replica_flags is not None:
>-            req8.replica_flags = replica_flags
>+            req.replica_flags = replica_flags
>         elif exop == drsuapi.DRSUAPI_EXOP_REPL_SECRET:
>-            req8.replica_flags = 0
>+            req.replica_flags = 0
>         else:
>-            req8.replica_flags = (drsuapi.DRSUAPI_DRS_INIT_SYNC |
>-                                  drsuapi.DRSUAPI_DRS_PER_SYNC |
>-                                  drsuapi.DRSUAPI_DRS_GET_ANC |
>-                                  drsuapi.DRSUAPI_DRS_NEVER_SYNCED |
>-                                  drsuapi.DRSUAPI_DRS_GET_ALL_GROUP_MEMBERSHIP)
>+            req.replica_flags = (drsuapi.DRSUAPI_DRS_INIT_SYNC |
>+                                 drsuapi.DRSUAPI_DRS_PER_SYNC |
>+                                 drsuapi.DRSUAPI_DRS_GET_ANC |
>+                                 drsuapi.DRSUAPI_DRS_NEVER_SYNCED |
>+                                 drsuapi.DRSUAPI_DRS_GET_ALL_GROUP_MEMBERSHIP)
>             if rodc:
>-                req8.replica_flags |= (
>-                    drsuapi.DRSUAPI_DRS_SPECIAL_SECRET_PROCESSING)
>+                req.replica_flags |= (
>+                     drsuapi.DRSUAPI_DRS_SPECIAL_SECRET_PROCESSING)
>             else:
>-                req8.replica_flags |= drsuapi.DRSUAPI_DRS_WRIT_REP
>+                req.replica_flags |= drsuapi.DRSUAPI_DRS_WRIT_REP
> 
>         if sync_forced:
>-            req8.replica_flags |= drsuapi.DRSUAPI_DRS_SYNC_FORCED
>+            req.replica_flags |= drsuapi.DRSUAPI_DRS_SYNC_FORCED
> 
>-        req8.max_object_count = 402
>-        req8.max_ndr_size = 402116
>-        req8.extended_op = exop
>-        req8.fsmo_info = 0
>-        req8.partial_attribute_set = None
>-        req8.partial_attribute_set_ex = None
>-        req8.mapping_ctr.num_mappings = 0
>-        req8.mapping_ctr.mappings = None
>+        req.max_object_count = 402
>+        req.max_ndr_size = 402116
>+        req.extended_op = exop
>+        req.fsmo_info = 0
>+        req.partial_attribute_set = None
>+        req.partial_attribute_set_ex = None
>+        req.mapping_ctr.num_mappings = 0
>+        req.mapping_ctr.mappings = None
> 
>         if not schema and rodc:
>-            req8.partial_attribute_set = drs_get_rodc_partial_attribute_set(self.samdb)
>+            req.partial_attribute_set = drs_get_rodc_partial_attribute_set(self.samdb)
> 
>-        if self.supported_extensions & drsuapi.DRSUAPI_SUPPORTED_EXTENSION_GETCHGREQ_V8:
>-            req_level = 8
>-            req = req8
>-        else:
>+        if not self.supported_extensions & drsuapi.DRSUAPI_SUPPORTED_EXTENSION_GETCHGREQ_V8:
>             req_level = 5
>             req5 = drsuapi.DsGetNCChangesRequest5()
>             for a in dir(req5):
>                 if a[0] != '_':
>-                    setattr(req5, a, getattr(req8, a))
>+                    setattr(req5, a, getattr(req, a))
>             req = req5
> 
>         num_objects = 0
>@@ -295,8 +313,21 @@ class drs_Replicate(object):
>             (level, ctr) = self.drs.DsGetNCChanges(self.drs_handle, req_level, req)
>             if ctr.first_object is None and ctr.object_count != 0:
>                 raise RuntimeError("DsGetNCChanges: NULL first_object with object_count=%u" % (ctr.object_count))
>-            self.net.replicate_chunk(self.replication_state, level, ctr,
>-                schema=schema, req_level=req_level, req=req)
>+
>+            try:
>+                self.net.replicate_chunk(self.replication_state, level, ctr,
>+                    schema=schema, req_level=req_level, req=req)
>+            except WERRORError as e:
>+                # Check if retrying with the GET_TGT flag set might resolve this error
>+                if self._should_retry_with_get_tgt(e[0], req):
>+
>+                    print("Missing target object - retrying with DRS_GET_TGT")
>+                    req.more_flags |= drsuapi.DRSUAPI_DRS_GET_TGT
>+
>+                    # try sending the request again
>+                    continue
>+                else:
>+                    raise e
> 
>             num_objects += ctr.object_count
> 
>-- 
>1.9.1
>
>
>From ea45aef2875aca3a68b47c9ea848ec62e265f3ac Mon Sep 17 00:00:00 2001
>From: Tim Beale <timbeale@catalyst.net.nz>
>Date: Thu, 22 Jun 2017 10:43:31 +1200
>Subject: [PATCH 14/20] replmd: Move where we store linked attributes
>
>There was a bug in my previous patch where the code would verify
>*all* links in the list, rather than just the ones that are new. And it
>would do this for every replication chunk it received, regardless of
>whether there were actually any links in that chunk.
>
>The problem is by the time we want to verify the attributes, we don't
>actually know which attributes are new. We can fix this by moving where
>we store the linked attributes from the start of processing the
>replication chunk to the end of processing the chunk. We can then verify
>the new linked attributes at the same time we store them.
>
>Longer-term we may want to try to apply the linked attribute at this
>point. This would save looking up the source/target objects twice, but
>it makes things a bit more complicated (attributes will usually apply at
>this point *most* of the time, but not *all* the time).
>
>Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
>Reviewed-by: Garming Sam <garming@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12972
>(cherry picked from commit 18ea167adef59b80982f13135947e58cb56a4f16)
>---
> source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 193 +++++++++++++-----------
> 1 file changed, 103 insertions(+), 90 deletions(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>index fde9b72..0a53d1d 100644
>--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>@@ -114,7 +114,8 @@ static int replmd_check_upgrade_links(struct ldb_context *ldb,
> 				      struct parsed_dn *dns, uint32_t count,
> 				      struct ldb_message_element *el,
> 				      const char *ldap_oid);
>-static int replmd_verify_linked_attributes(struct replmd_replicated_request *ar);
>+static int replmd_verify_linked_attribute(struct replmd_replicated_request *ar,
>+					  struct la_entry *la);
> 
> enum urgent_situation {
> 	REPL_URGENT_ON_CREATE = 1,
>@@ -6042,6 +6043,70 @@ static int replmd_replicated_apply_search_callback(struct ldb_request *req,
> 	return LDB_SUCCESS;
> }
> 
>+/**
>+ * Stores the linked attributes received in the replication chunk - these get
>+ * applied at the end of the transaction. We also check that each linked
>+ * attribute is valid, i.e. source and target objects are known.
>+ */
>+static int replmd_store_linked_attributes(struct replmd_replicated_request *ar)
>+{
>+	int ret = LDB_SUCCESS;
>+	uint32_t i;
>+	bool incomplete_subset;
>+	struct ldb_module *module = ar->module;
>+	struct replmd_private *replmd_private =
>+		talloc_get_type(ldb_module_get_private(module), struct replmd_private);
>+	struct ldb_context *ldb;
>+
>+	ldb = ldb_module_get_ctx(module);
>+	incomplete_subset = (ar->objs->dsdb_repl_flags & DSDB_REPL_FLAG_OBJECT_SUBSET);
>+
>+	DEBUG(4,("linked_attributes_count=%u\n", ar->objs->linked_attributes_count));
>+
>+	/* save away the linked attributes for the end of the transaction */
>+	for (i = 0; i < ar->objs->linked_attributes_count; i++) {
>+		struct la_entry *la_entry;
>+
>+		if (replmd_private->la_ctx == NULL) {
>+			replmd_private->la_ctx = talloc_new(replmd_private);
>+		}
>+		la_entry = talloc(replmd_private->la_ctx, struct la_entry);
>+		if (la_entry == NULL) {
>+			ldb_oom(ldb);
>+			return LDB_ERR_OPERATIONS_ERROR;
>+		}
>+		la_entry->la = talloc(la_entry, struct drsuapi_DsReplicaLinkedAttribute);
>+		if (la_entry->la == NULL) {
>+			talloc_free(la_entry);
>+			ldb_oom(ldb);
>+			return LDB_ERR_OPERATIONS_ERROR;
>+		}
>+		*la_entry->la = ar->objs->linked_attributes[i];
>+
>+		/*
>+		 * we may not be able to resolve link targets properly when
>+		 * dealing with subsets of objects, e.g. the source is a
>+		 * critical object and the target isn't
>+		 */
>+		la_entry->incomplete_replica = incomplete_subset;
>+
>+		/* we need to steal the non-scalars so they stay
>+		   around until the end of the transaction */
>+		talloc_steal(la_entry->la, la_entry->la->identifier);
>+		talloc_steal(la_entry->la, la_entry->la->value.blob);
>+
>+		ret = replmd_verify_linked_attribute(ar, la_entry);
>+
>+		if (ret != LDB_SUCCESS) {
>+			break;
>+		}
>+
>+		DLIST_ADD(replmd_private->la_list, la_entry);
>+	}
>+
>+	return ret;
>+}
>+
> static int replmd_replicated_uptodate_vector(struct replmd_replicated_request *ar);
> 
> static int replmd_replicated_apply_next(struct replmd_replicated_request *ar)
>@@ -6060,10 +6125,10 @@ static int replmd_replicated_apply_next(struct replmd_replicated_request *ar)
> 	if (ar->index_current >= ar->objs->num_objects) {
> 
> 		/*
>-		 * Now that we've applied all the objects, check that the new
>-		 * linked attributes only reference objects we know about
>+		 * Now that we've applied all the objects, check the new linked
>+		 * attributes and store them (we apply them in .prepare_commit)
> 		 */
>-		ret = replmd_verify_linked_attributes(ar);
>+		ret = replmd_store_linked_attributes(ar);
> 
> 		if (ret != LDB_SUCCESS) {
> 			return ret;
>@@ -6547,10 +6612,6 @@ static int replmd_extended_replicated_objects(struct ldb_module *module, struct
> 	struct replmd_replicated_request *ar;
> 	struct ldb_control **ctrls;
> 	int ret;
>-	uint32_t i;
>-	bool incomplete_subset;
>-	struct replmd_private *replmd_private =
>-		talloc_get_type(ldb_module_get_private(module), struct replmd_private);
> 
> 	ldb = ldb_module_get_ctx(module);
> 
>@@ -6606,45 +6667,6 @@ static int replmd_extended_replicated_objects(struct ldb_module *module, struct
> 
> 	ar->controls = req->controls;
> 	req->controls = ctrls;
>-	incomplete_subset = (ar->objs->dsdb_repl_flags & DSDB_REPL_FLAG_OBJECT_SUBSET);
>-
>-	DEBUG(4,("linked_attributes_count=%u\n", objs->linked_attributes_count));
>-
>-	/* save away the linked attributes for the end of the
>-	   transaction */
>-	for (i=0; i<ar->objs->linked_attributes_count; i++) {
>-		struct la_entry *la_entry;
>-
>-		if (replmd_private->la_ctx == NULL) {
>-			replmd_private->la_ctx = talloc_new(replmd_private);
>-		}
>-		la_entry = talloc(replmd_private->la_ctx, struct la_entry);
>-		if (la_entry == NULL) {
>-			ldb_oom(ldb);
>-			return LDB_ERR_OPERATIONS_ERROR;
>-		}
>-		la_entry->la = talloc(la_entry, struct drsuapi_DsReplicaLinkedAttribute);
>-		if (la_entry->la == NULL) {
>-			talloc_free(la_entry);
>-			ldb_oom(ldb);
>-			return LDB_ERR_OPERATIONS_ERROR;
>-		}
>-		*la_entry->la = ar->objs->linked_attributes[i];
>-
>-		/*
>-		 * we may not be able to resolve link targets properly when
>-		 * dealing with subsets of objects, e.g. the source is a
>-		 * critical object and the target isn't
>-		 */
>-		la_entry->incomplete_replica = incomplete_subset;
>-
>-		/* we need to steal the non-scalars so they stay
>-		   around until the end of the transaction */
>-		talloc_steal(la_entry->la, la_entry->la->identifier);
>-		talloc_steal(la_entry->la, la_entry->la->value.blob);
>-
>-		DLIST_ADD(replmd_private->la_list, la_entry);
>-	}
> 
> 	return replmd_replicated_apply_next(ar);
> }
>@@ -6939,58 +6961,49 @@ linked_attributes[0]:
> }
> 
> /**
>- * Verifies that the source and target objects are known for each linked
>- * attribute in the current transaction.
>+ * Verifies the source and target objects are known for a linked attribute
>  */
>-static int replmd_verify_linked_attributes(struct replmd_replicated_request *ar)
>+static int replmd_verify_linked_attribute(struct replmd_replicated_request *ar,
>+					  struct la_entry *la)
> {
> 	int ret = LDB_SUCCESS;
>-	struct la_entry *la;
>+	TALLOC_CTX *tmp_ctx = talloc_new(la);
> 	struct ldb_module *module = ar->module;
>-	TALLOC_CTX *tmp_ctx = talloc_new(ar);
>-	struct replmd_private *replmd_private =
>-		talloc_get_type(ldb_module_get_private(module), struct replmd_private);
>-
>-	for (la = DLIST_TAIL(replmd_private->la_list); la; la = DLIST_PREV(la)) {
>-		struct ldb_message *src_msg;
>-		const struct dsdb_attribute *attr;
>-		struct dsdb_dn *tgt_dsdb_dn;
>-		enum deletion_state del_state = OBJECT_NOT_DELETED;
>-		struct GUID guid = GUID_zero();
>-
>-		ret = replmd_extract_la_entry_details(module, la, tmp_ctx, &attr,
>-						      &src_msg, &tgt_dsdb_dn);
>+	struct ldb_message *src_msg;
>+	const struct dsdb_attribute *attr;
>+	struct dsdb_dn *tgt_dsdb_dn;
>+	enum deletion_state del_state = OBJECT_NOT_DELETED;
>+	struct GUID guid = GUID_zero();
> 
>-		/*
>-		 * When we fail to find the source object, the error code we pass
>-		 * back here is really important. It flags back to the callers to
>-		 * retry this request with DRSUAPI_DRS_GET_ANC. This case should
>-		 * never happen if we're replicating from a Samba DC, but it is
>-		 * needed to talk to a Windows DC
>-		 */
>-		if (ret == LDB_ERR_NO_SUCH_OBJECT) {
>-			ret = replmd_replicated_request_werror(ar, WERR_DS_DRA_MISSING_PARENT);
>-		}
>+	ret = replmd_extract_la_entry_details(module, la, tmp_ctx, &attr,
>+					      &src_msg, &tgt_dsdb_dn);
> 
>-		if (ret != LDB_SUCCESS) {
>-			break;
>-		}
>+	/*
>+	 * When we fail to find the source object, the error code we pass
>+	 * back here is really important. It flags back to the callers to
>+	 * retry this request with DRSUAPI_DRS_GET_ANC. This case should
>+	 * never happen if we're replicating from a Samba DC, but it is
>+	 * needed to talk to a Windows DC
>+	 */
>+	if (ret == LDB_ERR_NO_SUCH_OBJECT) {
>+		ret = replmd_replicated_request_werror(ar, WERR_DS_DRA_MISSING_PARENT);
>+	}
> 
>-		ret = replmd_check_target_exists(module, tgt_dsdb_dn, la,
>-						 src_msg->dn, &guid, &del_state);
>+	if (ret != LDB_SUCCESS) {
>+		talloc_free(tmp_ctx);
>+		return ret;
>+	}
> 
>-		/*
>-		 * When we fail to find the target object, the error code we pass
>-		 * back here is really important. It flags back to the callers to
>-		 * retry this request with DRSUAPI_DRS_GET_TGT
>-		 */
>-		if (ret == LDB_ERR_NO_SUCH_OBJECT) {
>-			ret = replmd_replicated_request_werror(ar, WERR_DS_DRA_RECYCLED_TARGET);
>-		}
>+	ret = replmd_check_target_exists(module, tgt_dsdb_dn, la,
>+					 src_msg->dn, &guid, &del_state);
> 
>-		if (ret != LDB_SUCCESS) {
>-			break;
>-		}
>+	/*
>+	 * When we fail to find the target object, the error code we pass
>+	 * back here is really important. It flags back to the callers to
>+	 * retry this request with DRSUAPI_DRS_GET_TGT
>+	 */
>+	if (ret == LDB_ERR_NO_SUCH_OBJECT) {
>+		ret = replmd_replicated_request_werror(ar, WERR_DS_DRA_RECYCLED_TARGET);
> 	}
> 
> 	talloc_free(tmp_ctx);
>-- 
>1.9.1
>
>
>From 1f2e8c942afbef12bb106b3c7a3ff56fae4e3c7f Mon Sep 17 00:00:00 2001
>From: Tim Beale <timbeale@catalyst.net.nz>
>Date: Wed, 19 Jul 2017 16:18:20 +1200
>Subject: [PATCH 15/20] replmd: Avoid dropping links if link target is deleted
>
>The server-side can potentially send the linked attribute before the
>target-object. This happens on Microsoft, and will happen on Samba once
>server-side GET_TGT support is added. In these cases there is a hole
>where the Samba client can silently drop the linked attribute.
>
>If the old copy of the target object was deleted/recycled, then the
>client can receive the new linked attribute before it realizes the target
>has now been reincarnated. It silently ignores the linked attribute,
>thinking its receiving out of date information, when really it's the
>client's copy of the target object that's out of date.
>
>In this case we want to retry with the GET_TGT flag set, which will
>force the updated version of the target object to be sent along with the
>linked attribute. This deleted/recycled target case is the main reason
>that Windows added the GET_TGT flag.
>
>If the server sends all the links at the end, instead of along with the
>source object, then this case can still be hit. If so, it will cause the
>server to restart the replication from the beginning again. This is
>probably preferential to silently dropping links.
>
>Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
>Reviewed-by: Garming Sam <garming@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12972
>(cherry picked from commit ab12aed7e13a9c9663ba04b6dd4f02acb7bff380)
>---
> source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 52 ++++++++++++++++---------
> 1 file changed, 34 insertions(+), 18 deletions(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>index 0a53d1d..8e2c64b 100644
>--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>@@ -6713,14 +6713,16 @@ static bool replmd_objects_have_same_nc(struct ldb_context *ldb,
> 
> /**
>  * Checks that the target object for a linked attribute exists.
>- * If it exists, its GUID and deletion-state are returned
>+ * @param guid returns the target object's GUID (is returned)if it exists)
>+ * @param ignore_link set to true if the linked attribute should be ignored
>+ * (i.e. the target doesn't exist, but that it's OK to skip the link)
>  */
> static int replmd_check_target_exists(struct ldb_module *module,
> 				      struct dsdb_dn *dsdb_dn,
> 				      struct la_entry *la_entry,
> 				      struct ldb_dn *source_dn,
> 				      struct GUID *guid,
>-				      enum deletion_state *target_deletion_state)
>+				      bool *ignore_link)
> {
> 	struct drsuapi_DsReplicaLinkedAttribute *la = la_entry->la;
> 	struct ldb_context *ldb = ldb_module_get_ctx(module);
>@@ -6729,10 +6731,10 @@ static int replmd_check_target_exists(struct ldb_module *module,
> 	const char *attrs[] = { "isDeleted", "isRecycled", NULL };
> 	NTSTATUS ntstatus;
> 	int ret;
>+	enum deletion_state target_deletion_state = OBJECT_REMOVED;
> 	bool active = (la->flags & DRSUAPI_DS_LINKED_ATTRIBUTE_FLAG_ACTIVE) ? true : false;
> 
>-	*target_deletion_state = OBJECT_REMOVED;
>-
>+	*ignore_link = false;
> 	ntstatus = dsdb_get_extended_dn_guid(dsdb_dn->dn, guid, "GUID");
> 
> 	if (!NT_STATUS_IS_OK(ntstatus) && !active) {
>@@ -6808,6 +6810,7 @@ static int replmd_check_target_exists(struct ldb_module *module,
> 				 ": Failed to find target %s linked from %s\n",
> 				 ldb_dn_get_linearized(dsdb_dn->dn),
> 				 ldb_dn_get_linearized(source_dn)));
>+			*ignore_link = true;
> 
> 		} else if (replmd_objects_have_same_nc(ldb, tmp_ctx, source_dn,
> 						       dsdb_dn->dn)) {
>@@ -6827,6 +6830,7 @@ static int replmd_check_target_exists(struct ldb_module *module,
> 			DEBUG(2,("Failed to resolve cross-partition link between %s and %s\n",
> 				 ldb_dn_get_linearized(source_dn),
> 				 ldb_dn_get_linearized(dsdb_dn->dn)));
>+			*ignore_link = true;
> 		}
> 	} else if (target_res->count != 1) {
> 		ldb_asprintf_errstring(ldb, "More than one object found matching objectGUID %s\n",
>@@ -6839,7 +6843,23 @@ static int replmd_check_target_exists(struct ldb_module *module,
> 
> 		/* Get the object's state (i.e. Not Deleted, Tombstone, etc) */
> 		replmd_deletion_state(module, target_msg,
>-				      target_deletion_state, NULL);
>+				      &target_deletion_state, NULL);
>+
>+		/*
>+		 * Check for deleted objects as per MS-DRSR 4.1.10.6.14
>+		 * ProcessLinkValue(). Link updates should not be sent for
>+		 * recycled and tombstone objects (deleting the links should
>+		 * happen when we delete the object). This probably means our
>+		 * copy of the target object isn't up to date.
>+		 */
>+		if (target_deletion_state >= OBJECT_RECYCLED) {
>+			ldb_asprintf_errstring(ldb,
>+					       "Deleted target %s GUID %s linked from %s\n",
>+					       ldb_dn_get_linearized(dsdb_dn->dn),
>+					       GUID_string(tmp_ctx, guid),
>+					       ldb_dn_get_linearized(source_dn));
>+			ret = LDB_ERR_NO_SUCH_OBJECT;
>+		}
> 	}
> 
> 	talloc_free(tmp_ctx);
>@@ -6972,8 +6992,8 @@ static int replmd_verify_linked_attribute(struct replmd_replicated_request *ar,
> 	struct ldb_message *src_msg;
> 	const struct dsdb_attribute *attr;
> 	struct dsdb_dn *tgt_dsdb_dn;
>-	enum deletion_state del_state = OBJECT_NOT_DELETED;
> 	struct GUID guid = GUID_zero();
>+	bool dummy;
> 
> 	ret = replmd_extract_la_entry_details(module, la, tmp_ctx, &attr,
> 					      &src_msg, &tgt_dsdb_dn);
>@@ -6995,7 +7015,7 @@ static int replmd_verify_linked_attribute(struct replmd_replicated_request *ar,
> 	}
> 
> 	ret = replmd_check_target_exists(module, tgt_dsdb_dn, la,
>-					 src_msg->dn, &guid, &del_state);
>+					 src_msg->dn, &guid, &dummy);
> 
> 	/*
> 	 * When we fail to find the target object, the error code we pass
>@@ -7032,9 +7052,8 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
> 	struct parsed_dn *pdn_list, *pdn, *next;
> 	struct GUID guid = GUID_zero();
> 	bool active = (la->flags & DRSUAPI_DS_LINKED_ATTRIBUTE_FLAG_ACTIVE)?true:false;
>-
>+	bool ignore_link;
> 	enum deletion_state deletion_state = OBJECT_NOT_DELETED;
>-	enum deletion_state target_deletion_state = OBJECT_NOT_DELETED;
> 
> 	/*
> 	 * get the attribute being modified, the search result for the source object,
>@@ -7049,7 +7068,7 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
> 	}
> 
> 	/*
>-	 * Check for deleted objects per MS-DRSR 4.1.10.6.13
>+	 * Check for deleted objects per MS-DRSR 4.1.10.6.14
> 	 * ProcessLinkValue, because link updates are not applied to
> 	 * recycled and tombstone objects.  We don't have to delete
> 	 * any existing link, that should have happened when the
>@@ -7084,7 +7103,7 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
> 	}
> 
> 	ret = replmd_check_target_exists(module, dsdb_dn, la_entry, msg->dn,
>-					 &guid, &target_deletion_state);
>+					 &guid, &ignore_link);
> 
> 	if (ret != LDB_SUCCESS) {
> 		talloc_free(tmp_ctx);
>@@ -7092,15 +7111,12 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
> 	}
> 
> 	/*
>-	 * Check for deleted objects per MS-DRSR 4.1.10.6.13
>-	 * ProcessLinkValue, because link updates are not applied to
>-	 * recycled and tombstone objects.  We don't have to delete
>-	 * any existing link, that should have happened when the
>-	 * object deletion was replicated or initiated.
>+	 * there are some cases where the target object doesn't exist, but it's
>+	 * OK to ignore the linked attribute
> 	 */
>-	if (target_deletion_state >= OBJECT_RECYCLED) {
>+	if (ignore_link) {
> 		talloc_free(tmp_ctx);
>-		return LDB_SUCCESS;
>+		return ret;
> 	}
> 
> 	/* see if this link already exists */
>-- 
>1.9.1
>
>
>From 57fd1716a4edd3a8e8c013d1f6764de1aa202c19 Mon Sep 17 00:00:00 2001
>From: Tim Beale <timbeale@catalyst.net.nz>
>Date: Thu, 20 Jul 2017 11:14:27 +1200
>Subject: [PATCH 16/20] replmd: Don't fail cycle if we get link for deleted
> object with GET_TGT
>
>We are going to end up supporting 2 different server schemes:
>A. the old/default behaviour of sending all the linked attributes last,
>   at the end of the replication cycle.
>B. the new/Microsoft way of sending the linked attributes interleaved
>   with the source/target objects.
>
>Normally if we're talking to a server using the old scheme-A, we won't
>ever use the GET_TGT flag. However, there are a couple of cases where
>it can happen:
>- A link to a new object was added during the replication cycle.
>- An object was deleted while the replication was in progress (and
>the linked attribute got queued before the object was deleted).
>
>Talking to an Samba DC running the old scheme will just cause it to
>start the replication cycle from scratch again, which is fairly
>harmless. However, there is a chance that the same thing can happen
>again, in which case the replication cycle will fail (because GET_TGT
>was already set).
>
>Even if we're using the new scheme (B), we could still potentially hit
>this case, as we can still queue up linked attributes between requests
>(group memberships can be larger than what can fit into a single
>replication chunk).
>
>If GET_TGT is set in the GetNcChanges request, then the local copy of
>the target object should always be up-to-date when we process the linked
>attribute. So if we still think the target object is deleted/recycled at
>this point, then it's safe to ignore the linked attribute (because we
>know our local copy is up-to-date). This logic matches the MS spec logic
>in ProcessLinkValue().
>
>Not failing the replication cycle may be beneficial if we're trying to
>do a full-sync of a large database. Otherwise it might be time-consuming
>and frustrating to repeat the sync unnecessarily.
>
>Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
>Reviewed-by: Garming Sam <garming@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12972
>(cherry picked from commit 89cf5c3f76e214fbd06ce461470eb64b338c5144)
>---
> source4/dsdb/repl/drepl_out_helpers.c           |  4 ++
> source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 51 +++++++++++++++----------
> source4/dsdb/samdb/samdb.h                      |  1 +
> source4/libnet/libnet_vampire.c                 |  4 ++
> 4 files changed, 39 insertions(+), 21 deletions(-)
>
>diff --git a/source4/dsdb/repl/drepl_out_helpers.c b/source4/dsdb/repl/drepl_out_helpers.c
>index 6e24e7f..6630804 100644
>--- a/source4/dsdb/repl/drepl_out_helpers.c
>+++ b/source4/dsdb/repl/drepl_out_helpers.c
>@@ -868,6 +868,10 @@ static void dreplsrv_op_pull_source_apply_changes_trigger(struct tevent_req *req
> 		dsdb_repl_flags |= DSDB_REPL_FLAG_OBJECT_SUBSET;
> 	}
> 
>+	if (state->op->more_flags & DRSUAPI_DRS_GET_TGT) {
>+		dsdb_repl_flags |= DSDB_REPL_FLAG_TARGETS_UPTODATE;
>+	}
>+
> 	if (state->op->extended_op != DRSUAPI_EXOP_NONE) {
> 		ret = dsdb_find_nc_root(service->samdb, partition,
> 					partition->dn, &nc_root);
>diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>index 8e2c64b..228622c 100644
>--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>@@ -74,7 +74,7 @@ struct replmd_private {
> struct la_entry {
> 	struct la_entry *next, *prev;
> 	struct drsuapi_DsReplicaLinkedAttribute *la;
>-	bool incomplete_replica;
>+	uint32_t dsdb_repl_flags;
> };
> 
> struct replmd_replicated_request {
>@@ -6052,14 +6052,12 @@ static int replmd_store_linked_attributes(struct replmd_replicated_request *ar)
> {
> 	int ret = LDB_SUCCESS;
> 	uint32_t i;
>-	bool incomplete_subset;
> 	struct ldb_module *module = ar->module;
> 	struct replmd_private *replmd_private =
> 		talloc_get_type(ldb_module_get_private(module), struct replmd_private);
> 	struct ldb_context *ldb;
> 
> 	ldb = ldb_module_get_ctx(module);
>-	incomplete_subset = (ar->objs->dsdb_repl_flags & DSDB_REPL_FLAG_OBJECT_SUBSET);
> 
> 	DEBUG(4,("linked_attributes_count=%u\n", ar->objs->linked_attributes_count));
> 
>@@ -6082,13 +6080,7 @@ static int replmd_store_linked_attributes(struct replmd_replicated_request *ar)
> 			return LDB_ERR_OPERATIONS_ERROR;
> 		}
> 		*la_entry->la = ar->objs->linked_attributes[i];
>-
>-		/*
>-		 * we may not be able to resolve link targets properly when
>-		 * dealing with subsets of objects, e.g. the source is a
>-		 * critical object and the target isn't
>-		 */
>-		la_entry->incomplete_replica = incomplete_subset;
>+		la_entry->dsdb_repl_flags = ar->objs->dsdb_repl_flags;
> 
> 		/* we need to steal the non-scalars so they stay
> 		   around until the end of the transaction */
>@@ -6793,17 +6785,19 @@ static int replmd_check_target_exists(struct ldb_module *module,
> 	if (target_res->count == 0) {
> 
> 		/*
>+		 * we may not be able to resolve link targets properly when
>+		 * dealing with subsets of objects, e.g. the source is a
>+		 * critical object and the target isn't
>+		 *
> 		 * TODO:
> 		 * When we implement Trusted Domains we need to consider
> 		 * whether they get treated as an incomplete replica here or not
> 		 */
>-		if (la_entry->incomplete_replica) {
>+		if (la_entry->dsdb_repl_flags & DSDB_REPL_FLAG_OBJECT_SUBSET) {
> 
> 			/*
>-			 * If we're only replicating a subset of objects (e.g.
>-			 * critical-only, single-object), then an unknown target
>-			 * is probably not a critical problem. We don't increase
>-			 * the highwater-mark so subsequent replications should
>+			 * We don't increase the highwater-mark in the object
>+			 * subset cases, so subsequent replications should
> 			 * resolve any missing links
> 			 */
> 			DEBUG(2,(__location__
>@@ -6853,12 +6847,27 @@ static int replmd_check_target_exists(struct ldb_module *module,
> 		 * copy of the target object isn't up to date.
> 		 */
> 		if (target_deletion_state >= OBJECT_RECYCLED) {
>-			ldb_asprintf_errstring(ldb,
>-					       "Deleted target %s GUID %s linked from %s\n",
>-					       ldb_dn_get_linearized(dsdb_dn->dn),
>-					       GUID_string(tmp_ctx, guid),
>-					       ldb_dn_get_linearized(source_dn));
>-			ret = LDB_ERR_NO_SUCH_OBJECT;
>+
>+			if (la_entry->dsdb_repl_flags & DSDB_REPL_FLAG_TARGETS_UPTODATE) {
>+
>+				/*
>+				 * target should already be uptodate so there's no
>+				 * point retrying - it's probably just bad timing
>+				 */
>+				*ignore_link = true;
>+				DEBUG(0, ("%s is deleted but up to date. "
>+					  "Ignoring link from %s\n",
>+					  ldb_dn_get_linearized(dsdb_dn->dn),
>+					  ldb_dn_get_linearized(source_dn)));
>+
>+			} else {
>+				ldb_asprintf_errstring(ldb,
>+						       "Deleted target %s GUID %s linked from %s",
>+						       ldb_dn_get_linearized(dsdb_dn->dn),
>+						       GUID_string(tmp_ctx, guid),
>+						       ldb_dn_get_linearized(source_dn));
>+				ret = LDB_ERR_NO_SUCH_OBJECT;
>+			}
> 		}
> 	}
> 
>diff --git a/source4/dsdb/samdb/samdb.h b/source4/dsdb/samdb/samdb.h
>index 2abaf4a..01eb1f3 100644
>--- a/source4/dsdb/samdb/samdb.h
>+++ b/source4/dsdb/samdb/samdb.h
>@@ -65,6 +65,7 @@ struct dsdb_control_current_partition {
> #define DSDB_REPL_FLAG_ADD_NCNAME	   4
> #define DSDB_REPL_FLAG_EXPECT_NO_SECRETS   8
> #define DSDB_REPL_FLAG_OBJECT_SUBSET       16
>+#define DSDB_REPL_FLAG_TARGETS_UPTODATE    32
> 
> #define DSDB_CONTROL_REPLICATED_UPDATE_OID "1.3.6.1.4.1.7165.4.3.3"
> 
>diff --git a/source4/libnet/libnet_vampire.c b/source4/libnet/libnet_vampire.c
>index d89256b..aa0ec91 100644
>--- a/source4/libnet/libnet_vampire.c
>+++ b/source4/libnet/libnet_vampire.c
>@@ -647,6 +647,10 @@ WERROR libnet_vampire_cb_store_chunk(void *private_data,
> 			is_exop = true;
> 		}
> 		req_replica_flags = c->req10->replica_flags;
>+
>+		if (c->req10->more_flags & DRSUAPI_DRS_GET_TGT) {
>+			dsdb_repl_flags |= DSDB_REPL_FLAG_TARGETS_UPTODATE;
>+		}
> 		break;
> 	default:
> 		return WERR_INVALID_PARAMETER;
>-- 
>1.9.1
>
>
>From d04b81f174736f0c619b119af88be03d74f84e58 Mon Sep 17 00:00:00 2001
>From: Tim Beale <timbeale@catalyst.net.nz>
>Date: Mon, 24 Jul 2017 16:20:58 +1200
>Subject: [PATCH 17/20] replmd: Try to add forward-link for unknown
> cross-partition links
>
>Previously Samba would just drop cross-partition links where the link
>target object is unknown. Instead, what we want to do is try to add the
>forward link for the GUID specified. We can't add the backlink because
>we don't know the target, however, dbcheck should be able to fix any
>missing backlinks.
>
>The new behaviour should now mean dbcheck will detect the problem and be
>able to fix it. It's still not ideal, but it's better than dropping the
>link completely.
>
>I've updated the log so that it has higher severity and tells the user
>what they need to do to fix it.
>
>These changes now mean that the selftests now detect an error - instead
>of completely dropping the serverReference, we now have a missing
>backlink. I've updated the selftests to fix up any missing
>serverReference backlinks before running dbcheck.
>
>Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
>Reviewed-by: Garming Sam <garming@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12972
>(cherry picked from commit fae5df891c11f642cbede9e4e3d845c49c5f86b8)
>---
> source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 14 +++++++-------
> testprogs/blackbox/dbcheck.sh                   |  8 ++++++++
> 2 files changed, 15 insertions(+), 7 deletions(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>index 228622c..d5234d5 100644
>--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>@@ -374,8 +374,9 @@ static int replmd_process_backlink(struct ldb_module *module, struct la_backlink
> 	ret = dsdb_module_dn_by_guid(module, frame, &bl->target_guid, &target_dn, parent);
> 	if (ret != LDB_SUCCESS) {
> 		struct GUID_txt_buf guid_str;
>-		DEBUG(2,(__location__ ": WARNING: Failed to find target DN for linked attribute with GUID %s\n",
>-			 GUID_buf_string(&bl->target_guid, &guid_str)));
>+		DBG_WARNING("Failed to find target DN for linked attribute with GUID %s\n",
>+			    GUID_buf_string(&bl->target_guid, &guid_str));
>+		DBG_WARNING("Please run 'samba-tool dbcheck' to resolve any missing backlinks.\n");
> 		talloc_free(frame);
> 		return LDB_SUCCESS;
> 	}
>@@ -6816,15 +6817,14 @@ static int replmd_check_target_exists(struct ldb_module *module,
> 		} else {
> 
> 			/*
>-			 * TODO:
>-			 * We don't handle cross-partition links well here (we
>-			 * could potentially lose them), but don't fail the
>-			 * replication.
>+			 * The target of the cross-partition link is missing.
>+			 * Continue and try to at least add the forward-link.
>+			 * This isn't great, but if we can add a partial link
>+			 * then it's better than nothing.
> 			 */
> 			DEBUG(2,("Failed to resolve cross-partition link between %s and %s\n",
> 				 ldb_dn_get_linearized(source_dn),
> 				 ldb_dn_get_linearized(dsdb_dn->dn)));
>-			*ignore_link = true;
> 		}
> 	} else if (target_res->count != 1) {
> 		ldb_asprintf_errstring(ldb, "More than one object found matching objectGUID %s\n",
>diff --git a/testprogs/blackbox/dbcheck.sh b/testprogs/blackbox/dbcheck.sh
>index 0f979ab..387ce70 100755
>--- a/testprogs/blackbox/dbcheck.sh
>+++ b/testprogs/blackbox/dbcheck.sh
>@@ -27,6 +27,13 @@ dbcheck_fix_stale_links() {
> 	$BINDIR/samba-tool dbcheck --quiet --fix --yes remove_plausible_deleted_DN_links --attrs="member msDS-NC-Replica-Locations msDS-NC-RO-Replica-Locations" --cross-ncs $ARGS
> }
> 
>+# This list of attributes can be freely extended
>+dbcheck_fix_crosspartition_backlinks() {
>+	# we may not know the target yet when we receive a cross-partition link,
>+	# which can result in a missing backlink
>+	$BINDIR/samba-tool dbcheck --quiet --fix --yes fix_all_missing_backlinks --attrs="serverReference" --cross-ncs $ARGS
>+}
>+
> # This test shows that this does not do anything to a current
> # provision (that would be a bug)
> dbcheck_reset_well_known_acls() {
>@@ -47,6 +54,7 @@ force_modules() {
> 
> dbcheck_fix_one_way_links
> dbcheck_fix_stale_links
>+dbcheck_fix_crosspartition_backlinks
> testit "dbcheck" dbcheck
> testit "reindex" reindex
> testit "fixed_attrs" fixed_attrs
>-- 
>1.9.1
>
>
>From 6a734d3ef71405c24c355467897ab6b26a1e2419 Mon Sep 17 00:00:00 2001
>From: Tim Beale <timbeale@catalyst.net.nz>
>Date: Tue, 6 Jun 2017 18:06:22 +1200
>Subject: [PATCH 18/20] getncchanges.py: Add a new test for replication
>
>This adds a new test to check that if objects are modified during a
>replication, then those objects don't wind up missing from the
>replication data.
>
>Note that when this scenario occurs, samba returns the objects in a
>different order to Windows. This test doesn't care what order the
>replicated objects get returned in, so long as they all have been
>received by the end of the test.
>
>As part of this, I've refactored _check_replication() in drs_base.py so
>it can be reused in new tests. In these cases, the objects are split up
>over multiple different chunks. So asserting that the objects are returned
>in a specific order makes it difficult to run the same test on both Samba
>and Windows.
>
>Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
>Reviewed-by: Garming Sam <garming@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12972
>(cherry picked from commit 4cfc29688584ca69c43abb770d1e721d1eab1480)
>---
> source4/selftest/tests.py                  |  10 ++
> source4/torture/drs/python/drs_base.py     |  70 ++++++++++----
> source4/torture/drs/python/getncchanges.py | 147 +++++++++++++++++++++++++++++
> 3 files changed, 208 insertions(+), 19 deletions(-)
> create mode 100644 source4/torture/drs/python/getncchanges.py
>
>diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
>index 4bcbdc6..a73ba60 100755
>--- a/source4/selftest/tests.py
>+++ b/source4/selftest/tests.py
>@@ -853,6 +853,16 @@ for env in ['vampire_dc', 'promoted_dc', 'vampire_2000_dc']:
>                            environ={'DC1': "$DC_SERVER", 'DC2': '$%s_SERVER' % env.upper()},
>                            extra_args=['-U$DOMAIN/$DC_USERNAME%$DC_PASSWORD'])
> 
>+# A side-effect of the getncchanges tests is that they will create hundreds of
>+# tombstone objects, so run them last to avoid interferring with (and slowing
>+# down) the other DRS tests
>+for env in ['vampire_dc', 'promoted_dc']:
>+    planoldpythontestsuite(env, "getncchanges",
>+			   extra_path=[os.path.join(samba4srcdir, 'torture/drs/python')],
>+			   name="samba4.drs.getncchanges.python(%s)" % env,
>+			   environ={'DC1': "$DC_SERVER", 'DC2': '$%s_SERVER' % env.upper()},
>+			   extra_args=['-U$DOMAIN/$DC_USERNAME%$DC_PASSWORD'])
>+
> for env in ['ad_dc_ntvfs']:
>     planoldpythontestsuite(env, "repl_rodc",
>                            extra_path=[os.path.join(samba4srcdir, 'torture/drs/python')],
>diff --git a/source4/torture/drs/python/drs_base.py b/source4/torture/drs/python/drs_base.py
>index f19efca..f313abd 100644
>--- a/source4/torture/drs/python/drs_base.py
>+++ b/source4/torture/drs/python/drs_base.py
>@@ -176,17 +176,15 @@ class DrsBaseTestCase(SambaToolCmdTest):
>         id.dn = str(res[0].dn)
>         return id
> 
>-    def _check_replication(self, expected_dns, replica_flags, expected_links=[],
>-                           drs_error=drsuapi.DRSUAPI_EXOP_ERR_NONE, drs=None, drs_handle=None,
>-                           highwatermark=None, uptodateness_vector=None,
>-                           more_flags=0, more_data=False,
>-                           dn_ordered=True, links_ordered=True,
>-                           max_objects=133, exop=0,
>-                           dest_dsa=drsuapi.DRSUAPI_DS_BIND_GUID_W2K3,
>-                           source_dsa=None, invocation_id=None, nc_dn_str=None,
>-                           nc_object_count=0, nc_linked_attributes_count=0):
>+    def _get_replication(self, replica_flags,
>+                          drs_error=drsuapi.DRSUAPI_EXOP_ERR_NONE, drs=None, drs_handle=None,
>+                          highwatermark=None, uptodateness_vector=None,
>+                          more_flags=0, max_objects=133, exop=0,
>+                          dest_dsa=drsuapi.DRSUAPI_DS_BIND_GUID_W2K3,
>+                          source_dsa=None, invocation_id=None, nc_dn_str=None):
>         """
>-        Makes sure that replication returns the specific error given.
>+        Builds a DsGetNCChanges request based on the information provided
>+        and returns the response received from the DC.
>         """
>         if source_dsa is None:
>             source_dsa = self.ldb_dc1.get_ntds_GUID()
>@@ -230,12 +228,51 @@ class DrsBaseTestCase(SambaToolCmdTest):
>         self.assertEqual(level, 6, "expected level 6 response!")
>         self.assertEqual(ctr.source_dsa_guid, misc.GUID(source_dsa))
>         self.assertEqual(ctr.source_dsa_invocation_id, misc.GUID(invocation_id))
>-        ctr6 = ctr
>-        self.assertEqual(ctr6.extended_ret, drs_error)
>+        self.assertEqual(ctr.extended_ret, drs_error)
>+
>+        return ctr
>+
>+    def _check_replication(self, expected_dns, replica_flags, expected_links=[],
>+                           drs_error=drsuapi.DRSUAPI_EXOP_ERR_NONE, drs=None, drs_handle=None,
>+                           highwatermark=None, uptodateness_vector=None,
>+                           more_flags=0, more_data=False,
>+                           dn_ordered=True, links_ordered=True,
>+                           max_objects=133, exop=0,
>+                           dest_dsa=drsuapi.DRSUAPI_DS_BIND_GUID_W2K3,
>+                           source_dsa=None, invocation_id=None, nc_dn_str=None,
>+                           nc_object_count=0, nc_linked_attributes_count=0):
>+        """
>+        Makes sure that replication returns the specific error given.
>+        """
>+
>+        # send a DsGetNCChanges to the DC
>+        ctr6 = self._get_replication(replica_flags,
>+                                     drs_error, drs, drs_handle,
>+                                     highwatermark, uptodateness_vector,
>+                                     more_flags, max_objects, exop, dest_dsa,
>+                                     source_dsa, invocation_id, nc_dn_str)
>+
>+        # check the response is what we expect
>         self._check_ctr6(ctr6, expected_dns, expected_links,
>-                         nc_object_count=nc_object_count)
>+                         nc_object_count=nc_object_count, more_data=more_data,
>+                         dn_ordered=dn_ordered)
>         return (ctr6.new_highwatermark, ctr6.uptodateness_vector)
> 
>+
>+    def _get_ctr6_dn_list(self, ctr6):
>+        """
>+        Returns the DNs contained in a DsGetNCChanges response.
>+        """
>+        dn_list = []
>+        next_object = ctr6.first_object
>+        for i in range(0, ctr6.object_count):
>+            dn_list.append(next_object.object.identifier.dn)
>+            next_object = next_object.next_object
>+        self.assertEqual(next_object, None)
>+
>+        return dn_list
>+
>+
>     def _check_ctr6(self, ctr6, expected_dns=[], expected_links=[],
>                     dn_ordered=True, links_ordered=True,
>                     more_data=False, nc_object_count=0,
>@@ -250,12 +287,7 @@ class DrsBaseTestCase(SambaToolCmdTest):
>         self.assertEqual(ctr6.nc_linked_attributes_count, nc_linked_attributes_count)
>         self.assertEqual(ctr6.drs_error[0], drs_error)
> 
>-        ctr6_dns = []
>-        next_object = ctr6.first_object
>-        for i in range(0, ctr6.object_count):
>-            ctr6_dns.append(next_object.object.identifier.dn)
>-            next_object = next_object.next_object
>-        self.assertEqual(next_object, None)
>+        ctr6_dns = self._get_ctr6_dn_list(ctr6)
> 
>         i = 0
>         for dn in expected_dns:
>diff --git a/source4/torture/drs/python/getncchanges.py b/source4/torture/drs/python/getncchanges.py
>new file mode 100644
>index 0000000..d1d6b2b
>--- /dev/null
>+++ b/source4/torture/drs/python/getncchanges.py
>@@ -0,0 +1,147 @@
>+#!/usr/bin/env python
>+# -*- coding: utf-8 -*-
>+#
>+# Tests various schema replication scenarios
>+#
>+# Copyright (C) Catalyst.Net Ltd. 2017
>+#
>+# This program is free software; you can redistribute it and/or modify
>+# it under the terms of the GNU General Public License as published by
>+# the Free Software Foundation; either version 3 of the License, or
>+# (at your option) any later version.
>+#
>+# This program is distributed in the hope that it will be useful,
>+# but WITHOUT ANY WARRANTY; without even the implied warranty of
>+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>+# GNU General Public License for more details.
>+#
>+# You should have received a copy of the GNU General Public License
>+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>+#
>+
>+#
>+# Usage:
>+#  export DC1=dc1_dns_name
>+#  export DC2=dc2_dns_name
>+#  export SUBUNITRUN=$samba4srcdir/scripting/bin/subunitrun
>+#  PYTHONPATH="$PYTHONPATH:$samba4srcdir/torture/drs/python" $SUBUNITRUN getncchanges -U"$DOMAIN/$DC_USERNAME"%"$DC_PASSWORD"
>+#
>+
>+import drs_base
>+import samba.tests
>+import ldb
>+from ldb import SCOPE_BASE
>+
>+from samba.dcerpc import drsuapi
>+
>+class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
>+    def setUp(self):
>+        super(DrsReplicaSyncIntegrityTestCase, self).setUp()
>+        self.base_dn = self.ldb_dc1.get_default_basedn()
>+        self.ou = "OU=uptodateness_test,%s" % self.base_dn
>+        self.ldb_dc1.add({
>+            "dn": self.ou,
>+            "objectclass": "organizationalUnit"})
>+        (self.drs, self.drs_handle) = self._ds_bind(self.dnsname_dc1)
>+        (self.default_hwm, self.default_utdv) = self._get_highest_hwm_utdv(self.ldb_dc1)
>+        self._debug = True
>+
>+    def tearDown(self):
>+        super(DrsReplicaSyncIntegrityTestCase, self).tearDown()
>+        # tidyup groups and users
>+        try:
>+            self.ldb_dc1.delete(self.ou, ["tree_delete:1"])
>+        except ldb.LdbError as (enum, string):
>+            if enum == ldb.ERR_NO_SUCH_OBJECT:
>+                pass
>+
>+    def add_object(self, dn):
>+        """Adds an OU object"""
>+        self.ldb_dc1.add({"dn": dn, "objectclass": "organizationalunit"})
>+        res = self.ldb_dc1.search(base=dn, scope=SCOPE_BASE)
>+        self.assertEquals(len(res), 1)
>+
>+    def modify_object(self, dn, attr, value):
>+        """Modifies an object's USN by adding an attribute value to it"""
>+        m = ldb.Message()
>+        m.dn = ldb.Dn(self.ldb_dc1, dn)
>+        m[attr] = ldb.MessageElement(value, ldb.FLAG_MOD_ADD, attr)
>+        self.ldb_dc1.modify(m)
>+
>+    def create_object_range(self, start, end, prefix=""):
>+        """
>+        Creates a block of objects. Object names are numbered sequentially,
>+        using the optional prefix supplied.
>+        """
>+        dn_list = []
>+
>+        # Create the parents first, then the children.
>+        # This makes it easier to see in debug when GET_ANC takes effect
>+        # because the parent/children become interleaved (by default,
>+        # this approach means the objects are organized into blocks of
>+        # parents and blocks of children together)
>+        for x in range(start, end):
>+            ou = "OU=test_ou_%s%d,%s" % (prefix, x, self.ou)
>+            self.add_object(ou)
>+            dn_list.append(ou)
>+
>+        return dn_list
>+
>+    def assert_expected_data(self, received_list, expected_list):
>+        """
>+        Asserts that we received all the DNs that we expected and
>+        none are missing.
>+        """
>+
>+        # Note that with GET_ANC Windows can end up sending the same parent
>+        # object multiple times, so this might be noteworthy but doesn't
>+        # warrant failing the test
>+        if (len(received_list) != len(expected_list)):
>+            print("Note: received %d objects but expected %d" %(len(received_list),
>+                                                                len(expected_list)))
>+
>+        # Check that we received every object that we were expecting
>+        for dn in expected_list:
>+            self.assertTrue(dn in received_list, "DN '%s' missing from replication." % dn)
>+
>+    def test_repl_integrity(self):
>+        """
>+        Modify the objects being replicated while the replication is still
>+        in progress and check that no object loss occurs.
>+        """
>+
>+        # The server behaviour differs between samba and Windows. Samba returns
>+        # the objects in the original order (up to the pre-modify HWM). Windows
>+        # incorporates the modified objects and returns them in the new order
>+        # (i.e. modified objects last), up to the post-modify HWM. The Microsoft
>+        # docs state the Windows behaviour is optional.
>+
>+        # Create a range of objects to replicate.
>+        expected_dn_list = self.create_object_range(0, 400)
>+        (orig_hwm, unused) = self._get_highest_hwm_utdv(self.ldb_dc1)
>+
>+        # We ask for the first page of 100 objects.
>+        # For this test, we don't care what order we receive the objects in,
>+        # so long as by the end we've received everything
>+        rxd_dn_list = []
>+        ctr6 = self._get_replication(drsuapi.DRSUAPI_DRS_WRIT_REP, max_objects=100)
>+        rxd_dn_list = self._get_ctr6_dn_list(ctr6)
>+
>+        # Modify some of the second page of objects. This should bump the highwatermark
>+        for x in range(100, 200):
>+            self.modify_object(expected_dn_list[x], "displayName", "OU%d" % x)
>+
>+        (post_modify_hwm, unused) = self._get_highest_hwm_utdv(self.ldb_dc1)
>+        self.assertTrue(post_modify_hwm.highest_usn > orig_hwm.highest_usn)
>+
>+        # Get the remaining blocks of data
>+        while ctr6.more_data:
>+            ctr6 = self._get_replication(drsuapi.DRSUAPI_DRS_WRIT_REP, max_objects=100,
>+                                         highwatermark=ctr6.new_highwatermark,
>+                                         uptodateness_vector=ctr6.uptodateness_vector)
>+            rxd_dn_list += self._get_ctr6_dn_list(ctr6)
>+
>+        # Check we still receive all the objects we're expecting
>+        self.assert_expected_data(rxd_dn_list, expected_dn_list)
>+
>+
>-- 
>1.9.1
>
>
>From 7f40472f94ad85176219d01898c1272c6c15ae09 Mon Sep 17 00:00:00 2001
>From: Tim Beale <timbeale@catalyst.net.nz>
>Date: Tue, 6 Jun 2017 18:21:40 +1200
>Subject: [PATCH 19/20] getncchanges.py: Add GET_ANC replication test case
>
>This test:
>- creates blocks of parent/child objects
>- modifies the parents, so the child gets received first in the
>  replication (which means the client has to use GET_ANC)
>- checks that we always receive the parent before the child (if not, it
>  either retries with GET_ANC, or asserts if GET_ANC is already set)
>- modifies the parent objects to change their USN while the
>  replication is in progress
>- checks that all expected objects are received by the end of the
>  test
>
>I've added a repl_get_next() function to help simulate a client's
>behaviour - if it encounters an object it doesn't know the parent of,
>then it retries with GET_ANC.
>
>Also added some debug to drs_base.py that developers can turn on to make
>it easier to see what objects we're actually receiving in the
>responses.
>
>Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
>Reviewed-by: Garming Sam <garming@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12972
>(cherry picked from commit 9f0ae6e44d0f6def6be7c44067c7fcfdf0d42db2)
>---
> source4/torture/drs/python/drs_base.py     |  25 ++++
> source4/torture/drs/python/getncchanges.py | 195 +++++++++++++++++++++++++++--
> 2 files changed, 212 insertions(+), 8 deletions(-)
>
>diff --git a/source4/torture/drs/python/drs_base.py b/source4/torture/drs/python/drs_base.py
>index f313abd..9ae1f0f 100644
>--- a/source4/torture/drs/python/drs_base.py
>+++ b/source4/torture/drs/python/drs_base.py
>@@ -68,6 +68,9 @@ class DrsBaseTestCase(SambaToolCmdTest):
>         self.dnsname_dc1 = self.info_dc1["dnsHostName"][0]
>         self.dnsname_dc2 = self.info_dc2["dnsHostName"][0]
> 
>+        # for debugging the test code
>+        self._debug = False
>+
>     def tearDown(self):
>         super(DrsBaseTestCase, self).tearDown()
> 
>@@ -176,6 +179,27 @@ class DrsBaseTestCase(SambaToolCmdTest):
>         id.dn = str(res[0].dn)
>         return id
> 
>+    def _ctr6_debug(self, ctr6):
>+        """
>+        Displays basic info contained in a DsGetNCChanges response.
>+        Having this debug code allows us to see the difference in behaviour
>+        between Samba and Windows easier. Turn on the self._debug flag to see it.
>+        """
>+
>+        if self._debug:
>+            print("------------ recvd CTR6 -------------")
>+
>+            next_object = ctr6.first_object
>+            for i in range(0, ctr6.object_count):
>+                print("Obj %d: %s %s" %(i, next_object.object.identifier.dn[:22],
>+                                        next_object.object.identifier.guid))
>+                next_object = next_object.next_object
>+
>+            print("Linked Attributes: %d" % ctr6.linked_attributes_count)
>+            print("HWM:     %d" %(ctr6.new_highwatermark.highest_usn))
>+            print("Tmp HWM: %d" %(ctr6.new_highwatermark.tmp_highest_usn))
>+            print("More data: %d" %(ctr6.more_data))
>+
>     def _get_replication(self, replica_flags,
>                           drs_error=drsuapi.DRSUAPI_EXOP_ERR_NONE, drs=None, drs_handle=None,
>                           highwatermark=None, uptodateness_vector=None,
>@@ -224,6 +248,7 @@ class DrsBaseTestCase(SambaToolCmdTest):
>             uptodateness_vector_v1.cursors = cursors
>             req10.uptodateness_vector = uptodateness_vector_v1
>         (level, ctr) = drs.DsGetNCChanges(drs_handle, 10, req10)
>+        self._ctr6_debug(ctr)
> 
>         self.assertEqual(level, 6, "expected level 6 response!")
>         self.assertEqual(ctr.source_dsa_guid, misc.GUID(source_dsa))
>diff --git a/source4/torture/drs/python/getncchanges.py b/source4/torture/drs/python/getncchanges.py
>index d1d6b2b..2f914d8 100644
>--- a/source4/torture/drs/python/getncchanges.py
>+++ b/source4/torture/drs/python/getncchanges.py
>@@ -44,7 +44,14 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
>             "objectclass": "organizationalUnit"})
>         (self.drs, self.drs_handle) = self._ds_bind(self.dnsname_dc1)
>         (self.default_hwm, self.default_utdv) = self._get_highest_hwm_utdv(self.ldb_dc1)
>-        self._debug = True
>+
>+        # 100 is the minimum max_objects that Microsoft seems to honour
>+        # (the max honoured is 400ish), so we use that in these tests
>+        self.max_objects = 100
>+        self.last_ctr = None
>+
>+        # store whether we used GET_ANC flags in the requests
>+        self.used_get_anc = False
> 
>     def tearDown(self):
>         super(DrsReplicaSyncIntegrityTestCase, self).tearDown()
>@@ -68,13 +75,23 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
>         m[attr] = ldb.MessageElement(value, ldb.FLAG_MOD_ADD, attr)
>         self.ldb_dc1.modify(m)
> 
>-    def create_object_range(self, start, end, prefix=""):
>+    def create_object_range(self, start, end, prefix="",
>+                            children=None, parent_list=None):
>         """
>         Creates a block of objects. Object names are numbered sequentially,
>-        using the optional prefix supplied.
>+        using the optional prefix supplied. If the children parameter is
>+        supplied it will create a parent-child hierarchy and return the
>+        top-level parents separately.
>         """
>         dn_list = []
> 
>+        # Use dummy/empty lists if we're not creating a parent/child hierarchy
>+        if children is None:
>+            children = []
>+
>+        if parent_list is None:
>+            parent_list = []
>+
>         # Create the parents first, then the children.
>         # This makes it easier to see in debug when GET_ANC takes effect
>         # because the parent/children become interleaved (by default,
>@@ -85,6 +102,16 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
>             self.add_object(ou)
>             dn_list.append(ou)
> 
>+            # keep track of the top-level parents (if needed)
>+            parent_list.append(ou)
>+
>+        # create the block of children (if needed)
>+        for x in range(start, end):
>+            for child in children:
>+                ou = "OU=test_ou_child%s%d,%s" % (child, x, parent_list[x])
>+                self.add_object(ou)
>+                dn_list.append(ou)
>+
>         return dn_list
> 
>     def assert_expected_data(self, received_list, expected_list):
>@@ -124,7 +151,7 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
>         # For this test, we don't care what order we receive the objects in,
>         # so long as by the end we've received everything
>         rxd_dn_list = []
>-        ctr6 = self._get_replication(drsuapi.DRSUAPI_DRS_WRIT_REP, max_objects=100)
>+        ctr6 = self.repl_get_next(rxd_dn_list)
>         rxd_dn_list = self._get_ctr6_dn_list(ctr6)
> 
>         # Modify some of the second page of objects. This should bump the highwatermark
>@@ -135,13 +162,165 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
>         self.assertTrue(post_modify_hwm.highest_usn > orig_hwm.highest_usn)
> 
>         # Get the remaining blocks of data
>-        while ctr6.more_data:
>-            ctr6 = self._get_replication(drsuapi.DRSUAPI_DRS_WRIT_REP, max_objects=100,
>-                                         highwatermark=ctr6.new_highwatermark,
>-                                         uptodateness_vector=ctr6.uptodateness_vector)
>+        while not self.replication_complete():
>+            ctr6 = self.repl_get_next(rxd_dn_list)
>             rxd_dn_list += self._get_ctr6_dn_list(ctr6)
> 
>         # Check we still receive all the objects we're expecting
>         self.assert_expected_data(rxd_dn_list, expected_dn_list)
> 
>+    def is_parent_known(self, dn, known_dn_list):
>+        """
>+        Returns True if the parent of the dn specified is in known_dn_list
>+        """
>+
>+        # we can sometimes get system objects like the RID Manager returned.
>+        # Ignore anything that is not under the test OU we created
>+        if self.ou not in dn:
>+            return True
>+
>+        # Remove the child portion from the name to get the parent's DN
>+        name_substrings = dn.split(",")
>+        del name_substrings[0]
>+
>+        parent_dn = ",".join(name_substrings)
>+
>+        # check either this object is a parent (it's parent is the top-level
>+        # test object), or its parent has been seen previously
>+        return parent_dn == self.ou or parent_dn in known_dn_list
>+
>+    def repl_get_next(self, initial_objects, get_anc=False):
>+        """
>+        Requests the next block of replication data. This tries to simulate
>+        client behaviour - if we receive a replicated object that we don't know
>+        the parent of, then re-request the block with the GET_ANC flag set.
>+        """
>+
>+        # we're just trying to mimic regular client behaviour here, so just
>+        # use the highwatermark in the last response we received
>+        if self.last_ctr:
>+            highwatermark = self.last_ctr.new_highwatermark
>+            uptodateness_vector = self.last_ctr.uptodateness_vector
>+        else:
>+            # this is the initial replication, so we're starting from the start
>+            highwatermark = None
>+            uptodateness_vector = None
>+
>+        # we'll add new objects as we discover them, so take a copy to modify
>+        known_objects = initial_objects[:]
>+
>+        # Ask for the next block of replication data
>+        replica_flags = drsuapi.DRSUAPI_DRS_WRIT_REP
>+
>+        if get_anc:
>+            replica_flags = drsuapi.DRSUAPI_DRS_WRIT_REP | drsuapi.DRSUAPI_DRS_GET_ANC
>+            self.used_get_anc = True
>+
>+        ctr6 = self._get_replication(replica_flags,
>+                                     max_objects=self.max_objects,
>+                                     highwatermark=highwatermark,
>+                                     uptodateness_vector=uptodateness_vector)
>+
>+        # check that we know the parent for every object received
>+        rxd_dn_list = self._get_ctr6_dn_list(ctr6)
>+
>+        for i in range(0, len(rxd_dn_list)):
>+
>+            dn = rxd_dn_list[i]
>+
>+            if self.is_parent_known(dn, known_objects):
>+
>+                # the new DN is now known so add it to the list.
>+                # It may be the parent of another child in this block
>+                known_objects.append(dn)
>+            else:
>+                # If we've already set the GET_ANC flag then it should mean
>+                # we receive the parents before the child
>+                self.assertFalse(get_anc, "Unknown parent for object %s" % dn)
>+
>+                print("Unknown parent for %s - try GET_ANC" % dn)
>+
>+                # try the same thing again with the GET_ANC flag set this time
>+                return self.repl_get_next(get_anc=True)
>+
>+        # store the last successful result so we know what HWM to request next
>+        self.last_ctr = ctr6
>+
>+        return ctr6
>+
>+    def replication_complete(self):
>+        """Returns True if the current/last replication cycle is complete"""
>+
>+        if self.last_ctr is None or self.last_ctr.more_data:
>+            return False
>+        else:
>+            return True
>+
>+    def test_repl_integrity_get_anc(self):
>+        """
>+        Modify the parent objects being replicated while the replication is still
>+        in progress (using GET_ANC) and check that no object loss occurs.
>+        """
>+
>+        # Note that GET_ANC behaviour varies between Windows and Samba.
>+        # On Samba GET_ANC results in the replication restarting from the very
>+        # beginning. After that, Samba remembers GET_ANC and also sends the
>+        # parents in subsequent requests (regardless of whether GET_ANC is
>+        # specified in the later request).
>+        # Windows only sends the parents if GET_ANC was specified in the last
>+        # request. It will also resend a parent, even if it's already sent the
>+        # parent in a previous response (whereas Samba doesn't).
>+
>+        # Create a small block of 50 parents, each with 2 children (A and B)
>+        # This is so that we receive some children in the first block, so we
>+        # can resend with GET_ANC before we learn too many parents
>+        parent_dn_list = []
>+        expected_dn_list = self.create_object_range(0, 50, prefix="parent",
>+                                                    children=("A", "B"),
>+                                                    parent_list=parent_dn_list)
>+
>+        # create the remaining parents and children
>+        expected_dn_list += self.create_object_range(50, 150, prefix="parent",
>+                                                     children=("A", "B"),
>+                                                     parent_list=parent_dn_list)
>+
>+        # We've now got objects in the following order:
>+        # [50 parents][100 children][100 parents][200 children]
>+
>+        # Modify the first parent so that it's now ordered last by USN
>+        # This means we set the GET_ANC flag pretty much straight away
>+        # because we receive the first child before the first parent
>+        self.modify_object(parent_dn_list[0], "displayName", "OU0")
>+
>+        # modify a later block of parents so they also get reordered
>+        for x in range(50, 100):
>+            self.modify_object(parent_dn_list[x], "displayName", "OU%d" % x)
>+
>+        # Get the first block of objects - this should resend the request with
>+        # GET_ANC set because we won't know about the first child's parent.
>+        # On samba GET_ANC essentially starts the sync from scratch again, so
>+        # we get this over with early before we learn too many parents
>+        rxd_dn_list = []
>+        ctr6 = self.repl_get_next(rxd_dn_list)
>+        rxd_dn_list = self._get_ctr6_dn_list(ctr6)
>+
>+        # modify the last chunk of parents. They should now have a USN higher
>+        # than the highwater-mark for the replication cycle
>+        for x in range(100, 150):
>+            self.modify_object(parent_dn_list[x], "displayName", "OU%d" % x)
>+
>+        # Get the remaining blocks of data - this will resend the request with
>+        # GET_ANC if it encounters an object it doesn't have the parent for.
>+        while not self.replication_complete():
>+            ctr6 = self.repl_get_next(rxd_dn_list)
>+            rxd_dn_list += self._get_ctr6_dn_list(ctr6)
>+
>+        # The way the test objects have been created should force
>+        # self.repl_get_next() to use the GET_ANC flag. If this doesn't
>+        # actually happen, then the test isn't doing its job properly
>+        self.assertTrue(self.used_get_anc,
>+                        "Test didn't use the GET_ANC flag as expected")
>+
>+        # Check we get all the objects we're expecting
>+        self.assert_expected_data(rxd_dn_list, expected_dn_list)
> 
>-- 
>1.9.1
>
>
>From aeaf9f653f51091c43eb3374240e8fd1d924d443 Mon Sep 17 00:00:00 2001
>From: Tim Beale <timbeale@catalyst.net.nz>
>Date: Wed, 12 Jul 2017 10:16:00 +1200
>Subject: [PATCH 20/20] getncchanges.py: Add test for GET_ANC and linked
> attributes
>
>Add a basic test that when we use GET_ANC and the parents have linked
>attributes, then we receive all the expected links and all the expected
>objects by the end of the test.
>
>This extends the test code to track what linked attributes get received
>and check whether they match what's present on the DC.
>
>Also made some minor cleanups to store the received objects/links each
>time we successfully receive a GETNCChanges response (this saves the
>test case having to repeat this code every time).
>
>Note that although this test involves linked attributes, it shouldn't
>exercise the GET_TGT case at all.
>
>Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
>Reviewed-by: Garming Sam <garming@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12972
>(cherry picked from commit 0bb4d28272914736a46dc5cdb0bef766ba7a4ab8)
>---
> source4/torture/drs/python/drs_base.py     |  46 ++++++---
> source4/torture/drs/python/getncchanges.py | 150 ++++++++++++++++++++++++-----
> 2 files changed, 157 insertions(+), 39 deletions(-)
>
>diff --git a/source4/torture/drs/python/drs_base.py b/source4/torture/drs/python/drs_base.py
>index 9ae1f0f..6083e43 100644
>--- a/source4/torture/drs/python/drs_base.py
>+++ b/source4/torture/drs/python/drs_base.py
>@@ -179,6 +179,27 @@ class DrsBaseTestCase(SambaToolCmdTest):
>         id.dn = str(res[0].dn)
>         return id
> 
>+    def _get_ctr6_links(self, ctr6):
>+        """
>+        Unpacks the linked attributes from a DsGetNCChanges response
>+        and returns them as a list.
>+        """
>+        ctr6_links = []
>+        for lidx in range(0, ctr6.linked_attributes_count):
>+            l = ctr6.linked_attributes[lidx]
>+            try:
>+                target = ndr_unpack(drsuapi.DsReplicaObjectIdentifier3,
>+                                    l.value.blob)
>+            except:
>+                target = ndr_unpack(drsuapi.DsReplicaObjectIdentifier3Binary,
>+                                    l.value.blob)
>+            al = AbstractLink(l.attid, l.flags,
>+                              l.identifier.guid,
>+                              target.guid, target.dn)
>+            ctr6_links.append(al)
>+
>+        return ctr6_links
>+
>     def _ctr6_debug(self, ctr6):
>         """
>         Displays basic info contained in a DsGetNCChanges response.
>@@ -196,6 +217,11 @@ class DrsBaseTestCase(SambaToolCmdTest):
>                 next_object = next_object.next_object
> 
>             print("Linked Attributes: %d" % ctr6.linked_attributes_count)
>+            ctr6_links = self._get_ctr6_links(ctr6)
>+            for link in ctr6_links:
>+                print("Link Tgt %s... <-- Src %s"
>+                      %(link.targetDN[:22], link.identifier))
>+
>             print("HWM:     %d" %(ctr6.new_highwatermark.highest_usn))
>             print("Tmp HWM: %d" %(ctr6.new_highwatermark.tmp_highest_usn))
>             print("More data: %d" %(ctr6.more_data))
>@@ -325,21 +351,9 @@ class DrsBaseTestCase(SambaToolCmdTest):
>             else:
>                 self.assertTrue(dn in ctr6_dns, "Couldn't find DN '%s' anywhere in ctr6 response." % dn)
> 
>-        ctr6_links = []
>+        # Extract the links from the response
>+        ctr6_links = self._get_ctr6_links(ctr6)
>         expected_links.sort()
>-        lidx = 0
>-        for lidx in range(0, ctr6.linked_attributes_count):
>-            l = ctr6.linked_attributes[lidx]
>-            try:
>-                target = ndr_unpack(drsuapi.DsReplicaObjectIdentifier3,
>-                                    l.value.blob)
>-            except:
>-                target = ndr_unpack(drsuapi.DsReplicaObjectIdentifier3Binary,
>-                                    l.value.blob)
>-            al = AbstractLink(l.attid, l.flags,
>-                              l.identifier.guid,
>-                              target.guid)
>-            ctr6_links.append(al)
> 
>         lidx = 0
>         for el in expected_links:
>@@ -420,13 +434,15 @@ class DrsBaseTestCase(SambaToolCmdTest):
> 
> 
> class AbstractLink:
>-    def __init__(self, attid, flags, identifier, targetGUID):
>+    def __init__(self, attid, flags, identifier, targetGUID,
>+                 targetDN=""):
>         self.attid = attid
>         self.flags = flags
>         self.identifier = str(identifier)
>         self.selfGUID_blob = ndr_pack(identifier)
>         self.targetGUID = str(targetGUID)
>         self.targetGUID_blob = ndr_pack(targetGUID)
>+        self.targetDN = targetDN
> 
>     def __repr__(self):
>         return "AbstractLink(0x%08x, 0x%08x, %s, %s)" % (
>diff --git a/source4/torture/drs/python/getncchanges.py b/source4/torture/drs/python/getncchanges.py
>index 2f914d8..7d48133 100644
>--- a/source4/torture/drs/python/getncchanges.py
>+++ b/source4/torture/drs/python/getncchanges.py
>@@ -45,6 +45,9 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
>         (self.drs, self.drs_handle) = self._ds_bind(self.dnsname_dc1)
>         (self.default_hwm, self.default_utdv) = self._get_highest_hwm_utdv(self.ldb_dc1)
> 
>+        self.rxd_dn_list = []
>+        self.rxd_links = []
>+
>         # 100 is the minimum max_objects that Microsoft seems to honour
>         # (the max honoured is 400ish), so we use that in these tests
>         self.max_objects = 100
>@@ -114,11 +117,12 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
> 
>         return dn_list
> 
>-    def assert_expected_data(self, received_list, expected_list):
>+    def assert_expected_data(self, expected_list):
>         """
>         Asserts that we received all the DNs that we expected and
>         none are missing.
>         """
>+        received_list = self.rxd_dn_list
> 
>         # Note that with GET_ANC Windows can end up sending the same parent
>         # object multiple times, so this might be noteworthy but doesn't
>@@ -150,9 +154,7 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
>         # We ask for the first page of 100 objects.
>         # For this test, we don't care what order we receive the objects in,
>         # so long as by the end we've received everything
>-        rxd_dn_list = []
>-        ctr6 = self.repl_get_next(rxd_dn_list)
>-        rxd_dn_list = self._get_ctr6_dn_list(ctr6)
>+        self.repl_get_next()
> 
>         # Modify some of the second page of objects. This should bump the highwatermark
>         for x in range(100, 200):
>@@ -163,11 +165,10 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
> 
>         # Get the remaining blocks of data
>         while not self.replication_complete():
>-            ctr6 = self.repl_get_next(rxd_dn_list)
>-            rxd_dn_list += self._get_ctr6_dn_list(ctr6)
>+            self.repl_get_next()
> 
>         # Check we still receive all the objects we're expecting
>-        self.assert_expected_data(rxd_dn_list, expected_dn_list)
>+        self.assert_expected_data(expected_dn_list)
> 
>     def is_parent_known(self, dn, known_dn_list):
>         """
>@@ -189,12 +190,8 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
>         # test object), or its parent has been seen previously
>         return parent_dn == self.ou or parent_dn in known_dn_list
> 
>-    def repl_get_next(self, initial_objects, get_anc=False):
>-        """
>-        Requests the next block of replication data. This tries to simulate
>-        client behaviour - if we receive a replicated object that we don't know
>-        the parent of, then re-request the block with the GET_ANC flag set.
>-        """
>+    def _repl_send_request(self, get_anc=False):
>+        """Sends a GetNCChanges request for the next block of replication data."""
> 
>         # we're just trying to mimic regular client behaviour here, so just
>         # use the highwatermark in the last response we received
>@@ -202,13 +199,10 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
>             highwatermark = self.last_ctr.new_highwatermark
>             uptodateness_vector = self.last_ctr.uptodateness_vector
>         else:
>-            # this is the initial replication, so we're starting from the start
>+            # this is the first replication chunk
>             highwatermark = None
>             uptodateness_vector = None
> 
>-        # we'll add new objects as we discover them, so take a copy to modify
>-        known_objects = initial_objects[:]
>-
>         # Ask for the next block of replication data
>         replica_flags = drsuapi.DRSUAPI_DRS_WRIT_REP
> 
>@@ -216,14 +210,30 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
>             replica_flags = drsuapi.DRSUAPI_DRS_WRIT_REP | drsuapi.DRSUAPI_DRS_GET_ANC
>             self.used_get_anc = True
> 
>-        ctr6 = self._get_replication(replica_flags,
>+        # return the response from the DC
>+        return self._get_replication(replica_flags,
>                                      max_objects=self.max_objects,
>                                      highwatermark=highwatermark,
>                                      uptodateness_vector=uptodateness_vector)
> 
>+    def repl_get_next(self, get_anc=False):
>+        """
>+        Requests the next block of replication data. This tries to simulate
>+        client behaviour - if we receive a replicated object that we don't know
>+        the parent of, then re-request the block with the GET_ANC flag set.
>+        """
>+
>+        # send a request to the DC and get the response
>+        ctr6 = self._repl_send_request(get_anc=get_anc)
>+
>         # check that we know the parent for every object received
>         rxd_dn_list = self._get_ctr6_dn_list(ctr6)
> 
>+        # we'll add new objects as we discover them, so take a copy of the
>+        # ones we already know about, so we can modify the list safely
>+        known_objects = self.rxd_dn_list[:]
>+
>+        # check that we know the parent for every object received
>         for i in range(0, len(rxd_dn_list)):
> 
>             dn = rxd_dn_list[i]
>@@ -246,6 +256,10 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
>         # store the last successful result so we know what HWM to request next
>         self.last_ctr = ctr6
> 
>+        # store the objects and links we received
>+        self.rxd_dn_list += self._get_ctr6_dn_list(ctr6)
>+        self.rxd_links += self._get_ctr6_links(ctr6)
>+
>         return ctr6
> 
>     def replication_complete(self):
>@@ -300,9 +314,7 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
>         # GET_ANC set because we won't know about the first child's parent.
>         # On samba GET_ANC essentially starts the sync from scratch again, so
>         # we get this over with early before we learn too many parents
>-        rxd_dn_list = []
>-        ctr6 = self.repl_get_next(rxd_dn_list)
>-        rxd_dn_list = self._get_ctr6_dn_list(ctr6)
>+        self.repl_get_next()
> 
>         # modify the last chunk of parents. They should now have a USN higher
>         # than the highwater-mark for the replication cycle
>@@ -312,8 +324,7 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
>         # Get the remaining blocks of data - this will resend the request with
>         # GET_ANC if it encounters an object it doesn't have the parent for.
>         while not self.replication_complete():
>-            ctr6 = self.repl_get_next(rxd_dn_list)
>-            rxd_dn_list += self._get_ctr6_dn_list(ctr6)
>+            self.repl_get_next()
> 
>         # The way the test objects have been created should force
>         # self.repl_get_next() to use the GET_ANC flag. If this doesn't
>@@ -322,5 +333,96 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
>                         "Test didn't use the GET_ANC flag as expected")
> 
>         # Check we get all the objects we're expecting
>-        self.assert_expected_data(rxd_dn_list, expected_dn_list)
>+        self.assert_expected_data(expected_dn_list)
>+
>+    def assert_expected_links(self, objects_with_links, link_attr="managedBy"):
>+        """
>+        Asserts that a GetNCChanges response contains any expected links
>+        for the objects it contains.
>+        """
>+        received_links = self.rxd_links
>+
>+        num_expected = len(objects_with_links)
>+
>+        self.assertTrue(len(received_links) == num_expected,
>+                        "Received %d links but expected %d"
>+                        %(len(received_links), num_expected))
>+
>+        for dn in objects_with_links:
>+            self.assert_object_has_link(dn, link_attr, received_links)
>+
>+    def assert_object_has_link(self, dn, link_attr, received_links):
>+        """
>+        Queries the object in the DB and asserts there is a link in the
>+        GetNCChanges response that matches.
>+        """
>+
>+        # Look up the link attribute in the DB
>+        # The extended_dn option will dump the GUID info for the link
>+        # attribute (as a hex blob)
>+        res = self.ldb_dc1.search(ldb.Dn(self.ldb_dc1, dn), attrs=[link_attr],
>+                                  controls=['extended_dn:1:0'], scope=ldb.SCOPE_BASE)
>+
>+        # We didn't find the expected link attribute in the DB for the object.
>+        # Something has gone wrong somewhere...
>+        self.assertTrue(link_attr in res[0], "%s in DB doesn't have attribute %s"
>+                        %(dn, link_attr))
>+
>+        # find the received link in the list and assert that the target and
>+        # source GUIDs match what's in the DB
>+        for val in res[0][link_attr]:
>+            # Work out the expected source and target GUIDs for the DB link
>+            target_dn = ldb.Dn(self.ldb_dc1, val)
>+            targetGUID_blob = target_dn.get_extended_component("GUID")
>+            sourceGUID_blob = res[0].dn.get_extended_component("GUID")
>+
>+            found = False
>+
>+            for link in received_links:
>+                if link.selfGUID_blob == sourceGUID_blob and \
>+                   link.targetGUID_blob == targetGUID_blob:
>+
>+                    found = True
>+
>+                    if self._debug:
>+                        print("Link %s --> %s" %(dn[:25], link.targetDN[:25]))
>+                    break
>+
>+            self.assertTrue(found, "Did not receive expected link for DN %s" % dn)
>+
>+    def test_repl_get_anc_link_attr(self):
>+        """
>+        A basic GET_ANC test where the parents have linked attributes
>+        """
>+
>+        # Create a block of 100 parents and 100 children
>+        parent_dn_list = []
>+        expected_dn_list = self.create_object_range(0, 100, prefix="parent",
>+                                                    children=("A"),
>+                                                    parent_list=parent_dn_list)
>+
>+        # Add links from the parents to the children
>+        for x in range(0, 100):
>+            self.modify_object(parent_dn_list[x], "managedBy", expected_dn_list[x + 100])
>+
>+        # add some filler objects at the end. This allows us to easily see
>+        # which chunk the links get sent in
>+        expected_dn_list += self.create_object_range(0, 100, prefix="filler")
>+
>+        # We've now got objects in the following order:
>+        # [100 x children][100 x parents][100 x filler]
>+
>+        # Get the replication data - because the block of children come first,
>+        # this should retry the request with GET_ANC
>+        while not self.replication_complete():
>+            self.repl_get_next()
>+
>+        self.assertTrue(self.used_get_anc,
>+                        "Test didn't use the GET_ANC flag as expected")
>+
>+        # Check we get all the objects we're expecting
>+        self.assert_expected_data(expected_dn_list)
>+
>+        # Check we received links for all the parents
>+        self.assert_expected_links(parent_dn_list)
> 
>-- 
>1.9.1
>

Flags:

metze: review? (abartlet)
metze: review? (garming)

Actions: View

Attachments on bug 12972: 13480 | 13581 | 13595 | 13605