diff options
author | Natanael Copa <ncopa@alpinelinux.org> | 2020-04-03 07:50:55 +0200 |
---|---|---|
committer | Natanael Copa <ncopa@alpinelinux.org> | 2020-04-03 07:50:55 +0200 |
commit | 515e98e6bdc49a7db3aa081e1de4fd5f727bb851 (patch) | |
tree | aabffd88c4f87869f6b18ef14129c881215e2c85 /main | |
parent | 4da1ee1a718f0e9dfd6a6e91f9348fa96a58567d (diff) | |
download | aports-515e98e6bdc49a7db3aa081e1de4fd5f727bb851.tar.bz2 aports-515e98e6bdc49a7db3aa081e1de4fd5f727bb851.tar.xz |
main/samba: fix CVE-2019-14902, CVE-2019-14907
fixes #11155
Diffstat (limited to 'main')
-rw-r--r-- | main/samba/APKBUILD | 7 | ||||
-rw-r--r-- | main/samba/samba-4.9.17-security-2020-01-21.patch | 1662 |
2 files changed, 1668 insertions, 1 deletions
diff --git a/main/samba/APKBUILD b/main/samba/APKBUILD index 6543808e76..50d1748996 100644 --- a/main/samba/APKBUILD +++ b/main/samba/APKBUILD @@ -1,7 +1,7 @@ # Maintainer: Natanael Copa <ncopa@alpinelinux.org> pkgname=samba pkgver=4.8.12 -pkgrel=1 +pkgrel=2 pkgdesc="Tools to access a server's filespace and printers via SMB" url="https://www.samba.org/" arch="all" @@ -51,6 +51,7 @@ source="https://us1.samba.org/samba/ftp/stable/$pkgname-$pkgver.tar.gz netdb-defines.patch netapp.patch samba-4.9.14-security-2019-10-29.patch + samba-4.9.17-security-2020-01-21.patch $pkgname.initd $pkgname.confd $pkgname.logrotate @@ -59,6 +60,9 @@ pkggroups="winbind" builddir="$srcdir/$pkgname-$pkgver" # secfixes: +# 4.8.12-r2: +# - CVE-2019-14902 +# - CVE-2019-14907 # 4.8.12-r1: # - CVE-2019-10218 # - CVE-2019-14833 @@ -539,6 +543,7 @@ a99e771f28d787dc22e832b97aa48a1c5e13ddc0c030c501a3c12819ff6e62800ef084b62930abe8 1854577d0e4457e27da367a6c7ec0fb5cfd63cefea0a39181c9d6e78cf8d3eb50878cdddeea3daeec955d00263151c2f86ea754ff4276ef98bc52c0276d9ffe8 netdb-defines.patch 202667cb0383414d9289cd67574f5e1140c9a0ff63bb82a746a59b2397a00db15654bfb30cb5ec1cd68a097899be0f849d9aab4c0d210152386c9e66c640f0c0 netapp.patch 8386db1209721fabb6acf52e498082ac3e70cd3a4454c54416b02aaa67b2906212383da7ddc06f77ca29cfbb9033407b1e958bcd9c7cdf369fe501f310a0f973 samba-4.9.14-security-2019-10-29.patch +b00163634fb262777cc8992192150beb5dc2dc45ace823557f1a35fe2448ab3559b7503db96b07c6a9382ddb62a3bd6f4e68e1849f64ec472dbea8abc6b54572 samba-4.9.17-security-2020-01-21.patch 8ffd93a2f8f1461d3eabf3cbc67f04f13d18d02b969e110b1cf5f23845264d774a7a79658cdf389bc594780c633266ff29aacc81dae627dcb3efe63364b027bd samba.initd 4faf581ecef3ec38319e3c4ab6d3995c51fd7ba83180dc5553a2ff4dfb92efadb43030c543292130c4ed0c281dc0972c6973d52d48062c5edb39bb1c4bbb6dd6 samba.confd 3458a4e1f8a8b44c966afb339b2dca51615be049f594c14911fc4d8203623deee416b6fe881436e246fc7d49c97a2b3bf9c5f33ba774302b24190a1103d6b67d samba.logrotate" diff --git a/main/samba/samba-4.9.17-security-2020-01-21.patch b/main/samba/samba-4.9.17-security-2020-01-21.patch new file mode 100644 index 0000000000..4847a8660b --- /dev/null +++ b/main/samba/samba-4.9.17-security-2020-01-21.patch @@ -0,0 +1,1662 @@ +From 77d55b64af6acd38a08096b89ee051bc4ce72f43 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/13] 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 c5a005a45389c8d8fc0eae7137eab1904ea92d42 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/13] 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 4afff32debe5ea4bf1219f42c3042eb65c3e1d6b 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/13] 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 17215b36b22d309a58a3b7bd08123f06e89657c9 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/13] 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 589d1e4846bbac0e5388af3ef0c6d6c41b5ff991 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/13] 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 0fa9a362e55abb289cbf0fe24baa09c45af4837e 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/13] 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 9ac2b09fa5a2de44967a0b190918825e7dca8d53 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/13] 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 9e6b09e0fd52c664de7f0589074fef872c753fa2 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/13] 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 7071888d5b556213be79545cac059a8b3f62baee 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/13] 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 16b377276ee82c04d069666e53deaa95a7633dd4 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/13] 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 + + +From 030fa9e5455125e30b71c90be80baadb657d8993 Mon Sep 17 00:00:00 2001 +From: Noel Power <noel.power@suse.com> +Date: Fri, 24 May 2019 13:37:00 +0000 +Subject: [PATCH 11/13] CVE-2019-14907 lib/util/charset: clang: Fix Value + stored to 'reason' is never read warning + +Fixes: + +lib/util/charset/convert_string.c:301:5: warning: Value stored to 'reason' is never read <--[clang] + +BUG: https://bugzilla.samba.org/show_bug.cgi?id=14208 + +Signed-off-by: Noel Power <noel.power@suse.com> +Reviewed-by: Gary Lockyer gary@catalyst.net.nz +(cherry picked from commit add47e288bc80c1bf45765d1588a9fa5998ea677) +--- + lib/util/charset/convert_string.c | 8 ++++---- + 1 file changed, 4 insertions(+), 4 deletions(-) + +diff --git a/lib/util/charset/convert_string.c b/lib/util/charset/convert_string.c +index 196302aacfd..34facab6fe6 100644 +--- a/lib/util/charset/convert_string.c ++++ b/lib/util/charset/convert_string.c +@@ -300,13 +300,13 @@ bool convert_string_handle(struct smb_iconv_handle *ic, + { + reason="No more room"; + if (from == CH_UNIX) { +- DEBUG(3,("E2BIG: convert_string(%s,%s): srclen=%u destlen=%u - '%s'\n", ++ DEBUG(3,("E2BIG: convert_string(%s,%s): srclen=%u destlen=%u - '%s' error: %s\n", + charset_name(ic, from), charset_name(ic, to), +- (unsigned int)srclen, (unsigned int)destlen, (const char *)src)); ++ (unsigned int)srclen, (unsigned int)destlen, (const char *)src, reason)); + } else { +- DEBUG(3,("E2BIG: convert_string(%s,%s): srclen=%u destlen=%u\n", ++ DEBUG(3,("E2BIG: convert_string(%s,%s): srclen=%u destlen=%u error: %s\n", + charset_name(ic, from), charset_name(ic, to), +- (unsigned int)srclen, (unsigned int)destlen)); ++ (unsigned int)srclen, (unsigned int)destlen, reason)); + } + break; + } +-- +2.17.1 + + +From ad0e68d354ad33c577dbf146fc4a1b8254857558 Mon Sep 17 00:00:00 2001 +From: Andrew Bartlett <abartlet@samba.org> +Date: Fri, 29 Nov 2019 20:58:47 +1300 +Subject: [PATCH 12/13] CVE-2019-14907 lib/util: Do not print the failed to + convert string into the logs +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +The string may be in another charset, or may be sensitive and +certainly may not be terminated. It is not safe to just print. + +Found by Robert Święcki using a fuzzer he wrote for smbd. + +BUG: https://bugzilla.samba.org/show_bug.cgi?id=14208 +Signed-off-by: Andrew Bartlett <abartlet@samba.org> + +(adapted from master commit) +--- + lib/util/charset/convert_string.c | 33 +++++++++++++++++-------------- + 1 file changed, 18 insertions(+), 15 deletions(-) + +diff --git a/lib/util/charset/convert_string.c b/lib/util/charset/convert_string.c +index 34facab6fe6..b546e056953 100644 +--- a/lib/util/charset/convert_string.c ++++ b/lib/util/charset/convert_string.c +@@ -293,31 +293,31 @@ bool convert_string_handle(struct smb_iconv_handle *ic, + switch(errno) { + case EINVAL: + reason="Incomplete multibyte sequence"; +- DEBUG(3,("convert_string_internal: Conversion error: %s(%s)\n", +- reason, (const char *)src)); ++ DBG_NOTICE("Conversion error: %s\n", ++ reason); + break; + case E2BIG: + { + reason="No more room"; + if (from == CH_UNIX) { +- DEBUG(3,("E2BIG: convert_string(%s,%s): srclen=%u destlen=%u - '%s' error: %s\n", +- charset_name(ic, from), charset_name(ic, to), +- (unsigned int)srclen, (unsigned int)destlen, (const char *)src, reason)); ++ DBG_NOTICE("E2BIG: convert_string(%s,%s): srclen=%u destlen=%u error: %s\n", ++ charset_name(ic, from), charset_name(ic, to), ++ (unsigned int)srclen, (unsigned int)destlen, reason); + } else { +- DEBUG(3,("E2BIG: convert_string(%s,%s): srclen=%u destlen=%u error: %s\n", +- charset_name(ic, from), charset_name(ic, to), +- (unsigned int)srclen, (unsigned int)destlen, reason)); ++ DBG_NOTICE("E2BIG: convert_string(%s,%s): srclen=%u destlen=%u error: %s\n", ++ charset_name(ic, from), charset_name(ic, to), ++ (unsigned int)srclen, (unsigned int)destlen, reason); + } + break; + } + case EILSEQ: + reason="Illegal multibyte sequence"; +- DEBUG(3,("convert_string_internal: Conversion error: %s(%s)\n", +- reason, (const char *)src)); ++ DBG_NOTICE("convert_string_internal: Conversion error: %s\n", ++ reason); + break; + default: +- DEBUG(0,("convert_string_internal: Conversion error: %s(%s)\n", +- reason, (const char *)src)); ++ DBG_ERR("convert_string_internal: Conversion error: %s\n", ++ reason); + break; + } + /* smb_panic(reason); */ +@@ -427,16 +427,19 @@ bool convert_string_talloc_handle(TALLOC_CTX *ctx, struct smb_iconv_handle *ic, + switch(errno) { + case EINVAL: + reason="Incomplete multibyte sequence"; +- DEBUG(3,("convert_string_talloc: Conversion error: %s(%s)\n",reason,inbuf)); ++ DBG_NOTICE("Conversion error: %s\n", ++ reason); + break; + case E2BIG: + goto convert; + case EILSEQ: + reason="Illegal multibyte sequence"; +- DEBUG(3,("convert_string_talloc: Conversion error: %s(%s)\n",reason,inbuf)); ++ DBG_NOTICE("Conversion error: %s\n", ++ reason); + break; + default: +- DEBUG(0,("Conversion error: %s(%s)\n",reason,inbuf)); ++ DBG_ERR("Conversion error: %s\n", ++ reason); + break; + } + /* smb_panic(reason); */ +-- +2.17.1 + + |