From 134c659d4025b2d85c825456fa0c81b47a9a8bb4 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Mon, 27 Feb 2023 13:55:36 +1300 Subject: CVE-2023-0614 s4-acl: Split out function to set up access checking variables These variables are often used together, and it is useful to have the setup code in one place. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270 Signed-off-by: Joseph Sutton Reviewed-by: Andrew Bartlett [abartlet@samba.org adapted to the use of acl_check_access_on_attribute as acl_check_access_on_attribute_implicit_owner is only in Samba 4.18 and newer] --- source4/dsdb/samdb/ldb_modules/acl_read.c | 113 +++++++++++++++++++++--------- 1 file changed, 80 insertions(+), 33 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/acl_read.c b/source4/dsdb/samdb/ldb_modules/acl_read.c index 3980c44345e..6659c71c965 100644 --- a/source4/dsdb/samdb/ldb_modules/acl_read.c +++ b/source4/dsdb/samdb/ldb_modules/acl_read.c @@ -70,6 +70,13 @@ struct aclread_private { struct ldb_val sd_cached_blob; }; +struct access_check_context { + struct security_descriptor *sd; + struct dom_sid sid_buf; + const struct dom_sid *sid; + const struct dsdb_class *objectclass; +}; + /* * the object has a parent, so we have to check for visibility * @@ -254,7 +261,7 @@ static int aclread_check_object_visible(struct aclread_context *ac, */ static int aclread_get_sd_from_ldb_message(struct aclread_context *ac, - struct ldb_message *acl_res, + const struct ldb_message *acl_res, struct security_descriptor **sd) { struct ldb_message_element *sd_element; @@ -358,7 +365,7 @@ static uint32_t get_attr_access_mask(const struct dsdb_attribute *attr, struct parse_tree_aclread_ctx { struct aclread_context *ac; TALLOC_CTX *mem_ctx; - struct dom_sid *sid; + const struct dom_sid *sid; struct ldb_dn *dn; struct security_descriptor *sd; const struct dsdb_class *objectclass; @@ -372,7 +379,7 @@ static int check_attr_access_rights(TALLOC_CTX *mem_ctx, const char *attr_name, struct aclread_context *ac, struct security_descriptor *sd, const struct dsdb_class *objectclass, - struct dom_sid *sid, struct ldb_dn *dn) + const struct dom_sid *sid, struct ldb_dn *dn) { int ret; const struct dsdb_attribute *attr = NULL; @@ -448,6 +455,69 @@ static const char * parse_tree_get_attr(struct ldb_parse_tree *tree) return attr; } +static int setup_access_check_context(struct aclread_context *ac, + const struct ldb_message *msg, + struct access_check_context *ctx) +{ + int ret; + + /* + * Fetch the schema so we can check which attributes are + * considered confidential. + */ + if (ac->schema == NULL) { + struct ldb_context *ldb = ldb_module_get_ctx(ac->module); + + /* Cache the schema for later use. */ + ac->schema = dsdb_get_schema(ldb, ac); + + if (ac->schema == NULL) { + return ldb_error(ldb, LDB_ERR_OPERATIONS_ERROR, + "aclread_callback: Error obtaining schema."); + } + } + + /* Fetch the object's security descriptor. */ + ret = aclread_get_sd_from_ldb_message(ac, msg, &ctx->sd); + if (ret != LDB_SUCCESS) { + ldb_debug_set(ldb_module_get_ctx(ac->module), LDB_DEBUG_FATAL, + "acl_read: cannot get descriptor of %s: %s\n", + ldb_dn_get_linearized(msg->dn), ldb_strerror(ret)); + return LDB_ERR_OPERATIONS_ERROR; + } else if (ctx->sd == NULL) { + ldb_debug_set(ldb_module_get_ctx(ac->module), LDB_DEBUG_FATAL, + "acl_read: cannot get descriptor of %s (attribute not found)\n", + ldb_dn_get_linearized(msg->dn)); + return LDB_ERR_OPERATIONS_ERROR; + } + /* + * Get the most specific structural object class for the ACL check + */ + ctx->objectclass = dsdb_get_structural_oc_from_msg(ac->schema, msg); + if (ctx->objectclass == NULL) { + ldb_asprintf_errstring(ldb_module_get_ctx(ac->module), + "acl_read: Failed to find a structural class for %s", + ldb_dn_get_linearized(msg->dn)); + return LDB_ERR_OPERATIONS_ERROR; + } + + /* Fetch the object's SID. */ + ret = samdb_result_dom_sid_buf(msg, "objectSid", &ctx->sid_buf); + if (ret == LDB_SUCCESS) { + ctx->sid = &ctx->sid_buf; + } else if (ret == LDB_ERR_NO_SUCH_ATTRIBUTE) { + /* This is expected. */ + ctx->sid = NULL; + } else { + ldb_asprintf_errstring(ldb_module_get_ctx(ac->module), + "acl_read: Failed to parse objectSid as dom_sid for %s", + ldb_dn_get_linearized(msg->dn)); + return ret; + } + + return LDB_SUCCESS; +} + /* * Checks a single attribute in the search parse-tree to make sure the user has * sufficient rights to view it. @@ -522,7 +592,7 @@ static int check_search_ops_access(struct aclread_context *ac, TALLOC_CTX *mem_ctx, struct security_descriptor *sd, const struct dsdb_class *objectclass, - struct dom_sid *sid, struct ldb_dn *dn, + const struct dom_sid *sid, struct ldb_dn *dn, bool *suppress_result) { int ret; @@ -585,10 +655,8 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares) struct ldb_message *msg; int ret; unsigned int i; - struct security_descriptor *sd = NULL; - struct dom_sid *sid = NULL; + struct access_check_context acl_ctx; TALLOC_CTX *tmp_ctx; - const struct dsdb_class *objectclass; bool suppress_result = false; ac = talloc_get_type_abort(req->context, struct aclread_context); @@ -604,32 +672,11 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares) switch (ares->type) { case LDB_REPLY_ENTRY: msg = ares->message; - ret = aclread_get_sd_from_ldb_message(ac, msg, &sd); + ret = setup_access_check_context(ac, msg, &acl_ctx); if (ret != LDB_SUCCESS) { - ldb_debug_set(ldb, LDB_DEBUG_FATAL, - "acl_read: cannot get descriptor of %s: %s\n", - ldb_dn_get_linearized(msg->dn), ldb_strerror(ret)); - ret = LDB_ERR_OPERATIONS_ERROR; - goto fail; - } else if (sd == NULL) { - ldb_debug_set(ldb, LDB_DEBUG_FATAL, - "acl_read: cannot get descriptor of %s (attribute not found)\n", - ldb_dn_get_linearized(msg->dn)); - ret = LDB_ERR_OPERATIONS_ERROR; - goto fail; - } - /* - * Get the most specific structural object class for the ACL check - */ - objectclass = dsdb_get_structural_oc_from_msg(ac->schema, msg); - if (objectclass == NULL) { - ldb_asprintf_errstring(ldb, "acl_read: Failed to find a structural class for %s", - ldb_dn_get_linearized(msg->dn)); - ret = LDB_ERR_OPERATIONS_ERROR; - goto fail; + return ret; } - sid = samdb_result_dom_sid(tmp_ctx, msg, "objectSid"); if (!ldb_dn_is_null(msg->dn)) { /* * this is a real object, so we have @@ -678,8 +725,8 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares) ret = acl_check_access_on_attribute(ac->module, tmp_ctx, - sd, - sid, + acl_ctx.sd, + acl_ctx.sid, access_mask, attr, objectclass); @@ -733,7 +780,7 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares) * check access rights for the search attributes, as well as the * attribute values actually being returned */ - ret = check_search_ops_access(ac, tmp_ctx, sd, objectclass, sid, + ret = check_search_ops_access(ac, tmp_ctx, acl_ctx.sd, acl_ctx.objectclass, acl_ctx.sid, msg->dn, &suppress_result); if (ret != LDB_SUCCESS) { ldb_debug_set(ldb, LDB_DEBUG_FATAL, -- cgit v1.2.1