CVE-2020-25722 selftest: Update user_account_control tests to pass against Windows...

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 <abartlet@samba.org>
Reviewed-by: Alexander Bokovoy <ab@samba.org>
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14753

Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
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 (file)
index 0000000..ad3af67
--- /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 fd0ae38a3f9946ef28052e70a09718d5d1414ffe..efb83b2dcfffff018d0a22c8a72b50451c6da299 100755 (executable)
--- 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,