From bdbfd84d7881efa6d53ae04e6586e6d6f33cbd1e Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher 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 --- 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 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 --- 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 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 --- 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 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 --- 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 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 --- 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 = "" % 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 = "" % 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