Hi, I've done some further analysis of the problem, and I think it is not directly related to SENTRY-1291. The problem manifests in CommonPrivilege.implies(privilege, model). My (cached) privilege looks like:
Server=server1->Db=authz->Table=words->Column=*->action=select The "privilege" I want to check looks like: Server=server1->Db=authz->Table=words->action=select; The problem is in the "for" loop in CommonPrivilege.implies. It loops on the parts of the second privilege, and matches up to "action=select". Here it tries to compare to "Column=*" of the cached privilege and fails on this line: https://github.com/apache/sentry/blob/a4924edc79b26f937e3e5ea3584f0b4307dd4135/sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/CommonPrivilege.java#L86 It's clear there's a bug here somewhere, but I'm not sure where - can someone please advise? Thanks, Colm. On Wed, Dec 13, 2017 at 8:28 PM, Na Li <lina...@cloudera.com> wrote: > Sasha, > > sentry-1291 is helpful for the problem that sentry privilege checks takes > too long with many explicit grants, which is useful for big customers. > Another approach that can improve the performance is to organize the > privileges according to the authorization hierarchy in a tree structure, so > finding match in ResourceAuthorizationProvider.doHasAccess() is in the > order of log(N), not linear of N, where N is the number of privileges. > > We can wait for Colm to confirm his issue is caused by sentry-1291. If so, > it may be fixed by selecting privileges by finding if the requesting > authorization object is prefix of cached privileges instead of exact match. > > in SimplePrivilegeCache > > public Set<String> listPrivileges(Set<String> groups, Set<String> users, > ActiveRoleSet roleSet, > Authorizable... authorizationHierarchy) { > Set<String> privileges = new HashSet<>(); > Set<StringBuilder> authzKeys = getAuthzKeys(authorizationHierarchy); > for (StringBuilder authzKey : authzKeys) { > if (cachedAuthzPrivileges.get(authzKey.toString()) != null) { > <- > instead of exact matching, add extension function to check if > authzKey.toString is the prefix of the key of the entries > in cachedAuthzPrivileges. > privileges.addAll(cachedAuthzPrivileges.get(authzKey.toString())); > } > } > > return privileges; > } > > Thanks, > > Lina > > On Wed, Dec 13, 2017 at 1:08 PM, Alexander Kolbasov <ak...@cloudera.com> > wrote: > > > I think that SENTRY-1291 should be just reverted - there are multiple > > issues with it and no one is actually using the fix. Anyone wants to do > it? > > > > - Alex > > > > On Wed, Dec 13, 2017 at 4:44 AM, Na Li <lina...@cloudera.com> wrote: > > > > > Colm, > > > > > > Glad you find the cause! > > > > > > You can revert Sentry-1291, and see if it works. If so, it is issue at > > > finding cached privileges. > > > > > > Cheers, > > > > > > Lina > > > > > > Sent from my iPhone > > > > > > > On Dec 13, 2017, at 4:58 AM, Colm O hEigeartaigh < > cohei...@apache.org> > > > wrote: > > > > > > > > Hi, > > > > > > > > I can see what the problem is (that the authorization hierarchy does > > not > > > > contain the column, and hence doesn't match against the cached > > > privilege), > > > > but I'm not sure about the best way to solve it. Either the way we > are > > > > creating the authorization hierarchy is incorrect (e.g. in > > > > HiveAuthzBindingHookBase) or else the way we are parsing the cached > > > > privilege is incorrect (e.g. in SimplePrivilegeCache/ > CommonPrivilege). > > > > > > > > Colm. > > > > > > > >> On Wed, Dec 13, 2017 at 5:57 AM, Na Li <lina...@cloudera.com> > wrote: > > > >> > > > >> Colm, > > > >> > > > >> I did not get chance to look into this issue today. Sorry about > that. > > > >> > > > >> You can add a e2e test case and set break point at where the > > > authorization > > > >> object hierarchy to a list of authorization objects, which is used > to > > do > > > >> exact match with cache > > > >> > > > >> Sent from my iPhone > > > >> > > > >>> On Dec 12, 2017, at 11:27 AM, Colm O hEigeartaigh < > > cohei...@apache.org > > > > > > > >> wrote: > > > >>> > > > >>> That would be great, thanks! > > > >>> > > > >>> Colm. > > > >>> > > > >>>> On Tue, Dec 12, 2017 at 4:36 PM, Na Li <lina...@cloudera.com> > > wrote: > > > >>>> > > > >>>> Colm, > > > >>>> > > > >>>> I suspect it is a bug in SENTRY-1291. I can take a look later > today. > > > >>>> > > > >>>> Thanks, > > > >>>> > > > >>>> Lina > > > >>>> > > > >>>> On Tue, Dec 12, 2017 at 4:32 AM, Colm O hEigeartaigh < > > > >> cohei...@apache.org> > > > >>>> wrote: > > > >>>> > > > >>>>> Hi all, > > > >>>>> > > > >>>>> I've updated some local testcases to work with Sentry 2.0.0 and > the > > > >> "v1" > > > >>>>> Hive binding (previously working fine using 1.8.0 and the "v2" > > > >> binding). > > > >>>>> > > > >>>>> I have a simple table called "words" (word STRING, count INT). I > am > > > >>>> making > > > >>>>> an SQL call as the user "bob", e.g. "SELECT * FROM words where > > count > > > == > > > >>>>> '100'". > > > >>>>> > > > >>>>> "bob" is in the "manager" group", which has the following role: > > > >>>>> > > > >>>>> select_all_role = > > > >>>>> Server=server1->Db=authz->Table=words->Column=*->action=select > > > >>>>> > > > >>>>> Essentially, authorization is denied even though the policy is > > > correct. > > > >>>> If > > > >>>>> I look at the SimplePrivilegeCache, the cached privilege is: > > > >>>>> > > > >>>>> server=server1->db=authz->table=words->column=*=[Server= > > > >>>>> server1->Db=authz->Table=words->Column=*->action=select] > > > >>>>> > > > >>>>> However, when "listPrivileges" is called, the authorizable > > hierarchy > > > >>>> looks > > > >>>>> like: > > > >>>>> > > > >>>>> Server [name=server1] > > > >>>>> Database [name=authz] > > > >>>>> Table [name=words] > > > >>>>> > > > >>>>> There is no "column" here, and a match is not made against the > > cached > > > >>>>> privilege as a result. Is this a bug or am I missing some > > > configuration > > > >>>>> switch? > > > >>>>> > > > >>>>> Colm. > > > >>>>> > > > >>>>> > > > >>>>> -- > > > >>>>> Colm O hEigeartaigh > > > >>>>> > > > >>>>> Talend Community Coder > > > >>>>> http://coders.talend.com > > > >>>>> > > > >>>> > > > >>> > > > >>> > > > >>> > > > >>> -- > > > >>> Colm O hEigeartaigh > > > >>> > > > >>> Talend Community Coder > > > >>> http://coders.talend.com > > > >> > > > > > > > > > > > > > > > > -- > > > > Colm O hEigeartaigh > > > > > > > > Talend Community Coder > > > > http://coders.talend.com > > > > > > -- Colm O hEigeartaigh Talend Community Coder http://coders.talend.com
