From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 996371FF17E for ; Thu, 13 Nov 2025 17:38:51 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 997BC24050; Thu, 13 Nov 2025 17:39:46 +0100 (CET) Mime-Version: 1.0 Date: Thu, 13 Nov 2025 17:39:13 +0100 Message-Id: To: "Thomas Lamprecht" X-Mailer: aerc 0.20.0 References: <20251106143836.288888-1-s.sterz@proxmox.com> <1763041798.famz3bfta9.astroid@yuna.none> <4793f1cc-d93a-46f6-b129-f95ee2b3b1ac@proxmox.com> In-Reply-To: <4793f1cc-d93a-46f6-b129-f95ee2b3b1ac@proxmox.com> From: "Shannon Sterz" X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1763051927316 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.062 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp v3 00/10] add support for checking acl permissions in (yew) front-ends X-BeenThere: pdm-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Datacenter Manager development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Datacenter Manager development discussion Cc: Proxmox Datacenter Manager development discussion , Thomas Lamprecht Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pdm-devel-bounces@lists.proxmox.com Sender: "pdm-devel" On Thu Nov 13, 2025 at 5:18 PM CET, Thomas Lamprecht wrote: > Am 13.11.25 um 15:26 schrieb Shannon Sterz: >>> high-level question: >>> >>> the actual privilege checks in the backend use the full set of ACLs. the >>> frontend can only ever see a subset of ACLs, since giving it all ACLs >>> would leak a lot of sensitive information. >>> >>> doesn't that mean that the frontend will make wrong decisions in some >>> scenarios? >>> >>> e.g., the backend currently doesn't return any group ACLs if you do >>> exact filtering. but group ACLs can influence the ACL resolution >> >> as discussed off list, that'd be true if the filter in >> `extract_acl_node_data` isn't adapted. i'll add a fixme comment to the >> next version of this series for now. >> >> how exactely to takle this will depend on how we implement groups: >> >> * does a user know that they are part of a group? > > No general objection, a lot of ACL system work that way, but should not > be required. > >> * if disclosing such membership is fine, is it fine to disclose what the >> group has access to in all cases? e.g. what if the user is part of a >> group, but certain acl entries are then restricted on top via a >> NoAccess privilege or similar? > > Disclosing access is IMO always fine, that can basically be probed anyway. > Where that access comes from does not really matter here. > >> * will looking up whether the user is part of a group be handled by the >> acl tree directly? (this is at least indicated by comments already >> present in `AclTreeNode::extract_group_roles()`) > > Probably, otherwise it would need to get the relevant acl's "injected". > >> most of these are difficult to answer without actually tackling an >> implementation of the group feature. not entirely sure how i can address >> this here beside adding that `fixme` comment. > > Seems OK to me. If we need more changes to make this safe enough for > groups we can still change this, be it by adding a new endpoint and > deprecating the old one, where the old one would then probably ignores > groups for the rest of its existence. > One option could then be to have something like the heuristic in PVE > but more generic (and maybe we're able to make it a bit easier to > understand), but tbh. that isn't exactly perfect either, and so trying > this route until we get an actual blocker seems still worthwhile to me. > > >> i suppose i could also try to extract the roles with `AclTree::roles`, >> which extracts the roles via `AclTreeNode::extract_roles` which is >> already somewhat opinionated about how groups should work here. not sure >> what is ideal here. > > That I did not looked closely enough into to answer for sure. thanks for your quick reply, i looked into this some more after i send this mail and i have a version of this series here now that uses `AclTreeNode::extract_roles` to extract the roles for user in general. the upshot of this approach is that we already use the acl tree's implementation here instead of manually extracting the roles. so once we add better support for groups in the acl tree, we'll pick them up here for free. context: the acl tree has somewhat half-baked support for groups already and supports extracting them correctly for a given user. the part that is missing there is the look up of what groups a user belongs too. so relying on that seems somewhat safe to me as the pre-existing codepaths have been around for a while now. it looks somewhat like this: if all_for_authid { if let Some(auth_id) = auth_id_filter { for (role, propagate) in node.extract_roles(auth_id, true) { to_return.push(AclListItem { path: path_str.to_owned(), propagate, // do not disclose what groups exist and by // making them always look like user permissions ugid_type: AclUgidType::User, ugid: auth_id.to_string(), roleid: role.to_string(), }) } } what do you think? should we go with this approach instead? _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel