Re: [pve-devel] [PATCH-SERIE pve-access-control/pve-manager/qemu-server] check permissions on local bridge

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

On June 5, 2023 1:37 am, Alexandre Derumier wrote:
> add vnet/localbridge permissions management
> 
> Hi,
> as we has discuted some weeks ago,
> this patche serie introduce management of acl for vnets && local bridges
> 
> I have reuse current sdn permissions path, to have common paths
> 
> /sdn/vnets/<zone>/<vnet>
> 
> where the local vmbr are in a virtual "localnetwork" zone
> 
> /sdn/vnets/local/<vnet>
> 
> Vlans permissions  are also handled with
> /sdn/vnets/<zone>/<vnet>/<tag>

these paths don't match the patches ;)

if the paths were like this, then we could go one step further and
admins could set propagate on the zone to hand out access to the full
zone, including all vnets *and* vlan tags, and we could just check the
vnet (or vnet+tag), and the zone would be implicitly checked as well (by
virtue of traversing the ACL path).

we'd need to check for consistency of zone+vnet when checking ACLs
though, which is not required right now.

> 
> if user have permissions on the zone, he have access to all vnets/vlan
> if user have permissions on the vnet/tag, he have access to only the specific vlan.
> if user have permissions on the vnet, he have access to all vlans of the vnet

these last two I'd do differently.

permission on vnet/tag => permission to use that vlan
permission on vnet => permission to use the vnet/bridge (without tag)

if I want to give permission for all tags, I can simply give out the
role on vnet with propagation. since the permissions are only checked
when (re)configuring a guest, it doesn't matter that that check is a bit
expensive/potentially checking a lot of paths..

> 
> I have reworked the sdn zone panel from the tree, to manage permissions
> on displayed vnets.
> 
> some screenshots:
> 
> https://mutulin1.odiso.net/sdnzone-perm.png
> https://mutulin1.odiso.net/localzone-perm.png

I didn't check the GUI patches in detail yet, but IMHO they are also
less important right now (they are only a convenience feature for the
new feature of configuring VLAN access).

we'd like to get the basic patches in place this week if possible, if
that is too soon I can also fold in some of my suggestions as
follow-ups, just tell me what works for you!

> for proxmox7: (for users be able to add permissions before upgrade to pve8)
> pve-access-control: patch1 (to new /vnet/vlan path)
> pve-manager : patch1-2 for the new gui

the access control changes should be enough, it's always possible to set
the ACLs using the regular ACL GUI and/or `pveum`. it might make sense
to have at least the local bridge ACL path (for the zone, or for the
zone and the bridges?) in the regular ACL selectors in 7.x as well, if
we pull in something in pve-manager, than IMHO it should be that, not
the full-flegded new panels.

I do think we need a second pve-access-control patch though (for a new
SDN.Use privilege and corresponding role), that also needs to go into
7.x

> changelog v2:
>  - use /vnets/vlan instead /vnets.vlan
>  - rework the bridge filtering when user have access only to a specific vlan
>  - api2 network: always check bridge access if no filter is defined
> 
> todo:
>  - add permissions on clone/restore ?
> 
> 
> 
> pve-access-control:
> 
> Alexandre Derumier (2):
>   access control: add /sdn/vnets/<vnet>/<vlan> path
>   rpcenvironnment: add check_sdn_bridge
> 
>  src/PVE/AccessControl.pm  |  1 +
>  src/PVE/RPCEnvironment.pm | 17 +++++++++++++++++
>  2 files changed, 18 insertions(+)
> 
> 
> pve-manager:
> 
> Alexandre Derumier (3):
>   add vnet permissions panel
>   add permissions management for "localnetwork" zone
>   api2: network: check permissions for local bridges
> 
>  PVE/API2/Cluster.pm                  |  12 ++
>  PVE/API2/Network.pm                  |  26 ++-
>  www/manager6/Makefile                |   2 +
>  www/manager6/sdn/Browser.js          |  17 +-
>  www/manager6/sdn/VnetACLView.js      | 299 +++++++++++++++++++++++++++
>  www/manager6/sdn/ZoneContentPanel.js |  41 ++++
>  www/manager6/sdn/ZoneContentView.js  |  52 ++++-
>  7 files changed, 420 insertions(+), 29 deletions(-)
>  create mode 100644 www/manager6/sdn/VnetACLView.js
>  create mode 100644 www/manager6/sdn/ZoneContentPanel.js
> 
> qemu-server:
> 
> Alexandre Derumier (1):
>   api2: add check_bridge_access for create/update vm
> 
>  PVE/API2/Qemu.pm | 38 +++++++++++++++++++++++++++++++++++++-
>  1 file changed, 37 insertions(+), 1 deletion(-)
> 
> 
> -- 
> 2.30.2
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
>