[Patches] for dbcheck (Re: [Patches] AD Database corruption after upgrade from <= 4.6 to 4.7 (bug #13228))

Stefan Metzmacher metze at samba.org
Wed Jan 31 16:05:50 UTC 2018


Am 31.01.2018 um 01:18 schrieb Andrew Bartlett via samba-technical:
> On Wed, 2018-01-31 at 00:43 +0100, Stefan Metzmacher wrote:
>>> Also, should we restrict the test to run when the DB doesn't have
>>> sortedLinks set (ie upgraded) so we avoid the expensive search and
>>> possible re-introduction of links that are both deactivated and
>>> expunged?
>>
>> As the duplicates could also happen as consequence of
>> https://bugzilla.samba.org/show_bug.cgi?id=13095
>> I think we need to keep them.
>> Maybe we can skip find_missing_forward_links_from_backlink,
>> but it can be a patch on top I guess.
> 
> This is the part that was concerning me.  I would be more comfortable
> in this mode, as we have seen extra backlinks (or missing forward
> links, it was never entirely clear) in production of 4.6 installs and
> am nervous about how widely --yes is used.
> 
> I also suspect (based on reports from customers on 4.6) that this might
> not be the end of the story, but don't have anything concrete yet.  

Here's an updated patchset, please let me know if you're happy to see
this in master?

Thanks!
metze

-------------- next part --------------
From b32cd469b28643519d39eae35e1a0d722f839e76 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 30 Jan 2018 10:40:36 +0100
Subject: [PATCH 01/23] selftest: run "samba.tests.common"

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
 selftest/tests.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/selftest/tests.py b/selftest/tests.py
index a94fc8c..9d40bbb 100644
--- a/selftest/tests.py
+++ b/selftest/tests.py
@@ -64,6 +64,7 @@ planpythontestsuite("none", "samba.tests.dcerpc.integer")
 planpythontestsuite("none", "samba.tests.param", py3_compatible=True)
 planpythontestsuite("none", "samba.tests.upgrade")
 planpythontestsuite("none", "samba.tests.core", py3_compatible=True)
+planpythontestsuite("none", "samba.tests.common")
 planpythontestsuite("none", "samba.tests.provision")
 planpythontestsuite("none", "samba.tests.samba3")
 planpythontestsuite("none", "samba.tests.strings")
-- 
1.9.1


From af3b9f0b9b8f7bbde4ad3a4836827558b27cf423 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 30 Jan 2018 10:39:30 +0100
Subject: [PATCH 02/23] python:tests: use TestCaseInTempDir for
 "samba.tests.common"

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
 python/samba/tests/common.py | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/python/samba/tests/common.py b/python/samba/tests/common.py
index 8794e9d..33c7270 100644
--- a/python/samba/tests/common.py
+++ b/python/samba/tests/common.py
@@ -23,7 +23,7 @@ from samba.common import *
 from samba.samdb import SamDB
 
 
-class CommonTests(samba.tests.TestCase):
+class CommonTests(samba.tests.TestCaseInTempDir):
 
     def test_normalise_int32(self):
         self.assertEquals('17', normalise_int32(17))
@@ -32,9 +32,10 @@ class CommonTests(samba.tests.TestCase):
         self.assertEquals('-1294967296', normalise_int32('3000000000'))
 
     def test_dsdb_Dn(self):
-        sam = samba.Ldb(url='dntest.ldb')
+        url = self.tempdir + "/test_dsdb_Dn.ldb"
+        sam = samba.Ldb(url=url)
         dn1 = dsdb_Dn(sam, "DC=foo,DC=bar")
         dn2 = dsdb_Dn(sam, "B:8:0000000D:<GUID=b3f0ec29-17f4-452a-b002-963e1909d101>;DC=samba,DC=example,DC=com")
         self.assertEquals(dn2.binary, "0000000D")
         self.assertEquals(13, dn2.get_binary_integer())
-        os.unlink('dntest.ldb')
+        os.unlink(url)
-- 
1.9.1


From 6ad11a2a952d3a39999048e460208ced97771f38 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 30 Jan 2018 11:09:40 +0100
Subject: [PATCH 03/23] python:tests: remove test_dsdb_Dn() to
 test_dsdb_Dn_binary()

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
 python/samba/tests/common.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/python/samba/tests/common.py b/python/samba/tests/common.py
index 33c7270..3c35225 100644
--- a/python/samba/tests/common.py
+++ b/python/samba/tests/common.py
@@ -31,8 +31,8 @@ class CommonTests(samba.tests.TestCaseInTempDir):
         self.assertEquals('-123', normalise_int32('-123'))
         self.assertEquals('-1294967296', normalise_int32('3000000000'))
 
-    def test_dsdb_Dn(self):
-        url = self.tempdir + "/test_dsdb_Dn.ldb"
+    def test_dsdb_Dn_binary(self):
+        url = self.tempdir + "/test_dsdb_Dn_binary.ldb"
         sam = samba.Ldb(url=url)
         dn1 = dsdb_Dn(sam, "DC=foo,DC=bar")
         dn2 = dsdb_Dn(sam, "B:8:0000000D:<GUID=b3f0ec29-17f4-452a-b002-963e1909d101>;DC=samba,DC=example,DC=com")
-- 
1.9.1


From 8e41d771971f1efc17240e4afd1c9123c786ba55 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 30 Jan 2018 11:09:55 +0100
Subject: [PATCH 04/23] python:tests: add test_dsdb_Dn_sorted() to
 "samba.tests.common"

Failing until dsdb_Dn implements the correct __cmp__() function.

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
 python/samba/tests/common.py             | 24 ++++++++++++++++++++++++
 selftest/knownfail.d/test_dsdb_Dn_sorted |  1 +
 2 files changed, 25 insertions(+)
 create mode 100644 selftest/knownfail.d/test_dsdb_Dn_sorted

diff --git a/python/samba/tests/common.py b/python/samba/tests/common.py
index 3c35225..49ae2b0 100644
--- a/python/samba/tests/common.py
+++ b/python/samba/tests/common.py
@@ -39,3 +39,27 @@ class CommonTests(samba.tests.TestCaseInTempDir):
         self.assertEquals(dn2.binary, "0000000D")
         self.assertEquals(13, dn2.get_binary_integer())
         os.unlink(url)
+
+    def test_dsdb_Dn_sorted(self):
+        url = self.tempdir + "/test_dsdb_Dn_sorted.ldb"
+        sam = samba.Ldb(url=url)
+        try:
+            dn1 = dsdb_Dn(sam, "B:8:0000000D:<GUID=b3f0ec29-17f4-452a-b002-963e1909d101>;OU=dn1,DC=samba,DC=example,DC=com")
+            dn2 = dsdb_Dn(sam, "B:8:0000000C:<GUID=b3f0ec29-17f4-452a-b002-963e1909d101>;OU=dn1,DC=samba,DC=example,DC=com")
+            dn3 = dsdb_Dn(sam, "B:8:0000000F:<GUID=00000000-17f4-452a-b002-963e1909d101>;OU=dn3,DC=samba,DC=example,DC=com")
+            dn4 = dsdb_Dn(sam, "B:8:00000000:<GUID=ffffffff-17f4-452a-b002-963e1909d101>;OU=dn4,DC=samba,DC=example,DC=com")
+            dn5 = dsdb_Dn(sam, "<GUID=ffffffff-27f4-452a-b002-963e1909d101>;OU=dn5,DC=samba,DC=example,DC=com")
+            dn6 = dsdb_Dn(sam, "<GUID=00000000-27f4-452a-b002-963e1909d101>;OU=dn6,DC=samba,DC=example,DC=com")
+            unsorted_links14 = [dn1,dn2,dn3,dn4]
+            sorted_vals14 = [str(dn) for dn in sorted(unsorted_links14)]
+            self.assertEquals(sorted_vals14[0], str(dn3))
+            self.assertEquals(sorted_vals14[1], str(dn2))
+            self.assertEquals(sorted_vals14[2], str(dn1))
+            self.assertEquals(sorted_vals14[3], str(dn4))
+            unsorted_links56 = [dn5,dn6]
+            sorted_vals56 = [str(dn) for dn in sorted(unsorted_links56)]
+            self.assertEquals(sorted_vals56[0], str(dn6))
+            self.assertEquals(sorted_vals56[1], str(dn5))
+        finally:
+            del sam
+            os.unlink(url)
diff --git a/selftest/knownfail.d/test_dsdb_Dn_sorted b/selftest/knownfail.d/test_dsdb_Dn_sorted
new file mode 100644
index 0000000..2377dcc
--- /dev/null
+++ b/selftest/knownfail.d/test_dsdb_Dn_sorted
@@ -0,0 +1 @@
+^samba.tests.common.samba.tests.common.CommonTests.test_dsdb_Dn_sorted
-- 
1.9.1


From 5bffcf3068644adea95fbc05ac137c9992df0ba4 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 30 Jan 2018 09:51:20 +0100
Subject: [PATCH 05/23] python/common: add __cmp__ function to dsdb_Dn similar
 to parsed_dn_compare()

Linked attribute values are sorted by objectGUID of the link target.
For C code we have parsed_dn_compare() to implement the logic,
the same is now available on python dsdb_Dn objects.

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
 python/samba/common.py                   | 17 +++++++++++++++++
 selftest/knownfail.d/test_dsdb_Dn_sorted |  1 -
 2 files changed, 17 insertions(+), 1 deletion(-)
 delete mode 100644 selftest/knownfail.d/test_dsdb_Dn_sorted

diff --git a/python/samba/common.py b/python/samba/common.py
index 20f170c..a915934 100644
--- a/python/samba/common.py
+++ b/python/samba/common.py
@@ -19,6 +19,8 @@
 
 import ldb
 import dsdb
+from samba.ndr import ndr_pack
+from samba.dcerpc import misc
 import binascii
 
 
@@ -93,6 +95,21 @@ class dsdb_Dn(object):
     def __str__(self):
         return self.prefix + str(self.dn.extended_str(mode=1))
 
+    def __cmp__(self, other):
+        ''' compare dsdb_Dn values similar to parsed_dn_compare()'''
+        dn1 = self
+        dn2 = other
+        guid1 = dn1.dn.get_extended_component("GUID")
+        guid1b = ndr_pack(misc.GUID(guid1))
+        guid2 = dn2.dn.get_extended_component("GUID")
+        guid2b = ndr_pack(misc.GUID(guid2))
+
+        v = cmp(guid1, guid2)
+        if v != 0:
+            return v
+        v = cmp(dn1.binary, dn2.binary)
+        return v
+
     def get_binary_integer(self):
         '''return binary part of a dsdb_Dn as an integer, or None'''
         if self.prefix == '':
diff --git a/selftest/knownfail.d/test_dsdb_Dn_sorted b/selftest/knownfail.d/test_dsdb_Dn_sorted
deleted file mode 100644
index 2377dcc..0000000
--- a/selftest/knownfail.d/test_dsdb_Dn_sorted
+++ /dev/null
@@ -1 +0,0 @@
-^samba.tests.common.samba.tests.common.CommonTests.test_dsdb_Dn_sorted
-- 
1.9.1


From d62cf25b72a1dccd483a2c38cd9b92e6b82e2ff4 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 24 Jan 2018 11:34:43 +0100
Subject: [PATCH 06/23] Revert "dbcheck: disable fixing duplicate linked
 attributes until we can recover lost forward links"

This reverts commit 43e3f79d54c5aeaea820865d298d4249cf47af99.

The real fix will follow in the next commits.

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

Signed-off-by: Ralph Boehme <slow at samba.org>
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 python/samba/dbchecker.py                     | 22 +++-------------------
 selftest/knownfail.d/dbcheck_duplicate_member |  5 -----
 2 files changed, 3 insertions(+), 24 deletions(-)
 delete mode 100644 selftest/knownfail.d/dbcheck_duplicate_member

diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
index 6e4c440..1933740 100644
--- a/python/samba/dbchecker.py
+++ b/python/samba/dbchecker.py
@@ -708,15 +708,9 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
                           "Failed to fix incorrect RMD_FLAGS %u" % rmd_flags):
             self.report("Fixed incorrect RMD_FLAGS %u" % (rmd_flags))
 
-    def err_orphaned_backlink(self, obj, attrname, val, link_name, target_dn, duplicate_links):
+    def err_orphaned_backlink(self, obj, attrname, val, link_name, target_dn):
         '''handle a orphaned backlink value'''
         self.report("ERROR: orphaned backlink attribute '%s' in %s for link %s in %s" % (attrname, obj.dn, link_name, target_dn))
-        if duplicate_links:
-            self.report("ERROR: FATAL! Most likely the corresponding forward link got lost!")
-            self.report("ERROR: FATAL! See https://bugzilla.samba.org/show_bug.cgi?id=13228")
-            self.report("Recovery handling will be implemented in a future version")
-            self.report("Not removing orphaned backlink %s" % attrname)
-            return
         if not self.confirm_all('Remove orphaned backlink %s' % attrname, 'fix_all_orphaned_backlinks'):
             self.report("Not removing orphaned backlink %s" % attrname)
             return
@@ -730,11 +724,6 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
     def err_duplicate_links(self, obj, attrname, vals):
         '''handle a duplicate links value'''
 
-        self.report("ERROR: FATAL! Most likely some forward link values for attribute '%s' in '%s' got lost!" % (attrname, obj.dn))
-        self.report("ERROR: FATAL! See https://bugzilla.samba.org/show_bug.cgi?id=13228")
-        self.report("Recovery handling will be implemented in a future version")
-        self.report("Not removing duplicate links in attribute '%s'" % attrname)
-        return
         if not self.confirm_all("Remove duplicate links in attribute '%s'" % attrname, 'fix_all_duplicate_links'):
             self.report("Not removing duplicate links in attribute '%s'" % attrname)
             return
@@ -907,7 +896,6 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
         else:
             reverse_syntax_oid = None
 
-        duplicate_links = False
         duplicate_dict = dict()
         duplicate_list = list()
         unique_dict = dict()
@@ -962,10 +950,6 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
             unique_dict[keystr] = dsdb_dn
 
         if len(duplicate_list) != 0:
-            duplicate_links = True
-            self.report("ERROR: FATAL! Most likely some forward link values for attribute '%s' in '%s' got lost!" % (attrname, obj.dn))
-            self.report("ERROR: FATAL! See https://bugzilla.samba.org/show_bug.cgi?id=13228")
-
             self.report("ERROR: Duplicate link values for attribute '%s' in '%s'" % (attrname, obj.dn))
             for keystr in duplicate_list:
                 d = duplicate_dict[keystr]
@@ -1164,7 +1148,7 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
                     error_count += 1
                     self.err_orphaned_backlink(obj, attrname,
                                                val, reverse_link_name,
-                                               dsdb_dn.dn, duplicate_links)
+                                               dsdb_dn.dn)
                     continue
                 # Only warn here and let the forward link logic fix it.
                 self.report("WARNING: Link (back) mismatch for '%s' (%d) on '%s' to '%s' (%d) on '%s'" % (
@@ -1196,7 +1180,7 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
                 else:
                     self.err_orphaned_backlink(res[0], reverse_link_name,
                                                obj.dn.extended_str(), attrname,
-                                               obj.dn, duplicate_links)
+                                               obj.dn)
                     diff_count += 1
 
 
diff --git a/selftest/knownfail.d/dbcheck_duplicate_member b/selftest/knownfail.d/dbcheck_duplicate_member
deleted file mode 100644
index 7ebb82b..0000000
--- a/selftest/knownfail.d/dbcheck_duplicate_member
+++ /dev/null
@@ -1,5 +0,0 @@
-^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.dbcheck_duplicate_member
-^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.check_expected_after_duplicate_links
-^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.duplicate_clean
-^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.dbcheck_clean2
-^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.dbcheck_clean3
-- 
1.9.1


From afff30d684838d15c0886940bbeee9075872b31f Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 25 Jan 2018 21:34:47 +0100
Subject: [PATCH 07/23] selftest/dbcheck: add a test for corrupt forward links
 restoration

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

Signed-off-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
 selftest/knownfail.d/samba4.blackbox.dbcheck-links |  2 +
 ...cted-after-dbcheck-forward-link-corruption.ldif | 24 +++++++
 ...dbcheck-link-output-forward-link-corruption.txt | 12 ++++
 testprogs/blackbox/dbcheck-links.sh                | 78 ++++++++++++++++++++++
 4 files changed, 116 insertions(+)
 create mode 100644 selftest/knownfail.d/samba4.blackbox.dbcheck-links
 create mode 100644 source4/selftest/provisions/release-4-5-0-pre1/expected-after-dbcheck-forward-link-corruption.ldif
 create mode 100644 source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output-forward-link-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 0000000..299f8b1
--- /dev/null
+++ b/selftest/knownfail.d/samba4.blackbox.dbcheck-links
@@ -0,0 +1,2 @@
+samba4.blackbox.dbcheck-links.release-4-5-0-pre1.dbcheck_forward_link_corruption\(none\)
+samba4.blackbox.dbcheck-links.release-4-5-0-pre1.check_expected_after_dbcheck_forward_link_corruption\(none\)
diff --git a/source4/selftest/provisions/release-4-5-0-pre1/expected-after-dbcheck-forward-link-corruption.ldif b/source4/selftest/provisions/release-4-5-0-pre1/expected-after-dbcheck-forward-link-corruption.ldif
new file mode 100644
index 0000000..0258bce
--- /dev/null
+++ b/source4/selftest/provisions/release-4-5-0-pre1/expected-after-dbcheck-forward-link-corruption.ldif
@@ -0,0 +1,24 @@
+# record 1
+dn: CN=dangling,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp
+memberOf: CN=Enterprise Admins,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp
+
+# record 2
+dn: CN=Enterprise Admins,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp
+member: CN=Administrator,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp
+member: CN=dangling,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp
+memberOf: CN=Administrators,CN=Builtin,DC=release-4-5-0-pre1,DC=samba,DC=corp
+memberOf: CN=Denied RODC Password Replication Group,CN=Users,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-forward-link-corruption.txt b/source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output-forward-link-corruption.txt
new file mode 100644
index 0000000..14ebd6b
--- /dev/null
+++ b/source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output-forward-link-corruption.txt
@@ -0,0 +1,12 @@
+Checking 226 objects
+WARNING: Link (back) mismatch for 'memberOf' (1) on 'CN=Administrator,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp' to 'member' (2) on 'CN=Enterprise Admins,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp'
+WARNING: Keep orphaned backlink attribute 'memberOf' in 'CN=dangling,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp' for link 'member' in 'CN=Enterprise Admins,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp'
+ERROR: Missing and duplicate forward link values for attribute 'member' in 'CN=Enterprise Admins,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp'
+Missing   link '<GUID=fd8a04ac-cea0-4921-b1a6-c173e1155c22>;<RMD_ADDTIME=131116484540000000>;<RMD_CHANGETIME=131116484540000000>;<RMD_FLAGS=0>;<RMD_INVOCID=ffffffff-4700-4700-4700-000000b13228>;<RMD_LOCAL_USN=3552>;<RMD_ORIGINATING_USN=1>;<RMD_VERSION=0>;<SID=S-1-5-21-4177067393-1453636373-93818738-1121>;CN=dangling,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp'
+Schedule readding missing forward link for attribute member [YES]
+Duplicate link '<GUID=f4616422-30ec-473b-9d6f-a9a2d7bd1e6a>;<RMD_ADDTIME=131116484540000000>;<RMD_CHANGETIME=131116484540000000>;<RMD_FLAGS=0>;<RMD_INVOCID=4e4496a3-7fb8-4f97-8a33-d238db8b5e2d>;<RMD_LOCAL_USN=0>;<RMD_ORIGINATING_USN=3552>;<RMD_VERSION=0>;<SID=S-1-5-21-4177067393-1453636373-93818738-500>;CN=Administrator,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp'
+Correct   link '<GUID=f4616422-30ec-473b-9d6f-a9a2d7bd1e6a>;<RMD_ADDTIME=131116484540000000>;<RMD_CHANGETIME=131116484540000000>;<RMD_FLAGS=0>;<RMD_INVOCID=4e4496a3-7fb8-4f97-8a33-d238db8b5e2d>;<RMD_LOCAL_USN=3552>;<RMD_ORIGINATING_USN=3552>;<RMD_VERSION=0>;<SID=S-1-5-21-4177067393-1453636373-93818738-500>;CN=Administrator,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp'
+RECHECK: 'Missing/Duplicate/Correct link' lines above for attribute 'member' in 'CN=Enterprise Admins,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp'
+Commit fixes for (missing/duplicate) forward links in attribute 'member' [YES]
+Fixed duplicate links in attribute 'member'
+Checked 226 objects (3 errors)
diff --git a/testprogs/blackbox/dbcheck-links.sh b/testprogs/blackbox/dbcheck-links.sh
index 0aeada0..778edf0 100755
--- a/testprogs/blackbox/dbcheck-links.sh
+++ b/testprogs/blackbox/dbcheck-links.sh
@@ -131,6 +131,80 @@ check_expected_after_duplicate_links() {
     fi
 }
 
+forward_link_corruption() {
+    #
+    # Step1: add a duplicate forward link from
+    # "CN=Enterprise Admins" to "CN=Administrator"
+    #
+    LDIF1=$(TZ=UTC $ldbsearch -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb -b 'CN=Enterprise Admins,CN=users,DC=release-4-5-0-pre1,DC=samba,DC=corp' -s base --reveal --extended-dn member)
+    DN=$(echo "${LDIF1}" | grep '^dn: ')
+    MSG=$(echo "${LDIF1}" | grep -v '^dn: ' | grep -v '^#' | grep -v '^$')
+    ldif=$PREFIX_ABS/${RELEASE}/forward_link_corruption1.ldif
+    {
+	echo "${DN}"
+	echo "changetype: modify"
+	echo "replace: member"
+	echo "${MSG}"
+	echo "${MSG}" | sed -e 's!RMD_LOCAL_USN=[1-9][0-9]*!RMD_LOCAL_USN=0!'
+    } > $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
+
+    #
+    # Step2: add user "dangling"
+    #
+    ldif=$PREFIX_ABS/${RELEASE}/forward_link_corruption2.ldif
+    cat > $ldif <<EOF
+dn: CN=dangling,CN=users,DC=release-4-5-0-pre1,DC=samba,DC=corp
+changetype: add
+objectclass: user
+samaccountname: dangling
+objectGUID: fd8a04ac-cea0-4921-b1a6-c173e1155c22
+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 a dangling backlink from
+    # "CN=dangling" to "CN=Enterprise Admins"
+    #
+    ldif=$PREFIX_ABS/${RELEASE}/forward_link_corruption3.ldif
+    {
+	echo "dn: CN=dangling,CN=users,DC=release-4-5-0-pre1,DC=samba,DC=corp"
+	echo "changetype: modify"
+	echo "add: memberOf"
+	echo "memberOf: <GUID=304ad703-468b-465e-9787-470b3dfd7d75>;<SID=S-1-5-21-4177067393-1453636373-93818738-519>;CN=Enterprise Admins,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp"
+    } > $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
+}
+
+dbcheck_forward_link_corruption() {
+    dbcheck "-forward-link-corruption" "1" ""
+    return $?
+}
+
+check_expected_after_dbcheck_forward_link_corruption() {
+    tmpldif=$PREFIX_ABS/$RELEASE/expected-after-dbcheck-forward-link-corruption.ldif.tmp
+    TZ=UTC $ldbsearch -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb '(|(cn=dangling)(cn=enterprise admins))' -s sub -b DC=release-4-5-0-pre1,DC=samba,DC=corp --show-deleted --sorted memberOf member > $tmpldif
+    diff $tmpldif $release_dir/expected-after-dbcheck-forward-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
@@ -198,6 +272,10 @@ if [ -d $release_dir ]; then
     testit "dbcheck_duplicate_member" dbcheck_duplicate_member
     testit "check_expected_after_duplicate_links" check_expected_after_duplicate_links
     testit "duplicate_clean" dbcheck_clean
+    testit "forward_link_corruption" forward_link_corruption
+    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 "dangling_one_way_link" dangling_one_way_link
     testit "dbcheck_one_way" dbcheck_one_way
     testit "dbcheck_clean2" dbcheck_clean
-- 
1.9.1


From 93b36f54d5791656dd716b16ff6cb6166be154f2 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 24 Jan 2018 19:31:23 +0100
Subject: [PATCH 08/23] dbcheck: rename and reorder err_orphaned_backlink
 arguments

In preperation of adding more arguments.

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

Pair-Programmed-With: Stefan Metzmacher <metze at samba.org>

Signed-off-by: Ralph Boehme <slow at samba.org>
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 python/samba/dbchecker.py | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
index 1933740..b56125d 100644
--- a/python/samba/dbchecker.py
+++ b/python/samba/dbchecker.py
@@ -708,18 +708,18 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
                           "Failed to fix incorrect RMD_FLAGS %u" % rmd_flags):
             self.report("Fixed incorrect RMD_FLAGS %u" % (rmd_flags))
 
-    def err_orphaned_backlink(self, obj, attrname, val, link_name, target_dn):
+    def err_orphaned_backlink(self, obj, backlink_attr, backlink_val, target_dn, forward_attr):
         '''handle a orphaned backlink value'''
-        self.report("ERROR: orphaned backlink attribute '%s' in %s for link %s in %s" % (attrname, obj.dn, link_name, target_dn))
-        if not self.confirm_all('Remove orphaned backlink %s' % attrname, 'fix_all_orphaned_backlinks'):
-            self.report("Not removing orphaned backlink %s" % attrname)
+        self.report("ERROR: orphaned backlink attribute '%s' in %s for link %s in %s" % (backlink_attr, obj.dn, forward_attr, target_dn))
+        if not self.confirm_all('Remove orphaned backlink %s' % backlink_attr, 'fix_all_orphaned_backlinks'):
+            self.report("Not removing orphaned backlink %s" % backlink_attr)
             return
         m = ldb.Message()
         m.dn = obj.dn
-        m['value'] = ldb.MessageElement(val, ldb.FLAG_MOD_DELETE, attrname)
+        m['value'] = ldb.MessageElement(backlink_val, ldb.FLAG_MOD_DELETE, backlink_attr)
         if self.do_modify(m, ["show_recycled:1", "relax:0"],
-                          "Failed to fix orphaned backlink %s" % attrname):
-            self.report("Fixed orphaned backlink %s" % (attrname))
+                          "Failed to fix orphaned backlink %s" % backlink_attr):
+            self.report("Fixed orphaned backlink %s" % (backlink_attr))
 
     def err_duplicate_links(self, obj, attrname, vals):
         '''handle a duplicate links value'''
@@ -1147,8 +1147,8 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
                 if match_count == 0:
                     error_count += 1
                     self.err_orphaned_backlink(obj, attrname,
-                                               val, reverse_link_name,
-                                               dsdb_dn.dn)
+                                               val, dsdb_dn.dn,
+                                               reverse_link_name)
                     continue
                 # Only warn here and let the forward link logic fix it.
                 self.report("WARNING: Link (back) mismatch for '%s' (%d) on '%s' to '%s' (%d) on '%s'" % (
@@ -1179,8 +1179,8 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
                     diff_count -= 1
                 else:
                     self.err_orphaned_backlink(res[0], reverse_link_name,
-                                               obj.dn.extended_str(), attrname,
-                                               obj.dn)
+                                               obj.dn.extended_str(), obj.dn,
+                                               attrname)
                     diff_count += 1
 
 
-- 
1.9.1


From 83229fb58588ac834b854e339a2f32d281ba70a4 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 25 Jan 2018 10:52:35 +0100
Subject: [PATCH 09/23] dbcheck: add forward_syntax argument to
 err_orphaned_backlink

Will be used in a subsequent commit.

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

Pair-Programmed-With: Stefan Metzmacher <metze at samba.org>

Signed-off-by: Ralph Boehme <slow at samba.org>
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 python/samba/dbchecker.py | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
index b56125d..8a9ee43 100644
--- a/python/samba/dbchecker.py
+++ b/python/samba/dbchecker.py
@@ -708,7 +708,7 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
                           "Failed to fix incorrect RMD_FLAGS %u" % rmd_flags):
             self.report("Fixed incorrect RMD_FLAGS %u" % (rmd_flags))
 
-    def err_orphaned_backlink(self, obj, backlink_attr, backlink_val, target_dn, forward_attr):
+    def err_orphaned_backlink(self, obj, backlink_attr, backlink_val, target_dn, forward_attr, forward_syntax):
         '''handle a orphaned backlink value'''
         self.report("ERROR: orphaned backlink attribute '%s' in %s for link %s in %s" % (backlink_attr, obj.dn, forward_attr, target_dn))
         if not self.confirm_all('Remove orphaned backlink %s' % backlink_attr, 'fix_all_orphaned_backlinks'):
@@ -1148,7 +1148,8 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
                     error_count += 1
                     self.err_orphaned_backlink(obj, attrname,
                                                val, dsdb_dn.dn,
-                                               reverse_link_name)
+                                               reverse_link_name,
+                                               reverse_syntax_oid)
                     continue
                 # Only warn here and let the forward link logic fix it.
                 self.report("WARNING: Link (back) mismatch for '%s' (%d) on '%s' to '%s' (%d) on '%s'" % (
@@ -1180,7 +1181,7 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
                 else:
                     self.err_orphaned_backlink(res[0], reverse_link_name,
                                                obj.dn.extended_str(), obj.dn,
-                                               attrname)
+                                               attrname, syntax_oid)
                     diff_count += 1
 
 
-- 
1.9.1


From 672f42497e4c2991fc2e9d780c381da04718ecaf Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 29 Jan 2018 22:48:42 +0100
Subject: [PATCH 10/23] dbcheck: only pass obj_dn to err_orphaned_backlink()

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
 python/samba/dbchecker.py | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
index 8a9ee43..b622ac2 100644
--- a/python/samba/dbchecker.py
+++ b/python/samba/dbchecker.py
@@ -708,14 +708,15 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
                           "Failed to fix incorrect RMD_FLAGS %u" % rmd_flags):
             self.report("Fixed incorrect RMD_FLAGS %u" % (rmd_flags))
 
-    def err_orphaned_backlink(self, obj, backlink_attr, backlink_val, target_dn, forward_attr, forward_syntax):
+    def err_orphaned_backlink(self, obj_dn, backlink_attr, backlink_val,
+                              target_dn, forward_attr, forward_syntax):
         '''handle a orphaned backlink value'''
-        self.report("ERROR: orphaned backlink attribute '%s' in %s for link %s in %s" % (backlink_attr, obj.dn, forward_attr, target_dn))
+        self.report("ERROR: orphaned backlink attribute '%s' in %s for link %s in %s" % (backlink_attr, obj_dn, forward_attr, target_dn))
         if not self.confirm_all('Remove orphaned backlink %s' % backlink_attr, 'fix_all_orphaned_backlinks'):
             self.report("Not removing orphaned backlink %s" % backlink_attr)
             return
         m = ldb.Message()
-        m.dn = obj.dn
+        m.dn = obj_dn
         m['value'] = ldb.MessageElement(backlink_val, ldb.FLAG_MOD_DELETE, backlink_attr)
         if self.do_modify(m, ["show_recycled:1", "relax:0"],
                           "Failed to fix orphaned backlink %s" % backlink_attr):
@@ -1146,7 +1147,7 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
                 # UNLESS, there is no forward link detected.
                 if match_count == 0:
                     error_count += 1
-                    self.err_orphaned_backlink(obj, attrname,
+                    self.err_orphaned_backlink(obj.dn, attrname,
                                                val, dsdb_dn.dn,
                                                reverse_link_name,
                                                reverse_syntax_oid)
@@ -1179,7 +1180,7 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
                                               dsdb_dn.dn)
                     diff_count -= 1
                 else:
-                    self.err_orphaned_backlink(res[0], reverse_link_name,
+                    self.err_orphaned_backlink(res[0].dn, reverse_link_name,
                                                obj.dn.extended_str(), obj.dn,
                                                attrname, syntax_oid)
                     diff_count += 1
-- 
1.9.1


From 4bc88b1352cb6eb7e8201767a86b528d60415a55 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 24 Jan 2018 19:37:55 +0100
Subject: [PATCH 11/23] dbcheck: rename err_duplicate_links arguments

In preperation of adding more arguments.

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

Pair-Programmed-With: Stefan Metzmacher <metze at samba.org>

Signed-off-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
 python/samba/dbchecker.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
index b622ac2..a73f2b1 100644
--- a/python/samba/dbchecker.py
+++ b/python/samba/dbchecker.py
@@ -722,18 +722,18 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
                           "Failed to fix orphaned backlink %s" % backlink_attr):
             self.report("Fixed orphaned backlink %s" % (backlink_attr))
 
-    def err_duplicate_links(self, obj, attrname, vals):
+    def err_duplicate_links(self, obj, forward_attr, forward_vals):
         '''handle a duplicate links value'''
 
-        if not self.confirm_all("Remove duplicate links in attribute '%s'" % attrname, 'fix_all_duplicate_links'):
-            self.report("Not removing duplicate links in attribute '%s'" % attrname)
+        if not self.confirm_all("Remove duplicate links in attribute '%s'" % forward_attr, 'fix_all_duplicate_links'):
+            self.report("Not removing duplicate links in attribute '%s'" % forward_attr)
             return
         m = ldb.Message()
         m.dn = obj.dn
-        m['value'] = ldb.MessageElement(vals, ldb.FLAG_MOD_REPLACE, attrname)
+        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"],
-                "Failed to fix duplicate links in attribute '%s'" % attrname):
-            self.report("Fixed duplicate links in attribute '%s'" % (attrname))
+                "Failed to fix duplicate links in attribute '%s'" % forward_attr):
+            self.report("Fixed duplicate links in attribute '%s'" % (forward_attr))
 
     def err_no_fsmoRoleOwner(self, obj):
         '''handle a missing fSMORoleOwner'''
-- 
1.9.1


From 9679c1f4c41590655846c98d5640ab631616abac Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 25 Jan 2018 14:41:58 +0100
Subject: [PATCH 12/23] dbcheck: add link direction to error message for
 duplicate links

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

Pair-Programmed-With: Stefan Metzmacher <metze at samba.org>

Signed-off-by: Ralph Boehme <slow at samba.org>
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 python/samba/dbchecker.py                                              | 3 ++-
 .../expected-dbcheck-link-output_duplicate_member.txt                  | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
index a73f2b1..1ab3eb0 100644
--- a/python/samba/dbchecker.py
+++ b/python/samba/dbchecker.py
@@ -951,7 +951,8 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
             unique_dict[keystr] = dsdb_dn
 
         if len(duplicate_list) != 0:
-            self.report("ERROR: Duplicate link values for attribute '%s' in '%s'" % (attrname, obj.dn))
+            self.report("ERROR: Duplicate forward link values for attribute '%s' in '%s'" % (attrname, obj.dn))
+
             for keystr in duplicate_list:
                 d = duplicate_dict[keystr]
                 for dd in d["delete"]:
diff --git a/source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output_duplicate_member.txt b/source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output_duplicate_member.txt
index baa11ca..cacddd2 100644
--- a/source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output_duplicate_member.txt
+++ b/source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output_duplicate_member.txt
@@ -1,6 +1,6 @@
 Checking 225 objects
 WARNING: Link (back) mismatch for 'memberOf' (1) on 'CN=Administrator,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp' to 'member' (2) on 'CN=Enterprise Admins,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp'
-ERROR: Duplicate link values for attribute 'member' in 'CN=Enterprise Admins,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp'
+ERROR: Duplicate forward link values for attribute 'member' in 'CN=Enterprise Admins,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp'
 Duplicate link '<GUID=f4616422-30ec-473b-9d6f-a9a2d7bd1e6a>;<RMD_ADDTIME=131116484540000000>;<RMD_CHANGETIME=131116484540000000>;<RMD_FLAGS=0>;<RMD_INVOCID=4e4496a3-7fb8-4f97-8a33-d238db8b5e2d>;<RMD_LOCAL_USN=0>;<RMD_ORIGINATING_USN=3552>;<RMD_VERSION=0>;<SID=S-1-5-21-4177067393-1453636373-93818738-500>;CN=Administrator,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp'
 Correct   link '<GUID=f4616422-30ec-473b-9d6f-a9a2d7bd1e6a>;<RMD_ADDTIME=131116484540000000>;<RMD_CHANGETIME=131116484540000000>;<RMD_FLAGS=0>;<RMD_INVOCID=4e4496a3-7fb8-4f97-8a33-d238db8b5e2d>;<RMD_LOCAL_USN=3552>;<RMD_ORIGINATING_USN=3552>;<RMD_VERSION=0>;<SID=S-1-5-21-4177067393-1453636373-93818738-500>;CN=Administrator,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp'
 Remove duplicate links in attribute 'member' [YES]
-- 
1.9.1


From 0b3fc89e1f72473b37904bde745ecb56d813e1ab Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 25 Jan 2018 14:36:52 +0100
Subject: [PATCH 13/23] dbcheck: rename err_duplicate_links() to
 err_recover_forward_links() and adjust the output message

It's really a fatal error to have duplicate values as it's very likely that
some forward links got lost.

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

Pair-Programmed-With: Stefan Metzmacher <metze at samba.org>

Signed-off-by: Ralph Boehme <slow at samba.org>
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 python/samba/dbchecker.py                                   | 13 ++++++++-----
 .../expected-dbcheck-link-output_duplicate_member.txt       |  3 ++-
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
index 1ab3eb0..308e4bf 100644
--- a/python/samba/dbchecker.py
+++ b/python/samba/dbchecker.py
@@ -65,7 +65,7 @@ class dbcheck(object):
         self.fix_undead_linked_attributes = False
         self.fix_all_missing_backlinks = False
         self.fix_all_orphaned_backlinks = False
-        self.fix_all_duplicate_links = False
+        self.recover_all_forward_links = False
         self.fix_rmd_flags = False
         self.fix_ntsecuritydescriptor = False
         self.fix_ntsecuritydescriptor_owner_group = False
@@ -722,11 +722,14 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
                           "Failed to fix orphaned backlink %s" % backlink_attr):
             self.report("Fixed orphaned backlink %s" % (backlink_attr))
 
-    def err_duplicate_links(self, obj, forward_attr, forward_vals):
+    def err_recover_forward_links(self, obj, forward_attr, forward_vals):
         '''handle a duplicate links value'''
 
-        if not self.confirm_all("Remove duplicate links in attribute '%s'" % forward_attr, 'fix_all_duplicate_links'):
-            self.report("Not removing duplicate links in attribute '%s'" % forward_attr)
+        self.report("RECHECK: 'Duplicate/Correct link' lines above for attribute '%s' in '%s'" % (forward_attr, obj.dn))
+
+        if not self.confirm_all("Commit fixes for (duplicate) forward links in attribute '%s'" % forward_attr, 'recover_all_forward_links'):
+            self.report("Not fixing corrupted (duplicate) forward links in attribute '%s' of '%s'" % (
+                        forward_attr, obj.dn))
             return
         m = ldb.Message()
         m.dn = obj.dn
@@ -963,7 +966,7 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
             for keystr in unique_list:
                 dsdb_dn = unique_dict[keystr]
                 vals.append(str(dsdb_dn))
-            self.err_duplicate_links(obj, attrname, vals)
+            self.err_recover_forward_links(obj, attrname, vals)
             # We should continue with the fixed values
             obj[attrname] = ldb.MessageElement(vals, ldb.FLAG_MOD_REPLACE, attrname)
 
diff --git a/source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output_duplicate_member.txt b/source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output_duplicate_member.txt
index cacddd2..61990e6 100644
--- a/source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output_duplicate_member.txt
+++ b/source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output_duplicate_member.txt
@@ -3,6 +3,7 @@ WARNING: Link (back) mismatch for 'memberOf' (1) on 'CN=Administrator,CN=Users,D
 ERROR: Duplicate forward link values for attribute 'member' in 'CN=Enterprise Admins,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp'
 Duplicate link '<GUID=f4616422-30ec-473b-9d6f-a9a2d7bd1e6a>;<RMD_ADDTIME=131116484540000000>;<RMD_CHANGETIME=131116484540000000>;<RMD_FLAGS=0>;<RMD_INVOCID=4e4496a3-7fb8-4f97-8a33-d238db8b5e2d>;<RMD_LOCAL_USN=0>;<RMD_ORIGINATING_USN=3552>;<RMD_VERSION=0>;<SID=S-1-5-21-4177067393-1453636373-93818738-500>;CN=Administrator,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp'
 Correct   link '<GUID=f4616422-30ec-473b-9d6f-a9a2d7bd1e6a>;<RMD_ADDTIME=131116484540000000>;<RMD_CHANGETIME=131116484540000000>;<RMD_FLAGS=0>;<RMD_INVOCID=4e4496a3-7fb8-4f97-8a33-d238db8b5e2d>;<RMD_LOCAL_USN=3552>;<RMD_ORIGINATING_USN=3552>;<RMD_VERSION=0>;<SID=S-1-5-21-4177067393-1453636373-93818738-500>;CN=Administrator,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp'
-Remove duplicate links in attribute 'member' [YES]
+RECHECK: 'Duplicate/Correct link' lines above for attribute 'member' in 'CN=Enterprise Admins,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp'
+Commit fixes for (duplicate) forward links in attribute 'member' [YES]
 Fixed duplicate links in attribute 'member'
 Checked 225 objects (1 errors)
-- 
1.9.1


From fab58faca7a0d5b63c2ad4e1772d8d48a0c1e99a Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 30 Jan 2018 09:39:40 +0100
Subject: [PATCH 14/23] dbcheck: remove ldb.FLAG_MOD_REPLACE when replacing
 search results for forward links

Search results don't have an ldb.FLAG_MOD_* flags set.

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
 python/samba/dbchecker.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
index 308e4bf..cccc498 100644
--- a/python/samba/dbchecker.py
+++ b/python/samba/dbchecker.py
@@ -968,7 +968,7 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
                 vals.append(str(dsdb_dn))
             self.err_recover_forward_links(obj, attrname, vals)
             # We should continue with the fixed values
-            obj[attrname] = ldb.MessageElement(vals, ldb.FLAG_MOD_REPLACE, attrname)
+            obj[attrname] = ldb.MessageElement(vals, 0, attrname)
 
         for val in obj[attrname]:
             dsdb_dn = dsdb_Dn(self.samdb, val, syntax_oid)
-- 
1.9.1


From 397a5ae35cb3d6e59cd470773750cfc1ebf32c92 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 30 Jan 2018 09:55:21 +0100
Subject: [PATCH 15/23] dbcheck: store fixed forward link attributes with the
 correct sorting

The corruption we're trying to fix messed up the sorting,
so there's no point in keeping the current order.

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
 python/samba/dbchecker.py | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
index cccc498..722184f 100644
--- a/python/samba/dbchecker.py
+++ b/python/samba/dbchecker.py
@@ -901,9 +901,7 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
             reverse_syntax_oid = None
 
         duplicate_dict = dict()
-        duplicate_list = list()
         unique_dict = dict()
-        unique_list = list()
         for val in obj[attrname]:
             if linkID & 1:
                 #
@@ -921,14 +919,12 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
             keystr = guidstr + dsdb_dn.prefix
             if keystr not in unique_dict:
                 unique_dict[keystr] = dsdb_dn
-                unique_list.append(keystr)
                 continue
             error_count += 1
             if keystr not in duplicate_dict:
                 duplicate_dict[keystr] = dict()
                 duplicate_dict[keystr]["keep"] = None
                 duplicate_dict[keystr]["delete"] = list()
-                duplicate_list.append(keystr)
 
             # Now check for the highest RMD_VERSION
             v1 = int(unique_dict[keystr].dn.get_extended_component("RMD_VERSION"))
@@ -953,19 +949,18 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
             duplicate_dict[keystr]["delete"].append(unique_dict[keystr])
             unique_dict[keystr] = dsdb_dn
 
-        if len(duplicate_list) != 0:
+        if len(duplicate_dict) != 0:
             self.report("ERROR: Duplicate forward link values for attribute '%s' in '%s'" % (attrname, obj.dn))
-
-            for keystr in duplicate_list:
+            for keystr in duplicate_dict.keys():
                 d = duplicate_dict[keystr]
                 for dd in d["delete"]:
                     self.report("Duplicate link '%s'" % dd)
                 self.report("Correct   link '%s'" % d["keep"])
 
-            vals = []
-            for keystr in unique_list:
-                dsdb_dn = unique_dict[keystr]
-                vals.append(str(dsdb_dn))
+            # We now construct the sorted dn values.
+            # They're sorted by the objectGUID of the target
+            # See dsdb_Dn.__cmp__()
+            vals = [str(dn) for dn in sorted(unique_dict.values())]
             self.err_recover_forward_links(obj, attrname, vals)
             # We should continue with the fixed values
             obj[attrname] = ldb.MessageElement(vals, 0, attrname)
-- 
1.9.1


From e9ebaefc5b88e5f038373ba27dc7e282f090f37d Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 24 Jan 2018 20:01:27 +0100
Subject: [PATCH 16/23] dbcheck: split out check_duplicate_links from check_dn

Refactoring, no change in behaviour.

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

Pair-Programmed-With: Stefan Metzmacher <metze at samba.org>

Signed-off-by: Ralph Boehme <slow at samba.org>
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 python/samba/dbchecker.py | 45 +++++++++++++++++++++++++++++----------------
 1 file changed, 29 insertions(+), 16 deletions(-)

diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
index 722184f..9185a1d 100644
--- a/python/samba/dbchecker.py
+++ b/python/samba/dbchecker.py
@@ -889,27 +889,23 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
                 return dsdb_dn
         return None
 
-    def check_dn(self, obj, attrname, syntax_oid):
-        '''check a DN attribute for correctness'''
+    def check_duplicate_links(self, obj, forward_attr, forward_syntax, forward_linkID, backlink_attr):
+        '''check a linked values for duplicate forward links'''
         error_count = 0
-        obj_guid = obj['objectGUID'][0]
-
-        linkID, reverse_link_name = self.get_attr_linkID_and_reverse_name(attrname)
-        if reverse_link_name is not None:
-            reverse_syntax_oid = self.samdb_schema.get_syntax_oid_from_lDAPDisplayName(reverse_link_name)
-        else:
-            reverse_syntax_oid = None
 
         duplicate_dict = dict()
         unique_dict = dict()
-        for val in obj[attrname]:
-            if linkID & 1:
-                #
-                # Only cleanup forward links here,
-                # back links are handled below.
-                break
 
-            dsdb_dn = dsdb_Dn(self.samdb, val, syntax_oid)
+        # Only forward links can have this problem
+        if forward_linkID & 1:
+            # If we got the reverse, skip it
+            return (error_count, duplicate_dict, unique_dict)
+
+        if backlink_attr is None:
+            return (error_count, duplicate_dict, unique_dict)
+
+        for val in obj[forward_attr]:
+            dsdb_dn = dsdb_Dn(self.samdb, val, forward_syntax)
 
             # all DNs should have a GUID component
             guid = dsdb_dn.dn.get_extended_component("GUID")
@@ -949,6 +945,23 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
             duplicate_dict[keystr]["delete"].append(unique_dict[keystr])
             unique_dict[keystr] = dsdb_dn
 
+
+        return (error_count, duplicate_dict, unique_dict)
+
+    def check_dn(self, obj, attrname, syntax_oid):
+        '''check a DN attribute for correctness'''
+        error_count = 0
+        obj_guid = obj['objectGUID'][0]
+
+        linkID, reverse_link_name = self.get_attr_linkID_and_reverse_name(attrname)
+        if reverse_link_name is not None:
+            reverse_syntax_oid = self.samdb_schema.get_syntax_oid_from_lDAPDisplayName(reverse_link_name)
+        else:
+            reverse_syntax_oid = None
+
+        error_count, duplicate_dict, unique_dict = \
+            self.check_duplicate_links(obj, attrname, syntax_oid, linkID, reverse_link_name)
+
         if len(duplicate_dict) != 0:
             self.report("ERROR: Duplicate forward link values for attribute '%s' in '%s'" % (attrname, obj.dn))
             for keystr in duplicate_dict.keys():
-- 
1.9.1


From 6395c67c6ead6886167f38f63250e96a027ac140 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 25 Jan 2018 10:34:29 +0100
Subject: [PATCH 17/23] dbcheck: add a dict where we remember attributes with
 duplicate links

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

Pair-Programmed-With: Stefan Metzmacher <metze at samba.org>

Signed-off-by: Ralph Boehme <slow at samba.org>
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 python/samba/dbchecker.py | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
index 9185a1d..787cea2 100644
--- a/python/samba/dbchecker.py
+++ b/python/samba/dbchecker.py
@@ -65,6 +65,7 @@ class dbcheck(object):
         self.fix_undead_linked_attributes = False
         self.fix_all_missing_backlinks = False
         self.fix_all_orphaned_backlinks = False
+        self.duplicate_link_cache = dict()
         self.recover_all_forward_links = False
         self.fix_rmd_flags = False
         self.fix_ntsecuritydescriptor = False
@@ -904,6 +905,10 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
         if backlink_attr is None:
             return (error_count, duplicate_dict, unique_dict)
 
+        duplicate_cache_key = "%s:%s" % (str(obj.dn), forward_attr)
+        if duplicate_cache_key not in self.duplicate_link_cache:
+            self.duplicate_link_cache[duplicate_cache_key] = False
+
         for val in obj[forward_attr]:
             dsdb_dn = dsdb_Dn(self.samdb, val, forward_syntax)
 
@@ -945,6 +950,8 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
             duplicate_dict[keystr]["delete"].append(unique_dict[keystr])
             unique_dict[keystr] = dsdb_dn
 
+        if error_count != 0:
+            self.duplicate_link_cache[duplicate_cache_key] = True
 
         return (error_count, duplicate_dict, unique_dict)
 
-- 
1.9.1


From 8b9a095d3244bbeea1335474d18aa1f91f7a8167 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 24 Jan 2018 22:24:15 +0100
Subject: [PATCH 18/23] dbcheck: add a helper function that checks is a value
 has duplicate links

Will be used in a subsequent commit.

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

Pair-Programmed-With: Stefan Metzmacher <metze at samba.org>

Signed-off-by: Ralph Boehme <slow at samba.org>
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 python/samba/dbchecker.py | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
index 787cea2..d4c653a 100644
--- a/python/samba/dbchecker.py
+++ b/python/samba/dbchecker.py
@@ -955,6 +955,38 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
 
         return (error_count, duplicate_dict, unique_dict)
 
+    def has_duplicate_links(self, dn, forward_attr, forward_syntax):
+        '''check a linked values for duplicate forward links'''
+        error_count = 0
+
+        duplicate_cache_key = "%s:%s" % (str(dn), forward_attr)
+        if duplicate_cache_key in self.duplicate_link_cache:
+            return self.duplicate_link_cache[duplicate_cache_key]
+
+        forward_linkID, backlink_attr = self.get_attr_linkID_and_reverse_name(forward_attr)
+
+        attrs = [forward_attr]
+        controls = ["extended_dn:1:1", "reveal_internals:0"]
+
+        # check its the right GUID
+        try:
+            res = self.samdb.search(base=str(dn), scope=ldb.SCOPE_BASE,
+                                    attrs=attrs, controls=controls)
+        except ldb.LdbError, (enum, estr):
+            if enum != ldb.ERR_NO_SUCH_OBJECT:
+                raise
+
+            return False
+
+        obj = res[0]
+        error_count, duplicate_dict, unique_dict = \
+            self.check_duplicate_links(obj, forward_attr, forward_syntax, forward_linkID, backlink_attr)
+
+        if duplicate_cache_key in self.duplicate_link_cache:
+            return self.duplicate_link_cache[duplicate_cache_key]
+
+        return False
+
     def check_dn(self, obj, attrname, syntax_oid):
         '''check a DN attribute for correctness'''
         error_count = 0
-- 
1.9.1


From c9212500e9511f4934eda063beff58c9770e2cb1 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 30 Jan 2018 12:19:31 +0100
Subject: [PATCH 19/23] dbcheck: make sure we always ask for the objectGUID
 attribute explicitly

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
 python/samba/dbchecker.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
index d4c653a..5ae57e2 100644
--- a/python/samba/dbchecker.py
+++ b/python/samba/dbchecker.py
@@ -1813,8 +1813,7 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
             attrs.append("systemFlags")
         if '*' in attrs:
             attrs.append("replPropertyMetaData")
-        else:
-            attrs.append("objectGUID")
+        attrs.append("objectGUID")
 
         try:
             sd_flags = 0
-- 
1.9.1


From a33d8e91b8d6ec2eb5f341bf2b4e4c1c99b0e055 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 30 Jan 2018 12:19:31 +0100
Subject: [PATCH 20/23] dbcheck: make sure we ask for replPropertyMetaData if
 we need to process any forward link attributes

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
 python/samba/dbchecker.py | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
index 5ae57e2..5b9c551 100644
--- a/python/samba/dbchecker.py
+++ b/python/samba/dbchecker.py
@@ -1811,7 +1811,19 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
             attrs.append(dn.get_rdn_name())
             attrs.append("isDeleted")
             attrs.append("systemFlags")
+        need_replPropertyMetaData = False
         if '*' in attrs:
+            need_replPropertyMetaData = True
+        else:
+            for a in attrs:
+                linkID, _ = self.get_attr_linkID_and_reverse_name(a)
+                if linkID == 0:
+                    continue
+                if linkID & 1:
+                    continue
+                need_replPropertyMetaData = True
+                break
+        if need_replPropertyMetaData:
             attrs.append("replPropertyMetaData")
         attrs.append("objectGUID")
 
-- 
1.9.1


From 5d50ce8d514fe1e298e1159bdc3188a14421b10b Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 25 Jan 2018 14:48:55 +0100
Subject: [PATCH 21/23] dbcheck: add
 find_missing_forward_links_from_backlinks()

find_missing_forward_links_from_backlinks() finds and returns missing forward-links by
searching all for all objects that link to the object in the backlink attribute.

This will be used in the next commit to restore forward links in a corrupted
forward link attribute by passing the missing backling objects to
err_recover_forward_links().

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

Pair-Programmed-With: Stefan Metzmacher <metze at samba.org>

Signed-off-by: Ralph Boehme <slow at samba.org>
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 python/samba/dbchecker.py | 96 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 96 insertions(+)

diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
index 5b9c551..8d8a193 100644
--- a/python/samba/dbchecker.py
+++ b/python/samba/dbchecker.py
@@ -987,6 +987,102 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
 
         return False
 
+    def find_missing_forward_links_from_backlinks(self, obj,
+                                                  forward_attr,
+                                                  forward_syntax,
+                                                  backlink_attr,
+                                                  forward_unique_dict):
+        '''Find all backlinks linking to obj_guid_str not already in forward_unique_dict'''
+        missing_forward_links = []
+        error_count = 0
+
+        if backlink_attr is None:
+            return (missing_forward_links, error_count)
+
+        if forward_syntax != ldb.SYNTAX_DN:
+            self.report("Not checking for missing forward links for syntax: %s",
+                        forward_syntax)
+            return (missing_forward_links, error_count)
+
+        try:
+            obj_guid = obj['objectGUID'][0]
+            obj_guid_str = str(ndr_unpack(misc.GUID, obj_guid))
+            filter = "(%s=<GUID=%s>)" % (backlink_attr, obj_guid_str)
+
+            res = self.samdb.search(expression=filter,
+                                    scope=ldb.SCOPE_SUBTREE, attrs=["objectGUID"],
+                                    controls=["extended_dn:1:1",
+                                              "search_options:1:2",
+                                              "paged_results:1:1000"])
+        except ldb.LdbError, (enum, estr):
+            raise
+
+        for r in res:
+            target_dn = dsdb_Dn(self.samdb, r.dn.extended_str(), forward_syntax)
+
+            guid = target_dn.dn.get_extended_component("GUID")
+            guidstr = str(misc.GUID(guid))
+            if guidstr in forward_unique_dict:
+                continue
+
+            # A valid forward link looks like this:
+            #
+            #    <GUID=9f92d30a-fc23-11e4-a5f6-30be15454808>;
+            #    <RMD_ADDTIME=131607546230000000>;
+            #    <RMD_CHANGETIME=131607546230000000>;
+            #    <RMD_FLAGS=0>;
+            #    <RMD_INVOCID=4e4496a3-7fb8-4f97-8a33-d238db8b5e2d>;
+            #    <RMD_LOCAL_USN=3765>;
+            #    <RMD_ORIGINATING_USN=3765>;
+            #    <RMD_VERSION=1>;
+            #    <SID=S-1-5-21-4177067393-1453636373-93818738-1124>;
+            #    CN=unsorted-u8,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp
+            #
+            # Note that versions older than Samba 4.8 create
+            # links with RMD_VERSION=0.
+            #
+            # Try to get the local_usn and time from objectClass
+            # if possible and fallback to any other one.
+            repl = ndr_unpack(drsblobs.replPropertyMetaDataBlob,
+                              obj['replPropertyMetadata'][0])
+            for o in repl.ctr.array:
+                local_usn = o.local_usn
+                t = o.originating_change_time
+                if o.attid == drsuapi.DRSUAPI_ATTID_objectClass:
+                    break
+
+            # We use a magic invocationID for restoring missing
+            # forward links to recover from bug #13228.
+            # This should allow some more future magic to fix the
+            # problem.
+            #
+            # It also means it looses the conflict resolution
+            # against almost every real invocation, if the
+            # version is also 0.
+            originating_invocid = misc.GUID("ffffffff-4700-4700-4700-000000b13228")
+            originating_usn = 1
+
+            rmd_addtime = t
+            rmd_changetime = t
+            rmd_flags = 0
+            rmd_invocid = originating_invocid
+            rmd_originating_usn = originating_usn
+            rmd_local_usn = local_usn
+            rmd_version = 0
+
+            target_dn.dn.set_extended_component("RMD_ADDTIME", str(rmd_addtime))
+            target_dn.dn.set_extended_component("RMD_CHANGETIME", str(rmd_changetime))
+            target_dn.dn.set_extended_component("RMD_FLAGS", str(rmd_flags))
+            target_dn.dn.set_extended_component("RMD_INVOCID", ndr_pack(rmd_invocid))
+            target_dn.dn.set_extended_component("RMD_ORIGINATING_USN", str(rmd_originating_usn))
+            target_dn.dn.set_extended_component("RMD_LOCAL_USN", str(rmd_local_usn))
+            target_dn.dn.set_extended_component("RMD_VERSION", str(rmd_version))
+
+            error_count += 1
+            missing_forward_links.append(target_dn)
+
+        return (missing_forward_links, error_count)
+
     def check_dn(self, obj, attrname, syntax_oid):
         '''check a DN attribute for correctness'''
         error_count = 0
-- 
1.9.1


From 459020904430df612db191fec8de71f44739c5f6 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 25 Jan 2018 14:48:55 +0100
Subject: [PATCH 22/23] dbcheck: add support for restoring missing forward
 links

This recovers broken databases with duplicate and missing
forward links.

See commit a25c99c9f1fd1814c56c21848c748cd0e038eed7 for
the fix that prevents to problem from happening.

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

Pair-Programmed-With: Stefan Metzmacher <metze at samba.org>

Signed-off-by: Ralph Boehme <slow at samba.org>
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 python/samba/dbchecker.py                          | 43 +++++++++++++++++++---
 selftest/knownfail.d/samba4.blackbox.dbcheck-links |  2 -
 ...pected-dbcheck-link-output_duplicate_member.txt |  4 +-
 3 files changed, 39 insertions(+), 10 deletions(-)
 delete mode 100644 selftest/knownfail.d/samba4.blackbox.dbcheck-links

diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
index 8d8a193..c2c95a2 100644
--- a/python/samba/dbchecker.py
+++ b/python/samba/dbchecker.py
@@ -65,6 +65,7 @@ class dbcheck(object):
         self.fix_undead_linked_attributes = False
         self.fix_all_missing_backlinks = False
         self.fix_all_orphaned_backlinks = False
+        self.fix_all_missing_forward_links = False
         self.duplicate_link_cache = dict()
         self.recover_all_forward_links = False
         self.fix_rmd_flags = False
@@ -710,8 +711,14 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
             self.report("Fixed incorrect RMD_FLAGS %u" % (rmd_flags))
 
     def err_orphaned_backlink(self, obj_dn, backlink_attr, backlink_val,
-                              target_dn, forward_attr, forward_syntax):
+                              target_dn, forward_attr, forward_syntax,
+                              check_duplicates=True):
         '''handle a orphaned backlink value'''
+        if check_duplicates is True and self.has_duplicate_links(target_dn, forward_attr, forward_syntax):
+            self.report("WARNING: Keep orphaned backlink attribute " + \
+                        "'%s' in '%s' for link '%s' in '%s'" % (
+                        backlink_attr, obj_dn, forward_attr, target_dn))
+            return
         self.report("ERROR: orphaned backlink attribute '%s' in %s for link %s in %s" % (backlink_attr, obj_dn, forward_attr, target_dn))
         if not self.confirm_all('Remove orphaned backlink %s' % backlink_attr, 'fix_all_orphaned_backlinks'):
             self.report("Not removing orphaned backlink %s" % backlink_attr)
@@ -726,10 +733,10 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
     def err_recover_forward_links(self, obj, forward_attr, forward_vals):
         '''handle a duplicate links value'''
 
-        self.report("RECHECK: 'Duplicate/Correct link' lines above for attribute '%s' in '%s'" % (forward_attr, obj.dn))
+        self.report("RECHECK: 'Missing/Duplicate/Correct link' lines above for attribute '%s' in '%s'" % (forward_attr, obj.dn))
 
-        if not self.confirm_all("Commit fixes for (duplicate) forward links in attribute '%s'" % forward_attr, 'recover_all_forward_links'):
-            self.report("Not fixing corrupted (duplicate) forward links in attribute '%s' of '%s'" % (
+        if not self.confirm_all("Commit fixes for (missing/duplicate) forward links in attribute '%s'" % forward_attr, 'recover_all_forward_links'):
+            self.report("Not fixing corrupted (missing/duplicate) forward links in attribute '%s' of '%s'" % (
                         forward_attr, obj.dn))
             return
         m = ldb.Message()
@@ -1098,7 +1105,31 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
             self.check_duplicate_links(obj, attrname, syntax_oid, linkID, reverse_link_name)
 
         if len(duplicate_dict) != 0:
-            self.report("ERROR: Duplicate forward link values for attribute '%s' in '%s'" % (attrname, obj.dn))
+
+            missing_forward_links, missing_error_count = \
+                self.find_missing_forward_links_from_backlinks(obj,
+                                                         attrname, syntax_oid,
+                                                         reverse_link_name,
+                                                         unique_dict)
+            error_count += missing_error_count
+
+            forward_links = [dn for dn in unique_dict.values()]
+
+            if missing_error_count != 0:
+                self.report("ERROR: Missing and duplicate forward link values for attribute '%s' in '%s'" % (
+                            attrname, obj.dn))
+            else:
+                self.report("ERROR: Duplicate forward link values for attribute '%s' in '%s'" % (attrname, obj.dn))
+            for m in missing_forward_links:
+                self.report("Missing   link '%s'" % (m))
+                if not self.confirm_all("Schedule readding missing forward link for attribute %s" % attrname,
+                                        'fix_all_missing_forward_links'):
+                    self.err_orphaned_backlink(m.dn, reverse_link_name,
+                                               obj.dn.extended_str(), obj.dn,
+                                               attrname, syntax_oid,
+                                               check_duplicates=False)
+                    continue
+                forward_links += [m]
             for keystr in duplicate_dict.keys():
                 d = duplicate_dict[keystr]
                 for dd in d["delete"]:
@@ -1108,7 +1139,7 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
             # We now construct the sorted dn values.
             # They're sorted by the objectGUID of the target
             # See dsdb_Dn.__cmp__()
-            vals = [str(dn) for dn in sorted(unique_dict.values())]
+            vals = [str(dn) for dn in sorted(forward_links)]
             self.err_recover_forward_links(obj, attrname, vals)
             # We should continue with the fixed values
             obj[attrname] = ldb.MessageElement(vals, 0, attrname)
diff --git a/selftest/knownfail.d/samba4.blackbox.dbcheck-links b/selftest/knownfail.d/samba4.blackbox.dbcheck-links
deleted file mode 100644
index 299f8b1..0000000
--- a/selftest/knownfail.d/samba4.blackbox.dbcheck-links
+++ /dev/null
@@ -1,2 +0,0 @@
-samba4.blackbox.dbcheck-links.release-4-5-0-pre1.dbcheck_forward_link_corruption\(none\)
-samba4.blackbox.dbcheck-links.release-4-5-0-pre1.check_expected_after_dbcheck_forward_link_corruption\(none\)
diff --git a/source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output_duplicate_member.txt b/source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output_duplicate_member.txt
index 61990e6..af788ef 100644
--- a/source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output_duplicate_member.txt
+++ b/source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output_duplicate_member.txt
@@ -3,7 +3,7 @@ WARNING: Link (back) mismatch for 'memberOf' (1) on 'CN=Administrator,CN=Users,D
 ERROR: Duplicate forward link values for attribute 'member' in 'CN=Enterprise Admins,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp'
 Duplicate link '<GUID=f4616422-30ec-473b-9d6f-a9a2d7bd1e6a>;<RMD_ADDTIME=131116484540000000>;<RMD_CHANGETIME=131116484540000000>;<RMD_FLAGS=0>;<RMD_INVOCID=4e4496a3-7fb8-4f97-8a33-d238db8b5e2d>;<RMD_LOCAL_USN=0>;<RMD_ORIGINATING_USN=3552>;<RMD_VERSION=0>;<SID=S-1-5-21-4177067393-1453636373-93818738-500>;CN=Administrator,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp'
 Correct   link '<GUID=f4616422-30ec-473b-9d6f-a9a2d7bd1e6a>;<RMD_ADDTIME=131116484540000000>;<RMD_CHANGETIME=131116484540000000>;<RMD_FLAGS=0>;<RMD_INVOCID=4e4496a3-7fb8-4f97-8a33-d238db8b5e2d>;<RMD_LOCAL_USN=3552>;<RMD_ORIGINATING_USN=3552>;<RMD_VERSION=0>;<SID=S-1-5-21-4177067393-1453636373-93818738-500>;CN=Administrator,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp'
-RECHECK: 'Duplicate/Correct link' lines above for attribute 'member' in 'CN=Enterprise Admins,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp'
-Commit fixes for (duplicate) forward links in attribute 'member' [YES]
+RECHECK: 'Missing/Duplicate/Correct link' lines above for attribute 'member' in 'CN=Enterprise Admins,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp'
+Commit fixes for (missing/duplicate) forward links in attribute 'member' [YES]
 Fixed duplicate links in attribute 'member'
 Checked 225 objects (1 errors)
-- 
1.9.1


From bb3e3d7ef82cdbd6a9b8858618b22121cc3accc8 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 31 Jan 2018 09:50:47 +0100
Subject: [PATCH 23/23] dbcheck: skip
 find_missing_forward_links_from_backlinks() if the db has the sortedLinks
 feature

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
 python/samba/dbchecker.py | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
index c2c95a2..d01c6f5 100644
--- a/python/samba/dbchecker.py
+++ b/python/samba/dbchecker.py
@@ -186,6 +186,23 @@ class dbcheck(object):
         else:
             self.rid_set_dn = None
 
+        self.compatibleFeatures = []
+        self.requiredFeatures = []
+
+        try:
+            res = self.samdb.search(scope=ldb.SCOPE_BASE,
+                                    base="@SAMBA_DSDB",
+                                    attrs=["compatibleFeatures",
+                                    "requiredFeatures"])
+            if "compatibleFeatures" in res[0]:
+                self.compatibleFeatures = res[0]["compatibleFeatures"]
+            if "requiredFeatures" in res[0]:
+                self.requiredFeatures = res[0]["requiredFeatures"]
+        except ldb.LdbError as (enum, estr):
+            if enum != ldb.ERR_NO_SUCH_OBJECT:
+                raise
+            pass
+
     def check_database(self, DN=None, scope=ldb.SCOPE_SUBTREE, controls=[], attrs=['*']):
         '''perform a database check, returning the number of errors found'''
         res = self.samdb.search(base=DN, scope=scope, attrs=['dn'], controls=controls)
@@ -1011,6 +1028,11 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
                         forward_syntax)
             return (missing_forward_links, error_count)
 
+        if "sortedLinks" in self.compatibleFeatures:
+            self.report("Not checking for missing forward links because the db " + \
+                        "has the sortedLinks feature")
+            return (missing_forward_links, error_count)
+
         try:
             obj_guid = obj['objectGUID'][0]
             obj_guid_str = str(ndr_unpack(misc.GUID, obj_guid))
-- 
1.9.1

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20180131/30395f9f/signature.sig>


More information about the samba-technical mailing list