[pve-devel] [PATCH access-control] api: ACL update: fix handling of Permissions.Modify

[pve-devel] [PATCH access-control] api: ACL update: fix handling of Permissions.Modify

[Fabian Grünbichler]

with 8.x, the scope of non-"Permissions.Modify"-based ACL update privileges
were reduced (so that users with for example, VM.Allocate on a VM could only
delegate their own privileges, but not arbitrary other ones). that additional
logic had a wrong guard and was accidentally triggered for calls where the user
had the "Permissions.Modify" privilege on the modified ACL path, but without
propagation set.

a user with "Permissions.Modify" on a path should be able to set arbitrary
ACLs for that path, even without propagation.

reported on the forum:

https://forum.proxmox.com/threads/privilege-permissions-modify-on-pool-will-not-propagade-to-contained-vms-anymore.151032/

Fixes: 46bfd59dfca655b263d1f905be37d985416717ac ("acls: restrict less-privileged ACL modifications")

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 src/PVE/API2/ACL.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/PVE/API2/ACL.pm b/src/PVE/API2/ACL.pm
index 93adb78..2a4d4ff 100644
--- a/src/PVE/API2/ACL.pm
+++ b/src/PVE/API2/ACL.pm
@@ -166,7 +166,8 @@ __PACKAGE__->register_method ({
 		    die "role '$role' does not exist\n"
 			if !$cfg->{roles}->{$role};
 
-		    if (!$auth_user_privs->{'Permissions.Modify'}) {
+		    # permissions() returns set privs as key, and propagate bit as value!
+		    if (!defined($auth_user_privs->{'Permissions.Modify'})) {
 			# 'perm-modify' allows /vms/* with VM.Allocate and similar restricted use cases
 			# filter those to only allow handing out a subset of currently active privs
 		        my $role_privs = $cfg->{roles}->{$role};
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] applied: [PATCH access-control] api: ACL update: fix handling of Permissions.Modify

[Thomas Lamprecht]

Am 11/07/2024 um 13:44 schrieb Fabian Grünbichler:
> with 8.x, the scope of non-"Permissions.Modify"-based ACL update privileges
> were reduced (so that users with for example, VM.Allocate on a VM could only
> delegate their own privileges, but not arbitrary other ones). that additional
> logic had a wrong guard and was accidentally triggered for calls where the user
> had the "Permissions.Modify" privilege on the modified ACL path, but without
> propagation set.
> 
> a user with "Permissions.Modify" on a path should be able to set arbitrary
> ACLs for that path, even without propagation.
> 
> reported on the forum:
> 
> https://forum.proxmox.com/threads/privilege-permissions-modify-on-pool-will-not-propagade-to-contained-vms-anymore.151032/

Could be:

Reported on the forum: https://forum.proxmox.com/threads/151032/

> 
> Fixes: 46bfd59dfca655b263d1f905be37d985416717ac ("acls: restrict less-privileged ACL modifications")
> 

please no extra newlines between trailers like Fixes or your S-o-b.

> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
>  src/PVE/API2/ACL.pm | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
>

applied, with above commit message nits addressed and reflowed to <= 70 cc,
thanks!


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel