sin/samba-mirror
1
0
Fork 0
mirror of https://github.com/samba-team/samba.git synced 2024-12-22 13:34:15 +03:00
Code

CVE-2020-25722: s4-acl: Make sure Control Access Rights honor the Applies-to attribute

Browse Source 
Validate Writes and Control Access Rights only grant access if the
object is of the type listed in the Right's appliesTo attribute. For
example, even though a Validated-SPN access may be granted to a user
object in the SD, it should only pass if the object is of class
computer This patch enforces the appliesTo attribute classes for
access checks from within the ldb stack.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14832

Signed-off-by: Nadezhda Ivanova <nivanova@symas.com>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
This commit is contained in:
Nadezhda Ivanova 2021-10-18 14:27:59 +03:00 committed by Jule Anger
parent 6121f31c0e
commit 8da6d0bf6f
6 changed files with 168 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
selftest/knownfail.d/bug-14832
View File

@ -1 +0,0 @@
^samba4.ldap.acl.python\(.*\).__main__.AclSPNTests.test_user_spn\(.*\)

11
source4/dsdb/common/util.c
View File

@ -1182,6 +1182,17 @@ struct ldb_dn *samdb_sites_dn(struct ldb_context *sam_ctx, TALLOC_CTX *mem_ctx)
return new_dn;
}
struct ldb_dn *samdb_extended_rights_dn(struct ldb_context *sam_ctx, TALLOC_CTX *mem_ctx)
{
struct ldb_dn *new_dn;
new_dn = ldb_dn_copy(mem_ctx, ldb_get_config_basedn(sam_ctx));
if ( ! ldb_dn_add_child_fmt(new_dn, "CN=Extended-Rights")) {
talloc_free(new_dn);
return NULL;
}
return new_dn;
}
/*
work out the domain sid for the current open ldb
*/

87
source4/dsdb/samdb/ldb_modules/acl.c
View File

@ -701,7 +701,12 @@ static int acl_check_spn(TALLOC_CTX *mem_ctx,
return LDB_SUCCESS;
}
ret = acl_check_extended_right(tmp_ctx, sd, acl_user_token(module),
ret = acl_check_extended_right(tmp_ctx,
module,
req,
objectclass,
sd,
acl_user_token(module),
GUID_DRS_VALIDATE_SPN,
SEC_ADS_SELF_WRITE,
sid);
@ -914,7 +919,7 @@ static int acl_add(struct ldb_module *module, struct ldb_request *req)
return ldb_next_request(module, req);
}
/* ckecks if modifications are allowed on "Member" attribute */
/* checks if modifications are allowed on "Member" attribute */
static int acl_check_self_membership(TALLOC_CTX *mem_ctx,
struct ldb_module *module,
struct ldb_request *req,
@ -928,6 +933,16 @@ static int acl_check_self_membership(TALLOC_CTX *mem_ctx,
struct ldb_context *ldb = ldb_module_get_ctx(module);
struct ldb_dn *user_dn;
struct ldb_message_element *member_el;
const struct ldb_message *msg = NULL;
if (req->operation == LDB_MODIFY) {
msg = req->op.mod.message;
} else if (req->operation == LDB_ADD) {
msg = req->op.add.message;
} else {
return LDB_ERR_OPERATIONS_ERROR;
}
/* if we have wp, we can do whatever we like */
if (acl_check_access_on_attribute(module,
mem_ctx,
@ -938,13 +953,13 @@ static int acl_check_self_membership(TALLOC_CTX *mem_ctx,
return LDB_SUCCESS;
}
/* if we are adding/deleting ourselves, check for self membership */
ret = dsdb_find_dn_by_sid(ldb, mem_ctx,
&acl_user_token(module)->sids[PRIMARY_USER_SID_INDEX],
ret = dsdb_find_dn_by_sid(ldb, mem_ctx,
&acl_user_token(module)->sids[PRIMARY_USER_SID_INDEX],
&user_dn);
if (ret != LDB_SUCCESS) {
return ret;
}
member_el = ldb_msg_find_element(req->op.mod.message, "member");
member_el = ldb_msg_find_element(msg, "member");
if (!member_el) {
return ldb_operr(ldb);
}
@ -958,13 +973,18 @@ static int acl_check_self_membership(TALLOC_CTX *mem_ctx,
return LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS;
}
}
ret = acl_check_extended_right(mem_ctx, sd, acl_user_token(module),
ret = acl_check_extended_right(mem_ctx,
module,
req,
objectclass,
sd,
acl_user_token(module),
GUID_DRS_SELF_MEMBERSHIP,
SEC_ADS_SELF_WRITE,
sid);
if (ret == LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS) {
dsdb_acl_debug(sd, acl_user_token(module),
req->op.mod.message->dn,
msg->dn,
true,
10);
}
@ -1024,6 +1044,9 @@ static int acl_check_password_rights(
* so we don't have to strict verification of the input.
*/
ret = acl_check_extended_right(tmp_ctx,
module,
req,
objectclass,
sd,
acl_user_token(module),
GUID_DRS_USER_CHANGE_PASSWORD,
@ -1047,7 +1070,12 @@ static int acl_check_password_rights(
* the only caller is samdb_set_password_internal(),
* so we don't have to strict verification of the input.
*/
ret = acl_check_extended_right(tmp_ctx, sd, acl_user_token(module),
ret = acl_check_extended_right(tmp_ctx,
module,
req,
objectclass,
sd,
acl_user_token(module),
GUID_DRS_FORCE_CHANGE_PASSWORD,
SEC_ADS_CONTROL_ACCESS,
sid);
@ -1100,7 +1128,12 @@ static int acl_check_password_rights(
if (rep_attr_cnt > 0) {
pav->pwd_reset = true;
ret = acl_check_extended_right(tmp_ctx, sd, acl_user_token(module),
ret = acl_check_extended_right(tmp_ctx,
module,
req,
objectclass,
sd,
acl_user_token(module),
GUID_DRS_FORCE_CHANGE_PASSWORD,
SEC_ADS_CONTROL_ACCESS,
sid);
@ -1110,7 +1143,12 @@ static int acl_check_password_rights(
if (add_attr_cnt != del_attr_cnt) {
pav->pwd_reset = true;
ret = acl_check_extended_right(tmp_ctx, sd, acl_user_token(module),
ret = acl_check_extended_right(tmp_ctx,
module,
req,
objectclass,
sd,
acl_user_token(module),
GUID_DRS_FORCE_CHANGE_PASSWORD,
SEC_ADS_CONTROL_ACCESS,
sid);
@ -1120,7 +1158,12 @@ static int acl_check_password_rights(
if (add_val_cnt == 1 && del_val_cnt == 1) {
pav->pwd_reset = false;
ret = acl_check_extended_right(tmp_ctx, sd, acl_user_token(module),
ret = acl_check_extended_right(tmp_ctx,
module,
req,
objectclass,
sd,
acl_user_token(module),
GUID_DRS_USER_CHANGE_PASSWORD,
SEC_ADS_CONTROL_ACCESS,
sid);
@ -1134,7 +1177,12 @@ static int acl_check_password_rights(
if (add_val_cnt == 1 && del_val_cnt == 0) {
pav->pwd_reset = true;
ret = acl_check_extended_right(tmp_ctx, sd, acl_user_token(module),
ret = acl_check_extended_right(tmp_ctx,
module,
req,
objectclass,
sd,
acl_user_token(module),
GUID_DRS_FORCE_CHANGE_PASSWORD,
SEC_ADS_CONTROL_ACCESS,
sid);
@ -1689,6 +1737,9 @@ static int acl_check_reanimate_tombstone(TALLOC_CTX *mem_ctx,
struct ldb_result *acl_res;
struct security_descriptor *sd = NULL;
struct dom_sid *sid = NULL;
const struct dsdb_schema *schema = NULL;
const struct dsdb_class *objectclass = NULL;
struct ldb_context *ldb = ldb_module_get_ctx(module);
static const char *acl_attrs[] = {
"nTSecurityDescriptor",
"objectClass",
@ -1709,10 +1760,20 @@ static int acl_check_reanimate_tombstone(TALLOC_CTX *mem_ctx,
ret = dsdb_get_sd_from_ldb_message(mem_ctx, req, acl_res->msgs[0], &sd);
sid = samdb_result_dom_sid(mem_ctx, acl_res->msgs[0], "objectSid");
schema = dsdb_get_schema(ldb, req);
if (!schema) {
return LDB_ERR_OPERATIONS_ERROR;
}
objectclass = dsdb_get_structural_oc_from_msg(schema, acl_res->msgs[0]);
if (ret != LDB_SUCCESS || !sd) {
return ldb_operr(ldb_module_get_ctx(module));
}
return acl_check_extended_right(mem_ctx, sd, acl_user_token(module),
return acl_check_extended_right(mem_ctx,
module,
req,
objectclass,
sd,
acl_user_token(module),
GUID_DRS_REANIMATE_TOMBSTONE,
SEC_ADS_CONTROL_ACCESS, sid);
}

40
source4/dsdb/samdb/ldb_modules/acl_util.c
View File

@ -197,6 +197,9 @@ fail:
/* checks for validated writes */
int acl_check_extended_right(TALLOC_CTX *mem_ctx,
struct ldb_module *module,
struct ldb_request *req,
const struct dsdb_class *objectclass,
struct security_descriptor *sd,
struct security_token *token,
const char *ext_right,
@ -208,6 +211,43 @@ int acl_check_extended_right(TALLOC_CTX *mem_ctx,
uint32_t access_granted;
struct object_tree *root = NULL;
TALLOC_CTX *tmp_ctx = talloc_new(mem_ctx);
static const char *no_attrs[] = { NULL };
struct ldb_result *extended_rights_res = NULL;
struct ldb_dn *extended_rights_dn = NULL;
struct ldb_context *ldb = ldb_module_get_ctx(module);
int ret = 0;
/*
* Find the extended right and check if applies to
* the objectclass of the object
*/
extended_rights_dn = samdb_extended_rights_dn(ldb, req);
if (!extended_rights_dn) {
ldb_set_errstring(ldb,
"access_check: CN=Extended-Rights dn could not be generated!");
return LDB_ERR_OPERATIONS_ERROR;
}
/* Note: we are checking only the structural object class. */
ret = dsdb_module_search(module, req, &extended_rights_res,
extended_rights_dn, LDB_SCOPE_ONELEVEL,
no_attrs,
DSDB_FLAG_NEXT_MODULE |
DSDB_FLAG_AS_SYSTEM,
req,
"(&(rightsGuid=%s)(appliesTo=%s))",
ext_right,
GUID_string(tmp_ctx,
&(objectclass->schemaIDGUID)));
if (ret != LDB_SUCCESS) {
return ret;
} else if (extended_rights_res->count == 0 ) {
ldb_debug(ldb, LDB_DEBUG_TRACE,
"acl_check_extended_right: Could not find appliesTo for %s\n",
ext_right);
return LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS;
}
GUID_from_string(ext_right, &right);

13
source4/dsdb/samdb/ldb_modules/dirsync.c
View File

@ -1066,7 +1066,9 @@ static int dirsync_ldb_search(struct ldb_module *module, struct ldb_request *req
if (!(dirsync_ctl->flags & LDAP_DIRSYNC_OBJECT_SECURITY)) {
struct dom_sid *sid;
struct security_descriptor *sd = NULL;
const char *acl_attrs[] = { "nTSecurityDescriptor", "objectSid", NULL };
const char *acl_attrs[] = { "nTSecurityDescriptor", "objectSid", "objectClass", NULL };
const struct dsdb_schema *schema = NULL;
const struct dsdb_class *objectclass = NULL;
/*
* If we don't have the flag and if we have the "replicate directory change" granted
* then we upgrade ourself to system to not be blocked by the acl
@ -1096,7 +1098,14 @@ static int dirsync_ldb_search(struct ldb_module *module, struct ldb_request *req
if (ret != LDB_SUCCESS) {
return ret;
}
ret = acl_check_extended_right(dsc, sd, acl_user_token(module), GUID_DRS_GET_CHANGES, SEC_ADS_CONTROL_ACCESS, sid);
schema = dsdb_get_schema(ldb, req);
if (!schema) {
return LDB_ERR_OPERATIONS_ERROR;
}
objectclass = dsdb_get_structural_oc_from_msg(schema, acl_res->msgs[0]);
ret = acl_check_extended_right(dsc, module, req, objectclass,
sd, acl_user_token(module),
GUID_DRS_GET_CHANGES, SEC_ADS_CONTROL_ACCESS, sid);
if (ret == LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS) {
return ret;

56
source4/dsdb/samdb/ldb_modules/samldb.c
View File

@ -2196,12 +2196,15 @@ static int samldb_check_user_account_control_objectclass_invariants(
return LDB_SUCCESS;
}
static int samldb_get_domain_secdesc(struct samldb_ctx *ac,
struct security_descriptor **domain_sd)
static int samldb_get_domain_secdesc_and_oc(struct samldb_ctx *ac,
struct security_descriptor **domain_sd,
const struct dsdb_class **objectclass)
{
const char * const sd_attrs[] = {"ntSecurityDescriptor", NULL};
const char * const sd_attrs[] = {"ntSecurityDescriptor", "objectClass", NULL};
struct ldb_result *res;
struct ldb_dn *domain_dn = ldb_get_default_basedn(ldb_module_get_ctx(ac->module));
const struct dsdb_schema *schema = NULL;
struct ldb_context *ldb = ldb_module_get_ctx(ac->module);
int ret = dsdb_module_search_dn(ac->module, ac, &res,
domain_dn,
sd_attrs,
@ -2214,6 +2217,11 @@ static int samldb_get_domain_secdesc(struct samldb_ctx *ac,
return ldb_module_operr(ac->module);
}
schema = dsdb_get_schema(ldb, ac->req);
if (!schema) {
return ldb_module_operr(ac->module);;
}
*objectclass = dsdb_get_structural_oc_from_msg(schema, res->msgs[0]);
return dsdb_get_sd_from_ldb_message(ldb_module_get_ctx(ac->module),
ac, res->msgs[0], domain_sd);
@ -2233,6 +2241,7 @@ static int samldb_check_user_account_control_acl(struct samldb_ctx *ac,
bool need_acl_check = false;
struct security_token *user_token;
struct security_descriptor *domain_sd;
const struct dsdb_class *objectclass = NULL;
const struct uac_to_guid {
uint32_t uac;
uint32_t priv_to_change_from;
@ -2318,7 +2327,7 @@ static int samldb_check_user_account_control_acl(struct samldb_ctx *ac,
return LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS;
}
ret = samldb_get_domain_secdesc(ac, &domain_sd);
ret = samldb_get_domain_secdesc_and_oc(ac, &domain_sd, &objectclass);
if (ret != LDB_SUCCESS) {
return ret;
}
@ -2349,7 +2358,11 @@ static int samldb_check_user_account_control_acl(struct samldb_ctx *ac,
ret = LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS;
}
} else if (map[i].guid) {
ret = acl_check_extended_right(ac, domain_sd,
ret = acl_check_extended_right(ac,
ac->module,
ac->req,
objectclass,
domain_sd,
user_token,
map[i].guid,
SEC_ADS_CONTROL_ACCESS,
@ -2689,12 +2702,11 @@ static int samldb_check_pwd_last_set_acl(struct samldb_ctx *ac,
{
struct ldb_context *ldb = ldb_module_get_ctx(ac->module);
int ret = 0;
struct ldb_result *res = NULL;
const char * const sd_attrs[] = {"ntSecurityDescriptor", NULL};
struct security_token *user_token = NULL;
struct security_descriptor *domain_sd = NULL;
struct ldb_dn *domain_dn = ldb_get_default_basedn(ldb_module_get_ctx(ac->module));
const char *operation = "";
const struct dsdb_class *objectclass = NULL;
if (dsdb_module_am_system(ac->module)) {
return LDB_SUCCESS;
@ -2716,24 +2728,15 @@ static int samldb_check_pwd_last_set_acl(struct samldb_ctx *ac,
return LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS;
}
ret = dsdb_module_search_dn(ac->module, ac, &res,
domain_dn,
sd_attrs,
DSDB_FLAG_NEXT_MODULE | DSDB_SEARCH_SHOW_DELETED,
ac->req);
ret = samldb_get_domain_secdesc_and_oc(ac, &domain_sd, &objectclass);
if (ret != LDB_SUCCESS) {
return ret;
}
if (res->count != 1) {
return ldb_module_operr(ac->module);
}
ret = dsdb_get_sd_from_ldb_message(ldb, ac, res->msgs[0], &domain_sd);
if (ret != LDB_SUCCESS) {
return ret;
}
ret = acl_check_extended_right(ac, domain_sd,
ret = acl_check_extended_right(ac,
ac->module,
ac->req,
objectclass,
domain_sd,
user_token,
GUID_DRS_UNEXPIRE_PASSWORD,
SEC_ADS_CONTROL_ACCESS,
@ -3763,16 +3766,21 @@ static int samldb_check_sensitive_attributes(struct samldb_ctx *ac)
el = ldb_msg_find_element(ac->msg, "msDS-SecondaryKrbTgtNumber");
if (el) {
struct security_descriptor *domain_sd;
const struct dsdb_class *objectclass = NULL;
/*
* msDS-SecondaryKrbTgtNumber allows the creator to
* become an RODC, this is trusted as an RODC
* account
*/
ret = samldb_get_domain_secdesc(ac, &domain_sd);
ret = samldb_get_domain_secdesc_and_oc(ac, &domain_sd, &objectclass);
if (ret != LDB_SUCCESS) {
return ret;
}
ret = acl_check_extended_right(ac, domain_sd,
ret = acl_check_extended_right(ac,
ac->module,
ac->req,
objectclass,
domain_sd,
user_token,
GUID_DRS_DS_INSTALL_REPLICA,
SEC_ADS_CONTROL_ACCESS,