dbcheck: detect the change after deletion bug

Old versions of 'samba-tool dbcheck' could reanimate
deleted objects, when running at the same time as the
tombstone garbage collection.

When the (deleted) parent of a deleted object
(with the DISALLOW_MOVE_ON_DELETE bit in systemFlags),
is removed before the object itself, dbcheck moved
it in the LostAndFound[Config] subtree of the partition
as an originating change. That means that the object
will be in tombstone state again for 180 days on the local
DC. And other DCs fail to replicate the object as
it's already removed completely there and the replication
only gives the name and lastKnownParent attributes, because
all other attributes should already be known to the other DC.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
index 88a9ddb0ab964450b7e1efb17eab76c5dece0b8f..7e573e6dec80590f6ba6bf15213b00add1a782a9 100644 (file)
--- a/python/samba/dbchecker.py
+++ b/python/samba/dbchecker.py
@@ -122,6 +122,7 @@ class dbcheck(object):
         self.fix_missing_deleted_objects = False
         self.fix_replica_locations = False
         self.fix_missing_rid_set_master = False
+        self.fix_changes_after_deletion_bug = False
 
         self.dn_set = set()
         self.link_id_cache = {}
@@ -210,6 +211,14 @@ class dbcheck(object):
         else:
             self.rid_set_dn = None
 
+        ntds_service_dn = "CN=Directory Service,CN=Windows NT,CN=Services,%s" % \
+                          self.samdb.get_config_basedn().get_linearized()
+        res = samdb.search(base=ntds_service_dn,
+                           scope=ldb.SCOPE_BASE,
+                           expression="(objectClass=nTDSService)",
+                           attrs=["tombstoneLifetime"])
+        self.tombstoneLifetime = int(res[0]["tombstoneLifetime"][0])
+
         self.compatibleFeatures = []
         self.requiredFeatures = []
 
@@ -1768,6 +1777,101 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
             self.report("Fixed attribute '%s' of '%s'\n" % (sd_attr, dn))
         self.samdb.set_session_info(self.system_session_info)
 
+    def find_changes_after_deletion(self, repl_val):
+        repl = ndr_unpack(drsblobs.replPropertyMetaDataBlob, repl_val)
+
+        isDeleted = self.find_repl_attid(repl, drsuapi.DRSUAPI_ATTID_isDeleted)
+
+        delete_time = samba.nttime2unix(isDeleted.originating_change_time)
+
+        tombstone_delta = self.tombstoneLifetime * (24 * 60 * 60)
+
+        found = []
+        for o in repl.ctr.array:
+            if o.attid == drsuapi.DRSUAPI_ATTID_isDeleted:
+                continue
+
+            if o.local_usn <= isDeleted.local_usn:
+                continue
+
+            if o.originating_change_time <= isDeleted.originating_change_time:
+                continue
+
+            change_time = samba.nttime2unix(o.originating_change_time)
+
+            delta = change_time - delete_time
+            if delta <= tombstone_delta:
+                continue
+
+            # If the modification happened after the tombstone lifetime
+            # has passed, we have a bug as the object might be deleted
+            # already on other DCs and won't be able to replicate
+            # back
+            found.append(o)
+
+        return found, isDeleted
+
+    def has_changes_after_deletion(self, dn, repl_val):
+        found, isDeleted = self.find_changes_after_deletion(repl_val)
+        if len(found) == 0:
+            return False
+
+        def report_attid(o):
+            try:
+                attname = self.samdb_schema.get_lDAPDisplayName_by_attid(o.attid)
+            except KeyError:
+                attname = "<unknown:0x%x08x>" % o.attid
+
+            self.report("%s: attid=0x%08x version=%d invocation=%s usn=%s (local=%s) at %s" % (
+                        attname, o.attid, o.version,
+                        o.originating_invocation_id,
+                        o.originating_usn,
+                        o.local_usn,
+                        time.ctime(samba.nttime2unix(o.originating_change_time))))
+
+        self.report("ERROR: object %s, has changes after deletion" % dn)
+        report_attid(isDeleted)
+        for o in found:
+            report_attid(o)
+
+        return True
+
+    def err_changes_after_deletion(self, dn, repl_val):
+        found, isDeleted = self.find_changes_after_deletion(repl_val)
+
+        in_schema_nc = dn.is_child_of(self.schema_dn)
+        rdn_attr = dn.get_rdn_name()
+        rdn_attid = self.samdb_schema.get_attid_from_lDAPDisplayName(rdn_attr,
+                                                     is_schema_nc=in_schema_nc)
+
+        unexpected = []
+        for o in found:
+            if o.attid == rdn_attid:
+                continue
+            if o.attid == drsuapi.DRSUAPI_ATTID_name:
+                continue
+            if o.attid == drsuapi.DRSUAPI_ATTID_lastKnownParent:
+                continue
+            try:
+                attname = self.samdb_schema.get_lDAPDisplayName_by_attid(o.attid)
+            except KeyError:
+                attname = "<unknown:0x%x08x>" % o.attid
+            unexpected.append(attname)
+
+        if len(unexpected) > 0:
+            self.report('Unexpeted attributes: %s' % ",".join(unexpected))
+            self.report('Not fixing changes after deletion bug')
+            return
+
+        if not self.confirm_all('Delete broken tombstone object %s deleted %s days ago?' % (
+                                dn, self.tombstoneLifetime), 'fix_changes_after_deletion_bug'):
+            self.report('Not fixing changes after deletion bug')
+            return
+
+        if self.do_delete(dn, ["relax:0"],
+                          "Failed to remove DN %s" % dn):
+            self.report("Removed DN %s" % dn)
+
     def has_replmetadata_zero_invocationid(self, dn, repl_meta_data):
         repl = ndr_unpack(drsblobs.replPropertyMetaDataBlob,
                           repl_meta_data)
@@ -2145,6 +2249,12 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
             if str(attrname).lower() == 'replpropertymetadata':
                 repl_meta_data_val = obj[attrname][0]
 
+        if isDeleted and repl_meta_data_val:
+            if self.has_changes_after_deletion(dn, repl_meta_data_val):
+                error_count += 1
+                self.err_changes_after_deletion(dn, repl_meta_data_val)
+                return error_count
+
         for attrname in obj:
             if attrname == 'dn' or attrname == "distinguishedName":
                 continue

diff --git a/selftest/knownfail.d/dbcheck-list-deleted b/selftest/knownfail.d/dbcheck-list-deleted
deleted file mode 100644 (file)
index 670e42b..0000000
--- a/selftest/knownfail.d/dbcheck-list-deleted
+++ /dev/null
@@ -1,2 +0,0 @@
-^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.lost_deleted_user2_clean
-^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.dbcheck_clean3

diff --git a/source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output-lost-deleted-user2.txt b/source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output-lost-deleted-user2.txt
index dfb7422ac0bf7e4d271a596ee391a397f0cd47ae..9b87ca10c57e9f5da33ed8f831b7417e4d8617e7 100644 (file)
--- a/source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output-lost-deleted-user2.txt
+++ b/source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output-lost-deleted-user2.txt
@@ -1,9 +1,8 @@
 Checking 232 objects
-ERROR: missing GUID component for lastKnownParent in object CN=fred\0ADEL:2301a64c-8765-4321-851e-12d4a711cfb4,CN=LostAndFound,DC=release-4-5-0-pre1,DC=samba,DC=corp - OU=removed,DC=release-4-5-0-pre1,DC=samba,DC=corp
-unable to find object for DN OU=removed,DC=release-4-5-0-pre1,DC=samba,DC=corp - (No such Base DN: OU=removed,DC=release-4-5-0-pre1,DC=samba,DC=corp)
-WARNING: no target object found for GUID component link lastKnownParent in deleted object CN=fred\0ADEL:2301a64c-8765-4321-851e-12d4a711cfb4,CN=LostAndFound,DC=release-4-5-0-pre1,DC=samba,DC=corp - OU=removed,DC=release-4-5-0-pre1,DC=samba,DC=corp
-Not removing dangling one-way link on deleted object (tombstone garbage collection in progress?)
-ERROR: wrong dn[CN=fred\0ADEL:2301a64c-8765-4321-851e-12d4a711cfb4,CN=LostAndFound,DC=release-4-5-0-pre1,DC=samba,DC=corp] cn='fred\nDEL:2301a64c-8765-4321-851e-12d4a711cfb4' name=b'fred\nDEL:2301a64c-8765-4321-851e-12d4a711cfb4' new_dn[CN=fred\0ADEL:2301a64c-8765-4321-851e-12d4a711cfb4,CN=Deleted Objects,DC=release-4-5-0-pre1,DC=samba,DC=corp]
-Rename CN=fred\0ADEL:2301a64c-8765-4321-851e-12d4a711cfb4,CN=LostAndFound,DC=release-4-5-0-pre1,DC=samba,DC=corp to CN=fred\0ADEL:2301a64c-8765-4321-851e-12d4a711cfb4,CN=Deleted Objects,DC=release-4-5-0-pre1,DC=samba,DC=corp? [YES]
-Renamed CN=fred\0ADEL:2301a64c-8765-4321-851e-12d4a711cfb4,CN=LostAndFound,DC=release-4-5-0-pre1,DC=samba,DC=corp into CN=fred\0ADEL:2301a64c-8765-4321-851e-12d4a711cfb4,CN=Deleted Objects,DC=release-4-5-0-pre1,DC=samba,DC=corp
-Checked 232 objects (2 errors)
+ERROR: object CN=fred\0ADEL:2301a64c-8765-4321-851e-12d4a711cfb4,CN=LostAndFound,DC=release-4-5-0-pre1,DC=samba,DC=corp, has changes after deletion
+isDeleted: attid=0x00020030 version=1 invocation=4e4496a3-7fb8-4f97-8a33-d238db8b5e2d usn=3746 (local=3746) at Wed Jun 29 04:36:39 2016
+name: attid=0x00090001 version=4 invocation=4e4496a3-7fb8-4f97-8a33-d238db8b5e2d usn=3772 (local=3772) at Mon Mar 11 13:28:24 2019
+lastKnownParent: attid=0x0009030d version=3 invocation=4e4496a3-7fb8-4f97-8a33-d238db8b5e2d usn=3773 (local=3773) at Mon Mar 11 13:28:24 2019
+Delete broken tombstone object CN=fred\0ADEL:2301a64c-8765-4321-851e-12d4a711cfb4,CN=LostAndFound,DC=release-4-5-0-pre1,DC=samba,DC=corp deleted 180 days ago? [YES]
+Removed DN CN=fred\0ADEL:2301a64c-8765-4321-851e-12d4a711cfb4,CN=LostAndFound,DC=release-4-5-0-pre1,DC=samba,DC=corp
+Checked 232 objects (1 errors)