From d564a5c816642269e0b6d0b37319fd47646487c0 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 2 Mar 2023 16:51:25 +1300 Subject: CVE-2023-0614 lib/ldb-samba: Add test for SAMBA_LDAP_MATCH_RULE_TRANSITIVE_EVAL / LDAP_MATCHING_RULE_IN_CHAIN with and ACL hidden attributes The chain for transitive evaluation does consider ACLs, avoiding the disclosure of confidential information. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270 Signed-off-by: Andrew Bartlett Reviewed-by: Joseph Sutton --- lib/ldb-samba/tests/match_rules.py | 127 ++++++++++++++++-------------- lib/ldb-samba/tests/match_rules_remote.py | 104 ++++++++++++++++++++++++ source4/selftest/tests.py | 1 + 3 files changed, 175 insertions(+), 57 deletions(-) create mode 100755 lib/ldb-samba/tests/match_rules_remote.py diff --git a/lib/ldb-samba/tests/match_rules.py b/lib/ldb-samba/tests/match_rules.py index 2af1dd6a070..2fe6c3e2264 100755 --- a/lib/ldb-samba/tests/match_rules.py +++ b/lib/ldb-samba/tests/match_rules.py @@ -20,13 +20,18 @@ from ldb import SCOPE_BASE, SCOPE_SUBTREE, SCOPE_ONELEVEL # Windows appear to preserve casing of the RDN and uppercase the other keys. -class MatchRulesTests(samba.tests.TestCase): +class MatchRulesTestsBase(samba.tests.TestCase): def setUp(self): - super(MatchRulesTests, self).setUp() - self.lp = lp - self.ldb = SamDB(host, credentials=creds, session_info=system_session(lp), lp=lp) + super().setUp() + self.lp = self.sambaopts.get_loadparm() + self.creds = self.credopts.get_credentials(self.lp) + + self.ldb = SamDB(self.host, credentials=self.creds, + session_info=system_session(self.lp), + lp=self.lp) self.base_dn = self.ldb.domain_dn() - self.ou = "OU=matchrulestest,%s" % self.base_dn + self.ou_rdn = "OU=matchrulestest" + self.ou = self.ou_rdn + "," + self.base_dn self.ou_users = "OU=users,%s" % self.ou self.ou_groups = "OU=groups,%s" % self.ou self.ou_computers = "OU=computers,%s" % self.ou @@ -212,6 +217,39 @@ class MatchRulesTests(samba.tests.TestCase): FLAG_MOD_ADD, "member") self.ldb.modify(m) + # Add a couple of ms-Exch-Configuration-Container to test forward-link + # attributes without backward link (addressBookRoots2) + # e1 + # |--> e2 + # | |--> c1 + self.ldb.add({ + "dn": "cn=e1,%s" % self.ou, + "objectclass": "msExchConfigurationContainer"}) + self.ldb.add({ + "dn": "cn=e2,%s" % self.ou, + "objectclass": "msExchConfigurationContainer"}) + + m = Message() + m.dn = Dn(self.ldb, "cn=e2,%s" % self.ou) + m["e1"] = MessageElement("cn=c1,%s" % self.ou_computers, + FLAG_MOD_ADD, "addressBookRoots2") + self.ldb.modify(m) + + m = Message() + m.dn = Dn(self.ldb, "cn=e1,%s" % self.ou) + m["e1"] = MessageElement("cn=e2,%s" % self.ou, + FLAG_MOD_ADD, "addressBookRoots2") + self.ldb.modify(m) + + + +class MatchRulesTests(MatchRulesTestsBase): + def setUp(self): + self.sambaopts = sambaopts + self.credopts = credopts + self.host = host + super().setUp() + # The msDS-RevealedUsers is owned by system and cannot be modified # directly. Set the schemaUpgradeInProgress flag as workaround # and create this hierarchy: @@ -251,33 +289,6 @@ class MatchRulesTests(samba.tests.TestCase): m["e1"] = MessageElement("0", FLAG_MOD_REPLACE, "schemaUpgradeInProgress") self.ldb.modify(m) - # Add a couple of ms-Exch-Configuration-Container to test forward-link - # attributes without backward link (addressBookRoots2) - # e1 - # |--> e2 - # | |--> c1 - self.ldb.add({ - "dn": "cn=e1,%s" % self.ou, - "objectclass": "msExchConfigurationContainer"}) - self.ldb.add({ - "dn": "cn=e2,%s" % self.ou, - "objectclass": "msExchConfigurationContainer"}) - - m = Message() - m.dn = Dn(self.ldb, "cn=e2,%s" % self.ou) - m["e1"] = MessageElement("cn=c1,%s" % self.ou_computers, - FLAG_MOD_ADD, "addressBookRoots2") - self.ldb.modify(m) - - m = Message() - m.dn = Dn(self.ldb, "cn=e1,%s" % self.ou) - m["e1"] = MessageElement("cn=e2,%s" % self.ou, - FLAG_MOD_ADD, "addressBookRoots2") - self.ldb.modify(m) - - def tearDown(self): - super(MatchRulesTests, self).tearDown() - self.ldb.delete(self.ou, controls=['tree_delete:0']) def test_u1_member_of_g4(self): # Search without transitive match must return 0 results @@ -953,8 +964,12 @@ class MatchRulesTests(samba.tests.TestCase): class MatchRuleConditionTests(samba.tests.TestCase): def setUp(self): super(MatchRuleConditionTests, self).setUp() - self.lp = lp - self.ldb = SamDB(host, credentials=creds, session_info=system_session(lp), lp=lp) + self.lp = sambaopts.get_loadparm() + self.creds = credopts.get_credentials(self.lp) + + self.ldb = SamDB(host, credentials=self.creds, + session_info=system_session(self.lp), + lp=self.lp) self.base_dn = self.ldb.domain_dn() self.ou = "OU=matchruleconditiontests,%s" % self.base_dn self.ou_users = "OU=users,%s" % self.ou @@ -1753,32 +1768,30 @@ class MatchRuleConditionTests(samba.tests.TestCase): self.ou_groups, self.ou_computers)) self.assertEqual(len(res1), 0) +if __name__ == "__main__": -parser = optparse.OptionParser("match_rules.py [options] ") -sambaopts = options.SambaOptions(parser) -parser.add_option_group(sambaopts) -parser.add_option_group(options.VersionOptions(parser)) - -# use command line creds if available -credopts = options.CredentialsOptions(parser) -parser.add_option_group(credopts) -opts, args = parser.parse_args() -subunitopts = SubunitOptions(parser) -parser.add_option_group(subunitopts) + parser = optparse.OptionParser("match_rules.py [options] ") + sambaopts = options.SambaOptions(parser) + parser.add_option_group(sambaopts) + parser.add_option_group(options.VersionOptions(parser)) -if len(args) < 1: - parser.print_usage() - sys.exit(1) + # use command line creds if available + credopts = options.CredentialsOptions(parser) + parser.add_option_group(credopts) + opts, args = parser.parse_args() + subunitopts = SubunitOptions(parser) + parser.add_option_group(subunitopts) -host = args[0] + if len(args) < 1: + parser.print_usage() + sys.exit(1) -lp = sambaopts.get_loadparm() -creds = credopts.get_credentials(lp) + host = args[0] -if "://" not in host: - if os.path.isfile(host): - host = "tdb://%s" % host - else: - host = "ldap://%s" % host + if "://" not in host: + if os.path.isfile(host): + host = "tdb://%s" % host + else: + host = "ldap://%s" % host -TestProgram(module=__name__, opts=subunitopts) + TestProgram(module=__name__, opts=subunitopts) diff --git a/lib/ldb-samba/tests/match_rules_remote.py b/lib/ldb-samba/tests/match_rules_remote.py new file mode 100755 index 00000000000..122231f2a60 --- /dev/null +++ b/lib/ldb-samba/tests/match_rules_remote.py @@ -0,0 +1,104 @@ +#!/usr/bin/env python3 + +import optparse +import sys +import os +import samba +import samba.getopt as options + +from samba.tests.subunitrun import SubunitOptions, TestProgram + +from samba.samdb import SamDB +from samba.auth import system_session +from samba import sd_utils +from samba.ndr import ndr_unpack +from ldb import Message, MessageElement, Dn, LdbError +from ldb import FLAG_MOD_ADD, FLAG_MOD_REPLACE, FLAG_MOD_DELETE +from ldb import SCOPE_BASE, SCOPE_SUBTREE, SCOPE_ONELEVEL + +from match_rules import MatchRulesTestsBase + + +class MatchRulesTestsUser(MatchRulesTestsBase): + def setUp(self): + self.sambaopts = sambaopts + self.credopts = credopts + self.host = host + super().setUp() + self.sd_utils = sd_utils.SDUtils(self.ldb) + + self.user_pass = "samba123@" + self.match_test_user = "matchtestuser" + self.ldb.newuser(self.match_test_user, + self.user_pass, + userou=self.ou_rdn) + user_creds = self.insta_creds(template=self.creds, + username=self.match_test_user, + userpass=self.user_pass) + self.user_ldb = SamDB(host, credentials=user_creds, lp=self.lp) + token_res = self.user_ldb.search(scope=SCOPE_BASE, + base="", + attrs=["tokenGroups"]) + self.user_sid = ndr_unpack(samba.dcerpc.security.dom_sid, + token_res[0]["tokenGroups"][0]) + + self.member_attr_guid = "bf9679c0-0de6-11d0-a285-00aa003049e2" + + def test_with_denied_link(self): + + # add an ACE that denies the user Read Property (RP) access to + # the member attr (which is similar to making the attribute + # confidential) + ace = "(OD;;RP;{0};;{1})".format(self.member_attr_guid, + self.user_sid) + g2_dn = Dn(self.ldb, "CN=g2,%s" % self.ou_groups) + + # add the ACE that denies access to the attr under test + self.sd_utils.dacl_add_ace(g2_dn, ace) + + # Search without transitive match must return 0 results + res1 = self.ldb.search("cn=g4,%s" % self.ou_groups, + scope=SCOPE_BASE, + expression="member=cn=u1,%s" % self.ou_users) + self.assertEqual(len(res1), 0) + + # Search with transitive match must return 1 results + res1 = self.ldb.search("cn=g4,%s" % self.ou_groups, + scope=SCOPE_BASE, + expression="member:1.2.840.113556.1.4.1941:=cn=u1,%s" % self.ou_users) + self.assertEqual(len(res1), 1) + self.assertEqual(str(res1[0].dn).lower(), ("CN=g4,%s" % self.ou_groups).lower()) + + # Search as a user match must return 0 results as the intermediate link can't be seen + res1 = self.user_ldb.search("cn=g4,%s" % self.ou_groups, + scope=SCOPE_BASE, + expression="member:1.2.840.113556.1.4.1941:=cn=u1,%s" % self.ou_users) + self.assertEqual(len(res1), 0) + + + +parser = optparse.OptionParser("match_rules_remote.py [options] ") +sambaopts = options.SambaOptions(parser) +parser.add_option_group(sambaopts) +parser.add_option_group(options.VersionOptions(parser)) + +# use command line creds if available +credopts = options.CredentialsOptions(parser) +parser.add_option_group(credopts) +opts, args = parser.parse_args() +subunitopts = SubunitOptions(parser) +parser.add_option_group(subunitopts) + +if len(args) < 1: + parser.print_usage() + sys.exit(1) + +host = args[0] + +if "://" not in host: + if os.path.isfile(host): + host = "tdb://%s" % host + else: + host = "ldap://%s" % host + +TestProgram(module=__name__, opts=subunitopts) diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py index 774b874edbd..d75f421747a 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -1322,6 +1322,7 @@ for env in ['ad_dc_default:local', 'schema_dc:local']: plantestsuite_loadlist("samba4.urgent_replication.python(ad_dc_ntvfs)", "ad_dc_ntvfs:local", [python, os.path.join(DSDB_PYTEST_DIR, "urgent_replication.py"), '$PREFIX_ABS/ad_dc_ntvfs/private/sam.ldb', '$LOADLIST', '$LISTOPT']) plantestsuite_loadlist("samba4.ldap.dirsync.python(ad_dc_ntvfs)", "ad_dc_ntvfs", [python, os.path.join(DSDB_PYTEST_DIR, "dirsync.py"), '$SERVER', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT']) plantestsuite_loadlist("samba4.ldap.match_rules.python", "ad_dc_ntvfs", [python, os.path.join(srcdir(), "lib/ldb-samba/tests/match_rules.py"), '$PREFIX_ABS/ad_dc_ntvfs/private/sam.ldb', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT']) +plantestsuite_loadlist("samba4.ldap.match_rules.python", "ad_dc_ntvfs", [python, os.path.join(srcdir(), "lib/ldb-samba/tests/match_rules_remote.py"), '$SERVER', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT']) plantestsuite("samba4.ldap.index.python", "none", [python, os.path.join(srcdir(), "lib/ldb-samba/tests/index.py")]) plantestsuite_loadlist("samba4.ldap.notification.python(ad_dc_ntvfs)", "ad_dc_ntvfs", [python, os.path.join(DSDB_PYTEST_DIR, "notification.py"), '$SERVER', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT']) plantestsuite_loadlist("samba4.ldap.sites.python(ad_dc_default)", "ad_dc_default", [python, os.path.join(DSDB_PYTEST_DIR, "sites.py"), '$SERVER', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT']) -- cgit v1.2.1