From: Stefan Metzmacher <metze@samba.org>
Date: Wed, 17 Jan 2018 07:07:03 +0000 (+0100)
Subject: repl_meta_data: fix linked attribute corruption on databases with unsorted links... 
X-Git-Tag: tevent-0.9.36~296
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=a25c99c9f1fd1814c56c21848c748cd0e038eed7;p=thirdparty%2Fsamba.git

repl_meta_data: fix linked attribute corruption on databases with unsorted links on expunge

This is really critical bug, it removes valid linked attributes.

When a DC was provisioned/joined with a Samba version older than 4.7
is upgraded to 4.7 (or later), it can happen that the garbage collection
(dsdb_garbage_collect_tombstones()), triggered periodically by the 'kcc' task
of 'samba' or my 'samba-tool domain tombstones expunge' corrupt the linked attributes.

This is similar to Bug #13095 - Broken linked attribute handling,
but it's not triggered by an originating change.

The bug happens in replmd_modify_la_delete()
were get_parsed_dns_trusted() generates a sorted array of
struct parsed_dn based on the values in old_el->values.

If the database doesn't support the sortedLinks compatibleFeatures
in the @SAMBA_DSDB record, it's very likely that
the array of old_dns is sorted differently than the values
in old_el->values.

The problem is that struct parsed_dn has just a pointer
'struct ldb_val *v' that points to the corresponding
value in old_el->values.

Now if vanish_links is true the damage happens here:

        if (vanish_links) {
                unsigned j = 0;
                for (i = 0; i < old_el->num_values; i++) {
                        if (old_dns[i].v != NULL) {
                                old_el->values[j] = *old_dns[i].v;
                                j++;
                        }
                }
                old_el->num_values = j;
        }

old_el->values[0] = *old_dns[0].v;
can change the value old_dns[1].v is pointing at!
That means that some values can get lost while others
are stored twice, because the LDB_FLAG_INTERNAL_DISABLE_SINGLE_VALUE_CHECK
allows it to be stored.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
---

diff --git a/selftest/knownfail.d/unsorted-links b/selftest/knownfail.d/unsorted-links
deleted file mode 100644
index 0b19ae5c116..00000000000
--- a/selftest/knownfail.d/unsorted-links
+++ /dev/null
@@ -1 +0,0 @@
-^samba4.blackbox.tombstones-expunge.release-4-5-0-pre1.check_expected_unsorted_links
diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index 198ac84c730..7646f942fca 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -2844,11 +2844,23 @@ static int replmd_modify_la_delete(struct ldb_module *module,
 
 	if (vanish_links) {
 		unsigned j = 0;
+		struct ldb_val *tmp_vals = NULL;
+
+		tmp_vals = talloc_array(tmp_ctx, struct ldb_val,
+					old_el->num_values);
+		if (tmp_vals == NULL) {
+			talloc_free(tmp_ctx);
+			return ldb_module_oom(module);
+		}
 		for (i = 0; i < old_el->num_values; i++) {
-			if (old_dns[i].v != NULL) {
-				old_el->values[j] = *old_dns[i].v;
-				j++;
+			if (old_dns[i].v == NULL) {
+				continue;
 			}
+			tmp_vals[j] = *old_dns[i].v;
+			j++;
+		}
+		for (i = 0; i < j; i++) {
+			old_el->values[i] = tmp_vals[i];
 		}
 		old_el->num_values = j;
 	}