> On April 28, 2015, 5:45 p.m., Prasad Mujumdar wrote: > > Patch overall looks fine. A couple of comments/suggestions - > > - Since we are allowing RELOAD, we should add hive.reloadable.aux.jars.path > > (ConfVars.HIVERELOADABLEJARS) to the restrict list. Otherwise this would > > become a loophole to load unauthorized jars in HiveServer2. > > - I think we should also allow 'ADD FILE[S]' and 'LIST FILE[S]' since the > > transform now enforces the URI privilege when the file is actually invoked > > in a query. > > Dapeng Sun wrote: > Thank you for your review, Prasad > - Since we are allowing RELOAD, we should add > hive.reloadable.aux.jars.path (ConfVars.HIVERELOADABLEJARS) to the restrict > list. Otherwise this would become a loophole to load unauthorized jars in > HiveServer2. > Good suggestion, I will update it in next patch. > - I think we should also allow 'ADD FILE[S]' and 'LIST FILE[S]' since the > transform now enforces the URI privilege when the file is actually invoked in > a query. > I try to add "ADD" to whitelist, but it failed, it seems only AuthzV2 > support it, if only V2 support it, could we file another ticket and fix it > the ticket after V2 finished in SENTRY? > > https://github.com/apache/hive/blob/0af6cb42725659740a022044c6cc464ef1cf4e6b/ql/src/java/org/apache/hadoop/hive/ql/processors/CommandUtil.java#L55
Sounds good. Thanks! - Prasad ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33622/#review81844 ----------------------------------------------------------- On April 29, 2015, 8 a.m., Dapeng Sun wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33622/ > ----------------------------------------------------------- > > (Updated April 29, 2015, 8 a.m.) > > > Review request for sentry, Colin Ma and Prasad Mujumdar. > > > Bugs: SENTRY-702 > https://issues.apache.org/jira/browse/SENTRY-702 > > > Repository: sentry > > > Description > ------- > > Read whitelist from SENTRY configuration. > > > Diffs > ----- > > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingSessionHook.java > 0fa4a87 > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/conf/HiveAuthzConf.java > 0a3b509 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestReloadPrivileges.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/33622/diff/ > > > Testing > ------- > > > Thanks, > > Dapeng Sun > >
