Web lists-archives.com

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




Hi,

as a lot of SerNet customers are having trouble with corrupted
linked attributes, my colleague Ralph Böhme and I developed
patches for 'samba-tool dbcheck' to recover the missing
forward links (in most cases missing member attributes).

I'm currently running a private autobuild with these patches
and my colleague Björn Baumbach is currently testing SAMBA+
packages with the patches included, which will be released
as soon as possible.

As the patches re-add members to groups administrators may want
avoid using '--yes' and ack the re-added members explicitly.

The patches have enough review tags already, additional
review isn't required, we'll wait a bit to collect some feedback
from others, before pushing.

Once the patches are reviewed for master, we'll also release
a new upstream 4.7 release with the fixes included.

More technical details:

As we lost the replication meta data for the forward link,
we create them using a special invocationId
ffffffff-4700-4700-4700-000000b13228 and an originating_usn
of 1. The add/changetime/local_usn are the one from the last
'objectClass' modification (which typically never changes and therefor
matches the object creation time). We also use version = 0
in order to match the link creation of 4.7 and older releases.

This way we can easily identify recreated forward links
and we avoid a new meta data stamp and incrementing of
the highestCommitedUSN. So each affected dc will just recover
the value in the local database. And any incoming
replication should overwrite the value again.

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

metze

Am 22.01.2018 um 10:49 schrieb Stefan Metzmacher via samba-technical:
> Hi,
> 
> here're patches to avoid a database corruption with linked attributes,
> e.g. member/memberOf.
> 
> See https://bugzilla.samba.org/show_bug.cgi?id=13228
> 
> As a temporary solution admins can add "server services = -kcc" to the
> global section of smb.conf.
> 
> Also DO NOT repair the following errors with samba-tool dbcheck!
> "Remove duplicate links in attribute"
> and
> "ERROR: orphaned backlink"
> as this removes the ability to repair the database
> in the next round of patches!
> 
> Please review and push:-)
> 
> Thanks!
> metze
> 


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

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

Signed-off-by: Stefan Metzmacher <metze@xxxxxxxxx>
Reviewed-by: Ralph Boehme <slow@xxxxxxxxx>
---
 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@xxxxxxxxx>
Date: Tue, 30 Jan 2018 10:39:30 +0100
Subject: [PATCH 02/22] python:tests: use TestCaseInTempDir for
 "samba.tests.common"

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

Signed-off-by: Stefan Metzmacher <metze@xxxxxxxxx>
Reviewed-by: Ralph Boehme <slow@xxxxxxxxx>
---
 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@xxxxxxxxx>
Date: Tue, 30 Jan 2018 11:09:40 +0100
Subject: [PATCH 03/22] 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@xxxxxxxxx>
Reviewed-by: Ralph Boehme <slow@xxxxxxxxx>
---
 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@xxxxxxxxx>
Date: Tue, 30 Jan 2018 11:09:55 +0100
Subject: [PATCH 04/22] 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@xxxxxxxxx>
Reviewed-by: Ralph Boehme <slow@xxxxxxxxx>
---
 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@xxxxxxxxx>
Date: Tue, 30 Jan 2018 09:51:20 +0100
Subject: [PATCH 05/22] 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@xxxxxxxxx>
Reviewed-by: Ralph Boehme <slow@xxxxxxxxx>
---
 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@xxxxxxxxx>
Date: Wed, 24 Jan 2018 11:34:43 +0100
Subject: [PATCH 06/22] 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@xxxxxxxxx>
Signed-off-by: Stefan Metzmacher <metze@xxxxxxxxx>
---
 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 2da9659ba0ce9ac2480197babc27635ed4e0a048 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@xxxxxxxxx>
Date: Thu, 25 Jan 2018 21:34:47 +0100
Subject: [PATCH 07/22] 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@xxxxxxxxx>
Reviewed-by: Stefan Metzmacher <metze@xxxxxxxxx>
---
 selftest/knownfail.d/samba4.blackbox.dbcheck-links |  2 +
 ...cted-after-dbcheck-forward-link-corruption.ldif | 24 +++++++
 ...dbcheck-link-output-forward-link-corruption.txt | 13 ++++
 testprogs/blackbox/dbcheck-links.sh                | 78 ++++++++++++++++++++++
 4 files changed, 117 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..eadd9019
--- /dev/null
+++ b/source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output-forward-link-corruption.txt
@@ -0,0 +1,13 @@
+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 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'
+Readd missing forward link for attribute member [YES]
+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'
+ERROR: corrupted forward links in attribute 'member' in 'CN=Enterprise Admins,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp'
+Recover 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 70d8515c133d305f14326e8de6b97c5365467308 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@xxxxxxxxx>
Date: Wed, 24 Jan 2018 19:31:23 +0100
Subject: [PATCH 08/22] 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@xxxxxxxxx>

Signed-off-by: Ralph Boehme <slow@xxxxxxxxx>
Signed-off-by: Stefan Metzmacher <metze@xxxxxxxxx>
---
 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 591e32b84585eef32441458375d958a78960800c Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@xxxxxxxxx>
Date: Thu, 25 Jan 2018 10:52:35 +0100
Subject: [PATCH 09/22] 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@xxxxxxxxx>

Signed-off-by: Ralph Boehme <slow@xxxxxxxxx>
Signed-off-by: Stefan Metzmacher <metze@xxxxxxxxx>
---
 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 c2e7b20026c5ca165fbd0b3e2c40d5435118b01f Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@xxxxxxxxx>
Date: Mon, 29 Jan 2018 22:48:42 +0100
Subject: [PATCH 10/22] 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@xxxxxxxxx>
Reviewed-by: Ralph Boehme <slow@xxxxxxxxx>
---
 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 ad2af9ba228447751ab8584dcdfd27d5b0182b84 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@xxxxxxxxx>
Date: Wed, 24 Jan 2018 19:37:55 +0100
Subject: [PATCH 11/22] 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@xxxxxxxxx>

Signed-off-by: Ralph Boehme <slow@xxxxxxxxx>
Reviewed-by: Stefan Metzmacher <metze@xxxxxxxxx>
---
 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 dcc5a1d226b42840424571f67610aa12c6737e24 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@xxxxxxxxx>
Date: Thu, 25 Jan 2018 14:41:58 +0100
Subject: [PATCH 12/22] 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@xxxxxxxxx>

Signed-off-by: Ralph Boehme <slow@xxxxxxxxx>
Signed-off-by: Stefan Metzmacher <metze@xxxxxxxxx>
---
 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 f4a427f071f0c027f0930c4d74cc89d591cbe9d2 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@xxxxxxxxx>
Date: Thu, 25 Jan 2018 14:36:52 +0100
Subject: [PATCH 13/22] 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@xxxxxxxxx>

Signed-off-by: Ralph Boehme <slow@xxxxxxxxx>
Signed-off-by: Stefan Metzmacher <metze@xxxxxxxxx>
---
 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..8a44991 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("ERROR: corrupted forward links in attribute '%s' in '%s'" % (forward_attr, obj.dn))
+
+        if not self.confirm_all("Recover forward links in attribute '%s'" % forward_attr, 'recover_all_forward_links'):
+            self.report("Not fixing corrupted 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..14b887d 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]
+ERROR: corrupted forward links in attribute 'member' in 'CN=Enterprise Admins,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp'
+Recover forward links in attribute 'member' [YES]
 Fixed duplicate links in attribute 'member'
 Checked 225 objects (1 errors)
-- 
1.9.1


From ef7e6b25b7e8f8278a2cc59c0f81877bfb5e396d Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@xxxxxxxxx>
Date: Tue, 30 Jan 2018 09:39:40 +0100
Subject: [PATCH 14/22] 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@xxxxxxxxx>
Reviewed-by: Ralph Boehme <slow@xxxxxxxxx>
---
 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 8a44991..e1ff320 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 3e2f103bb41a23345966c83e22e823c91c82793e Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@xxxxxxxxx>
Date: Tue, 30 Jan 2018 09:55:21 +0100
Subject: [PATCH 15/22] 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@xxxxxxxxx>
Reviewed-by: Ralph Boehme <slow@xxxxxxxxx>
---
 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 e1ff320..b0b28c1 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 34e8c77a5b77bcaef51982b2bfbfddc80d70b293 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@xxxxxxxxx>
Date: Wed, 24 Jan 2018 20:01:27 +0100
Subject: [PATCH 16/22] 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@xxxxxxxxx>

Signed-off-by: Ralph Boehme <slow@xxxxxxxxx>
Signed-off-by: Stefan Metzmacher <metze@xxxxxxxxx>
---
 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 b0b28c1..c5ece51 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 5882507b3141b6bc96ef5acf535fc8516d5e7402 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@xxxxxxxxx>
Date: Thu, 25 Jan 2018 10:34:29 +0100
Subject: [PATCH 17/22] 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@xxxxxxxxx>

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

diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
index c5ece51..e2b55c0 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 4c7f40a86dee20ab3debab1b106bfe1eed2279ba Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@xxxxxxxxx>
Date: Wed, 24 Jan 2018 22:24:15 +0100
Subject: [PATCH 18/22] 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@xxxxxxxxx>

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

diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
index e2b55c0..10ef41f 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 edfb6349bace9e32de9076f50e63beb1db29f0eb Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@xxxxxxxxx>
Date: Tue, 30 Jan 2018 12:19:31 +0100
Subject: [PATCH 19/22] 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@xxxxxxxxx>
Reviewed-by: Ralph Boehme <slow@xxxxxxxxx>
---
 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 10ef41f..7addc16 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 cd0b2650838c36627e9b0a6370abd65f0d4c3efb Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@xxxxxxxxx>
Date: Tue, 30 Jan 2018 12:19:31 +0100
Subject: [PATCH 20/22] 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@xxxxxxxxx>
Reviewed-by: Ralph Boehme <slow@xxxxxxxxx>
---
 python/samba/dbchecker.py | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
index 7addc16..bbd8282 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 8be06f2565dfcad1a424d65bf46dda96441ed6a4 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@xxxxxxxxx>
Date: Thu, 25 Jan 2018 14:48:55 +0100
Subject: [PATCH 21/22] 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@xxxxxxxxx>

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

diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
index bbd8282..b0c4d45 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 f061c01a3860b31d3fd8d15164a00e644b2cf7bd Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@xxxxxxxxx>
Date: Thu, 25 Jan 2018 14:48:55 +0100
Subject: [PATCH 22/22] 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@xxxxxxxxx>

Signed-off-by: Ralph Boehme <slow@xxxxxxxxx>
Signed-off-by: Stefan Metzmacher <metze@xxxxxxxxx>
---
 python/samba/dbchecker.py                          | 34 ++++++++++++++++++++--
 selftest/knownfail.d/samba4.blackbox.dbcheck-links |  2 --
 2 files changed, 32 insertions(+), 4 deletions(-)
 delete mode 100644 selftest/knownfail.d/samba4.blackbox.dbcheck-links

diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
index b0c4d45..ea776dc 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)
@@ -1098,6 +1105,29 @@ 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:
+
+            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 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("Readd 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]
             self.report("ERROR: Duplicate forward link values for attribute '%s' in '%s'" % (attrname, obj.dn))
             for keystr in duplicate_dict.keys():
                 d = duplicate_dict[keystr]
@@ -1108,7 +1138,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\)
-- 
1.9.1

Attachment: signature.asc
Description: OpenPGP digital signature

-- 
To unsubscribe from this list go to the following URL and read the
instructions:  https://lists.samba.org/mailman/options/samba