From: Tim Beale <timbeale@catalyst.net.nz>
Date: Mon, 18 Sep 2017 23:59:58 +0000 (+1200)
Subject: replmd: Partial fix for single-valued link conflict
X-Git-Tag: tevent-0.9.34~155
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=70d532a5c79dc386bf55db1d32865044872a6905;p=thirdparty%2Fsamba.git

replmd: Partial fix for single-valued link conflict

This is the first part of the fix for resolving a single-valued link
conflict.

When processing the replication data for a linked attribute, if we don't
find a match for the link target value, check if the link is a
single-valued attribute and it currently has an active link. If so, then
use the active link instead.

This change means we delete the existing active link (and backlink)
before adding the new link. This prevents the failure in the subsequent
dsdb_check_single_valued_link() check that was happening previously
(because the link would end up with 2 active values).

This is only a partial fix. It stops replication from failing completely
if we ever hit this situation (which means the test is no longer
hitting an assertion when replicating). However, ideally the existing
active link should be retained and just marked as deleted (with this
change, the existing link is overwritten completely).

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13055

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
---

diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index 73ec9c83232..2a1069c0109 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -7142,6 +7142,56 @@ static int replmd_verify_linked_attribute(struct replmd_replicated_request *ar,
 	return ret;
 }
 
+/**
+ * Finds the current active Parsed-DN value for a single-valued linked
+ * attribute, if one exists.
+ * @param ret_pdn assigned the active Parsed-DN, or NULL if none was found
+ * @returns LDB_SUCCESS (regardless of whether a match was found), unless
+ * an error occurred
+ */
+static int replmd_get_active_singleval_link(struct ldb_module *module,
+					    TALLOC_CTX *mem_ctx,
+					    struct parsed_dn pdn_list[],
+					    unsigned int count,
+					    const struct dsdb_attribute *attr,
+					    struct parsed_dn **ret_pdn)
+{
+	unsigned int i;
+
+	*ret_pdn = NULL;
+
+	if (!(attr->ldb_schema_attribute->flags & LDB_ATTR_FLAG_SINGLE_VALUE)) {
+
+		/* nothing to do for multi-valued linked attributes */
+		return LDB_SUCCESS;
+	}
+
+	for (i = 0; i < count; i++) {
+		int ret = LDB_SUCCESS;
+		struct parsed_dn *pdn = &pdn_list[i];
+
+		/* skip any inactive links */
+		if (dsdb_dn_is_deleted_val(pdn->v)) {
+			continue;
+		}
+
+		/* we've found an active value for this attribute */
+		*ret_pdn = pdn;
+
+		if (pdn->dsdb_dn == NULL) {
+			struct ldb_context *ldb = ldb_module_get_ctx(module);
+
+			ret = really_parse_trusted_dn(mem_ctx, ldb, pdn,
+						      attr->syntax->ldap_oid);
+		}
+
+		return ret;
+	}
+
+	/* no active link found */
+	return LDB_SUCCESS;
+}
+
 /**
  * @returns true if the replication linked attribute info is newer than we
  * already have in our DB
@@ -7276,6 +7326,28 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
 		return ret;
 	}
 
+	if (pdn == NULL && active) {
+
+		/*
+		 * check if there's a conflict for single-valued links, i.e.
+		 * an active linked attribute already exists, but it has a
+		 * different target value
+		 */
+		ret = replmd_get_active_singleval_link(module, tmp_ctx, pdn_list,
+						       old_el->num_values, attr,
+						       &pdn);
+		if (ret != LDB_SUCCESS) {
+			talloc_free(tmp_ctx);
+			return ret;
+		}
+
+		if (pdn != NULL) {
+			DBG_WARNING("Link conflict for %s linked from %s\n",
+				    ldb_dn_get_linearized(pdn->dsdb_dn->dn),
+				    ldb_dn_get_linearized(msg->dn));
+		}
+	}
+
 	if (!replmd_link_update_is_newer(pdn, la)) {
 		DEBUG(3,("Discarding older DRS linked attribute update to %s on %s from %s\n",
 			 old_el->name, ldb_dn_get_linearized(msg->dn),
@@ -7299,7 +7371,7 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
 			ret = replmd_add_backlink(module, replmd_private,
 						  schema, 
 						  msg->dn,
-						  &guid, false, attr,
+						  &pdn->guid, false, attr,
 						  parent);
 			if (ret != LDB_SUCCESS) {
 				talloc_free(tmp_ctx);
diff --git a/source4/torture/drs/python/link_conflicts.py b/source4/torture/drs/python/link_conflicts.py
index c41d03689d1..036472bb9ea 100644
--- a/source4/torture/drs/python/link_conflicts.py
+++ b/source4/torture/drs/python/link_conflicts.py
@@ -193,11 +193,8 @@ class DrsReplicaLinkConflictTestCase(drs_base.DrsBaseTestCase):
         self.ensure_unique_timestamp()
         self.add_link_attr(self.ldb_dc2, src_ou, "managedBy", target2_ou)
 
-        # try to sync the 2 DCs (this currently fails)
-        try:
-            self.sync_DCs(sync_order=sync_order)
-        except Exception, e:
-            self.fail("Replication could not resolve link conflict: %s" % e)
+        # sync the 2 DCs
+        self.sync_DCs(sync_order=sync_order)
 
         res1 = self.ldb_dc1.search(base="<GUID=%s>" % src_guid,
                                   scope=SCOPE_BASE, attrs=["managedBy"])