[pve-devel] [PATCH manager] api2: network: anybridge: re-add regular bridges

[DERUMIER, Alexandre]

> given that
> - we can't require some new ACL path/priv for regular bridges until
> the 
>   next major release (as that would be quite the breaking change ;))
> - removing access to the last VNET would suddenly make all regular 
>   bridges available (again) with your original patch, which is
> strange 
>   behaviour for an ACL (removing a positive ACL shouldn't give me
> more 
>   access ;))
> - with the original patch it's impossible to say "user FOO can play 
>   around with SDN but also use regular bridges" without 'faking' ACLs
>   for vnets that aren't actually vnets, but regular bridges
> 

yes, I known it's a big strange/ugly.

I have tried to find a way to hide vmbrX because multiples users have
requested it.
https://forum.proxmox.com/threads/sdn-group-pool-permissions.93872/

But it's still beta, so, we can wait for next major release to have a
clean implementation.

>I think the current variant is what we can have for the time being -
> and 
> then with 8.0 either say
> - 'regular bridges also require FOO on /sdn/vnets/vmbrX' (identical
> to 
>   vnets, but a bit ugly as the ACL path is technically 'wrong')
> - 'regular bridges require FOO on /network/bridge/vmbrX' (like vnets,
>   but different ACL path/namespace, probably with some helper that
> 'does 
>   the right thing' in places where we don't care what kind it is)
> and then filter here and in other places accordingly?
> 

Yes, that's seem fine.
I could be great to have a true acl management to vmbrX :)


Le mercredi 27 avril 2022 à 14:45 +0200, Fabian Grünbichler a écrit :
> On April 27, 2022 2:32 pm, DERUMIER, Alexandre wrote:
> > Hi Fabian
> > Le mercredi 27 avril 2022 à 13:36 +0200, Fabian Grünbichler a
> > écrit :
> > > commit 052fbb2a4d1bdeb490b2e3b67cd7555e460ebe93 introduced
> > > permission
> > > > checks here that caused all regular bridges to be removed from
> > > > the
> > > > returned list as soon as the SDN package is installed, unless
> > > > the
> > > > user
> > > > is root at pam or there exists a VNET with the same ID.
> > > > 
> > mmm, this is unexpected.
> > with my original patch :
> > 
> > if user don't have any permissions on vnets:
> >   - all vmbrX bridges are displayed
> >   - no vnets is displayed
> > 
> > if user have a permission on at least 1vnet:
> >   - vmbrX bridge are no more displayed  (untie you add a specific
> > permissions with /sdn/vnets/vmbrX)
> >   - only vnet with permissions are displayed
> > 
> > 
> > I didn' see, but Thomas have reworked it:
> > https://antiphishing.cetsi.fr/proxy/v3?i=SHV0Y1JZQjNyckJFa3dUQiblhF5YcUqtiWCaK_ri0kk&r=T0hnMlUyVEgwNmlmdHc1NQqeTQ1pLQVNn4UPLJn2W6e9Hh50epHxcxJAGCrIHvKB1souhZXB265bSkydEfNuQg&f=V3p0eFlQOUZ4czh2enpJS729B9MwEV8JxHeeNPHyzEMH7fBkB3EUDz0PUKSsrg4P&u=https%3A//git.proxmox.com/%3Fp%3Dpve-manager.git%3Ba%3Dcommit%3Bh%3D640c0b26891c408d0456c355b3724c1be18cc75f&k=ZVd0
> > 
> > and the behaviour seem to be different:
> > 
> > if user don't have any permissions on vnets:
> >   - no vmbrX displayed   ----> different behaviour
> >   - no vnets is displayed
> > 
> > if user have a permission on at least 1vnet:
> >   - vmbrX bridge are no more displayed  (untile you add a specific
> > permissions with /sdn/vnets/vmbrX)
> >   - only vnet with permissions are displayed
> > 
> > 
> > with your patch, it seem to be different too:
> > 
> > if user don't have any permissions on vnets:
> >   - vmbrX displayed  
> >   - no vnets is displayed
> > 
> > if user have a permission on at least 1vnet:
> >   - vmbrX brige are still displayed ----> different behaviour
> >   - only vnet with permissions are displayed
> 
> yeah that's true. I missed which commit was the culprit (relying on
> the 
> commit messages)
> 
> given that
> - we can't require some new ACL path/priv for regular bridges until
> the 
>   next major release (as that would be quite the breaking change ;))
> - removing access to the last VNET would suddenly make all regular 
>   bridges available (again) with your original patch, which is
> strange 
>   behaviour for an ACL (removing a positive ACL shouldn't give me
> more 
>   access ;))
> - with the original patch it's impossible to say "user FOO can play 
>   around with SDN but also use regular bridges" without 'faking' ACLs
>   for vnets that aren't actually vnets, but regular bridges
> 
> I think the current variant is what we can have for the time being -
> and 
> then with 8.0 either say
> - 'regular bridges also require FOO on /sdn/vnets/vmbrX' (identical
> to 
>   vnets, but a bit ugly as the ACL path is technically 'wrong')
> - 'regular bridges require FOO on /network/bridge/vmbrX' (like vnets,
>   but different ACL path/namespace, probably with some helper that
> 'does 
>   the right thing' in places where we don't care what kind it is)
> and then filter here and in other places accordingly?
>