[pve-devel] [PATCH access-control 1/2] fix #1148: allow up to three levels of pool nesting

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

with ACLs being inherited along the pool hierarchy.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    v1: encode max pool nesting in REs

 src/PVE/AccessControl.pm | 10 ++++++++--
 src/test/perm-test6.pl   | 16 ++++++++++++++++
 src/test/test6.cfg       |  5 +++++
 3 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/src/PVE/AccessControl.pm b/src/PVE/AccessControl.pm
index 9600e59..4e3b077 100644
--- a/src/PVE/AccessControl.pm
+++ b/src/PVE/AccessControl.pm
@@ -1264,7 +1264,7 @@ sub check_path {
 	|/nodes
 	|/nodes/[[:alnum:]\.\-\_]+
 	|/pool
-	|/pool/[[:alnum:]\.\-\_]+
+	|/pool/[A-Za-z0-9\.\-_]+(?:/[A-Za-z0-9\.\-_]+){0,2}
 	|/sdn
 	|/sdn/controllers
 	|/sdn/controllers/[[:alnum:]\_\-]+
@@ -1318,8 +1318,14 @@ PVE::JSONSchema::register_format('pve-poolid', \&verify_poolname);
 sub verify_poolname {
     my ($poolname, $noerr) = @_;
 
-    if ($poolname !~ m/^[A-Za-z0-9\.\-_]+$/) {
+    if (split("/", $poolname) > 3) {
+	die "pool name '$poolname' nested too deeply (max levels = 3)\n" if !$noerr;
 
+	return undef;
+    }
+
+    # also adapt check_path above if changed!
+    if ($poolname !~ m!^[A-Za-z0-9\.\-_]+(?:/[A-Za-z0-9\.\-_]+){0,2}$!) {
 	die "pool name '$poolname' contains invalid characters\n" if !$noerr;
 
 	return undef;
diff --git a/src/test/perm-test6.pl b/src/test/perm-test6.pl
index 0b0d036..c2d40fc 100755
--- a/src/test/perm-test6.pl
+++ b/src/test/perm-test6.pl
@@ -75,6 +75,22 @@ check_roles('User4@pve', '/vms/500', '');
 # with pool
 check_permissions('User4@pve', '/vms/500', '');
 
+# without pool, checking no access on parent pool
+check_roles('intern@pve', '/vms/600', '');
+# once more, with VM in nested pool
+check_roles('intern@pve', '/vms/700', '');
+# with propagated ACL
+check_roles('User4@pve', '/vms/700', '');
+# with pool, checking no access on parent pool
+check_permissions('intern@pve', '/vms/600', '');
+# once more, with VM in nested pool
+check_permissions('intern@pve', '/vms/700', 'VM.Audit');
+# with propagated ACL
+check_permissions('User4@pve', '/vms/700', 'VM.Console');
+
+# check nested pool permissions
+check_roles('intern@pve', '/pool/marketing/interns', 'RoleINTERN');
+check_roles('User4@pve', '/pool/marketing/interns', 'RoleMARKETING');
 
 check_permissions('User1@pve', '/vms/600', 'VM.Console');
 check_permissions('User2@pve', '/vms/600', 'VM.Console');
diff --git a/src/test/test6.cfg b/src/test/test6.cfg
index 4986910..661f56a 100644
--- a/src/test/test6.cfg
+++ b/src/test/test6.cfg
@@ -2,16 +2,20 @@ user:User1@pve:1:
 user:User2@pve:1:
 user:User3@pve:1:
 user:User4@pve:1:
+user:intern@pve:1:
 
 group:DEVEL:User1@pve,User2@pve,User3@pve:
 group:MARKETING:User1@pve,User4@pve:
+group:INTERNS:intern@pve:
 
 role:RoleDEVEL:VM.PowerMgmt:
 role:RoleMARKETING:VM.Console:
+role:RoleINTERN:VM.Audit:
 role:RoleTEST1:VM.Console:
 
 acl:1:/pool/devel:@DEVEL:RoleDEVEL:
 acl:1:/pool/marketing:@MARKETING:RoleMARKETING:
+acl:1:/pool/marketing/interns:@INTERNS:RoleINTERN:
 
 acl:1:/vms:@DEVEL:RoleTEST1:
 acl:1:/vms:User3@pve:NoAccess:
@@ -19,3 +23,4 @@ acl:1:/vms/300:@MARKETING:RoleTEST1:
 
 pool:devel:MITS development:500,501,502:store1 store2:
 pool:marketing:MITS marketing:600:store1:
+pool:marketing/interns:MITS marketing intern:700:store3:
-- 
2.39.2