From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id EC21389B2 for ; Mon, 6 Mar 2023 10:42:58 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id C6CCD2EF2F for ; Mon, 6 Mar 2023 10:42:28 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Mon, 6 Mar 2023 10:42:28 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id E0F6541D6F for ; Mon, 6 Mar 2023 10:42:27 +0100 (CET) Message-ID: <66c708b5-55db-ad60-bbdb-25762fd8160c@proxmox.com> Date: Mon, 6 Mar 2023 10:42:27 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:111.0) Gecko/20100101 Thunderbird/111.0 To: Proxmox VE development discussion , =?UTF-8?Q?Fabian_Gr=C3=BCnbichler?= References: <20230215094413.196673-1-f.gruenbichler@proxmox.com> Content-Language: en-GB, de-AT From: Thomas Lamprecht In-Reply-To: <20230215094413.196673-1-f.gruenbichler@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL -0.201 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment POISEN_SPAM_PILL 0.1 Meta: its spam POISEN_SPAM_PILL_1 0.1 random spam to be learned in bayes POISEN_SPAM_PILL_3 0.1 random spam to be learned in bayes SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pve-devel] applied: [PATCH access-control] fix #4518: improve ACL computation performance X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 06 Mar 2023 09:42:59 -0000 Am 15/02/2023 um 10:44 schrieb Fabian Gr=C3=BCnbichler: > by switching to a tree-based in-memory structure, like we do in PBS. >=20 > instead of parsing ACL entries into a hash using the full ACL path as k= ey for > each entry, parse them into a tree-like nested hash. when evaluating AC= Ls, > iterating over all path prefixes starting at '/' is needed anyway, so t= his is a > more natural way to store and access the parsed configuration. >=20 > some performance data, timing `pveum user permissions $user > /dev/null= ` for > various amounts of ACL entries in user.cfg >=20 > entries | stock | patched | speedup > ------------------------------------- > 1k | 1.234s | 0.241s | 5.12 > 2k | 4.480s | 0.262s | 17.09 > 20k | 7m25s | 0.987s | 450.86 >=20 > similarly, an /access/ticket request such as the one happening on login= goes > down from 4.27s to 109ms with 2k entries (testing with 20k entries fail= s > because the request times out after 30s, but with the patch it takes 33= 6ms). >=20 > the underlying issue is that these two code paths not only iterate over= *all > defined ACL paths* to get a complete picture of a user's/token's privil= eges, > but the fact that that ACL computation for each checked path itself did= another > such loop in PVE::AccessControl::roles(). >=20 > it is enough to iterate over the to-be-checked ACL path in a component-= wise > fashion in order to handle role propagation, e.g., when looking at /a/b= /c/d, > iterate over >=20 > / > /a > /a/b > /a/b/c > /a/b/c/d >=20 > in turn instead of all defined ACL paths. >=20 > Signed-off-by: Fabian Gr=C3=BCnbichler > --- > we only use(d) $cfg->{acl} in pve-access-control, and the API doesn't e= xpose > the full parsed user.cfg anywhere, since we have separate endpoints for= each > type of entity stored within, so I don't think this counts as breaking = change. >=20 > could of course still be post-poned to 8.0 if desired. >=20 > src/PVE/API2/ACL.pm | 25 ++++--- > src/PVE/AccessControl.pm | 132 ++++++++++++++++++++++++++----------= > src/PVE/RPCEnvironment.pm | 14 ++-- > src/test/parser_writer.pl | 52 ++++++++++---- > src/test/realm_sync_test.pl | 54 ++++++--------- > 5 files changed, 179 insertions(+), 98 deletions(-) >=20 > applied, thanks!