The Samba-Bugzilla – Attachment 15688 Details for
Bug 12497
[SECURITY] CVE-2019-14902 Replication of ACLs down subtree on AD Directory not automatic
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
patch for Samba 4.9 (cherry-picked/backported from master patch) v2
CVE-2019-14902-sd-repl-4.9-02.patch (text/plain), 52.56 KB, created by
Andrew Bartlett
on 2019-12-16 04:19:08 UTC
(
hide
)
Description:
patch for Samba 4.9 (cherry-picked/backported from master patch) v2
Filename:
MIME Type:
Creator:
Andrew Bartlett
Created:
2019-12-16 04:19:08 UTC
Size:
52.56 KB
patch
obsolete
>From a95e207f9f39076a31112727b780b250248a3edb Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Thu, 28 Nov 2019 17:16:16 +1300 >Subject: [PATCH 01/10] CVE-2019-14902 selftest: Add test for replication of > inherited security descriptors > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=12497 > >Signed-off-by: Andrew Bartlett <abartlet@samba.org> >--- > selftest/knownfail.d/repl_secdesc | 2 + > source4/selftest/tests.py | 5 + > source4/torture/drs/python/repl_secdesc.py | 258 +++++++++++++++++++++ > 3 files changed, 265 insertions(+) > create mode 100644 selftest/knownfail.d/repl_secdesc > create mode 100644 source4/torture/drs/python/repl_secdesc.py > >diff --git a/selftest/knownfail.d/repl_secdesc b/selftest/knownfail.d/repl_secdesc >new file mode 100644 >index 00000000000..2aa24c61375 >--- /dev/null >+++ b/selftest/knownfail.d/repl_secdesc >@@ -0,0 +1,2 @@ >+^samba4.drs.repl_secdesc.python\(.*\).repl_secdesc.ReplAclTestCase.test_acl_inheirt_renamed_object_in_conflict >+^samba4.drs.repl_secdesc.python\(.*\).repl_secdesc.ReplAclTestCase.test_acl_inherit_existing_object >diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py >index 2ec0bee923b..7244535791d 100755 >--- a/source4/selftest/tests.py >+++ b/source4/selftest/tests.py >@@ -1004,6 +1004,11 @@ for env in ['vampire_dc', 'promoted_dc']: > extra_path=[os.path.join(samba4srcdir, 'torture/drs/python')], > environ={'DC1': "$DC_SERVER", 'DC2': '$%s_SERVER' % env.upper()}, > extra_args=['-U$DOMAIN/$DC_USERNAME%$DC_PASSWORD']) >+ planoldpythontestsuite(env, "repl_secdesc", >+ name="samba4.drs.repl_secdesc.python(%s)" % env, >+ extra_path=[os.path.join(samba4srcdir, 'torture/drs/python')], >+ environ={'DC1': "$DC_SERVER", 'DC2': '$SERVER'}, >+ extra_args=['-U$DOMAIN/$DC_USERNAME%$DC_PASSWORD']) > planoldpythontestsuite(env, "repl_move", > extra_path=[os.path.join(samba4srcdir, 'torture/drs/python')], > name="samba4.drs.repl_move.python(%s)" % env, >diff --git a/source4/torture/drs/python/repl_secdesc.py b/source4/torture/drs/python/repl_secdesc.py >new file mode 100644 >index 00000000000..4ed449a8a18 >--- /dev/null >+++ b/source4/torture/drs/python/repl_secdesc.py >@@ -0,0 +1,258 @@ >+#!/usr/bin/env python3 >+# -*- coding: utf-8 -*- >+# >+# Unix SMB/CIFS implementation. >+# Copyright (C) Catalyst.Net Ltd. 2017 >+# Copyright (C) Andrew Bartlett <abartlet@samba.org> 2019 >+# >+# This program is free software; you can redistribute it and/or modify >+# it under the terms of the GNU General Public License as published by >+# the Free Software Foundation; either version 3 of the License, or >+# (at your option) any later version. >+# >+# This program is distributed in the hope that it will be useful, >+# but WITHOUT ANY WARRANTY; without even the implied warranty of >+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >+# GNU General Public License for more details. >+# >+# You should have received a copy of the GNU General Public License >+# along with this program. If not, see <http://www.gnu.org/licenses/>. >+# >+import drs_base >+import ldb >+import samba >+from samba import sd_utils >+from ldb import LdbError >+ >+class ReplAclTestCase(drs_base.DrsBaseTestCase): >+ >+ def setUp(self): >+ super(ReplAclTestCase, self).setUp() >+ self.sd_utils_dc1 = sd_utils.SDUtils(self.ldb_dc1) >+ self.sd_utils_dc2 = sd_utils.SDUtils(self.ldb_dc2) >+ >+ self.ou = samba.tests.create_test_ou(self.ldb_dc1, >+ "test_acl_inherit") >+ >+ # disable replication for the tests so we can control at what point >+ # the DCs try to replicate >+ self._disable_all_repl(self.dnsname_dc1) >+ self._disable_all_repl(self.dnsname_dc2) >+ >+ # make sure DCs are synchronized before the test >+ self._net_drs_replicate(DC=self.dnsname_dc2, fromDC=self.dnsname_dc1, forced=True) >+ self._net_drs_replicate(DC=self.dnsname_dc1, fromDC=self.dnsname_dc2, forced=True) >+ >+ def tearDown(self): >+ self.ldb_dc1.delete(self.ou, ["tree_delete:1"]) >+ >+ # re-enable replication >+ self._enable_all_repl(self.dnsname_dc1) >+ self._enable_all_repl(self.dnsname_dc2) >+ >+ super(ReplAclTestCase, self).tearDown() >+ >+ def test_acl_inheirt_new_object_1_pass(self): >+ # Set the inherited ACL on the parent OU >+ mod = "(A;CIOI;GA;;;SY)" >+ self.sd_utils_dc1.dacl_add_ace(self.ou, mod) >+ >+ # Make a new object >+ dn = ldb.Dn(self.ldb_dc1, "OU=l2,%s" % self.ou) >+ self.ldb_dc1.add({"dn": dn, "objectclass": "organizationalUnit"}) >+ >+ self._net_drs_replicate(DC=self.dnsname_dc2, >+ fromDC=self.dnsname_dc1, >+ forced=True) >+ >+ # Confirm inherited ACLs are identical >+ >+ self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(dn), >+ self.sd_utils_dc2.get_sd_as_sddl(dn)) >+ >+ def test_acl_inheirt_new_object(self): >+ # Set the inherited ACL on the parent OU >+ mod = "(A;CIOI;GA;;;SY)" >+ self.sd_utils_dc1.dacl_add_ace(self.ou, mod) >+ >+ # Replicate to DC2 >+ >+ self._net_drs_replicate(DC=self.dnsname_dc2, >+ fromDC=self.dnsname_dc1, >+ forced=True) >+ >+ # Make a new object >+ dn = ldb.Dn(self.ldb_dc1, "OU=l2,%s" % self.ou) >+ self.ldb_dc1.add({"dn": dn, "objectclass": "organizationalUnit"}) >+ >+ self._net_drs_replicate(DC=self.dnsname_dc2, >+ fromDC=self.dnsname_dc1, >+ forced=True) >+ >+ # Confirm inherited ACLs are identical >+ >+ self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(dn), >+ self.sd_utils_dc2.get_sd_as_sddl(dn)) >+ >+ def test_acl_inherit_existing_object(self): >+ # Make a new object >+ dn = ldb.Dn(self.ldb_dc1, "OU=l2,%s" % self.ou) >+ self.ldb_dc1.add({"dn": dn, "objectclass": "organizationalUnit"}) >+ >+ try: >+ self.ldb_dc2.search(scope=ldb.SCOPE_BASE, >+ base=dn, >+ attrs=[]) >+ self.fail() >+ except LdbError as err: >+ enum = err.args[0] >+ self.assertEqual(enum, ldb.ERR_NO_SUCH_OBJECT) >+ >+ self._net_drs_replicate(DC=self.dnsname_dc2, >+ fromDC=self.dnsname_dc1, >+ forced=True) >+ >+ # Confirm it is now replicated >+ self.ldb_dc2.search(scope=ldb.SCOPE_BASE, >+ base=dn, >+ attrs=[]) >+ >+ # Set the inherited ACL on the parent OU >+ mod = "(A;CIOI;GA;;;SY)" >+ self.sd_utils_dc1.dacl_add_ace(self.ou, mod) >+ >+ # Replicate to DC2 >+ >+ self._net_drs_replicate(DC=self.dnsname_dc2, >+ fromDC=self.dnsname_dc1, >+ forced=True) >+ >+ # Confirm inherited ACLs are identical >+ >+ self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(dn), >+ self.sd_utils_dc2.get_sd_as_sddl(dn)) >+ >+ def test_acl_inheirt_existing_object_1_pass(self): >+ # Make a new object >+ dn = ldb.Dn(self.ldb_dc1, "OU=l2,%s" % self.ou) >+ self.ldb_dc1.add({"dn": dn, "objectclass": "organizationalUnit"}) >+ >+ try: >+ self.ldb_dc2.search(scope=ldb.SCOPE_BASE, >+ base=dn, >+ attrs=[]) >+ self.fail() >+ except LdbError as err: >+ enum = err.args[0] >+ self.assertEqual(enum, ldb.ERR_NO_SUCH_OBJECT) >+ >+ # Set the inherited ACL on the parent OU >+ mod = "(A;CIOI;GA;;;SY)" >+ self.sd_utils_dc1.dacl_add_ace(self.ou, mod) >+ >+ # Replicate to DC2 >+ >+ self._net_drs_replicate(DC=self.dnsname_dc2, >+ fromDC=self.dnsname_dc1, >+ forced=True) >+ >+ # Confirm inherited ACLs are identical >+ >+ self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(dn), >+ self.sd_utils_dc2.get_sd_as_sddl(dn)) >+ >+ def test_acl_inheirt_renamed_object(self): >+ # Make a new object >+ new_ou = samba.tests.create_test_ou(self.ldb_dc1, >+ "acl_test_l2") >+ >+ sub_ou_dn = ldb.Dn(self.ldb_dc1, "OU=l2,%s" % self.ou) >+ >+ try: >+ self.ldb_dc2.search(scope=ldb.SCOPE_BASE, >+ base=new_ou, >+ attrs=[]) >+ self.fail() >+ except LdbError as err: >+ enum = err.args[0] >+ self.assertEqual(enum, ldb.ERR_NO_SUCH_OBJECT) >+ >+ self._net_drs_replicate(DC=self.dnsname_dc2, >+ fromDC=self.dnsname_dc1, >+ forced=True) >+ >+ # Confirm it is now replicated >+ self.ldb_dc2.search(scope=ldb.SCOPE_BASE, >+ base=new_ou, >+ attrs=[]) >+ >+ # Set the inherited ACL on the parent OU on DC1 >+ mod = "(A;CIOI;GA;;;SY)" >+ self.sd_utils_dc1.dacl_add_ace(self.ou, mod) >+ >+ # Replicate to DC2 >+ >+ self._net_drs_replicate(DC=self.dnsname_dc2, >+ fromDC=self.dnsname_dc1, >+ forced=True) >+ >+ # Rename to under self.ou >+ >+ self.ldb_dc1.rename(new_ou, sub_ou_dn) >+ >+ # Replicate to DC2 >+ >+ self._net_drs_replicate(DC=self.dnsname_dc2, >+ fromDC=self.dnsname_dc1, >+ forced=True) >+ >+ # Confirm inherited ACLs are identical >+ self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(sub_ou_dn), >+ self.sd_utils_dc2.get_sd_as_sddl(sub_ou_dn)) >+ >+ >+ def test_acl_inheirt_renamed_object_in_conflict(self): >+ # Make a new object to be renamed under self.ou >+ new_ou = samba.tests.create_test_ou(self.ldb_dc1, >+ "acl_test_l2") >+ >+ # Make a new OU under self.ou (on DC2) >+ sub_ou_dn = ldb.Dn(self.ldb_dc2, "OU=l2,%s" % self.ou) >+ self.ldb_dc2.add({"dn": sub_ou_dn, >+ "objectclass": "organizationalUnit"}) >+ >+ # Set the inherited ACL on the parent OU >+ mod = "(A;CIOI;GA;;;SY)" >+ self.sd_utils_dc1.dacl_add_ace(self.ou, mod) >+ >+ # Replicate to DC2 >+ >+ self._net_drs_replicate(DC=self.dnsname_dc2, >+ fromDC=self.dnsname_dc1, >+ forced=True) >+ >+ # Rename to under self.ou >+ self.ldb_dc1.rename(new_ou, sub_ou_dn) >+ >+ # Replicate to DC2 (will cause a conflict, DC1 to win, version >+ # is higher since named twice) >+ >+ self._net_drs_replicate(DC=self.dnsname_dc2, >+ fromDC=self.dnsname_dc1, >+ forced=True) >+ >+ children = self.ldb_dc2.search(scope=ldb.SCOPE_ONELEVEL, >+ base=self.ou, >+ attrs=[]) >+ for child in children: >+ self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(sub_ou_dn), >+ self.sd_utils_dc2.get_sd_as_sddl(child.dn)) >+ >+ # Replicate back >+ self._net_drs_replicate(DC=self.dnsname_dc1, >+ fromDC=self.dnsname_dc2, >+ forced=True) >+ >+ for child in children: >+ self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(child.dn), >+ self.sd_utils_dc2.get_sd_as_sddl(child.dn)) >-- >2.17.1 > > >From a7883a2f31760cced47be13216b2f30b1f707c7f Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Tue, 10 Dec 2019 15:16:24 +1300 >Subject: [PATCH 02/10] CVE-2019-14902 selftest: Add test for a special case > around replicated renames > >It appears Samba is currently string-name based in the ACL inheritence code. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=12497 > >Signed-off-by: Andrew Bartlett <abartlet@samba.org> >--- > selftest/knownfail.d/repl_secdesc | 1 + > source4/torture/drs/python/repl_secdesc.py | 69 ++++++++++++++++++++++ > 2 files changed, 70 insertions(+) > >diff --git a/selftest/knownfail.d/repl_secdesc b/selftest/knownfail.d/repl_secdesc >index 2aa24c61375..7d554ff237a 100644 >--- a/selftest/knownfail.d/repl_secdesc >+++ b/selftest/knownfail.d/repl_secdesc >@@ -1,2 +1,3 @@ > ^samba4.drs.repl_secdesc.python\(.*\).repl_secdesc.ReplAclTestCase.test_acl_inheirt_renamed_object_in_conflict > ^samba4.drs.repl_secdesc.python\(.*\).repl_secdesc.ReplAclTestCase.test_acl_inherit_existing_object >+^samba4.drs.repl_secdesc.python\(.*\).repl_secdesc.ReplAclTestCase.test_acl_inheirt_renamed_child_object >diff --git a/source4/torture/drs/python/repl_secdesc.py b/source4/torture/drs/python/repl_secdesc.py >index 4ed449a8a18..58861af3bac 100644 >--- a/source4/torture/drs/python/repl_secdesc.py >+++ b/source4/torture/drs/python/repl_secdesc.py >@@ -211,6 +211,75 @@ class ReplAclTestCase(drs_base.DrsBaseTestCase): > self.sd_utils_dc2.get_sd_as_sddl(sub_ou_dn)) > > >+ def test_acl_inheirt_renamed_child_object(self): >+ # Make a new OU >+ new_ou = samba.tests.create_test_ou(self.ldb_dc1, >+ "acl_test_l2") >+ >+ # Here is where the new OU will end up at the end. >+ sub2_ou_dn_final = ldb.Dn(self.ldb_dc1, "OU=l2,%s" % self.ou) >+ >+ sub3_ou_dn = ldb.Dn(self.ldb_dc1, "OU=l3,%s" % new_ou) >+ sub3_ou_dn_final = ldb.Dn(self.ldb_dc1, "OU=l3,%s" % sub2_ou_dn_final) >+ >+ self.ldb_dc1.add({"dn": sub3_ou_dn, >+ "objectclass": "organizationalUnit"}) >+ >+ sub4_ou_dn = ldb.Dn(self.ldb_dc1, "OU=l4,%s" % sub3_ou_dn) >+ sub4_ou_dn_final = ldb.Dn(self.ldb_dc1, "OU=l4,%s" % sub3_ou_dn_final) >+ >+ self.ldb_dc1.add({"dn": sub4_ou_dn, >+ "objectclass": "organizationalUnit"}) >+ >+ try: >+ self.ldb_dc2.search(scope=ldb.SCOPE_BASE, >+ base=new_ou, >+ attrs=[]) >+ self.fail() >+ except LdbError as err: >+ enum = err.args[0] >+ self.assertEqual(enum, ldb.ERR_NO_SUCH_OBJECT) >+ >+ self._net_drs_replicate(DC=self.dnsname_dc2, >+ fromDC=self.dnsname_dc1, >+ forced=True) >+ >+ # Confirm it is now replicated >+ self.ldb_dc2.search(scope=ldb.SCOPE_BASE, >+ base=new_ou, >+ attrs=[]) >+ >+ # >+ # Given a tree new_ou -> l3 -> l4 >+ # >+ >+ # Set the inherited ACL on the grandchild OU (l3) on DC1 >+ mod = "(A;CIOI;GA;;;SY)" >+ self.sd_utils_dc1.dacl_add_ace(sub3_ou_dn, mod) >+ >+ # Rename new_ou (l2) to under self.ou (this must happen second). If the >+ # inheritence between l3 and l4 is name-based, this could >+ # break. >+ >+ # The tree is now self.ou -> l2 -> l3 -> l4 >+ >+ self.ldb_dc1.rename(new_ou, sub2_ou_dn_final) >+ >+ # Replicate to DC2 >+ >+ self._net_drs_replicate(DC=self.dnsname_dc2, >+ fromDC=self.dnsname_dc1, >+ forced=True) >+ >+ # Confirm set ACLs (on l3 ) are identical. >+ self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(sub3_ou_dn_final), >+ self.sd_utils_dc2.get_sd_as_sddl(sub3_ou_dn_final)) >+ >+ # Confirm inherited ACLs (from l3 to l4) are identical. >+ self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(sub4_ou_dn_final), >+ self.sd_utils_dc2.get_sd_as_sddl(sub4_ou_dn_final)) >+ >+ > def test_acl_inheirt_renamed_object_in_conflict(self): > # Make a new object to be renamed under self.ou > new_ou = samba.tests.create_test_ou(self.ldb_dc1, >-- >2.17.1 > > >From 482e2a33e52603f329476202caffd9843f898121 Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Mon, 16 Dec 2019 11:29:27 +1300 >Subject: [PATCH 03/10] selftest: Add test to confirm ACL inheritence really > happens > >While we have a seperate test (sec_descriptor.py) that confirms inheritance in >general we want to lock in these specific patterns as this test covers >rename. > >Signed-off-by: Andrew Bartlett <abartlet@samba.org> >--- > source4/torture/drs/python/repl_secdesc.py | 115 +++++++++++++++++---- > 1 file changed, 94 insertions(+), 21 deletions(-) > >diff --git a/source4/torture/drs/python/repl_secdesc.py b/source4/torture/drs/python/repl_secdesc.py >index 58861af3bac..58212907e23 100644 >--- a/source4/torture/drs/python/repl_secdesc.py >+++ b/source4/torture/drs/python/repl_secdesc.py >@@ -28,6 +28,10 @@ class ReplAclTestCase(drs_base.DrsBaseTestCase): > > def setUp(self): > super(ReplAclTestCase, self).setUp() >+ self.mod = "(A;CIOI;GA;;;SY)" >+ self.mod_becomes = "(A;OICIIO;GA;;;SY)" >+ self.mod_inherits_as = "(A;OICIIOID;GA;;;SY)" >+ > self.sd_utils_dc1 = sd_utils.SDUtils(self.ldb_dc1) > self.sd_utils_dc2 = sd_utils.SDUtils(self.ldb_dc2) > >@@ -54,8 +58,11 @@ class ReplAclTestCase(drs_base.DrsBaseTestCase): > > def test_acl_inheirt_new_object_1_pass(self): > # Set the inherited ACL on the parent OU >- mod = "(A;CIOI;GA;;;SY)" >- self.sd_utils_dc1.dacl_add_ace(self.ou, mod) >+ self.sd_utils_dc1.dacl_add_ace(self.ou, self.mod) >+ >+ # Assert ACL set stuck as expected >+ self.assertIn(self.mod_becomes, >+ self.sd_utils_dc1.get_sd_as_sddl(self.ou)) > > # Make a new object > dn = ldb.Dn(self.ldb_dc1, "OU=l2,%s" % self.ou) >@@ -65,15 +72,24 @@ class ReplAclTestCase(drs_base.DrsBaseTestCase): > fromDC=self.dnsname_dc1, > forced=True) > >- # Confirm inherited ACLs are identical >+ # Assert ACL replicated as expected >+ self.assertIn(self.mod_becomes, >+ self.sd_utils_dc2.get_sd_as_sddl(self.ou)) > >+ # Confirm inherited ACLs are identical and were inherited >+ >+ self.assertIn(self.mod_inherits_as, >+ self.sd_utils_dc1.get_sd_as_sddl(dn)) > self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(dn), > self.sd_utils_dc2.get_sd_as_sddl(dn)) > > def test_acl_inheirt_new_object(self): > # Set the inherited ACL on the parent OU >- mod = "(A;CIOI;GA;;;SY)" >- self.sd_utils_dc1.dacl_add_ace(self.ou, mod) >+ self.sd_utils_dc1.dacl_add_ace(self.ou, self.mod) >+ >+ # Assert ACL set stuck as expected >+ self.assertIn(self.mod_becomes, >+ self.sd_utils_dc1.get_sd_as_sddl(self.ou)) > > # Replicate to DC2 > >@@ -89,8 +105,14 @@ class ReplAclTestCase(drs_base.DrsBaseTestCase): > fromDC=self.dnsname_dc1, > forced=True) > >- # Confirm inherited ACLs are identical >+ # Assert ACL replicated as expected >+ self.assertIn(self.mod_becomes, >+ self.sd_utils_dc2.get_sd_as_sddl(self.ou)) > >+ # Confirm inherited ACLs are identical and were inheritied >+ >+ self.assertIn(self.mod_inherits_as, >+ self.sd_utils_dc1.get_sd_as_sddl(dn)) > self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(dn), > self.sd_utils_dc2.get_sd_as_sddl(dn)) > >@@ -118,8 +140,11 @@ class ReplAclTestCase(drs_base.DrsBaseTestCase): > attrs=[]) > > # Set the inherited ACL on the parent OU >- mod = "(A;CIOI;GA;;;SY)" >- self.sd_utils_dc1.dacl_add_ace(self.ou, mod) >+ self.sd_utils_dc1.dacl_add_ace(self.ou, self.mod) >+ >+ # Assert ACL set stuck as expected >+ self.assertIn(self.mod_becomes, >+ self.sd_utils_dc1.get_sd_as_sddl(self.ou)) > > # Replicate to DC2 > >@@ -127,8 +152,14 @@ class ReplAclTestCase(drs_base.DrsBaseTestCase): > fromDC=self.dnsname_dc1, > forced=True) > >- # Confirm inherited ACLs are identical >+ # Confirm inherited ACLs are identical and were inherited > >+ # Assert ACL replicated as expected >+ self.assertIn(self.mod_becomes, >+ self.sd_utils_dc2.get_sd_as_sddl(self.ou)) >+ >+ self.assertIn(self.mod_inherits_as, >+ self.sd_utils_dc1.get_sd_as_sddl(dn)) > self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(dn), > self.sd_utils_dc2.get_sd_as_sddl(dn)) > >@@ -147,8 +178,11 @@ class ReplAclTestCase(drs_base.DrsBaseTestCase): > self.assertEqual(enum, ldb.ERR_NO_SUCH_OBJECT) > > # Set the inherited ACL on the parent OU >- mod = "(A;CIOI;GA;;;SY)" >- self.sd_utils_dc1.dacl_add_ace(self.ou, mod) >+ self.sd_utils_dc1.dacl_add_ace(self.ou, self.mod) >+ >+ # Assert ACL set as expected >+ self.assertIn(self.mod_becomes, >+ self.sd_utils_dc1.get_sd_as_sddl(self.ou)) > > # Replicate to DC2 > >@@ -156,8 +190,14 @@ class ReplAclTestCase(drs_base.DrsBaseTestCase): > fromDC=self.dnsname_dc1, > forced=True) > >- # Confirm inherited ACLs are identical >+ # Assert ACL replicated as expected >+ self.assertIn(self.mod_becomes, >+ self.sd_utils_dc2.get_sd_as_sddl(self.ou)) > >+ # Confirm inherited ACLs are identical and were inherited >+ >+ self.assertIn(self.mod_inherits_as, >+ self.sd_utils_dc1.get_sd_as_sddl(dn)) > self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(dn), > self.sd_utils_dc2.get_sd_as_sddl(dn)) > >@@ -187,8 +227,11 @@ class ReplAclTestCase(drs_base.DrsBaseTestCase): > attrs=[]) > > # Set the inherited ACL on the parent OU on DC1 >- mod = "(A;CIOI;GA;;;SY)" >- self.sd_utils_dc1.dacl_add_ace(self.ou, mod) >+ self.sd_utils_dc1.dacl_add_ace(self.ou, self.mod) >+ >+ # Assert ACL set as expected >+ self.assertIn(self.mod_becomes, >+ self.sd_utils_dc1.get_sd_as_sddl(self.ou)) > > # Replicate to DC2 > >@@ -196,6 +239,10 @@ class ReplAclTestCase(drs_base.DrsBaseTestCase): > fromDC=self.dnsname_dc1, > forced=True) > >+ # Assert ACL replicated as expected >+ self.assertIn(self.mod_becomes, >+ self.sd_utils_dc2.get_sd_as_sddl(self.ou)) >+ > # Rename to under self.ou > > self.ldb_dc1.rename(new_ou, sub_ou_dn) >@@ -206,7 +253,9 @@ class ReplAclTestCase(drs_base.DrsBaseTestCase): > fromDC=self.dnsname_dc1, > forced=True) > >- # Confirm inherited ACLs are identical >+ # Confirm inherited ACLs are identical and were inherited >+ self.assertIn(self.mod_inherits_as, >+ self.sd_utils_dc1.get_sd_as_sddl(sub_ou_dn)) > self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(sub_ou_dn), > self.sd_utils_dc2.get_sd_as_sddl(sub_ou_dn)) > >@@ -254,8 +303,11 @@ class ReplAclTestCase(drs_base.DrsBaseTestCase): > # > > # Set the inherited ACL on the grandchild OU (l3) on DC1 >- mod = "(A;CIOI;GA;;;SY)" >- self.sd_utils_dc1.dacl_add_ace(sub3_ou_dn, mod) >+ self.sd_utils_dc1.dacl_add_ace(sub3_ou_dn, self.mod) >+ >+ # Assert ACL set stuck as expected >+ self.assertIn(self.mod_becomes, >+ self.sd_utils_dc1.get_sd_as_sddl(sub3_ou_dn)) > > # Rename new_ou (l2) to under self.ou (this must happen second). If the > # inheritence between l3 and l4 is name-based, this could >@@ -265,17 +317,26 @@ class ReplAclTestCase(drs_base.DrsBaseTestCase): > > self.ldb_dc1.rename(new_ou, sub2_ou_dn_final) > >+ # Assert ACL set remained as expected >+ self.assertIn(self.mod_becomes, >+ self.sd_utils_dc1.get_sd_as_sddl(sub3_ou_dn_final)) >+ > # Replicate to DC2 > > self._net_drs_replicate(DC=self.dnsname_dc2, > fromDC=self.dnsname_dc1, > forced=True) > >- # Confirm set ACLs (on l3 ) are identical. >+ # Confirm set ACLs (on l3 ) are identical and were inherited >+ self.assertIn(self.mod_becomes, >+ self.sd_utils_dc2.get_sd_as_sddl(sub3_ou_dn_final)) > self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(sub3_ou_dn_final), > self.sd_utils_dc2.get_sd_as_sddl(sub3_ou_dn_final)) > >- # Confirm inherited ACLs (from l3 to l4) are identical. >+ # Confirm inherited ACLs (from l3 to l4) are identical >+ # and where inherited >+ self.assertIn(self.mod_inherits_as, >+ self.sd_utils_dc1.get_sd_as_sddl(sub4_ou_dn_final)) > self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(sub4_ou_dn_final), > self.sd_utils_dc2.get_sd_as_sddl(sub4_ou_dn_final)) > >@@ -291,8 +352,11 @@ class ReplAclTestCase(drs_base.DrsBaseTestCase): > "objectclass": "organizationalUnit"}) > > # Set the inherited ACL on the parent OU >- mod = "(A;CIOI;GA;;;SY)" >- self.sd_utils_dc1.dacl_add_ace(self.ou, mod) >+ self.sd_utils_dc1.dacl_add_ace(self.ou, self.mod) >+ >+ # Assert ACL set stuck as expected >+ self.assertIn(self.mod_becomes, >+ self.sd_utils_dc1.get_sd_as_sddl(self.ou)) > > # Replicate to DC2 > >@@ -302,6 +366,8 @@ class ReplAclTestCase(drs_base.DrsBaseTestCase): > > # Rename to under self.ou > self.ldb_dc1.rename(new_ou, sub_ou_dn) >+ self.assertIn(self.mod_inherits_as, >+ self.sd_utils_dc1.get_sd_as_sddl(sub_ou_dn)) > > # Replicate to DC2 (will cause a conflict, DC1 to win, version > # is higher since named twice) >@@ -314,6 +380,8 @@ class ReplAclTestCase(drs_base.DrsBaseTestCase): > base=self.ou, > attrs=[]) > for child in children: >+ self.assertIn(self.mod_inherits_as, >+ self.sd_utils_dc2.get_sd_as_sddl(child.dn)) > self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(sub_ou_dn), > self.sd_utils_dc2.get_sd_as_sddl(child.dn)) > >@@ -322,6 +390,11 @@ class ReplAclTestCase(drs_base.DrsBaseTestCase): > fromDC=self.dnsname_dc2, > forced=True) > >+ self.assertIn(self.mod_inherits_as, >+ self.sd_utils_dc1.get_sd_as_sddl(sub_ou_dn)) >+ > for child in children: >+ self.assertIn(self.mod_inherits_as, >+ self.sd_utils_dc1.get_sd_as_sddl(child.dn)) > self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(child.dn), > self.sd_utils_dc2.get_sd_as_sddl(child.dn)) >-- >2.17.1 > > >From b349264efa3f96e4f41d9578e57f19884cfb7351 Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Tue, 26 Nov 2019 15:44:32 +1300 >Subject: [PATCH 04/10] CVE-2019-14902 dsdb: Explain that > descriptor_sd_propagation_recursive() is proctected by a transaction > >This means we can trust the DB did not change between the two search >requests. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=12497 > >Signed-off-by: Andrew Bartlett <abartlet@samba.org> >--- > source4/dsdb/samdb/ldb_modules/descriptor.c | 3 +++ > 1 file changed, 3 insertions(+) > >diff --git a/source4/dsdb/samdb/ldb_modules/descriptor.c b/source4/dsdb/samdb/ldb_modules/descriptor.c >index 9018b750ab5..fb2854438e1 100644 >--- a/source4/dsdb/samdb/ldb_modules/descriptor.c >+++ b/source4/dsdb/samdb/ldb_modules/descriptor.c >@@ -1199,6 +1199,9 @@ static int descriptor_sd_propagation_recursive(struct ldb_module *module, > * LDB_SCOPE_SUBTREE searches are expensive. > * > * Note: that we do not search for deleted/recycled objects >+ * >+ * We know this is safe against a rename race as we are in the >+ * prepare_commit(), so must be in a transaction. > */ > ret = dsdb_module_search(module, > change, >-- >2.17.1 > > >From 59e542a1787d5b30e7099bd10aacd3144847acc0 Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Tue, 26 Nov 2019 16:17:32 +1300 >Subject: [PATCH 05/10] CVE-2019-14902 dsdb: Add comments explaining why SD > propagation needs to be done here > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=12497 > >Signed-off-by: Andrew Bartlett <abartlet@samba.org> >--- > source4/dsdb/samdb/ldb_modules/descriptor.c | 7 +++++++ > 1 file changed, 7 insertions(+) > >diff --git a/source4/dsdb/samdb/ldb_modules/descriptor.c b/source4/dsdb/samdb/ldb_modules/descriptor.c >index fb2854438e1..7070affa645 100644 >--- a/source4/dsdb/samdb/ldb_modules/descriptor.c >+++ b/source4/dsdb/samdb/ldb_modules/descriptor.c >@@ -876,6 +876,9 @@ static int descriptor_modify(struct ldb_module *module, struct ldb_request *req) > return ldb_oom(ldb); > } > >+ /* >+ * Force SD propagation on children of this record >+ */ > ret = dsdb_module_schedule_sd_propagation(module, nc_root, > dn, false); > if (ret != LDB_SUCCESS) { >@@ -966,6 +969,10 @@ static int descriptor_rename(struct ldb_module *module, struct ldb_request *req) > return ldb_oom(ldb); > } > >+ /* >+ * Force SD propagation on this record (get a new >+ * inherited SD from the potentially new parent >+ */ > ret = dsdb_module_schedule_sd_propagation(module, nc_root, > newdn, true); > if (ret != LDB_SUCCESS) { >-- >2.17.1 > > >From 1270fc09e7731f9e8b75920f7c0ea90c81b139ca Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Fri, 6 Dec 2019 17:54:23 +1300 >Subject: [PATCH 06/10] CVE-2019-14902 dsdb: Ensure we honour both > change->force_self and change->force_children > >If we are renaming a DN we can be in a situation where we need to > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=12497 > >Signed-off-by: Andrew Bartlett <abartlet@samba.org> >--- > source4/dsdb/samdb/ldb_modules/descriptor.c | 7 +++++++ > 1 file changed, 7 insertions(+) > >diff --git a/source4/dsdb/samdb/ldb_modules/descriptor.c b/source4/dsdb/samdb/ldb_modules/descriptor.c >index 7070affa645..b9f465fc36f 100644 >--- a/source4/dsdb/samdb/ldb_modules/descriptor.c >+++ b/source4/dsdb/samdb/ldb_modules/descriptor.c >@@ -1291,6 +1291,13 @@ static int descriptor_sd_propagation_recursive(struct ldb_module *module, > > if (cur != NULL) { > DLIST_REMOVE(change->children, cur); >+ } else if (i == 0) { >+ /* >+ * in the change->force_self case >+ * res->msgs[0]->elements was not overwritten, >+ * so set cur here >+ */ >+ cur = change; > } > > for (c = stopped_stack; c; c = stopped_stack) { >-- >2.17.1 > > >From 3ad0c9ff10e54e065c40c0c2623e88567053790b Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Fri, 6 Dec 2019 18:05:54 +1300 >Subject: [PATCH 07/10] CVE-2019-14902 repl_meta_data: schedule SD propagation > to a renamed DN > >We need to check the SD of the parent if we rename, it is not the same as an incoming SD change. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=12497 > >Signed-off-by: Andrew Bartlett <abartlet@samba.org> >--- > source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 17 ++++++++++++++++- > 1 file changed, 16 insertions(+), 1 deletion(-) > >diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c >index 04a51ecab51..52ff3d75ee2 100644 >--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c >+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c >@@ -6290,7 +6290,22 @@ static int replmd_replicated_apply_merge(struct replmd_replicated_request *ar) > ar->index_current, msg->num_elements); > > if (renamed) { >- sd_updated = true; >+ /* >+ * This is an new name for this object, so we must >+ * inherit from the parent >+ * >+ * This is needed because descriptor is above >+ * repl_meta_data in the module stack, so this will >+ * not be trigered 'naturally' by the flow of >+ * operations. >+ */ >+ ret = dsdb_module_schedule_sd_propagation(ar->module, >+ ar->objs->partition_dn, >+ msg->dn, >+ true); >+ if (ret != LDB_SUCCESS) { >+ return ldb_operr(ldb); >+ } > } > > if (sd_updated && !isDeleted) { >-- >2.17.1 > > >From 0754135b09bba6dd62e5607d0f38ada93e425d93 Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Tue, 26 Nov 2019 15:50:35 +1300 >Subject: [PATCH 08/10] CVE-2019-14902 repl_meta_data: Fix issue where > inherited Security Descriptors were not replicated. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=12497 > >Signed-off-by: Andrew Bartlett <abartlet@samba.org> >--- > selftest/knownfail.d/repl_secdesc | 1 - > .../dsdb/samdb/ldb_modules/repl_meta_data.c | 22 ++++++++++++++++++- > 2 files changed, 21 insertions(+), 2 deletions(-) > >diff --git a/selftest/knownfail.d/repl_secdesc b/selftest/knownfail.d/repl_secdesc >index 7d554ff237a..13a9ce458dd 100644 >--- a/selftest/knownfail.d/repl_secdesc >+++ b/selftest/knownfail.d/repl_secdesc >@@ -1,3 +1,2 @@ > ^samba4.drs.repl_secdesc.python\(.*\).repl_secdesc.ReplAclTestCase.test_acl_inheirt_renamed_object_in_conflict >-^samba4.drs.repl_secdesc.python\(.*\).repl_secdesc.ReplAclTestCase.test_acl_inherit_existing_object > ^samba4.drs.repl_secdesc.python\(.*\).repl_secdesc.ReplAclTestCase.test_acl_inheirt_renamed_child_object >diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c >index 52ff3d75ee2..9812ded99fb 100644 >--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c >+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c >@@ -5527,6 +5527,15 @@ static int replmd_replicated_apply_add(struct replmd_replicated_request *ar) > replmd_ldb_message_sort(msg, ar->schema); > > if (!remote_isDeleted) { >+ /* >+ * Ensure any local ACL inheritence is applied from >+ * the parent object. >+ * >+ * This is needed because descriptor is above >+ * repl_meta_data in the module stack, so this will >+ * not be trigered 'naturally' by the flow of >+ * operations. >+ */ > ret = dsdb_module_schedule_sd_propagation(ar->module, > ar->objs->partition_dn, > msg->dn, true); >@@ -6309,9 +6318,20 @@ static int replmd_replicated_apply_merge(struct replmd_replicated_request *ar) > } > > if (sd_updated && !isDeleted) { >+ /* >+ * This is an existing object, so there is no need to >+ * inherit from the parent, but we must inherit any >+ * incoming changes to our child objects. >+ * >+ * This is needed because descriptor is above >+ * repl_meta_data in the module stack, so this will >+ * not be trigered 'naturally' by the flow of >+ * operations. >+ */ > ret = dsdb_module_schedule_sd_propagation(ar->module, > ar->objs->partition_dn, >- msg->dn, true); >+ msg->dn, >+ false); > if (ret != LDB_SUCCESS) { > return ldb_operr(ldb); > } >-- >2.17.1 > > >From 313b30ef4a1483e6599306aa9a6fdb4b4cf674c8 Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Fri, 6 Dec 2019 18:26:42 +1300 >Subject: [PATCH 09/10] CVE-2019-14902 repl_meta_data: Set renamed = true (and > so do SD inheritance) after any rename > >Previously if there was a conflict, but the incoming object would still >win, this was not marked as a rename, and so inheritence was not done. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=12497 > >Signed-off-by: Andrew Bartlett <abartlet@samba.org> >--- > selftest/knownfail.d/repl_secdesc | 1 - > source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 13 +++++++++++++ > 2 files changed, 13 insertions(+), 1 deletion(-) > >diff --git a/selftest/knownfail.d/repl_secdesc b/selftest/knownfail.d/repl_secdesc >index 13a9ce458dd..9dd632d99ed 100644 >--- a/selftest/knownfail.d/repl_secdesc >+++ b/selftest/knownfail.d/repl_secdesc >@@ -1,2 +1 @@ >-^samba4.drs.repl_secdesc.python\(.*\).repl_secdesc.ReplAclTestCase.test_acl_inheirt_renamed_object_in_conflict > ^samba4.drs.repl_secdesc.python\(.*\).repl_secdesc.ReplAclTestCase.test_acl_inheirt_renamed_child_object >diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c >index 9812ded99fb..e67c3b0281e 100644 >--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c >+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c >@@ -6134,6 +6134,19 @@ static int replmd_replicated_apply_merge(struct replmd_replicated_request *ar) > * replmd_replicated_apply_search_callback()) > */ > ret = replmd_replicated_handle_rename(ar, msg, ar->req, &renamed); >+ >+ /* >+ * This looks strange, but we must set this after any >+ * rename, otherwise the SD propegation will not >+ * happen (which might matter if we have a new parent) >+ * >+ * The additional case of calling >+ * replmd_op_name_modify_callback (below) is: >+ * - a no-op if there was no name change >+ * and >+ * - called in the default case regardless. >+ */ >+ renamed = true; > } > > if (ret != LDB_SUCCESS) { >-- >2.17.1 > > >From e1cc844bb8478c2966f2e3626ccd16a1b8a428d9 Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Thu, 12 Dec 2019 14:44:57 +1300 >Subject: [PATCH 10/10] CVE-2019-14902 dsdb: Change basis of descriptor module > deferred processing to be GUIDs > >We can not process on the basis of a DN, as the DN may have changed in a rename, >not only that this module can see, but also from repl_meta_data below. > >Therefore remove all the complex tree-based change processing, leaving only >a tree-based sort of the possible objects to be changed, and a single >stopped_dn variable containing the DN to stop processing below (after >a no-op change). > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=12497 > >Signed-off-by: Andrew Bartlett <abartlet@samba.org> >--- > selftest/knownfail.d/repl_secdesc | 1 - > source4/dsdb/samdb/ldb_modules/acl_util.c | 4 +- > source4/dsdb/samdb/ldb_modules/descriptor.c | 296 +++++++++--------- > .../dsdb/samdb/ldb_modules/repl_meta_data.c | 7 +- > source4/dsdb/samdb/samdb.h | 2 +- > 5 files changed, 156 insertions(+), 154 deletions(-) > delete mode 100644 selftest/knownfail.d/repl_secdesc > >diff --git a/selftest/knownfail.d/repl_secdesc b/selftest/knownfail.d/repl_secdesc >deleted file mode 100644 >index 9dd632d99ed..00000000000 >--- a/selftest/knownfail.d/repl_secdesc >+++ /dev/null >@@ -1 +0,0 @@ >-^samba4.drs.repl_secdesc.python\(.*\).repl_secdesc.ReplAclTestCase.test_acl_inheirt_renamed_child_object >diff --git a/source4/dsdb/samdb/ldb_modules/acl_util.c b/source4/dsdb/samdb/ldb_modules/acl_util.c >index 6d645b10fe2..b9931795e19 100644 >--- a/source4/dsdb/samdb/ldb_modules/acl_util.c >+++ b/source4/dsdb/samdb/ldb_modules/acl_util.c >@@ -286,7 +286,7 @@ uint32_t dsdb_request_sd_flags(struct ldb_request *req, bool *explicit) > > int dsdb_module_schedule_sd_propagation(struct ldb_module *module, > struct ldb_dn *nc_root, >- struct ldb_dn *dn, >+ struct GUID guid, > bool include_self) > { > struct ldb_context *ldb = ldb_module_get_ctx(module); >@@ -299,7 +299,7 @@ int dsdb_module_schedule_sd_propagation(struct ldb_module *module, > } > > op->nc_root = nc_root; >- op->dn = dn; >+ op->guid = guid; > op->include_self = include_self; > > ret = dsdb_module_extended(module, op, NULL, >diff --git a/source4/dsdb/samdb/ldb_modules/descriptor.c b/source4/dsdb/samdb/ldb_modules/descriptor.c >index b9f465fc36f..daa08c2ebc7 100644 >--- a/source4/dsdb/samdb/ldb_modules/descriptor.c >+++ b/source4/dsdb/samdb/ldb_modules/descriptor.c >@@ -46,9 +46,8 @@ > > struct descriptor_changes { > struct descriptor_changes *prev, *next; >- struct descriptor_changes *children; > struct ldb_dn *nc_root; >- struct ldb_dn *dn; >+ struct GUID guid; > bool force_self; > bool force_children; > struct ldb_dn *stopped_dn; >@@ -771,7 +770,8 @@ static int descriptor_modify(struct ldb_module *module, struct ldb_request *req) > current_attrs, > DSDB_FLAG_NEXT_MODULE | > DSDB_FLAG_AS_SYSTEM | >- DSDB_SEARCH_SHOW_RECYCLED, >+ DSDB_SEARCH_SHOW_RECYCLED | >+ DSDB_SEARCH_SHOW_EXTENDED_DN, > req); > if (ret != LDB_SUCCESS) { > ldb_debug(ldb, LDB_DEBUG_ERROR,"descriptor_modify: Could not find %s\n", >@@ -832,7 +832,7 @@ static int descriptor_modify(struct ldb_module *module, struct ldb_request *req) > user_sd = old_sd; > } > >- sd = get_new_descriptor(module, dn, req, >+ sd = get_new_descriptor(module, current_res->msgs[0]->dn, req, > objectclass, parent_sd, > user_sd, old_sd, sd_flags); > if (sd == NULL) { >@@ -869,18 +869,32 @@ static int descriptor_modify(struct ldb_module *module, struct ldb_request *req) > return ldb_oom(ldb); > } > } else if (cmp_ret != 0) { >+ struct GUID guid; > struct ldb_dn *nc_root; >+ NTSTATUS status; > >- ret = dsdb_find_nc_root(ldb, msg, dn, &nc_root); >+ ret = dsdb_find_nc_root(ldb, >+ msg, >+ current_res->msgs[0]->dn, >+ &nc_root); > if (ret != LDB_SUCCESS) { > return ldb_oom(ldb); > } > >+ status = dsdb_get_extended_dn_guid(current_res->msgs[0]->dn, >+ &guid, >+ "GUID"); >+ if (!NT_STATUS_IS_OK(status)) { >+ return ldb_operr(ldb); >+ } >+ > /* > * Force SD propagation on children of this record > */ >- ret = dsdb_module_schedule_sd_propagation(module, nc_root, >- dn, false); >+ ret = dsdb_module_schedule_sd_propagation(module, >+ nc_root, >+ guid, >+ false); > if (ret != LDB_SUCCESS) { > return ldb_operr(ldb); > } >@@ -963,20 +977,31 @@ static int descriptor_rename(struct ldb_module *module, struct ldb_request *req) > > 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); > } > >- /* >- * Force SD propagation on this record (get a new >- * inherited SD from the potentially new parent >- */ >- ret = dsdb_module_schedule_sd_propagation(module, nc_root, >- newdn, true); >- if (ret != LDB_SUCCESS) { >- return ldb_operr(ldb); >+ 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 >+ */ >+ ret = dsdb_module_schedule_sd_propagation(module, >+ nc_root, >+ guid, >+ true); >+ if (ret != LDB_SUCCESS) { >+ return ldb_operr(ldb); >+ } > } > } > >@@ -992,9 +1017,7 @@ static int descriptor_extended_sec_desc_propagation(struct ldb_module *module, > struct ldb_context *ldb = ldb_module_get_ctx(module); > struct dsdb_extended_sec_desc_propagation_op *op; > TALLOC_CTX *parent_mem = NULL; >- struct descriptor_changes *parent_change = NULL; > struct descriptor_changes *c; >- int ret; > > op = talloc_get_type(req->op.extended.data, > struct dsdb_extended_sec_desc_propagation_op); >@@ -1011,32 +1034,6 @@ static int descriptor_extended_sec_desc_propagation(struct ldb_module *module, > > parent_mem = descriptor_private->trans_mem; > >- for (c = descriptor_private->changes; c; c = c->next) { >- ret = ldb_dn_compare(c->nc_root, op->nc_root); >- if (ret != 0) { >- continue; >- } >- >- ret = ldb_dn_compare(c->dn, op->dn); >- if (ret == 0) { >- if (op->include_self) { >- c->force_self = true; >- } else { >- c->force_children = true; >- } >- return ldb_module_done(req, NULL, NULL, LDB_SUCCESS); >- } >- >- ret = ldb_dn_compare_base(c->dn, op->dn); >- if (ret != 0) { >- continue; >- } >- >- parent_mem = c; >- parent_change = c; >- break; >- } >- > c = talloc_zero(parent_mem, struct descriptor_changes); > if (c == NULL) { > return ldb_module_oom(module); >@@ -1045,21 +1042,14 @@ static int descriptor_extended_sec_desc_propagation(struct ldb_module *module, > if (c->nc_root == NULL) { > return ldb_module_oom(module); > } >- c->dn = ldb_dn_copy(c, op->dn); >- if (c->dn == NULL) { >- return ldb_module_oom(module); >- } >+ c->guid = op->guid; > if (op->include_self) { > c->force_self = true; > } else { > c->force_children = true; > } > >- if (parent_change != NULL) { >- DLIST_ADD_END(parent_change->children, c); >- } else { >- DLIST_ADD_END(descriptor_private->changes, c); >- } >+ DLIST_ADD_END(descriptor_private->changes, c); > > return ldb_module_done(req, NULL, NULL, LDB_SUCCESS); > } >@@ -1179,41 +1169,75 @@ static int descriptor_sd_propagation_msg_sort(struct ldb_message **m1, > return ldb_dn_compare(dn2, dn1); > } > >-static int descriptor_sd_propagation_dn_sort(struct ldb_dn *dn1, >- struct ldb_dn *dn2) >-{ >- /* >- * This sorts in tree order, parents first >- */ >- return ldb_dn_compare(dn2, dn1); >-} >- > static int descriptor_sd_propagation_recursive(struct ldb_module *module, > struct descriptor_changes *change) > { >- struct ldb_context *ldb = ldb_module_get_ctx(module); >+ struct ldb_result *guid_res = NULL; > struct ldb_result *res = NULL; > unsigned int i; > const char * const no_attrs[] = { "@__NONE__", NULL }; >- struct descriptor_changes *c; >- struct descriptor_changes *stopped_stack = NULL; >- enum ldb_scope scope; >+ struct ldb_dn *stopped_dn = NULL; >+ struct GUID_txt_buf guid_buf; > int ret; >+ bool stop = false; > > /* >- * First confirm this object has children, or exists (depending on change->force_self) >+ * First confirm this object has children, or exists >+ * (depending on change->force_self) > * > * LDB_SCOPE_SUBTREE searches are expensive. > * >- * Note: that we do not search for deleted/recycled objects >- * > * We know this is safe against a rename race as we are in the > * prepare_commit(), so must be in a transaction. > */ >+ >+ /* Find the DN by GUID, as this is stable under rename */ >+ ret = dsdb_module_search(module, >+ change, >+ &guid_res, >+ change->nc_root, >+ LDB_SCOPE_SUBTREE, >+ no_attrs, >+ DSDB_FLAG_NEXT_MODULE | >+ DSDB_FLAG_AS_SYSTEM | >+ DSDB_SEARCH_SHOW_DELETED | >+ DSDB_SEARCH_SHOW_RECYCLED, >+ NULL, /* parent_req */ >+ "(objectGUID=%s)", >+ GUID_buf_string(&change->guid, >+ &guid_buf)); >+ >+ if (ret != LDB_SUCCESS) { >+ return ret; >+ } >+ >+ if (guid_res->count != 1) { >+ /* >+ * We were just given this GUID during the same >+ * transaction, if it is missing this is a big >+ * problem. >+ * >+ * Cleanup of tombstones does not trigger this module >+ * as it just does a delete. >+ */ >+ ldb_asprintf_errstring(ldb_module_get_ctx(module), >+ "failed to find GUID %s under %s " >+ "for transaction-end SD inheritance: %d results", >+ GUID_buf_string(&change->guid, >+ &guid_buf), >+ ldb_dn_get_linearized(change->nc_root), >+ guid_res->count); >+ return LDB_ERR_OPERATIONS_ERROR; >+ } >+ >+ /* >+ * OK, so there was a parent, are there children? Note: that >+ * this time we do not search for deleted/recycled objects >+ */ > ret = dsdb_module_search(module, > change, > &res, >- change->dn, >+ guid_res->msgs[0]->dn, > LDB_SCOPE_ONELEVEL, > no_attrs, > DSDB_FLAG_NEXT_MODULE | >@@ -1221,26 +1245,55 @@ static int descriptor_sd_propagation_recursive(struct ldb_module *module, > NULL, /* parent_req */ > "(objectClass=*)"); > if (ret != LDB_SUCCESS) { >+ /* >+ * LDB_ERR_NO_SUCH_OBJECT, say if the DN was a deleted >+ * object, is ignored by the caller >+ */ > return ret; > } > > if (res->count == 0 && !change->force_self) { >+ /* All done, no children */ > TALLOC_FREE(res); > return LDB_SUCCESS; >- } else if (res->count == 0 && change->force_self) { >- scope = LDB_SCOPE_BASE; >- } else { >- scope = LDB_SCOPE_SUBTREE; > } > > /* >+ * First, if we are in force_self mode (eg renamed under new >+ * parent) then apply the SD to the top object >+ */ >+ if (change->force_self) { >+ ret = descriptor_sd_propagation_object(module, >+ guid_res->msgs[0], >+ &stop); >+ if (ret != LDB_SUCCESS) { >+ TALLOC_FREE(guid_res); >+ return ret; >+ } >+ >+ if (stop == true && !change->force_children) { >+ /* There was no change, nothing more to do */ >+ TALLOC_FREE(guid_res); >+ return LDB_SUCCESS; >+ } >+ >+ if (res->count == 0) { >+ /* All done! */ >+ TALLOC_FREE(guid_res); >+ return LDB_SUCCESS; >+ } >+ } >+ >+ /* >+ * Look for children >+ * > * Note: that we do not search for deleted/recycled objects > */ > ret = dsdb_module_search(module, > change, > &res, >- change->dn, >- scope, >+ guid_res->msgs[0]->dn, >+ LDB_SCOPE_SUBTREE, > no_attrs, > DSDB_FLAG_NEXT_MODULE | > DSDB_FLAG_AS_SYSTEM, >@@ -1253,90 +1306,39 @@ static int descriptor_sd_propagation_recursive(struct ldb_module *module, > TYPESAFE_QSORT(res->msgs, res->count, > descriptor_sd_propagation_msg_sort); > >- for (c = change->children; c; c = c->next) { >- struct ldb_message *msg = NULL; >- >- BINARY_ARRAY_SEARCH_P(res->msgs, res->count, dn, c->dn, >- descriptor_sd_propagation_dn_sort, >- msg); >- >- if (msg == NULL) { >- ldb_debug(ldb, LDB_DEBUG_WARNING, >- "descriptor_sd_propagation_recursive: " >- "%s not found under %s", >- ldb_dn_get_linearized(c->dn), >- ldb_dn_get_linearized(change->dn)); >- continue; >- } >- >- msg->elements = (struct ldb_message_element *)c; >- } >- >- DLIST_ADD(stopped_stack, change); >- >- if (change->force_self) { >- i = 0; >- } else { >- i = 1; >- } >- >- for (; i < res->count; i++) { >- struct descriptor_changes *cur; >- bool stop = false; >- >- cur = talloc_get_type(res->msgs[i]->elements, >- struct descriptor_changes); >- res->msgs[i]->elements = NULL; >- res->msgs[i]->num_elements = 0; >- >- if (cur != NULL) { >- DLIST_REMOVE(change->children, cur); >- } else if (i == 0) { >+ /* We start from 1, the top object has been done */ >+ for (i = 1; i < res->count; i++) { >+ /* >+ * ldb_dn_compare_base() does not match for NULL but >+ * this is clearer >+ */ >+ if (stopped_dn != NULL) { >+ ret = ldb_dn_compare_base(stopped_dn, >+ res->msgs[i]->dn); > /* >- * in the change->force_self case >- * res->msgs[0]->elements was not overwritten, >- * so set cur here >+ * Skip further processing of this >+ * sub-subtree > */ >- cur = change; >- } >- >- for (c = stopped_stack; c; c = stopped_stack) { >- ret = ldb_dn_compare_base(c->dn, >- res->msgs[i]->dn); >- if (ret == 0) { >- break; >- } >- >- c->stopped_dn = NULL; >- DLIST_REMOVE(stopped_stack, c); >- } >- >- if (cur != NULL) { >- DLIST_ADD(stopped_stack, cur); >- } >- >- if (stopped_stack->stopped_dn != NULL) { >- ret = ldb_dn_compare_base(stopped_stack->stopped_dn, >- res->msgs[i]->dn); > if (ret == 0) { > continue; > } >- stopped_stack->stopped_dn = NULL; > } >- >- ret = descriptor_sd_propagation_object(module, res->msgs[i], >+ ret = descriptor_sd_propagation_object(module, >+ res->msgs[i], > &stop); > if (ret != LDB_SUCCESS) { > return ret; > } > >- if (cur != NULL && cur->force_children) { >- continue; >- } >- > if (stop) { >- stopped_stack->stopped_dn = res->msgs[i]->dn; >- continue; >+ /* >+ * If this child didn't change, then nothing >+ * under it needs to change >+ * >+ * res has been sorted into tree order so the >+ * next few entries can be skipped >+ */ >+ stopped_dn = res->msgs[i]->dn; > } > } > >diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c >index e67c3b0281e..a2a6bcc98f3 100644 >--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c >+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c >@@ -5538,7 +5538,8 @@ static int replmd_replicated_apply_add(struct replmd_replicated_request *ar) > */ > ret = dsdb_module_schedule_sd_propagation(ar->module, > ar->objs->partition_dn, >- msg->dn, true); >+ ar->objs->objects[ar->index_current].object_guid, >+ true); > if (ret != LDB_SUCCESS) { > return replmd_replicated_request_error(ar, ret); > } >@@ -6323,7 +6324,7 @@ static int replmd_replicated_apply_merge(struct replmd_replicated_request *ar) > */ > ret = dsdb_module_schedule_sd_propagation(ar->module, > ar->objs->partition_dn, >- msg->dn, >+ ar->objs->objects[ar->index_current].object_guid, > true); > if (ret != LDB_SUCCESS) { > return ldb_operr(ldb); >@@ -6343,7 +6344,7 @@ static int replmd_replicated_apply_merge(struct replmd_replicated_request *ar) > */ > ret = dsdb_module_schedule_sd_propagation(ar->module, > ar->objs->partition_dn, >- msg->dn, >+ ar->objs->objects[ar->index_current].object_guid, > false); > if (ret != LDB_SUCCESS) { > return ldb_operr(ldb); >diff --git a/source4/dsdb/samdb/samdb.h b/source4/dsdb/samdb/samdb.h >index e1b0e4aa4e3..3f47b863a83 100644 >--- a/source4/dsdb/samdb/samdb.h >+++ b/source4/dsdb/samdb/samdb.h >@@ -338,7 +338,7 @@ struct dsdb_extended_allocate_rid { > #define DSDB_EXTENDED_SEC_DESC_PROPAGATION_OID "1.3.6.1.4.1.7165.4.4.7" > struct dsdb_extended_sec_desc_propagation_op { > struct ldb_dn *nc_root; >- struct ldb_dn *dn; >+ struct GUID guid; > bool include_self; > }; > >-- >2.17.1 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Flags:
abartlet
:
review?
(
gary
)
gary
:
review+
abartlet
:
ci-passed+
Actions:
View
Attachments on
bug 12497
:
15633
|
15670
|
15678
|
15679
|
15680
|
15681
|
15685
|
15686
|
15687
| 15688 |
15689
|
15706
|
15708
|
15738