You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@bloodhound.apache.org by Anze Staric <an...@gmail.com> on 2013/04/03 11:32:03 UTC
TRAC_ADMIN in product environments
While working on search security, I have found a problem with
multiproduct permissions. If a user has TRAC_ADMIN privileges in
global environment, but no permissions in product environment (DELETE
FROM permission where product='p1'; ), he is unable to view tickets,
wikis, ... from product p1 (TICKET_VIEW privileges are required to
perform this operation on Ticket #1.).
A fix for this is fairly simple, but I have been wandering if this
behaviour is by design.
Anze
Patch that enables TRAC_ADMIN to inherit permissions in product environments:
Index: bloodhound_multiproduct/multiproduct/perm.py
===================================================================
--- bloodhound_multiproduct/multiproduct/perm.py (revision 1463489)
+++ bloodhound_multiproduct/multiproduct/perm.py (working copy)
@@ -40,13 +40,15 @@
from multiproduct.env import ProductEnvironment
if isinstance(self.env, ProductEnvironment):
+ permsys = PermissionSystem(self.env.parent)
+ is_trac_admin =
bool(permsys.check_permission('TRAC_ADMIN', username,
+ resource, perm))
if action == 'TRAC_ADMIN':
- # Always lookup TRAC_ADMIN permission in global scope
- permsys = PermissionSystem(self.env.parent)
- return bool(permsys.check_permission(action, username,
- resource, perm))
- elif username == self.env.product.owner:
- # Product owner granted with PRODUCT_ADMIN permission ootb
+ return is_trac_admin
+
+ elif username == self.env.product.owner or is_trac_admin:
+ # Product owner and trac admins granted with
+ # PRODUCT_ADMIN permission ootb
permsys = PermissionSystem(self.env)
# FIXME: would `action != 'TRAC_ADMIN'` be enough ?
return True if action in permsys.get_actions() and \
Re: TRAC_ADMIN in product environments
Posted by Anze Staric <an...@gmail.com>.
Patch provided in #438 fixes my problem, thanks.
Anze
On Thu, Apr 4, 2013 at 12:11 AM, Olemis Lang <ol...@gmail.com> wrote:
> On 4/3/13, Olemis Lang <ol...@gmail.com> wrote:
>> On 4/3/13, Anze Staric <an...@gmail.com> wrote:
>>>> + permsys = PermissionSystem(self.env.parent)
>>>> + is_trac_admin =
>>>> bool(permsys.check_permission('TRAC_ADMIN', username,
>>>> + resource,
>>>> perm))
>>>
>>>> Your patch implies a potential security threat in the sense that , if
>>>> `TRAC_ADMIN` somehow gets included in user's permission in product
>>>> scope then that user could also raise privileges level and get control
>>>> of the whole site by becoming TRAC_ADMIN in global scope as well
>>>
>>> I was under the impression that creating a PermissionSystem with
>>> parent env creates it in global scope (the first added line in the
>>> patch). If user had TRAC_ADMIN permission only in product scope,
>>> checking for permission TRAC_ADMIN in globla scope should return
>>> False.
>>>
>>
>> Checks in global and product scopes (via PermissionSystem) are
>> completely decoupled and unrelated . It's even possible to have two
>> different stores in global vs product .
>>
>> Please try https://issues.apache.org/bloodhound/ticket/438#comment:15
>> Test cases included and , if not (accurate | enough) please suggest
>> modifications (e.g. create another one on top of it)
>>
>
> To be more precise , what's wrong with the initial implementation
> (i.e. not your patch) is the check for ` action == 'TRAC_ADMIN' ` .
>
> PS: I reviewed your patch once again and understood what you were
> saying , but still your patch was not enough to fix that issue .
>
> --
> Regards,
>
> Olemis.
Re: TRAC_ADMIN in product environments
Posted by Olemis Lang <ol...@gmail.com>.
On 4/3/13, Olemis Lang <ol...@gmail.com> wrote:
> On 4/3/13, Anze Staric <an...@gmail.com> wrote:
>>> + permsys = PermissionSystem(self.env.parent)
>>> + is_trac_admin =
>>> bool(permsys.check_permission('TRAC_ADMIN', username,
>>> + resource,
>>> perm))
>>
>>> Your patch implies a potential security threat in the sense that , if
>>> `TRAC_ADMIN` somehow gets included in user's permission in product
>>> scope then that user could also raise privileges level and get control
>>> of the whole site by becoming TRAC_ADMIN in global scope as well
>>
>> I was under the impression that creating a PermissionSystem with
>> parent env creates it in global scope (the first added line in the
>> patch). If user had TRAC_ADMIN permission only in product scope,
>> checking for permission TRAC_ADMIN in globla scope should return
>> False.
>>
>
> Checks in global and product scopes (via PermissionSystem) are
> completely decoupled and unrelated . It's even possible to have two
> different stores in global vs product .
>
> Please try https://issues.apache.org/bloodhound/ticket/438#comment:15
> Test cases included and , if not (accurate | enough) please suggest
> modifications (e.g. create another one on top of it)
>
To be more precise , what's wrong with the initial implementation
(i.e. not your patch) is the check for ` action == 'TRAC_ADMIN' ` .
PS: I reviewed your patch once again and understood what you were
saying , but still your patch was not enough to fix that issue .
--
Regards,
Olemis.
Re: TRAC_ADMIN in product environments
Posted by Olemis Lang <ol...@gmail.com>.
On 4/3/13, Anze Staric <an...@gmail.com> wrote:
>> + permsys = PermissionSystem(self.env.parent)
>> + is_trac_admin =
>> bool(permsys.check_permission('TRAC_ADMIN', username,
>> + resource,
>> perm))
>
>> Your patch implies a potential security threat in the sense that , if
>> `TRAC_ADMIN` somehow gets included in user's permission in product
>> scope then that user could also raise privileges level and get control
>> of the whole site by becoming TRAC_ADMIN in global scope as well
>
> I was under the impression that creating a PermissionSystem with
> parent env creates it in global scope (the first added line in the
> patch). If user had TRAC_ADMIN permission only in product scope,
> checking for permission TRAC_ADMIN in globla scope should return
> False.
>
Checks in global and product scopes (via PermissionSystem) are
completely decoupled and unrelated . It's even possible to have two
different stores in global vs product .
Please try https://issues.apache.org/bloodhound/ticket/438#comment:15
Test cases included and , if not (accurate | enough) please suggest
modifications (e.g. create another one on top of it)
Thanks
--
Regards,
Olemis.
Re: TRAC_ADMIN in product environments
Posted by Anze Staric <an...@gmail.com>.
> + permsys = PermissionSystem(self.env.parent)
> + is_trac_admin =
> bool(permsys.check_permission('TRAC_ADMIN', username,
> + resource, perm))
> Your patch implies a potential security threat in the sense that , if
> `TRAC_ADMIN` somehow gets included in user's permission in product
> scope then that user could also raise privileges level and get control
> of the whole site by becoming TRAC_ADMIN in global scope as well
I was under the impression that creating a PermissionSystem with
parent env creates it in global scope (the first added line in the
patch). If user had TRAC_ADMIN permission only in product scope,
checking for permission TRAC_ADMIN in globla scope should return
False.
Anze
Re: TRAC_ADMIN in product environments
Posted by Olemis Lang <ol...@gmail.com>.
On 4/3/13, Anze Staric <an...@gmail.com> wrote:
> While working on search security, I have found a problem with
> multiproduct permissions. If a user has TRAC_ADMIN privileges in
> global environment, but no permissions in product environment (DELETE
> FROM permission where product='p1'; ), he is unable to view tickets,
> wikis, ... from product p1 (TICKET_VIEW privileges are required to
> perform this operation on Ticket #1.).
>
you are , that's an issue .
> A fix for this is fairly simple, but I have been wandering if this
> behaviour is by design.
>
yes it's by design .
>
> Anze
>
> Patch that enables TRAC_ADMIN to inherit permissions in product
> environments:
>
Your patch implies a potential security threat in the sense that , if
`TRAC_ADMIN` somehow gets included in user's permission in product
scope then that user could also raise privileges level and get control
of the whole site by becoming TRAC_ADMIN in global scope as well ,
also gaining access to admin panels blacklisted for product admins
(e.g. all those related to file paths server side) . That's terribly
bad. Therefore the reason for ignoring TRAC_ADMIN entries in product
perms store .
So a patch is actually needed but needs to be slightly different ...
what needs to be done is to check for global TRAC_ADMIN condition and
allow all actions *listed by product's PermissionSystem* to such users
...
Thanks for detecting this issue . I'll write a test case today for you
as a reference to get this tested . If a ticket is created I'll attach
a patch in there .
--
Regards,
Olemis.