From 4716819e5cbccf53c5a492983f8ba7fb9e8502f5 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 2 Feb 2017 16:27:35 +1300 Subject: [PATCH 1/2] dbcheck: Do not regard old one-way-links as errors Samba does not maintain one way links when the target is deleted or renamed so do not fail dbcheck because of such links, but allow them to be updated. This matters because administrators and make test expect that normal Samba operation do NOT cause the database to become corrupt, and any error from dbcheck tends to trigger alarms (or test failures). If an object pointed at by a one way link is renamed or deleted in normal operations (such as intersiteTopologyGenerator pointing at a demoted DC), or make test, then this could trigger. Signed-off-by: Andrew Bartlett Reviewed-by: Garming Sam BUG: https://bugzilla.samba.org/show_bug.cgi?id=12577 (cherry picked from commit 35bfc62a31c9ad73449594ddd48f76f50e0abade) --- python/samba/dbchecker.py | 47 +++++++++++++++++++++++++++++++++++------- testprogs/blackbox/renamedc.sh | 6 ++++-- 2 files changed, 43 insertions(+), 10 deletions(-) diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py index 3fcfbc05..22819de 100644 --- a/python/samba/dbchecker.py +++ b/python/samba/dbchecker.py @@ -59,6 +59,7 @@ class dbcheck(object): self.fix_all_string_dn_component_mismatch = False self.fix_all_GUID_dn_component_mismatch = False self.fix_all_SID_dn_component_mismatch = False + self.fix_all_old_dn_string_component_mismatch = False self.fix_all_metadata = False self.fix_time_metadata = False self.fix_undead_linked_attributes = False @@ -574,6 +575,23 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base))) "Failed to fix %s on attribute %s" % (errstr, attrname)): self.report("Fixed %s on attribute %s" % (errstr, attrname)) + def err_dn_string_component_old(self, dn, attrname, val, dsdb_dn, correct_dn): + """handle a DN string being incorrect""" + self.report("NOTE: old (due to rename or delete) DN string component for %s in object %s - %s" % (attrname, dn, val)) + dsdb_dn.dn = correct_dn + + if not self.confirm_all('Change DN to %s?' % str(dsdb_dn), + 'fix_all_old_dn_string_component_mismatch'): + self.report("Not fixing old string component") + return + m = ldb.Message() + m.dn = dn + m['old_value'] = ldb.MessageElement(val, ldb.FLAG_MOD_DELETE, attrname) + m['new_value'] = ldb.MessageElement(str(dsdb_dn), ldb.FLAG_MOD_ADD, attrname) + if self.do_modify(m, ["show_recycled:1"], + "Failed to fix old DN string on attribute %s" % (attrname)): + self.report("Fixed old DN string on attribute %s" % (attrname)) + def err_dn_component_target_mismatch(self, dn, attrname, val, dsdb_dn, correct_dn, mismatch_type): """handle a DN string being incorrect""" self.report("ERROR: incorrect DN %s component for %s in object %s - %s" % (mismatch_type, attrname, dn, val)) @@ -914,12 +932,16 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base))) if rmd_flags & 1: continue - # check the DN matches in string form - if str(res[0].dn) != str(dsdb_dn.dn): - error_count += 1 - self.err_dn_component_target_mismatch(obj.dn, attrname, val, dsdb_dn, - res[0].dn, "string") - continue + # assert the DN matches in string form, where a reverse + # link exists, otherwise (below) offer to fix it as a non-error. + # The string form is essentially only kept for forensics, + # as we always re-resolve by GUID in normal operations. + if reverse_link_name is not None: + if str(res[0].dn) != str(dsdb_dn.dn): + error_count += 1 + self.err_dn_component_target_mismatch(obj.dn, attrname, val, dsdb_dn, + res[0].dn, "string") + continue if res[0].dn.get_extended_component("GUID") != dsdb_dn.dn.get_extended_component("GUID"): error_count += 1 @@ -933,9 +955,18 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base))) res[0].dn, "SID") continue + # Now we have checked the GUID and SID, offer to fix old + # DN strings as a non-error (for forward links with no + # backlink). Samba does not maintain this string + # otherwise, so we don't increment error_count. + if reverse_link_name is None: + if str(res[0].dn) != str(dsdb_dn.dn): + self.err_dn_string_component_old(obj.dn, attrname, val, dsdb_dn, + res[0].dn) + continue - # check the reverse_link is correct if there should be one - if reverse_link_name is not None: + else: + # check the reverse_link is correct if there should be one match_count = 0 if reverse_link_name in res[0]: for v in res[0][reverse_link_name]: diff --git a/testprogs/blackbox/renamedc.sh b/testprogs/blackbox/renamedc.sh index 3eb5817..7767d9d 100755 --- a/testprogs/blackbox/renamedc.sh +++ b/testprogs/blackbox/renamedc.sh @@ -65,8 +65,10 @@ testrenamedc2() { } dbcheck_fix() { + # Unlike most calls to dbcheck --fix, this will not trigger an error, as + # we do not flag an error count for this old DN string case. $BINDIR/samba-tool dbcheck --cross-ncs -s $PREFIX/renamedc_test/etc/smb.conf --fix \ - --quiet --yes fix_all_string_dn_component_mismatch \ + --quiet --yes fix_all_old_dn_string_component_mismatch \ --attrs="fsmoRoleOwner interSiteTopologyGenerator msDS-NC-Replica-Locations" } @@ -83,7 +85,7 @@ testit "confirmrenamedc_sAMAccountName" confirmrenamedc_sAMAccountName || failed testit "confirmrenamedc_dNSHostName" confirmrenamedc_dNSHostName || failed=`expr $failed + 1` testit "confirmrenamedc_rootdse_dnsHostName" confirmrenamedc_rootdse_dnsHostName || failed=`expr $failed + 1` testit "confirmrenamedc_rootdse_dsServiceName" confirmrenamedc_rootdse_dsServiceName || failed=`expr $failed + 1` -testit_expect_failure "dbcheck_fix" dbcheck_fix || failed=`expr $failed + 1` +testit "dbcheck_fix" dbcheck_fix || failed=`expr $failed + 1` testit "dbcheck" dbcheck || failed=`expr $failed + 1` testit "renamedc2" testrenamedc2 || failed=`expr $failed + 1` -- 1.9.1 From 3e7bddd5dace46a807cdc7ae7fdccdf4123f7f93 Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Wed, 8 Feb 2017 15:24:14 +1300 Subject: [PATCH 2/2] dbcheck-links: Test that dbcheck against one-way links does not error Signed-off-by: Garming Sam BUG: https://bugzilla.samba.org/show_bug.cgi?id=12577 Pair-programmed-with: Bob Campbell Reviewed-by: Andrew Bartlett Autobuild-User(master): Andrew Bartlett Autobuild-Date(master): Mon Feb 13 07:33:08 CET 2017 on sn-devel-144 (cherry picked from commit 44ee31675afd277d429cb246525741110f8fceec) --- .../release-4-5-0-pre1/dangling-one-way-link.ldif | 15 +++++++++++++++ testprogs/blackbox/dbcheck-links.sh | 10 ++++++++++ 2 files changed, 25 insertions(+) create mode 100644 source4/selftest/provisions/release-4-5-0-pre1/dangling-one-way-link.ldif diff --git a/source4/selftest/provisions/release-4-5-0-pre1/dangling-one-way-link.ldif b/source4/selftest/provisions/release-4-5-0-pre1/dangling-one-way-link.ldif new file mode 100644 index 0000000..c215c06 --- /dev/null +++ b/source4/selftest/provisions/release-4-5-0-pre1/dangling-one-way-link.ldif @@ -0,0 +1,15 @@ +dn: CN=secretary,CN=users,DC=release-4-5-0-pre1,DC=samba,DC=corp +changetype: add +objectclass: user +samaccountname: secretary + +dn: CN=dangling-one-way,CN=users,DC=release-4-5-0-pre1,DC=samba,DC=corp +changetype: add +objectclass: user +samaccountname: dangling-one-way +secretary: CN=secretary,CN=users,DC=release-4-5-0-pre1,DC=samba,DC=corp + +dn: CN=secretary,CN=users,DC=release-4-5-0-pre1,DC=samba,DC=corp +changetype: modrdn +newrdn: cn=new-secretary +deleteoldrdn: 1 diff --git a/testprogs/blackbox/dbcheck-links.sh b/testprogs/blackbox/dbcheck-links.sh index 11592f0..0799a50 100755 --- a/testprogs/blackbox/dbcheck-links.sh +++ b/testprogs/blackbox/dbcheck-links.sh @@ -157,6 +157,14 @@ check_expected_after_objects() { fi } +dangling_one_way() { + ldif=$release_dir/dangling-one-way-link.ldif + TZ=UTC $ldbmodify -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb $ldif + if [ "$?" != "0" ]; then + return 1 + fi +} + if [ -d $release_dir ]; then testit $RELEASE undump testit "add_two_more_users" add_two_more_users @@ -169,6 +177,8 @@ if [ -d $release_dir ]; then testit "check_expected_after_deleted_links" check_expected_after_deleted_links testit "check_expected_after_links" check_expected_after_links testit "check_expected_after_objects" check_expected_after_objects + testit "dangling_one_way" dangling_one_way + testit "dbcheck_clean" dbcheck_clean else subunit_start_test $RELEASE subunit_skip_test $RELEASE <