Attachment 14887 Details for Bug 13816 – Testing patches for master

[patch] Testing patches for master

tmp.diff.txt (text/plain), 15.01 KB, created by Stefan Metzmacher on 2019-03-01 15:53:01 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Metzmacher

Created: 2019-03-01 15:53:01 UTC

Size: 15.01 KB

patch

obsolete

>From bdbfd84d7881efa6d53ae04e6586e6d6f33cbd1e Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Mon, 25 Feb 2019 15:09:36 +0100
>Subject: [PATCH 1/5] dbcheck: do isDeleted, systemFlags and
> replPropertyMetaData detection first
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13816
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>---
> python/samba/dbchecker.py | 25 ++++++++++++++-----------
> 1 file changed, 14 insertions(+), 11 deletions(-)
>
>diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
>index 705be388827e..15170e1d7129 100644
>--- a/python/samba/dbchecker.py
>+++ b/python/samba/dbchecker.py
>@@ -2098,7 +2098,6 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
>         error_count = 0
>         set_attrs_from_md = set()
>         set_attrs_seen = set()
>-        got_repl_property_meta_data = False
>         got_objectclass = False
> 
>         nc_dn = self.samdb.get_nc_root(obj.dn)
>@@ -2115,6 +2114,18 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
>         name_val = None
>         isDeleted = False
>         systemFlags = 0
>+        repl_meta_data_val = None
>+
>+        for attrname in obj:
>+            if str(attrname).lower() == 'isdeleted':
>+                if str(obj[attrname][0]) != "FALSE":
>+                    isDeleted = True
>+
>+            if str(attrname).lower() == 'systemflags':
>+                systemFlags = int(obj[attrname][0])
>+
>+            if str(attrname).lower() == 'replpropertymetadata':
>+                repl_meta_data_val = obj[attrname][0]
> 
>         for attrname in obj:
>             if attrname == 'dn' or attrname == "distinguishedName":
>@@ -2140,13 +2151,6 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
>                 else:
>                     object_rdn_val = str(obj[attrname][0])
> 
>-            if str(attrname).lower() == 'isdeleted':
>-                if str(obj[attrname][0]) != "FALSE":
>-                    isDeleted = True
>-
>-            if str(attrname).lower() == 'systemflags':
>-                systemFlags = int(obj[attrname][0])
>-
>             if str(attrname).lower() == 'replpropertymetadata':
>                 if self.has_replmetadata_zero_invocationid(dn, obj[attrname][0]):
>                     error_count += 1
>@@ -2176,7 +2180,6 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
>                         self.report("ERROR: Not fixing incorrect initial attributeID in '%s' on '%s', it should be objectClass" %
>                                     (attrname, str(dn)))
> 
>-                got_repl_property_meta_data = True
>                 continue
> 
>             if str(attrname).lower() == 'ntsecuritydescriptor':
>@@ -2354,13 +2357,13 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
>                 self.report("ERROR: Not fixing %s=%r on '%s'" % (object_rdn_attr, object_rdn_val, str(obj.dn)))
> 
>         show_dn = True
>-        if got_repl_property_meta_data:
>+        if repl_meta_data_val:
>             if obj.dn == deleted_objects_dn:
>                 isDeletedAttId = 131120
>                 # It's 29/12/9999 at 23:59:59 UTC as specified in MS-ADTS 7.1.1.4.2 Deleted Objects Container
> 
>                 expectedTimeDo = 2650466015990000000
>-                originating = self.get_originating_time(obj["replPropertyMetaData"][0], isDeletedAttId)
>+                originating = self.get_originating_time(repl_meta_data_val, isDeletedAttId)
>                 if originating != expectedTimeDo:
>                     if self.confirm_all("Fix isDeleted originating_change_time on '%s'" % str(dn), 'fix_time_metadata'):
>                         nmsg = ldb.Message()
>-- 
>2.17.1
>
>
>From a4114fe25aa5d111053a5c16ac277de8a0a22655 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Mon, 25 Feb 2019 15:35:22 +0100
>Subject: [PATCH 2/5] dbcheck: don't move already deleted objects to
> LostAndFound
>
>This would typically happen when the garbage collection
>removed a parent object before a child object (both with
>the DISALLOW_MOVE_ON_DELETE bit set in systemFlags),
>while dbcheck is running at the same time as the garbage collection.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13816
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>---
> python/samba/dbchecker.py | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
>diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
>index 15170e1d7129..34a126b780b7 100644
>--- a/python/samba/dbchecker.py
>+++ b/python/samba/dbchecker.py
>@@ -2398,8 +2398,13 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
>         except ldb.LdbError as e11:
>             (enum, estr) = e11.args
>             if enum == ldb.ERR_NO_SUCH_OBJECT:
>-                self.err_missing_parent(obj)
>-                error_count += 1
>+                if isDeleted:
>+                    self.report("WARNING: parent object not found for %s" % (obj.dn))
>+                    self.report("Not moving to LostAndFound "
>+                                "(tombstone garbage collection in progress?)")
>+                else:
>+                    self.err_missing_parent(obj)
>+                    error_count += 1
>             else:
>                 raise
> 
>-- 
>2.17.1
>
>
>From 6f366f125d7102ad680c81f87c10f290758d9d88 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Mon, 25 Feb 2019 15:35:22 +0100
>Subject: [PATCH 3/5] dbcheck: don't remove dangling one-way links on already
> deleted objects
>
>This would typically happen when the garbage collection
>removed a parent object before a child object (both with
>the DISALLOW_MOVE_ON_DELETE bit set in systemFlags),
>while dbcheck is running at the same time as the garbage collection.
>In this case the lastKnownParent attributes points a non existing
>object.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13816
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>---
> python/samba/dbchecker.py | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
>diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
>index 34a126b780b7..4fd5ec970e84 100644
>--- a/python/samba/dbchecker.py
>+++ b/python/samba/dbchecker.py
>@@ -571,6 +571,19 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
>     def err_missing_target_dn_or_GUID(self, dn, attrname, val, dsdb_dn):
>         """handle a missing target DN (if specified, GUID form can't be found,
>         and otherwise DN string form can't be found)"""
>+
>+        # Don't change anything if the object itself is deleted
>+        if str(dn).find('\\0ADEL') != -1:
>+            # We don't bump the error count as Samba produces these
>+            # in normal operation
>+            self.report("WARNING: no target object found for GUID "
>+                        "component link %s in deleted object "
>+                        "%s - %s" % (attrname, dn, val))
>+            self.report("Not removing dangling one-way "
>+                        "link on deleted object "
>+                        "(tombstone garbage collection in progress?)")
>+            return 0
>+
>         # check if its a backlink
>         linkID, _ = self.get_attr_linkID_and_reverse_name(attrname)
>         if (linkID & 1 == 0) and str(dsdb_dn).find('\\0ADEL') == -1:
>-- 
>2.17.1
>
>
>From 84d0a31405c78ca11bb3a9dbd84137c5040d3afe Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Thu, 28 Feb 2019 18:16:27 +0100
>Subject: [PATCH 4/5] dbcheck: add find_repl_attid() helper function
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13816
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>---
> python/samba/dbchecker.py | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
>diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
>index 4fd5ec970e84..7bb2a5e32e42 100644
>--- a/python/samba/dbchecker.py
>+++ b/python/samba/dbchecker.py
>@@ -1501,6 +1501,13 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
> 
>         return error_count
> 
>+    def find_repl_attid(self, repl, attid):
>+        for o in repl.ctr.array:
>+            if o.attid == attid:
>+                return o
>+
>+        return None
>+
>     def get_originating_time(self, val, attid):
>         '''Read metadata properties and return the originating time for
>            a given attributeId.
>@@ -1509,11 +1516,9 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
>         '''
> 
>         repl = ndr_unpack(drsblobs.replPropertyMetaDataBlob, val)
>-
>-        for o in repl.ctr.array:
>-            if o.attid == attid:
>-                return o.originating_change_time
>-
>+        o = self.find_repl_attid(repl, attid)
>+        if o is not None:
>+            return o.originating_change_time
>         return 0
> 
>     def process_metadata(self, dn, val):
>-- 
>2.17.1
>
>
>From 6cd4b421923114e4e0b06e612c1fc22eff92239b Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Thu, 28 Feb 2019 18:22:18 +0100
>Subject: [PATCH 5/5] 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>
>---
> python/samba/dbchecker.py | 103 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 103 insertions(+)
>
>diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
>index 7bb2a5e32e42..bd9615b0d2f9 100644
>--- 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,94 @@ 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)
>+
>+        unexpected = []
>+        for o in found:
>+            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 +2242,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
>-- 
>2.17.1
>

Actions: View

Attachments on bug 13816: 14887 | 14909 | 14946 | 15005 | 15006 | 15007