[Patches] Fix db corruption happening while changing primaryGroupID (bug #13418)

Stefan Metzmacher metze at samba.org
Fri Oct 26 19:51:16 UTC 2018


Hi,

> These patches prevent the corruption from happening and allow samba-tool
> dbcheck to repair an affected database.
> 
> For more details see https://bugzilla.samba.org/show_bug.cgi?id=13418.
> 
> I created a merge request here:
> https://gitlab.com/samba-team/samba/merge_requests/90
> The pipeline is running here:
> https://gitlab.com/samba-team/devel/samba/pipelines/33196085
> And one with the same content already passed:
> https://gitlab.com/samba-team/devel/samba/pipelines/33148483
> 
> Please review and push:-)

These patches had some interaction with other tests and
revealed a bug in the group_audit code.

I've updated the "testprogs/blackbox: add
samba4.blackbox.test_primary_group test" commit to
remove the testuser/group at the end in order to
avoid leaking them to other tests.

These patches passed about 8 private autobuilds.

A pipeline is now running here:
https://gitlab.com/samba-team/devel/samba/pipelines/34485522

Please review and push:-)

Thanks!
metze
-------------- next part --------------
From 0b65e5b7e9d9cbd2a302b034246f9d4cdaacebaf Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 8 Oct 2018 15:35:52 +0200
Subject: [PATCH 01/14] 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 at samba.org>
---
 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 648f04944b88..0ea51977775e 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 2e1b82be58b3dea1e3adfe557b55462044df58d9 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 8 Oct 2018 17:13:13 +0200
Subject: [PATCH 02/14] s4:dsdb: fix comment on
 DSDB_CONTROL_DBCHECK_FIX_LINK_DN_NAME

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 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 805a1f65fa83..08dc67082a90 100644
--- a/source4/dsdb/samdb/samdb.h
+++ b/source4/dsdb/samdb/samdb.h
@@ -132,7 +132,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 043b6015d63352a3b2e8a8c4964b9390e5feea4c Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 16 Oct 2018 15:16:18 +0200
Subject: [PATCH 03/14] 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 at samba.org>
---
 .../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 6b5ceb556c99..24817e40fb5c 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -430,6 +430,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 2198438eb70c4026c7d126004dbeeeeea721aa60 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 8 Oct 2018 17:13:52 +0200
Subject: [PATCH 04/14] 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 at samba.org>
---
 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 19fb75dfc8b3..36cc80d45352 100644
--- a/source4/dsdb/pydsdb.c
+++ b/source4/dsdb/pydsdb.c
@@ -1659,6 +1659,7 @@ MODULE_INIT_FUNC(dsdb)
 	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 08dc67082a90..e1b0e4aa4e3e 100644
--- a/source4/dsdb/samdb/samdb.h
+++ b/source4/dsdb/samdb/samdb.h
@@ -135,6 +135,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 0ea51977775e..a84675843658 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 ecc19aa39a710ba6930f9f79f5453afb11f893a4 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 8 Oct 2018 17:14:28 +0200
Subject: [PATCH 05/14] 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 at samba.org>
---
 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 dcdbb893095e..690d5d9f1845 100644
--- a/python/samba/dbchecker.py
+++ b/python/samba/dbchecker.py
@@ -402,10 +402,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 as err:
             if self.in_transaction:
-- 
2.17.1


From 77e0c382f3d566d2e02b2d8f57428f5a899d078d Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 12 Oct 2018 15:56:18 +0200
Subject: [PATCH 06/14] dbchecker: Fix missing <SID=...> on linked attributes

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 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 690d5d9f1845..c70ca7bc2123 100644
--- a/python/samba/dbchecker.py
+++ b/python/samba/dbchecker.py
@@ -79,6 +79,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
@@ -698,6 +699,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))
@@ -1323,7 +1356,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 5c71de3c85389dd29ac1e1c2be988d5bf67be7f6 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 12 Oct 2018 15:56:18 +0200
Subject: [PATCH 07/14] 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 at samba.org>
---
 .../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 dfcf03ccf6de72715cb6cabc6657a08dcf2250a1 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 12 Oct 2018 18:43:25 +0200
Subject: [PATCH 08/14] 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 at samba.org>
---
 .../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 584fd2185a2c..e5baefc4cb43 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -3120,8 +3120,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;
@@ -3130,8 +3131,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
@@ -3165,10 +3164,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];
 
@@ -3177,7 +3172,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",
@@ -3213,22 +3208,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;
@@ -3500,7 +3495,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 4f739a00290c042f97b5f34dd717ecbe584510b2 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 12 Oct 2018 18:43:25 +0200
Subject: [PATCH 09/14] 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 at samba.org>
---
 .../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 e5baefc4cb43..d5d9c4f8eeb1 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -2402,12 +2402,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)
@@ -2420,17 +2419,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
@@ -2526,15 +2518,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,
@@ -2576,14 +2569,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;
@@ -3222,8 +3215,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 f666c56bf47f0f629899ce0a56f8c028b5a88e71 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 12 Oct 2018 19:34:08 +0200
Subject: [PATCH 10/14] 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 at samba.org>
---
 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 d5d9c4f8eeb1..5aa3ed7badaa 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -2449,7 +2449,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 cd7db06331448dccdc43b258cc507a00cbcff854 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 12 Oct 2018 18:43:25 +0200
Subject: [PATCH 11/14] 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 at samba.org>
---
 .../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 5aa3ed7badaa..3ce69e06920e 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -2615,12 +2615,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)
@@ -2634,16 +2633,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) {
@@ -2707,7 +2700,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) {
@@ -2725,8 +2718,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;
@@ -2788,7 +2782,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,
@@ -2822,14 +2816,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);
@@ -3208,8 +3203,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 bd621626523e8849119030bff5a9131eca6c67f7 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 12 Oct 2018 18:43:25 +0200
Subject: [PATCH 12/14] 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 at samba.org>
---
 .../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 3ce69e06920e..5ef5fee6fa2d 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -2874,12 +2874,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)
@@ -2888,7 +2887,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;
@@ -2899,11 +2897,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
@@ -3003,8 +2996,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);
@@ -3012,7 +3005,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,
@@ -3037,8 +3030,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);
@@ -3048,7 +3041,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,
@@ -3070,14 +3063,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,
@@ -3196,8 +3189,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 a8caa8671f941e368b5696dc130b353df1a17113 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 12 Oct 2018 15:56:18 +0200
Subject: [PATCH 13/14] 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 at samba.org>
---
 .../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 a37b55c40469..6bfced3b7d2b 100644
--- a/source4/dsdb/samdb/ldb_modules/extended_dn_store.c
+++ b/source4/dsdb/samdb/ldb_modules/extended_dn_store.c
@@ -722,6 +722,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)) {
@@ -746,6 +747,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 5ef5fee6fa2d..231390949ac5 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);
@@ -2485,6 +2487,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
@@ -3310,6 +3415,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);
 
@@ -3455,6 +3561,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");
 
@@ -3475,6 +3619,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 a7995e31cc18..30741f5cb7a8 100644
--- a/source4/dsdb/samdb/ldb_modules/samldb.c
+++ b/source4/dsdb/samdb/ldb_modules/samldb.c
@@ -3808,9 +3808,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 d3bcea7b87c7960d50879251f8fb35eb910456e9 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 24 Aug 2018 15:33:49 +0200
Subject: [PATCH 14/14] 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 at samba.org>
---
 .../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 30741f5cb7a8..e69228c32c75 100644
--- a/source4/dsdb/samdb/ldb_modules/samldb.c
+++ b/source4/dsdb/samdb/ldb_modules/samldb.c
@@ -1680,9 +1680,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 };
 
@@ -1696,10 +1701,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);
 
@@ -1763,7 +1773,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));
@@ -1783,7 +1793,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));
@@ -1796,11 +1806,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;
 	}
@@ -1812,8 +1827,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;
 	}
@@ -1831,8 +1845,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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20181026/20c0c79f/signature-0001.sig>


More information about the samba-technical mailing list