Attachment 14550 Details for Bug 13418 – Patches for v4-8-test

[patch] Patches for v4-8-test

tmp48.diff.txt (text/plain), 75.71 KB, created by Stefan Metzmacher on 2018-10-30 09:42:55 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Metzmacher

Created: 2018-10-30 09:42:55 UTC

Size: 75.71 KB

patch

obsolete

>From e2c060293daad3c7cc588b6cc5582f302fcbd798 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Fri, 29 Jun 2018 14:53:19 +1200
>Subject: [PATCH 01/17] dbcheck: Use symbolic control name for
> DSDB_CONTROL_DBCHECK_FIX_DUPLICATE_LINKS
>
>While we do not wish to encourage use of this control, manually typed OIDs are
>even more trouble, so pass out via pydsdb.
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
>(cherry picked from commit c7fd68088d84232a2f4074ca278b5448ef624afd)
>---
> python/samba/dbchecker.py | 2 +-
> source4/dsdb/pydsdb.c     | 1 +
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
>index b2b8b0c9558f..df2f6f614a37 100644
>--- a/python/samba/dbchecker.py
>+++ b/python/samba/dbchecker.py
>@@ -759,7 +759,7 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
>         m = ldb.Message()
>         m.dn = obj.dn
>         m['value'] = ldb.MessageElement(forward_vals, ldb.FLAG_MOD_REPLACE, forward_attr)
>-        if self.do_modify(m, ["local_oid:1.3.6.1.4.1.7165.4.3.19.2:1"],
>+        if self.do_modify(m, ["local_oid:%s:1" % dsdb.DSDB_CONTROL_DBCHECK_FIX_DUPLICATE_LINKS],
>                 "Failed to fix duplicate links in attribute '%s'" % forward_attr):
>             self.report("Fixed duplicate links in attribute '%s'" % (forward_attr))
>             duplicate_cache_key = "%s:%s" % (str(obj.dn), forward_attr)
>diff --git a/source4/dsdb/pydsdb.c b/source4/dsdb/pydsdb.c
>index 8cf3ef5e4f68..afc5394b641e 100644
>--- a/source4/dsdb/pydsdb.c
>+++ b/source4/dsdb/pydsdb.c
>@@ -1569,6 +1569,7 @@ void initdsdb(void)
> 	ADD_DSDB_STRING(DSDB_SYNTAX_OR_NAME);
> 	ADD_DSDB_STRING(DSDB_CONTROL_DBCHECK);
> 	ADD_DSDB_STRING(DSDB_CONTROL_DBCHECK_MODIFY_RO_REPLICA);
>+	ADD_DSDB_STRING(DSDB_CONTROL_DBCHECK_FIX_DUPLICATE_LINKS);
> 	ADD_DSDB_STRING(DSDB_CONTROL_REPLMD_VANISH_LINKS);
> 	ADD_DSDB_STRING(DSDB_CONTROL_PERMIT_INTERDOMAIN_TRUST_UAC_OID);
> 	ADD_DSDB_STRING(DSDB_CONTROL_SKIP_DUPLICATES_CHECK_OID);
>-- 
>2.17.1
>
>
>From d95c05c8dcdb7202058e30abf704a2eeb8505fd7 Mon Sep 17 00:00:00 2001
>From: Tim Beale <timbeale@catalyst.net.nz>
>Date: Fri, 25 May 2018 14:05:27 +1200
>Subject: [PATCH 02/17] dbchecker: Fixing up incorrect DNs wasn't working
>
>dbcheck would fail to fix up attributes where the extended DN's GUID is
>correct, but the DN itself is incorrect. The code failed attempting to
>remove the old/incorrect DN, e.g.
>
> NOTE: old (due to rename or delete) DN string component for
> objectCategory in object CN=alice,CN=Users,DC=samba,DC=example,DC=com -
> <GUID=7bfdf9d8-62f9-420c-8a71-e3d3e931c91e>;
>   CN=Person,CN=Schema,CN=Configuration,DC=samba,DC=bad,DC=com
> Change DN to <GUID=7bfdf9d8-62f9-420c-8a71-e3d3e931c91e>;
>   CN=Person,CN=Schema,CN=Configuration,DC=samba,DC=example,DC=com?
> [y/N/all/none] y
> Failed to fix old DN string on attribute objectCategory : (16,
> "attribute 'objectCategory': no matching attribute value while deleting
> attribute on 'CN=alice,CN=Users,DC=samba,DC=example,DC=com'")
>
>The problem was the LDB message specified the value to delete with its
>full DN, including the GUID. The LDB code then helpfully corrected this
>value on the way through, so that the DN got updated to reflect the
>correct DN (i.e. 'DC=example,DC=com') of the object matching that GUID,
>rather than the incorrect DN (i.e. 'DC=bad,DC=com') that we were trying
>to remove. Because the requested value and the existing DB value didn't
>match, the operation failed.
>
>We can avoid this problem by passing down just the DN (not the extended
>DN) of the value we want to delete. Without the GUID portion of the DN,
>the LDB code will no longer try to correct it on the way through, and
>the dbcheck operation will succeed.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13495
>
>Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
>Pair-programmed-with: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 22208f52e6096fbe9413b8ff339d9446851e0874)
>---
> python/samba/dbchecker.py                     | 19 ++++--
> source4/dsdb/pydsdb.c                         |  1 +
> .../dsdb/samdb/ldb_modules/repl_meta_data.c   | 64 ++++++++++++++++++
> source4/dsdb/samdb/samdb.h                    |  3 +
> ...-after-dbcheck-oneway-link-corruption.ldif | 19 ++++++
> ...eck-link-output-oneway-link-corruption.txt |  5 ++
> testprogs/blackbox/dbcheck-links.sh           | 65 +++++++++++++++++++
> 7 files changed, 171 insertions(+), 5 deletions(-)
> create mode 100644 source4/selftest/provisions/release-4-5-0-pre1/expected-after-dbcheck-oneway-link-corruption.ldif
> create mode 100644 source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output-oneway-link-corruption.txt
>
>diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
>index df2f6f614a37..34856108a227 100644
>--- a/python/samba/dbchecker.py
>+++ b/python/samba/dbchecker.py
>@@ -650,7 +650,8 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
>         m.dn = dn
>         m['old_value'] = ldb.MessageElement(val, ldb.FLAG_MOD_DELETE, attrname)
>         m['new_value'] = ldb.MessageElement(str(dsdb_dn), ldb.FLAG_MOD_ADD, attrname)
>-        if self.do_modify(m, ["show_recycled:1"],
>+        if self.do_modify(m, ["show_recycled:1",
>+                              "local_oid:%s:1" % dsdb.DSDB_CONTROL_DBCHECK_FIX_LINK_DN_NAME],
>                           "Failed to fix old DN string on attribute %s" % (attrname)):
>             self.report("Fixed old DN string on attribute %s" % (attrname))
> 
>@@ -1289,14 +1290,22 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
>                                                       res[0].dn, "SID")
>                 continue
> 
>+            # Only for non-links, not even forward-only links
>+            # (otherwise this breaks repl_meta_data):
>+            #
>             # Now we have checked the GUID and SID, offer to fix old
>-            # DN strings as a non-error (for forward links with no
>+            # DN strings as a non-error (DNs, not links so no
>             # backlink).  Samba does not maintain this string
>             # otherwise, so we don't increment error_count.
>             if reverse_link_name is None:
>-                if str(res[0].dn) != str(dsdb_dn.dn):
>-                    self.err_dn_string_component_old(obj.dn, attrname, val, dsdb_dn,
>-                                                     res[0].dn)
>+                if linkID == 0 and str(res[0].dn) != str(dsdb_dn.dn):
>+                    # Pass in the old/bad DN without the <GUID=...> part,
>+                    # otherwise the LDB code will correct it on the way through
>+                    # (Note: we still want to preserve the DSDB DN prefix in the
>+                    # case of binary DNs)
>+                    bad_dn = dsdb_dn.prefix + dsdb_dn.dn.get_linearized()
>+                    self.err_dn_string_component_old(obj.dn, attrname, bad_dn,
>+                                                     dsdb_dn, res[0].dn)
>                 continue
> 
>             # check the reverse_link is correct if there should be one
>diff --git a/source4/dsdb/pydsdb.c b/source4/dsdb/pydsdb.c
>index afc5394b641e..16cfa0364430 100644
>--- a/source4/dsdb/pydsdb.c
>+++ b/source4/dsdb/pydsdb.c
>@@ -1570,6 +1570,7 @@ void initdsdb(void)
> 	ADD_DSDB_STRING(DSDB_CONTROL_DBCHECK);
> 	ADD_DSDB_STRING(DSDB_CONTROL_DBCHECK_MODIFY_RO_REPLICA);
> 	ADD_DSDB_STRING(DSDB_CONTROL_DBCHECK_FIX_DUPLICATE_LINKS);
>+	ADD_DSDB_STRING(DSDB_CONTROL_DBCHECK_FIX_LINK_DN_NAME);
> 	ADD_DSDB_STRING(DSDB_CONTROL_REPLMD_VANISH_LINKS);
> 	ADD_DSDB_STRING(DSDB_CONTROL_PERMIT_INTERDOMAIN_TRUST_UAC_OID);
> 	ADD_DSDB_STRING(DSDB_CONTROL_SKIP_DUPLICATES_CHECK_OID);
>diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>index 62f58addfde6..a2d803e6ed4d 100644
>--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>@@ -3329,6 +3329,7 @@ static int replmd_modify(struct ldb_module *module, struct ldb_request *req)
> 	const struct ldb_message_element *guid_el = NULL;
> 	struct ldb_control *sd_propagation_control;
> 	struct ldb_control *fix_links_control = NULL;
>+	struct ldb_control *fix_dn_name_control = NULL;
> 	struct replmd_private *replmd_private =
> 		talloc_get_type(ldb_module_get_private(module), struct replmd_private);
> 
>@@ -3387,6 +3388,69 @@ static int replmd_modify(struct ldb_module *module, struct ldb_request *req)
> 		return ldb_next_request(module, req);
> 	}
> 
>+	fix_dn_name_control = ldb_request_get_control(req,
>+					DSDB_CONTROL_DBCHECK_FIX_LINK_DN_NAME);
>+	if (fix_dn_name_control != NULL) {
>+		struct dsdb_schema *schema = NULL;
>+		const struct dsdb_attribute *sa = NULL;
>+
>+		if (req->op.mod.message->num_elements != 2) {
>+			return ldb_module_operr(module);
>+		}
>+
>+		if (req->op.mod.message->elements[0].flags != LDB_FLAG_MOD_DELETE) {
>+			return ldb_module_operr(module);
>+		}
>+
>+		if (req->op.mod.message->elements[1].flags != LDB_FLAG_MOD_ADD) {
>+			return ldb_module_operr(module);
>+		}
>+
>+		if (req->op.mod.message->elements[0].num_values != 1) {
>+			return ldb_module_operr(module);
>+		}
>+
>+		if (req->op.mod.message->elements[1].num_values != 1) {
>+			return ldb_module_operr(module);
>+		}
>+
>+		schema = dsdb_get_schema(ldb, req);
>+		if (schema == NULL) {
>+			return ldb_module_operr(module);
>+		}
>+
>+		if (ldb_attr_cmp(req->op.mod.message->elements[0].name,
>+				 req->op.mod.message->elements[1].name) != 0) {
>+			return ldb_module_operr(module);
>+		}
>+
>+		sa = dsdb_attribute_by_lDAPDisplayName(schema,
>+				req->op.mod.message->elements[0].name);
>+		if (sa == NULL) {
>+			return ldb_module_operr(module);
>+		}
>+
>+		if (sa->dn_format == DSDB_INVALID_DN) {
>+			return ldb_module_operr(module);
>+		}
>+
>+		if (sa->linkID != 0) {
>+			return ldb_module_operr(module);
>+		}
>+
>+		/*
>+		 * If we are run from dbcheck and we are not updating
>+		 * a link (as these would need to be sorted and so
>+		 * can't go via such a simple update, then do not
>+		 * trigger replicated updates and a new USN from this
>+		 * change, it wasn't a real change, just a new
>+		 * (correct) string DN
>+		 */
>+
>+		fix_dn_name_control->critical = false;
>+		return ldb_next_request(module, req);
>+	}
>+
> 	ldb_debug(ldb, LDB_DEBUG_TRACE, "replmd_modify\n");
> 
> 	guid_el = ldb_msg_find_element(req->op.mod.message, "objectGUID");
>diff --git a/source4/dsdb/samdb/samdb.h b/source4/dsdb/samdb/samdb.h
>index eb5274028068..7b809c76415b 100644
>--- a/source4/dsdb/samdb/samdb.h
>+++ b/source4/dsdb/samdb/samdb.h
>@@ -130,6 +130,9 @@ struct dsdb_control_password_change {
> /* passed by dbcheck to fix duplicate linked attributes (bug #13095) */
> #define DSDB_CONTROL_DBCHECK_FIX_DUPLICATE_LINKS "1.3.6.1.4.1.7165.4.3.19.2"
> 
>+/* passed by dbcheck to fix the DN strong of a one-way-link (bug #13495) */
>+#define DSDB_CONTROL_DBCHECK_FIX_LINK_DN_NAME "1.3.6.1.4.1.7165.4.3.19.3"
>+
> /* passed when importing plain text password on upgrades */
> #define DSDB_CONTROL_PASSWORD_BYPASS_LAST_SET_OID "1.3.6.1.4.1.7165.4.3.20"
> 
>diff --git a/source4/selftest/provisions/release-4-5-0-pre1/expected-after-dbcheck-oneway-link-corruption.ldif b/source4/selftest/provisions/release-4-5-0-pre1/expected-after-dbcheck-oneway-link-corruption.ldif
>new file mode 100644
>index 000000000000..b2142df74247
>--- /dev/null
>+++ b/source4/selftest/provisions/release-4-5-0-pre1/expected-after-dbcheck-oneway-link-corruption.ldif
>@@ -0,0 +1,19 @@
>+# record 1
>+dn: OU=dangling-ou2,DC=release-4-5-0-pre1,DC=samba,DC=corp
>+
>+# record 2
>+dn: OU=dangling-from,DC=release-4-5-0-pre1,DC=samba,DC=corp
>+seeAlso: OU=dangling-ou2,DC=release-4-5-0-pre1,DC=samba,DC=corp
>+
>+# Referral
>+ref: ldap:///CN=Configuration,DC=release-4-5-0-pre1,DC=samba,DC=corp
>+
>+# Referral
>+ref: ldap:///DC=DomainDnsZones,DC=release-4-5-0-pre1,DC=samba,DC=corp
>+
>+# Referral
>+ref: ldap:///DC=ForestDnsZones,DC=release-4-5-0-pre1,DC=samba,DC=corp
>+
>+# returned 5 records
>+# 2 entries
>+# 3 referrals
>diff --git a/source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output-oneway-link-corruption.txt b/source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output-oneway-link-corruption.txt
>new file mode 100644
>index 000000000000..69d1e516d14c
>--- /dev/null
>+++ b/source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output-oneway-link-corruption.txt
>@@ -0,0 +1,5 @@
>+Checking 228 objects
>+NOTE: old (due to rename or delete) DN string component for seeAlso in object OU=dangling-from,DC=release-4-5-0-pre1,DC=samba,DC=corp - OU=dangling-ou,DC=release-4-5-0-pre1,DC=samba,DC=corp
>+Change DN to <GUID=20600e7c-92bb-492e-9552-f3ed7f8a2cad>;OU=dangling-ou2,DC=release-4-5-0-pre1,DC=samba,DC=corp? [YES]
>+Fixed old DN string on attribute seeAlso
>+Checked 228 objects (0 errors)
>diff --git a/testprogs/blackbox/dbcheck-links.sh b/testprogs/blackbox/dbcheck-links.sh
>index 778edf002c90..13811ddb461b 100755
>--- a/testprogs/blackbox/dbcheck-links.sh
>+++ b/testprogs/blackbox/dbcheck-links.sh
>@@ -205,6 +205,67 @@ check_expected_after_dbcheck_forward_link_corruption() {
>     fi
> }
> 
>+oneway_link_corruption() {
>+    #
>+    # Step1: add  OU "dangling-ou"
>+    #
>+    ldif=$PREFIX_ABS/${RELEASE}/oneway_link_corruption.ldif
>+    cat > $ldif <<EOF
>+dn: OU=dangling-ou,DC=release-4-5-0-pre1,DC=samba,DC=corp
>+changetype: add
>+objectclass: organizationalUnit
>+objectGUID: 20600e7c-92bb-492e-9552-f3ed7f8a2cad
>+EOF
>+
>+    out=$(TZ=UTC $ldbmodify -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb --relax $ldif)
>+    if [ "$?" != "0" ]; then
>+	echo "ldbmodify returned:\n$out"
>+	return 1
>+    fi
>+
>+    #
>+    # Step2: add  msExchConfigurationContainer "dangling-msexch"
>+    #
>+    ldif=$PREFIX_ABS/${RELEASE}/oneway_link_corruption2.ldif
>+    cat > $ldif <<EOF
>+dn: OU=dangling-from,DC=release-4-5-0-pre1,DC=samba,DC=corp
>+changetype: add
>+objectclass: organizationalUnit
>+seeAlso: OU=dangling-ou,DC=release-4-5-0-pre1,DC=samba,DC=corp
>+EOF
>+
>+    out=$(TZ=UTC $ldbmodify -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb $ldif)
>+    if [ "$?" != "0" ]; then
>+	echo "ldbmodify returned:\n$out"
>+	return 1
>+    fi
>+
>+    #
>+    # Step3: rename dangling-ou to dangling-ou2
>+    #
>+    # Because this is a one-way link we don't fix it at runtime
>+    #
>+    out=$(TZ=UTC $ldbrename -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb OU=dangling-ou,DC=release-4-5-0-pre1,DC=samba,DC=corp OU=dangling-ou2,DC=release-4-5-0-pre1,DC=samba,DC=corp)
>+    if [ "$?" != "0" ]; then
>+	echo "ldbmodify returned:\n$out"
>+	return 1
>+    fi
>+}
>+
>+dbcheck_oneway_link_corruption() {
>+    dbcheck "-oneway-link-corruption" "0" ""
>+    return $?
>+}
>+
>+check_expected_after_dbcheck_oneway_link_corruption() {
>+    tmpldif=$PREFIX_ABS/$RELEASE/expected-after-dbcheck-oneway-link-corruption.ldif.tmp
>+    TZ=UTC $ldbsearch -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb '(|(ou=dangling-ou)(ou=dangling-ou2)(ou=dangling-from))' -s sub -b DC=release-4-5-0-pre1,DC=samba,DC=corp --show-deleted --sorted seeAlso > $tmpldif
>+    diff $tmpldif $release_dir/expected-after-dbcheck-oneway-link-corruption.ldif
>+    if [ "$?" != "0" ]; then
>+	return 1
>+    fi
>+}
>+
> dbcheck_dangling_multi_valued() {
> 
>     $PYTHON $BINDIR/samba-tool dbcheck -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb --fix --yes
>@@ -276,6 +337,10 @@ if [ -d $release_dir ]; then
>     testit "dbcheck_forward_link_corruption" dbcheck_forward_link_corruption
>     testit "check_expected_after_dbcheck_forward_link_corruption" check_expected_after_dbcheck_forward_link_corruption
>     testit "forward_link_corruption_clean" dbcheck_clean
>+    testit "oneway_link_corruption" oneway_link_corruption
>+    testit "dbcheck_oneway_link_corruption" dbcheck_oneway_link_corruption
>+    testit "check_expected_after_dbcheck_oneway_link_corruption" check_expected_after_dbcheck_oneway_link_corruption
>+    testit "oneway_link_corruption_clean" dbcheck_clean
>     testit "dangling_one_way_link" dangling_one_way_link
>     testit "dbcheck_one_way" dbcheck_one_way
>     testit "dbcheck_clean2" dbcheck_clean
>-- 
>2.17.1
>
>
>From 4e1141dccf1f61fee767f62aa655579890e40a51 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Mon, 8 Oct 2018 15:35:52 +0200
>Subject: [PATCH 03/17] schema_samba4.ldif: add allocation of
> DSDB_CONTROL_DBCHECK_FIX_LINK_DN_NAME
>
>This was already allocated in source4/dsdb/samdb/samdb.h with
>commit 22208f52e6096fbe9413b8ff339d9446851e0874.
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 0189f23f5bda263c7462366ee16b2fe4bcda0119)
>---
> source4/setup/schema_samba4.ldif | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/source4/setup/schema_samba4.ldif b/source4/setup/schema_samba4.ldif
>index 6aafc9e5f493..40641c46dd71 100644
>--- a/source4/setup/schema_samba4.ldif
>+++ b/source4/setup/schema_samba4.ldif
>@@ -214,6 +214,7 @@
> #Allocated: DSDB_CONTROL_DBCHECK 1.3.6.1.4.1.7165.4.3.19
> #Allocated: DSDB_CONTROL_DBCHECK_MODIFY_RO_REPLICA 1.3.6.1.4.1.7165.4.3.19.1
> #Allocated: DSDB_CONTROL_DBCHECK_FIX_DUPLICATE_LINKS 1.3.6.1.4.1.7165.4.3.19.2
>+#Allocated: DSDB_CONTROL_DBCHECK_FIX_LINK_DN_NAME 1.3.6.1.4.1.7165.4.3.19.3
> #Allocated: DSDB_CONTROL_PASSWORD_BYPASS_LAST_SET_OID 1.3.6.1.4.1.7165.4.3.20
> #Allocated: DSDB_CONTROL_SEC_DESC_PROPAGATION_OID 1.3.6.1.4.1.7165.4.3.21
> #Allocated: DSDB_CONTROL_PERMIT_INTERDOMAIN_TRUST_UAC_OID 1.3.6.1.4.1.7165.4.3.23
>-- 
>2.17.1
>
>
>From 38ad525f045a4a644a04369690ec7a319a9e6239 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Mon, 8 Oct 2018 17:13:13 +0200
>Subject: [PATCH 04/17] s4:dsdb: fix comment on
> DSDB_CONTROL_DBCHECK_FIX_LINK_DN_NAME
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 60131b4452d43b3792e7f27a4190c88e7aabb1b4)
>---
> source4/dsdb/samdb/samdb.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source4/dsdb/samdb/samdb.h b/source4/dsdb/samdb/samdb.h
>index 7b809c76415b..7d1164ffa49c 100644
>--- a/source4/dsdb/samdb/samdb.h
>+++ b/source4/dsdb/samdb/samdb.h
>@@ -130,7 +130,7 @@ struct dsdb_control_password_change {
> /* passed by dbcheck to fix duplicate linked attributes (bug #13095) */
> #define DSDB_CONTROL_DBCHECK_FIX_DUPLICATE_LINKS "1.3.6.1.4.1.7165.4.3.19.2"
> 
>-/* passed by dbcheck to fix the DN strong of a one-way-link (bug #13495) */
>+/* passed by dbcheck to fix the DN string of a one-way-link (bug #13495) */
> #define DSDB_CONTROL_DBCHECK_FIX_LINK_DN_NAME "1.3.6.1.4.1.7165.4.3.19.3"
> 
> /* passed when importing plain text password on upgrades */
>-- 
>2.17.1
>
>
>From dd5092e64ea9822ba3a5894e1cccbda3356eecc2 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Tue, 16 Oct 2018 15:16:18 +0200
>Subject: [PATCH 05/17] testprogs/blackbox: add
> samba4.blackbox.test_primary_group test
>
>This demonstrates the bug, that happens when the primaryGroupID
>of a user is changed.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13418
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 364ed537e0bcb3a97cae0f2d1ff72de9423ce0e6)
>---
> .../samba4.blackbox.test_primary_group        |  2 +
> source4/selftest/tests.py                     |  2 +
> testprogs/blackbox/test_primary_group.sh      | 86 +++++++++++++++++++
> 3 files changed, 90 insertions(+)
> create mode 100644 selftest/knownfail.d/samba4.blackbox.test_primary_group
> create mode 100755 testprogs/blackbox/test_primary_group.sh
>
>diff --git a/selftest/knownfail.d/samba4.blackbox.test_primary_group b/selftest/knownfail.d/samba4.blackbox.test_primary_group
>new file mode 100644
>index 000000000000..779f6808c977
>--- /dev/null
>+++ b/selftest/knownfail.d/samba4.blackbox.test_primary_group
>@@ -0,0 +1,2 @@
>+^samba4.blackbox.test_primary_group.dbcheck.*run1
>+^samba4.blackbox.test_primary_group.dbcheck.*run2
>diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
>index fd9475156d9e..0841341d7592 100755
>--- a/source4/selftest/tests.py
>+++ b/source4/selftest/tests.py
>@@ -406,6 +406,8 @@ for env in ["ad_member", "s4member", "ad_dc_ntvfs", "chgdcpass"]:
> plantestsuite("samba4.blackbox.samba_tool(ad_dc_ntvfs:local)", "ad_dc_ntvfs:local", [os.path.join(samba4srcdir, "utils/tests/test_samba_tool.sh"), '$SERVER', '$SERVER_IP', '$USERNAME', '$PASSWORD', '$DOMAIN', smbclient4])
> plantestsuite("samba4.blackbox.net_rpc_user(ad_dc)", "ad_dc", [os.path.join(bbdir, "test_net_rpc_user.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$DOMAIN'])
> 
>+plantestsuite("samba4.blackbox.test_primary_group", "ad_dc:local", [os.path.join(bbdir, "test_primary_group.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$DOMAIN', '$PREFIX_ABS'])
>+
> if have_heimdal_support:
>     for env in ["ad_dc_ntvfs", "ad_dc"]:
>         plantestsuite("samba4.blackbox.pkinit(%s:local)" % env, "%s:local" % env, [os.path.join(bbdir, "test_pkinit_heimdal.sh"), '$SERVER', 'pkinit', '$PASSWORD', '$REALM', '$DOMAIN', '$PREFIX/%s' % env, "aes256-cts-hmac-sha1-96", smbclient4, configuration])
>diff --git a/testprogs/blackbox/test_primary_group.sh b/testprogs/blackbox/test_primary_group.sh
>new file mode 100755
>index 000000000000..c2d803e1d15c
>--- /dev/null
>+++ b/testprogs/blackbox/test_primary_group.sh
>@@ -0,0 +1,86 @@
>+#!/bin/bash
>+
>+if [ $# -lt 5 ]; then
>+cat <<EOF
>+Usage: test_primary_group.sh SERVER USERNAME PASSWORD DOMAIN PREFIX_ABS
>+EOF
>+exit 1;
>+fi
>+
>+TMPDIR="$PREFIX_ABS/$(basename $0)"
>+export TMPDIR
>+
>+SERVER=$1
>+USERNAME=$2
>+PASSWORD=$3
>+DOMAIN=$4
>+PREFIX_ABS=$5
>+shift 5
>+failed=0
>+
>+. `dirname $0`/subunit.sh
>+. `dirname $0`/common_test_fns.inc
>+
>+TZ=UTC
>+export TZ
>+
>+N=$(date +%H%M%S)
>+
>+testuser="testuser$N"
>+testgroup="testgroup$N"
>+
>+echo "testuser: $testuser"
>+echo "testgroup: $testgroup"
>+
>+testit "mkdir -p '${TMPDIR}'" mkdir -p ${TMPDIR} || failed=`expr $failed + 1`
>+
>+testit "create '$testuser'" $VALGRIND $PYTHON $BINDIR/samba-tool user create "$testuser" Password.1 || failed=`expr $failed + 1`
>+testit "add '$testgroup'" $VALGRIND $PYTHON $BINDIR/samba-tool group add "$testgroup" || failed=`expr $failed + 1`
>+testit "addmembers '$testgroup' '$testuser'" $VALGRIND $PYTHON $BINDIR/samba-tool group addmembers "$testgroup" "$testuser" || failed=`expr $failed + 1`
>+
>+testit "search1" $VALGRIND $BINDIR/ldbsearch -H ldap://$SERVER_IP -U$USERNAME%$PASSWORD -d0 sAMAccountName="$testgroup" objectSid || failed=`expr $failed + 1`
>+ldif="${TMPDIR}/search1.ldif"
>+$VALGRIND $BINDIR/ldbsearch -H ldap://$SERVER_IP -U$USERNAME%$PASSWORD -d0 sAMAccountName=$testgroup objectSid > $ldif
>+rid=$(cat $ldif | sed -n 's/^objectSid: S-1-5-21-.*-.*-.*-//p')
>+
>+testit "search2" $VALGRIND $BINDIR/ldbsearch -H ldap://$SERVER_IP -U$USERNAME%$PASSWORD -d0 sAMAccountName="$testuser" dn || failed=`expr $failed + 1`
>+ldif="${TMPDIR}/search2.ldif"
>+$VALGRIND $BINDIR/ldbsearch -H ldap://$SERVER_IP -U$USERNAME%$PASSWORD -d0 sAMAccountName=$testuser dn > $ldif
>+user_dn=$(cat $ldif | sed -n 's/^dn: //p')
>+
>+ldif="${TMPDIR}/modify1.ldif"
>+cat > $ldif <<EOF
>+dn: $user_dn
>+changetype: modify
>+replace: primaryGroupID
>+primaryGroupID: $rid
>+EOF
>+testit "Change primaryGroupID to $rid" $VALGRIND $BINDIR/ldbmodify -H ldap://$SERVER_IP -U$USERNAME%$PASSWORD -d0 --verbose < $ldif || failed=`expr $failed + 1`
>+
>+testit "dbcheck run1" $VALGRIND $PYTHON $BINDIR/samba-tool dbcheck --attrs=member || failed=`expr $failed + 1`
>+
>+ldif="${TMPDIR}/modify2.ldif"
>+cat > $ldif <<EOF
>+dn: $user_dn
>+changetype: modify
>+replace: primaryGroupID
>+primaryGroupID: 513
>+EOF
>+testit "Change primaryGroupID to 513" $VALGRIND $BINDIR/ldbmodify  -H ldap://$SERVER_IP -U$USERNAME%$PASSWORD -d0 < $ldif || failed=`expr $failed + 1`
>+
>+testit "dbcheck run2" $VALGRIND $PYTHON $BINDIR/samba-tool dbcheck --attrs=member || failed=`expr $failed + 1`
>+
>+testit "delete '$testuser'" $VALGRIND $PYTHON $BINDIR/samba-tool user delete "$testuser" || failed=`expr $failed + 1`
>+testit "delete '$testgroup'" $VALGRIND $PYTHON $BINDIR/samba-tool group delete "$testgroup" || failed=`expr $failed + 1`
>+
>+#
>+# As we don't support phantom objects and virtual backlinks
>+# the deletion of the user and group cause dangling links,
>+# which are detected like this:
>+#
>+# WARNING: target DN is deleted for member in object
>+#
>+testit_expect_failure "dbcheck run3" $VALGRIND $PYTHON $BINDIR/samba-tool dbcheck --attrs=member --fix --yes || failed=`expr $failed + 1`
>+testit "dbcheck run4" $VALGRIND $PYTHON $BINDIR/samba-tool dbcheck --attrs=member || failed=`expr $failed + 1`
>+
>+exit $failed
>-- 
>2.17.1
>
>
>From 5b2ae291c8c4bf7f3f4261c2bfc2411559ca7979 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Mon, 8 Oct 2018 17:13:52 +0200
>Subject: [PATCH 06/17] s4:dsdb: add DSDB_CONTROL_DBCHECK_FIX_LINK_DN_SID oid
>
>This will be used to fix missing <SID=> components in future.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13418
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit bb9c9e49a5e82f19626cb1b12ec9189fff5114e8)
>---
> source4/dsdb/pydsdb.c            | 1 +
> source4/dsdb/samdb/samdb.h       | 3 +++
> source4/setup/schema_samba4.ldif | 1 +
> 3 files changed, 5 insertions(+)
>
>diff --git a/source4/dsdb/pydsdb.c b/source4/dsdb/pydsdb.c
>index 16cfa0364430..7f25f7574dcb 100644
>--- a/source4/dsdb/pydsdb.c
>+++ b/source4/dsdb/pydsdb.c
>@@ -1571,6 +1571,7 @@ void initdsdb(void)
> 	ADD_DSDB_STRING(DSDB_CONTROL_DBCHECK_MODIFY_RO_REPLICA);
> 	ADD_DSDB_STRING(DSDB_CONTROL_DBCHECK_FIX_DUPLICATE_LINKS);
> 	ADD_DSDB_STRING(DSDB_CONTROL_DBCHECK_FIX_LINK_DN_NAME);
>+	ADD_DSDB_STRING(DSDB_CONTROL_DBCHECK_FIX_LINK_DN_SID);
> 	ADD_DSDB_STRING(DSDB_CONTROL_REPLMD_VANISH_LINKS);
> 	ADD_DSDB_STRING(DSDB_CONTROL_PERMIT_INTERDOMAIN_TRUST_UAC_OID);
> 	ADD_DSDB_STRING(DSDB_CONTROL_SKIP_DUPLICATES_CHECK_OID);
>diff --git a/source4/dsdb/samdb/samdb.h b/source4/dsdb/samdb/samdb.h
>index 7d1164ffa49c..2db8dd4d4d88 100644
>--- a/source4/dsdb/samdb/samdb.h
>+++ b/source4/dsdb/samdb/samdb.h
>@@ -133,6 +133,9 @@ struct dsdb_control_password_change {
> /* passed by dbcheck to fix the DN string of a one-way-link (bug #13495) */
> #define DSDB_CONTROL_DBCHECK_FIX_LINK_DN_NAME "1.3.6.1.4.1.7165.4.3.19.3"
> 
>+/* passed by dbcheck to fix the DN SID of a one-way-link (bug #13418) */
>+#define DSDB_CONTROL_DBCHECK_FIX_LINK_DN_SID "1.3.6.1.4.1.7165.4.3.19.4"
>+
> /* passed when importing plain text password on upgrades */
> #define DSDB_CONTROL_PASSWORD_BYPASS_LAST_SET_OID "1.3.6.1.4.1.7165.4.3.20"
> 
>diff --git a/source4/setup/schema_samba4.ldif b/source4/setup/schema_samba4.ldif
>index 40641c46dd71..219b232f7535 100644
>--- a/source4/setup/schema_samba4.ldif
>+++ b/source4/setup/schema_samba4.ldif
>@@ -215,6 +215,7 @@
> #Allocated: DSDB_CONTROL_DBCHECK_MODIFY_RO_REPLICA 1.3.6.1.4.1.7165.4.3.19.1
> #Allocated: DSDB_CONTROL_DBCHECK_FIX_DUPLICATE_LINKS 1.3.6.1.4.1.7165.4.3.19.2
> #Allocated: DSDB_CONTROL_DBCHECK_FIX_LINK_DN_NAME 1.3.6.1.4.1.7165.4.3.19.3
>+#Allocated: DSDB_CONTROL_DBCHECK_FIX_LINK_DN_SID 1.3.6.1.4.1.7165.4.3.19.4
> #Allocated: DSDB_CONTROL_PASSWORD_BYPASS_LAST_SET_OID 1.3.6.1.4.1.7165.4.3.20
> #Allocated: DSDB_CONTROL_SEC_DESC_PROPAGATION_OID 1.3.6.1.4.1.7165.4.3.21
> #Allocated: DSDB_CONTROL_PERMIT_INTERDOMAIN_TRUST_UAC_OID 1.3.6.1.4.1.7165.4.3.23
>-- 
>2.17.1
>
>
>From 3da79616e13797bd9b2f57de34454dc53926e90d Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Mon, 8 Oct 2018 17:14:28 +0200
>Subject: [PATCH 07/17] dbchecker: improve verbose output of do_modify()
>
>This makes it easier to debug dbcheck problems.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13418
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit c5c99b569569ce36cac94e967ca53e3182abd6f7)
>---
> python/samba/dbchecker.py | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
>index 34856108a227..189230503aca 100644
>--- a/python/samba/dbchecker.py
>+++ b/python/samba/dbchecker.py
>@@ -378,10 +378,11 @@ systemFlags: -1946157056%s""" % (dn, guid_suffix),
> 
>     def do_modify(self, m, controls, msg, validate=True):
>         '''perform a modify with optional verbose output'''
>+        controls = controls + ["local_oid:%s:0" % dsdb.DSDB_CONTROL_DBCHECK]
>         if self.verbose:
>             self.report(self.samdb.write_ldif(m, ldb.CHANGETYPE_MODIFY))
>+            self.report("controls: %r" % controls)
>         try:
>-            controls = controls + ["local_oid:%s:0" % dsdb.DSDB_CONTROL_DBCHECK]
>             self.samdb.modify(m, controls=controls, validate=validate)
>         except Exception, err:
>             if self.in_transaction:
>-- 
>2.17.1
>
>
>From 0b9b83891131892ee07cd884635b6325ab84e162 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Fri, 12 Oct 2018 15:56:18 +0200
>Subject: [PATCH 08/17] dbchecker: Fix missing <SID=...> on linked attributes
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13418
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit a801799ebe26780653f4ed3fa3fc633e31871f7d)
>---
> python/samba/dbchecker.py | 42 ++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 41 insertions(+), 1 deletion(-)
>
>diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
>index 189230503aca..2619b9bc72e3 100644
>--- a/python/samba/dbchecker.py
>+++ b/python/samba/dbchecker.py
>@@ -59,6 +59,7 @@ class dbcheck(object):
>         self.fix_all_string_dn_component_mismatch = False
>         self.fix_all_GUID_dn_component_mismatch = False
>         self.fix_all_SID_dn_component_mismatch = False
>+        self.fix_all_SID_dn_component_missing = False
>         self.fix_all_old_dn_string_component_mismatch = False
>         self.fix_all_metadata = False
>         self.fix_time_metadata = False
>@@ -673,6 +674,38 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
>                           "Failed to fix incorrect DN %s on attribute %s" % (mismatch_type, attrname)):
>             self.report("Fixed incorrect DN %s on attribute %s" % (mismatch_type, attrname))
> 
>+    def err_dn_component_missing_target_sid(self, dn, attrname, val, dsdb_dn, target_sid_blob):
>+        """handle a DN string being incorrect"""
>+        self.report("ERROR: missing DN SID component for %s in object %s - %s" % (attrname, dn, val))
>+
>+        if len(dsdb_dn.prefix) != 0:
>+            self.report("Not fixing missing DN SID on DN+BINARY or DN+STRING")
>+            return
>+
>+        correct_dn = ldb.Dn(self.samdb, dsdb_dn.dn.extended_str())
>+        correct_dn.set_extended_component("SID", target_sid_blob)
>+
>+        if not self.confirm_all('Change DN to %s?' % correct_dn.extended_str(),
>+                                'fix_all_SID_dn_component_missing'):
>+            self.report("Not fixing missing DN SID component")
>+            return
>+
>+        target_guid_blob = correct_dn.get_extended_component("GUID")
>+        guid_sid_dn = ldb.Dn(self.samdb, "")
>+        guid_sid_dn.set_extended_component("GUID", target_guid_blob)
>+        guid_sid_dn.set_extended_component("SID", target_sid_blob)
>+
>+        m = ldb.Message()
>+        m.dn = dn
>+        m['new_value'] = ldb.MessageElement(guid_sid_dn.extended_str(), ldb.FLAG_MOD_ADD, attrname)
>+        controls = [
>+            "show_recycled:1",
>+            "local_oid:%s:1" % dsdb.DSDB_CONTROL_DBCHECK_FIX_LINK_DN_SID
>+        ]
>+        if self.do_modify(m, controls,
>+                          "Failed to ADD missing DN SID on attribute %s" % (attrname)):
>+            self.report("Fixed missing DN SID on attribute %s" % (attrname))
>+
>     def err_unknown_attribute(self, obj, attrname):
>         '''handle an unknown attribute error'''
>         self.report("ERROR: unknown attribute '%s' in %s" % (attrname, obj.dn))
>@@ -1285,7 +1318,14 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
>                                                       res[0].dn, "GUID")
>                 continue
> 
>-            if res[0].dn.get_extended_component("SID") != dsdb_dn.dn.get_extended_component("SID"):
>+            target_sid = res[0].dn.get_extended_component("SID")
>+            link_sid = dsdb_dn.dn.get_extended_component("SID")
>+            if link_sid is None and target_sid is not None:
>+                error_count += 1
>+                self.err_dn_component_missing_target_sid(obj.dn, attrname, val,
>+                                                         dsdb_dn, target_sid)
>+                continue
>+            if link_sid != target_sid:
>                 error_count += 1
>                 self.err_dn_component_target_mismatch(obj.dn, attrname, val, dsdb_dn,
>                                                       res[0].dn, "SID")
>-- 
>2.17.1
>
>
>From 912ebcbd1a4ee5cd84cb0192e7dc4da7542bf586 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Fri, 12 Oct 2018 15:56:18 +0200
>Subject: [PATCH 09/17] blackbox/dbcheck-links: Test broken links with missing
> <SID=...> on linked attributes
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13418
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit f81771c8593327e058b9cb4330d7e77083df3ea9)
>---
> .../knownfail.d/samba4.blackbox.dbcheck-links |   6 +
> ...ink-output-missing-link-sid-corruption.txt |   8 ++
> testprogs/blackbox/dbcheck-links.sh           | 110 ++++++++++++++++++
> 3 files changed, 124 insertions(+)
> create mode 100644 selftest/knownfail.d/samba4.blackbox.dbcheck-links
> create mode 100644 source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output-missing-link-sid-corruption.txt
>
>diff --git a/selftest/knownfail.d/samba4.blackbox.dbcheck-links b/selftest/knownfail.d/samba4.blackbox.dbcheck-links
>new file mode 100644
>index 000000000000..ee207a53ab74
>--- /dev/null
>+++ b/selftest/knownfail.d/samba4.blackbox.dbcheck-links
>@@ -0,0 +1,6 @@
>+# The first one fails and all others are follow up failures...
>+^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.dbcheck_missing_link_sid_corruption
>+^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.missing_link_sid_clean
>+^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.dbcheck_clean3
>+^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.dbcheck_dangling_multi_valued
>+^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.dangling_multi_valued_check_equal_or_too_many
>diff --git a/source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output-missing-link-sid-corruption.txt b/source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output-missing-link-sid-corruption.txt
>new file mode 100644
>index 000000000000..34576157f25d
>--- /dev/null
>+++ b/source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output-missing-link-sid-corruption.txt
>@@ -0,0 +1,8 @@
>+Change DN to <GUID=0da8f25e-d110-11e8-80b7-3c970ec68461>;<RMD_ADDTIME=123456789000000000>;<RMD_CHANGETIME=123456789000000000>;<RMD_FLAGS=1>;<RMD_INVOCID=4e4496a3-7fb8-4f97-8a33-d238db8b5e2d>;<RMD_LOCAL_USN=3769>;<RMD_ORIGINATING_USN=3769>;<RMD_VERSION=2>;<SID=S-1-5-21-4177067393-1453636373-93818738-771>;CN=missingsidu1,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp? [YES]
>+Change DN to <GUID=66eb8f52-d110-11e8-ab9b-3c970ec68461>;<RMD_ADDTIME=123456789000000000>;<RMD_CHANGETIME=123456789000000000>;<RMD_FLAGS=0>;<RMD_INVOCID=4e4496a3-7fb8-4f97-8a33-d238db8b5e2d>;<RMD_LOCAL_USN=3768>;<RMD_ORIGINATING_USN=3768>;<RMD_VERSION=1>;<SID=S-1-5-21-4177067393-1453636373-93818738-772>;CN=missingsidu2,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp? [YES]
>+Checked 231 objects (2 errors)
>+Checking 231 objects
>+ERROR: missing DN SID component for member in object CN=missingsidg3,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp - <GUID=0da8f25e-d110-11e8-80b7-3c970ec68461>;<RMD_ADDTIME=123456789000000000>;<RMD_CHANGETIME=123456789000000000>;<RMD_FLAGS=1>;<RMD_INVOCID=4e4496a3-7fb8-4f97-8a33-d238db8b5e2d>;<RMD_LOCAL_USN=3769>;<RMD_ORIGINATING_USN=3769>;<RMD_VERSION=2>;CN=missingsidu1,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp
>+ERROR: missing DN SID component for member in object CN=missingsidg3,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp - <GUID=66eb8f52-d110-11e8-ab9b-3c970ec68461>;<RMD_ADDTIME=123456789000000000>;<RMD_CHANGETIME=123456789000000000>;<RMD_FLAGS=0>;<RMD_INVOCID=4e4496a3-7fb8-4f97-8a33-d238db8b5e2d>;<RMD_LOCAL_USN=3768>;<RMD_ORIGINATING_USN=3768>;<RMD_VERSION=1>;CN=missingsidu2,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp
>+Fixed missing DN SID on attribute member
>+Fixed missing DN SID on attribute member
>diff --git a/testprogs/blackbox/dbcheck-links.sh b/testprogs/blackbox/dbcheck-links.sh
>index 13811ddb461b..9798813004c5 100755
>--- a/testprogs/blackbox/dbcheck-links.sh
>+++ b/testprogs/blackbox/dbcheck-links.sh
>@@ -131,6 +131,113 @@ check_expected_after_duplicate_links() {
>     fi
> }
> 
>+missing_link_sid_corruption() {
>+    # Step1: add user "missingsidu1"
>+    #
>+    ldif=$PREFIX_ABS/${RELEASE}/missing_link_sid_corruption1.ldif
>+    cat > $ldif <<EOF
>+dn: CN=missingsidu1,CN=users,DC=release-4-5-0-pre1,DC=samba,DC=corp
>+changetype: add
>+objectclass: user
>+samaccountname: missingsidu1
>+objectGUID: 0da8f25e-d110-11e8-80b7-3c970ec68461
>+objectSid: S-1-5-21-4177067393-1453636373-93818738-771
>+EOF
>+
>+    out=$(TZ=UTC $ldbmodify -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb --relax $ldif)
>+    if [ "$?" != "0" ]; then
>+	echo "ldbmodify returned:\n$out"
>+	return 1
>+    fi
>+
>+    # Step2: add user "missingsidu2"
>+    #
>+    ldif=$PREFIX_ABS/${RELEASE}/missing_link_sid_corruption2.ldif
>+    cat > $ldif <<EOF
>+dn: CN=missingsidu2,CN=users,DC=release-4-5-0-pre1,DC=samba,DC=corp
>+changetype: add
>+objectclass: user
>+samaccountname: missingsidu2
>+objectGUID: 66eb8f52-d110-11e8-ab9b-3c970ec68461
>+objectSid: S-1-5-21-4177067393-1453636373-93818738-772
>+EOF
>+
>+    out=$(TZ=UTC $ldbmodify -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb --relax $ldif)
>+    if [ "$?" != "0" ]; then
>+	echo "ldbmodify returned:\n$out"
>+	return 1
>+    fi
>+
>+    # Step3: add group "missingsidg3" and add users as members
>+    #
>+    ldif=$PREFIX_ABS/${RELEASE}/missing_link_sid_corruption3.ldif
>+    cat > $ldif <<EOF
>+dn: CN=missingsidg3,CN=users,DC=release-4-5-0-pre1,DC=samba,DC=corp
>+changetype: add
>+objectclass: group
>+samaccountname: missingsidg3
>+objectGUID: fd992424-d114-11e8-bb36-3c970ec68461
>+objectSid: S-1-5-21-4177067393-1453636373-93818738-773
>+member: CN=missingsidu1,CN=users,DC=release-4-5-0-pre1,DC=samba,DC=corp
>+member: CN=missingsidu2,CN=users,DC=release-4-5-0-pre1,DC=samba,DC=corp
>+EOF
>+
>+    out=$(TZ=UTC $ldbmodify -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb --relax $ldif)
>+    if [ "$?" != "0" ]; then
>+	echo "ldbmodify returned:\n$out"
>+	return 1
>+    fi
>+
>+    # Step4: remove one user again, so that we have one deleted link
>+    #
>+    ldif=$PREFIX_ABS/${RELEASE}/missing_link_sid_corruption4.ldif
>+    cat > $ldif <<EOF
>+dn: CN=missingsidg3,CN=users,DC=release-4-5-0-pre1,DC=samba,DC=corp
>+changetype: modify
>+delete: member
>+member: CN=missingsidu1,CN=users,DC=release-4-5-0-pre1,DC=samba,DC=corp
>+EOF
>+
>+    out=$(TZ=UTC $ldbmodify -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb --relax $ldif)
>+    if [ "$?" != "0" ]; then
>+	echo "ldbmodify returned:\n$out"
>+	return 1
>+    fi
>+
>+    #
>+    # Step5: remove the SIDS from the links
>+    #
>+    LDIF1=$(TZ=UTC $ldbsearch -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb -b 'CN=missingsidg3,CN=users,DC=release-4-5-0-pre1,DC=samba,DC=corp' -s base --reveal --extended-dn --show-binary member)
>+    DN=$(echo "${LDIF1}" | grep '^dn: ')
>+    MSG=$(echo "${LDIF1}" | grep -v '^dn: ' | grep -v '^#' | grep -v '^$')
>+    ldif=$PREFIX_ABS/${RELEASE}/missing_link_sid_corruption5.ldif
>+    {
>+	echo "${DN}"
>+	echo "changetype: modify"
>+	echo "replace: member"
>+	#echo "${MSG}"
>+	echo "${MSG}" | sed \
>+		-e 's!<SID=S-1-5-21-4177067393-1453636373-93818738-771>;!!g' \
>+		-e 's!<SID=S-1-5-21-4177067393-1453636373-93818738-772>;!!g' \
>+		-e 's!RMD_ADDTIME=[1-9][0-9]*!RMD_ADDTIME=123456789000000000!g' \
>+		-e 's!RMD_CHANGETIME=[1-9][0-9]*!RMD_CHANGETIME=123456789000000000!g' \
>+		| cat
>+    } > $ldif
>+
>+    out=$(TZ=UTC $ldbmodify -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb.d/DC%3DRELEASE-4-5-0-PRE1,DC%3DSAMBA,DC%3DCORP.ldb $ldif)
>+    if [ "$?" != "0" ]; then
>+	echo "ldbmodify returned:\n$out"
>+	return 1
>+    fi
>+
>+    return 0
>+}
>+
>+dbcheck_missing_link_sid_corruption() {
>+    dbcheck "-missing-link-sid-corruption" "1" ""
>+    return $?
>+}
>+
> forward_link_corruption() {
>     #
>     # Step1: add a duplicate forward link from
>@@ -344,6 +451,9 @@ if [ -d $release_dir ]; then
>     testit "dangling_one_way_link" dangling_one_way_link
>     testit "dbcheck_one_way" dbcheck_one_way
>     testit "dbcheck_clean2" dbcheck_clean
>+    testit "missing_link_sid_corruption" missing_link_sid_corruption
>+    testit "dbcheck_missing_link_sid_corruption" dbcheck_missing_link_sid_corruption
>+    testit "missing_link_sid_clean" dbcheck_clean
>     testit "dangling_one_way_dn" dangling_one_way_dn
>     testit "deleted_one_way_dn" deleted_one_way_dn
>     testit "dbcheck_clean3" dbcheck_clean
>-- 
>2.17.1
>
>
>From 05f06c474bda5f6ccf00d31c46e06377b8f08bf7 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Fri, 12 Oct 2018 18:43:25 +0200
>Subject: [PATCH 10/17] s4:repl_meta_data: pass down struct
> replmd_replicated_request to replmd_modify_handle_linked_attribs()
>
>This will simplify further changes.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13418
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 966c7febaf0245516481bde924ea6cd738eeb78b)
>---
> .../dsdb/samdb/ldb_modules/repl_meta_data.c   | 25 ++++++++-----------
> 1 file changed, 10 insertions(+), 15 deletions(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>index a2d803e6ed4d..50ebb6148c86 100644
>--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>@@ -3116,8 +3116,9 @@ static int replmd_modify_la_replace(struct ldb_module *module,
>  */
> static int replmd_modify_handle_linked_attribs(struct ldb_module *module,
> 					       struct replmd_private *replmd_private,
>+					       struct replmd_replicated_request *ac,
> 					       struct ldb_message *msg,
>-					       uint64_t seq_num, time_t t,
>+					       time_t t,
> 					       struct ldb_request *parent)
> {
> 	struct ldb_result *res;
>@@ -3126,8 +3127,6 @@ static int replmd_modify_handle_linked_attribs(struct ldb_module *module,
> 	struct ldb_context *ldb = ldb_module_get_ctx(module);
> 	struct ldb_message *old_msg;
> 
>-	const struct dsdb_schema *schema;
>-
> 	if (dsdb_functional_level(ldb) == DS_DOMAIN_FUNCTION_2000) {
> 		/*
> 		 * Nothing special is required for modifying or vanishing links
>@@ -3161,10 +3160,6 @@ static int replmd_modify_handle_linked_attribs(struct ldb_module *module,
> 	if (ret != LDB_SUCCESS) {
> 		return ret;
> 	}
>-	schema = dsdb_get_schema(ldb, res);
>-	if (!schema) {
>-		return LDB_ERR_OPERATIONS_ERROR;
>-	}
> 
> 	old_msg = res->msgs[0];
> 
>@@ -3173,7 +3168,7 @@ static int replmd_modify_handle_linked_attribs(struct ldb_module *module,
> 		struct ldb_message_element *old_el, *new_el;
> 		unsigned int mod_type = LDB_FLAG_MOD_TYPE(el->flags);
> 		const struct dsdb_attribute *schema_attr
>-			= dsdb_attribute_by_lDAPDisplayName(schema, el->name);
>+			= dsdb_attribute_by_lDAPDisplayName(ac->schema, el->name);
> 		if (!schema_attr) {
> 			ldb_asprintf_errstring(ldb,
> 					       "%s: attribute %s is not a valid attribute in schema",
>@@ -3209,22 +3204,22 @@ static int replmd_modify_handle_linked_attribs(struct ldb_module *module,
> 		switch (mod_type) {
> 		case LDB_FLAG_MOD_REPLACE:
> 			ret = replmd_modify_la_replace(module, replmd_private,
>-						       schema, msg, el, old_el,
>-						       schema_attr, seq_num, t,
>+						       ac->schema, msg, el, old_el,
>+						       schema_attr, ac->seq_num, t,
> 						       old_msg->dn,
> 						       parent);
> 			break;
> 		case LDB_FLAG_MOD_DELETE:
> 			ret = replmd_modify_la_delete(module, replmd_private,
>-						      schema, msg, el, old_el,
>-						      schema_attr, seq_num, t,
>+						      ac->schema, msg, el, old_el,
>+						      schema_attr, ac->seq_num, t,
> 						      old_msg->dn,
> 						      parent);
> 			break;
> 		case LDB_FLAG_MOD_ADD:
> 			ret = replmd_modify_la_add(module, replmd_private,
>-						   schema, msg, el, old_el,
>-						   schema_attr, seq_num, t,
>+						   ac->schema, msg, el, old_el,
>+						   schema_attr, ac->seq_num, t,
> 						   old_msg->dn,
> 						   parent);
> 			break;
>@@ -3496,7 +3491,7 @@ static int replmd_modify(struct ldb_module *module, struct ldb_request *req)
> 	}
> 
> 	ret = replmd_modify_handle_linked_attribs(module, replmd_private,
>-						  msg, ac->seq_num, t, req);
>+						  ac, msg, t, req);
> 	if (ret != LDB_SUCCESS) {
> 		talloc_free(ac);
> 		return ret;
>-- 
>2.17.1
>
>
>From b0f11b6f373773379b626e54e94eb899dce918c7 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Fri, 12 Oct 2018 18:43:25 +0200
>Subject: [PATCH 11/17] s4:repl_meta_data: pass down struct
> replmd_replicated_request to replmd_modify_la_add()
>
>This will simplify further changes.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13418
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 42e69a86ca583e3cb20c63b9c6930b4b3425485d)
>---
> .../dsdb/samdb/ldb_modules/repl_meta_data.c   | 27 +++++++------------
> 1 file changed, 10 insertions(+), 17 deletions(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>index 50ebb6148c86..d203c5712440 100644
>--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>@@ -2398,12 +2398,11 @@ static int replmd_update_la_val(TALLOC_CTX *mem_ctx, struct ldb_val *v, struct d
>  */
> static int replmd_modify_la_add(struct ldb_module *module,
> 				struct replmd_private *replmd_private,
>-				const struct dsdb_schema *schema,
>+				struct replmd_replicated_request *ac,
> 				struct ldb_message *msg,
> 				struct ldb_message_element *el,
> 				struct ldb_message_element *old_el,
> 				const struct dsdb_attribute *schema_attr,
>-				uint64_t seq_num,
> 				time_t t,
> 				struct ldb_dn *msg_dn,
> 				struct ldb_request *parent)
>@@ -2416,17 +2415,10 @@ static int replmd_modify_la_add(struct ldb_module *module,
> 	unsigned old_num_values = old_el ? old_el->num_values : 0;
> 	unsigned num_values = 0;
> 	unsigned max_num_values;
>-	const struct GUID *invocation_id;
> 	struct ldb_context *ldb = ldb_module_get_ctx(module);
> 	NTTIME now;
> 	unix_to_nt_time(&now, t);
> 
>-	invocation_id = samdb_ntds_invocation_id(ldb);
>-	if (!invocation_id) {
>-		talloc_free(tmp_ctx);
>-		return LDB_ERR_OPERATIONS_ERROR;
>-	}
>-
> 	/* get the DNs to be added, fully parsed.
> 	 *
> 	 * We need full parsing because they came off the wire and we don't
>@@ -2522,15 +2514,16 @@ static int replmd_modify_la_add(struct ldb_module *module,
> 			ret = replmd_update_la_val(new_values, exact->v,
> 						   dns[i].dsdb_dn,
> 						   exact->dsdb_dn,
>-						   invocation_id, seq_num,
>-						   seq_num, now, false);
>+						   &ac->our_invocation_id,
>+						   ac->seq_num, ac->seq_num,
>+						   now, false);
> 			if (ret != LDB_SUCCESS) {
> 				talloc_free(tmp_ctx);
> 				return ret;
> 			}
> 
> 			ret = replmd_add_backlink(module, replmd_private,
>-						  schema,
>+						  ac->schema,
> 						  msg_dn,
> 						  &dns[i].guid, 
> 						  true,
>@@ -2572,14 +2565,14 @@ static int replmd_modify_la_add(struct ldb_module *module,
> 		}
> 
> 		ret = replmd_add_backlink(module, replmd_private,
>-					  schema, msg_dn,
>+					  ac->schema, msg_dn,
> 					  &dns[i].guid,
> 					  true, schema_attr,
> 					  parent);
> 		/* Make the new linked attribute ldb_val. */
> 		ret = replmd_build_la_val(new_values, &new_values[num_values],
>-					  dns[i].dsdb_dn, invocation_id,
>-					  seq_num, now);
>+					  dns[i].dsdb_dn, &ac->our_invocation_id,
>+					  ac->seq_num, now);
> 		if (ret != LDB_SUCCESS) {
> 			talloc_free(tmp_ctx);
> 			return ret;
>@@ -3218,8 +3211,8 @@ static int replmd_modify_handle_linked_attribs(struct ldb_module *module,
> 			break;
> 		case LDB_FLAG_MOD_ADD:
> 			ret = replmd_modify_la_add(module, replmd_private,
>-						   ac->schema, msg, el, old_el,
>-						   schema_attr, ac->seq_num, t,
>+						   ac, msg, el, old_el,
>+						   schema_attr, t,
> 						   old_msg->dn,
> 						   parent);
> 			break;
>-- 
>2.17.1
>
>
>From 30203927397b626d9c5fd701e1ac1909dbd005d4 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Fri, 12 Oct 2018 19:34:08 +0200
>Subject: [PATCH 12/17] s4:repl_meta_data: add missing \n to a DEBUG message in
> replmd_modify_la_add()
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13418
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 70a306d0bd6806d1fd00d45e3d8cc70c73d09f79)
>---
> source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>index d203c5712440..385c831dd726 100644
>--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>@@ -2445,7 +2445,7 @@ static int replmd_modify_la_add(struct ldb_module *module,
> 	max_num_values = old_num_values + el->num_values;
> 	if (max_num_values < old_num_values) {
> 		DEBUG(0, ("we seem to have overflow in replmd_modify_la_add. "
>-			  "old values: %u, new values: %u, sum: %u",
>+			  "old values: %u, new values: %u, sum: %u\n",
> 			  old_num_values, el->num_values, max_num_values));
> 		talloc_free(tmp_ctx);
> 		return LDB_ERR_OPERATIONS_ERROR;
>-- 
>2.17.1
>
>
>From a1819ac7710b98fcf5bce6aa41eaa8b21b708120 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Fri, 12 Oct 2018 18:43:25 +0200
>Subject: [PATCH 13/17] s4:repl_meta_data: pass down struct
> replmd_replicated_request to replmd_modify_la_delete()
>
>This will simplify further changes.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13418
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 738b52eb0856c8fcdbb8589e8061bcc14700c23a)
>---
> .../dsdb/samdb/ldb_modules/repl_meta_data.c   | 27 ++++++++-----------
> 1 file changed, 11 insertions(+), 16 deletions(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>index 385c831dd726..b350e4969e4d 100644
>--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>@@ -2611,12 +2611,11 @@ static int replmd_modify_la_add(struct ldb_module *module,
>  */
> static int replmd_modify_la_delete(struct ldb_module *module,
> 				   struct replmd_private *replmd_private,
>-				   const struct dsdb_schema *schema,
>+				   struct replmd_replicated_request *ac,
> 				   struct ldb_message *msg,
> 				   struct ldb_message_element *el,
> 				   struct ldb_message_element *old_el,
> 				   const struct dsdb_attribute *schema_attr,
>-				   uint64_t seq_num,
> 				   time_t t,
> 				   struct ldb_dn *msg_dn,
> 				   struct ldb_request *parent)
>@@ -2630,16 +2629,10 @@ static int replmd_modify_la_delete(struct ldb_module *module,
> 	bool vanish_links = false;
> 	unsigned int num_to_delete = el->num_values;
> 	uint32_t rmd_flags;
>-	const struct GUID *invocation_id;
> 	NTTIME now;
> 
> 	unix_to_nt_time(&now, t);
> 
>-	invocation_id = samdb_ntds_invocation_id(ldb);
>-	if (!invocation_id) {
>-		return LDB_ERR_OPERATIONS_ERROR;
>-	}
>-
> 	if (old_el == NULL || old_el->num_values == 0) {
> 		/* there is nothing to delete... */
> 		if (num_to_delete == 0) {
>@@ -2703,7 +2696,7 @@ static int replmd_modify_la_delete(struct ldb_module *module,
> 				}
> 			}
> 			ret = replmd_add_backlink(module, replmd_private,
>-						  schema, msg_dn, &p->guid,
>+						  ac->schema, msg_dn, &p->guid,
> 						  false, schema_attr,
> 						  parent);
> 			if (ret != LDB_SUCCESS) {
>@@ -2721,8 +2714,9 @@ static int replmd_modify_la_delete(struct ldb_module *module,
> 
> 			ret = replmd_update_la_val(old_el->values, p->v,
> 						   p->dsdb_dn, p->dsdb_dn,
>-						   invocation_id, seq_num,
>-						   seq_num, now, true);
>+						   &ac->our_invocation_id,
>+						   ac->seq_num, ac->seq_num,
>+						   now, true);
> 			if (ret != LDB_SUCCESS) {
> 				talloc_free(tmp_ctx);
> 				return ret;
>@@ -2784,7 +2778,7 @@ static int replmd_modify_la_delete(struct ldb_module *module,
> 			/* remove the backlink */
> 			ret = replmd_add_backlink(module,
> 						  replmd_private,
>-						  schema, 
>+						  ac->schema,
> 						  msg_dn,
> 						  &p->guid,
> 						  false, schema_attr,
>@@ -2818,14 +2812,15 @@ static int replmd_modify_la_delete(struct ldb_module *module,
> 
> 		ret = replmd_update_la_val(old_el->values, exact->v,
> 					   exact->dsdb_dn, exact->dsdb_dn,
>-					   invocation_id, seq_num, seq_num,
>+					   &ac->our_invocation_id,
>+					   ac->seq_num, ac->seq_num,
> 					   now, true);
> 		if (ret != LDB_SUCCESS) {
> 			talloc_free(tmp_ctx);
> 			return ret;
> 		}
> 		ret = replmd_add_backlink(module, replmd_private,
>-					  schema, msg_dn,
>+					  ac->schema, msg_dn,
> 					  &p->guid,
> 					  false, schema_attr,
> 					  parent);
>@@ -3204,8 +3199,8 @@ static int replmd_modify_handle_linked_attribs(struct ldb_module *module,
> 			break;
> 		case LDB_FLAG_MOD_DELETE:
> 			ret = replmd_modify_la_delete(module, replmd_private,
>-						      ac->schema, msg, el, old_el,
>-						      schema_attr, ac->seq_num, t,
>+						      ac, msg, el, old_el,
>+						      schema_attr, t,
> 						      old_msg->dn,
> 						      parent);
> 			break;
>-- 
>2.17.1
>
>
>From ac7972f33f812b810459ad40b53952f5427915da Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Fri, 12 Oct 2018 18:43:25 +0200
>Subject: [PATCH 14/17] s4:repl_meta_data: pass down struct
> replmd_replicated_request to replmd_modify_la_replace()
>
>This will simplify further changes.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13418
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 1ef145d9d72d847055f6aba8a0070b3e1cfdabbc)
>---
> .../dsdb/samdb/ldb_modules/repl_meta_data.c   | 31 +++++++------------
> 1 file changed, 12 insertions(+), 19 deletions(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>index b350e4969e4d..2a3a86ed27ed 100644
>--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>@@ -2870,12 +2870,11 @@ static int replmd_modify_la_delete(struct ldb_module *module,
>  */
> static int replmd_modify_la_replace(struct ldb_module *module,
> 				    struct replmd_private *replmd_private,
>-				    const struct dsdb_schema *schema,
>+				    struct replmd_replicated_request *ac,
> 				    struct ldb_message *msg,
> 				    struct ldb_message_element *el,
> 				    struct ldb_message_element *old_el,
> 				    const struct dsdb_attribute *schema_attr,
>-				    uint64_t seq_num,
> 				    time_t t,
> 				    struct ldb_dn *msg_dn,
> 				    struct ldb_request *parent)
>@@ -2884,7 +2883,6 @@ static int replmd_modify_la_replace(struct ldb_module *module,
> 	struct parsed_dn *dns, *old_dns;
> 	TALLOC_CTX *tmp_ctx = talloc_new(msg);
> 	int ret;
>-	const struct GUID *invocation_id;
> 	struct ldb_context *ldb = ldb_module_get_ctx(module);
> 	struct ldb_val *new_values = NULL;
> 	const char *ldap_oid = schema_attr->syntax->ldap_oid;
>@@ -2895,11 +2893,6 @@ static int replmd_modify_la_replace(struct ldb_module *module,
> 
> 	unix_to_nt_time(&now, t);
> 
>-	invocation_id = samdb_ntds_invocation_id(ldb);
>-	if (!invocation_id) {
>-		return LDB_ERR_OPERATIONS_ERROR;
>-	}
>-
> 	/*
> 	 * The replace operation is unlike the replace and delete cases in that
> 	 * we need to look at every existing link to see whether it is being
>@@ -2999,8 +2992,8 @@ static int replmd_modify_la_replace(struct ldb_module *module,
> 				ret = replmd_update_la_val(new_values, old_p->v,
> 							   old_p->dsdb_dn,
> 							   old_p->dsdb_dn,
>-							   invocation_id,
>-							   seq_num, seq_num,
>+							   &ac->our_invocation_id,
>+							   ac->seq_num, ac->seq_num,
> 							   now, true);
> 				if (ret != LDB_SUCCESS) {
> 					talloc_free(tmp_ctx);
>@@ -3008,7 +3001,7 @@ static int replmd_modify_la_replace(struct ldb_module *module,
> 				}
> 
> 				ret = replmd_add_backlink(module, replmd_private,
>-							  schema, 
>+							  ac->schema,
> 							  msg_dn,
> 							  &old_p->guid, false,
> 							  schema_attr,
>@@ -3033,8 +3026,8 @@ static int replmd_modify_la_replace(struct ldb_module *module,
> 			ret = replmd_update_la_val(new_values, old_p->v,
> 						   new_p->dsdb_dn,
> 						   old_p->dsdb_dn,
>-						   invocation_id,
>-						   seq_num, seq_num,
>+						   &ac->our_invocation_id,
>+						   ac->seq_num, ac->seq_num,
> 						   now, false);
> 			if (ret != LDB_SUCCESS) {
> 				talloc_free(tmp_ctx);
>@@ -3044,7 +3037,7 @@ static int replmd_modify_la_replace(struct ldb_module *module,
> 			rmd_flags = dsdb_dn_rmd_flags(old_p->dsdb_dn->dn);
> 			if ((rmd_flags & DSDB_RMD_FLAG_DELETED) != 0) {
> 				ret = replmd_add_backlink(module, replmd_private,
>-							  schema, 
>+							  ac->schema,
> 							  msg_dn,
> 							  &new_p->guid, true,
> 							  schema_attr,
>@@ -3066,14 +3059,14 @@ static int replmd_modify_la_replace(struct ldb_module *module,
> 			ret = replmd_build_la_val(new_values,
> 						  new_p->v,
> 						  new_p->dsdb_dn,
>-						  invocation_id,
>-						  seq_num, now);
>+						  &ac->our_invocation_id,
>+						  ac->seq_num, now);
> 			if (ret != LDB_SUCCESS) {
> 				talloc_free(tmp_ctx);
> 				return ret;
> 			}
> 			ret = replmd_add_backlink(module, replmd_private,
>-						  schema,
>+						  ac->schema,
> 						  msg_dn,
> 						  &new_p->guid, true,
> 						  schema_attr,
>@@ -3192,8 +3185,8 @@ static int replmd_modify_handle_linked_attribs(struct ldb_module *module,
> 		switch (mod_type) {
> 		case LDB_FLAG_MOD_REPLACE:
> 			ret = replmd_modify_la_replace(module, replmd_private,
>-						       ac->schema, msg, el, old_el,
>-						       schema_attr, ac->seq_num, t,
>+						       ac, msg, el, old_el,
>+						       schema_attr, t,
> 						       old_msg->dn,
> 						       parent);
> 			break;
>-- 
>2.17.1
>
>
>From 7e80d8df6bd485448095e726010a8999923bc585 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Fri, 12 Oct 2018 15:56:18 +0200
>Subject: [PATCH 15/17] s4:repl_meta_data: add support for
> DSDB_CONTROL_DBCHECK_FIX_LINK_DN_SID
>
>This will be used by dbcheck in the next commits.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13418
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 0386307e34097f5d9233c970983c7306d1705a87)
>---
> .../knownfail.d/samba4.blackbox.dbcheck-links |   6 -
> .../samdb/ldb_modules/extended_dn_store.c     |   7 +
> .../dsdb/samdb/ldb_modules/repl_meta_data.c   | 145 ++++++++++++++++++
> source4/dsdb/samdb/ldb_modules/samldb.c       |  12 +-
> 4 files changed, 161 insertions(+), 9 deletions(-)
> delete mode 100644 selftest/knownfail.d/samba4.blackbox.dbcheck-links
>
>diff --git a/selftest/knownfail.d/samba4.blackbox.dbcheck-links b/selftest/knownfail.d/samba4.blackbox.dbcheck-links
>deleted file mode 100644
>index ee207a53ab74..000000000000
>--- a/selftest/knownfail.d/samba4.blackbox.dbcheck-links
>+++ /dev/null
>@@ -1,6 +0,0 @@
>-# The first one fails and all others are follow up failures...
>-^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.dbcheck_missing_link_sid_corruption
>-^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.missing_link_sid_clean
>-^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.dbcheck_clean3
>-^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.dbcheck_dangling_multi_valued
>-^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.dangling_multi_valued_check_equal_or_too_many
>diff --git a/source4/dsdb/samdb/ldb_modules/extended_dn_store.c b/source4/dsdb/samdb/ldb_modules/extended_dn_store.c
>index a32ab8d6f935..8e5a2ba7ae94 100644
>--- a/source4/dsdb/samdb/ldb_modules/extended_dn_store.c
>+++ b/source4/dsdb/samdb/ldb_modules/extended_dn_store.c
>@@ -376,6 +376,7 @@ static int extended_dn_modify(struct ldb_module *module, struct ldb_request *req
> 	unsigned int i, j;
> 	struct extended_dn_context *ac;
> 	struct ldb_control *fix_links_control = NULL;
>+	struct ldb_control *fix_link_sid_ctrl = NULL;
> 	int ret;
> 
> 	if (ldb_dn_is_special(req->op.mod.message->dn)) {
>@@ -400,6 +401,12 @@ static int extended_dn_modify(struct ldb_module *module, struct ldb_request *req
> 		return ldb_next_request(module, req);
> 	}
> 
>+	fix_link_sid_ctrl = ldb_request_get_control(ac->req,
>+					DSDB_CONTROL_DBCHECK_FIX_LINK_DN_SID);
>+	if (fix_link_sid_ctrl != NULL) {
>+		return ldb_next_request(module, req);
>+	}
>+
> 	for (i=0; i < req->op.mod.message->num_elements; i++) {
> 		const struct ldb_message_element *el = &req->op.mod.message->elements[i];
> 		const struct dsdb_attribute *schema_attr
>diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>index 2a3a86ed27ed..5bdc57366da6 100644
>--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>@@ -112,6 +112,8 @@ struct replmd_replicated_request {
> 	bool is_urgent;
> 
> 	bool isDeleted;
>+
>+	bool fix_link_sid;
> };
> 
> static int replmd_replicated_apply_merge(struct replmd_replicated_request *ar);
>@@ -2481,6 +2483,109 @@ static int replmd_modify_la_add(struct ldb_module *module,
> 			return err;
> 		}
> 
>+		if (ac->fix_link_sid) {
>+			char *fixed_dnstring = NULL;
>+			struct dom_sid tmp_sid = { 0, };
>+			DATA_BLOB sid_blob = data_blob_null;
>+			enum ndr_err_code ndr_err;
>+			NTSTATUS status;
>+			int num;
>+
>+			if (exact == NULL) {
>+				talloc_free(tmp_ctx);
>+				return ldb_operr(ldb);
>+			}
>+
>+			if (dns[i].dsdb_dn->dn_format != DSDB_NORMAL_DN) {
>+				talloc_free(tmp_ctx);
>+				return ldb_operr(ldb);
>+			}
>+
>+			/*
>+			 * Only "<GUID=...><SID=...>" is allowed.
>+			 *
>+			 * We get the GUID to just to find the old
>+			 * value and the SID in order to add it
>+			 * to the found value.
>+			 */
>+
>+			num = ldb_dn_get_comp_num(dns[i].dsdb_dn->dn);
>+			if (num != 0) {
>+				talloc_free(tmp_ctx);
>+				return ldb_operr(ldb);
>+			}
>+
>+			num = ldb_dn_get_extended_comp_num(dns[i].dsdb_dn->dn);
>+			if (num != 2) {
>+				talloc_free(tmp_ctx);
>+				return ldb_operr(ldb);
>+			}
>+
>+			status = dsdb_get_extended_dn_sid(exact->dsdb_dn->dn,
>+							  &tmp_sid, "SID");
>+			if (NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND)) {
>+				/* this is what we expect */
>+			} else if (NT_STATUS_IS_OK(status)) {
>+				struct GUID_txt_buf guid_str;
>+				ldb_debug_set(ldb, LDB_DEBUG_FATAL,
>+						       "i[%u] SID NOT MISSING... Attribute %s already "
>+						       "exists for target GUID %s, SID %s, DN: %s",
>+						       i, el->name,
>+						       GUID_buf_string(&exact->guid,
>+								       &guid_str),
>+						       dom_sid_string(tmp_ctx, &tmp_sid),
>+						       dsdb_dn_get_extended_linearized(tmp_ctx,
>+							       exact->dsdb_dn, 1));
>+				talloc_free(tmp_ctx);
>+				return LDB_ERR_ATTRIBUTE_OR_VALUE_EXISTS;
>+			} else {
>+				talloc_free(tmp_ctx);
>+				return ldb_operr(ldb);
>+			}
>+
>+			status = dsdb_get_extended_dn_sid(dns[i].dsdb_dn->dn,
>+							  &tmp_sid, "SID");
>+			if (!NT_STATUS_IS_OK(status)) {
>+				struct GUID_txt_buf guid_str;
>+				ldb_asprintf_errstring(ldb,
>+						       "NO SID PROVIDED... Attribute %s already "
>+						       "exists for target GUID %s",
>+						       el->name,
>+						       GUID_buf_string(&exact->guid,
>+								       &guid_str));
>+				talloc_free(tmp_ctx);
>+				return LDB_ERR_ATTRIBUTE_OR_VALUE_EXISTS;
>+			}
>+
>+			ndr_err = ndr_push_struct_blob(&sid_blob, tmp_ctx, &tmp_sid,
>+						       (ndr_push_flags_fn_t)ndr_push_dom_sid);
>+			if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
>+				talloc_free(tmp_ctx);
>+				return ldb_operr(ldb);
>+			}
>+
>+			ret = ldb_dn_set_extended_component(exact->dsdb_dn->dn, "SID", &sid_blob);
>+			data_blob_free(&sid_blob);
>+			if (ret != LDB_SUCCESS) {
>+				talloc_free(tmp_ctx);
>+				return ret;
>+			}
>+
>+			fixed_dnstring = dsdb_dn_get_extended_linearized(
>+					new_values, exact->dsdb_dn, 1);
>+			if (fixed_dnstring == NULL) {
>+				talloc_free(tmp_ctx);
>+				return ldb_operr(ldb);
>+			}
>+
>+			/*
>+			 * We just replace the existing value...
>+			 */
>+			*exact->v = data_blob_string_const(fixed_dnstring);
>+
>+			continue;
>+		}
>+
> 		if (exact != NULL) {
> 			/*
> 			 * We are trying to add one that exists, which is only
>@@ -3306,6 +3411,7 @@ static int replmd_modify(struct ldb_module *module, struct ldb_request *req)
> 	struct ldb_control *sd_propagation_control;
> 	struct ldb_control *fix_links_control = NULL;
> 	struct ldb_control *fix_dn_name_control = NULL;
>+	struct ldb_control *fix_dn_sid_control = NULL;
> 	struct replmd_private *replmd_private =
> 		talloc_get_type(ldb_module_get_private(module), struct replmd_private);
> 
>@@ -3451,6 +3557,44 @@ static int replmd_modify(struct ldb_module *module, struct ldb_request *req)
> 		return LDB_ERR_OPERATIONS_ERROR;
> 	}
> 
>+	fix_dn_sid_control = ldb_request_get_control(req,
>+					DSDB_CONTROL_DBCHECK_FIX_LINK_DN_SID);
>+	if (fix_dn_sid_control != NULL) {
>+		const struct dsdb_attribute *sa = NULL;
>+
>+		if (msg->num_elements != 1) {
>+			talloc_free(ac);
>+			return ldb_module_operr(module);
>+		}
>+
>+		if (msg->elements[0].flags != LDB_FLAG_MOD_ADD) {
>+			talloc_free(ac);
>+			return ldb_module_operr(module);
>+		}
>+
>+		if (msg->elements[0].num_values != 1) {
>+			talloc_free(ac);
>+			return ldb_module_operr(module);
>+		}
>+
>+		sa = dsdb_attribute_by_lDAPDisplayName(ac->schema,
>+				msg->elements[0].name);
>+		if (sa == NULL) {
>+			talloc_free(ac);
>+			return ldb_module_operr(module);
>+		}
>+
>+		if (sa->dn_format != DSDB_NORMAL_DN) {
>+			talloc_free(ac);
>+			return ldb_module_operr(module);
>+		}
>+
>+		fix_dn_sid_control->critical = false;
>+		ac->fix_link_sid = true;
>+
>+		goto handle_linked_attribs;
>+	}
>+
> 	ldb_msg_remove_attr(msg, "whenChanged");
> 	ldb_msg_remove_attr(msg, "uSNChanged");
> 
>@@ -3471,6 +3615,7 @@ static int replmd_modify(struct ldb_module *module, struct ldb_request *req)
> 		return ret;
> 	}
> 
>+ handle_linked_attribs:
> 	ret = replmd_modify_handle_linked_attribs(module, replmd_private,
> 						  ac, msg, t, req);
> 	if (ret != LDB_SUCCESS) {
>diff --git a/source4/dsdb/samdb/ldb_modules/samldb.c b/source4/dsdb/samdb/ldb_modules/samldb.c
>index 734d0be26a93..81d8c96437c9 100644
>--- a/source4/dsdb/samdb/ldb_modules/samldb.c
>+++ b/source4/dsdb/samdb/ldb_modules/samldb.c
>@@ -3770,9 +3770,15 @@ static int samldb_modify(struct ldb_module *module, struct ldb_request *req)
> 
> 	el = ldb_msg_find_element(ac->msg, "member");
> 	if (el != NULL) {
>-		ret = samldb_member_check(ac);
>-		if (ret != LDB_SUCCESS) {
>-			return ret;
>+		struct ldb_control *fix_link_sid_ctrl = NULL;
>+
>+		fix_link_sid_ctrl = ldb_request_get_control(ac->req,
>+					DSDB_CONTROL_DBCHECK_FIX_LINK_DN_SID);
>+		if (fix_link_sid_ctrl == NULL) {
>+			ret = samldb_member_check(ac);
>+			if (ret != LDB_SUCCESS) {
>+				return ret;
>+			}
> 		}
> 	}
> 
>-- 
>2.17.1
>
>
>From 92b652746a667f381890fc6649bb4e5ac8926025 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Fri, 24 Aug 2018 15:33:49 +0200
>Subject: [PATCH 16/17] s4:samldb: internally use extended dns while changing
> the primaryGroupID field
>
>This is important, otherwise we'll loose the <SID=> component of the
>linked attribute.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13418
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 7a36cb30b716d56b84e894851c1a18e9eb3a0964)
>---
> .../samba4.blackbox.test_primary_group        |  2 --
> source4/dsdb/samdb/ldb_modules/samldb.c       | 29 ++++++++++++++-----
> 2 files changed, 21 insertions(+), 10 deletions(-)
> delete mode 100644 selftest/knownfail.d/samba4.blackbox.test_primary_group
>
>diff --git a/selftest/knownfail.d/samba4.blackbox.test_primary_group b/selftest/knownfail.d/samba4.blackbox.test_primary_group
>deleted file mode 100644
>index 779f6808c977..000000000000
>--- a/selftest/knownfail.d/samba4.blackbox.test_primary_group
>+++ /dev/null
>@@ -1,2 +0,0 @@
>-^samba4.blackbox.test_primary_group.dbcheck.*run1
>-^samba4.blackbox.test_primary_group.dbcheck.*run2
>diff --git a/source4/dsdb/samdb/ldb_modules/samldb.c b/source4/dsdb/samdb/ldb_modules/samldb.c
>index 81d8c96437c9..49c63cf12720 100644
>--- a/source4/dsdb/samdb/ldb_modules/samldb.c
>+++ b/source4/dsdb/samdb/ldb_modules/samldb.c
>@@ -1642,9 +1642,14 @@ static int samldb_prim_group_change(struct samldb_ctx *ac)
> 	struct ldb_result *res, *group_res;
> 	struct ldb_message_element *el;
> 	struct ldb_message *msg;
>+	uint32_t search_flags =
>+		DSDB_FLAG_NEXT_MODULE | DSDB_SEARCH_SHOW_EXTENDED_DN;
> 	uint32_t prev_rid, new_rid, uac;
> 	struct dom_sid *prev_sid, *new_sid;
> 	struct ldb_dn *prev_prim_group_dn, *new_prim_group_dn;
>+	const char *new_prim_group_dn_ext_str = NULL;
>+	struct ldb_dn *user_dn = NULL;
>+	const char *user_dn_ext_str = NULL;
> 	int ret;
> 	const char * const noattrs[] = { NULL };
> 
>@@ -1658,10 +1663,15 @@ static int samldb_prim_group_change(struct samldb_ctx *ac)
> 	/* Fetch information from the existing object */
> 
> 	ret = dsdb_module_search_dn(ac->module, ac, &res, ac->msg->dn, attrs,
>-				    DSDB_FLAG_NEXT_MODULE, ac->req);
>+				    search_flags, ac->req);
> 	if (ret != LDB_SUCCESS) {
> 		return ret;
> 	}
>+	user_dn = res->msgs[0]->dn;
>+	user_dn_ext_str = ldb_dn_get_extended_linearized(ac, user_dn, 1);
>+	if (user_dn_ext_str == NULL) {
>+		return ldb_operr(ldb);
>+	}
> 
> 	uac = ldb_msg_find_attr_as_uint(res->msgs[0], "userAccountControl", 0);
> 
>@@ -1725,7 +1735,7 @@ static int samldb_prim_group_change(struct samldb_ctx *ac)
> 	ret = dsdb_module_search(ac->module, ac, &group_res,
> 				 ldb_get_default_basedn(ldb),
> 				 LDB_SCOPE_SUBTREE,
>-				 noattrs, DSDB_FLAG_NEXT_MODULE,
>+				 noattrs, search_flags,
> 				 ac->req,
> 				 "(objectSid=%s)",
> 				 ldap_encode_ndr_dom_sid(ac, prev_sid));
>@@ -1745,7 +1755,7 @@ static int samldb_prim_group_change(struct samldb_ctx *ac)
> 	ret = dsdb_module_search(ac->module, ac, &group_res,
> 				 ldb_get_default_basedn(ldb),
> 				 LDB_SCOPE_SUBTREE,
>-				 noattrs, DSDB_FLAG_NEXT_MODULE,
>+				 noattrs, search_flags,
> 				 ac->req,
> 				 "(objectSid=%s)",
> 				 ldap_encode_ndr_dom_sid(ac, new_sid));
>@@ -1758,11 +1768,16 @@ static int samldb_prim_group_change(struct samldb_ctx *ac)
> 		return LDB_ERR_UNWILLING_TO_PERFORM;
> 	}
> 	new_prim_group_dn = group_res->msgs[0]->dn;
>+	new_prim_group_dn_ext_str = ldb_dn_get_extended_linearized(ac,
>+							new_prim_group_dn, 1);
>+	if (new_prim_group_dn_ext_str == NULL) {
>+		return ldb_operr(ldb);
>+	}
> 
> 	/* We need to be already a normal member of the new primary
> 	 * group in order to be successful. */
> 	el = samdb_find_attribute(ldb, res->msgs[0], "memberOf",
>-				  ldb_dn_get_linearized(new_prim_group_dn));
>+				  new_prim_group_dn_ext_str);
> 	if (el == NULL) {
> 		return LDB_ERR_UNWILLING_TO_PERFORM;
> 	}
>@@ -1774,8 +1789,7 @@ static int samldb_prim_group_change(struct samldb_ctx *ac)
> 	}
> 	msg->dn = new_prim_group_dn;
> 
>-	ret = samdb_msg_add_delval(ldb, msg, msg, "member",
>-				   ldb_dn_get_linearized(ac->msg->dn));
>+	ret = samdb_msg_add_delval(ldb, msg, msg, "member", user_dn_ext_str);
> 	if (ret != LDB_SUCCESS) {
> 		return ret;
> 	}
>@@ -1793,8 +1807,7 @@ static int samldb_prim_group_change(struct samldb_ctx *ac)
> 	}
> 	msg->dn = prev_prim_group_dn;
> 
>-	ret = samdb_msg_add_addval(ldb, msg, msg, "member",
>-				   ldb_dn_get_linearized(ac->msg->dn));
>+	ret = samdb_msg_add_addval(ldb, msg, msg, "member", user_dn_ext_str);
> 	if (ret != LDB_SUCCESS) {
> 		return ret;
> 	}
>-- 
>2.17.1
>
>
>From 788461553711149f3dd52c79a11f52de40448fe7 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Tue, 30 Oct 2018 15:56:43 +1300
>Subject: [PATCH 17/17] dsdb: Add comments explaining the limitations of our
> current backlink behaviour
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13418
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Tim Beale <timbeale@catalyst.net.nz>
>
>Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
>Autobuild-Date(master): Tue Oct 30 10:32:51 CET 2018 on sn-devel-144
>
>(cherry picked from commit 852e1db12b0afa04a738c03bb2609c084fe96a7f)
>---
> .../samdb/ldb_modules/linked_attributes.c     | 18 +++++++++++++-
> .../dsdb/samdb/ldb_modules/repl_meta_data.c   | 24 ++++++++++++++-----
> testprogs/blackbox/test_primary_group.sh      |  6 ++++-
> 3 files changed, 40 insertions(+), 8 deletions(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/linked_attributes.c b/source4/dsdb/samdb/ldb_modules/linked_attributes.c
>index c6beb25e58b4..c3f1a1484baa 100644
>--- a/source4/dsdb/samdb/ldb_modules/linked_attributes.c
>+++ b/source4/dsdb/samdb/ldb_modules/linked_attributes.c
>@@ -25,7 +25,23 @@
>  *
>  *  Component: ldb linked_attributes module
>  *
>- *  Description: Module to ensure linked attribute pairs remain in sync
>+ *  Description: Module to ensure linked attribute pairs (i.e. forward-links
>+ *  and backlinks) remain in sync.
>+ *
>+ *  Backlinks are 'plain' links (without extra metadata). When the link target
>+ *  object is modified (e.g. renamed), we use the backlinks to keep the link
>+ *  source object updated. Note there are some cases where we can't do this:
>+ *    - one-way links, which don't have a corresponding backlink
>+ *    - two-way deactivated links, i.e. when a user is removed from a group,
>+ *      the forward 'member' link still exists (but is inactive), however, the
>+ *      'memberOf' backlink is deleted.
>+ *  In these cases, we can end up with a dangling forward link which is
>+ *  incorrect (i.e. the target has been renamed or deleted). We have dbcheck
>+ *  rules to detect and fix this, and cope otherwise by filtering at runtime
>+ *  (i.e. in the extended_dn module).
>+ *
>+ *  See also repl_meta_data.c, which handles updating links for deleted
>+ *  objects, as well as link changes received from another DC.
>  *
>  *  Author: Andrew Bartlett
>  */
>diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>index 5bdc57366da6..c2eafd0a5215 100644
>--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>@@ -4367,6 +4367,10 @@ static int replmd_delete_internals(struct ldb_module *module, struct ldb_request
> 	     - preserved if in above list, or is rDN
> 	  - remove all linked attribs from this object
> 	  - remove all links from other objects to this object
>+	    (note we use the backlinks to do this, so we won't find one-way
>+	     links that still point to this object, or deactivated two-way
>+	     links, i.e. 'member' after the user has been removed from the
>+	     group)
> 	  - add lastKnownParent
> 	  - update replPropertyMetaData?
> 
>@@ -4488,12 +4492,12 @@ static int replmd_delete_internals(struct ldb_module *module, struct ldb_request
> 
> 			if (sa->linkID & 1) {
> 				/*
>-				  we have a backlink in this object
>-				  that needs to be removed. We're not
>-				  allowed to remove it directly
>-				  however, so we instead setup a
>-				  modify to delete the corresponding
>-				  forward link
>+				 * we have a backlink in this object
>+				 * that needs to be removed. We're not
>+				 * allowed to remove it directly
>+				 * however, so we instead setup a
>+				 * modify to delete the corresponding
>+				 * forward link
> 				 */
> 				ret = replmd_delete_remove_link(module, schema,
> 								replmd_private,
>@@ -7626,6 +7630,14 @@ static int replmd_delete_link_value(struct ldb_module *module,
> 	/* if the existing link is active, remove its backlink */
> 	if (is_active) {
> 
>+		/*
>+		 * NOTE WELL: After this we will never (at runtime) be
>+		 * able to find this forward link (for instant
>+		 * removal) if/when the link target is deleted.
>+		 *
>+		 * We have dbcheck rules to cover this and cope otherwise
>+		 * by filtering at runtime (i.e. in the extended_dn module).
>+		 */
> 		ret = replmd_add_backlink(module, replmd_private, schema,
> 					  src_obj_dn, target_guid, false,
> 					  attr, NULL);
>diff --git a/testprogs/blackbox/test_primary_group.sh b/testprogs/blackbox/test_primary_group.sh
>index c2d803e1d15c..0fbc287114ad 100755
>--- a/testprogs/blackbox/test_primary_group.sh
>+++ b/testprogs/blackbox/test_primary_group.sh
>@@ -75,11 +75,15 @@ testit "delete '$testgroup'" $VALGRIND $PYTHON $BINDIR/samba-tool group delete "
> 
> #
> # As we don't support phantom objects and virtual backlinks
>-# the deletion of the user and group cause dangling links,
>+# the deletion of the user prior to the group causes dangling links,
> # which are detected like this:
> #
> # WARNING: target DN is deleted for member in object
> #
>+# Specifically, this happens because after the member link is
>+# deactivated the memberOf is gone, and so there is no way to find the
>+# now redundant forward link to clean it up.
>+#
> testit_expect_failure "dbcheck run3" $VALGRIND $PYTHON $BINDIR/samba-tool dbcheck --attrs=member --fix --yes || failed=`expr $failed + 1`
> testit "dbcheck run4" $VALGRIND $PYTHON $BINDIR/samba-tool dbcheck --attrs=member || failed=`expr $failed + 1`
> 
>-- 
>2.17.1
>

Flags:

abartlet: review+

Actions: View

Attachments on bug 13418: 14171 | 14177 | 14549 | 14550