Complicated Permissions: A Cautionary Tale?

29 views
Skip to first unread message

Doug Kelly

unread,
May 1, 2015, 11:48:36 AM5/1/15
to repo-d...@googlegroups.com
Not sure if I want this to be a cautionary tale, or perhaps solicit feedback for ways this could be better, but I'll try my best to do both.

The basic issue is, if you have a complicated permissions model, I've just recently found that it seems best to make the permissions as non-redundant as possible.  When scanning permissions, Gerrit has to enumerate the membership of groups you are (or are not) in, which can cause a significant amount of database access.  Here's the exact scenario:

* Most projects have Read on refs/* granted to Registered Users (from the All-Projects level).
* Some (rare) projects have Read Exclusive on refs/* set to deny Registered Users at an intermediate (inherited) level.
* Each project (whether inherited from All-Projects or the intermediate level) has an explicit Read on refs/* granted to a database group.

When running ls-projects, we found the time it took was much greater than before.  Using a combination of tcpdump (to confirm the database was being chatty) and JVisualVM (using sampling counters), we confirmed that a majority of the time was being spent in SQL calls, specifically determining group membership.  This basically came out of ProjectControl's canPerformOnAnyRef(), which first determines if the user matches the rule, but then uses the controlRefFor().canPerform() to ensure there's not a block or exclusive permission that would prevent them from having access.  I changed match() to simply return true instead of user.getEffectiveGroups().contains(uuid) as a test, and it turns out, it's the result of this call which is loading the membership of *every* group encountered.  This makes (some) sense: it has no way of determining if a subgroup of the group listed in the ACL is one of the groups the user is in.

Now, how can this possibly be improved?  The obvious would be "use less complicated ACLs" -- this would at least resolve the problem from a functional standpoint.  But, is there a possible performance gain to be had here, without compromising the integrity of the group / permissions system? I think there may be... Perhaps if rules are evaluated twice, rules can be checked first for an exact match (with no included group memberships), then checked for the "long" match, and finally evaluated to ensure there are no blocking rules.  The one thing I can think of in this case is that it may be optimizing in my particular instance, but it may be detrimental to other users.

Are there any other thoughts here?

Thanks!
Reply all
Reply to author
Forward
0 new messages