• Bug#1033661: marked as done (unblock: samba/2:4.17.7+dfsg-1) (10/12)

    From Debian Bug Tracking System@21:1/5 to All on Thu Mar 30 17:00:01 2023
    [continued from previous message]

    - private_data->sd_cached_blob = sd_element->values[0];
    - talloc_steal(private_data, sd_element->values[0].data);
    - } else {
    - private_data->sd_cached_blob = ldb_val_dup(private_data,
    - &sd_element->values[0]);
    - if (private_data->sd_cached_blob.data == NULL) {
    - TALLOC_FREE(*sd);
    - return ldb_operr(ldb);
    - }
    + private_data->sd_cached_blob = ldb_val_dup(private_data,
    + &sd_element->values[0]);
    + if (private_data->sd_cached_blob.data == NULL) {
    + TALLOC_FREE(*sd);
    + return ldb_operr(ldb);
    }

    talloc_unlink(private_data, private_data->sd_cached);
    @@ -327,6 +510,23 @@
    return LDB_SUCCESS;
    }

    +/* Check whether the attribute is a password attribute. */
    +static bool attr_is_secret(const char *attr, const struct aclread_private *private_data)
    +{
    + const char **found = NULL;
    +
    + if (private_data->password_attrs == NULL) {
    + return false;
    + }
    +
    + BINARY_ARRAY_SEARCH_V(private_data->password_attrs,
    + private_data->num_password_attrs,
    + attr,
    + ldb_attr_cmp,
    + found);
    + return found != NULL;
    +}
    +
    /*
    * Returns the access mask required to read a given attribute
    */
    @@ -362,61 +562,59 @@
    return access_mask;
    }

    -/* helper struct for traversing the attributes in the search-tree */
    -struct parse_tree_aclread_ctx {
    - struct aclread_context *ac;
    - TALLOC_CTX *mem_ctx;
    - struct dom_sid *sid;
    - struct ldb_dn *dn;
    - struct security_descriptor *sd;
    - const struct dsdb_class *objectclass;
    - bool suppress_result;
    -};
    -
    /*
    - * Checks that the user has sufficient access rights to view an attribute
    + * Checks that the user has sufficient access rights to view an attribute, else
    + * marks it as inaccessible.
    */
    -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) +static int acl_redact_attr(TALLOC_CTX *mem_ctx,
    + struct ldb_message_element *el,
    + struct aclread_context *ac,
    + const struct aclread_private *private_data,
    + const struct ldb_message *msg,
    + const struct dsdb_schema *schema,
    + const struct security_descriptor *sd,
    + const struct dom_sid *sid,
    + const struct dsdb_class *objectclass)
    {
    int ret;
    const struct dsdb_attribute *attr = NULL;
    uint32_t access_mask;
    struct ldb_context *ldb = ldb_module_get_ctx(ac->module);

    - attr = dsdb_attribute_by_lDAPDisplayName(ac->schema, attr_name);
    + if (attr_is_secret(el->name, private_data)) {
    + ldb_msg_element_mark_inaccessible(el);
    + return LDB_SUCCESS;
    + }
    +
    + /* Look up the attribute in the schema. */
    + attr = dsdb_attribute_by_lDAPDisplayName(schema, el->name);
    if (!attr) {
    ldb_debug_set(ldb,
    - LDB_DEBUG_TRACE,
    - "acl_read: %s cannot find attr[%s] in schema,"
    - "ignoring\n",
    - ldb_dn_get_linearized(dn), attr_name);
    - return LDB_SUCCESS;
    + LDB_DEBUG_FATAL,
    + "acl_read: %s cannot find attr[%s] in schema\n", + ldb_dn_get_linearized(msg->dn), el->name);
    + return LDB_ERR_OPERATIONS_ERROR;
    }

    access_mask = get_attr_access_mask(attr, ac->sd_flags);
    -
    - /* the access-mask should be non-zero. Skip attribute otherwise */
    if (access_mask == 0) {
    DBG_ERR("Could not determine access mask for attribute %s\n",
    - attr_name);
    + el->name);
    + ldb_msg_element_mark_inaccessible(el);
    return LDB_SUCCESS;
    }

    + /* We must check whether the user has rights to view the attribute. */ +
    ret = acl_check_access_on_attribute(ac->module, mem_ctx, sd, sid,
    access_mask, attr, objectclass);

    if (ret == LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS) {
    - return ret;
    - }
    -
    - if (ret != LDB_SUCCESS) {
    + ldb_msg_element_mark_inaccessible(el);
    + } else if (ret != LDB_SUCCESS) {
    ldb_debug_set(ldb, LDB_DEBUG_FATAL,
    "acl_read: %s check attr[%s] gives %s - %s\n",
    - ldb_dn_get_linearized(dn), attr_name,
    + ldb_dn_get_linearized(msg->dn), el->name,
    ldb_strerror(ret), ldb_errstring(ldb));
    return ret;
    }
    @@ -424,152 +622,112 @@
    return LDB_SUCCESS;
    }

    -/*
    - * Returns the attribute name for this particular level of a search operation - * parse-tree.
    - */
    -static const char * parse_tree_get_attr(struct ldb_parse_tree *tree)
    +static int setup_access_check_context(struct aclread_context *ac,
    + const struct ldb_message *msg,
    + struct access_check_context *ctx)
    {
    - const char *attr = NULL;
    -
    - switch (tree->operation) {
    - case LDB_OP_EQUALITY:
    - case LDB_OP_GREATER:
    - case LDB_OP_LESS:
    - case LDB_OP_APPROX:
    - attr = tree->u.equality.attr;
    - break;
    - case LDB_OP_SUBSTRING:
    - attr = tree->u.substring.attr;
    - break;
    - case LDB_OP_PRESENT:
    - attr = tree->u.present.attr;
    - break;
    - case LDB_OP_EXTENDED:
    - attr = tree->u.extended.attr;
    - break;
    -
    - /* we'll check LDB_OP_AND/_OR/_NOT children later on in the walk */
    - default:
    - break;
    - }
    - return attr;
    -}
    -
    -/*
    - * Checks a single attribute in the search parse-tree to make sure the user has
    - * sufficient rights to view it.
    - */
    -static int parse_tree_check_attr_access(struct ldb_parse_tree *tree,
    - void *private_context)
    -{
    - struct parse_tree_aclread_ctx *ctx = NULL;
    - const char *attr_name = NULL;
    int ret;
    - static const char * const attrs_always_present[] = {
    - "objectClass",
    - "distinguishedName",
    - "name",
    - "objectGUID",
    - NULL
    - };
    -
    - ctx = (struct parse_tree_aclread_ctx *)private_context;

    /*
    - * we can skip any further checking if we already know that this object - * shouldn't be visible in this user's search
    + * Fetch the schema so we can check which attributes are
    + * considered confidential.
    */
    - if (ctx->suppress_result) {
    - return LDB_SUCCESS;
    - }
    + if (ac->schema == NULL) {
    + struct ldb_context *ldb = ldb_module_get_ctx(ac->module);

    - /* skip this level of the search-tree if it has no attribute to check */
    - attr_name = parse_tree_get_attr(tree);
    - if (attr_name == NULL) {
    - return LDB_SUCCESS;
    + /* 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;
    + }
    /*
    - * If the search filter is checking for an attribute's presence, and the
    - * attribute is always present, we can skip access rights checks. Every - * object has these attributes, and so there's no security reason to
    - * hide their presence.
    - * Note: the acl.py tests (e.g. test_search1()) rely on this exception. - * I.e. even if we lack Read Property (RP) rights for a child object, it
    - * should still appear as a visible object in 'objectClass=*' searches, - * so long as we have List Contents (LC) rights for the object.
    + * Get the most specific structural object class for the ACL check
    */
    - if (tree->operation == LDB_OP_PRESENT &&
    - is_attr_in_list(attrs_always_present, attr_name)) {
    - return LDB_SUCCESS;
    + 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;
    }

    - ret = check_attr_access_rights(ctx->mem_ctx, attr_name, ctx->ac,
    - ctx->sd, ctx->objectclass, ctx->sid,
    - ctx->dn);
    -
    - /*
    - * if the user does not have the rights to view this attribute, then we - * should not return the object as a search result, i.e. act as if the - * object doesn't exist (for this particular user, at least)
    - */
    - if (ret == LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS) {
    - ctx->suppress_result = true;
    - return LDB_SUCCESS;
    + /* 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 ret;
    + return LDB_SUCCESS;
    }

    /*
    - * Traverse the search-tree to check that the user has sufficient access rights
    - * to view all the attributes.
    + * Whether this attribute was added to perform access checks and must be
    + * removed.
    */
    -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,
    - bool *suppress_result)
    +static bool should_remove_attr(const char *attr, const struct aclread_context *ac)
    {
    - int ret;
    - struct parse_tree_aclread_ctx ctx = { 0 };
    - struct ldb_parse_tree *tree = ac->req->op.search.tree;
    + if (ac->added_nTSecurityDescriptor &&
    + ldb_attr_cmp("nTSecurityDescriptor", attr) == 0)
    + {
    + return true;
    + }
    +
    + if (ac->added_objectSid &&
    + ldb_attr_cmp("objectSid", attr) == 0)
    + {
    + return true;
    + }

    - ctx.ac = ac;
    - ctx.mem_ctx = mem_ctx;
    - ctx.suppress_result = false;
    - ctx.sid = sid;
    - ctx.dn = dn;
    - ctx.sd = sd;
    - ctx.objectclass = objectclass;
    + if (ac->added_instanceType &&
    + ldb_attr_cmp("instanceType", attr) == 0)
    + {
    + return true;
    + }

    - /* walk the search tree, checking each attribute as we go */
    - ret = ldb_parse_tree_walk(tree, parse_tree_check_attr_access, &ctx);
    + if (ac->added_objectClass &&
    + ldb_attr_cmp("objectClass", attr) == 0)
    + {
    + return true;
    + }

    - /* return whether this search result should be hidden to this user */
    - *suppress_result = ctx.suppress_result;
    - return ret;
    + return false;
    }

    static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares)
    {
    - struct ldb_context *ldb;
    struct aclread_context *ac;
    - struct ldb_message *ret_msg;
    + struct aclread_private *private_data = NULL;
    struct ldb_message *msg;
    int ret;
    - size_t num_of_attrs = 0;
    - unsigned int i, k = 0;
    - struct security_descriptor *sd = NULL;
    - struct dom_sid *sid = NULL;
    - TALLOC_CTX *tmp_ctx;
    - const struct dsdb_class *objectclass;
    - bool suppress_result = false;
    + unsigned int i;
    + struct access_check_context acl_ctx;

    - ac = talloc_get_type(req->context, struct aclread_context);
    - ldb = ldb_module_get_ctx(ac->module);
    + ac = talloc_get_type_abort(req->context, struct aclread_context);
    if (!ares) {
    return ldb_module_done(ac->req, NULL, NULL, LDB_ERR_OPERATIONS_ERROR );
    }
    @@ -577,36 +735,10 @@
    return ldb_module_done(ac->req, ares->controls,
    ares->response, ares->error);
    }
    - tmp_ctx = talloc_new(ac);
    switch (ares->type) {
    case LDB_REPLY_ENTRY:
    msg = ares->message;
    - ret = aclread_get_sd_from_ldb_message(ac, msg, &sd);
    - 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;
    - }

    - sid = samdb_result_dom_sid(tmp_ctx, msg, "objectSid");
    if (!ldb_dn_is_null(msg->dn)) {
    /*
    * this is a real object, so we have
    @@ -614,187 +746,90 @@
    */
    ret = aclread_check_object_visible(ac, msg, req);
    if (ret == LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS) {
    - talloc_free(tmp_ctx);
    return LDB_SUCCESS;
    } else if (ret != LDB_SUCCESS) {
    + struct ldb_context *ldb = ldb_module_get_ctx(ac->module);
    ldb_debug_set(ldb, LDB_DEBUG_FATAL,
    "acl_read: %s check parent %s - %s\n",
    ldb_dn_get_linearized(msg->dn),
    ldb_strerror(ret),
    ldb_errstring(ldb));
    - goto fail;
    + return ldb_module_done(ac->req, NULL, NULL, ret);
    }
    }

    /* for every element in the message check RP */
    - for (i=0; i < msg->num_elements; i++) {
    - const struct dsdb_attribute *attr;
    - bool is_sd, is_objectsid, is_instancetype, is_objectclass;
    - uint32_t access_mask;
    - attr = dsdb_attribute_by_lDAPDisplayName(ac->schema,
    - msg->elements[i].name);
    - if (!attr) {
    - ldb_debug_set(ldb, LDB_DEBUG_FATAL,
    - "acl_read: %s cannot find attr[%s] in of schema\n",
    - ldb_dn_get_linearized(msg->dn),
    - msg->elements[i].name);
    - ret = LDB_ERR_OPERATIONS_ERROR;
    - goto fail;
    - }
    - is_sd = ldb_attr_cmp("nTSecurityDescriptor",
    - msg->elements[i].name) == 0;
    - is_objectsid = ldb_attr_cmp("objectSid",
    - msg->elements[i].name) == 0;
    - is_instancetype = ldb_attr_cmp("instanceType",
    - msg->elements[i].name) == 0;
    - is_objectclass = ldb_attr_cmp("objectClass",
    - msg->elements[i].name) == 0;
    - /* these attributes were added to perform access checks and must be removed */
    - if (is_objectsid && ac->added_objectSid) {
    - aclread_mark_inaccesslible(&msg->elements[i]); - continue;
    - }
    - if (is_instancetype && ac->added_instanceType) {
    - aclread_mark_inaccesslible(&msg->elements[i]); - continue;
    - }
    - if (is_objectclass && ac->added_objectClass) {
    - aclread_mark_inaccesslible(&msg->elements[i]); - continue;
    - }
    - if (is_sd && ac->added_nTSecurityDescriptor) {
    - aclread_mark_inaccesslible(&msg->elements[i]); + for (i = 0; i < msg->num_elements; ++i) {
    + struct ldb_message_element *el = &msg->elements[i];
    +
    + /* Remove attributes added to perform access checks. */ + if (should_remove_attr(el->name, ac)) {
    + ldb_msg_element_mark_inaccessible(el);
    continue;
    }

    - access_mask = get_attr_access_mask(attr, ac->sd_flags); -
    - if (access_mask == 0) {
    - aclread_mark_inaccesslible(&msg->elements[i]); + if (acl_element_is_access_checked(el)) {
    + /* We will have already checked this attribute. */
    continue;
    }

    - ret = acl_check_access_on_attribute(ac->module,
    - tmp_ctx,
    - sd,
    - sid,
    - access_mask,
    - attr,
    - objectclass);
    -
    /*
    - * Dirsync control needs the replpropertymetadata attribute
    - * so return it as it will be removed by the control
    - * in anycase.
    + * We need to fetch the security descriptor to check
    + * this attribute.
    */
    - if (ret == LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS) {
    - bool in_search_filter;
    + break;
    + }

    - /* check if attr is part of the search filter */
    - in_search_filter = dsdb_attr_in_parse_tree(ac->req->op.search.tree,
    - msg->elements[i].name);
    -
    - if (in_search_filter) {
    -
    - /*
    - * We are doing dirysnc answers
    - * and the object shouldn't be returned (normally)
    - * but we will return it without replPropertyMetaData
    - * so that the dirysync module will do what is needed
    - * (remove the object if it is not deleted, or return
    - * just the objectGUID if it's deleted).
    - */
    - if (ac->indirsync) {
    - ldb_msg_remove_attr(msg, "replPropertyMetaData");
    - break;
    - } else {
    -
    - /* do not return this entry */ - talloc_free(tmp_ctx);
    - return LDB_SUCCESS;
    - }
    - } else {
    - aclread_mark_inaccesslible(&msg->elements[i]);
    - }
    - } else if (ret != LDB_SUCCESS) {
    - ldb_debug_set(ldb, LDB_DEBUG_FATAL,
    - "acl_read: %s check attr[%s] gives %s - %s\n",
    - ldb_dn_get_linearized(msg->dn),
    - msg->elements[i].name,
    - ldb_strerror(ret),
    - ldb_errstring(ldb));
    - goto fail;
    - }
    + if (i == msg->num_elements) {
    + /* All elements have been checked. */
    + goto reply_entry_done;
    }

    - /*
    - * 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,
    - msg->dn, &suppress_result);
    + ret = setup_access_check_context(ac, msg, &acl_ctx);
    if (ret != LDB_SUCCESS) {
    - ldb_debug_set(ldb, LDB_DEBUG_FATAL,
    - "acl_read: %s check search ops %s - %s\n",
    - ldb_dn_get_linearized(msg->dn),
    - ldb_strerror(ret), ldb_errstring(ldb));
    - goto fail;
    + return ret;
    }

    - if (suppress_result) {
    + private_data = talloc_get_type_abort(ldb_module_get_private(ac->module),
    + struct aclread_private);

    - /*
    - * As per the above logic, we strip replPropertyMetaData
    - * out of the msg so that the dirysync module will do
    - * what is needed (return just the objectGUID if it's, - * deleted, or remove the object if it is not).
    - */
    - if (ac->indirsync) {
    - ldb_msg_remove_attr(msg, "replPropertyMetaData");
    - } else {
    - talloc_free(tmp_ctx);
    - return LDB_SUCCESS;
    - }
    - }
    + for (/* begin where we left off */; i < msg->num_elements; ++i) {
    + struct ldb_message_element *el = &msg->elements[i];

    - for (i=0; i < msg->num_elements; i++) {
    - if (!aclread_is_inaccessible(&msg->elements[i])) {
    - num_of_attrs++;
    - }
    - }
    - /*create a new message to return*/
    - ret_msg = ldb_msg_new(ac->req);
    - ret_msg->dn = msg->dn;
    - talloc_steal(ret_msg, msg->dn);
    - ret_msg->num_elements = num_of_attrs;
    - if (num_of_attrs > 0) {
    - ret_msg->elements = talloc_array(ret_msg,
    - struct ldb_message_element,
    - num_of_attrs);
    - if (ret_msg->elements == NULL) {
    - return ldb_oom(ldb);
    + /* Remove attributes added to perform access checks. */ + if (should_remove_attr(el->name, ac)) {
    + ldb_msg_element_mark_inaccessible(el);
    + continue;
    }
    - for (i=0; i < msg->num_elements; i++) {
    - bool to_remove = aclread_is_inaccessible(&msg->elements[i]);
    - if (!to_remove) {
    - ret_msg->elements[k] = msg->elements[i];
    - talloc_steal(ret_msg->elements, msg->elements[i].name);
    - talloc_steal(ret_msg->elements, msg->elements[i].values);
    - k++;
    - }
    +
    + if (acl_element_is_access_checked(el)) {
    + /* We will have already checked this attribute. */
    + continue;
    }
    +
    /*
    - * This should not be needed, but some modules
    - * may allocate values on the wrong context...
    + * We need to check whether the attribute is secret,
    + * confidential, or access-controlled.
    */
    - talloc_steal(ret_msg->elements, msg);
    - } else {
    - ret_msg->elements = NULL;
    + ret = acl_redact_attr(ac,
    + el,
    + ac,
    + private_data,
    + msg,
    + ac->schema,
    + acl_ctx.sd,
    + acl_ctx.sid,
    + acl_ctx.objectclass);
    + if (ret != LDB_SUCCESS) {
    + return ldb_module_done(ac->req, NULL, NULL, ret);
    + }
    }
    - talloc_free(tmp_ctx);
    +
    + reply_entry_done:
    + ldb_msg_remove_inaccessible(msg);

    ac->num_entries++;
    - return ldb_module_send_entry(ac->req, ret_msg, ares->controls); + return ldb_module_send_entry(ac->req, msg, ares->controls);
    case LDB_REPLY_REFERRAL:
    return ldb_module_send_referral(ac->req, ares->referral);
    case LDB_REPLY_DONE:
    @@ -813,9 +848,6 @@

    }
    return LDB_SUCCESS;
    -fail:
    - talloc_free(tmp_ctx);
    - return ldb_module_done(ac->req, NULL, NULL, ret);
    }


    @@ -825,8 +857,7 @@
    int ret;
    struct aclread_context *ac;
    struct ldb_request *down_req;
    - struct ldb_control *as_system = ldb_request_get_control(req, LDB_CONTROL_AS_SYSTEM_OID);
    - uint32_t flags = ldb_req_get_custom_flags(req);
    + bool am_system;
    struct ldb_result *res;
    struct aclread_private *p;
    bool need_sd = false;
    @@ -843,11 +874,16 @@
    ldb = ldb_module_get_ctx(module);
    p = talloc_get_type(ldb_module_get_private(module), struct aclread_private);

    + am_system = ldb_request_get_control(req, LDB_CONTROL_AS_SYSTEM_OID) != NULL;
    + if (!am_system) {
    + am_system = dsdb_module_am_system(module);
    + }
    +
    /* skip access checks if we are system or system control is supplied
    * or this is not LDAP server request */
    if (!p || !p->enabled ||
    - dsdb_module_am_system(module)
    - || as_system || !is_untrusted) {
    + am_system ||
    + !is_untrusted) {
    return ldb_next_request(module, req);
    }
    /* no checks on special dn */
    @@ -861,15 +897,6 @@
    }
    ac->module = module;
    ac->req = req;
    - ac->schema = dsdb_get_schema(ldb, req);
    - if (flags & DSDB_ACL_CHECKS_DIRSYNC_FLAG) {
    - ac->indirsync = true;
    - } else {
    - ac->indirsync = false;
    - }
    - if (!ac->schema) {
    - return ldb_operr(ldb);
    - }

    attrs = req->op.search.attrs;
    if (attrs == NULL) {
    @@ -926,7 +953,7 @@
    ac->added_nTSecurityDescriptor = true;
    }

    - ac->attrs = req->op.search.attrs;
    + ac->am_administrator = dsdb_module_am_administrator(module);

    /* check accessibility of base */
    if (!ldb_dn_is_null(req->op.search.base)) {
    @@ -970,19 +997,287 @@
    return LDB_ERR_OPERATIONS_ERROR;
    }

    + /*
    + * We provide 'ac' as the control value, which is then used by the
    + * callback to avoid double-work.
    + */
    + ret = ldb_request_add_control(down_req, DSDB_CONTROL_ACL_READ_OID, false, ac);
    + if (ret != LDB_SUCCESS) {
    + return ldb_error(ldb, ret,
    + "acl_read: Error adding acl_read control.");
    + }
    +
    return ldb_next_request(module, down_req);
    }

    +/*
    + * Here we mark inaccessible attributes known to be looked for in the
    + * filter. This only redacts attributes found in the search expression. If any + * extended attribute match rules examine different attributes without their own
    + * access control checks, a security bypass is possible.
    + */
    +static int acl_redact_msg_for_filter(struct ldb_module *module, struct ldb_request *req, struct ldb_message *msg)
    +{
    + struct ldb_context *ldb = ldb_module_get_ctx(module);
    + const struct aclread_private *private_data = NULL;
    + struct ldb_control *control = NULL;
    + struct aclread_context *ac = NULL;
    + struct access_check_context acl_ctx;
    + int ret;
    + unsigned i;
    +
    + /*
    + * The private data contains a list of attributes which are to be
    + * considered secret.
    + */
    + private_data = talloc_get_type(ldb_module_get_private(module), struct aclread_private);
    + if (private_data == NULL) {
    + return ldb_error(ldb, LDB_ERR_OPERATIONS_ERROR,
    + "aclread_private data is missing");
    + }
    + if (!private_data->enabled) {
    + return LDB_SUCCESS;
    + }
    +
    + control = ldb_request_get_control(req, DSDB_CONTROL_ACL_READ_OID);
    + if (control == NULL) {
    + /*
    + * We've bypassed the acl_read module for this request, and
    + * should skip redaction in this case.
    + */
    + return LDB_SUCCESS;
    + }
    +
    + ac = talloc_get_type_abort(control->data, struct aclread_context);
    +
    + if (!ac->got_tree_attrs) {
    + ret = ldb_parse_tree_collect_acl_attrs(module, ac, &ac->tree_attrs, req->op.search.tree);
    + if (ret != LDB_SUCCESS) {
    + return ret;
    + }
    + ac->got_tree_attrs = true;
    + }
    +
    + for (i = 0; i < msg->num_elements; ++i) {
    + struct ldb_message_element *el = &msg->elements[i];
    +
    + /* Is the attribute mentioned in the search expression? */
    + if (attr_in_vec(&ac->tree_attrs, el->name)) {
    + /*
    + * We need to fetch the security descriptor to check
    + * this element.
    + */
    + break;
    + }
    +
    + /*
    + * This attribute is not in the search filter, so we can leave + * handling it till aclread_callback(), by which time we know
    + * this object is a match. This saves work checking ACLs if the + * search is unindexed and most objects don't match the filter. + */
    + }
    +
    + if (i == msg->num_elements) {
    + /* All elements have been checked. */
    + return LDB_SUCCESS;
    + }
    +
    + ret = setup_access_check_context(ac, msg, &acl_ctx);
    + if (ret != LDB_SUCCESS) {
    + return ret;
    + }
    +
    + /* For every element in the message and the parse tree, check RP. */
    +
    + for (/* begin where we left off */; i < msg->num_elements; ++i) {
    + struct ldb_message_element *el = &msg->elements[i];
    +
    + /* Is the attribute mentioned in the search expression? */
    + if (!attr_in_vec(&ac->tree_attrs, el->name)) {
    + /*
    + * If not, leave it for later and check the next
    + * attribute.
    + */
    + continue;
    + }
    +
    + /*
    + * We need to check whether the attribute is secret,
    + * confidential, or access-controlled.
    + */
    + ret = acl_redact_attr(ac,
    + el,
    + ac,
    + private_data,
    + msg,
    + ac->schema,
    + acl_ctx.sd,
    + acl_ctx.sid,
    + acl_ctx.objectclass);
    + if (ret != LDB_SUCCESS) {
    + return ret;
    + }
    +
    + acl_element_mark_access_checked(el);
    + }
    +
    + return LDB_SUCCESS;
    +}
    +
    +static int ldb_attr_cmp_fn(const void *_a, const void *_b)
    +{
    + const char * const *a = _a;
    + const char * const *b = _b;
    +
    + return ldb_attr_cmp(*a, *b);
    +}
    +
    static int aclread_init(struct ldb_module *module)
    {
    struct ldb_context *ldb = ldb_module_get_ctx(module);
    + unsigned int i, n, j;
    + TALLOC_CTX *mem_ctx = NULL;
    + int ret;
    + bool userPassword_support;
    + static const char * const attrs[] = { "passwordAttribute", NULL };
    + static const char * const secret_attrs[] = {
    + DSDB_SECRET_ATTRIBUTES
    + };
    + struct ldb_result *res;
    + struct ldb_message *msg;
    + struct ldb_message_element *password_attributes;
    struct aclread_private *p = talloc_zero(module, struct aclread_private);
    if (p == NULL) {
    return ldb_module_oom(module);
    }
    p->enabled = lpcfg_parm_bool(ldb_get_opaque(ldb, "loadparm"), NULL, "acl", "search", true);
    +
    + ret = ldb_mod_register_control(module, LDB_CONTROL_SD_FLAGS_OID);
    + if (ret != LDB_SUCCESS) {
    + ldb_debug(ldb, LDB_DEBUG_ERROR,
    + "acl_module_init: Unable to register sd_flags control with rootdse!\n");
    + return ldb_operr(ldb);
    + }
    +
    ldb_module_set_private(module, p);
    - return ldb_next_init(module);
    +
    + mem_ctx = talloc_new(module);
    + if (!mem_ctx) {
    + return ldb_oom(ldb);
    + }
    +
    + ret = dsdb_module_search_dn(module, mem_ctx, &res,
    + ldb_dn_new(mem_ctx, ldb, "@KLUDGEACL"),
    + attrs,
    + DSDB_FLAG_NEXT_MODULE |
    + DSDB_FLAG_AS_SYSTEM,
    + NULL);
    + if (ret != LDB_SUCCESS) {
    + goto done;
    + }
    + if (res->count == 0) {
    + goto done;
    + }
    +
    + if (res->count > 1) {
    + talloc_free(mem_ctx);
    + return LDB_ERR_CONSTRAINT_VIOLATION;
    + }
    +
    + msg = res->msgs[0];
    +
    + password_attributes = ldb_msg_find_element(msg, "passwordAttribute");
    + if (!password_attributes) {
    + goto done;
    + }
    + p->password_attrs = talloc_array(p, const char *,
    + password_attributes->num_values +
    + ARRAY_SIZE(secret_attrs));
    + if (!p->password_attrs) {
    + talloc_free(mem_ctx);
    + return ldb_oom(ldb);
    + }
    +
    + n = 0;
    + for (i=0; i < password_attributes->num_values; i++) {
    + p->password_attrs[n] = (const char *)password_attributes->values[i].data;
    + talloc_steal(p->password_attrs, password_attributes->values[i].data);
    + n++;
    + }
    +
    + for (i=0; i < ARRAY_SIZE(secret_attrs); i++) {
    + bool found = false;
    +
    + for (j=0; j < n; j++) {
    + if (strcasecmp(p->password_attrs[j], secret_attrs[i]) == 0) {
    + found = true;
    + break;
    + }
    + }
    +
    + if (found) {
    + continue;
    + }
    +
    + p->password_attrs[n] = talloc_strdup(p->password_attrs,
    + secret_attrs[i]);
    + if (p->password_attrs[n] == NULL) {
    + talloc_free(mem_ctx);
    + return ldb_oom(ldb);
    + }
    + n++;
    + }
    + p->num_password_attrs = n;
    +
    + /* Sort the password attributes so we can use binary search. */
    + TYPESAFE_QSORT(p->password_attrs, p->num_password_attrs, ldb_attr_cmp_fn);
    +
    + ret = ldb_register_redact_callback(ldb, acl_redact_msg_for_filter, module);
    + if (ret != LDB_SUCCESS) {
    + return ret;
    + }
    +
    +done:
    + talloc_free(mem_ctx);
    + ret = ldb_next_init(module);
    +
    + if (ret != LDB_SUCCESS) {
    + return ret;
    + }
    +
    + if (p->password_attrs != NULL) {
    + /*
    + * Check this after the modules have be initialised so we can
    + * actually read the backend DB.
    + */
    + userPassword_support = dsdb_user_password_support(module,
    + module,
    + NULL);
    + if (!userPassword_support) {
    + const char **found = NULL;
    +
    + /*
    + * Remove the userPassword attribute, as it is not
    + * considered secret.
    + */
    + BINARY_ARRAY_SEARCH_V(p->password_attrs,
    + p->num_password_attrs,
    + "userPassword",
    + ldb_attr_cmp,
    + found);
    + if (found != NULL) {
    + size_t found_idx = found - p->password_attrs;
    +
    + /* Shift following elements backwards by one. */
    + for (i = found_idx; i < p->num_password_attrs - 1; ++i) {
    + p->password_attrs[i] = p->password_attrs[i + 1];
    + }
    + --p->num_password_attrs;
    + }
    + }
    + }
    + return ret;
    }

    static const struct ldb_module_ops ldb_aclread_module_ops = {
    diff -Nru samba-4.17.6+dfsg/source4/dsdb/samdb/ldb_modules/acl_util.c samba-4.17.7+dfsg/source4/dsdb/samdb/ldb_modules/acl_util.c
    --- samba-4.17.6+dfsg/source4/dsdb/samdb/ldb_modules/acl_util.c 2022-08-08 17:15:39.548193500 +0300
    +++ samba-4.17.7+dfsg/source4/dsdb/samdb/ldb_modules/acl_util.c 2023-03-20 12:03:44.507649400 +0300
    @@ -97,8 +97,8 @@

    int acl_check_access_on_attribute(struct ldb_module *module,
    TALLOC_CTX *mem_ctx,
    - struct security_descriptor *sd,
    - struct dom_sid *rp_sid,
    + const struct security_descriptor *sd,
    + const struct dom_sid *rp_sid,
    uint32_t access_mask,
    const struct dsdb_attribute *attr,
    const struct dsdb_class *objectclass)
    @@ -298,7 +298,7 @@

    sd_control = ldb_request_get_control(req, LDB_CONTROL_SD_FLAGS_OID);
    if (sd_control != NULL && sd_control->data != NULL) {
    - struct ldb_sd_flags_control *sdctr = (struct ldb_sd_flags_control *)sd_control->data;
    + struct ldb_sd_flags_control *sdctr = talloc_get_type_abort(sd_control->data, struct ldb_sd_flags_control);

    sd_flags = sdctr->secinfo_flags;

    diff -Nru samba-4.17.6+dfsg/source4/dsdb