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 01D3F8DAB2 for ; Wed, 9 Nov 2022 13:53:28 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id D07371C3B3 for ; Wed, 9 Nov 2022 13:52:57 +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 ; Wed, 9 Nov 2022 13:52:56 +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 49A1C422E9 for ; Wed, 9 Nov 2022 13:52:56 +0100 (CET) Message-ID: <609dbc46-fe64-0c8f-24ce-b19903b21762@proxmox.com> Date: Wed, 9 Nov 2022 13:52:54 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:107.0) Gecko/20100101 Thunderbird/107.0 Content-Language: en-GB To: Proxmox VE development discussion , =?UTF-8?Q?Fabian_Gr=C3=BCnbichler?= References: <20220920125041.3636561-1-d.csapak@proxmox.com> <20220920125041.3636561-7-d.csapak@proxmox.com> <1667994471.zlbn23bpo0.astroid@yuna.none> From: Thomas Lamprecht In-Reply-To: <1667994471.zlbn23bpo0.astroid@yuna.none> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL -0.183 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 NICE_REPLY_A -0.001 Looks like a legit reply (A) 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 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [rpcenvironment.pm, accesscontrol.pm] Subject: Re: [pve-devel] [PATCH access-control v3 1/1] PVE/AccessControl: add Hardware.* privileges and /hardware/ paths 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: Wed, 09 Nov 2022 12:53:28 -0000 Am 09/11/2022 um 13:05 schrieb Fabian Gr=C3=BCnbichler: > On September 20, 2022 2:50 pm, Dominik Csapak wrote: >> so that we can assign privileges on hardware level >> >> this will generate a new role (PVEHardwareAdmin) >> >> Signed-off-by: Dominik Csapak >> --- >> src/PVE/AccessControl.pm | 13 +++++++++++++ >> src/PVE/RPCEnvironment.pm | 3 ++- >> 2 files changed, 15 insertions(+), 1 deletion(-) >> >> diff --git a/src/PVE/AccessControl.pm b/src/PVE/AccessControl.pm >> index c32dcc3..9cbc376 100644 >> --- a/src/PVE/AccessControl.pm >> +++ b/src/PVE/AccessControl.pm >> @@ -1080,6 +1080,17 @@ my $privgroups =3D { >> 'Pool.Audit', >> ], >> }, >> + Hardware =3D> { >> + root =3D> [ >> + 'Hardware.Configure', # create/edit mappings >> + ], >> + admin =3D> [ >> + 'Hardware.Use', >> + ], >> + audit =3D> [ >> + 'Hardware.Audit', >> + ], >> + }, >=20 > I guess the rationale here was that currently hardware is root only, so= having >=20 > admin =3D> Configure, > user =3D> Use, > audit =3D> Audit, >=20 > would mean the existing PVEAdmin roles would gain something that was pr= eviously > root only?=20 >=20 > note that the current patch still means that for the "Administrator" ro= le > anyway, since that gets *all* defined privileges.. (which might also be= > something worthy of calling out somewhere?) >=20 > it still might make sense to put Hardware.Use into the user category fo= r > consistency's sake? also not sure whether it would be worth it to re-th= ink > "Configure" (a bit more explicit) vs. "Modify" (consistent with existin= g > schema).. I'd rather keep it at .Modify IIUC and it will one allow to modify mappin= gs, IMO its worth to keep this aligned, and actually we could go for=20 Sys.HW.Modify Sys.HW.Use For what is Hardware.Audit though? (the commit message really needs to co= ntain more info...) is it for just seeing a HW mapping? as the underlying devi= ce details on a specific node are already handled by Sys.Audit. Because if its for HW mappings only I feel like it may not be required in= itially we should be able to cover all by Hardware.Modify for managing them and a= nd Hardware.Use, for being allowed to use a specific one, but just listing c= ould wait for some actual use case. Also, maybe call it HWMap.Modify and possibly make it a sub-group of a (n= ew) Cluster. priv group and /cluster/hw-map acl path. Cluster.HWMap.Use Cluster.HWMap.Modify and /cluster/hw-id/{id} IMO a bit more clear about what this actually covers, as a general /hardw= are one sounds a bit ominous IMO for our users, and we want to have Cluster.M= odify et al. anyway someday for making cluster non-root create/join/editable. (yes this isn't all that important and a bit close to bike shedding, but = we got to keep this pretty much forever (or have a PITA upgrade) so it's IMO wor= th to spent a bit more time picking colors of the shed here ;)