From 16526c6b7b01706f387e0ada141eeb1166250c1a Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 15 Dec 2022 12:05:55 +1300 Subject: [PATCH 01/21] s4-dsdb: Add tests of SamDB.get_nc_root() BUG: https://bugzilla.samba.org/show_bug.cgi?id=10635 Signed-off-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher (cherry picked from commit 2c7bb58703c1fa26782ac6959ea7d81fccf3905c) --- python/samba/tests/dsdb.py | 122 ++++++++++++++++++++++++++ selftest/knownfail.d/dsdb_get_nc_root | 10 +++ 2 files changed, 132 insertions(+) create mode 100644 selftest/knownfail.d/dsdb_get_nc_root diff --git a/python/samba/tests/dsdb.py b/python/samba/tests/dsdb.py index f4f7a705626..6c52994ece7 100644 --- a/python/samba/tests/dsdb.py +++ b/python/samba/tests/dsdb.py @@ -1029,6 +1029,128 @@ class DsdbTests(TestCase): str(part_dn) + "," + str(domain_dn)), self.samdb.normalize_dn_in_domain(part_dn)) +class DsdbNCRootTests(TestCase): + + def setUp(self): + super().setUp() + self.lp = samba.tests.env_loadparm() + self.creds = Credentials() + self.creds.guess(self.lp) + self.session = system_session() + self.samdb = SamDB(session_info=self.session, + credentials=self.creds, + lp=self.lp) + self.remote = False + + # These all use the local mode of operation inside + # dsdb_find_nc_root() using the partitions control + def test_dsdb_dn_nc_root_sid(self): + dom_sid = self.samdb.get_domain_sid() + domain_dn = ldb.Dn(self.samdb, self.samdb.domain_dn()) + dn = ldb.Dn(self.samdb, f"") + try: + nc_root = self.samdb.get_nc_root(dn) + except ldb.LdbError as e: + (code, msg) = e.args + self.fail("Got unexpected exception %d - %s " + % (code, msg)) + self.assertEqual(domain_dn, nc_root) + + def test_dsdb_dn_nc_root_admin_sid(self): + dom_sid = self.samdb.get_domain_sid() + domain_dn = ldb.Dn(self.samdb, self.samdb.domain_dn()) + dn = ldb.Dn(self.samdb, f"") + try: + nc_root = self.samdb.get_nc_root(dn) + except ldb.LdbError as e: + (code, msg) = e.args + self.fail("Got unexpected exception %d - %s " + % (code, msg)) + self.assertEqual(domain_dn, nc_root) + + def test_dsdb_dn_nc_root_users_container(self): + dom_sid = self.samdb.get_domain_sid() + domain_dn = ldb.Dn(self.samdb, self.samdb.domain_dn()) + dn = ldb.Dn(self.samdb, f"CN=Users,{domain_dn}") + try: + nc_root = self.samdb.get_nc_root(dn) + except ldb.LdbError as e: + (code, msg) = e.args + self.fail("Got unexpected exception %d - %s " + % (code, msg)) + self.assertEqual(domain_dn, nc_root) + + def test_dsdb_dn_nc_root_new_dn(self): + dom_sid = self.samdb.get_domain_sid() + domain_dn = ldb.Dn(self.samdb, self.samdb.domain_dn()) + dn = ldb.Dn(self.samdb, f"CN=Xnotexisting,CN=Users,{domain_dn}") + try: + nc_root = self.samdb.get_nc_root(dn) + except ldb.LdbError as e: + (code, msg) = e.args + self.fail("Got unexpected exception %d - %s " + % (code, msg)) + self.assertEqual(domain_dn, nc_root) + + def test_dsdb_dn_nc_root_new_dn_with_guid(self): + domain_dn = ldb.Dn(self.samdb, self.samdb.domain_dn()) + dn = ldb.Dn(self.samdb, f";CN=Xnotexisting,CN=Users,{domain_dn}") + try: + nc_root = self.samdb.get_nc_root(dn) + except ldb.LdbError as e: + (code, msg) = e.args + self.fail("Got unexpected exception %d - %s " + % (code, msg)) + self.assertEqual(domain_dn, nc_root) + + def test_dsdb_dn_nc_root_guid(self): + ntds_guid = self.samdb.get_ntds_GUID() + configuration_dn = self.samdb.get_config_basedn() + dn = ldb.Dn(self.samdb, f"") + try: + nc_root = self.samdb.get_nc_root(dn) + except ldb.LdbError as e: + (code, msg) = e.args + self.fail("Got unexpected exception %d - %s " + % (code, msg)) + self.assertEqual(configuration_dn, nc_root) + + def test_dsdb_dn_nc_root_misleading_to_noexisting_guid(self): + ntds_guid = self.samdb.get_ntds_GUID() + configuration_dn = self.samdb.get_config_basedn() + domain_dn = ldb.Dn(self.samdb, self.samdb.domain_dn()) + dn = ldb.Dn(self.samdb, f";CN=Xnotexisting,CN=Users,{domain_dn}") + try: + nc_root = self.samdb.get_nc_root(dn) + except ldb.LdbError as e: + (code, msg) = e.args + self.fail("Got unexpected exception %d - %s " + % (code, msg)) + self.assertEqual(configuration_dn, nc_root) + + def test_dsdb_dn_nc_root_misleading_to_existing_guid(self): + ntds_guid = self.samdb.get_ntds_GUID() + configuration_dn = self.samdb.get_config_basedn() + domain_dn = ldb.Dn(self.samdb, self.samdb.domain_dn()) + dn = ldb.Dn(self.samdb, f";{domain_dn}") + try: + nc_root = self.samdb.get_nc_root(dn) + except ldb.LdbError as e: + (code, msg) = e.args + self.fail("Got unexpected exception %d - %s " + % (code, msg)) + self.assertEqual(configuration_dn, nc_root) + +class DsdbRemoteNCRootTests(DsdbNCRootTests): + def setUp(self): + super().setUp() + # Reconnect to the remote LDAP port + self.samdb = SamDB(url="ldap://%s" % samba.tests.env_get_var_value('SERVER'), + session_info=self.session, + credentials=self.get_credentials(), + lp=self.lp) + self.remote = True + class DsdbFullScanTests(TestCase): diff --git a/selftest/knownfail.d/dsdb_get_nc_root b/selftest/knownfail.d/dsdb_get_nc_root new file mode 100644 index 00000000000..0a18996aa70 --- /dev/null +++ b/selftest/knownfail.d/dsdb_get_nc_root @@ -0,0 +1,10 @@ +^samba.tests.dsdb.samba.tests.dsdb.DsdbNCRootTests.test_dsdb_dn_nc_root_admin_sid +^samba.tests.dsdb.samba.tests.dsdb.DsdbNCRootTests.test_dsdb_dn_nc_root_guid +^samba.tests.dsdb.samba.tests.dsdb.DsdbNCRootTests.test_dsdb_dn_nc_root_misleading_to_existing_guid +^samba.tests.dsdb.samba.tests.dsdb.DsdbNCRootTests.test_dsdb_dn_nc_root_misleading_to_noexisting_guid +^samba.tests.dsdb.samba.tests.dsdb.DsdbNCRootTests.test_dsdb_dn_nc_root_sid +^samba.tests.dsdb.samba.tests.dsdb.DsdbRemoteNCRootTests.test_dsdb_dn_nc_root_admin_sid +^samba.tests.dsdb.samba.tests.dsdb.DsdbRemoteNCRootTests.test_dsdb_dn_nc_root_guid +^samba.tests.dsdb.samba.tests.dsdb.DsdbRemoteNCRootTests.test_dsdb_dn_nc_root_misleading_to_existing_guid +^samba.tests.dsdb.samba.tests.dsdb.DsdbRemoteNCRootTests.test_dsdb_dn_nc_root_misleading_to_noexisting_guid +^samba.tests.dsdb.samba.tests.dsdb.DsdbRemoteNCRootTests.test_dsdb_dn_nc_root_sid -- 2.25.1 From 0204bd63b694350a72d02e0697ca156509b01277 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Fri, 2 Dec 2022 10:07:53 +1300 Subject: [PATCH 02/21] s4-selftest/drs Add test of expected return code for invaid DNs in GetNCChanges BUG: https://bugzilla.samba.org/show_bug.cgi?id=10635 Signed-off-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher (cherry picked from commit bee45e6b29b97e0cab19a9c3cf692d9a7585a717) --- selftest/knownfail.d/getncchanges | 5 ++ source4/torture/drs/python/drs_base.py | 4 +- source4/torture/drs/python/getnc_exop.py | 82 +++++++++++++++++++++++- 3 files changed, 88 insertions(+), 3 deletions(-) diff --git a/selftest/knownfail.d/getncchanges b/selftest/knownfail.d/getncchanges index 5ef1bc98bef..b716ff83797 100644 --- a/selftest/knownfail.d/getncchanges +++ b/selftest/knownfail.d/getncchanges @@ -4,3 +4,8 @@ samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegri samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_repl_get_tgt_chain\(promoted_dc\) samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_repl_get_tgt_and_anc\(promoted_dc\) samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_repl_get_tgt_multivalued_links\(promoted_dc\) +# New tests for GetNCChanges with a GUID and a bad DN, like Azure AD Cloud Sync +^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_InvalidDestDSA_and_GUID +^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_InvalidNC_DummyDN_InvalidGUID +^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_InvalidNC_DummyDN_InvalidGUID_REPL_OBJ +^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_InvalidNC_DummyDN_InvalidGUID_RID_ALLOC diff --git a/source4/torture/drs/python/drs_base.py b/source4/torture/drs/python/drs_base.py index c5f7682d563..d87918456f1 100644 --- a/source4/torture/drs/python/drs_base.py +++ b/source4/torture/drs/python/drs_base.py @@ -432,13 +432,15 @@ class DrsBaseTestCase(SambaToolCmdTest): def _exop_req8(self, dest_dsa, invocation_id, nc_dn_str, exop, replica_flags=0, max_objects=0, partial_attribute_set=None, - partial_attribute_set_ex=None, mapping_ctr=None): + partial_attribute_set_ex=None, mapping_ctr=None, nc_guid=None): req8 = drsuapi.DsGetNCChangesRequest8() req8.destination_dsa_guid = misc.GUID(dest_dsa) if dest_dsa else misc.GUID() req8.source_dsa_invocation_id = misc.GUID(invocation_id) req8.naming_context = drsuapi.DsReplicaObjectIdentifier() req8.naming_context.dn = str(nc_dn_str) + if nc_guid is not None: + req8.naming_context.guid = nc_guid req8.highwatermark = drsuapi.DsReplicaHighWaterMark() req8.highwatermark.tmp_highest_usn = 0 req8.highwatermark.reserved_usn = 0 diff --git a/source4/torture/drs/python/getnc_exop.py b/source4/torture/drs/python/getnc_exop.py index 446c7821d54..8582eb17c66 100644 --- a/source4/torture/drs/python/getnc_exop.py +++ b/source4/torture/drs/python/getnc_exop.py @@ -240,6 +240,60 @@ class DrsReplicaSyncTestCase(drs_base.DrsBaseTestCase): (enum, estr) = e1.args self.assertEqual(enum, werror.WERR_DS_CANT_FIND_EXPECTED_NC) + def test_InvalidNC_DummyDN_InvalidGUID(self): + """Test full replication on a totally invalid GUID fails with the right error code""" + fsmo_dn = self.ldb_dc1.get_schema_basedn() + (fsmo_owner, fsmo_not_owner) = self._determine_fSMORoleOwner(fsmo_dn) + + req8 = self._exop_req8(dest_dsa="9c637462-5b8c-4467-aef2-bdb1f57bc4ef", + invocation_id=fsmo_owner["invocation_id"], + nc_dn_str="DummyDN", + nc_guid=misc.GUID("c2d2f745-1610-4e93-964b-d4ba73eb32f8"), + exop=drsuapi.DRSUAPI_EXOP_NONE) + + (drs, drs_handle) = self._ds_bind(fsmo_owner["dns_name"]) + try: + (level, ctr) = drs.DsGetNCChanges(drs_handle, 8, req8) + except WERRORError as e1: + (enum, estr) = e1.args + self.assertEqual(enum, werror.WERR_DS_DRA_BAD_NC) + + def test_InvalidNC_DummyDN_InvalidGUID_REPL_OBJ(self): + """Test single object replication on a totally invalid GUID fails with the right error code""" + fsmo_dn = self.ldb_dc1.get_schema_basedn() + (fsmo_owner, fsmo_not_owner) = self._determine_fSMORoleOwner(fsmo_dn) + + req8 = self._exop_req8(dest_dsa="9c637462-5b8c-4467-aef2-bdb1f57bc4ef", + invocation_id=fsmo_owner["invocation_id"], + nc_dn_str="DummyDN", + nc_guid=misc.GUID("c2d2f745-1610-4e93-964b-d4ba73eb32f8"), + exop=drsuapi.DRSUAPI_EXOP_REPL_OBJ) + + (drs, drs_handle) = self._ds_bind(fsmo_owner["dns_name"]) + try: + (level, ctr) = drs.DsGetNCChanges(drs_handle, 8, req8) + except WERRORError as e1: + (enum, estr) = e1.args + self.assertEqual(enum, werror.WERR_DS_DRA_BAD_DN) + + def test_InvalidNC_DummyDN_InvalidGUID_RID_ALLOC(self): + """Test RID Allocation on a totally invalid GUID fails with the right error code""" + fsmo_dn = self.ldb_dc1.get_schema_basedn() + (fsmo_owner, fsmo_not_owner) = self._determine_fSMORoleOwner(fsmo_dn) + + req8 = self._exop_req8(dest_dsa="9c637462-5b8c-4467-aef2-bdb1f57bc4ef", + invocation_id=fsmo_owner["invocation_id"], + nc_dn_str="DummyDN", + nc_guid=misc.GUID("c2d2f745-1610-4e93-964b-d4ba73eb32f8"), + exop=drsuapi.DRSUAPI_EXOP_FSMO_RID_ALLOC) + + (drs, drs_handle) = self._ds_bind(self.dnsname_dc1, ip=self.url_dc1) + try: + (level, ctr) = drs.DsGetNCChanges(drs_handle, 8, req8) + except WERRORError as e1: + (enum, estr) = e1.args + self.assertEqual(enum, werror.WERR_DS_DRA_BAD_NC) + def test_link_utdv_hwm(self): """Test verify the DRS_GET_ANC behavior.""" @@ -597,12 +651,35 @@ class DrsReplicaSyncTestCase(drs_base.DrsBaseTestCase): self.assertEqual(ctr.source_dsa_guid, misc.GUID(fsmo_owner["ntds_guid"])) self.assertEqual(ctr.source_dsa_invocation_id, misc.GUID(fsmo_owner["invocation_id"])) + def test_InvalidDestDSA_and_GUID(self): + """Test role transfer with invalid destination DSA guid""" + fsmo_dn = self.ldb_dc1.get_schema_basedn() + (fsmo_owner, fsmo_not_owner) = self._determine_fSMORoleOwner(fsmo_dn) + + req8 = self._exop_req8(dest_dsa="9c637462-5b8c-4467-aef2-bdb1f57bc4ef", + invocation_id=fsmo_owner["invocation_id"], + nc_dn_str="DummyDN", + nc_guid=misc.GUID("c2d2f745-1610-4e93-964b-d4ba73eb32f8"), + exop=drsuapi.DRSUAPI_EXOP_FSMO_REQ_ROLE) + + (drs, drs_handle) = self._ds_bind(fsmo_owner["dns_name"]) + try: + (level, ctr) = drs.DsGetNCChanges(drs_handle, 8, req8) + except WERRORError as e1: + (enum, estr) = e1.args + self.fail("DsGetNCChanges failed with {estr}") + self.assertEqual(level, 6, "Expected level 6 response!") + self._check_exop_failed(ctr, drsuapi.DRSUAPI_EXOP_ERR_UNKNOWN_CALLER) + self.assertEqual(ctr.source_dsa_guid, misc.GUID(fsmo_owner["ntds_guid"])) + self.assertEqual(ctr.source_dsa_invocation_id, misc.GUID(fsmo_owner["invocation_id"])) + class DrsReplicaPrefixMapTestCase(drs_base.DrsBaseTestCase): def setUp(self): super(DrsReplicaPrefixMapTestCase, self).setUp() self.base_dn = self.ldb_dc1.get_default_basedn() - self.ou = "ou=pfm_exop,%s" % self.base_dn + self.ou = "ou=pfm_exop%d,%s" % (random.randint(0, 4294967295), + self.base_dn) self.ldb_dc1.add({ "dn": self.ou, "objectclass": "organizationalUnit"}) @@ -948,7 +1025,8 @@ class DrsReplicaSyncSortTestCase(drs_base.DrsBaseTestCase): def setUp(self): super(DrsReplicaSyncSortTestCase, self).setUp() self.base_dn = self.ldb_dc1.get_default_basedn() - self.ou = "ou=sort_exop,%s" % self.base_dn + self.ou = "ou=sort_exop%d,%s" % (random.randint(0, 4294967295), + self.base_dn) self.ldb_dc1.add({ "dn": self.ou, "objectclass": "organizationalUnit"}) -- 2.25.1 From 658027403caa05b750ee721a430547c26af7a8c3 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Fri, 2 Dec 2022 11:42:55 +1300 Subject: [PATCH 03/21] s4-selftest/drs Allow some DRS tests to operate against an IP This is not comprehensive, but makes some manual test runs easier by avoiding the need for DNS names to resolve. BUG: https://bugzilla.samba.org/show_bug.cgi?id=10635 Signed-off-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher (cherry picked from commit a150a2dcb1fc7fc7f606838de17ad4d3e6072bda) --- source4/torture/drs/python/drs_base.py | 7 +++++-- source4/torture/drs/python/getnc_exop.py | 20 ++++++++++---------- source4/torture/drs/python/repl_move.py | 11 ++--------- source4/torture/drs/python/repl_schema.py | 9 +-------- 4 files changed, 18 insertions(+), 29 deletions(-) diff --git a/source4/torture/drs/python/drs_base.py b/source4/torture/drs/python/drs_base.py index d87918456f1..f072c8eb80c 100644 --- a/source4/torture/drs/python/drs_base.py +++ b/source4/torture/drs/python/drs_base.py @@ -492,8 +492,11 @@ class DrsBaseTestCase(SambaToolCmdTest): return req10 - def _ds_bind(self, server_name, creds=None): - binding_str = "ncacn_ip_tcp:%s[seal]" % server_name + def _ds_bind(self, server_name, creds=None, ip=None): + if ip is None: + binding_str = f"ncacn_ip_tcp:{server_name}[seal]" + else: + binding_str = f"ncacn_ip_tcp:{ip}[seal,target_hostname={server_name}]" if creds is None: creds = self.get_credentials() diff --git a/source4/torture/drs/python/getnc_exop.py b/source4/torture/drs/python/getnc_exop.py index 8582eb17c66..2d58790785f 100644 --- a/source4/torture/drs/python/getnc_exop.py +++ b/source4/torture/drs/python/getnc_exop.py @@ -85,7 +85,7 @@ class DrsReplicaSyncTestCase(drs_base.DrsBaseTestCase): self.ldb_dc1.add({ "dn": self.ou, "objectclass": "organizationalUnit"}) - (self.drs, self.drs_handle) = self._ds_bind(self.dnsname_dc1) + (self.drs, self.drs_handle) = self._ds_bind(self.dnsname_dc1, ip=self.url_dc1) (self.default_hwm, self.default_utdv) = self._get_highest_hwm_utdv(self.ldb_dc1) def tearDown(self): @@ -251,7 +251,7 @@ class DrsReplicaSyncTestCase(drs_base.DrsBaseTestCase): nc_guid=misc.GUID("c2d2f745-1610-4e93-964b-d4ba73eb32f8"), exop=drsuapi.DRSUAPI_EXOP_NONE) - (drs, drs_handle) = self._ds_bind(fsmo_owner["dns_name"]) + (drs, drs_handle) = self._ds_bind(self.dnsname_dc1, ip=self.url_dc1) try: (level, ctr) = drs.DsGetNCChanges(drs_handle, 8, req8) except WERRORError as e1: @@ -269,7 +269,7 @@ class DrsReplicaSyncTestCase(drs_base.DrsBaseTestCase): nc_guid=misc.GUID("c2d2f745-1610-4e93-964b-d4ba73eb32f8"), exop=drsuapi.DRSUAPI_EXOP_REPL_OBJ) - (drs, drs_handle) = self._ds_bind(fsmo_owner["dns_name"]) + (drs, drs_handle) = self._ds_bind(self.dnsname_dc1, ip=self.url_dc1) try: (level, ctr) = drs.DsGetNCChanges(drs_handle, 8, req8) except WERRORError as e1: @@ -702,7 +702,7 @@ class DrsReplicaPrefixMapTestCase(drs_base.DrsBaseTestCase): dc_guid_1 = self.ldb_dc1.get_invocation_id() - drs, drs_handle = self._ds_bind(self.dnsname_dc1) + drs, drs_handle = self._ds_bind(self.dnsname_dc1, ip=self.url_dc1) req8 = self._exop_req8(dest_dsa=None, invocation_id=dc_guid_1, @@ -721,7 +721,7 @@ class DrsReplicaPrefixMapTestCase(drs_base.DrsBaseTestCase): partial_attribute_set = self.get_partial_attribute_set([99999]) dc_guid_1 = self.ldb_dc1.get_invocation_id() - drs, drs_handle = self._ds_bind(self.dnsname_dc1) + drs, drs_handle = self._ds_bind(self.dnsname_dc1, ip=self.url_dc1) try: pfm = self._samdb_fetch_pfm_and_schi() @@ -754,7 +754,7 @@ class DrsReplicaPrefixMapTestCase(drs_base.DrsBaseTestCase): partial_attribute_set = self.get_partial_attribute_set([drsuapi.DRSUAPI_ATTID_unicodePwd]) dc_guid_1 = self.ldb_dc1.get_invocation_id() - drs, drs_handle = self._ds_bind(self.dnsname_dc1) + drs, drs_handle = self._ds_bind(self.dnsname_dc1, ip=self.url_dc1) try: pfm = self._samdb_fetch_pfm_and_schi() @@ -828,7 +828,7 @@ class DrsReplicaPrefixMapTestCase(drs_base.DrsBaseTestCase): partial_attribute_set = self.get_partial_attribute_set([drsuapi.DRSUAPI_ATTID_name]) dc_guid_1 = self.ldb_dc1.get_invocation_id() - drs, drs_handle = self._ds_bind(self.dnsname_dc1) + drs, drs_handle = self._ds_bind(self.dnsname_dc1, ip=self.url_dc1) try: pfm = self._samdb_fetch_pfm_and_schi() @@ -901,7 +901,7 @@ class DrsReplicaPrefixMapTestCase(drs_base.DrsBaseTestCase): partial_attribute_set_ex = self.get_partial_attribute_set([drsuapi.DRSUAPI_ATTID_unicodePwd]) dc_guid_1 = self.ldb_dc1.get_invocation_id() - drs, drs_handle = self._ds_bind(self.dnsname_dc1) + drs, drs_handle = self._ds_bind(self.dnsname_dc1, ip=self.url_dc1) try: pfm = self._samdb_fetch_pfm_and_schi() @@ -1117,7 +1117,7 @@ class DrsReplicaSyncSortTestCase(drs_base.DrsBaseTestCase): dc_guid_1 = self.ldb_dc1.get_invocation_id() - drs, drs_handle = self._ds_bind(self.dnsname_dc1) + drs, drs_handle = self._ds_bind(self.dnsname_dc1, ip=self.url_dc1) req8 = self._exop_req8(dest_dsa=None, invocation_id=dc_guid_1, @@ -1177,7 +1177,7 @@ class DrsReplicaSyncSortTestCase(drs_base.DrsBaseTestCase): dc_guid_1 = self.ldb_dc1.get_invocation_id() - drs, drs_handle = self._ds_bind(self.dnsname_dc1) + drs, drs_handle = self._ds_bind(self.dnsname_dc1, ip=self.url_dc1) # Make sure the max objects count is high enough req8 = self._exop_req8(dest_dsa=None, diff --git a/source4/torture/drs/python/repl_move.py b/source4/torture/drs/python/repl_move.py index 19389cdb8af..3827c7cb39f 100644 --- a/source4/torture/drs/python/repl_move.py +++ b/source4/torture/drs/python/repl_move.py @@ -48,13 +48,6 @@ from samba.dcerpc.drsuapi import * class DrsMoveObjectTestCase(drs_base.DrsBaseTestCase): - def _ds_bind(self, server_name): - binding_str = "ncacn_ip_tcp:%s[print,seal]" % server_name - - drs = drsuapi(binding_str, self.get_loadparm(), self.get_credentials()) - (drs_handle, supported_extensions) = drs_DsBind(drs) - return (drs, drs_handle) - def setUp(self): super(DrsMoveObjectTestCase, self).setUp() # disable automatic replication temporary @@ -89,8 +82,8 @@ class DrsMoveObjectTestCase(drs_base.DrsBaseTestCase): self.dc1_guid = self.ldb_dc1.get_invocation_id() self.dc2_guid = self.ldb_dc2.get_invocation_id() - self.drs_dc1 = self._ds_bind(self.dnsname_dc1) - self.drs_dc2 = self._ds_bind(self.dnsname_dc2) + self.drs_dc1 = self._ds_bind(self.dnsname_dc1, ip=self.url_dc1) + self.drs_dc2 = self._ds_bind(self.dnsname_dc2, ip=self.url_dc2) def tearDown(self): try: diff --git a/source4/torture/drs/python/repl_schema.py b/source4/torture/drs/python/repl_schema.py index b46a87af51e..44317ac8819 100644 --- a/source4/torture/drs/python/repl_schema.py +++ b/source4/torture/drs/python/repl_schema.py @@ -52,13 +52,6 @@ class DrsReplSchemaTestCase(drs_base.DrsBaseTestCase): # current Class or Attribute object id obj_id = 0 - def _ds_bind(self, server_name): - binding_str = "ncacn_ip_tcp:%s[seal]" % server_name - - drs = drsuapi.drsuapi(binding_str, self.get_loadparm(), self.get_credentials()) - (drs_handle, supported_extensions) = drs_DsBind(drs) - return (drs, drs_handle) - def _exop_req8(self, dest_dsa, invocation_id, nc_dn_str, exop, replica_flags=0, max_objects=0): req8 = drsuapi.DsGetNCChangesRequest8() @@ -281,7 +274,7 @@ class DrsReplSchemaTestCase(drs_base.DrsBaseTestCase): dc_guid_1 = self.ldb_dc1.get_invocation_id() - drs, drs_handle = self._ds_bind(self.dnsname_dc1) + drs, drs_handle = self._ds_bind(self.dnsname_dc1, ip=self.url_dc1) req8 = self._exop_req8(dest_dsa=None, invocation_id=dc_guid_1, -- 2.25.1 From eefd75d6ecdfbef999e80a02e088d562e73fe6e7 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Fri, 2 Dec 2022 11:56:38 +1300 Subject: [PATCH 04/21] s4-selftest/drs Allow re-run of DRS tests after failed cleanup Using a random base is a useful start, even if the better solution also includes a self.AddCleanup() BUG: https://bugzilla.samba.org/show_bug.cgi?id=10635 Signed-off-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher (cherry picked from commit 3204d1350b21704474e577cb5f3f2439b673c421) --- source4/torture/drs/python/getnc_exop.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/source4/torture/drs/python/getnc_exop.py b/source4/torture/drs/python/getnc_exop.py index 2d58790785f..885156cc6e2 100644 --- a/source4/torture/drs/python/getnc_exop.py +++ b/source4/torture/drs/python/getnc_exop.py @@ -81,7 +81,8 @@ class DrsReplicaSyncTestCase(drs_base.DrsBaseTestCase): def setUp(self): super(DrsReplicaSyncTestCase, self).setUp() self.base_dn = self.ldb_dc1.get_default_basedn() - self.ou = "OU=test_getncchanges,%s" % self.base_dn + self.ou = "OU=test_getncchanges%d,%s" % (random.randint(0, 4294967295), + self.base_dn) self.ldb_dc1.add({ "dn": self.ou, "objectclass": "organizationalUnit"}) -- 2.25.1 From bd9d5d54ec75e1bf09f616b4d6af22f1aca49ea7 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Fri, 2 Dec 2022 15:30:05 +1300 Subject: [PATCH 05/21] s4-selftest/drs: Confirm GetNCChanges REPL_OBJ works with a DummyDN and real GUID BUG: https://bugzilla.samba.org/show_bug.cgi?id=10635 Signed-off-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher (cherry picked from commit 70faccae6d595056174af8d63b3437c9fe3805aa) --- selftest/knownfail.d/getncchanges | 3 + source4/torture/drs/python/getnc_exop.py | 80 ++++++++++++++++++++++-- 2 files changed, 77 insertions(+), 6 deletions(-) diff --git a/selftest/knownfail.d/getncchanges b/selftest/knownfail.d/getncchanges index b716ff83797..cb6c8c630e3 100644 --- a/selftest/knownfail.d/getncchanges +++ b/selftest/knownfail.d/getncchanges @@ -9,3 +9,6 @@ samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegri ^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_InvalidNC_DummyDN_InvalidGUID ^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_InvalidNC_DummyDN_InvalidGUID_REPL_OBJ ^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_InvalidNC_DummyDN_InvalidGUID_RID_ALLOC +^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_InvalidDestDSA_and_GUID_RID_ALLOC +^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_DummyDN_valid_GUID_REPL_OBJ +^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_DummyDN_valid_GUID_REPL_SECRET diff --git a/source4/torture/drs/python/getnc_exop.py b/source4/torture/drs/python/getnc_exop.py index 885156cc6e2..9c16ecca323 100644 --- a/source4/torture/drs/python/getnc_exop.py +++ b/source4/torture/drs/python/getnc_exop.py @@ -241,8 +241,8 @@ class DrsReplicaSyncTestCase(drs_base.DrsBaseTestCase): (enum, estr) = e1.args self.assertEqual(enum, werror.WERR_DS_CANT_FIND_EXPECTED_NC) - def test_InvalidNC_DummyDN_InvalidGUID(self): - """Test full replication on a totally invalid GUID fails with the right error code""" + def test_InvalidNC_DummyDN_InvalidGUID_REPL_OBJ(self): + """Test single object replication on a totally invalid GUID fails with the right error code""" fsmo_dn = self.ldb_dc1.get_schema_basedn() (fsmo_owner, fsmo_not_owner) = self._determine_fSMORoleOwner(fsmo_dn) @@ -250,16 +250,16 @@ class DrsReplicaSyncTestCase(drs_base.DrsBaseTestCase): invocation_id=fsmo_owner["invocation_id"], nc_dn_str="DummyDN", nc_guid=misc.GUID("c2d2f745-1610-4e93-964b-d4ba73eb32f8"), - exop=drsuapi.DRSUAPI_EXOP_NONE) + exop=drsuapi.DRSUAPI_EXOP_REPL_OBJ) (drs, drs_handle) = self._ds_bind(self.dnsname_dc1, ip=self.url_dc1) try: (level, ctr) = drs.DsGetNCChanges(drs_handle, 8, req8) except WERRORError as e1: (enum, estr) = e1.args - self.assertEqual(enum, werror.WERR_DS_DRA_BAD_NC) + self.assertEqual(enum, werror.WERR_DS_DRA_BAD_DN) - def test_InvalidNC_DummyDN_InvalidGUID_REPL_OBJ(self): + def test_InvalidNC_DummyDN_InvalidGUID_REPL_SECRET(self): """Test single object replication on a totally invalid GUID fails with the right error code""" fsmo_dn = self.ldb_dc1.get_schema_basedn() (fsmo_owner, fsmo_not_owner) = self._determine_fSMORoleOwner(fsmo_dn) @@ -295,6 +295,52 @@ class DrsReplicaSyncTestCase(drs_base.DrsBaseTestCase): (enum, estr) = e1.args self.assertEqual(enum, werror.WERR_DS_DRA_BAD_NC) + def test_DummyDN_valid_GUID_REPL_OBJ(self): + dc_guid_1 = self.ldb_dc1.get_invocation_id() + drs, drs_handle = self._ds_bind(self.dnsname_dc1, ip=self.url_dc1) + + res = self.ldb_dc1.search(base=self.ou, scope=SCOPE_BASE, + attrs=["objectGUID"]) + + guid = misc.GUID(res[0]["objectGUID"][0]) + + req8 = self._exop_req8(dest_dsa=None, + invocation_id=dc_guid_1, + nc_dn_str="DummyDN", + nc_guid=guid, + exop=drsuapi.DRSUAPI_EXOP_REPL_OBJ) + + try: + (level, ctr) = drs.DsGetNCChanges(drs_handle, 8, req8) + except WERRORError as e1: + (enum, estr) = e1.args + self.fail(f"Failed to call GetNCChanges with EXOP_REPL_OBJ and a GUID: {estr}") + + self.assertEqual(ctr.first_object.object.identifier.guid, guid) + + def test_DummyDN_valid_GUID_REPL_SECRET(self): + dc_guid_1 = self.ldb_dc1.get_invocation_id() + drs, drs_handle = self._ds_bind(self.dnsname_dc1, ip=self.url_dc1) + + res = self.ldb_dc1.search(base=self.ou, scope=SCOPE_BASE, + attrs=["objectGUID"]) + + guid = misc.GUID(res[0]["objectGUID"][0]) + + req8 = self._exop_req8(dest_dsa=None, + invocation_id=dc_guid_1, + nc_dn_str="DummyDN", + nc_guid=guid, + exop=drsuapi.DRSUAPI_EXOP_REPL_SECRET) + + try: + (level, ctr) = drs.DsGetNCChanges(drs_handle, 8, req8) + except WERRORError as e1: + (enum, estr) = e1.args + + # We expect to get as far as failing on the missing dest_dsa + self.assertEqual(enum, werror.WERR_DS_DRA_DB_ERROR) + def test_link_utdv_hwm(self): """Test verify the DRS_GET_ANC behavior.""" @@ -668,7 +714,29 @@ class DrsReplicaSyncTestCase(drs_base.DrsBaseTestCase): (level, ctr) = drs.DsGetNCChanges(drs_handle, 8, req8) except WERRORError as e1: (enum, estr) = e1.args - self.fail("DsGetNCChanges failed with {estr}") + self.fail(f"DsGetNCChanges failed with {estr}") + self.assertEqual(level, 6, "Expected level 6 response!") + self._check_exop_failed(ctr, drsuapi.DRSUAPI_EXOP_ERR_UNKNOWN_CALLER) + self.assertEqual(ctr.source_dsa_guid, misc.GUID(fsmo_owner["ntds_guid"])) + self.assertEqual(ctr.source_dsa_invocation_id, misc.GUID(fsmo_owner["invocation_id"])) + + def test_InvalidDestDSA_and_GUID_RID_ALLOC(self): + """Test role transfer with invalid destination DSA guid""" + fsmo_dn = self.ldb_dc1.get_schema_basedn() + (fsmo_owner, fsmo_not_owner) = self._determine_fSMORoleOwner(fsmo_dn) + + req8 = self._exop_req8(dest_dsa="9c637462-5b8c-4467-aef2-bdb1f57bc4ef", + invocation_id=fsmo_owner["invocation_id"], + nc_dn_str="DummyDN", + nc_guid=misc.GUID("c2d2f745-1610-4e93-964b-d4ba73eb32f8"), + exop=drsuapi.DRSUAPI_EXOP_FSMO_RID_ALLOC) + + (drs, drs_handle) = self._ds_bind(fsmo_owner["dns_name"]) + try: + (level, ctr) = drs.DsGetNCChanges(drs_handle, 8, req8) + except WERRORError as e1: + (enum, estr) = e1.args + self.fail(f"DsGetNCChanges failed with {estr}") self.assertEqual(level, 6, "Expected level 6 response!") self._check_exop_failed(ctr, drsuapi.DRSUAPI_EXOP_ERR_UNKNOWN_CALLER) self.assertEqual(ctr.source_dsa_guid, misc.GUID(fsmo_owner["ntds_guid"])) -- 2.25.1 From 58e6b4a678b1921df38096455bd0b377431b3f7f Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 15 Dec 2022 16:02:27 +1300 Subject: [PATCH 06/21] s4-selftest/drs: Confirm GetNCChanges full replication works with a DummyDN and real GUID BUG: https://bugzilla.samba.org/show_bug.cgi?id=10635 Signed-off-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher (cherry picked from commit 539221dda33f03a1abf5ee5f3153db0fe1a9bfe6) --- selftest/knownfail.d/getncchanges | 2 + source4/torture/drs/python/getncchanges.py | 52 +++++++++++++++++++++- 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/selftest/knownfail.d/getncchanges b/selftest/knownfail.d/getncchanges index cb6c8c630e3..62329aa877a 100644 --- a/selftest/knownfail.d/getncchanges +++ b/selftest/knownfail.d/getncchanges @@ -12,3 +12,5 @@ samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegri ^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_InvalidDestDSA_and_GUID_RID_ALLOC ^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_DummyDN_valid_GUID_REPL_OBJ ^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_DummyDN_valid_GUID_REPL_SECRET +^samba4.drs.getncchanges.python\(.*\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_DummyDN_valid_GUID_full_repl +^samba4.drs.getncchanges.python\(.*\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_InvalidNC_DummyDN_InvalidGUID_full_repl diff --git a/source4/torture/drs/python/getncchanges.py b/source4/torture/drs/python/getncchanges.py index a35e86088dc..1d25c58c51d 100644 --- a/source4/torture/drs/python/getncchanges.py +++ b/source4/torture/drs/python/getncchanges.py @@ -34,8 +34,9 @@ import ldb from ldb import SCOPE_BASE import random -from samba.dcerpc import drsuapi - +from samba.dcerpc import drsuapi, misc +from samba import WERRORError +from samba import werror class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase): def setUp(self): @@ -1173,6 +1174,53 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase): num_expected=500) + def test_InvalidNC_DummyDN_InvalidGUID_full_repl(self): + """Test full replication on a totally invalid GUID fails with the right error code""" + dc_guid_1 = self.ldb_dc1.get_invocation_id() + drs, drs_handle = self._ds_bind(self.dnsname_dc1, ip=self.url_dc1) + + req8 = self._exop_req8(dest_dsa="9c637462-5b8c-4467-aef2-bdb1f57bc4ef", + invocation_id=dc_guid_1, + nc_dn_str="DummyDN", + nc_guid=misc.GUID("c2d2f745-1610-4e93-964b-d4ba73eb32f8"), + exop=drsuapi.DRSUAPI_EXOP_NONE, + max_objects=1) + + (drs, drs_handle) = self._ds_bind(self.dnsname_dc1, ip=self.url_dc1) + try: + (level, ctr) = drs.DsGetNCChanges(drs_handle, 8, req8) + except WERRORError as e1: + (enum, estr) = e1.args + self.assertEqual(enum, werror.WERR_DS_DRA_BAD_NC) + + def test_DummyDN_valid_GUID_full_repl(self): + dc_guid_1 = self.ldb_dc1.get_invocation_id() + drs, drs_handle = self._ds_bind(self.dnsname_dc1, ip=self.url_dc1) + + res = self.ldb_dc1.search(base=self.base_dn, scope=SCOPE_BASE, + attrs=["objectGUID"]) + + guid = misc.GUID(res[0]["objectGUID"][0]) + + req8 = self._exop_req8(dest_dsa=None, + invocation_id=dc_guid_1, + nc_dn_str="DummyDN", + nc_guid=guid, + replica_flags=drsuapi.DRSUAPI_DRS_WRIT_REP | + drsuapi.DRSUAPI_DRS_GET_ANC, + exop=drsuapi.DRSUAPI_EXOP_NONE, + max_objects=1) + + try: + (level, ctr) = drs.DsGetNCChanges(drs_handle, 8, req8) + except WERRORError as e1: + (enum, estr) = e1.args + self.fail(f"Failed to call GetNCChanges with DummyDN and a GUID: {estr}") + + # The NC should be the first object returned due to GET_ANC + self.assertEqual(ctr.first_object.object.identifier.guid, guid) + + class DcConnection: """Helper class to track a connection to another DC""" -- 2.25.1 From 454bde200343952cdc7ae3518399d9fa4dfe87b2 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 15 Dec 2022 16:02:55 +1300 Subject: [PATCH 07/21] s4-selftest/drs: Confirm GetNCChanges REPL_SECRET works with a DummyDN and real GUID BUG: https://bugzilla.samba.org/show_bug.cgi?id=10635 Signed-off-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher (cherry picked from commit 7c43388576f768db564aaf15a47d3f9ce5796fb3) --- selftest/knownfail.d/getncchanges | 1 + source4/torture/drs/python/drs_base.py | 4 ++- source4/torture/drs/python/repl_rodc.py | 46 +++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 1 deletion(-) diff --git a/selftest/knownfail.d/getncchanges b/selftest/knownfail.d/getncchanges index 62329aa877a..317d78c41b1 100644 --- a/selftest/knownfail.d/getncchanges +++ b/selftest/knownfail.d/getncchanges @@ -14,3 +14,4 @@ samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegri ^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_DummyDN_valid_GUID_REPL_SECRET ^samba4.drs.getncchanges.python\(.*\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_DummyDN_valid_GUID_full_repl ^samba4.drs.getncchanges.python\(.*\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_InvalidNC_DummyDN_InvalidGUID_full_repl +^samba4.drs.repl_rodc.python\(.*\).repl_rodc.DrsRodcTestCase.test_admin_repl_secrets_DummyDN_GUID diff --git a/source4/torture/drs/python/drs_base.py b/source4/torture/drs/python/drs_base.py index f072c8eb80c..db7a87a33fe 100644 --- a/source4/torture/drs/python/drs_base.py +++ b/source4/torture/drs/python/drs_base.py @@ -464,13 +464,15 @@ class DrsBaseTestCase(SambaToolCmdTest): def _getnc_req10(self, dest_dsa, invocation_id, nc_dn_str, exop, replica_flags=0, max_objects=0, partial_attribute_set=None, partial_attribute_set_ex=None, mapping_ctr=None, - more_flags=0): + more_flags=0, nc_guid=None): req10 = drsuapi.DsGetNCChangesRequest10() req10.destination_dsa_guid = misc.GUID(dest_dsa) if dest_dsa else misc.GUID() req10.source_dsa_invocation_id = misc.GUID(invocation_id) req10.naming_context = drsuapi.DsReplicaObjectIdentifier() req10.naming_context.dn = str(nc_dn_str) + if nc_guid is not None: + req10.naming_context.guid = nc_guid req10.highwatermark = drsuapi.DsReplicaHighWaterMark() req10.highwatermark.tmp_highest_usn = 0 req10.highwatermark.reserved_usn = 0 diff --git a/source4/torture/drs/python/repl_rodc.py b/source4/torture/drs/python/repl_rodc.py index 21e70b8bc6f..8a4577499b8 100644 --- a/source4/torture/drs/python/repl_rodc.py +++ b/source4/torture/drs/python/repl_rodc.py @@ -161,6 +161,52 @@ class DrsRodcTestCase(drs_base.DrsBaseTestCase): # Check that the user has been added to msDSRevealedUsers self._assert_in_revealed_users(user_dn, expected_user_attributes) + def test_admin_repl_secrets_DummyDN_GUID(self): + """ + When a secret attribute is set to be replicated to an RODC with the + admin credentials, it should always replicate regardless of whether + or not it's in the Allowed RODC Password Replication Group. + """ + rand = random.randint(1, 10000000) + expected_user_attributes = [drsuapi.DRSUAPI_ATTID_lmPwdHistory, + drsuapi.DRSUAPI_ATTID_supplementalCredentials, + drsuapi.DRSUAPI_ATTID_ntPwdHistory, + drsuapi.DRSUAPI_ATTID_unicodePwd, + drsuapi.DRSUAPI_ATTID_dBCSPwd] + + user_name = "test_rodcA_%s" % rand + user_dn = "CN=%s,%s" % (user_name, self.ou) + self.ldb_dc1.add({ + "dn": user_dn, + "objectclass": "user", + "sAMAccountName": user_name + }) + + res = self.ldb_dc1.search(base=user_dn, scope=ldb.SCOPE_BASE, + attrs=["objectGUID"]) + + user_guid = misc.GUID(res[0]["objectGUID"][0]) + + # Store some secret on this user + self.ldb_dc1.setpassword("(sAMAccountName=%s)" % user_name, 'penguin12#', False, user_name) + + req10 = self._getnc_req10(dest_dsa=str(self.rodc_ctx.ntds_guid), + invocation_id=self.ldb_dc1.get_invocation_id(), + nc_dn_str="DummyDN", + nc_guid=user_guid, + exop=drsuapi.DRSUAPI_EXOP_REPL_SECRET, + partial_attribute_set=drs_get_rodc_partial_attribute_set(self.ldb_dc1, self.tmp_samdb), + max_objects=133, + replica_flags=0) + try: + (level, ctr) = self.drs.DsGetNCChanges(self.drs_handle, 10, req10) + except WERRORError as e1: + (enum, estr) = e1.args + self.fail(f"DsGetNCChanges failed with {estr}") + + # Check that the user has been added to msDSRevealedUsers + self._assert_in_revealed_users(user_dn, expected_user_attributes) + def test_rodc_repl_secrets(self): """ When a secret attribute is set to be replicated to an RODC with -- 2.25.1 From b1a9b55f525451e3b31b35c758e6a6de8bf5d335 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 26 Jan 2023 09:44:01 +1300 Subject: [PATCH 08/21] s4-dsdb: Schedule SD propegation only after successful rename This avoids needing to anticipate errors that the rename might give while allowing the dsdb_find_nc_root() routine to become stricter. The problem is that dsdb_find_nc_root() will soon do a real search and so fail more often, but these failures will give "wrong" error codes. We do not need to do this work if the operation fails, so put this in the callback. BUG: https://bugzilla.samba.org/show_bug.cgi?id=10635 Signed-off-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher (cherry picked from commit 7032b86cd5c1456318558ed95f8890e353117ced) --- source4/dsdb/samdb/ldb_modules/descriptor.c | 134 ++++++++++++++------ 1 file changed, 95 insertions(+), 39 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/descriptor.c b/source4/dsdb/samdb/ldb_modules/descriptor.c index aef2e284dc7..bf0f8ec1849 100644 --- a/source4/dsdb/samdb/ldb_modules/descriptor.c +++ b/source4/dsdb/samdb/ldb_modules/descriptor.c @@ -1012,11 +1012,88 @@ static int descriptor_search(struct ldb_module *module, struct ldb_request *req) return ldb_next_request(ac->module, down_req); } +static int descriptor_rename_callback(struct ldb_request *req, + struct ldb_reply *ares) +{ + struct descriptor_context *ac = NULL; + struct ldb_context *ldb = NULL; + struct ldb_dn *newdn = req->op.rename.newdn; + struct GUID guid; + struct ldb_dn *nc_root; + struct GUID parent_guid = { .time_low = 0 }; + int ret; + + ac = talloc_get_type_abort(req->context, struct descriptor_context); + ldb = ldb_module_get_ctx(ac->module); + + if (!ares) { + return ldb_module_done(ac->req, NULL, NULL, + LDB_ERR_OPERATIONS_ERROR); + } + if (ares->error != LDB_SUCCESS) { + return ldb_module_done(ac->req, ares->controls, + ares->response, ares->error); + } + + if (ares->type != LDB_REPLY_DONE) { + return ldb_module_done(ac->req, NULL, NULL, + LDB_ERR_OPERATIONS_ERROR); + } + + ret = dsdb_module_guid_by_dn(ac->module, + newdn, + &guid, + req); + if (ret != LDB_SUCCESS) { + return ldb_module_done(ac->req, NULL, NULL, + ret); + } + ret = dsdb_find_nc_root(ldb, req, newdn, &nc_root); + if (ret != LDB_SUCCESS) { + return ldb_module_done(ac->req, NULL, NULL, + ret); + } + + /* + * After a successful rename, force SD propagation on this + * record (get a new inherited SD from the potentially new + * parent + * + * We don't know the parent guid here (it is filled in as + * all-zero in the initialiser above), but we're not in a hot + * code path here, as the "descriptor" module is located above + * the "repl_meta_data", only originating changes are handled + * here. + * + * If it turns out to be a problem we may search for the new + * parent guid. + */ + + ret = dsdb_module_schedule_sd_propagation(ac->module, + nc_root, + guid, + parent_guid, + true); + if (ret != LDB_SUCCESS) { + ret = ldb_operr(ldb); + return ldb_module_done(ac->req, NULL, NULL, + ret); + } + + return ldb_module_done(ac->req, ares->controls, + ares->response, ares->error); +} + + + + static int descriptor_rename(struct ldb_module *module, struct ldb_request *req) { + struct descriptor_context *ac = NULL; struct ldb_context *ldb = ldb_module_get_ctx(module); struct ldb_dn *olddn = req->op.rename.olddn; struct ldb_dn *newdn = req->op.rename.newdn; + struct ldb_request *down_req; int ret; /* do not manipulate our control entries */ @@ -1027,49 +1104,28 @@ static int descriptor_rename(struct ldb_module *module, struct ldb_request *req) ldb_debug(ldb, LDB_DEBUG_TRACE,"descriptor_rename: %s\n", ldb_dn_get_linearized(olddn)); - if (ldb_dn_compare(olddn, newdn) != 0) { - struct ldb_dn *nc_root; - struct GUID guid; - - ret = dsdb_find_nc_root(ldb, req, newdn, &nc_root); - if (ret != LDB_SUCCESS) { - return ldb_oom(ldb); - } + if (ldb_dn_compare(olddn, newdn) == 0) { + /* No special work required for a case-only rename */ + return ldb_next_request(module, req); + } - ret = dsdb_module_guid_by_dn(module, - olddn, - &guid, - req); - if (ret == LDB_SUCCESS) { - /* - * Without disturbing any errors if the olddn - * does not exit, force SD propagation on - * this record (get a new inherited SD from - * the potentially new parent - * - * We don't now the parent guid here, - * but we're not in a hot code path here, - * as the "descriptor" module is located - * above the "repl_meta_data", only - * originating changes are handled here. - * - * If it turns out to be a problem we may - * search for the new parent guid. - */ - struct GUID parent_guid = { .time_low = 0 }; + ac = descriptor_init_context(module, req); + if (ac == NULL) { + return ldb_operr(ldb); + } - ret = dsdb_module_schedule_sd_propagation(module, - nc_root, - guid, - parent_guid, - true); - if (ret != LDB_SUCCESS) { - return ldb_operr(ldb); - } - } + ret = ldb_build_rename_req(&down_req, ldb, ac, + req->op.rename.olddn, + req->op.rename.newdn, + req->controls, + ac, descriptor_rename_callback, + req); + LDB_REQ_SET_LOCATION(down_req); + if (ret != LDB_SUCCESS) { + return ret; } - return ldb_next_request(module, req); + return ldb_next_request(module, down_req); } static void descriptor_changes_parser(TDB_DATA key, TDB_DATA data, void *private_data) -- 2.25.1 From 85c859579c2d55dff92c18c08b824a77ad692e2b Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Mon, 5 Dec 2022 22:21:29 +1300 Subject: [PATCH 09/21] s4-dsdb: Make dsdb_find_nc_root() first try and use DSDB_CONTROL_CURRENT_PARTITION_OID This allows lookup of a DN with a GUID only or GUID and string, possibly not yet in the database, yet still getting the correct result. BUG: https://bugzilla.samba.org/show_bug.cgi?id=10635 Signed-off-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher (cherry picked from commit d0444be4b74bdad6a731bc5fcf86da6142b03539) --- selftest/knownfail.d/dsdb_get_nc_root | 10 -- source4/dsdb/common/util.c | 237 +++++++++++++++++++++++++- 2 files changed, 234 insertions(+), 13 deletions(-) delete mode 100644 selftest/knownfail.d/dsdb_get_nc_root diff --git a/selftest/knownfail.d/dsdb_get_nc_root b/selftest/knownfail.d/dsdb_get_nc_root deleted file mode 100644 index 0a18996aa70..00000000000 --- a/selftest/knownfail.d/dsdb_get_nc_root +++ /dev/null @@ -1,10 +0,0 @@ -^samba.tests.dsdb.samba.tests.dsdb.DsdbNCRootTests.test_dsdb_dn_nc_root_admin_sid -^samba.tests.dsdb.samba.tests.dsdb.DsdbNCRootTests.test_dsdb_dn_nc_root_guid -^samba.tests.dsdb.samba.tests.dsdb.DsdbNCRootTests.test_dsdb_dn_nc_root_misleading_to_existing_guid -^samba.tests.dsdb.samba.tests.dsdb.DsdbNCRootTests.test_dsdb_dn_nc_root_misleading_to_noexisting_guid -^samba.tests.dsdb.samba.tests.dsdb.DsdbNCRootTests.test_dsdb_dn_nc_root_sid -^samba.tests.dsdb.samba.tests.dsdb.DsdbRemoteNCRootTests.test_dsdb_dn_nc_root_admin_sid -^samba.tests.dsdb.samba.tests.dsdb.DsdbRemoteNCRootTests.test_dsdb_dn_nc_root_guid -^samba.tests.dsdb.samba.tests.dsdb.DsdbRemoteNCRootTests.test_dsdb_dn_nc_root_misleading_to_existing_guid -^samba.tests.dsdb.samba.tests.dsdb.DsdbRemoteNCRootTests.test_dsdb_dn_nc_root_misleading_to_noexisting_guid -^samba.tests.dsdb.samba.tests.dsdb.DsdbRemoteNCRootTests.test_dsdb_dn_nc_root_sid diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c index be0a2cd4a33..97444fc1d3e 100644 --- a/source4/dsdb/common/util.c +++ b/source4/dsdb/common/util.c @@ -4124,10 +4124,12 @@ static int dsdb_dn_compare_ptrs(struct ldb_dn **dn1, struct ldb_dn **dn2) } /* - find a NC root given a DN within the NC + find a NC root given a DN within the NC by reading the rootDSE namingContexts */ -int dsdb_find_nc_root(struct ldb_context *samdb, TALLOC_CTX *mem_ctx, struct ldb_dn *dn, - struct ldb_dn **nc_root) +static int dsdb_find_nc_root_string_based(struct ldb_context *samdb, + TALLOC_CTX *mem_ctx, + struct ldb_dn *dn, + struct ldb_dn **nc_root) { const char *root_attrs[] = { "namingContexts", NULL }; TALLOC_CTX *tmp_ctx; @@ -4212,6 +4214,235 @@ int dsdb_find_nc_root(struct ldb_context *samdb, TALLOC_CTX *mem_ctx, struct ldb return ldb_error(samdb, LDB_ERR_NO_SUCH_OBJECT, __func__); } +struct dsdb_get_partition_and_dn { + TALLOC_CTX *mem_ctx; + unsigned int count; + struct ldb_dn *dn; + struct ldb_dn *partition_dn; + bool want_partition_dn; +}; + +static int dsdb_get_partition_and_dn(struct ldb_request *req, + struct ldb_reply *ares) +{ + int ret; + struct dsdb_get_partition_and_dn *context = req->context; + struct ldb_control *partition_ctrl = NULL; + struct dsdb_control_current_partition *partition = NULL; + + if (!ares) { + return ldb_request_done(req, LDB_ERR_OPERATIONS_ERROR); + } + if (ares->error != LDB_SUCCESS + && ares->error != LDB_ERR_NO_SUCH_OBJECT) { + return ldb_request_done(req, ares->error); + } + + switch (ares->type) { + case LDB_REPLY_ENTRY: + if (context->count != 0) { + return ldb_request_done(req, + LDB_ERR_CONSTRAINT_VIOLATION); + } + context->count++; + + context->dn = talloc_steal(context->mem_ctx, + ares->message->dn); + break; + + case LDB_REPLY_REFERRAL: + talloc_free(ares); + return ldb_request_done(req, LDB_SUCCESS); + + case LDB_REPLY_DONE: + partition_ctrl + = ldb_reply_get_control(ares, + DSDB_CONTROL_CURRENT_PARTITION_OID); + if (!context->want_partition_dn || + partition_ctrl == NULL) { + ret = ares->error; + talloc_free(ares); + + return ldb_request_done(req, ret); + } + + partition + = talloc_get_type_abort(partition_ctrl->data, + struct dsdb_control_current_partition); + context->partition_dn + = ldb_dn_copy(context->mem_ctx, partition->dn); + if (context->partition_dn == NULL) { + return ldb_request_done(req, + LDB_ERR_OPERATIONS_ERROR); + } + + ret = ares->error; + talloc_free(ares); + + return ldb_request_done(req, ret); + } + + talloc_free(ares); + return LDB_SUCCESS; +} + +/* + find a NC root given a DN within the NC + */ +int dsdb_find_nc_root(struct ldb_context *samdb, TALLOC_CTX *mem_ctx, struct ldb_dn *dn, + struct ldb_dn **nc_root) +{ + TALLOC_CTX *tmp_ctx; + int ret; + struct ldb_request *req; + struct ldb_result *res; + struct ldb_dn *search_dn = dn; + static const char * attrs[] = { NULL }; + bool has_extended = ldb_dn_has_extended(dn); + bool has_normal_components = ldb_dn_get_comp_num(dn) >= 1; + struct dsdb_get_partition_and_dn context = { + .mem_ctx = mem_ctx, + .want_partition_dn = nc_root != NULL + }; + + if (!has_extended && !has_normal_components) { + return ldb_error(samdb, LDB_ERR_NO_SUCH_OBJECT, + "Request for NC root for rootDSE (\"\") deined."); + } + + tmp_ctx = talloc_new(samdb); + if (tmp_ctx == NULL) { + return ldb_oom(samdb); + } + + res = talloc_zero(tmp_ctx, struct ldb_result); + if (res == NULL) { + talloc_free(tmp_ctx); + return ldb_oom(samdb); + } + + if (has_extended && has_normal_components) { + bool minimise_ok; + search_dn = ldb_dn_copy(tmp_ctx, dn); + if (search_dn == NULL) { + talloc_free(tmp_ctx); + return ldb_oom(samdb); + } + minimise_ok = ldb_dn_minimise(search_dn); + if (!minimise_ok) { + talloc_free(tmp_ctx); + return ldb_operr(samdb); + } + } + + ret = ldb_build_search_req(&req, samdb, tmp_ctx, + search_dn, + LDB_SCOPE_BASE, + NULL, + attrs, + NULL, + &context, + dsdb_get_partition_and_dn, + NULL); + if (ret != LDB_SUCCESS) { + talloc_free(tmp_ctx); + return ret; + } + + ret = ldb_request_add_control(req, + DSDB_CONTROL_CURRENT_PARTITION_OID, + false, NULL); + if (ret != LDB_SUCCESS) { + talloc_free(tmp_ctx); + return ret; + } + + ret = dsdb_request_add_controls(req, + DSDB_SEARCH_SHOW_RECYCLED| + DSDB_SEARCH_SHOW_DELETED| + DSDB_SEARCH_SHOW_DN_IN_STORAGE_FORMAT); + if (ret != LDB_SUCCESS) { + talloc_free(tmp_ctx); + return ret; + } + + ret = ldb_request(samdb, req); + if (ret == LDB_SUCCESS) { + ret = ldb_wait(req->handle, LDB_WAIT_ALL); + } + + /* + * This could be a new DN, not in the DB, which is OK. If we + * don't need the normalised DN, we can continue. + * + * We may be told the partition it would be in in the search + * reply control, or if not we can do a string-based match. + */ + + if (ret == LDB_ERR_NO_SUCH_OBJECT) { + ret = LDB_SUCCESS; + ldb_reset_err_string(samdb); + } else if (ret != LDB_SUCCESS) { + talloc_free(tmp_ctx); + return ret; + } + + + /* + * If the user did not need to find the nc_root, + * we are done + */ + if (nc_root == NULL) { + talloc_free(tmp_ctx); + return ret; + } + + /* + * When we are working locally, both for the case were + * we find the DN, and the case where we fail, we get + * back via controls the partition it was in or should + * have been in, to return to the client + */ + if (context.partition_dn != NULL) { + (*nc_root) = context.partition_dn; + + talloc_free(tmp_ctx); + return ret; + } + + /* + * This is a remote operation, which is a little harder as we + * have a work out the nc_root from the list of NCs. If we did + * at least resolve the DN to a string, get that now, it makes + * the string-based match below possible for a GUID-based + * input over remote LDAP. + */ + if (context.dn) { + dn = context.dn; + } else if (has_extended && !has_normal_components) { + ldb_asprintf_errstring(samdb, + "Cannot determine NC root " + "for a not-found bare extended DN %s.", + ldb_dn_get_extended_linearized(tmp_ctx, dn, 1)); + talloc_free(tmp_ctx); + return LDB_ERR_NO_SUCH_OBJECT; + } + + /* + * Either we are working aginast a remote LDAP + * server or the object doesn't exist locally. + * + * This means any GUID that was present in the DN + * therefore could not be evaluated, so do a + * string-based match instead. + */ + talloc_free(tmp_ctx); + return dsdb_find_nc_root_string_based(samdb, + mem_ctx, + dn, + nc_root); +} + /* find the deleted objects DN for any object, by looking for the NC -- 2.25.1 From ddd098caa15c4c35b012cf1bba33d3b5b82ca86e Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 15 Dec 2022 18:52:20 +1300 Subject: [PATCH 10/21] s4-dsdb: Add better debugging to dsdb_objects_have_same_nc() BUG: https://bugzilla.samba.org/show_bug.cgi?id=10635 Signed-off-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher (cherry picked from commit 0f501b2316af6568003e520848c1ec80c286fd36) --- source4/dsdb/common/util.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c index 97444fc1d3e..58eef3abb66 100644 --- a/source4/dsdb/common/util.c +++ b/source4/dsdb/common/util.c @@ -6063,8 +6063,8 @@ bool dsdb_objects_have_same_nc(struct ldb_context *ldb, ret = LDB_ERR_OTHER; } if (ret != LDB_SUCCESS) { - DBG_ERR("Failed to find base DN for source %s\n", - ldb_dn_get_linearized(source_dn)); + DBG_ERR("Failed to find base DN for source %s: %s\n", + ldb_dn_get_linearized(source_dn), ldb_errstring(ldb)); talloc_free(tmp_ctx); return true; } @@ -6075,8 +6075,8 @@ bool dsdb_objects_have_same_nc(struct ldb_context *ldb, ret = LDB_ERR_OTHER; } if (ret != LDB_SUCCESS) { - DBG_ERR("Failed to find base DN for target %s\n", - ldb_dn_get_linearized(target_dn)); + DBG_ERR("Failed to find base DN for target %s: %s\n", + ldb_dn_get_linearized(target_dn), ldb_errstring(ldb)); talloc_free(tmp_ctx); return true; } -- 2.25.1 From 11ba414d277136aa3725831db7cb00970f7ae4b2 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Mon, 12 Dec 2022 16:15:03 +1300 Subject: [PATCH 11/21] s4-dsdb: Add dsdb_normalise_dn_and_find_nc_root() around dsdb_find_nc_root() Reuse the search done for dsdb_find_nc_root() to normalise the DN. This will allow a GUID-input DN to be compared safely with a RID Manager DN or Naming Context. BUG: https://bugzilla.samba.org/show_bug.cgi?id=10635 Signed-off-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher (cherry picked from commit 8e1122420efd11a91aa1c5d60c0cc8fd9ffaf157) --- source4/dsdb/common/util.c | 35 +++++++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c index 58eef3abb66..029a7d69bf3 100644 --- a/source4/dsdb/common/util.c +++ b/source4/dsdb/common/util.c @@ -4289,8 +4289,11 @@ static int dsdb_get_partition_and_dn(struct ldb_request *req, /* find a NC root given a DN within the NC */ -int dsdb_find_nc_root(struct ldb_context *samdb, TALLOC_CTX *mem_ctx, struct ldb_dn *dn, - struct ldb_dn **nc_root) +int dsdb_normalise_dn_and_find_nc_root(struct ldb_context *samdb, + TALLOC_CTX *mem_ctx, + struct ldb_dn *dn, + struct ldb_dn **normalised_dn, + struct ldb_dn **nc_root) { TALLOC_CTX *tmp_ctx; int ret; @@ -4380,6 +4383,10 @@ int dsdb_find_nc_root(struct ldb_context *samdb, TALLOC_CTX *mem_ctx, struct ldb */ if (ret == LDB_ERR_NO_SUCH_OBJECT) { + if (normalised_dn != NULL) { + talloc_free(tmp_ctx); + return ret; + } ret = LDB_SUCCESS; ldb_reset_err_string(samdb); } else if (ret != LDB_SUCCESS) { @@ -4387,6 +4394,16 @@ int dsdb_find_nc_root(struct ldb_context *samdb, TALLOC_CTX *mem_ctx, struct ldb return ret; } + if (normalised_dn != NULL) { + if (context.count != 1) { + /* No results */ + ldb_asprintf_errstring(samdb, + "Request for NC root for %s failed to return any results.", + ldb_dn_get_linearized(dn)); + return LDB_ERR_NO_SUCH_OBJECT; + } + *normalised_dn = context.dn; + } /* * If the user did not need to find the nc_root, @@ -4443,6 +4460,20 @@ int dsdb_find_nc_root(struct ldb_context *samdb, TALLOC_CTX *mem_ctx, struct ldb nc_root); } +/* + find a NC root given a DN within the NC + */ +int dsdb_find_nc_root(struct ldb_context *samdb, + TALLOC_CTX *mem_ctx, + struct ldb_dn *dn, + struct ldb_dn **nc_root) +{ + return dsdb_normalise_dn_and_find_nc_root(samdb, + mem_ctx, + dn, + NULL, + nc_root); +} /* find the deleted objects DN for any object, by looking for the NC -- 2.25.1 From 2c9f8c5be1be8f5667a84b93c4f0665dcb2750d5 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Mon, 12 Dec 2022 16:15:44 +1300 Subject: [PATCH 12/21] s4-rpc_server/drsuapi: Use dsdb_normalise_dn_and_find_nc_root() This reuses the search done for dsdb_find_nc_root() to normalise the DN. This will allow a GUID-input DN to be compared safely with a RID Manager DN or Naming Context. BUG: https://bugzilla.samba.org/show_bug.cgi?id=10635 Signed-off-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher (cherry picked from commit e96dfc74b3ece40fe64a33aa8b8d810b576982bd) --- source4/rpc_server/drsuapi/updaterefs.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/source4/rpc_server/drsuapi/updaterefs.c b/source4/rpc_server/drsuapi/updaterefs.c index 289dc8117ce..7450ddd3a31 100644 --- a/source4/rpc_server/drsuapi/updaterefs.c +++ b/source4/rpc_server/drsuapi/updaterefs.c @@ -196,6 +196,7 @@ WERROR drsuapi_UpdateRefs(struct imessaging_context *msg_ctx, WERROR werr; int ret; struct ldb_dn *dn; + struct ldb_dn *dn_normalised; struct ldb_dn *nc_root; struct ldb_context *sam_ctx = b_state->sam_ctx_system?b_state->sam_ctx_system:b_state->sam_ctx; struct dcerpc_binding_handle *irpc_handle; @@ -227,13 +228,18 @@ WERROR drsuapi_UpdateRefs(struct imessaging_context *msg_ctx, dn = drs_ObjectIdentifier_to_dn(mem_ctx, sam_ctx, req->naming_context); W_ERROR_HAVE_NO_MEMORY(dn); - ret = dsdb_find_nc_root(sam_ctx, dn, dn, &nc_root); + ret = dsdb_normalise_dn_and_find_nc_root(sam_ctx, dn, + dn, + &dn_normalised, + &nc_root); if (ret != LDB_SUCCESS) { DEBUG(2, ("Didn't find a nc for %s\n", ldb_dn_get_linearized(dn))); return WERR_DS_DRA_BAD_NC; } - if (ldb_dn_compare(dn, nc_root) != 0) { - DEBUG(2, ("dn %s is not equal to %s\n", ldb_dn_get_linearized(dn), ldb_dn_get_linearized(nc_root))); + if (ldb_dn_compare(dn_normalised, nc_root) != 0) { + DBG_NOTICE("dn %s is not equal to %s\n", + ldb_dn_get_linearized(dn_normalised), + ldb_dn_get_linearized(nc_root)); return WERR_DS_DRA_BAD_NC; } -- 2.25.1 From 6890d84782cb3a749b12f4d25ebffb4aaa365453 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Mon, 12 Dec 2022 09:47:36 +1300 Subject: [PATCH 13/21] s4-dsdb: rework drs_ObjectIdentifier_to_dn() into drs_ObjectIdentifier_to_dn_and_nc_root() This make this funciton the gatekeeper between the wire format and the internal struct ldb_dn, checking if the DN exists and which NC it belongs to along the way, and presenting only a DB-returned DN for internal processing. BUG: https://bugzilla.samba.org/show_bug.cgi?id=10635 Signed-off-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher (cherry picked from commit aee2039e63ceeb5e69a0461fb77e0f18278e4dc4) --- selftest/knownfail.d/getncchanges | 4 +- source4/dsdb/common/dsdb_dn.c | 33 +++++++++++ source4/rpc_server/drsuapi/drsutil.c | 28 ++++++--- source4/rpc_server/drsuapi/getncchanges.c | 72 +++++++++++++++++++---- source4/rpc_server/drsuapi/updaterefs.c | 29 +++++---- 5 files changed, 133 insertions(+), 33 deletions(-) diff --git a/selftest/knownfail.d/getncchanges b/selftest/knownfail.d/getncchanges index 317d78c41b1..7415f572710 100644 --- a/selftest/knownfail.d/getncchanges +++ b/selftest/knownfail.d/getncchanges @@ -6,12 +6,10 @@ samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegri samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_repl_get_tgt_multivalued_links\(promoted_dc\) # New tests for GetNCChanges with a GUID and a bad DN, like Azure AD Cloud Sync ^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_InvalidDestDSA_and_GUID -^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_InvalidNC_DummyDN_InvalidGUID +^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_InvalidNC_DummyDN_InvalidGUID_REPL_SECRET ^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_InvalidNC_DummyDN_InvalidGUID_REPL_OBJ -^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_InvalidNC_DummyDN_InvalidGUID_RID_ALLOC ^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_InvalidDestDSA_and_GUID_RID_ALLOC ^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_DummyDN_valid_GUID_REPL_OBJ ^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_DummyDN_valid_GUID_REPL_SECRET -^samba4.drs.getncchanges.python\(.*\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_DummyDN_valid_GUID_full_repl ^samba4.drs.getncchanges.python\(.*\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_InvalidNC_DummyDN_InvalidGUID_full_repl ^samba4.drs.repl_rodc.python\(.*\).repl_rodc.DrsRodcTestCase.test_admin_repl_secrets_DummyDN_GUID diff --git a/source4/dsdb/common/dsdb_dn.c b/source4/dsdb/common/dsdb_dn.c index e348ab6aa94..04aab1593b1 100644 --- a/source4/dsdb/common/dsdb_dn.c +++ b/source4/dsdb/common/dsdb_dn.c @@ -396,3 +396,36 @@ struct ldb_dn *drs_ObjectIdentifier_to_dn(TALLOC_CTX *mem_ctx, talloc_free(dn_string); return new_dn; } + +/* + * Safely convert a drsuapi_DsReplicaObjectIdentifier into a validated + * LDB DN of an existing DB entry, and/or find the NC root + * + * Finally, we must return the DN as found in the DB, as otherwise a + * subsequence ldb_dn_compare(dn, nc_root) will fail (as this is based + * on the string components). + */ +int drs_ObjectIdentifier_to_dn_and_nc_root(TALLOC_CTX *mem_ctx, + struct ldb_context *ldb, + struct drsuapi_DsReplicaObjectIdentifier *nc, + struct ldb_dn **normalised_dn, + struct ldb_dn **nc_root) +{ + int ret; + struct ldb_dn *new_dn = NULL; + + new_dn = drs_ObjectIdentifier_to_dn(mem_ctx, + ldb, + nc); + if (new_dn == NULL) { + return LDB_ERR_INVALID_DN_SYNTAX; + } + + ret = dsdb_normalise_dn_and_find_nc_root(ldb, + mem_ctx, + new_dn, + normalised_dn, + nc_root); + TALLOC_FREE(new_dn); + return ret; +} diff --git a/source4/rpc_server/drsuapi/drsutil.c b/source4/rpc_server/drsuapi/drsutil.c index 7897c35d2e9..48423bb6655 100644 --- a/source4/rpc_server/drsuapi/drsutil.c +++ b/source4/rpc_server/drsuapi/drsutil.c @@ -191,8 +191,19 @@ WERROR drs_security_access_check(struct ldb_context *sam_ctx, struct drsuapi_DsReplicaObjectIdentifier *nc, const char *ext_right) { - struct ldb_dn *dn = drs_ObjectIdentifier_to_dn(mem_ctx, sam_ctx, nc); + struct ldb_dn *dn; WERROR werr; + int ret; + + ret = drs_ObjectIdentifier_to_dn_and_nc_root(mem_ctx, + sam_ctx, + nc, + &dn, + NULL); + if (ret != LDB_SUCCESS) { + return WERR_DS_DRA_BAD_DN; + } + werr = drs_security_access_check_log(sam_ctx, mem_ctx, token, dn, ext_right); talloc_free(dn); return werr; @@ -207,17 +218,20 @@ WERROR drs_security_access_check_nc_root(struct ldb_context *sam_ctx, struct drsuapi_DsReplicaObjectIdentifier *nc, const char *ext_right) { - struct ldb_dn *dn, *nc_root; + struct ldb_dn *nc_root; WERROR werr; int ret; - dn = drs_ObjectIdentifier_to_dn(mem_ctx, sam_ctx, nc); - W_ERROR_HAVE_NO_MEMORY(dn); - ret = dsdb_find_nc_root(sam_ctx, dn, dn, &nc_root); + ret = drs_ObjectIdentifier_to_dn_and_nc_root(mem_ctx, + sam_ctx, + nc, + NULL, + &nc_root); if (ret != LDB_SUCCESS) { - return WERR_DS_CANT_FIND_EXPECTED_NC; + return WERR_DS_DRA_BAD_NC; } + werr = drs_security_access_check_log(sam_ctx, mem_ctx, token, nc_root, ext_right); - talloc_free(dn); + talloc_free(nc_root); return werr; } diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c index 7f50587dd1e..b2dea15ac8e 100644 --- a/source4/rpc_server/drsuapi/getncchanges.c +++ b/source4/rpc_server/drsuapi/getncchanges.c @@ -1091,9 +1091,20 @@ static WERROR getncchanges_rid_alloc(struct drsuapi_bind_state *b_state, return WERR_DS_DRA_INTERNAL_ERROR; } - req_dn = drs_ObjectIdentifier_to_dn(mem_ctx, ldb, req10->naming_context); - if (!ldb_dn_validate(req_dn) || - ldb_dn_compare(req_dn, *rid_manager_dn) != 0) { + ret = drs_ObjectIdentifier_to_dn_and_nc_root(mem_ctx, + ldb, + req10->naming_context, + &req_dn, + NULL); + if (ret != LDB_SUCCESS) { + DBG_ERR("RID Alloc request for invalid DN %s: %s\n", + drs_ObjectIdentifier_to_string(mem_ctx, req10->naming_context), + ldb_strerror(ret)); + ctr6->extended_ret = DRSUAPI_EXOP_ERR_MISMATCH; + return WERR_OK; + } + + if (ldb_dn_compare(req_dn, *rid_manager_dn) != 0) { /* that isn't the RID Manager DN */ DEBUG(0,(__location__ ": RID Alloc request for wrong DN %s\n", drs_ObjectIdentifier_to_string(mem_ctx, req10->naming_context))); @@ -1250,7 +1261,17 @@ static WERROR getncchanges_repl_secret(struct drsuapi_bind_state *b_state, * Which basically means that if you have GET_ALL_CHANGES rights (~== RWDC) * then you can do EXOP_REPL_SECRETS */ - obj_dn = drs_ObjectIdentifier_to_dn(mem_ctx, b_state->sam_ctx_system, ncRoot); + ret = drs_ObjectIdentifier_to_dn_and_nc_root(mem_ctx, + b_state->sam_ctx_system, + ncRoot, + &obj_dn, + NULL); + if (ret != LDB_SUCCESS) { + DBG_ERR("RevealSecretRequest for for invalid DN %s\n", + drs_ObjectIdentifier_to_string(mem_ctx, ncRoot)); + goto failed; + } + if (!ldb_dn_validate(obj_dn)) goto failed; if (has_get_all_changes) { @@ -1362,8 +1383,9 @@ static WERROR getncchanges_change_master(struct drsuapi_bind_state *b_state, - verify that we are the current master */ - req_dn = drs_ObjectIdentifier_to_dn(mem_ctx, ldb, req10->naming_context); - if (!ldb_dn_validate(req_dn)) { + ret = drs_ObjectIdentifier_to_dn_and_nc_root(mem_ctx, ldb, req10->naming_context, + &req_dn, NULL); + if (ret != LDB_SUCCESS) { /* that is not a valid dn */ DEBUG(0,(__location__ ": FSMO role transfer request for invalid DN %s\n", drs_ObjectIdentifier_to_string(mem_ctx, req10->naming_context))); @@ -1389,8 +1411,16 @@ static WERROR getncchanges_change_master(struct drsuapi_bind_state *b_state, /* change the current master */ msg = ldb_msg_new(ldb); W_ERROR_HAVE_NO_MEMORY(msg); - msg->dn = drs_ObjectIdentifier_to_dn(msg, ldb, req10->naming_context); - W_ERROR_HAVE_NO_MEMORY(msg->dn); + ret = drs_ObjectIdentifier_to_dn_and_nc_root(msg, ldb, req10->naming_context, + &msg->dn, NULL); + if (ret != LDB_SUCCESS) { + /* that is not a valid dn */ + DBG_ERR("FSMO role transfer request for invalid DN %s: %s\n", + drs_ObjectIdentifier_to_string(mem_ctx, req10->naming_context), + ldb_strerror(ret)); + ctr6->extended_ret = DRSUAPI_EXOP_ERR_MISMATCH; + return WERR_OK; + } /* TODO: make sure ntds_dn is a valid nTDSDSA object */ ret = dsdb_find_dn_by_guid(ldb, msg, &req10->destination_dsa_guid, 0, &ntds_dn); @@ -2864,7 +2894,21 @@ allowed: /* see if a previous replication has been abandoned */ if (getnc_state) { - struct ldb_dn *new_dn = drs_ObjectIdentifier_to_dn(getnc_state, sam_ctx, ncRoot); + struct ldb_dn *new_dn; + ret = drs_ObjectIdentifier_to_dn_and_nc_root(getnc_state, + sam_ctx, + ncRoot, + &new_dn, + NULL); + if (ret != LDB_SUCCESS) { + /* + * This can't fail as we have done this above + * implicitly but not got the DN out + */ + DBG_ERR("Bad DN '%s'\n", + drs_ObjectIdentifier_to_string(mem_ctx, ncRoot)); + return WERR_DS_DRA_INVALID_PARAMETER; + } if (ldb_dn_compare(new_dn, getnc_state->ncRoot_dn) != 0) { DEBUG(0,(__location__ ": DsGetNCChanges 2nd replication on different DN %s %s (last_dn %s)\n", ldb_dn_get_linearized(new_dn), @@ -2899,9 +2943,13 @@ allowed: uint32_t nc_instanceType; struct ldb_dn *ncRoot_dn; - ncRoot_dn = drs_ObjectIdentifier_to_dn(mem_ctx, sam_ctx, ncRoot); - if (ncRoot_dn == NULL) { - return WERR_NOT_ENOUGH_MEMORY; + ret = drs_ObjectIdentifier_to_dn_and_nc_root(mem_ctx, + sam_ctx, + ncRoot, + &ncRoot_dn, + NULL); + if (ret != LDB_SUCCESS) { + return WERR_DS_DRA_BAD_DN; } ret = dsdb_search_dn(sam_ctx, mem_ctx, &res, diff --git a/source4/rpc_server/drsuapi/updaterefs.c b/source4/rpc_server/drsuapi/updaterefs.c index 7450ddd3a31..5d2bc6e949c 100644 --- a/source4/rpc_server/drsuapi/updaterefs.c +++ b/source4/rpc_server/drsuapi/updaterefs.c @@ -195,7 +195,6 @@ WERROR drsuapi_UpdateRefs(struct imessaging_context *msg_ctx, { WERROR werr; int ret; - struct ldb_dn *dn; struct ldb_dn *dn_normalised; struct ldb_dn *nc_root; struct ldb_context *sam_ctx = b_state->sam_ctx_system?b_state->sam_ctx_system:b_state->sam_ctx; @@ -226,14 +225,11 @@ WERROR drsuapi_UpdateRefs(struct imessaging_context *msg_ctx, return WERR_DS_DRA_INVALID_PARAMETER; } - dn = drs_ObjectIdentifier_to_dn(mem_ctx, sam_ctx, req->naming_context); - W_ERROR_HAVE_NO_MEMORY(dn); - ret = dsdb_normalise_dn_and_find_nc_root(sam_ctx, dn, - dn, - &dn_normalised, - &nc_root); + ret = drs_ObjectIdentifier_to_dn_and_nc_root(mem_ctx, sam_ctx, req->naming_context, + &dn_normalised, &nc_root); if (ret != LDB_SUCCESS) { - DEBUG(2, ("Didn't find a nc for %s\n", ldb_dn_get_linearized(dn))); + DBG_WARNING("Didn't find a nc for %s\n", + ldb_dn_get_linearized(dn_normalised)); return WERR_DS_DRA_BAD_NC; } if (ldb_dn_compare(dn_normalised, nc_root) != 0) { @@ -249,7 +245,10 @@ WERROR drsuapi_UpdateRefs(struct imessaging_context *msg_ctx, * This means that in the usual case, it will never open it and never * bother to refresh the dreplsrv. */ - werr = uref_check_dest(sam_ctx, mem_ctx, dn, &req->dest_dsa_guid, + werr = uref_check_dest(sam_ctx, + mem_ctx, + dn_normalised, + &req->dest_dsa_guid, req->options); if (W_ERROR_EQUAL(werr, WERR_DS_DRA_REF_ALREADY_EXISTS) || W_ERROR_EQUAL(werr, WERR_DS_DRA_REF_NOT_FOUND)) { @@ -266,7 +265,11 @@ WERROR drsuapi_UpdateRefs(struct imessaging_context *msg_ctx, } if (req->options & DRSUAPI_DRS_DEL_REF) { - werr = uref_del_dest(sam_ctx, mem_ctx, dn, &req->dest_dsa_guid, req->options); + werr = uref_del_dest(sam_ctx, + mem_ctx, + dn_normalised, + &req->dest_dsa_guid, + req->options); if (!W_ERROR_IS_OK(werr)) { DEBUG(0,("Failed to delete repsTo for %s: %s\n", GUID_string(mem_ctx, &req->dest_dsa_guid), @@ -287,7 +290,11 @@ WERROR drsuapi_UpdateRefs(struct imessaging_context *msg_ctx, dest.source_dsa_obj_guid = req->dest_dsa_guid; dest.replica_flags = req->options; - werr = uref_add_dest(sam_ctx, mem_ctx, dn, &dest, req->options); + werr = uref_add_dest(sam_ctx, + mem_ctx, + dn_normalised, + &dest, + req->options); if (!W_ERROR_IS_OK(werr)) { DEBUG(0,("Failed to add repsTo for %s: %s\n", GUID_string(mem_ctx, &dest.source_dsa_obj_guid), -- 2.25.1 From fb355d6434adc705eac605ebd8116079cf376e2c Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 31 Jan 2023 13:29:05 +1300 Subject: [PATCH 14/21] s4-drs: Make drs_ObjectIdentifier_to_dn() safer and able to cope with DummyDN values We want to totally ignore the string DN if there is a GUID, as clients like "Microsoft Azure AD connect cloud sync" will set a literal "DummyDN" string. RN: Use of the Azure AD Connect cloud sync tool is now supported for password hash synchronisation, allowing Samba AD Domains to synchronise passwords with this popular cloud environment. BUG: https://bugzilla.samba.org/show_bug.cgi?id=10635 Signed-off-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher (cherry picked from commit 73f3ece8b2b44ac4b3323a08fb969f29bf2b0380) --- selftest/knownfail.d/getncchanges | 3 - source4/dsdb/common/dsdb_dn.c | 150 ++++++++++++++++++++-- source4/rpc_server/drsuapi/getncchanges.c | 30 +++-- source4/rpc_server/drsuapi/updaterefs.c | 14 +- 4 files changed, 165 insertions(+), 32 deletions(-) diff --git a/selftest/knownfail.d/getncchanges b/selftest/knownfail.d/getncchanges index 7415f572710..1ced3870b95 100644 --- a/selftest/knownfail.d/getncchanges +++ b/selftest/knownfail.d/getncchanges @@ -9,7 +9,4 @@ samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegri ^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_InvalidNC_DummyDN_InvalidGUID_REPL_SECRET ^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_InvalidNC_DummyDN_InvalidGUID_REPL_OBJ ^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_InvalidDestDSA_and_GUID_RID_ALLOC -^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_DummyDN_valid_GUID_REPL_OBJ ^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_DummyDN_valid_GUID_REPL_SECRET -^samba4.drs.getncchanges.python\(.*\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_InvalidNC_DummyDN_InvalidGUID_full_repl -^samba4.drs.repl_rodc.python\(.*\).repl_rodc.DrsRodcTestCase.test_admin_repl_secrets_DummyDN_GUID diff --git a/source4/dsdb/common/dsdb_dn.c b/source4/dsdb/common/dsdb_dn.c index 04aab1593b1..c86848fd697 100644 --- a/source4/dsdb/common/dsdb_dn.c +++ b/source4/dsdb/common/dsdb_dn.c @@ -359,10 +359,12 @@ int dsdb_dn_string_comparison(struct ldb_context *ldb, void *mem_ctx, } /* - format a drsuapi_DsReplicaObjectIdentifier naming context as a string + * format a drsuapi_DsReplicaObjectIdentifier naming context as a string for debugging + * + * When forming a DN for DB access you must use drs_ObjectIdentifier_to_dn() */ -char *drs_ObjectIdentifier_to_string(TALLOC_CTX *mem_ctx, - struct drsuapi_DsReplicaObjectIdentifier *nc) +char *drs_ObjectIdentifier_to_debug_string(TALLOC_CTX *mem_ctx, + struct drsuapi_DsReplicaObjectIdentifier *nc) { char *ret = NULL; TALLOC_CTX *tmp_ctx = talloc_new(mem_ctx); @@ -386,21 +388,147 @@ char *drs_ObjectIdentifier_to_string(TALLOC_CTX *mem_ctx, return ret; } -struct ldb_dn *drs_ObjectIdentifier_to_dn(TALLOC_CTX *mem_ctx, - struct ldb_context *ldb, - struct drsuapi_DsReplicaObjectIdentifier *nc) +/* + * Safely convert a drsuapi_DsReplicaObjectIdentifier into an LDB DN + * + * We need to have GUID and SID prority and not allow extended + * components in the DN. + * + * We must also totally honour the prority even if the string DN is not valid or able to parse as a DN. + */ +static struct ldb_dn *drs_ObjectIdentifier_to_dn(TALLOC_CTX *mem_ctx, + struct ldb_context *ldb, + struct drsuapi_DsReplicaObjectIdentifier *nc) { - char *dn_string = drs_ObjectIdentifier_to_string(mem_ctx, nc); - struct ldb_dn *new_dn; - new_dn = ldb_dn_new(mem_ctx, ldb, dn_string); - talloc_free(dn_string); - return new_dn; + struct ldb_dn *new_dn = NULL; + + if (!GUID_all_zero(&nc->guid)) { + struct GUID_txt_buf buf; + char *guid = GUID_buf_string(&nc->guid, &buf); + + new_dn = ldb_dn_new_fmt(mem_ctx, + ldb, + "", + guid); + if (new_dn == NULL) { + DBG_ERR("Failed to prepare drs_ObjectIdentifier " + "GUID %s into a DN\n", + guid); + return NULL; + } + + return new_dn; + } + + if (nc->__ndr_size_sid != 0 && nc->sid.sid_rev_num != 0) { + struct dom_sid_buf buf; + char *sid = dom_sid_str_buf(&nc->sid, &buf); + + new_dn = ldb_dn_new_fmt(mem_ctx, + ldb, + "", + sid); + if (new_dn == NULL) { + DBG_ERR("Failed to prepare drs_ObjectIdentifier " + "SID %s into a DN\n", + sid); + return NULL; + } + return new_dn; + } + + if (nc->__ndr_size_dn != 0 && nc->dn) { + int dn_comp_num = 0; + bool new_dn_valid = false; + + new_dn = ldb_dn_new(mem_ctx, ldb, nc->dn); + if (new_dn == NULL) { + /* Set to WARNING as this is user-controlled, don't print the value into the logs */ + DBG_WARNING("Failed to parse string DN in " + "drs_ObjectIdentifier into an LDB DN\n"); + return NULL; + } + + new_dn_valid = ldb_dn_validate(new_dn); + if (!new_dn_valid) { + /* + * Set to WARNING as this is user-controlled, + * but can print the value into the logs as it + * parsed a bit + */ + DBG_WARNING("Failed to validate string DN [%s] in " + "drs_ObjectIdentifier as an LDB DN\n", + ldb_dn_get_linearized(new_dn)); + return NULL; + } + + dn_comp_num = ldb_dn_get_comp_num(new_dn); + if (dn_comp_num <= 0) { + /* + * Set to WARNING as this is user-controlled, + * but can print the value into the logs as it + * parsed a bit + */ + DBG_WARNING("DN [%s] in drs_ObjectIdentifier " + "must have 1 or more components\n", + ldb_dn_get_linearized(new_dn)); + return NULL; + } + + if (ldb_dn_is_special(new_dn)) { + /* + * Set to WARNING as this is user-controlled, + * but can print the value into the logs as it + * parsed a bit + */ + DBG_WARNING("New string DN [%s] in " + "drs_ObjectIdentifier is a " + "special LDB DN\n", + ldb_dn_get_linearized(new_dn)); + return NULL; + } + + /* + * We want this just to be a string DN, extended + * components are manually handled above + */ + if (ldb_dn_has_extended(new_dn)) { + /* + * Set to WARNING as this is user-controlled, + * but can print the value into the logs as it + * parsed a bit + */ + DBG_WARNING("Refusing to parse New string DN [%s] in " + "drs_ObjectIdentifier as an " + "extended LDB DN " + "(GUIDs and SIDs should be in the " + ".guid and .sid IDL elelements, " + "not in the string\n", + ldb_dn_get_extended_linearized(mem_ctx, + new_dn, + 1)); + return NULL; + } + return new_dn; + } + + DBG_WARNING("Refusing to parse empty string DN " + "(and no GUID or SID) " + "drs_ObjectIdentifier into a empty " + "(eg RootDSE) LDB DN\n"); + return NULL; } /* * Safely convert a drsuapi_DsReplicaObjectIdentifier into a validated * LDB DN of an existing DB entry, and/or find the NC root * + * We need to have GUID and SID prority and not allow extended + * components in the DN. + * + * We must also totally honour the prority even if the string DN is + * not valid or able to parse as a DN. + * * Finally, we must return the DN as found in the DB, as otherwise a * subsequence ldb_dn_compare(dn, nc_root) will fail (as this is based * on the string components). diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c index b2dea15ac8e..02a6dd3f803 100644 --- a/source4/rpc_server/drsuapi/getncchanges.c +++ b/source4/rpc_server/drsuapi/getncchanges.c @@ -1098,7 +1098,7 @@ static WERROR getncchanges_rid_alloc(struct drsuapi_bind_state *b_state, NULL); if (ret != LDB_SUCCESS) { DBG_ERR("RID Alloc request for invalid DN %s: %s\n", - drs_ObjectIdentifier_to_string(mem_ctx, req10->naming_context), + drs_ObjectIdentifier_to_debug_string(mem_ctx, req10->naming_context), ldb_strerror(ret)); ctr6->extended_ret = DRSUAPI_EXOP_ERR_MISMATCH; return WERR_OK; @@ -1106,8 +1106,9 @@ static WERROR getncchanges_rid_alloc(struct drsuapi_bind_state *b_state, if (ldb_dn_compare(req_dn, *rid_manager_dn) != 0) { /* that isn't the RID Manager DN */ - DEBUG(0,(__location__ ": RID Alloc request for wrong DN %s\n", - drs_ObjectIdentifier_to_string(mem_ctx, req10->naming_context))); + DBG_ERR("RID Alloc request for wrong DN %s\n", + drs_ObjectIdentifier_to_debug_string(mem_ctx, + req10->naming_context)); ctr6->extended_ret = DRSUAPI_EXOP_ERR_MISMATCH; return WERR_OK; } @@ -1200,7 +1201,7 @@ static WERROR getncchanges_repl_secret(struct drsuapi_bind_state *b_state, WERROR werr; DEBUG(3,(__location__ ": DRSUAPI_EXOP_REPL_SECRET extended op on %s\n", - drs_ObjectIdentifier_to_string(mem_ctx, ncRoot))); + drs_ObjectIdentifier_to_debug_string(mem_ctx, ncRoot))); /* * we need to work out if we will allow this DC to @@ -1268,7 +1269,7 @@ static WERROR getncchanges_repl_secret(struct drsuapi_bind_state *b_state, NULL); if (ret != LDB_SUCCESS) { DBG_ERR("RevealSecretRequest for for invalid DN %s\n", - drs_ObjectIdentifier_to_string(mem_ctx, ncRoot)); + drs_ObjectIdentifier_to_debug_string(mem_ctx, ncRoot)); goto failed; } @@ -1353,7 +1354,7 @@ static WERROR getncchanges_repl_obj(struct drsuapi_bind_state *b_state, struct drsuapi_DsReplicaObjectIdentifier *ncRoot = req10->naming_context; DEBUG(3,(__location__ ": DRSUAPI_EXOP_REPL_OBJ extended op on %s\n", - drs_ObjectIdentifier_to_string(mem_ctx, ncRoot))); + drs_ObjectIdentifier_to_debug_string(mem_ctx, ncRoot))); ctr6->extended_ret = DRSUAPI_EXOP_ERR_SUCCESS; return WERR_OK; @@ -1387,8 +1388,9 @@ static WERROR getncchanges_change_master(struct drsuapi_bind_state *b_state, &req_dn, NULL); if (ret != LDB_SUCCESS) { /* that is not a valid dn */ - DEBUG(0,(__location__ ": FSMO role transfer request for invalid DN %s\n", - drs_ObjectIdentifier_to_string(mem_ctx, req10->naming_context))); + DBG_ERR("FSMO role transfer request for invalid DN %s: %s\n", + drs_ObjectIdentifier_to_debug_string(mem_ctx, req10->naming_context), + ldb_strerror(ret)); ctr6->extended_ret = DRSUAPI_EXOP_ERR_MISMATCH; return WERR_OK; } @@ -1416,7 +1418,7 @@ static WERROR getncchanges_change_master(struct drsuapi_bind_state *b_state, if (ret != LDB_SUCCESS) { /* that is not a valid dn */ DBG_ERR("FSMO role transfer request for invalid DN %s: %s\n", - drs_ObjectIdentifier_to_string(mem_ctx, req10->naming_context), + drs_ObjectIdentifier_to_debug_string(mem_ctx, req10->naming_context), ldb_strerror(ret)); ctr6->extended_ret = DRSUAPI_EXOP_ERR_MISMATCH; return WERR_OK; @@ -2906,7 +2908,7 @@ allowed: * implicitly but not got the DN out */ DBG_ERR("Bad DN '%s'\n", - drs_ObjectIdentifier_to_string(mem_ctx, ncRoot)); + drs_ObjectIdentifier_to_debug_string(mem_ctx, ncRoot)); return WERR_DS_DRA_INVALID_PARAMETER; } if (ldb_dn_compare(new_dn, getnc_state->ncRoot_dn) != 0) { @@ -3063,7 +3065,7 @@ allowed: if (!ldb_dn_validate(getnc_state->ncRoot_dn) || ldb_dn_is_null(getnc_state->ncRoot_dn)) { DEBUG(0,(__location__ ": Bad DN '%s'\n", - drs_ObjectIdentifier_to_string(mem_ctx, ncRoot))); + drs_ObjectIdentifier_to_debug_string(mem_ctx, ncRoot))); return WERR_DS_DRA_INVALID_PARAMETER; } @@ -3502,7 +3504,8 @@ allowed: &ureq); if (!W_ERROR_IS_OK(werr)) { DEBUG(0,(__location__ ": Failed UpdateRefs on %s for %s in DsGetNCChanges - %s\n", - drs_ObjectIdentifier_to_string(mem_ctx, ncRoot), ureq.dest_dsa_dns_name, + drs_ObjectIdentifier_to_debug_string(mem_ctx, ncRoot), + ureq.dest_dsa_dns_name, win_errstr(werr))); } } @@ -3632,7 +3635,8 @@ allowed: DEBUG(r->out.ctr->ctr6.more_data?4:2, ("DsGetNCChanges with uSNChanged >= %llu flags 0x%08x on %s gave %u objects (done %u/%u) %u links (done %u/%u (as %s))\n", (unsigned long long)(req10->highwatermark.highest_usn+1), - req10->replica_flags, drs_ObjectIdentifier_to_string(mem_ctx, ncRoot), + req10->replica_flags, + drs_ObjectIdentifier_to_debug_string(mem_ctx, ncRoot), r->out.ctr->ctr6.object_count, i, r->out.ctr->ctr6.more_data?getnc_state->num_records:i, r->out.ctr->ctr6.linked_attributes_count, diff --git a/source4/rpc_server/drsuapi/updaterefs.c b/source4/rpc_server/drsuapi/updaterefs.c index 5d2bc6e949c..0be675ffe21 100644 --- a/source4/rpc_server/drsuapi/updaterefs.c +++ b/source4/rpc_server/drsuapi/updaterefs.c @@ -206,7 +206,7 @@ WERROR drsuapi_UpdateRefs(struct imessaging_context *msg_ctx, DEBUG(4,("DsReplicaUpdateRefs for host '%s' with GUID %s options 0x%08x nc=%s\n", req->dest_dsa_dns_name, GUID_string(mem_ctx, &req->dest_dsa_guid), req->options, - drs_ObjectIdentifier_to_string(mem_ctx, req->naming_context))); + drs_ObjectIdentifier_to_debug_string(mem_ctx, req->naming_context))); /* * 4.1.26.2 Server Behavior of the IDL_DRSUpdateRefs Method @@ -228,14 +228,18 @@ WERROR drsuapi_UpdateRefs(struct imessaging_context *msg_ctx, ret = drs_ObjectIdentifier_to_dn_and_nc_root(mem_ctx, sam_ctx, req->naming_context, &dn_normalised, &nc_root); if (ret != LDB_SUCCESS) { - DBG_WARNING("Didn't find a nc for %s\n", - ldb_dn_get_linearized(dn_normalised)); + DBG_WARNING("Didn't find a nc for %s: %s\n", + drs_ObjectIdentifier_to_debug_string(mem_ctx, + req->naming_context), + ldb_errstring(sam_ctx)); return WERR_DS_DRA_BAD_NC; } if (ldb_dn_compare(dn_normalised, nc_root) != 0) { - DBG_NOTICE("dn %s is not equal to %s\n", + DBG_NOTICE("dn %s is not equal to %s (from %s)\n", ldb_dn_get_linearized(dn_normalised), - ldb_dn_get_linearized(nc_root)); + ldb_dn_get_linearized(nc_root), + drs_ObjectIdentifier_to_debug_string(mem_ctx, + req->naming_context)); return WERR_DS_DRA_BAD_NC; } -- 2.25.1 From cbaea678cab63addae7840f946d834b028801a93 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Fri, 16 Dec 2022 14:22:20 +1300 Subject: [PATCH 15/21] s4-rpc_server/drsuapi: Return correct error code for an invalid DN to EXOP_REPL_OBJ/EXOP_REPL_OBJ BUG: https://bugzilla.samba.org/show_bug.cgi?id=10635 Signed-off-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher (cherry picked from commit cbe18353d8d7b2a35b965e4fc8c895ac497e67e8) --- selftest/knownfail.d/getncchanges | 2 -- source4/rpc_server/drsuapi/getncchanges.c | 14 ++++++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/selftest/knownfail.d/getncchanges b/selftest/knownfail.d/getncchanges index 1ced3870b95..e97f61b10a4 100644 --- a/selftest/knownfail.d/getncchanges +++ b/selftest/knownfail.d/getncchanges @@ -6,7 +6,5 @@ samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegri samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_repl_get_tgt_multivalued_links\(promoted_dc\) # New tests for GetNCChanges with a GUID and a bad DN, like Azure AD Cloud Sync ^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_InvalidDestDSA_and_GUID -^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_InvalidNC_DummyDN_InvalidGUID_REPL_SECRET -^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_InvalidNC_DummyDN_InvalidGUID_REPL_OBJ ^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_InvalidDestDSA_and_GUID_RID_ALLOC ^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_DummyDN_valid_GUID_REPL_SECRET diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c index 02a6dd3f803..e2d0781fc4a 100644 --- a/source4/rpc_server/drsuapi/getncchanges.c +++ b/source4/rpc_server/drsuapi/getncchanges.c @@ -2794,6 +2794,20 @@ WERROR dcesrv_drsuapi_DsGetNCChanges(struct dcesrv_call_state *dce_call, TALLOC_ session_info->security_token, req10->naming_context, GUID_DRS_GET_CHANGES); + + if (W_ERROR_EQUAL(werr, WERR_DS_DRA_BAD_NC)) { + /* + * These extended operations need a different error if + * the supplied DN can't be found + */ + switch (req10->extended_op) { + case DRSUAPI_EXOP_REPL_OBJ: + case DRSUAPI_EXOP_REPL_SECRET: + return WERR_DS_DRA_BAD_DN; + default: + return werr; + } + } if (!W_ERROR_IS_OK(werr)) { return werr; } -- 2.25.1 From 92fec1ff1a51adb0e31ad6886e862cf2ee40d1a7 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Wed, 25 Jan 2023 15:17:44 +1300 Subject: [PATCH 16/21] s4-dsdb: Split samdb_get_ntds_obj_by_guid() out of samdb_is_rodc() This will allow the logic here to be tighened up and shared in the next few commits. BUG: https://bugzilla.samba.org/show_bug.cgi?id=10635 Signed-off-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher (cherry picked from commit d5a2af3feae98057ba29de444d308d499d633941) --- source4/dsdb/common/util.c | 59 ++++++++++++++++++++++++++++++-------- 1 file changed, 47 insertions(+), 12 deletions(-) diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c index 029a7d69bf3..43fa670006d 100644 --- a/source4/dsdb/common/util.c +++ b/source4/dsdb/common/util.c @@ -3550,9 +3550,49 @@ int drsuapi_DsReplicaCursor_compare(const struct drsuapi_DsReplicaCursor *c1, return GUID_compare(&c1->source_dsa_invocation_id, &c2->source_dsa_invocation_id); } +/* + * Return the NTDS object for a GUID, confirming it is in the + * configuration partition and a nTDSDSA object + */ +int samdb_get_ntds_obj_by_guid(TALLOC_CTX *mem_ctx, + struct ldb_context *sam_ctx, + const struct GUID *objectGUID, + const char **attrs, + struct ldb_message **msg) +{ + int ret; + struct ldb_result *res; + struct GUID_txt_buf guid_buf; + char *guid_str = GUID_buf_string(objectGUID, &guid_buf); + struct ldb_dn *config_dn = NULL; + + config_dn = ldb_get_config_basedn(sam_ctx); + if (config_dn == NULL) { + return ldb_operr(sam_ctx); + } + + ret = dsdb_search(sam_ctx, + mem_ctx, + &res, + config_dn, + LDB_SCOPE_SUBTREE, + attrs, + DSDB_SEARCH_ONE_ONLY, + "objectGUID=%s", + guid_str); + if (ret != LDB_SUCCESS) { + return ret; + } + if (msg) { + *msg = talloc_steal(mem_ctx, res->msgs[0]); + } + TALLOC_FREE(res); + return ret; +} + /* - see if a computer identified by its invocationId is a RODC + see if a computer identified by its objectGUID is a RODC */ int samdb_is_rodc(struct ldb_context *sam_ctx, const struct GUID *objectGUID, bool *is_rodc) { @@ -3561,20 +3601,15 @@ int samdb_is_rodc(struct ldb_context *sam_ctx, const struct GUID *objectGUID, bo 3) if not present then not a RODC 4) if present and TRUE then is a RODC */ - struct ldb_dn *config_dn; const char *attrs[] = { "msDS-isRODC", NULL }; int ret; - struct ldb_result *res; + struct ldb_message *msg; TALLOC_CTX *tmp_ctx = talloc_new(sam_ctx); - config_dn = ldb_get_config_basedn(sam_ctx); - if (!config_dn) { - talloc_free(tmp_ctx); - return ldb_operr(sam_ctx); - } - - ret = dsdb_search(sam_ctx, tmp_ctx, &res, config_dn, LDB_SCOPE_SUBTREE, attrs, - DSDB_SEARCH_ONE_ONLY, "objectGUID=%s", GUID_string(tmp_ctx, objectGUID)); + ret = samdb_get_ntds_obj_by_guid(tmp_ctx, + sam_ctx, + objectGUID, + attrs, &msg); if (ret == LDB_ERR_NO_SUCH_OBJECT) { *is_rodc = false; @@ -3590,7 +3625,7 @@ int samdb_is_rodc(struct ldb_context *sam_ctx, const struct GUID *objectGUID, bo return ret; } - ret = ldb_msg_find_attr_as_bool(res->msgs[0], "msDS-isRODC", 0); + ret = ldb_msg_find_attr_as_bool(msg, "msDS-isRODC", 0); *is_rodc = (ret == 1); talloc_free(tmp_ctx); -- 2.25.1 From 3d3a2c376f73bb204a1f5648060e70d7ab80ebe6 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Wed, 25 Jan 2023 15:18:47 +1300 Subject: [PATCH 17/21] s4-dsdb: Require that the NTDS object is an nTDSDSA objectclass This should avoid a user being able to specify the GUID of a different type of object. BUG: https://bugzilla.samba.org/show_bug.cgi?id=10635 Signed-off-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher (cherry picked from commit adb776149e5ac0eb346992775610627106e1a986) --- source4/dsdb/common/util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c index 43fa670006d..a30ae662c1e 100644 --- a/source4/dsdb/common/util.c +++ b/source4/dsdb/common/util.c @@ -3578,7 +3578,7 @@ int samdb_get_ntds_obj_by_guid(TALLOC_CTX *mem_ctx, LDB_SCOPE_SUBTREE, attrs, DSDB_SEARCH_ONE_ONLY, - "objectGUID=%s", + "(&(objectGUID=%s)(objectClass=nTDSDSA))", guid_str); if (ret != LDB_SUCCESS) { return ret; -- 2.25.1 From 0ba11e1801e7e2829d73551596bb12f9e2a10ed4 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Wed, 25 Jan 2023 15:24:01 +1300 Subject: [PATCH 18/21] s4-drsuapi: Use samdb_get_ntds_obj_by_guid() to find RODC in REPL_SECRET We need to find the RODC per the destination_dsa_guid to mark the secrets as having been replicated, and by using samdb_get_ntds_obj_by_guid() we are stricter in the checks, as the RODC has to be the right objectClass (nTDSDSA) and under the CN=Configuration partition. BUG: https://bugzilla.samba.org/show_bug.cgi?id=10635 Signed-off-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher (cherry picked from commit 09ec6a1db2d3b831548bf7d66475c486be29b1d1) --- source4/rpc_server/drsuapi/getncchanges.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c index e2d0781fc4a..0e4272a9b62 100644 --- a/source4/rpc_server/drsuapi/getncchanges.c +++ b/source4/rpc_server/drsuapi/getncchanges.c @@ -1188,9 +1188,11 @@ static WERROR getncchanges_repl_secret(struct drsuapi_bind_state *b_state, { struct drsuapi_DsReplicaObjectIdentifier *ncRoot = req10->naming_context; struct ldb_dn *obj_dn = NULL; + struct ldb_message *ntds_msg = NULL; struct ldb_dn *ntds_dn = NULL, *server_dn = NULL; struct ldb_dn *rodc_dn, *krbtgt_link_dn; int ret; + const char *ntds_attrs[] = { NULL }; const char *rodc_attrs[] = { "msDS-KrbTgtLink", "msDS-NeverRevealGroup", "msDS-RevealOnDemandGroup", @@ -1223,13 +1225,17 @@ static WERROR getncchanges_repl_secret(struct drsuapi_bind_state *b_state, * * If we are the RODC, we will check that this matches the SID. */ - ret = dsdb_find_dn_by_guid(b_state->sam_ctx_system, mem_ctx, - &req10->destination_dsa_guid, 0, - &ntds_dn); + ret = samdb_get_ntds_obj_by_guid(mem_ctx, + b_state->sam_ctx_system, + &req10->destination_dsa_guid, + ntds_attrs, + &ntds_msg); if (ret != LDB_SUCCESS) { goto failed; } + ntds_dn = ntds_msg->dn; + server_dn = ldb_dn_get_parent(mem_ctx, ntds_dn); if (server_dn == NULL) { goto failed; -- 2.25.1 From 2deeb7b249ff982c6f442125e14dbdc676ffff89 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Wed, 25 Jan 2023 14:18:11 +1300 Subject: [PATCH 19/21] s4-rpc_server: Pre-check destination_dsa_guid in GetNCChanges for validity This allows our new tests to pass as these need to be checked first. BUG: https://bugzilla.samba.org/show_bug.cgi?id=10635 Signed-off-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher (cherry picked from commit 115a3a10440f44ba11029be5ae3a05534a7b98c0) --- selftest/knownfail.d/getncchanges | 2 -- source4/rpc_server/drsuapi/getncchanges.c | 42 +++++++++++++++++++++-- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/selftest/knownfail.d/getncchanges b/selftest/knownfail.d/getncchanges index e97f61b10a4..7adc669855d 100644 --- a/selftest/knownfail.d/getncchanges +++ b/selftest/knownfail.d/getncchanges @@ -5,6 +5,4 @@ samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegri samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_repl_get_tgt_and_anc\(promoted_dc\) samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_repl_get_tgt_multivalued_links\(promoted_dc\) # New tests for GetNCChanges with a GUID and a bad DN, like Azure AD Cloud Sync -^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_InvalidDestDSA_and_GUID -^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_InvalidDestDSA_and_GUID_RID_ALLOC ^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_DummyDN_valid_GUID_REPL_SECRET diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c index 0e4272a9b62..57bd50b1268 100644 --- a/source4/rpc_server/drsuapi/getncchanges.c +++ b/source4/rpc_server/drsuapi/getncchanges.c @@ -2774,9 +2774,45 @@ WERROR dcesrv_drsuapi_DsGetNCChanges(struct dcesrv_call_state *dce_call, TALLOC_ return WERR_DS_DRA_SOURCE_DISABLED; } - /* Perform access checks. */ - /* TODO: we need to support a sync on a specific non-root - * DN. We'll need to find the real partition root here */ + /* + * Help our tests pass by pre-checking the + * destination_dsa_guid before the NC permissions. Info on + * valid DSA GUIDs is not sensitive so this isn't a leak + */ + switch (req10->extended_op) { + case DRSUAPI_EXOP_FSMO_REQ_ROLE: + case DRSUAPI_EXOP_FSMO_RID_ALLOC: + case DRSUAPI_EXOP_FSMO_RID_REQ_ROLE: + case DRSUAPI_EXOP_FSMO_REQ_PDC: + case DRSUAPI_EXOP_FSMO_ABANDON_ROLE: + { + const char *attrs[] = { NULL }; + + ret = samdb_get_ntds_obj_by_guid(mem_ctx, + sam_ctx, + &req10->destination_dsa_guid, + attrs, + NULL); + if (ret == LDB_ERR_NO_SUCH_OBJECT) { + /* + * Error out with an EXOP error but success at + * the top level return value + */ + r->out.ctr->ctr6.extended_ret = DRSUAPI_EXOP_ERR_UNKNOWN_CALLER; + return WERR_OK; + } else if (ret != LDB_SUCCESS) { + return WERR_DS_DRA_INTERNAL_ERROR; + } + + break; + } + case DRSUAPI_EXOP_REPL_SECRET: + case DRSUAPI_EXOP_REPL_OBJ: + case DRSUAPI_EXOP_NONE: + break; + } + + /* Perform access checks. */ ncRoot = req10->naming_context; if (ncRoot == NULL) { DEBUG(0,(__location__ ": Request for DsGetNCChanges with no NC\n")); -- 2.25.1 From 3ea86f08906666d320a982cf20dcf3b1485beed8 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Wed, 25 Jan 2023 16:01:48 +1300 Subject: [PATCH 20/21] s4-drsuapi: Clarify role of drs_security_access_check_nc_root() BUG: https://bugzilla.samba.org/show_bug.cgi?id=10635 Signed-off-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher (cherry picked from commit 1838f349c94b878de1740af35351a2e8e0c8cffb) --- source4/rpc_server/drsuapi/getncchanges.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c index 57bd50b1268..ca805d9f958 100644 --- a/source4/rpc_server/drsuapi/getncchanges.c +++ b/source4/rpc_server/drsuapi/getncchanges.c @@ -2830,7 +2830,11 @@ WERROR dcesrv_drsuapi_DsGetNCChanges(struct dcesrv_call_state *dce_call, TALLOC_ user_sid = &session_info->security_token->sids[PRIMARY_USER_SID_INDEX]; - /* all clients must have GUID_DRS_GET_CHANGES */ + /* + * all clients must have GUID_DRS_GET_CHANGES. This finds the + * actual NC root of the given value and checks that, allowing + * REPL_OBJ to work safely + */ werr = drs_security_access_check_nc_root(sam_ctx, mem_ctx, session_info->security_token, -- 2.25.1 From e86739a452f1119782533aa94fc92b7b02d24b35 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Wed, 25 Jan 2023 15:24:57 +1300 Subject: [PATCH 21/21] s4-drsuapi: Give an error that matches windows on destination_dsa_guid lookup failure BUG: https://bugzilla.samba.org/show_bug.cgi?id=10635 Signed-off-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher Autobuild-User(master): Stefan Metzmacher Autobuild-Date(master): Tue Jan 31 13:43:54 UTC 2023 on atb-devel-224 (cherry picked from commit 0f2978bbc0ed5b65d75c20472650a749643312e7) --- selftest/knownfail.d/getncchanges | 2 -- source4/rpc_server/drsuapi/getncchanges.c | 14 ++++++++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/selftest/knownfail.d/getncchanges b/selftest/knownfail.d/getncchanges index 7adc669855d..5ef1bc98bef 100644 --- a/selftest/knownfail.d/getncchanges +++ b/selftest/knownfail.d/getncchanges @@ -4,5 +4,3 @@ samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegri samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_repl_get_tgt_chain\(promoted_dc\) samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_repl_get_tgt_and_anc\(promoted_dc\) samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_repl_get_tgt_multivalued_links\(promoted_dc\) -# New tests for GetNCChanges with a GUID and a bad DN, like Azure AD Cloud Sync -^samba4.drs.getnc_exop.python\(.*\).getnc_exop.DrsReplicaSyncTestCase.test_DummyDN_valid_GUID_REPL_SECRET diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c index ca805d9f958..74b173c3965 100644 --- a/source4/rpc_server/drsuapi/getncchanges.c +++ b/source4/rpc_server/drsuapi/getncchanges.c @@ -1201,6 +1201,7 @@ static WERROR getncchanges_repl_secret(struct drsuapi_bind_state *b_state, const char *obj_attrs[] = { "tokenGroups", "objectSid", "UserAccountControl", "msDS-KrbTgtLinkBL", NULL }; struct ldb_result *rodc_res = NULL, *obj_res = NULL; WERROR werr; + struct GUID_txt_buf guid_buf; DEBUG(3,(__location__ ": DRSUAPI_EXOP_REPL_SECRET extended op on %s\n", drs_ObjectIdentifier_to_debug_string(mem_ctx, ncRoot))); @@ -1231,7 +1232,7 @@ static WERROR getncchanges_repl_secret(struct drsuapi_bind_state *b_state, ntds_attrs, &ntds_msg); if (ret != LDB_SUCCESS) { - goto failed; + goto dest_dsa_error; } ntds_dn = ntds_msg->dn; @@ -1245,7 +1246,7 @@ static WERROR getncchanges_repl_secret(struct drsuapi_bind_state *b_state, "serverReference", machine_dn); if (ret != LDB_SUCCESS) { - goto failed; + goto dest_dsa_error; } /* @@ -1346,6 +1347,15 @@ failed: ldb_dn_get_linearized(obj_dn), dom_sid_string(mem_ctx, user_sid))); ctr6->extended_ret = DRSUAPI_EXOP_ERR_NONE; return WERR_DS_DRA_BAD_DN; + +dest_dsa_error: + DBG_WARNING("Failed secret replication for %s by RODC %s as dest_dsa_guid %s is invalid\n", + ldb_dn_get_linearized(obj_dn), + dom_sid_string(mem_ctx, user_sid), + GUID_buf_string(&req10->destination_dsa_guid, + &guid_buf)); + ctr6->extended_ret = DRSUAPI_EXOP_ERR_NONE; + return WERR_DS_DRA_DB_ERROR; } /* -- 2.25.1