You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Dapeng Sun <da...@intel.com> on 2015/04/28 11:04:31 UTC
Review Request 33622: SENTRY-702:Hive binding should support RELOAD
command
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33622/
-----------------------------------------------------------
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
Re: Review Request 33622: SENTRY-702:Hive binding should support
RELOAD command
Posted by Prasad Mujumdar <pr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33622/#review81971
-----------------------------------------------------------
Ship it!
- Prasad Mujumdar
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
>
>
Re: Review Request 33622: SENTRY-702:Hive binding should support
RELOAD command
Posted by Dapeng Sun <da...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33622/
-----------------------------------------------------------
(Updated 四月 29, 2015, 4 p.m.)
Review request for sentry, Colin Ma and Prasad Mujumdar.
Changes
-------
Updated patch according Prasad's comments.
Bugs: SENTRY-702
https://issues.apache.org/jira/browse/SENTRY-702
Repository: sentry
Description
-------
Read whitelist from SENTRY configuration.
Diffs (updated)
-----
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
Re: Review Request 33622: SENTRY-702:Hive binding should support
RELOAD command
Posted by Dapeng Sun <da...@intel.com>.
> On 四月 29, 2015, 1:45 a.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.
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
- Dapeng
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33622/#review81844
-----------------------------------------------------------
On 四月 28, 2015, 5:04 p.m., Dapeng Sun wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33622/
> -----------------------------------------------------------
>
> (Updated 四月 28, 2015, 5:04 p.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
>
>
Re: Review Request 33622: SENTRY-702:Hive binding should support
RELOAD command
Posted by Prasad Mujumdar <pr...@cloudera.com>.
> 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
>
>
Re: Review Request 33622: SENTRY-702:Hive binding should support
RELOAD command
Posted by Prasad Mujumdar <pr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33622/#review81844
-----------------------------------------------------------
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.
- Prasad Mujumdar
On April 28, 2015, 9:04 a.m., Dapeng Sun wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33622/
> -----------------------------------------------------------
>
> (Updated April 28, 2015, 9:04 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
>
>