From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id A8D1F1FF0E9 for ; Thu, 16 Jul 2026 01:22:21 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 5279021333; Thu, 16 Jul 2026 01:22:07 +0200 (CEST) From: Thomas Lamprecht To: pve-devel@lists.proxmox.com Subject: [RFC PATCH access-control 1/4] acl: support 'additive' option on ACL entries Date: Thu, 16 Jul 2026 01:19:59 +0200 Message-ID: <20260715232121.1009607-2-t.lamprecht@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260715232121.1009607-1-t.lamprecht@proxmox.com> References: <20260715232121.1009607-1-t.lamprecht@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1784157668365 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.240 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_LOW -0.7 Sender listed at https://www.dnswl.org/, low trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: PGPHTY5ND6KYQFWTIKFGO4ZCBLMLO5TN X-Message-ID-Hash: PGPHTY5ND6KYQFWTIKFGO4ZCBLMLO5TN X-MailFrom: t.lamprecht@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: By default a more specific ACL entry on a deeper path fully replaces the roles inherited from ancestor paths. That is what you want for scoped delegation, but not for broad grants that should hold across the whole tree, like an auditor group that must keep read access no matter what narrower ACLs exist below it. Add an optional 'additive' flag to ACL entries. An additive entry's roles are merged into those of deeper entries instead of being replaced by them, so the roles in effect at a path are the union of that path's own roles and any additive roles inherited from above. NoAccess on an additive entry still blocks everything below it. Store the flag per role, not just per member, so it maps one-to-one to a config line; otherwise a second role on the same member could be turned additive by accident when the config is written back out. Additive only makes sense together with propagate, so reject it without, and let the parser warn and drop the flag if it meets that combination in a hand-edited config. Signed-off-by: Thomas Lamprecht --- src/PVE/AccessControl.pm | 247 +++++++++++++++++++++++++++++--------- src/test/Makefile | 1 + src/test/parser_writer.pl | 149 +++++++++++++++++++++++ src/test/perm-test9.pl | 181 ++++++++++++++++++++++++++++ src/test/test9.cfg | 86 +++++++++++++ 5 files changed, 606 insertions(+), 58 deletions(-) create mode 100644 src/test/perm-test9.pl create mode 100644 src/test/test9.cfg diff --git a/src/PVE/AccessControl.pm b/src/PVE/AccessControl.pm index 0d632b3..01dbfd6 100644 --- a/src/PVE/AccessControl.pm +++ b/src/PVE/AccessControl.pm @@ -1000,6 +1000,22 @@ sub find_acl_tree_node { return $node; } +# Set or clear the per-role additive flag on an ACL node. Clearing prunes the sparse +# 'additive_{users,groups,tokens}' hashes so writers and the API never see a stale entry. +sub update_acl_additive { + my ($acl_node, $type, $id, $role, $additive) = @_; + + if ($additive) { + $acl_node->{$type}->{$id}->{$role} = 1; + return; + } + return if !$acl_node->{$type} || !$acl_node->{$type}->{$id}; + + delete($acl_node->{$type}->{$id}->{$role}); + delete($acl_node->{$type}->{$id}) if !%{ $acl_node->{$type}->{$id} }; + delete($acl_node->{$type}) if !%{ $acl_node->{$type} }; +} + sub add_user_group { my ($username, $usercfg, $group) = @_; @@ -1025,6 +1041,8 @@ sub delete_user_acl { delete($acl_node->{users}->{$username}) if $acl_node->{users}->{$username}; + delete($acl_node->{additive_users}->{$username}) + if $acl_node->{additive_users} && $acl_node->{additive_users}->{$username}; }; iterate_acl_tree("/", $usercfg->{acl_root}, $code); @@ -1038,6 +1056,8 @@ sub delete_group_acl { delete($acl_node->{groups}->{$group}) if $acl_node->{groups}->{$group}; + delete($acl_node->{additive_groups}->{$group}) + if $acl_node->{additive_groups} && $acl_node->{additive_groups}->{$group}; }; iterate_acl_tree("/", $usercfg->{acl_root}, $code); @@ -1509,10 +1529,31 @@ sub parse_user_config { } } elsif ($et eq 'acl') { - my ($propagate, $pathtxt, $uglist, $rolelist) = @data; + my ($propagate, $pathtxt, $uglist, $rolelist, $options) = @data; $propagate = $propagate ? 1 : 0; + # The on-disk option for 'additive' is '+', to keep ACL lines compact; the API and + # tooling use the long name 'additive'. + my $additive = 0; + if ($options) { + for my $opt (split_list($options)) { + if ($opt eq '+') { + $additive = 1; + } else { + warn "user config - ignore unknown acl option '$opt'\n"; + } + } + } + + # A non-propagating entry has nothing to merge down, so an 'additive' flag on it is + # meaningless: strip it with a warning. The API rejects the same combo up front. + if ($additive && !$propagate) { + warn "user config - 'additive' requires 'propagate' for acl entry" + . " at path '$pathtxt', ignoring flag\n"; + $additive = 0; + } + if (my $path = normalize_path($pathtxt)) { my $acl_node; foreach my $role (split_list($rolelist)) { @@ -1527,6 +1568,18 @@ sub parse_user_config { next; } + # Warn if a duplicate line for the same (member, role) at this path sets + # conflicting flags; the last line still wins, but the conflict is surfaced. + my $warn_conflict = sub { + my ($roles_hash, $add_hash, $label, $id) = @_; + return if !defined($roles_hash->{$id}->{$role}); + my $prev_prop = $roles_hash->{$id}->{$role}; + my $prev_add = $add_hash->{$id}->{$role} ? 1 : 0; + return if $prev_prop == $propagate && $prev_add == $additive; + warn "user config - duplicate acl line for $label '$id' role '$role'" + . " at '$pathtxt' with conflicting flags; last line wins\n"; + }; + foreach my $ug (split_list($uglist)) { my ($group) = $ug =~ m/^@(\S+)$/; @@ -1535,18 +1588,43 @@ sub parse_user_config { warn "user config - ignore invalid acl group '$group'\n"; } $acl_node = find_acl_tree_node($cfg->{acl_root}, $path) if !$acl_node; + $warn_conflict->( + $acl_node->{groups}, + $acl_node->{additive_groups} // {}, + 'group', + $group, + ); $acl_node->{groups}->{$group}->{$role} = $propagate; + update_acl_additive( + $acl_node, 'additive_groups', $group, $role, $additive, + ); } elsif (PVE::Auth::Plugin::verify_username($ug, 1)) { if (!$cfg->{users}->{$ug}) { # user does not exist warn "user config - ignore invalid acl member '$ug'\n"; } $acl_node = find_acl_tree_node($cfg->{acl_root}, $path) if !$acl_node; + $warn_conflict->( + $acl_node->{users}, + $acl_node->{additive_users} // {}, + 'user', + $ug, + ); $acl_node->{users}->{$ug}->{$role} = $propagate; + update_acl_additive($acl_node, 'additive_users', $ug, $role, $additive); } elsif (my ($user, $token) = split_tokenid($ug, 1)) { if (check_token_exist($cfg, $user, $token, 1)) { $acl_node = find_acl_tree_node($cfg->{acl_root}, $path) if !$acl_node; + $warn_conflict->( + $acl_node->{tokens}, + $acl_node->{additive_tokens} // {}, + 'token', + $ug, + ); $acl_node->{tokens}->{$ug}->{$role} = $propagate; + update_acl_additive( + $acl_node, 'additive_tokens', $ug, $role, $additive, + ); } else { warn "user config - ignore invalid acl token '$ug'\n"; } @@ -1707,26 +1785,32 @@ sub write_user_config { $data .= "\n"; + # Bucket a member's roles by (propagate, additive) so each flag combination emits as its own + # acl line; otherwise a non-additive role sharing a (member, path) would be silently promoted + # to additive on the next write. my $collect_rolelist_members = sub { - my ($acl_members, $result, $prefix, $exclude) = @_; + my ($acl_members, $additive_members, $result, $prefix, $exclude) = @_; foreach my $member (keys %$acl_members) { next if $exclude && $member eq $exclude; - my $l0 = ''; - my $l1 = ''; + my $member_additive = $additive_members->{$member} // {}; + + my $buckets = {}; foreach my $role (sort keys %{ $acl_members->{$member} }) { my $propagate = $acl_members->{$member}->{$role}; - if ($propagate) { - $l1 .= ',' if $l1; - $l1 .= $role; - } else { - $l0 .= ',' if $l0; - $l0 .= $role; + # The parser rejects 'additive' on a non-propagating entry, so strip it here too + # to keep the written config round-trip-stable even if in-memory state drifted. + my $additive = ($propagate && $member_additive->{$role}) ? 1 : 0; + $buckets->{$propagate}->{$additive}->{$role} = 1; + } + for my $propagate (keys %$buckets) { + for my $additive (keys %{ $buckets->{$propagate} }) { + my $rolelist = + join(',', sort keys %{ $buckets->{$propagate}->{$additive} }); + $result->{$propagate}->{$additive}->{$rolelist}->{"${prefix}${member}"} = 1; } } - $result->{0}->{$l0}->{"${prefix}${member}"} = 1 if $l0; - $result->{1}->{$l1}->{"${prefix}${member}"} = 1 if $l1; } }; @@ -1738,20 +1822,39 @@ sub write_user_config { my $rolelist_members = {}; - $collect_rolelist_members->($d->{'groups'}, $rolelist_members, '@'); + $collect_rolelist_members->( + $d->{'groups'}, + $d->{'additive_groups'} // {}, + $rolelist_members, + '@', + ); # no need to save 'root@pam', it is always 'Administrator' - $collect_rolelist_members->($d->{'users'}, $rolelist_members, '', 'root@pam'); + $collect_rolelist_members->( + $d->{'users'}, + $d->{'additive_users'} // {}, + $rolelist_members, + '', + 'root@pam', + ); - $collect_rolelist_members->($d->{'tokens'}, $rolelist_members, ''); + $collect_rolelist_members->( + $d->{'tokens'}, + $d->{'additive_tokens'} // {}, + $rolelist_members, + '', + ); foreach my $propagate (0, 1) { - my $filtered = $rolelist_members->{$propagate}; - foreach my $rolelist (sort keys %$filtered) { - my $uglist = join(',', sort keys %{ $filtered->{$rolelist} }); - $data .= "acl:$propagate:$path:$uglist:$rolelist:\n"; + foreach my $additive (0, 1) { + my $filtered = $rolelist_members->{$propagate}->{$additive}; + next if !$filtered; + foreach my $rolelist (sort keys %$filtered) { + my $uglist = join(',', sort keys %{ $filtered->{$rolelist} }); + my $opts = $additive ? '+:' : ''; + $data .= "acl:$propagate:$path:$uglist:$rolelist:$opts\n"; + } } - } }, ); @@ -1815,6 +1918,38 @@ sub roles { } my $roles = {}; + # Roles carried forward from additive entries along the walk. Earliest writer wins, so a + # closer additive entry does not mask a farther one. This is independent of tier precedence: + # a carried role may come from any tier, even one whose entry did not win its level. + my $carried = {}; + + # Harvest a tier's additive roles into $carried. They carry propagate=1 by parser invariant + # (additive requires propagate); test truthiness, not definedness, so any in-memory drift to + # propagate=0 is filtered out and cannot carry to deeper paths. + my $harvest_additive = sub { + my ($ri, $additive_map) = @_; + return if !$ri || !$additive_map; + for my $role (keys %$additive_map) { + next if defined($carried->{$role}); + next if !$ri->{$role}; + $carried->{$role} = $ri->{$role}; + } + }; + + # Collect a tier's roles that apply at this level, dropping non-propagating ones unless + # $final. $final is passed in, not closed over, because it is rebound each loop iteration. + my $collect_from_entry = sub { + my ($ri, $final) = @_; + + my $new; + for my $role (keys %$ri) { + my $propagate = $ri->{$role}; + next if !$final && !$propagate; + $new = {} if !$new; + $new->{$role} = $propagate; + } + return $new; + }; my $split = [split("/", $path)]; if ($path eq '/') { @@ -1822,7 +1957,6 @@ sub roles { } my $acl = $cfg->{acl_root}; - my $i = 0; while (@$split) { my $p = shift @$split; @@ -1831,66 +1965,63 @@ sub roles { $acl = $acl->{children}->{$p}; } - #print "CHECKACL $path $p\n"; - #print "ACL $path = " . Dumper ($acl); + # Harvest every tier's additive roles before the precedence check below picks $roles, so + # a same-level higher-tier override cannot erase the carry of a lower-tier additive entry. + $harvest_additive->($acl->{tokens}->{$user}, ($acl->{additive_tokens} // {})->{$user}); + $harvest_additive->($acl->{users}->{$user}, ($acl->{additive_users} // {})->{$user}); + for my $g (keys %{ $acl->{groups} }) { + next if !$cfg->{groups}->{$g}->{users}->{$user}; + $harvest_additive->($acl->{groups}->{$g}, ($acl->{additive_groups} // {})->{$g}); + } + + # Tier precedence (token > user > group): the first matching tier replaces $roles for this + # level, a deeper level may replace it again. The carry harvested above is unaffected. if (my $ri = $acl->{tokens}->{$user}) { - my $new; - foreach my $role (keys %$ri) { - my $propagate = $ri->{$role}; - if ($final || $propagate) { - #print "APPLY ROLE $p $user $role\n"; - $new = {} if !$new; - $new->{$role} = $propagate; - } - } + my $new = $collect_from_entry->($ri, $final); if ($new) { - $roles = $new; # overwrite previous settings + $roles = $new; next; } } if (my $ri = $acl->{users}->{$user}) { - my $new; - foreach my $role (keys %$ri) { - my $propagate = $ri->{$role}; - if ($final || $propagate) { - #print "APPLY ROLE $p $user $role\n"; - $new = {} if !$new; - $new->{$role} = $propagate; - } - } + my $new = $collect_from_entry->($ri, $final); if ($new) { - $roles = $new; # overwrite previous settings + $roles = $new; next; # user privs always override group privs } } my $new; - foreach my $g (keys %{ $acl->{groups} }) { + for my $g (keys %{ $acl->{groups} }) { next if !$cfg->{groups}->{$g}->{users}->{$user}; - if (my $ri = $acl->{groups}->{$g}) { - foreach my $role (keys %$ri) { - my $propagate = $ri->{$role}; - if ($final || $propagate) { - #print "APPLY ROLE $p \@$g $role\n"; - $new = {} if !$new; - $new->{$role} = $propagate; - } - } + my $ri = $acl->{groups}->{$g}; + next if !$ri; + my $g_new = $collect_from_entry->($ri, $final); + next if !$g_new; + $new = {} if !$new; + for my $role (keys %$g_new) { + $new->{$role} = $g_new->{$role}; } } if ($new) { - $roles = $new; # overwrite previous settings + $roles = $new; next; } } + # NoAccess from an additive ancestor blocks everything below, even a deeper non-additive grant. + if (defined($carried->{NoAccess})) { + return { 'NoAccess' => $carried->{NoAccess} }; + } + return { 'NoAccess' => $roles->{NoAccess} } if defined($roles->{NoAccess}); - #return () if defined ($roles->{NoAccess}); - #print "permission $user $path = " . Dumper ($roles); - - #print "roles $user $path = " . join (',', @ra) . "\n"; + # Merge carried roles in; on a name clash the held $roles win, as the deeper ACL is more + # specific. + for my $role (keys %$carried) { + $roles->{$role} = $carried->{$role} if !defined($roles->{$role}); + } return $roles; } diff --git a/src/test/Makefile b/src/test/Makefile index 53f2da7..c41ee1d 100644 --- a/src/test/Makefile +++ b/src/test/Makefile @@ -12,5 +12,6 @@ check: perl -I.. perm-test6.pl perl -I.. perm-test7.pl perl -I.. perm-test8.pl + perl -I.. perm-test9.pl perl -I.. realm_sync_test.pl perl -I.. api-tests.pl diff --git a/src/test/parser_writer.pl b/src/test/parser_writer.pl index ea2778e..451e786 100755 --- a/src/test/parser_writer.pl +++ b/src/test/parser_writer.pl @@ -377,6 +377,47 @@ my $default_cfg = { }, }, }, + acl_additive_user => { + 'path' => '/', + users => { + 'test@pam' => { + 'PVEVMAdmin' => 1, + }, + }, + additive_users => { + 'test@pam' => { + 'PVEVMAdmin' => 1, + }, + }, + }, + acl_additive_group => { + 'path' => '/storage', + groups => { + 'testgroup' => { + 'PVEDatastoreAdmin' => 1, + }, + }, + additive_groups => { + 'testgroup' => { + 'PVEDatastoreAdmin' => 1, + }, + }, + }, + acl_additive_user_mixed => { + # same member at same path with one additive and one non-additive role + 'path' => '/', + users => { + 'test@pam' => { + 'PVEAuditor' => 1, + 'PVEVMAdmin' => 1, + }, + }, + additive_users => { + 'test@pam' => { + 'PVEVMAdmin' => 1, + }, + }, + }, }; $default_cfg->{'acl_complex_mixed_root'} = { @@ -457,6 +498,11 @@ my $default_raw = { 'acl_complex_mixed_2' => 'acl:1:/storage:@testgroup,test@pam:PVEDatastoreAdmin:', 'acl_complex_mixed_3' => 'acl:1:/storage:@another,test2@pam:PVEDatastoreUser:', 'acl_missing_role' => 'acl:1:/storage:test@pam:MissingRole:', + 'acl_additive_user' => 'acl:1:/:test@pam:PVEVMAdmin:+:', + 'acl_additive_group' => 'acl:1:/storage:@testgroup:PVEDatastoreAdmin:+:', + # canonical output: one line per (propagate, additive) bucket, sorted + 'acl_additive_user_mixed_canonical' => 'acl:1:/:test@pam:PVEAuditor:' . "\n" + . 'acl:1:/:test@pam:PVEVMAdmin:+:', }, }; @@ -931,6 +977,109 @@ my $tests = [ . "\n\n\n\n\n" . $default_raw->{acl}->{'acl_simple_user'} . "\n", }, + { + name => "acl_additive_user", + config => { + users => default_users_with([$default_cfg->{test_pam}]), + roles => default_roles(), + acl_root => default_acls_with([$default_cfg->{acl_additive_user}]), + }, + raw => "" + . $default_raw->{users}->{'root@pam'} . "\n" + . $default_raw->{users}->{'test_pam'} + . "\n\n\n\n\n" + . $default_raw->{acl}->{'acl_additive_user'} . "\n", + }, + { + name => "acl_additive_group", + config => { + users => default_users_with([$default_cfg->{test_pam_with_group}]), + groups => default_groups_with([$default_cfg->{test_group_single_member}]), + roles => default_roles(), + acl_root => default_acls_with([$default_cfg->{acl_additive_group}]), + }, + raw => "" + . $default_raw->{users}->{'root@pam'} . "\n" + . $default_raw->{users}->{'test_pam'} . "\n\n" + . $default_raw->{groups}->{'test_group_single_member'} + . "\n\n\n\n" + . $default_raw->{acl}->{'acl_additive_group'} . "\n", + }, + { + # Regression: two roles on the same (member, path), one additive and one not, must + # round-trip as two separate lines. A per-member (rather than per-role) additive flag + # would merge them into one '+:' line, silently promoting the non-additive role. + name => "acl_additive_mixed_roles_roundtrip", + config => { + users => default_users_with([$default_cfg->{test_pam}]), + roles => default_roles(), + acl_root => default_acls_with([$default_cfg->{acl_additive_user_mixed}]), + }, + raw => "" + . $default_raw->{users}->{'root@pam'} . "\n" + . $default_raw->{users}->{'test_pam'} + . "\n\n\n\n\n" + . $default_raw->{acl}->{'acl_additive_user_mixed_canonical'} . "\n", + }, + { + # Duplicate (member, path, role) lines with conflicting flags: last line wins and the + # parser warns. Here the second line clears the additive flag that the first line set. + name => "acl_additive_conflict_warns", + raw => "" + . $default_raw->{users}->{'root@pam'} . "\n" + . $default_raw->{users}->{'test_pam'} + . "\n\n\n\n\n" + . 'acl:1:/:test@pam:PVEVMAdmin:+:' . "\n" + . 'acl:1:/:test@pam:PVEVMAdmin:' . "\n", + expected_config => { + users => default_users_with([$default_cfg->{test_pam}]), + roles => default_roles(), + acl_root => default_acls_with( + [{ + 'path' => '/', + users => { + 'test@pam' => { + 'PVEVMAdmin' => 1, + }, + }, + }], + ), + }, + expected_raw => "" + . $default_raw->{users}->{'root@pam'} . "\n" + . $default_raw->{users}->{'test_pam'} + . "\n\n\n\n\n" + . $default_raw->{acl}->{'acl_simple_user'} . "\n", + }, + { + # Reject 'additive' without 'propagate': the flag is dropped with a warning and only the + # propagate=0 entry survives. + name => "acl_additive_requires_propagate", + raw => "" + . $default_raw->{users}->{'root@pam'} . "\n" + . $default_raw->{users}->{'test_pam'} + . "\n\n\n\n\n" + . 'acl:0:/:test@pam:PVEVMAdmin:+:' . "\n", + expected_config => { + users => default_users_with([$default_cfg->{test_pam}]), + roles => default_roles(), + acl_root => default_acls_with( + [{ + 'path' => '/', + users => { + 'test@pam' => { + 'PVEVMAdmin' => 0, + }, + }, + }], + ), + }, + expected_raw => "" + . $default_raw->{users}->{'root@pam'} . "\n" + . $default_raw->{users}->{'test_pam'} + . "\n\n\n\n\n" + . 'acl:0:/:test@pam:PVEVMAdmin:' . "\n", + }, { name => "acl_complex_mixed", config => { diff --git a/src/test/perm-test9.pl b/src/test/perm-test9.pl new file mode 100644 index 0000000..a63b719 --- /dev/null +++ b/src/test/perm-test9.pl @@ -0,0 +1,181 @@ +#!/usr/bin/perl + +use v5.36; + +use Test::More; + +use PVE::AccessControl; +use PVE::RPCEnvironment; + +my $rpcenv = PVE::RPCEnvironment->init('cli'); +$rpcenv->init_request(userconfig => 'test9.cfg'); + +sub check_roles($user, $path, $expected, $desc) { + my $roles = PVE::AccessControl::roles($rpcenv->{user_cfg}, $user, $path); + is(join(',', sort keys %$roles), $expected, "$desc - roles($user, $path)"); +} + +sub check_permissions($user, $path, $expected, $desc) { + my $perm = $rpcenv->permissions($user, $path); + is(join(',', sort keys %$perm), $expected, "$desc - perms($user, $path)"); +} + +# Each group is a Test::More subtest; each case lists a user plus expected role/perm sets per path. +# The case desc plus user/path is emitted with every assertion to self-locate failures. +my @groups = ( + { + name => 'additive carry across a deeper replace', + cases => [ + { + desc => 'User2 (ADMINS+OPS): RoleADMIN carried, merged with deeper RoleOPS', + user => 'User2@pve', + roles => { '/pool/top/sub' => 'RoleADMIN,RoleOPS' }, + # exercises pool->vm projection and role->priv expansion + perms => { '/vms/200' => 'VM.Audit,VM.Console,VM.PowerMgmt' }, + }, + { + desc => 'User3 (OPS only): additive at top inert when not in @ADMINS', + user => 'User3@pve', + roles => { '/pool/top/sub' => 'RoleOPS' }, + }, + ], + }, + { + name => 'multi-level walk preserves carry across replaces', + cases => [ + { + desc => 'User2: carry survives replace at sub, propagates through deep', + user => 'User2@pve', + roles => { '/pool/top/sub/deep' => 'RoleADMIN,RoleOPS' }, + }, + { + desc => 'User4 (only deep ACL, no group): additive at unrelated ancestor inert', + user => 'User4@pve', + roles => { '/pool/top/sub/deep' => 'RoleCHILD' }, + }, + ], + }, + { + name => 'additive carry survives a non-propagating deeper entry', + cases => [ + { + desc => 'User2: carried RoleADMIN merges with a deeper propagate=0 RoleCHILD', + user => 'User2@pve', + roles => { '/pool/nonprop/mid' => 'RoleADMIN,RoleCHILD' }, + }, + { + desc => 'User2: propagate=0 RoleCHILD does not descend, carried RoleADMIN does', + user => 'User2@pve', + roles => { '/pool/nonprop/mid/leaf' => 'RoleADMIN' }, + }, + ], + }, + { + name => 'baseline (no additive flag) keeps replace semantics', + cases => [ + { + desc => 'User2: deeper @OPS replaces RoleADMIN, no carry without flag', + user => 'User2@pve', + roles => { '/pool/baseline/child' => 'RoleOPS' }, + perms => { '/vms/500' => 'VM.Audit' }, + }, + ], + }, + { + name => 'NoAccess on additive entry blocks deeper grants', + cases => [ + { + desc => 'User1: additive NoAccess at parent overrides child RoleADMIN', + user => 'User1@pve', + roles => { '/pool/denied/child' => 'NoAccess' }, + perms => { '/vms/700' => '' }, + }, + ], + }, + { + name => 'deeper non-additive NoAccess overrides a carried additive grant', + cases => [ + { + desc => 'User1: child NoAccess wins over the carried additive RoleADMIN', + user => 'User1@pve', + roles => { '/pool/denygrant/child' => 'NoAccess' }, + perms => { '/vms/931' => '' }, + }, + ], + }, + { + name => 'multi-group at same path: only the additive group carries', + cases => [ + { + desc => 'MixUser (GI additive + GN non-additive): same-level union still works', + user => 'MixUser@pve', + roles => { '/pool/mixgroups' => 'RoleGI,RoleGN' }, + }, + { + desc => 'User2 (ADMINS non-additive + OPS additive): RoleADMIN must not leak', + user => 'User2@pve', + roles => { '/pool/multigrp/child' => 'RoleCHILD,RoleOPS' }, + }, + ], + }, + { + name => 'additive is independent of same-level tier precedence', + cases => [ + { + desc => 'User2 (group-tier additive + user-tier non-additive at same path)', + user => 'User2@pve', + roles => { + '/pool/tieroverlap' => 'RoleADMIN,RoleCHILD', + '/pool/tieroverlap/child' => 'RoleADMIN,RoleCHILD', + }, + }, + ], + }, + { + name => 'privsep tokens do not inherit user-tier additive carry', + cases => [ + { + desc => 'TokenUser user view: RoleADMIN carried past child @OPS replace', + user => 'TokenUser@pve', + roles => { '/pool/tokenpool/child' => 'RoleADMIN,RoleOPS' }, + }, + { + desc => 'privsep token: own ACL only, no carry from user-tier additive', + user => 'TokenUser@pve!privsep', + roles => { '/pool/tokenpool/child' => 'RoleOPS' }, + perms => { '/vms/901' => 'VM.Audit' }, + }, + { + desc => 'full token: short-circuits to user roles incl. carried RoleADMIN', + user => 'TokenUser@pve!full', + roles => { '/pool/tokenpool/child' => 'RoleADMIN,RoleOPS' }, + perms => { '/vms/901' => 'VM.Audit,VM.Console,VM.PowerMgmt' }, + }, + { + desc => 'privsep token: its own additive entry carries past a deeper replace', + user => 'TokenUser@pve!privsep', + roles => { '/pool/tokadd/child' => 'RoleADMIN,RoleOPS' }, + }, + ], + }, +); + +for my $group (@groups) { + subtest $group->{name} => sub { + for my $case ($group->{cases}->@*) { + my $desc = $case->{desc}; + if (my $r = $case->{roles}) { + for my $path (sort keys %$r) { + check_roles($case->{user}, $path, $r->{$path}, $desc); + } + } + if (my $p = $case->{perms}) { + for my $path (sort keys %$p) { + check_permissions($case->{user}, $path, $p->{$path}, $desc); + } + } + } + }; +} + +done_testing(); diff --git a/src/test/test9.cfg b/src/test/test9.cfg new file mode 100644 index 0000000..c7cce08 --- /dev/null +++ b/src/test/test9.cfg @@ -0,0 +1,86 @@ +user:User1@pve:1: +user:User2@pve:1: +user:User3@pve:1: +user:User4@pve:1: +user:MixUser@pve:1: +user:TokenUser@pve:1: + +token:TokenUser@pve!privsep:0:1:: +token:TokenUser@pve!full:0:0:: + +group:ADMINS:User1@pve,User2@pve: +group:OPS:User2@pve,User3@pve: +group:GI:MixUser@pve: +group:GN:MixUser@pve: + +role:RoleADMIN:VM.PowerMgmt,VM.Console: +role:RoleOPS:VM.Audit: +role:RoleCHILD:VM.Migrate: +role:RoleGI:VM.Snapshot: +role:RoleGN:VM.Backup: + +pool:top:top pool:100:: +pool:top/sub::200,201:store1: +pool:top/sub/deep::300:: + +pool:baseline::400:: +pool:baseline/child::500:store2: + +pool:denied:denied pool:600:: +pool:denied/child::700:: + +pool:mixgroups::800:: +pool:mixgroups/child::801:: + +pool:multigrp::850:: +pool:multigrp/child::851:: + +pool:tieroverlap::870:: +pool:tieroverlap/child::871:: + +pool:tokenpool::900:: +pool:tokenpool/child::901:: + +pool:nonprop::920:: +pool:nonprop/mid::921:: +pool:nonprop/mid/leaf::922:: + +pool:denygrant::930:: +pool:denygrant/child::931:: + +pool:tokadd::940:: +pool:tokadd/child::941:: + +acl:1:/pool/top:@ADMINS:RoleADMIN:+: +acl:1:/pool/top/sub:@OPS:RoleOPS: +acl:1:/pool/top/sub/deep:User4@pve:RoleCHILD: + +acl:1:/pool/baseline:@ADMINS:RoleADMIN: +acl:1:/pool/baseline/child:@OPS:RoleOPS: + +acl:1:/pool/denied:User1@pve:NoAccess:+: +acl:1:/pool/denied/child:User1@pve:RoleADMIN: + +acl:1:/pool/mixgroups:@GI:RoleGI:+: +acl:1:/pool/mixgroups:@GN:RoleGN: +acl:1:/pool/mixgroups/child:@OPS:RoleOPS: + +acl:1:/pool/multigrp:@ADMINS:RoleADMIN: +acl:1:/pool/multigrp:@OPS:RoleOPS:+: +acl:1:/pool/multigrp/child:User2@pve:RoleCHILD: + +acl:1:/pool/tieroverlap:@ADMINS:RoleADMIN:+: +acl:1:/pool/tieroverlap:User2@pve:RoleCHILD: + +acl:1:/pool/tokenpool:TokenUser@pve:RoleADMIN:+: +acl:1:/pool/tokenpool/child:TokenUser@pve:RoleOPS: +acl:1:/pool/tokenpool/child:TokenUser@pve!privsep:RoleOPS: + +acl:1:/pool/nonprop:@ADMINS:RoleADMIN:+: +acl:0:/pool/nonprop/mid:User2@pve:RoleCHILD: + +acl:1:/pool/denygrant:User1@pve:RoleADMIN:+: +acl:1:/pool/denygrant/child:User1@pve:NoAccess: + +acl:1:/pool/tokadd:TokenUser@pve!privsep:RoleADMIN:+: +acl:1:/pool/tokadd/child:TokenUser@pve!privsep:RoleOPS: -- 2.47.3