From: Andrew Bartlett Date: Fri, 13 Aug 2021 05:42:23 +0000 (+1200) Subject: CVE-2020-25722 dsdb: Restrict the setting of privileged attributes during LDAP add... X-Git-Tag: samba-4.13.14~211 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=448585950bda2c1daab8ffeb3971870ed0416634;p=thirdparty%2Fsamba.git CVE-2020-25722 dsdb: Restrict the setting of privileged attributes during LDAP add/modify The remaining failures in the priv_attrs (not the strict one) test are due to missing objectclass constraints on the administrator which should be addressed, but are not a security issue. A better test for confirming constraints between objectclass and userAccountControl UF_NORMAL_ACCONT/UF_WORKSTATION_TRUST values would be user_account_control.py. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14703 BUG: https://bugzilla.samba.org/show_bug.cgi?id=14778 BUG: https://bugzilla.samba.org/show_bug.cgi?id=14775 Signed-off-by: Andrew Bartlett Reviewed-by: Douglas Bagnall --- diff --git a/selftest/knownfail.d/priv_attr b/selftest/knownfail.d/priv_attr index ab6db192aae..c3a779010d9 100644 --- a/selftest/knownfail.d/priv_attr +++ b/selftest/knownfail.d/priv_attr @@ -1,31 +1,7 @@ -samba4.priv_attrs.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_msDS-AllowedToDelegateTo_add_CC_WP_computer -samba4.priv_attrs.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_msDS-AllowedToDelegateTo_add_CC_WP_user -samba4.priv_attrs.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_msDS-AllowedToDelegateTo_add_CC_default_computer -samba4.priv_attrs.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_msDS-AllowedToDelegateTo_add_CC_default_user -samba4.priv_attrs.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_sidHistory_add_CC_WP_computer -samba4.priv_attrs.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_sidHistory_add_CC_WP_user -samba4.priv_attrs.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_sidHistory_add_CC_default_computer -samba4.priv_attrs.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_sidHistory_add_CC_default_user -samba4.priv_attrs.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_sidHistory_add_admin-add_WP_computer -samba4.priv_attrs.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_sidHistory_add_admin-add_WP_user -samba4.priv_attrs.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_sidHistory_add_admin-add_default_computer -samba4.priv_attrs.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_sidHistory_add_admin-add_default_user samba4.priv_attrs.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_userAccountControl-a2d-user_add_admin-add_WP_computer samba4.priv_attrs.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_userAccountControl-a2d-user_add_admin-add_WP_user samba4.priv_attrs.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_userAccountControl-a2d-user_add_admin-add_default_computer samba4.priv_attrs.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_userAccountControl-a2d-user_add_admin-add_default_user -samba4.priv_attrs.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_msDS-SecondaryKrbTgtNumber_add_CC_WP_computer -samba4.priv_attrs.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_msDS-SecondaryKrbTgtNumber_add_CC_WP_user -samba4.priv_attrs.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_msDS-SecondaryKrbTgtNumber_add_CC_default_computer -samba4.priv_attrs.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_msDS-SecondaryKrbTgtNumber_add_CC_default_user -samba4.priv_attrs.strict.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_msDS-AllowedToDelegateTo_add_CC_WP_computer -samba4.priv_attrs.strict.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_msDS-AllowedToDelegateTo_add_CC_WP_user -samba4.priv_attrs.strict.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_msDS-AllowedToDelegateTo_add_CC_default_computer -samba4.priv_attrs.strict.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_msDS-AllowedToDelegateTo_add_CC_default_user -samba4.priv_attrs.strict.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_msDS-SecondaryKrbTgtNumber_add_CC_WP_computer -samba4.priv_attrs.strict.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_msDS-SecondaryKrbTgtNumber_add_CC_WP_user -samba4.priv_attrs.strict.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_msDS-SecondaryKrbTgtNumber_add_CC_default_computer -samba4.priv_attrs.strict.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_msDS-SecondaryKrbTgtNumber_add_CC_default_user samba4.priv_attrs.strict.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_sidHistory_add_CC_WP_computer samba4.priv_attrs.strict.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_sidHistory_add_CC_WP_user samba4.priv_attrs.strict.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_sidHistory_add_CC_default_computer diff --git a/source4/dsdb/samdb/ldb_modules/samldb.c b/source4/dsdb/samdb/ldb_modules/samldb.c index 4da8564c77a..cb5fda324a4 100644 --- a/source4/dsdb/samdb/ldb_modules/samldb.c +++ b/source4/dsdb/samdb/ldb_modules/samldb.c @@ -1976,6 +1976,29 @@ static int samldb_check_user_account_control_invariants(struct samldb_ctx *ac, return ret; } +static int samldb_get_domain_secdesc(struct samldb_ctx *ac, + struct security_descriptor **domain_sd) +{ + const char * const sd_attrs[] = {"ntSecurityDescriptor", NULL}; + struct ldb_result *res; + struct ldb_dn *domain_dn = ldb_get_default_basedn(ldb_module_get_ctx(ac->module)); + int ret = dsdb_module_search_dn(ac->module, ac, &res, + domain_dn, + sd_attrs, + DSDB_FLAG_NEXT_MODULE | DSDB_SEARCH_SHOW_DELETED, + ac->req); + if (ret != LDB_SUCCESS) { + return ret; + } + if (res->count != 1) { + return ldb_module_operr(ac->module); + } + + return dsdb_get_sd_from_ldb_message(ldb_module_get_ctx(ac->module), + ac, res->msgs[0], domain_sd); + +} + /** * Validate that the restriction in point 5 of MS-SAMR 3.1.1.8.10 userAccountControl is honoured * @@ -1987,12 +2010,8 @@ static int samldb_check_user_account_control_acl(struct samldb_ctx *ac, { int i, ret = 0; bool need_acl_check = false; - struct ldb_result *res; - const char * const sd_attrs[] = {"ntSecurityDescriptor", NULL}; 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; @@ -2078,21 +2097,7 @@ static int samldb_check_user_account_control_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); - 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); - + ret = samldb_get_domain_secdesc(ac, &domain_sd); if (ret != LDB_SUCCESS) { return ret; } @@ -2154,6 +2159,8 @@ static int samldb_check_user_account_control_acl(struct samldb_ctx *ac, return ldb_module_operr(ac->module); } if (map[i].guid) { + struct ldb_dn *domain_dn + = ldb_get_default_basedn(ldb_module_get_ctx(ac->module)); dsdb_acl_debug(domain_sd, acl_user_token(ac->module), domain_dn, true, @@ -3486,7 +3493,98 @@ static char *refer_if_rodc(struct ldb_context *ldb, struct ldb_request *req, return NULL; } +/* + * Restrict all access to sensitive attributes. + * + * We don't want to even inspect the values, so we can use the same + * routine for ADD and MODIFY. + * + */ + +static int samldb_check_sensitive_attributes(struct samldb_ctx *ac) +{ + struct ldb_message_element *el = NULL; + struct security_token *user_token = NULL; + int ret; + + if (dsdb_module_am_system(ac->module)) { + return LDB_SUCCESS; + } + + user_token = acl_user_token(ac->module); + if (user_token == NULL) { + return LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS; + } + + el = ldb_msg_find_element(ac->msg, "sidHistory"); + if (el) { + /* + * sidHistory is restricted to the (not implemented + * yet in Samba) DsAddSidHistory call (direct LDB access is + * as SYSTEM so will bypass this). + * + * If you want to modify this, say to merge domains, + * directly modify the sam.ldb as root. + */ + ldb_asprintf_errstring(ldb_module_get_ctx(ac->module), + "sidHistory " + "(entry %s) cannot be created " + "or changed over LDAP!", + ldb_dn_get_linearized(ac->msg->dn)); + return LDB_ERR_UNWILLING_TO_PERFORM; + } + el = ldb_msg_find_element(ac->msg, "msDS-SecondaryKrbTgtNumber"); + if (el) { + struct security_descriptor *domain_sd; + /* + * msDS-SecondaryKrbTgtNumber allows the creator to + * become an RODC, this is trusted as an RODC + * account + */ + ret = samldb_get_domain_secdesc(ac, &domain_sd); + if (ret != LDB_SUCCESS) { + return ret; + } + ret = acl_check_extended_right(ac, domain_sd, + user_token, + GUID_DRS_DS_INSTALL_REPLICA, + SEC_ADS_CONTROL_ACCESS, + NULL); + if (ret != LDB_SUCCESS) { + ldb_asprintf_errstring(ldb_module_get_ctx(ac->module), + "msDS-SecondaryKrbTgtNumber " + "(entry %s) cannot be created " + "or changed without " + "DS-Install-Replica extended right!", + ldb_dn_get_linearized(ac->msg->dn)); + return ret; + } + } + + el = ldb_msg_find_element(ac->msg, "msDS-AllowedToDelegateTo"); + if (el) { + /* + * msDS-AllowedToDelegateTo is incredibly powerful, + * given that it allows a server to become ANY USER on + * the target server only listed by SPN so needs to be + * protected just as the userAccountControl + * UF_TRUSTED_FOR_DELEGATION is. + */ + + bool have_priv = security_token_has_privilege(user_token, + SEC_PRIV_ENABLE_DELEGATION); + if (have_priv == false) { + ldb_asprintf_errstring(ldb_module_get_ctx(ac->module), + "msDS-AllowedToDelegateTo " + "(entry %s) cannot be created " + "or changed without SePrivEnableDelegation!", + ldb_dn_get_linearized(ac->msg->dn)); + return LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS; + } + } + return LDB_SUCCESS; +} /* add */ static int samldb_add(struct ldb_module *module, struct ldb_request *req) { @@ -3533,6 +3631,12 @@ static int samldb_add(struct ldb_module *module, struct ldb_request *req) return ldb_operr(ldb); } + ret = samldb_check_sensitive_attributes(ac); + if (ret != LDB_SUCCESS) { + talloc_free(ac); + return ret; + } + el = ldb_msg_find_element(ac->msg, "fSMORoleOwner"); if (el != NULL) { ret = samldb_fsmo_role_owner_check(ac); @@ -3740,6 +3844,12 @@ static int samldb_modify(struct ldb_module *module, struct ldb_request *req) return ldb_operr(ldb); } + ret = samldb_check_sensitive_attributes(ac); + if (ret != LDB_SUCCESS) { + talloc_free(ac); + return ret; + } + if (is_undelete == NULL) { el = ldb_msg_find_element(ac->msg, "primaryGroupID"); if (el != NULL) {