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

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>

diff --git a/selftest/knownfail.d/bug-14832 b/selftest/knownfail.d/bug-14832
deleted file mode 100644 (file)
index 059a177..0000000
--- a/selftest/knownfail.d/bug-14832
+++ /dev/null
@@ -1 +0,0 @@
-^samba4.ldap.acl.python\(.*\).__main__.AclSPNTests.test_user_spn\(.*\)
\ No newline at end of file

diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c
index 769e589e265085e211bd535950937e70242cd536..9b4afa452153db65d4b774cbdda79ed385d38bd4 100644 (file)
--- a/source4/dsdb/common/util.c
+++ b/source4/dsdb/common/util.c
@@ -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
 */

diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c
index 802969d50d5f7b10b8ba8b89a030c8047cee908c..723c5ae07cabb677819315b715de26ea0b9d17b3 100644 (file)
--- a/source4/dsdb/samdb/ldb_modules/acl.c
+++ b/source4/dsdb/samdb/ldb_modules/acl.c
@@ -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);
 }

diff --git a/source4/dsdb/samdb/ldb_modules/acl_util.c b/source4/dsdb/samdb/ldb_modules/acl_util.c
index f917d99517a4dc3ed540ad7f4aec26130f24fe0a..08a95c1c310e2b41fa5f786f0c235e60b2ee4963 100644 (file)
--- a/source4/dsdb/samdb/ldb_modules/acl_util.c
+++ b/source4/dsdb/samdb/ldb_modules/acl_util.c
@@ -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);
 

diff --git a/source4/dsdb/samdb/ldb_modules/dirsync.c b/source4/dsdb/samdb/ldb_modules/dirsync.c
index 0aa3edac3dc3c77e9f9a1bfeb9674c104031a0e7..fa57af49e8f5227919fa9b59e1c7757c3269d6bb 100644 (file)
--- a/source4/dsdb/samdb/ldb_modules/dirsync.c
+++ b/source4/dsdb/samdb/ldb_modules/dirsync.c
@@ -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;

diff --git a/source4/dsdb/samdb/ldb_modules/samldb.c b/source4/dsdb/samdb/ldb_modules/samldb.c
index 140cc22cc53d28705ea846edbe39b01ec5e3a517..edf153d9fb9e95c3a192dc8b319ec61aa7afe166 100644 (file)
--- a/source4/dsdb/samdb/ldb_modules/samldb.c
+++ b/source4/dsdb/samdb/ldb_modules/samldb.c
@@ -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);
-       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_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_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,