[Pkg-samba-maint] [samba] 16/19: CVE-2015-8467: samdb: Match MS15-096 behaviour for userAccountControl
Jelmer Vernooij
jelmer at moszumanska.debian.org
Fri Dec 18 13:08:29 UTC 2015
This is an automated email from the git hooks/post-receive script.
jelmer pushed a commit to branch upstream_4.3
in repository samba.
commit b000da128b5fb519d2d3f2e7fd20e4a25b7dae7d
Author: Andrew Bartlett <abartlet at samba.org>
Date: Wed Nov 18 17:36:21 2015 +1300
CVE-2015-8467: samdb: Match MS15-096 behaviour for userAccountControl
Swapping between account types is now restricted
Bug: https://bugzilla.samba.org/show_bug.cgi?id=11552
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
source4/dsdb/samdb/ldb_modules/samldb.c | 24 ++++++++-
source4/dsdb/tests/python/user_account_control.py | 63 +++++++++++++++++++----
2 files changed, 76 insertions(+), 11 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/samldb.c b/source4/dsdb/samdb/ldb_modules/samldb.c
index e3a7db2..df285d9 100644
--- a/source4/dsdb/samdb/ldb_modules/samldb.c
+++ b/source4/dsdb/samdb/ldb_modules/samldb.c
@@ -1558,12 +1558,15 @@ static int samldb_check_user_account_control_acl(struct samldb_ctx *ac,
struct security_token *user_token;
struct security_descriptor *domain_sd;
struct ldb_dn *domain_dn = ldb_get_default_basedn(ldb_module_get_ctx(ac->module));
+ struct ldb_context *ldb = ldb_module_get_ctx(ac->module);
const struct uac_to_guid {
uint32_t uac;
+ uint32_t priv_to_change_from;
const char *oid;
const char *guid;
enum sec_privilege privilege;
bool delete_is_privileged;
+ bool admin_required;
const char *error_string;
} map[] = {
{
@@ -1592,6 +1595,16 @@ static int samldb_check_user_account_control_acl(struct samldb_ctx *ac,
.error_string = "Adding the UF_PARTIAL_SECRETS_ACCOUNT bit in userAccountControl requires the DS-Install-Replica right that was not given on the Domain object"
},
{
+ .uac = UF_WORKSTATION_TRUST_ACCOUNT,
+ .priv_to_change_from = UF_NORMAL_ACCOUNT,
+ .error_string = "Swapping UF_NORMAL_ACCOUNT to UF_WORKSTATION_TRUST_ACCOUNT requires the user to be a member of the domain admins group"
+ },
+ {
+ .uac = UF_NORMAL_ACCOUNT,
+ .priv_to_change_from = UF_WORKSTATION_TRUST_ACCOUNT,
+ .error_string = "Swapping UF_WORKSTATION_TRUST_ACCOUNT to UF_NORMAL_ACCOUNT requires the user to be a member of the domain admins group"
+ },
+ {
.uac = UF_INTERDOMAIN_TRUST_ACCOUNT,
.oid = DSDB_CONTROL_PERMIT_INTERDOMAIN_TRUST_UAC_OID,
.error_string = "Updating the UF_INTERDOMAIN_TRUST_ACCOUNT bit in userAccountControl is not permitted over LDAP. This bit is restricted to the LSA CreateTrustedDomain interface",
@@ -1643,7 +1656,7 @@ static int samldb_check_user_account_control_acl(struct samldb_ctx *ac,
return ldb_module_operr(ac->module);
}
- ret = dsdb_get_sd_from_ldb_message(ldb_module_get_ctx(ac->module),
+ ret = dsdb_get_sd_from_ldb_message(ldb,
ac, res->msgs[0], &domain_sd);
if (ret != LDB_SUCCESS) {
@@ -1670,12 +1683,19 @@ static int samldb_check_user_account_control_acl(struct samldb_ctx *ac,
if (have_priv == false) {
ret = LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS;
}
- } else {
+ } else if (map[i].priv_to_change_from & user_account_control_old) {
+ bool is_admin = security_token_has_builtin_administrators(user_token);
+ if (is_admin == false) {
+ ret = LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS;
+ }
+ } else if (map[i].guid) {
ret = acl_check_extended_right(ac, domain_sd,
user_token,
map[i].guid,
SEC_ADS_CONTROL_ACCESS,
sid);
+ } else {
+ ret = LDB_SUCCESS;
}
if (ret != LDB_SUCCESS) {
break;
diff --git a/source4/dsdb/tests/python/user_account_control.py b/source4/dsdb/tests/python/user_account_control.py
index 16c7f81..a53c4f9 100644
--- a/source4/dsdb/tests/python/user_account_control.py
+++ b/source4/dsdb/tests/python/user_account_control.py
@@ -240,6 +240,16 @@ class UserAccountControlTests(samba.tests.TestCase):
m.dn = res[0].dn
m["userAccountControl"] = ldb.MessageElement(str(samba.dsdb.UF_WORKSTATION_TRUST_ACCOUNT),
ldb.FLAG_MOD_REPLACE, "userAccountControl")
+ try:
+ self.samdb.modify(m)
+ self.fail("Unexpectedly able to set userAccountControl to be an Workstation on %s" % m.dn)
+ except LdbError, (enum, estr):
+ 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),
+ ldb.FLAG_MOD_REPLACE, "userAccountControl")
self.samdb.modify(m)
m = ldb.Message()
@@ -306,7 +316,12 @@ class UserAccountControlTests(samba.tests.TestCase):
m.dn = res[0].dn
m["userAccountControl"] = ldb.MessageElement(str(samba.dsdb.UF_WORKSTATION_TRUST_ACCOUNT),
ldb.FLAG_MOD_REPLACE, "userAccountControl")
- self.samdb.modify(m)
+ try:
+ self.samdb.modify(m)
+ self.fail("Unexpectedly able to set userAccountControl to be an Workstation on %s" % m.dn)
+ except LdbError, (enum, estr):
+ self.assertEqual(ldb.ERR_INSUFFICIENT_ACCESS_RIGHTS, enum)
+
def test_admin_mod_uac(self):
computername=self.computernames[0]
@@ -382,9 +397,10 @@ class UserAccountControlTests(samba.tests.TestCase):
priv_to_auth_users_bits = set([UF_PASSWD_NOTREQD, UF_ENCRYPTED_TEXT_PASSWORD_ALLOWED,
UF_DONT_EXPIRE_PASSWD])
- # These bits really are privileged
+ # These bits really are privileged, or can't be changed from UF_NORMAL as a non-admin
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_WORKSTATION_TRUST_ACCOUNT])
invalid_bits = set([UF_TEMP_DUPLICATE_ACCOUNT, UF_PARTIAL_SECRETS_ACCOUNT])
@@ -446,7 +462,7 @@ class UserAccountControlTests(samba.tests.TestCase):
int("0x10000000", 16), int("0x20000000", 16), int("0x40000000", 16), int("0x80000000", 16)])
super_priv_bits = set([UF_INTERDOMAIN_TRUST_ACCOUNT])
- priv_to_remove_bits = set([UF_TRUSTED_FOR_DELEGATION, UF_TRUSTED_TO_AUTHENTICATE_FOR_DELEGATION])
+ priv_to_remove_bits = set([UF_TRUSTED_FOR_DELEGATION, UF_TRUSTED_TO_AUTHENTICATE_FOR_DELEGATION, UF_WORKSTATION_TRUST_ACCOUNT])
for bit in bits:
# Reset this to the initial position, just to be sure
@@ -507,6 +523,31 @@ class UserAccountControlTests(samba.tests.TestCase):
except LdbError, (enum, estr):
self.fail("Unable to set userAccountControl bit 0x%08X on %s: %s" % (bit, m.dn, estr))
+ res = self.admin_samdb.search("%s" % self.base_dn,
+ expression="(&(objectClass=computer)(samAccountName=%s$))" % computername,
+ scope=SCOPE_SUBTREE,
+ attrs=["userAccountControl"])
+
+ if bit in account_types:
+ self.assertEqual(int(res[0]["userAccountControl"][0]),
+ bit|UF_ACCOUNTDISABLE|UF_PASSWD_NOTREQD,
+ "bit 0X%08x should have been added (0X%08x vs 0X%08x)"
+ % (bit, int(res[0]["userAccountControl"][0]),
+ bit|UF_ACCOUNTDISABLE|UF_PASSWD_NOTREQD))
+ elif bit in ignored_bits:
+ self.assertEqual(int(res[0]["userAccountControl"][0]),
+ UF_NORMAL_ACCOUNT|UF_ACCOUNTDISABLE|UF_PASSWD_NOTREQD,
+ "bit 0X%08x should have been added (0X%08x vs 0X%08x)"
+ % (bit, int(res[0]["userAccountControl"][0]),
+ UF_NORMAL_ACCOUNT|UF_ACCOUNTDISABLE|UF_PASSWD_NOTREQD))
+
+ else:
+ self.assertEqual(int(res[0]["userAccountControl"][0]),
+ bit|UF_NORMAL_ACCOUNT|UF_ACCOUNTDISABLE|UF_PASSWD_NOTREQD,
+ "bit 0X%08x should have been added (0X%08x vs 0X%08x)"
+ % (bit, int(res[0]["userAccountControl"][0]),
+ bit|UF_NORMAL_ACCOUNT|UF_ACCOUNTDISABLE|UF_PASSWD_NOTREQD))
+
try:
m = ldb.Message()
m.dn = res[0].dn
@@ -520,7 +561,7 @@ class UserAccountControlTests(samba.tests.TestCase):
if bit in priv_to_remove_bits:
self.assertEqual(enum, ldb.ERR_INSUFFICIENT_ACCESS_RIGHTS)
else:
- self.fail("Unexpectedly able to remove userAccountControl bit 0x%08X on %s: %s" % (bit, m.dn, estr))
+ self.fail("Unexpectedly unable to remove userAccountControl bit 0x%08X on %s: %s" % (bit, m.dn, estr))
res = self.admin_samdb.search("%s" % self.base_dn,
expression="(&(objectClass=computer)(samAccountName=%s$))" % computername,
@@ -528,9 +569,14 @@ class UserAccountControlTests(samba.tests.TestCase):
attrs=["userAccountControl"])
if bit in priv_to_remove_bits:
- self.assertEqual(int(res[0]["userAccountControl"][0]),
- bit|UF_NORMAL_ACCOUNT|UF_ACCOUNTDISABLE|UF_PASSWD_NOTREQD,
- "bit 0X%08x should not have been removed" % bit)
+ if bit in account_types:
+ self.assertEqual(int(res[0]["userAccountControl"][0]),
+ bit|UF_ACCOUNTDISABLE|UF_PASSWD_NOTREQD,
+ "bit 0X%08x should not have been removed" % bit)
+ else:
+ self.assertEqual(int(res[0]["userAccountControl"][0]),
+ bit|UF_NORMAL_ACCOUNT|UF_ACCOUNTDISABLE|UF_PASSWD_NOTREQD,
+ "bit 0X%08x should not have been removed" % bit)
else:
self.assertEqual(int(res[0]["userAccountControl"][0]),
UF_NORMAL_ACCOUNT|UF_ACCOUNTDISABLE|UF_PASSWD_NOTREQD,
@@ -553,7 +599,6 @@ class UserAccountControlTests(samba.tests.TestCase):
self.sd_utils.dacl_add_ace("OU=test_computer_ou1," + self.base_dn, mod)
invalid_bits = set([UF_TEMP_DUPLICATE_ACCOUNT, UF_PARTIAL_SECRETS_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])
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-samba/samba.git
More information about the Pkg-samba-maint
mailing list