[pve-devel] [RFC access-control] API/ticket: rework coarse grained permission computation

[Thomas Lamprecht]

We accessed methods from PVE::Storage here but did not define a
"use PVE::Storage". This thus only worked if modules if the
PVE::Storage module got pulled in by something else, by luck.
Simply including said use statement is not an option because
pve-storage is already dependent from pve-access-control, and we want
to avoid cyclic dependencies, especially on the perl module level.

The reason the offending module was used in the first place here
stems from the way how this coarse grained permissions are
calculated.
We check all permission object paths for privileges for an user.
So we got all vmids and all storage ids and computed paths from them.
This works, but is overkill and led to this "illegal" module use.

Instead I opt to not generating all possible paths, but just check
the ones configured plus a small required static set of top level
paths - this allows to generalize handling of the special root at pam
and "normal" users.

It has to be noted that this method is in general just intended for a
coarse capability check to allow hiding a few UI elements which are
not generated by backend calls (which are already permission aware).
The real checks get done by each backend call, automatically for
simple ones and semi-automatically for complex ones.
---

It could make sense to move this out from API to AccessControl, make
it static and add tests for it?
Because of the missing use its a bit harder to do so previous to
applying this patch, sadly...

 PVE/API2/AccessControl.pm | 99 +++++++++++++++++------------------------------
 1 file changed, 36 insertions(+), 63 deletions(-)

diff --git a/PVE/API2/AccessControl.pm b/PVE/API2/AccessControl.pm
index 5d59d9f..318ee15 100644
--- a/PVE/API2/AccessControl.pm
+++ b/PVE/API2/AccessControl.pm
@@ -136,73 +136,46 @@ my $compute_api_permission = sub {
 
     my $usercfg = $rpcenv->{user_cfg};
 
-    my $nodelist = PVE::Cluster::get_nodelist();
-    my $vmlist = PVE::Cluster::get_vmlist() || {};
-    my $idlist = $vmlist->{ids} || {};
-
-    my $cfg = PVE::Storage::config();
-    my @sids =  PVE::Storage::storage_ids ($cfg);
-
-    my $res = {
-	vms => {},
-	storage => {},
-	access => {},
-	nodes => {},
-	dc => {},
+    my $res = {};
+    my $priv_re_map = {
+	vms => qr/VM\.|Permissions\.Modify/,
+	access => qr/(User|Group)\.|Permissions\.Modify/,
+	storage => qr/Datastore\./,
+	nodes => qr/Sys\.|Permissions\.Modify/,
+	dc => qr/Sys\.Audit/,
     };
-
-    my $extract_vm_caps = sub {
-	my ($path) = @_;
-	
-	my $perm = $rpcenv->permissions($authuser, $path);
-	foreach my $priv (keys %$perm) {
-	    next if !($priv eq 'Permissions.Modify' || $priv =~ m/^VM\./);
-	    $res->{vms}->{$priv} = 1;	
-	}
-    };
-
-    foreach my $pool (keys %{$usercfg->{pools}}) {
-	&$extract_vm_caps("/pool/$pool");
-    }
-
-    foreach my $vmid (keys %$idlist, '__phantom__') {
-	&$extract_vm_caps("/vms/$vmid");
-    }
-
-    foreach my $storeid (@sids, '__phantom__') {
-	my $perm = $rpcenv->permissions($authuser, "/storage/$storeid");
-	foreach my $priv (keys %$perm) {
-	    next if !($priv eq 'Permissions.Modify' || $priv =~ m/^Datastore\./);
-	    $res->{storage}->{$priv} = 1;
-	}
-    }
-
-    foreach my $path (('/access/groups')) {
-	my $perm = $rpcenv->permissions($authuser, $path);
-	foreach my $priv (keys %$perm) {
-	    next if $priv !~ m/^(User|Group)\./;
-	    $res->{access}->{$priv} = 1;
-	}
-    }
-
-    foreach my $group (keys %{$usercfg->{users}->{$authuser}->{groups}}, '__phantom__') {
-	my $perm = $rpcenv->permissions($authuser, "/access/groups/$group");
-	if ($perm->{'User.Modify'}) {
-	    $res->{access}->{'User.Modify'} = 1;
-	}
-    }
-
-    foreach my $node (@$nodelist) {
-	my $perm = $rpcenv->permissions($authuser, "/nodes/$node");
-	foreach my $priv (keys %$perm) {
-	    next if $priv !~ m/^Sys\./;
-	    $res->{nodes}->{$priv} = 1;
+    map { $res->{$_} = {} } keys %$priv_re_map;
+
+    my $required_paths = ['/', '/nodes', '/access/groups', '/vms', '/storage'];
+
+    my $checked_paths = {};
+    foreach my $path (@$required_paths, keys %{$usercfg->{acl}}) {
+	next if $checked_paths->{$path};
+	$checked_paths->{$path} = 1;
+
+	my $path_perm = $rpcenv->permissions($authuser, $path);
+
+	my $toplevel = ($path =~ /^\/(\w+)/) ? $1 : 'dc';
+	if ($toplevel eq 'pool') {
+	    foreach my $priv (keys %$path_perm) {
+		if ($priv =~ m/^VM\./) {
+		    $res->{vms}->{$priv} = 1;
+		} elsif ($priv =~ m/^Datastore\./) {
+		    $res->{storage}->{$priv} = 1;
+		} elsif ($priv eq 'Permissions.Modify') {
+		    $res->{storage}->{$priv} = 1;
+		    $res->{vms}->{$priv} = 1;
+		}
+	    }
+	} else {
+	    my $priv_regex = $priv_re_map->{$toplevel} // next;
+	    foreach my $priv (keys %$path_perm) {
+		next if $priv !~ m/^($priv_regex)/;
+		$res->{$toplevel}->{$priv} = 1;
+	    }
 	}
     }
 
-    my $perm = $rpcenv->permissions($authuser, "/");
-    $res->{dc}->{'Sys.Audit'} = 1 if $perm->{'Sys.Audit'};
-
     return $res;
 };
 
-- 
2.11.0