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>
(cherry picked from commit a1658b306d85452407388b91a745078c9c1f7dc7)

3 files changed, 116 insertions, 10 deletions

diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
index 4b21cab80b7..98bd5776b20 100644
--- a/python/samba/dbchecker.py
+++ b/python/samba/dbchecker.py
@@ -100,6 +100,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 = {}
@@ -187,6 +188,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 = []
 
@@ -1724,6 +1733,100 @@ 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,
@@ -2104,6 +2207,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
index 670e42b747c..00000000000
--- 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 dfb7422ac0b..9b87ca10c57 100644
--- 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)