From 160524e2bc43d217bc4cf57d80ec92c7d81f7147 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 30 Jan 2018 10:39:30 +0100 Subject: [PATCH 01/22] python:tests: use TestCaseInTempDir for "samba.tests.common" BUG: https://bugzilla.samba.org/show_bug.cgi?id=13228 Signed-off-by: Stefan Metzmacher Reviewed-by: Ralph Boehme (cherry picked from commit 681e0a1745b45c6ac22d394b9e78cb67007d7dc4) --- 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:;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 cf71af6f15875228937366b5ded834e2c65ddd79 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 30 Jan 2018 11:09:40 +0100 Subject: [PATCH 02/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 Reviewed-by: Ralph Boehme (cherry picked from commit 1341780dcf9ec0c5d852fbbb77c5e00db2ad6564) --- 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:;DC=samba,DC=example,DC=com") -- 1.9.1 From d24979dbf32010ee237d3c15a20b4295adb8297f Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 30 Jan 2018 11:09:55 +0100 Subject: [PATCH 03/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 Reviewed-by: Ralph Boehme (cherry picked from commit c56eb49119117a1a06afb0a76630ae5c7a1ca30c) --- 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:;OU=dn1,DC=samba,DC=example,DC=com") + dn2 = dsdb_Dn(sam, "B:8:0000000C:;OU=dn1,DC=samba,DC=example,DC=com") + dn3 = dsdb_Dn(sam, "B:8:0000000F:;OU=dn3,DC=samba,DC=example,DC=com") + dn4 = dsdb_Dn(sam, "B:8:00000000:;OU=dn4,DC=samba,DC=example,DC=com") + dn5 = dsdb_Dn(sam, ";OU=dn5,DC=samba,DC=example,DC=com") + dn6 = dsdb_Dn(sam, ";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 f306536e23f4f9dad1695d0b0a5a5cb54ba2871f Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 30 Jan 2018 09:51:20 +0100 Subject: [PATCH 04/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 Reviewed-by: Ralph Boehme (cherry picked from commit 55d466549a3113f7625acdd6eb42f71cf63719b5) --- 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 bff04efb23943681c0e81de24f6218c522b2660f Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 24 Jan 2018 11:34:43 +0100 Subject: [PATCH 05/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 Signed-off-by: Stefan Metzmacher (cherry picked from commit 8c01acd56274a5cb5926622cacab997cb62dd5a9) --- 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 35324e22ca66898e6960bfa916d89faa4360c353 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 25 Jan 2018 21:34:47 +0100 Subject: [PATCH 06/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 Reviewed-by: Stefan Metzmacher (cherry picked from commit 52bd0b09804621e6de9ee0a377a442a42e07ee05) --- 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 ';;;;;;;;;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 ';;;;;;;;;CN=Administrator,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp' +Correct link ';;;;;;;;;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 <;;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 7f9621e221e5f15fd679b3511380b945ec79fcda Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 24 Jan 2018 19:31:23 +0100 Subject: [PATCH 07/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 Signed-off-by: Ralph Boehme Signed-off-by: Stefan Metzmacher (cherry picked from commit 4a71394c6a30e8a1b5c6553f7410148dbf2e4a80) --- 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 be4c6c3207127585f6a245ecc9a093140af7914a Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 25 Jan 2018 10:52:35 +0100 Subject: [PATCH 08/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 Signed-off-by: Ralph Boehme Signed-off-by: Stefan Metzmacher (cherry picked from commit 6f77503871fcb815e474cb76d14e22f7a8f083c9) --- 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 a924fcd2c30877e1c29bbfff7b09694fc48d8ada Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 29 Jan 2018 22:48:42 +0100 Subject: [PATCH 09/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 Reviewed-by: Ralph Boehme (cherry picked from commit 9f47fe6c4a8bde4abfee3c774d9667e6a3439a45) --- 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 7f23e03337e3008cd92d96df4a1641bafaf81e70 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 24 Jan 2018 19:37:55 +0100 Subject: [PATCH 10/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 Signed-off-by: Ralph Boehme Reviewed-by: Stefan Metzmacher (cherry picked from commit a651cc79d64b9bcc1d5fee9b2ef8800a1579dea1) --- 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 bc801ad3d058a901e8ee858866126a5044e3881c Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 25 Jan 2018 14:41:58 +0100 Subject: [PATCH 11/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 Signed-off-by: Ralph Boehme Signed-off-by: Stefan Metzmacher (cherry picked from commit dc43d31cd20fd12d2758b73ec0318215b8fbedfb) --- 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 ';;;;;;;;;CN=Administrator,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp' Correct link ';;;;;;;;;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 577e87d2825c873d4951c7039cadd99f69bf7151 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 25 Jan 2018 14:36:52 +0100 Subject: [PATCH 12/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 Signed-off-by: Ralph Boehme Signed-off-by: Stefan Metzmacher (cherry picked from commit ec433f8531a822dd40b343fbf3244157a5ecd544) --- 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 ';;;;;;;;;CN=Administrator,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp' Correct link ';;;;;;;;;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 45d30b9952b4627f8a716ed3e55935b2e6d42a7e Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 30 Jan 2018 09:39:40 +0100 Subject: [PATCH 13/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 Reviewed-by: Ralph Boehme (cherry picked from commit b0bc3f60084e5998dd34aada2ac7377d390affc6) --- 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 5512ec351ad61215c776d0e2e6c2760eb2515031 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 30 Jan 2018 09:55:21 +0100 Subject: [PATCH 14/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 Reviewed-by: Ralph Boehme (cherry picked from commit 7df17c0a8dffceb053ca806c9426d493b4837b1a) --- 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 1d0aad1e2c70620b5fdaa4956ab92f94789dc790 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 24 Jan 2018 20:01:27 +0100 Subject: [PATCH 15/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 Signed-off-by: Ralph Boehme Signed-off-by: Stefan Metzmacher (cherry picked from commit 44a8782d71676517f0991f279f2472391ecede3b) --- 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 d46c01add5d93e64bce9ef92ac51a9714923291a Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 25 Jan 2018 10:34:29 +0100 Subject: [PATCH 16/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 Signed-off-by: Ralph Boehme Signed-off-by: Stefan Metzmacher (cherry picked from commit e4cc062fa98f65369f3bde24a987c2651632cb06) --- 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 51ee49a08695e0bff69e55c11b6470155f368206 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 24 Jan 2018 22:24:15 +0100 Subject: [PATCH 17/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 Signed-off-by: Ralph Boehme Signed-off-by: Stefan Metzmacher (cherry picked from commit e258b4fb281d8577c425e05b35ce05cf128617ea) --- 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 fbcdb0a8013d4afc24b51210b97886d54ca9b0fb Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 30 Jan 2018 12:19:31 +0100 Subject: [PATCH 18/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 Reviewed-by: Ralph Boehme (cherry picked from commit 20598033866ca3d0fdad1edf3cb39e4614eae112) --- 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 33f01c20b80b288035f681a83c4c392fe3ef5c85 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 30 Jan 2018 12:19:31 +0100 Subject: [PATCH 19/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 Reviewed-by: Ralph Boehme (cherry picked from commit 182fb3c4c9db8715d0dbcbc3d1aa0655b5cb29f1) --- 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 e13d0552ce4646798555e05423ede01a0cfaf1f5 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 25 Jan 2018 14:48:55 +0100 Subject: [PATCH 20/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 Signed-off-by: Ralph Boehme Signed-off-by: Stefan Metzmacher (cherry picked from commit d59f201388e8a16688adda145734dab8e27b785f) --- 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=)" % (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: + # + # ; + # ; + # ; + # ; + # ; + # ; + # ; + # ; + # ; + # 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 97e9367c6f88c8aed2015d9f41ab3119a4eb82dd Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 25 Jan 2018 14:48:55 +0100 Subject: [PATCH 21/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 Signed-off-by: Ralph Boehme Signed-off-by: Stefan Metzmacher (cherry picked from commit 5bf823d68bd33ee3160175a18a3838eff4e3cbb2) --- 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 ';;;;;;;;;CN=Administrator,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp' Correct link ';;;;;;;;;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 0a445e7e69b60099512c018ffca7dd4093bbef89 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 31 Jan 2018 09:50:47 +0100 Subject: [PATCH 22/22] 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 Reviewed-by: Ralph Boehme Autobuild-User(master): Stefan Metzmacher Autobuild-Date(master): Mon Feb 5 18:32:51 CET 2018 on sn-devel-144 (cherry picked from commit 0c3348feb09f4f0ba85455b8c3ff5c5fa60d139b) --- python/samba/dbchecker.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py index c2c95a2..b2b8b0c 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) @@ -745,6 +762,9 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base))) 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'" % forward_attr): self.report("Fixed duplicate links in attribute '%s'" % (forward_attr)) + duplicate_cache_key = "%s:%s" % (str(obj.dn), forward_attr) + assert duplicate_cache_key in self.duplicate_link_cache + self.duplicate_link_cache[duplicate_cache_key] = False def err_no_fsmoRoleOwner(self, obj): '''handle a missing fSMORoleOwner''' @@ -1011,6 +1031,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