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

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 (file)
index 0b19ae5..0000000
--- 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 198ac84c730d7fe97b8a595f8b26735722088ba3..7646f942fcaec837e50d9a187bc7c7bf7020c625 100644 (file)
--- 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;
        }