dsdb: Fix rename and RDN handling for replPropertyMetaData

This matches Windows 2012R2, which both has the RDN not sorted last and has it updated with the local
invocation_id and a local version.

The RDN attribute, unlike name, is not replicated over DRS, so the impact for interopability extends only to
the incorrect RDN values that we were finding with dbcheck (values that did not match the name values).

Finally, we always force the RDN to match the name attribute, which avoids issues
in dbcheck where these diverge.  As such, we can finally remove dbcheck as a
flapping test, last re-added in e4bab3a8282d263eb2391bc7e8a6fd64ae068935

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

2 files changed, 114 insertions, 72 deletions

diff --git a/source4/dsdb/repl/replicated_objects.c b/source4/dsdb/repl/replicated_objects.c
index 674074c350f..919dd46ca1f 100644
--- a/source4/dsdb/repl/replicated_objects.c
+++ b/source4/dsdb/repl/replicated_objects.c
@@ -357,13 +357,9 @@ WERROR dsdb_convert_object_ex(struct ldb_context *ldb,
 	NTTIME whenChanged = 0;
 	time_t whenChanged_t;
 	const char *whenChanged_s;
-	struct drsuapi_DsReplicaAttribute *name_a = NULL;
-	struct drsuapi_DsReplicaMetaData *name_d = NULL;
-	struct replPropertyMetaData1 *rdn_m = NULL;
 	struct dom_sid *sid = NULL;
 	uint32_t rid = 0;
 	uint32_t attr_count;
-	int ret;
 
 	if (!in->object.identifier) {
 		return WERR_FOOBAR;
@@ -394,7 +390,7 @@ WERROR dsdb_convert_object_ex(struct ldb_context *ldb,
 
 	msg->num_elements	= in->object.attribute_ctr.num_attributes;
 	msg->elements		= talloc_array(msg, struct ldb_message_element,
-					       msg->num_elements + 1); /* +1 because of the RDN attribute */
+					       msg->num_elements);
 	W_ERROR_HAVE_NO_MEMORY(msg->elements);
 
 	md = talloc(mem_ctx, struct replPropertyMetaDataBlob);
@@ -406,7 +402,7 @@ WERROR dsdb_convert_object_ex(struct ldb_context *ldb,
 	md->ctr.ctr1.reserved	= 0;
 	md->ctr.ctr1.array	= talloc_array(mem_ctx,
 					       struct replPropertyMetaData1,
-					       md->ctr.ctr1.count + 1); /* +1 because of the RDN attribute */
+					       md->ctr.ctr1.count);
 	W_ERROR_HAVE_NO_MEMORY(md->ctr.ctr1.array);
 
 	for (i=0, attr_count=0; i < in->meta_data_ctr->count; i++, attr_count++) {
@@ -485,73 +481,35 @@ WERROR dsdb_convert_object_ex(struct ldb_context *ldb,
 		m->originating_usn		= d->originating_usn;
 		m->local_usn			= 0;
 
-		if (d->originating_change_time > whenChanged) {
-			whenChanged = d->originating_change_time;
-		}
-
 		if (a->attid == DRSUAPI_ATTID_name) {
-			name_a = a;
-			name_d = d;
-		}
-	}
-
-	msg->num_elements = attr_count;
-	md->ctr.ctr1.count = attr_count;
-	if (name_a) {
-		rdn_m = &md->ctr.ctr1.array[md->ctr.ctr1.count];
-	}
-
-	if (rdn_m) {
-		struct ldb_message_element *el;
-		const char *rdn_name = NULL;
-		const struct ldb_val *rdn_value = NULL;
-		const struct dsdb_attribute *rdn_attr = NULL;
-		uint32_t rdn_attid;
-
-		/*
-		 * We only need the schema calls for the RDN in this
-		 * codepath, and by doing this we avoid needing to
-		 * have the dsdb_attribute_by_lDAPDisplayName accessor
-		 * working during the schema load.
-		 */
-		rdn_name	= ldb_dn_get_rdn_name(msg->dn);
-		rdn_attr	= dsdb_attribute_by_lDAPDisplayName(schema, rdn_name);
-		if (!rdn_attr) {
-			return WERR_FOOBAR;
-		}
-		rdn_attid	= rdn_attr->attributeID_id;
-		rdn_value	= ldb_dn_get_rdn_val(msg->dn);
-
-		el = ldb_msg_find_element(msg, rdn_attr->lDAPDisplayName);
-		if (!el) {
-			ret = ldb_msg_add_value(msg, rdn_attr->lDAPDisplayName, rdn_value, NULL);
-			if (ret != LDB_SUCCESS) {
+			const struct ldb_val *rdn_val = ldb_dn_get_rdn_val(msg->dn);
+			if (rdn_val == NULL) {
+				DEBUG(0, ("Unxpectedly unable to get RDN from %s for validation",
+					  ldb_dn_get_linearized(msg->dn)));
 				return WERR_FOOBAR;
 			}
-		} else {
-			if (el->num_values != 1) {
-				DEBUG(0,(__location__ ": Unexpected num_values=%u\n",
-					 el->num_values));
-				return WERR_FOOBAR;				
+			if (e->num_values != 1) {
+				DEBUG(0, ("Unxpectedly got wrong number of attribute values (got %u, expected 1) when checking RDN against name of %s",
+					  e->num_values,
+					  ldb_dn_get_linearized(msg->dn)));
+				return WERR_FOOBAR;
 			}
-			if (!ldb_val_equal_exact(&el->values[0], rdn_value)) {
-				DEBUG(0,(__location__ ": RDN value changed? '%*.*s' '%*.*s'\n",
-					 (int)el->values[0].length, (int)el->values[0].length, el->values[0].data,
-					 (int)rdn_value->length, (int)rdn_value->length, rdn_value->data));
-				return WERR_FOOBAR;				
+			if (data_blob_cmp(rdn_val,
+					  &e->values[0]) != 0) {
+				DEBUG(0, ("Unxpectedly got mismatching RDN values when checking RDN against name of %s",
+					  ldb_dn_get_linearized(msg->dn)));
+				return WERR_FOOBAR;
 			}
 		}
-
-		rdn_m->attid				= rdn_attid;
-		rdn_m->version				= name_d->version;
-		rdn_m->originating_change_time		= name_d->originating_change_time;
-		rdn_m->originating_invocation_id	= name_d->originating_invocation_id;
-		rdn_m->originating_usn			= name_d->originating_usn;
-		rdn_m->local_usn			= 0;
-		md->ctr.ctr1.count++;
+		if (d->originating_change_time > whenChanged) {
+			whenChanged = d->originating_change_time;
+		}
 
 	}
 
+	msg->num_elements = attr_count;
+	md->ctr.ctr1.count = attr_count;
+
 	if (instanceType_e == NULL) {
 		return WERR_FOOBAR;
 	}
diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index cb980a9dbca..0d378202ae1 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -1042,7 +1042,7 @@ static int replmd_add(struct ldb_module *module, struct ldb_request *req)
 	nmd.ctr.ctr1.count = ni;
 
 	/*
-	 * sort meta data array, and move the rdn attribute entry to the end
+	 * sort meta data array
 	 */
 	ret = replmd_replPropertyMetaDataCtr1_sort_and_verify(ldb, &nmd.ctr.ctr1, msg->dn);
 	if (ret != LDB_SUCCESS) {
@@ -1326,6 +1326,52 @@ static int replmd_update_rpmd_element(struct ldb_context *ldb,
 	return LDB_SUCCESS;
 }
 
+/*
+ * Bump the replPropertyMetaData version on an attribute, and if it
+ * has changed (or forced by leaving rdn_old NULL), update the value
+ * in the entry.
+ *
+ * This is important, as calling a modify operation may not change the
+ * version number if the values appear unchanged, but a rename between
+ * parents bumps this value.
+ *
+ */
+static int replmd_update_rpmd_rdn_attr(struct ldb_context *ldb,
+				       struct ldb_message *msg,
+				       const struct ldb_val *rdn_new,
+				       const struct ldb_val *rdn_old,
+				       struct replPropertyMetaDataBlob *omd,
+				       struct replmd_replicated_request *ar,
+				       NTTIME now,
+				       bool is_schema_nc)
+{
+	struct ldb_message_element new_el = {
+		.flags = LDB_FLAG_MOD_REPLACE,
+		.name = ldb_dn_get_rdn_name(msg->dn),
+		.num_values = 1,
+		.values = discard_const_p(struct ldb_val, rdn_new)
+	};
+	struct ldb_message_element old_el = {
+		.flags = LDB_FLAG_MOD_REPLACE,
+		.name = ldb_dn_get_rdn_name(msg->dn),
+		.num_values = rdn_old ? 1 : 0,
+		.values = discard_const_p(struct ldb_val, rdn_old)
+	};
+
+	if (ldb_msg_element_equal_ordered(&new_el, &old_el) == false) {
+		int ret = ldb_msg_add(msg, &new_el, LDB_FLAG_MOD_REPLACE);
+		if (ret != LDB_SUCCESS) {
+			return ldb_oom(ldb);
+		}
+	}
+
+	return replmd_update_rpmd_element(ldb, msg, &new_el, NULL,
+					  omd, ar->schema, &ar->seq_num,
+					  &ar->our_invocation_id,
+					  now, is_schema_nc, ar->req);
+
+}
+
 static uint64_t find_max_local_usn(struct replPropertyMetaDataBlob omd)
 {
 	uint32_t count = omd.ctr.ctr1.count;
@@ -4031,10 +4077,19 @@ static int replmd_replicated_apply_add(struct replmd_replicated_request *ar)
 	unsigned int i;
 	int ret;
 	bool remote_isDeleted = false;
+	bool is_schema_nc;
+	NTTIME now;
+	time_t t = time(NULL);
+	const struct ldb_val *rdn_val;
+	struct replmd_private *replmd_private =
+		talloc_get_type(ldb_module_get_private(ar->module),
+				struct replmd_private);
+	unix_to_nt_time(&now, t);
 
 	ldb = ldb_module_get_ctx(ar->module);
 	msg = ar->objs->objects[ar->index_current].msg;
 	md = ar->objs->objects[ar->index_current].meta_data;
+	is_schema_nc = ldb_dn_compare_base(replmd_private->schema_dn, msg->dn) == 0;
 
 	ret = ldb_sequence_number(ldb, LDB_SEQ_NEXT, &ar->seq_num);
 	if (ret != LDB_SUCCESS) {
@@ -4099,10 +4154,18 @@ static int replmd_replicated_apply_add(struct replmd_replicated_request *ar)
 
 	/*
 	 * the meta data array is already sorted by the caller, except
-	 * for the RDN, which needs to be put in the right spot.
+	 * for the RDN, which needs to be added.
 	 */
 
 
+	rdn_val = ldb_dn_get_rdn_val(msg->dn);
+	ret = replmd_update_rpmd_rdn_attr(ldb, msg, rdn_val, NULL,
+				     md, ar, now, is_schema_nc);
+	if (ret != LDB_SUCCESS) {
+		ldb_asprintf_errstring(ldb, "%s: error during DRS repl ADD: %s", __func__, ldb_errstring(ldb));
+		return replmd_replicated_request_error(ar, ret);
+	}
+
 	ret = replmd_replPropertyMetaDataCtr1_sort_and_verify(ldb, &md->ctr.ctr1, msg->dn);
 	if (ret != LDB_SUCCESS) {
 		ldb_asprintf_errstring(ldb, "%s: error during DRS repl ADD: %s", __func__, ldb_errstring(ldb));
@@ -4392,6 +4455,11 @@ static int replmd_replicated_handle_rename(struct replmd_replicated_request *ar,
 
 	ret = dsdb_module_rename(ar->module, ar->search_msg->dn, msg->dn,
 				 DSDB_FLAG_NEXT_MODULE, ar->req);
+	if (ret == LDB_SUCCESS) {
+		talloc_free(tmp_ctx);
+		*renamed = true;
+		return ret;
+	}
 
 	if (ret != LDB_ERR_ENTRY_ALREADY_EXISTS) {
 		talloc_free(tmp_ctx);
@@ -4608,11 +4676,21 @@ static int replmd_replicated_apply_merge(struct replmd_replicated_request *ar)
 	bool take_remote_isDeleted = false;
 	bool sd_updated = false;
 	bool renamed = false;
+	bool is_schema_nc = false;
 	NTSTATUS nt_status;
+	const struct ldb_val *old_rdn, *new_rdn;
+	struct replmd_private *replmd_private =
+		talloc_get_type(ldb_module_get_private(ar->module),
+				struct replmd_private);
+	NTTIME now;
+	time_t t = time(NULL);
+	unix_to_nt_time(&now, t);
 
 	ldb = ldb_module_get_ctx(ar->module);
 	msg = ar->objs->objects[ar->index_current].msg;
 
+	is_schema_nc = ldb_dn_compare_base(replmd_private->schema_dn, msg->dn) == 0;
+
 	rmd = ar->objs->objects[ar->index_current].meta_data;
 	ZERO_STRUCT(omd);
 	omd.version = 1;
@@ -4690,7 +4768,6 @@ static int replmd_replicated_apply_merge(struct replmd_replicated_request *ar)
 		 * the peer has an older name to what we have (see
 		 * replmd_replicated_apply_search_callback())
 		 */
-		renamed = true;
 		ret = replmd_replicated_handle_rename(ar, msg, ar->req, &renamed);
 	}
 
@@ -4808,12 +4885,19 @@ static int replmd_replicated_apply_merge(struct replmd_replicated_request *ar)
 	 */
 	nmd.ctr.ctr1.count = ni;
 
+	new_rdn = ldb_dn_get_rdn_val(msg->dn);
+	old_rdn = ldb_dn_get_rdn_val(ar->search_msg->dn);
+
+	if (renamed) {
+		ret = replmd_update_rpmd_rdn_attr(ldb, msg, new_rdn, old_rdn,
+						  &nmd, ar, now, is_schema_nc);
+		if (ret != LDB_SUCCESS) {
+			ldb_asprintf_errstring(ldb, "%s: error during DRS repl merge: %s", __func__, ldb_errstring(ldb));
+			return replmd_replicated_request_error(ar, ret);
+		}
+	}
 	/*
-	 * the rdn attribute (the alias for the name attribute),
-	 * 'cn' for most objects is the last entry in the meta data array
-	 * we have stored
-	 *
-	 * sort the new meta data array so it is slotted into the right place
+	 * sort the new meta data array
 	 */
 	ret = replmd_replPropertyMetaDataCtr1_sort_and_verify(ldb, &nmd.ctr.ctr1, msg->dn);
 	if (ret != LDB_SUCCESS) {