Re: [pve-devel] [PATCH pve-manager 2/2] api2 : network: anybridge: don't display bridges if user have access to vnets.

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

On 07/08/2021 18:00, alexandre derumier wrote:
> Hi,
> abou this patch, I'm not sure it's the right way, a forum user request
> that also sdnadmin can view vmbrX.
> 
> I don't known how to hide correctly vmbrX bridge, as currently , we
> don't have any permissions management,
> and I don't want to break current users setup.
> 
> maybe could we add a special permission like "noaccess" with path like
> /bridge/vmbrX  ?
> (we currently have a role "noaccess", but it's simply a role without
> any permission.
> 

We do not have "revoking privileges", only empty roles so adding that would
change the whole ACL system quite a bit and IMO make it a bit more confusing that
it already can be.

Currently we handle giving more permissions in general but less permissions
on a specific ACL object by just always using the more restrictive ACL
configurations for specific paths.
I.e., if one configures Admin on / with propagate and an empty role (like
NoAccess) on /node/foo then they have no Admin privs on the foo node.

I.e., you do not need to invent a negative priv. simply setting a empty or less
privileged role on a more specific path is enough.

So rather the IMO a bit more integrated way of achieving that would be adding
a `/nodes/<node-id>/iface/<iface-id>` ACL path or the like. I mean, if we'd
be sure it would be only always a bridge then we could avoid making that path
node specific, as for them we assume that the one with the same ID are network
wise "the same" already on different nodes in a cluster.

I mean, we could also just see bridges as a vnet and handle them with the
`/sdn/vnets/<id>` path? Checking for every bridge/vnet if some priv. is
available, we probably would need two privileges, one for "allowed to use"
and a "allowed to modify" priv.

(disclaimer, fresh back from vacation and did not really looked into this in
detail, so just holler if something is stupid or not fitting the existing
permission stuff of SDN :))

> 
> Le jeudi 05 août 2021 à 16:59 +0200, Alexandre Derumier a écrit :
>> This remove vmbr* from bridgeselector if user have access to vnets.
>> (as currently, we don't have have permission management on vmbr$)
>>
>> Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
>> ---
>>  PVE/API2/Network.pm | 19 ++++++++++++-------
>>  1 file changed, 12 insertions(+), 7 deletions(-)
>>
>> diff --git a/PVE/API2/Network.pm b/PVE/API2/Network.pm
>> index a26f36d2..02bd3bdb 100644
>> --- a/PVE/API2/Network.pm
>> +++ b/PVE/API2/Network.pm
>> @@ -226,6 +226,7 @@ __PACKAGE__->register_method({
>>         my ($param) = @_;
>>  
>>         my $rpcenv = PVE::RPCEnvironment::get();
>> +       my $authuser = $rpcenv->get_user();
>>  
>>         my $tmp = PVE::INotify::read_file('interfaces', 1);
>>         my $config = $tmp->{data};
>> @@ -238,20 +239,24 @@ __PACKAGE__->register_method({
>>         delete $ifaces->{lo}; # do not list the loopback device
>>  
>>         if ($param->{type}) {
>> +           my $vnets = {};
>> +           my $filtered_sdn = undef;
>> +           if ($have_sdn && $param->{type} eq 'any_bridge') {
>> +               $vnets = PVE::Network::SDN::get_local_vnets();
>> +               $filtered_sdn = 1 if $authuser ne 'root@pam' && keys
>> %{$vnets} > 0;
>> +           }
>> +
>>             foreach my $k (keys %$ifaces) {
>>                 my $type = $ifaces->{$k}->{type};
>>                 my $match =  ($param->{type} eq $type) || (
>>                     ($param->{type} eq 'any_bridge') && 
>>                     ($type eq 'bridge' || $type eq 'OVSBridge'));
>> -               delete $ifaces->{$k} if !$match;
>> +               delete $ifaces->{$k} if !$match || $filtered_sdn;
>>             }
>>  
>> -           if ($have_sdn && $param->{type} eq 'any_bridge') {
>> -               my $vnets = PVE::Network::SDN::get_local_vnets();
>> -               map {
>> -                   $ifaces->{$_} = $vnets->{$_};
>> -               } keys %$vnets;
>> -           }
>> +           map {
>> +               $ifaces->{$_} = $vnets->{$_};
>> +           } keys %$vnets;
>>         }
>>  
>>         return PVE::RESTHandler::hash_to_array($ifaces, 'iface');
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>