1
0
mirror of https://github.com/samba-team/samba.git synced 2025-02-28 01:58:17 +03:00

dsdb: Improve userAccountControl handling

We now always check the ACL and invarient rules using the same function

The change to libds is because UF_PARTIAL_SECRETS_ACCOUNT is a flag,
not an account type

This list should only be of the account exclusive account type bits.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=10993

Pair-programmed-with: Garming Sam <garming@catalyst.net.nz>
Signed-off-by: Garming Sam <garming@catalyst.net.nz>
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
This commit is contained in:
Andrew Bartlett 2014-12-10 14:15:54 +13:00
parent bf99abb5db
commit 735605a6b0
2 changed files with 158 additions and 32 deletions

View File

@ -945,10 +945,10 @@ static int samldb_schema_info_update(struct samldb_ctx *ac)
} }
static int samldb_prim_group_tester(struct samldb_ctx *ac, uint32_t rid); static int samldb_prim_group_tester(struct samldb_ctx *ac, uint32_t rid);
static int samldb_check_user_account_control_acl(struct samldb_ctx *ac, static int samldb_check_user_account_control_rules(struct samldb_ctx *ac,
struct dom_sid *sid, struct dom_sid *sid,
uint32_t user_account_control, uint32_t user_account_control,
uint32_t user_account_control_old); uint32_t user_account_control_old);
/* /*
* "Objectclass" trigger (MS-SAMR 3.1.1.8.1) * "Objectclass" trigger (MS-SAMR 3.1.1.8.1)
@ -1066,9 +1066,10 @@ static int samldb_objectclass_trigger(struct samldb_ctx *ac)
uac_generated = true; uac_generated = true;
} }
/* Temporary duplicate accounts aren't allowed */ ret = samldb_check_user_account_control_rules(ac, NULL,
if ((user_account_control & UF_TEMP_DUPLICATE_ACCOUNT) != 0) { user_account_control, 0);
return LDB_ERR_OTHER; if (ret != LDB_SUCCESS) {
return ret;
} }
/* Workstation and (read-only) DC objects do need objectclass "computer" */ /* Workstation and (read-only) DC objects do need objectclass "computer" */
@ -1160,11 +1161,6 @@ static int samldb_objectclass_trigger(struct samldb_ctx *ac)
} }
} }
ret = samldb_check_user_account_control_acl(ac, NULL,
user_account_control, 0);
if (ret != LDB_SUCCESS) {
return ret;
}
} }
break; break;
} }
@ -1452,6 +1448,110 @@ static int samldb_prim_group_trigger(struct samldb_ctx *ac)
return ret; return ret;
} }
static int samldb_check_user_account_control_invariants(struct samldb_ctx *ac,
uint32_t user_account_control)
{
int i, ret = 0;
bool need_check = false;
const struct uac_to_guid {
uint32_t uac;
bool never;
uint32_t needs;
uint32_t not_with;
const char *error_string;
} map[] = {
{
.uac = UF_TEMP_DUPLICATE_ACCOUNT,
.never = true,
.error_string = "Updating the UF_TEMP_DUPLICATE_ACCOUNT flag is never allowed"
},
{
.uac = UF_PARTIAL_SECRETS_ACCOUNT,
.needs = UF_WORKSTATION_TRUST_ACCOUNT,
.error_string = "Setting UF_PARTIAL_SECRETS_ACCOUNT only permitted with UF_WORKSTATION_TRUST_ACCOUNT"
},
{
.uac = UF_TRUSTED_FOR_DELEGATION,
.not_with = UF_PARTIAL_SECRETS_ACCOUNT,
.error_string = "Setting UF_TRUSTED_FOR_DELEGATION not allowed with UF_PARTIAL_SECRETS_ACCOUNT"
},
{
.uac = UF_NORMAL_ACCOUNT,
.not_with = UF_ACCOUNT_TYPE_MASK & ~UF_NORMAL_ACCOUNT,
.error_string = "Setting more than one account type not permitted"
},
{
.uac = UF_WORKSTATION_TRUST_ACCOUNT,
.not_with = UF_ACCOUNT_TYPE_MASK & ~UF_WORKSTATION_TRUST_ACCOUNT,
.error_string = "Setting more than one account type not permitted"
},
{
.uac = UF_INTERDOMAIN_TRUST_ACCOUNT,
.not_with = UF_ACCOUNT_TYPE_MASK & ~UF_INTERDOMAIN_TRUST_ACCOUNT,
.error_string = "Setting more than one account type not permitted"
},
{
.uac = UF_SERVER_TRUST_ACCOUNT,
.not_with = UF_ACCOUNT_TYPE_MASK & ~UF_SERVER_TRUST_ACCOUNT,
.error_string = "Setting more than one account type not permitted"
},
{
.uac = UF_TRUSTED_FOR_DELEGATION,
.not_with = UF_PARTIAL_SECRETS_ACCOUNT,
.error_string = "Setting UF_TRUSTED_FOR_DELEGATION not allowed with UF_PARTIAL_SECRETS_ACCOUNT"
}
};
for (i = 0; i < ARRAY_SIZE(map); i++) {
if (user_account_control & map[i].uac) {
need_check = true;
break;
}
}
if (need_check == false) {
return LDB_SUCCESS;
}
for (i = 0; i < ARRAY_SIZE(map); i++) {
uint32_t this_uac = user_account_control & map[i].uac;
if (this_uac != 0) {
if (map[i].never) {
ret = LDB_ERR_OTHER;
break;
} else if (map[i].needs != 0) {
if ((map[i].needs & user_account_control) == 0) {
ret = LDB_ERR_OTHER;
break;
}
} else if (map[i].not_with != 0) {
if ((map[i].not_with & user_account_control) != 0) {
ret = LDB_ERR_OTHER;
break;
}
}
}
}
if (ret != LDB_SUCCESS) {
switch (ac->req->operation) {
case LDB_ADD:
ldb_asprintf_errstring(ldb_module_get_ctx(ac->module),
"Failed to add %s: %s",
ldb_dn_get_linearized(ac->msg->dn),
map[i].error_string);
break;
case LDB_MODIFY:
ldb_asprintf_errstring(ldb_module_get_ctx(ac->module),
"Failed to modify %s: %s",
ldb_dn_get_linearized(ac->msg->dn),
map[i].error_string);
break;
default:
return ldb_module_operr(ac->module);
}
}
return ret;
}
/** /**
* Validate that the restriction in point 5 of MS-SAMR 3.1.1.8.10 userAccountControl is honoured * Validate that the restriction in point 5 of MS-SAMR 3.1.1.8.10 userAccountControl is honoured
* *
@ -1619,6 +1719,24 @@ static int samldb_check_user_account_control_acl(struct samldb_ctx *ac,
return ret; return ret;
} }
static int samldb_check_user_account_control_rules(struct samldb_ctx *ac,
struct dom_sid *sid,
uint32_t user_account_control,
uint32_t user_account_control_old)
{
int ret;
ret = samldb_check_user_account_control_invariants(ac, user_account_control);
if (ret != LDB_SUCCESS) {
return ret;
}
ret = samldb_check_user_account_control_acl(ac, sid, user_account_control, user_account_control_old);
if (ret != LDB_SUCCESS) {
return ret;
}
return ret;
}
/** /**
* This function is called on LDB modify operations. It performs some additions/ * This function is called on LDB modify operations. It performs some additions/
* replaces on the current LDB message when "userAccountControl" changes. * replaces on the current LDB message when "userAccountControl" changes.
@ -1725,11 +1843,21 @@ static int samldb_user_account_control_change(struct samldb_ctx *ac)
if (new_ufa == 0) { if (new_ufa == 0) {
/* /*
* When there is no account type embedded in "userAccountControl" * When there is no account type embedded in "userAccountControl"
* fall back to the old. * fall back to UF_NORMAL_ACCOUNT.
*/ */
new_ufa = old_ufa; new_ufa = UF_NORMAL_ACCOUNT;
new_uac |= new_ufa; new_uac |= new_ufa;
} }
sid = samdb_result_dom_sid(res, res->msgs[0], "objectSid");
if (sid == NULL) {
return ldb_module_operr(ac->module);
}
ret = samldb_check_user_account_control_rules(ac, sid, new_uac, old_uac);
if (ret != LDB_SUCCESS) {
return ret;
}
new_atype = ds_uf2atype(new_ufa); new_atype = ds_uf2atype(new_ufa);
new_pgrid = ds_uf2prim_group_rid(new_uac); new_pgrid = ds_uf2prim_group_rid(new_uac);
@ -1848,16 +1976,6 @@ static int samldb_user_account_control_change(struct samldb_ctx *ac)
ldb_msg_remove_attr(ac->msg, "userAccountControl"); ldb_msg_remove_attr(ac->msg, "userAccountControl");
} }
sid = samdb_result_dom_sid(res, res->msgs[0], "objectSid");
if (sid == NULL) {
return ldb_module_operr(ac->module);
}
ret = samldb_check_user_account_control_acl(ac, sid, new_uac, old_uac);
if (ret != LDB_SUCCESS) {
return ret;
}
return LDB_SUCCESS; return LDB_SUCCESS;
} }

View File

@ -317,6 +317,16 @@ class UserAccountControlTests(samba.tests.TestCase):
self.assertEqual(int(res[0]["userAccountControl"][0]), UF_NORMAL_ACCOUNT|UF_ACCOUNTDISABLE|UF_PASSWD_NOTREQD) self.assertEqual(int(res[0]["userAccountControl"][0]), UF_NORMAL_ACCOUNT|UF_ACCOUNTDISABLE|UF_PASSWD_NOTREQD)
m = ldb.Message()
m.dn = res[0].dn
m["userAccountControl"] = ldb.MessageElement(str(UF_WORKSTATION_TRUST_ACCOUNT|UF_PARTIAL_SECRETS_ACCOUNT|UF_TRUSTED_FOR_DELEGATION),
ldb.FLAG_MOD_REPLACE, "userAccountControl")
try:
self.admin_samdb.modify(m)
self.fail("Unexpectedly able to set userAccountControl to UF_WORKSTATION_TRUST_ACCOUNT|UF_PARTIAL_SECRETS_ACCOUNT|UF_TRUSTED_FOR_DELEGATION on %s" % m.dn)
except LdbError, (enum, estr):
self.assertEqual(ldb.ERR_OTHER, enum)
m = ldb.Message() m = ldb.Message()
m.dn = res[0].dn m.dn = res[0].dn
m["userAccountControl"] = ldb.MessageElement(str(UF_WORKSTATION_TRUST_ACCOUNT|UF_PARTIAL_SECRETS_ACCOUNT), m["userAccountControl"] = ldb.MessageElement(str(UF_WORKSTATION_TRUST_ACCOUNT|UF_PARTIAL_SECRETS_ACCOUNT),
@ -340,7 +350,7 @@ class UserAccountControlTests(samba.tests.TestCase):
scope=SCOPE_SUBTREE, scope=SCOPE_SUBTREE,
attrs=["userAccountControl"]) attrs=["userAccountControl"])
self.assertEqual(int(res[0]["userAccountControl"][0]), UF_WORKSTATION_TRUST_ACCOUNT| UF_ACCOUNTDISABLE) self.assertEqual(int(res[0]["userAccountControl"][0]), UF_NORMAL_ACCOUNT| UF_ACCOUNTDISABLE)
def test_uac_bits_set(self): def test_uac_bits_set(self):
@ -372,10 +382,9 @@ class UserAccountControlTests(samba.tests.TestCase):
# These bits really are privileged # These bits really are privileged
priv_bits = set([UF_INTERDOMAIN_TRUST_ACCOUNT, UF_SERVER_TRUST_ACCOUNT, 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])
invalid_bits = set([UF_TEMP_DUPLICATE_ACCOUNT]) invalid_bits = set([UF_TEMP_DUPLICATE_ACCOUNT, UF_PARTIAL_SECRETS_ACCOUNT])
for bit in bits: for bit in bits:
m = ldb.Message() m = ldb.Message()
@ -420,7 +429,7 @@ class UserAccountControlTests(samba.tests.TestCase):
"description") "description")
self.samdb.modify(m) self.samdb.modify(m)
invalid_bits = set([UF_TEMP_DUPLICATE_ACCOUNT]) invalid_bits = set([UF_TEMP_DUPLICATE_ACCOUNT, UF_PARTIAL_SECRETS_ACCOUNT])
super_priv_bits = set([UF_INTERDOMAIN_TRUST_ACCOUNT]) super_priv_bits = set([UF_INTERDOMAIN_TRUST_ACCOUNT])
@ -483,7 +492,7 @@ class UserAccountControlTests(samba.tests.TestCase):
self.sd_utils.dacl_add_ace("OU=test_computer_ou1," + self.base_dn, mod) self.sd_utils.dacl_add_ace("OU=test_computer_ou1," + self.base_dn, mod)
invalid_bits = set([UF_TEMP_DUPLICATE_ACCOUNT]) 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 # 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, priv_to_auth_users_bits = set([UF_PASSWD_NOTREQD, UF_ENCRYPTED_TEXT_PASSWORD_ALLOWED,
@ -491,8 +500,7 @@ class UserAccountControlTests(samba.tests.TestCase):
# These bits really are privileged # These bits really are privileged
priv_bits = set([UF_INTERDOMAIN_TRUST_ACCOUNT, UF_SERVER_TRUST_ACCOUNT, 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])
for bit in bits: for bit in bits:
try: try: