The Samba-Bugzilla – Attachment 17827 Details for
Bug 15329
Reduce flapping of ridalloc test
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch in master backported to Samba 4.17
ridalloc-4.17.patch (text/plain), 17.31 KB, created by
Andrew Bartlett
on 2023-03-15 19:30:25 UTC
(
hide
)
Description:
Patch in master backported to Samba 4.17
Filename:
MIME Type:
Creator:
Andrew Bartlett
Created:
2023-03-15 19:30:25 UTC
Size:
17.31 KB
patch
obsolete
>From 4b6c5a0186189a970fe5d05eee3f7e73971c70e3 Mon Sep 17 00:00:00 2001 >From: Joseph Sutton <josephsutton@catalyst.net.nz> >Date: Wed, 14 Sep 2022 13:21:34 +1200 >Subject: [PATCH 1/3] CVE-2020-25720 pydsdb: Add AD schema GUID constants > >This helps reduce the profusion of magic constant values in Python >tests. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14810 >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15329 >Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz> >Reviewed-by: Andrew Bartlett <abartlet@samba.org> >(cherry picked from commit 2563f85237bd4260b7b527f3695f27da4cc61a74) > >[abartlet@samba.org Required context for backport of bug 15329 to > Samba 4.17] >--- > libds/common/flags.h | 14 ++++++++++++++ > source4/dsdb/pydsdb.c | 13 +++++++++++++ > 2 files changed, 27 insertions(+) > >diff --git a/libds/common/flags.h b/libds/common/flags.h >index 75e04b0c488..ec40e4ba01b 100644 >--- a/libds/common/flags.h >+++ b/libds/common/flags.h >@@ -237,6 +237,20 @@ > /* wellknown GUIDs for optional directory features */ > #define DS_GUID_FEATURE_RECYCLE_BIN "766ddcd8-acd0-445e-f3b9-a7f9b6744f2a" > >+/* GUIDs for AD schema attributes and classes */ >+#define DS_GUID_SCHEMA_ATTR_DEPARTMENT "bf96794f-0de6-11d0-a285-00aa003049e2" >+#define DS_GUID_SCHEMA_ATTR_DNS_HOST_NAME "72e39547-7b18-11d1-adef-00c04fd8d5cd" >+#define DS_GUID_SCHEMA_ATTR_INSTANCE_TYPE "bf96798c-0de6-11d0-a285-00aa003049e2" >+#define DS_GUID_SCHEMA_ATTR_MS_SFU_30 "16c5d1d3-35c2-4061-a870-a5cefda804f0" >+#define DS_GUID_SCHEMA_ATTR_NT_SECURITY_DESCRIPTOR "bf9679e3-0de6-11d0-a285-00aa003049e2" >+#define DS_GUID_SCHEMA_ATTR_PRIMARY_GROUP_ID "bf967a00-0de6-11d0-a285-00aa003049e2" >+#define DS_GUID_SCHEMA_ATTR_SERVICE_PRINCIPAL_NAME "f3a64788-5306-11d1-a9c5-0000f80367c1" >+#define DS_GUID_SCHEMA_ATTR_USER_ACCOUNT_CONTROL "bf967a68-0de6-11d0-a285-00aa003049e2" >+#define DS_GUID_SCHEMA_ATTR_USER_PASSWORD "bf967a6e-0de6-11d0-a285-00aa003049e2" >+#define DS_GUID_SCHEMA_CLASS_COMPUTER "bf967a86-0de6-11d0-a285-00aa003049e2" >+#define DS_GUID_SCHEMA_CLASS_MANAGED_SERVICE_ACCOUNT "ce206244-5827-4a86-ba1c-1c0c386c1b64" >+#define DS_GUID_SCHEMA_CLASS_USER "bf967aba-0de6-11d0-a285-00aa003049e2" >+ > /* dsHeuristics character indexes see MS-ADTS 7.1.1.2.4.1.2 */ > > #define DS_HR_SUPFIRSTLASTANR 0x00000001 >diff --git a/source4/dsdb/pydsdb.c b/source4/dsdb/pydsdb.c >index bcfc7e95478..04074f3dc3b 100644 >--- a/source4/dsdb/pydsdb.c >+++ b/source4/dsdb/pydsdb.c >@@ -1725,5 +1725,18 @@ MODULE_INIT_FUNC(dsdb) > ADD_DSDB_STRING(DS_GUID_SYSTEMS_CONTAINER); > ADD_DSDB_STRING(DS_GUID_USERS_CONTAINER); > >+ ADD_DSDB_STRING(DS_GUID_SCHEMA_ATTR_DEPARTMENT); >+ ADD_DSDB_STRING(DS_GUID_SCHEMA_ATTR_DNS_HOST_NAME); >+ ADD_DSDB_STRING(DS_GUID_SCHEMA_ATTR_INSTANCE_TYPE); >+ ADD_DSDB_STRING(DS_GUID_SCHEMA_ATTR_MS_SFU_30); >+ ADD_DSDB_STRING(DS_GUID_SCHEMA_ATTR_NT_SECURITY_DESCRIPTOR); >+ ADD_DSDB_STRING(DS_GUID_SCHEMA_ATTR_PRIMARY_GROUP_ID); >+ ADD_DSDB_STRING(DS_GUID_SCHEMA_ATTR_SERVICE_PRINCIPAL_NAME); >+ ADD_DSDB_STRING(DS_GUID_SCHEMA_ATTR_USER_ACCOUNT_CONTROL); >+ ADD_DSDB_STRING(DS_GUID_SCHEMA_ATTR_USER_PASSWORD); >+ ADD_DSDB_STRING(DS_GUID_SCHEMA_CLASS_COMPUTER); >+ ADD_DSDB_STRING(DS_GUID_SCHEMA_CLASS_MANAGED_SERVICE_ACCOUNT); >+ ADD_DSDB_STRING(DS_GUID_SCHEMA_CLASS_USER); >+ > return m; > } >-- >2.25.1 > > >From b35b1341947f8c4178cd3a0d7d8911450a97617e Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Thu, 9 Mar 2023 17:02:35 +1300 >Subject: [PATCH 2/3] selftest/drs: Demonstrate ERROR(ldb): uncaught exception > - Deleted target CN=NTDS Settings... in join > >"samba-tool domain join" uses the replication API in a strange way, perhaps no longer >required, except that we often still have folks upgrading from very old Samba versions. > >By deferring the writing out to the DB of link replication to the very end, we have a >better chance that all the objects required are present, however the situation may >have changed during the cycle, and a link could still be sent, pointing to a deleted >object. > >We currently fail in this situation. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15329 > >Signed-off-by: Andrew Bartlett <abartlet@samba.org> >Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz> >(cherry picked from commit 2d41bcce83a976b85636c92d6fc38c63fdde5431) >--- > .../knownfail.d/replicate_against_deleted | 1 + > source4/dsdb/pydsdb.c | 2 + > source4/dsdb/samdb/samdb.h | 2 + > source4/torture/drs/python/ridalloc_exop.py | 135 ++++++++++++++++++ > 4 files changed, 140 insertions(+) > create mode 100644 selftest/knownfail.d/replicate_against_deleted > >diff --git a/selftest/knownfail.d/replicate_against_deleted b/selftest/knownfail.d/replicate_against_deleted >new file mode 100644 >index 00000000000..9caa5346a45 >--- /dev/null >+++ b/selftest/knownfail.d/replicate_against_deleted >@@ -0,0 +1 @@ >+samba4.drs.ridalloc_exop.python\(.*\).ridalloc_exop.DrsReplicaSyncTestCase.test_replicate_against_deleted_objects_transaction >diff --git a/source4/dsdb/pydsdb.c b/source4/dsdb/pydsdb.c >index 04074f3dc3b..fb71acac1ae 100644 >--- a/source4/dsdb/pydsdb.c >+++ b/source4/dsdb/pydsdb.c >@@ -1738,5 +1738,7 @@ MODULE_INIT_FUNC(dsdb) > ADD_DSDB_STRING(DS_GUID_SCHEMA_CLASS_MANAGED_SERVICE_ACCOUNT); > ADD_DSDB_STRING(DS_GUID_SCHEMA_CLASS_USER); > >+ ADD_DSDB_STRING(DSDB_FULL_JOIN_REPLICATION_COMPLETED_OPAQUE_NAME); >+ > return m; > } >diff --git a/source4/dsdb/samdb/samdb.h b/source4/dsdb/samdb/samdb.h >index 3db7704307f..ad75cf244c4 100644 >--- a/source4/dsdb/samdb/samdb.h >+++ b/source4/dsdb/samdb/samdb.h >@@ -358,6 +358,8 @@ struct dsdb_extended_dn_store_format { > > #define DSDB_OPAQUE_PARTITION_MODULE_MSG_OPAQUE_NAME "DSDB_OPAQUE_PARTITION_MODULE_MSG" > >+#define DSDB_FULL_JOIN_REPLICATION_COMPLETED_OPAQUE_NAME "DSDB_FULL_JOIN_REPLICATION_COMPLETED" >+ > #define DSDB_ACL_CHECKS_DIRSYNC_FLAG 0x1 > #define DSDB_SAMDB_MINIMUM_ALLOWED_RID 1000 > >diff --git a/source4/torture/drs/python/ridalloc_exop.py b/source4/torture/drs/python/ridalloc_exop.py >index 8efdd6a0603..0d46eee542b 100644 >--- a/source4/torture/drs/python/ridalloc_exop.py >+++ b/source4/torture/drs/python/ridalloc_exop.py >@@ -46,6 +46,7 @@ from samba.auth import system_session, admin_session > from samba.dbchecker import dbcheck > from samba.ndr import ndr_pack > from samba.dcerpc import security >+from samba import drs_utils, dsdb > > > class DrsReplicaSyncTestCase(drs_base.DrsBaseTestCase): >@@ -676,3 +677,137 @@ class DrsReplicaSyncTestCase(drs_base.DrsBaseTestCase): > finally: > self._test_force_demote(fsmo_owner['dns_name'], "RIDALLOCTEST7") > shutil.rmtree(targetdir, ignore_errors=True) >+ >+ def test_replicate_against_deleted_objects_transaction(self): >+ """Not related to RID allocation, but uses the infrastructure here. >+ Do a join, create a link between two objects remotely, but >+ remove the target locally. Show that we need to set a magic >+ opaque if there is an outer transaction. >+ >+ """ >+ fsmo_dn = ldb.Dn(self.ldb_dc1, "CN=RID Manager$,CN=System," + self.ldb_dc1.domain_dn()) >+ (fsmo_owner, fsmo_not_owner) = self._determine_fSMORoleOwner(fsmo_dn) >+ >+ test_user4 = "ridalloctestuser4" >+ test_group = "ridalloctestgroup1" >+ >+ self.ldb_dc1.newuser(test_user4, "P@ssword!") >+ >+ self.addCleanup(self.ldb_dc1.deleteuser, test_user4) >+ >+ self.ldb_dc1.newgroup(test_group) >+ self.addCleanup(self.ldb_dc1.deletegroup, test_group) >+ >+ targetdir = self._test_join(self.dnsname_dc1, "RIDALLOCTEST8") >+ try: >+ # Connect to the database >+ ldb_url = "tdb://%s" % os.path.join(targetdir, "private/sam.ldb") >+ lp = self.get_loadparm() >+ >+ new_ldb = SamDB(ldb_url, >+ session_info=system_session(lp), lp=lp) >+ >+ destination_dsa_guid = misc.GUID(new_ldb.get_ntds_GUID()) >+ >+ repl = drs_utils.drs_Replicate(f'ncacn_ip_tcp:{self.dnsname_dc1}[seal]', >+ lp, >+ self.get_credentials(), >+ new_ldb, >+ destination_dsa_guid) >+ >+ source_dsa_invocation_id = misc.GUID(self.ldb_dc1.invocation_id) >+ >+ # Add the link on the remote DC >+ self.ldb_dc1.add_remove_group_members(test_group, [test_user4]) >+ >+ # Starting a transaction overrides, currently the logic >+ # inside repl.replicatate to retry with GET_TGT which in >+ # turn tells the repl_meta_data module that the most up to >+ # date info is already available >+ new_ldb.transaction_start() >+ repl.replicate(self.ldb_dc1.domain_dn(), >+ source_dsa_invocation_id, >+ destination_dsa_guid) >+ >+ # Delete the user locally, before applying the links. >+ # This simulates getting the delete in the replciation >+ # stream. >+ new_ldb.deleteuser(test_user4) >+ >+ # This fails as the user has been deleted locally but a remote link is sent >+ self.assertRaises(ldb.LdbError, new_ldb.transaction_commit) >+ >+ new_ldb.transaction_start() >+ repl.replicate(self.ldb_dc1.domain_dn(), >+ source_dsa_invocation_id, >+ destination_dsa_guid) >+ >+ # Delete the user locally (the previous transaction >+ # doesn't apply), before applying the links. This >+ # simulates getting the delete in the replciation stream. >+ new_ldb.deleteuser(test_user4) >+ >+ new_ldb.set_opaque_integer(dsdb.DSDB_FULL_JOIN_REPLICATION_COMPLETED_OPAQUE_NAME, >+ 1) >+ >+ # This should now work >+ try: >+ new_ldb.transaction_commit() >+ except ldb.LdbError as e: >+ self.fail(f"Failed to replicate despite setting opaque with {e.args[1]}") >+ >+ finally: >+ self._test_force_demote(self.dnsname_dc1, "RIDALLOCTEST8") >+ shutil.rmtree(targetdir, ignore_errors=True) >+ >+ def test_replicate_against_deleted_objects_normal(self): >+ """Not related to RID allocation, but uses the infrastructure here. >+ Do a join, create a link between two objects remotely, but >+ remove the target locally. . >+ >+ """ >+ fsmo_dn = ldb.Dn(self.ldb_dc1, "CN=RID Manager$,CN=System," + self.ldb_dc1.domain_dn()) >+ (fsmo_owner, fsmo_not_owner) = self._determine_fSMORoleOwner(fsmo_dn) >+ >+ test_user5 = "ridalloctestuser5" >+ test_group2 = "ridalloctestgroup2" >+ >+ self.ldb_dc1.newuser(test_user5, "P@ssword!") >+ self.addCleanup(self.ldb_dc1.deleteuser, test_user5) >+ >+ self.ldb_dc1.newgroup(test_group2) >+ self.addCleanup(self.ldb_dc1.deletegroup, test_group2) >+ >+ targetdir = self._test_join(self.dnsname_dc1, "RIDALLOCTEST9") >+ try: >+ # Connect to the database >+ ldb_url = "tdb://%s" % os.path.join(targetdir, "private/sam.ldb") >+ lp = self.get_loadparm() >+ >+ new_ldb = SamDB(ldb_url, >+ session_info=system_session(lp), lp=lp) >+ >+ destination_dsa_guid = misc.GUID(new_ldb.get_ntds_GUID()) >+ >+ repl = drs_utils.drs_Replicate(f'ncacn_ip_tcp:{self.dnsname_dc1}[seal]', >+ lp, >+ self.get_credentials(), >+ new_ldb, >+ destination_dsa_guid) >+ >+ source_dsa_invocation_id = misc.GUID(self.ldb_dc1.invocation_id) >+ >+ # Add the link on the remote DC >+ self.ldb_dc1.add_remove_group_members(test_group2, [test_user5]) >+ >+ # Delete the user locally >+ new_ldb.deleteuser(test_user5) >+ >+ # Confirm replication copes with a link to a locally deleted user >+ repl.replicate(self.ldb_dc1.domain_dn(), >+ source_dsa_invocation_id, >+ destination_dsa_guid) >+ >+ finally: >+ self._test_force_demote(self.dnsname_dc1, "RIDALLOCTEST9") >+ shutil.rmtree(targetdir, ignore_errors=True) >-- >2.25.1 > > >From 0726e60059613de1615c6f4fe8616e7d5795acce Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Thu, 9 Mar 2023 20:25:06 +1300 >Subject: [PATCH 3/3] dsdb: Avoid ERROR(ldb): uncaught exception - Deleted > target CN=NTDS Settings... in join > >"samba-tool domain join" uses the replication API in a strange way, perhaps no longer >required, except that we often still have folks upgrading from very old Samba versions. > >When deferring the writing out to the DB of link replication to the very end, there >is a greater opportunity for the deletion of an object to have been sent with the >other objects, and have the link applied later. > >This tells the repl_meta_data code to behave as if GET_TGT had been sent at the >time the link was returned, allowing a link to a deleted object to be silently >discarded. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15329 > >Signed-off-by: Andrew Bartlett <abartlet@samba.org> >Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz> >(cherry picked from commit bfc33b47bb428233e100f75e7a725ac52179f823) >--- > python/samba/join.py | 19 +++++++++++++++++++ > .../knownfail.d/replicate_against_deleted | 1 - > .../dsdb/samdb/ldb_modules/repl_meta_data.c | 13 ++++++++++++- > 3 files changed, 31 insertions(+), 2 deletions(-) > delete mode 100644 selftest/knownfail.d/replicate_against_deleted > >diff --git a/python/samba/join.py b/python/samba/join.py >index 650bb5a08ae..30d33d43f11 100644 >--- a/python/samba/join.py >+++ b/python/samba/join.py >@@ -50,6 +50,7 @@ import tempfile > from collections import OrderedDict > from samba.common import get_string > from samba.netcmd import CommandError >+from samba import dsdb > > > class DCJoinException(Exception): >@@ -937,6 +938,10 @@ class DCJoinContext(object): > """Replicate the SAM.""" > > ctx.logger.info("Starting replication") >+ >+ # A global transaction is started so that linked attributes >+ # are applied at the very end, once all partitions are >+ # replicated. This helps get all cross-partition links. > ctx.local_samdb.transaction_start() > try: > source_dsa_invocation_id = misc.GUID(ctx.samdb.get_invocation_id()) >@@ -1057,7 +1062,21 @@ class DCJoinContext(object): > ctx.local_samdb.transaction_cancel() > raise > else: >+ >+ # This is a special case, we have completed a full >+ # replication so if a link comes to us that points to a >+ # deleted object, and we asked for all objects already, we >+ # just have to ignore it, the chance to re-try the >+ # replication with GET_TGT has long gone. This can happen >+ # if the object is deleted and sent to us after the link >+ # was sent, as we are processing all links in the >+ # transaction_commit(). >+ if not ctx.domain_replica_flags & drsuapi.DRSUAPI_DRS_CRITICAL_ONLY: >+ ctx.local_samdb.set_opaque_integer(dsdb.DSDB_FULL_JOIN_REPLICATION_COMPLETED_OPAQUE_NAME, >+ 1) > ctx.local_samdb.transaction_commit() >+ ctx.local_samdb.set_opaque_integer(dsdb.DSDB_FULL_JOIN_REPLICATION_COMPLETED_OPAQUE_NAME, >+ 0) > ctx.logger.info("Committed SAM database") > > # A large replication may have caused our LDB connection to the >diff --git a/selftest/knownfail.d/replicate_against_deleted b/selftest/knownfail.d/replicate_against_deleted >deleted file mode 100644 >index 9caa5346a45..00000000000 >--- a/selftest/knownfail.d/replicate_against_deleted >+++ /dev/null >@@ -1 +0,0 @@ >-samba4.drs.ridalloc_exop.python\(.*\).ridalloc_exop.DrsReplicaSyncTestCase.test_replicate_against_deleted_objects_transaction >diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c >index c1ea5ad90f8..175a02d3ba7 100644 >--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c >+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c >@@ -7533,6 +7533,16 @@ static int replmd_allow_missing_target(struct ldb_module *module, > source_dn, > target_dn); > if (is_in_same_nc) { >+ /* >+ * We allow the join.py code to point out that all >+ * replication is completed, so failing now would just >+ * trigger errors, rather than trigger a GET_TGT >+ */ >+ int *finished_full_join_ptr = >+ talloc_get_type(ldb_get_opaque(ldb, >+ DSDB_FULL_JOIN_REPLICATION_COMPLETED_OPAQUE_NAME), >+ int); >+ bool finished_full_join = finished_full_join_ptr && *finished_full_join_ptr; > > /* > * if the target is already be up-to-date there's no point in >@@ -7540,7 +7550,8 @@ static int replmd_allow_missing_target(struct ldb_module *module, > * on a one-way link was deleted. We ignore the link rather > * than failing the replication cycle completely > */ >- if (dsdb_repl_flags & DSDB_REPL_FLAG_TARGETS_UPTODATE) { >+ if (finished_full_join >+ || dsdb_repl_flags & DSDB_REPL_FLAG_TARGETS_UPTODATE) { > *ignore_link = true; > DBG_WARNING("%s is %s " > "but up to date. Ignoring link from %s\n", >-- >2.25.1 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Flags:
jsutton
:
review+
Actions:
View
Attachments on
bug 15329
: 17827 |
17828