From af8184dcb9cffcb9981827b2f934e9966f639320 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 028f0b3c77e053e525d602107ca9185462ce9de6 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 bc657b3861335a9b2e517b3700939b61a489a483 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 ce7fc5f192bca11fe370a9f8fc486119a4049505 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 24f39da910bba9c30a2901f3043420ca164bc207 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 | 110 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+) diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py index 7bb2a5e32e42..12b9b2621251 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,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 = "" % 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 = "" % 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 -- 2.17.1