From: Andrew Bartlett Date: Mon, 30 Aug 2021 06:17:47 +0000 (+1200) Subject: CVE-2020-25722 selftest: Update user_account_control tests to pass against Windows... X-Git-Tag: samba-4.13.14~216 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ff8f61b7e309849686634b4ea1df036f6d243260;p=thirdparty%2Fsamba.git CVE-2020-25722 selftest: Update user_account_control tests to pass against Windows 2019 This gets us closer to passing against Windows 2019, without making major changes to what was tested. More tests are needed, but it is important to get what was being tested tested again. Account types (eg UF_NORMAL_ACCOUNT, UF_WORKSTATION_TRUST_ACCOUNT) are now required on all objects, this can't be omitted any more. Also for UF_NORMAL_ACCOUNT for these accounts without a password set |UF_PASSWD_NOTREQD must be included. Signed-off-by: Andrew Bartlett Reviewed-by: Alexander Bokovoy BUG: https://bugzilla.samba.org/show_bug.cgi?id=14753 Autobuild-User(master): Andrew Bartlett Autobuild-Date(master): Wed Sep 15 08:49:11 UTC 2021 on sn-devel-184 (cherry picked from commit d12cb47724c2e8d19a28286d4c3ef72271a002fd) --- diff --git a/selftest/knownfail.d/user_account_control b/selftest/knownfail.d/user_account_control new file mode 100644 index 00000000000..ad3af678708 --- /dev/null +++ b/selftest/knownfail.d/user_account_control @@ -0,0 +1 @@ +^samba4.user_account_control.python\(ad_dc_default\).__main__.UserAccountControlTests.test_add_computer_cc_normal_bare.ad_dc_default diff --git a/source4/dsdb/tests/python/user_account_control.py b/source4/dsdb/tests/python/user_account_control.py index fd0ae38a3f9..efb83b2dcff 100755 --- a/source4/dsdb/tests/python/user_account_control.py +++ b/source4/dsdb/tests/python/user_account_control.py @@ -84,7 +84,7 @@ bits = [UF_SCRIPT, UF_ACCOUNTDISABLE, UF_00000004, UF_HOMEDIR_REQUIRED, UF_PARTIAL_SECRETS_ACCOUNT, UF_USE_AES_KEYS, int("0x10000000", 16), int("0x20000000", 16), int("0x40000000", 16), int("0x80000000", 16)] -account_types = set([UF_NORMAL_ACCOUNT, UF_WORKSTATION_TRUST_ACCOUNT, UF_SERVER_TRUST_ACCOUNT]) +account_types = set([UF_NORMAL_ACCOUNT, UF_WORKSTATION_TRUST_ACCOUNT, UF_SERVER_TRUST_ACCOUNT, UF_INTERDOMAIN_TRUST_ACCOUNT]) @DynamicTestCase @@ -108,6 +108,11 @@ class UserAccountControlTests(samba.tests.TestCase): cls.generate_dynamic_test("test_uac_bits_set", bit_str, bit, bit_str) + cls.generate_dynamic_test("test_uac_bits_add", + "UF_NORMAL_ACCOUNT_UF_PASSWD_NOTREQD", + UF_NORMAL_ACCOUNT|UF_PASSWD_NOTREQD, + "UF_NORMAL_ACCOUNT|UF_PASSWD_NOTREQD") + def add_computer_ldap(self, computername, others=None, samdb=None): if samdb is None: @@ -272,7 +277,7 @@ class UserAccountControlTests(samba.tests.TestCase): m = ldb.Message() m.dn = res[0].dn - m["userAccountControl"] = ldb.MessageElement(str(samba.dsdb.UF_NORMAL_ACCOUNT), + m["userAccountControl"] = ldb.MessageElement(str(samba.dsdb.UF_NORMAL_ACCOUNT|UF_PASSWD_NOTREQD), ldb.FLAG_MOD_REPLACE, "userAccountControl") self.samdb.modify(m) @@ -334,9 +339,10 @@ class UserAccountControlTests(samba.tests.TestCase): (enum, estr) = e10.args self.assertEqual(ldb.ERR_INSUFFICIENT_ACCESS_RIGHTS, enum) + m = ldb.Message() m.dn = res[0].dn - m["userAccountControl"] = ldb.MessageElement(str(samba.dsdb.UF_NORMAL_ACCOUNT), + m["userAccountControl"] = ldb.MessageElement(str(samba.dsdb.UF_NORMAL_ACCOUNT|UF_PASSWD_NOTREQD), ldb.FLAG_MOD_REPLACE, "userAccountControl") self.samdb.modify(m) @@ -351,6 +357,50 @@ class UserAccountControlTests(samba.tests.TestCase): (enum, estr) = e11.args self.assertEqual(ldb.ERR_INSUFFICIENT_ACCESS_RIGHTS, enum) + def test_add_computer_cc_normal_bare(self): + user_sid = self.sd_utils.get_object_sid(self.unpriv_user_dn) + mod = "(OA;;CC;bf967a86-0de6-11d0-a285-00aa003049e2;;%s)" % str(user_sid) + + old_sd = self.sd_utils.read_sd_on_dn(self.OU) + self.sd_utils.dacl_add_ace(self.OU, mod) + + computername = self.computernames[0] + sd = ldb.MessageElement((ndr_pack(self.sd_reference_modify)), + ldb.FLAG_MOD_ADD, + "nTSecurityDescriptor") + self.add_computer_ldap(computername, + others={"nTSecurityDescriptor": sd}) + + res = self.admin_samdb.search("%s" % self.base_dn, + expression="(&(objectClass=computer)(samAccountName=%s$))" % computername, + scope=SCOPE_SUBTREE, + attrs=["ntSecurityDescriptor"]) + + desc = res[0]["nTSecurityDescriptor"][0] + desc = ndr_unpack(security.descriptor, desc, allow_remaining=True) + + sddl = desc.as_sddl(self.domain_sid) + self.assertEqual(self.sd_reference_modify.as_sddl(self.domain_sid), sddl) + + m = ldb.Message() + m.dn = res[0].dn + m["description"] = ldb.MessageElement( + ("A description"), ldb.FLAG_MOD_REPLACE, + "description") + self.samdb.modify(m) + + m = ldb.Message() + m.dn = res[0].dn + m["userAccountControl"] = ldb.MessageElement(str(samba.dsdb.UF_NORMAL_ACCOUNT), + ldb.FLAG_MOD_REPLACE, "userAccountControl") + try: + self.samdb.modify(m) + self.fail("Unexpectedly able to set userAccountControl to be an Normal account without |UF_PASSWD_NOTREQD on %s" % m.dn) + except LdbError as e7: + (enum, estr) = e7.args + self.assertEqual(ldb.ERR_UNWILLING_TO_PERFORM, enum) + + def test_admin_mod_uac(self): computername = self.computernames[0] self.add_computer_ldap(computername, samdb=self.admin_samdb) @@ -469,13 +519,23 @@ class UserAccountControlTests(samba.tests.TestCase): self.sd_utils.dacl_add_ace(self.OU, mod) computername = self.computernames[0] - self.add_computer_ldap(computername, others={"userAccountControl": [str(account_type)]}) + if account_type == UF_WORKSTATION_TRUST_ACCOUNT: + self.add_computer_ldap(computername, others={"userAccountControl": [str(account_type)]}) + else: + self.add_computer_ldap(computername) - res = self.admin_samdb.search("%s" % self.base_dn, - expression="(&(objectClass=computer)(samAccountName=%s$))" % computername, + res = self.admin_samdb.search(self.OU, + expression=f"(cn={computername})", scope=SCOPE_SUBTREE, attrs=["userAccountControl"]) - self.assertEqual(int(res[0]["userAccountControl"][0]), account_type) + self.assertEqual(len(res), 1) + + orig_uac = int(res[0]["userAccountControl"][0]) + if account_type == UF_WORKSTATION_TRUST_ACCOUNT: + self.assertEqual(orig_uac, account_type) + else: + self.assertEqual(orig_uac & UF_NORMAL_ACCOUNT, + account_type) m = ldb.Message() m.dn = res[0].dn @@ -504,7 +564,7 @@ class UserAccountControlTests(samba.tests.TestCase): # Reset this to the initial position, just to be sure m = ldb.Message() m.dn = res[0].dn - m["userAccountControl"] = ldb.MessageElement(str(account_type), + m["userAccountControl"] = ldb.MessageElement(str(orig_uac), ldb.FLAG_MOD_REPLACE, "userAccountControl") self.admin_samdb.modify(m) @@ -513,7 +573,11 @@ class UserAccountControlTests(samba.tests.TestCase): scope=SCOPE_SUBTREE, attrs=["userAccountControl"]) - self.assertEqual(int(res[0]["userAccountControl"][0]), account_type) + if account_type == UF_WORKSTATION_TRUST_ACCOUNT: + self.assertEqual(orig_uac, account_type) + else: + self.assertEqual(orig_uac & UF_NORMAL_ACCOUNT, + account_type) m = ldb.Message() m.dn = res[0].dn @@ -521,6 +585,7 @@ class UserAccountControlTests(samba.tests.TestCase): ldb.FLAG_MOD_REPLACE, "userAccountControl") try: self.admin_samdb.modify(m) + if bit in invalid_bits: self.fail("Should have been unable to set userAccountControl bit 0x%08X on %s" % (bit, m.dn)) @@ -534,6 +599,19 @@ class UserAccountControlTests(samba.tests.TestCase): self.assertEqual(enum, ldb.ERR_INSUFFICIENT_ACCESS_RIGHTS) # No point going on, try the next bit continue + + elif (account_type == UF_NORMAL_ACCOUNT) \ + and (bit in account_types) \ + and (bit != account_type): + self.assertEqual(enum, ldb.ERR_UNWILLING_TO_PERFORM) + continue + + elif (account_type == UF_WORKSTATION_TRUST_ACCOUNT) \ + and (bit != UF_NORMAL_ACCOUNT) \ + and (bit != account_type): + self.assertEqual(enum, ldb.ERR_UNWILLING_TO_PERFORM) + continue + else: self.fail("Unable to set userAccountControl bit 0x%08X on %s: %s" % (bit, m.dn, estr)) @@ -637,17 +715,27 @@ class UserAccountControlTests(samba.tests.TestCase): self.sd_utils.dacl_add_ace(self.OU, mod) - invalid_bits = set([UF_TEMP_DUPLICATE_ACCOUNT, UF_PARTIAL_SECRETS_ACCOUNT]) + invalid_bits = set([UF_TEMP_DUPLICATE_ACCOUNT]) + # UF_NORMAL_ACCOUNT is invalid alone, needs UF_PASSWD_NOTREQD + unwilling_bits = set([UF_NORMAL_ACCOUNT]) + # These bits are privileged, but authenticated users have that CAR by default, so this is a pain to test priv_to_auth_users_bits = set([UF_PASSWD_NOTREQD, UF_ENCRYPTED_TEXT_PASSWORD_ALLOWED, UF_DONT_EXPIRE_PASSWD]) # These bits really are privileged priv_bits = set([UF_INTERDOMAIN_TRUST_ACCOUNT, UF_SERVER_TRUST_ACCOUNT, - UF_TRUSTED_FOR_DELEGATION, UF_TRUSTED_TO_AUTHENTICATE_FOR_DELEGATION]) + UF_TRUSTED_FOR_DELEGATION, UF_TRUSTED_TO_AUTHENTICATE_FOR_DELEGATION, + UF_PARTIAL_SECRETS_ACCOUNT]) + + if bit not in account_types and ((bit & UF_NORMAL_ACCOUNT) == 0): + bit_add = bit|UF_WORKSTATION_TRUST_ACCOUNT + else: + bit_add = bit try: - self.add_computer_ldap(computername, others={"userAccountControl": [str(bit)]}) + + self.add_computer_ldap(computername, others={"userAccountControl": [str(bit_add)]}) delete_force(self.admin_samdb, "CN=%s,%s" % (computername, self.OU)) if bit in priv_bits: self.fail("Unexpectdly able to set userAccountControl bit 0x%08X (%s) on %s" @@ -664,6 +752,8 @@ class UserAccountControlTests(samba.tests.TestCase): computername)) elif bit in priv_bits: self.assertEqual(enum, ldb.ERR_INSUFFICIENT_ACCESS_RIGHTS) + elif bit in unwilling_bits: + self.assertEqual(enum, ldb.ERR_UNWILLING_TO_PERFORM) else: self.fail("Unable to set userAccountControl bit 0x%08X (%s) on %s: %s" % (bit,