You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Na Li via Review Board <no...@reviews.apache.org> on 2018/06/11 22:45:26 UTC
Review Request 67539: SENTRY-2264: It is possible to elevate
privileges from DROP using alter table rename
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67539/
-----------------------------------------------------------
Review request for sentry, kalyan kumar kalvagadda and Sergio Pena.
Bugs: sentry-2264
https://issues.apache.org/jira/browse/sentry-2264
Repository: sentry
Description
-------
change privilege for table rename
Diffs
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 4f932ea
Diff: https://reviews.apache.org/r/67539/diff/1/
Testing
-------
Thanks,
Na Li
Re: Review Request 67539: SENTRY-2264: It is possible to elevate
privileges from DROP using alter table rename
Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67539/#review205368
-----------------------------------------------------------
Ship it!
Ship It!
- Sergio Pena
On June 26, 2018, 4:36 a.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67539/
> -----------------------------------------------------------
>
> (Updated June 26, 2018, 4:36 a.m.)
>
>
> Review request for sentry, kalyan kumar kalvagadda and Sergio Pena.
>
>
> Bugs: sentry-2264
> https://issues.apache.org/jira/browse/sentry-2264
>
>
> Repository: sentry
>
>
> Description
> -------
>
> change privilege for table rename
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 4f932ea
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperationsPart1.java 1e72990
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperationsPart2.java cf89b5d
>
>
> Diff: https://reviews.apache.org/r/67539/diff/3/
>
>
> Testing
> -------
>
> tests in TestDbOperationsPart1 and TestDbOperationsPart2
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 67539: SENTRY-2264: It is possible to elevate
privileges from DROP using alter table rename
Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67539/
-----------------------------------------------------------
(Updated June 26, 2018, 4:36 a.m.)
Review request for sentry, kalyan kumar kalvagadda and Sergio Pena.
Bugs: sentry-2264
https://issues.apache.org/jira/browse/sentry-2264
Repository: sentry
Description
-------
change privilege for table rename
Diffs
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 4f932ea
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperationsPart1.java 1e72990
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperationsPart2.java cf89b5d
Diff: https://reviews.apache.org/r/67539/diff/3/
Testing (updated)
-------
tests in TestDbOperationsPart1 and TestDbOperationsPart2
Thanks,
Na Li
Re: Review Request 67539: SENTRY-2264: It is possible to elevate
privileges from DROP using alter table rename
Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67539/
-----------------------------------------------------------
(Updated June 26, 2018, 4:22 a.m.)
Review request for sentry, kalyan kumar kalvagadda and Sergio Pena.
Bugs: sentry-2264
https://issues.apache.org/jira/browse/sentry-2264
Repository: sentry
Description
-------
change privilege for table rename
Diffs (updated)
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 4f932ea
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperationsPart1.java 1e72990
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperationsPart2.java cf89b5d
Diff: https://reviews.apache.org/r/67539/diff/3/
Changes: https://reviews.apache.org/r/67539/diff/2-3/
Testing
-------
Thanks,
Na Li
Re: Review Request 67539: SENTRY-2264: It is possible to elevate
privileges from DROP using alter table rename
Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67539/#review204713
-----------------------------------------------------------
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java
Lines 108-113 (original), 108-114 (patched)
<https://reviews.apache.org/r/67539/#comment287348>
The ALTER privilege is also required in the source table as it is the action the user is doing ALTER TABLE.
We don't have a DELETE privilege yet, so should we treat this case as the user requires ALL privileges in the source table instead?
Why is the ALTER privilege required on the destination?
Is the INSERT on the database needed? This means the user won't be able to move tables between databases they have CREATE privileges. The CREATE comes with OWNER privileges, so the user will end up having ALL privileges in the table anyway. Which brings an interesting question, if I have ALL privileges (but not ownership) and I move the table, then I will transfer the ownership to me. We need to check if HMS generates only an ALTER operation in this cases of if it generates DROP and CREATE events which will complicate things.
If ownership is disabled, then If the user has ALL privileges in the source table, then when moving the table those privileges will be moved so the user will have ALL privileges in the destination table.
- Sergio Pena
On June 12, 2018, 8:16 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67539/
> -----------------------------------------------------------
>
> (Updated June 12, 2018, 8:16 p.m.)
>
>
> Review request for sentry, kalyan kumar kalvagadda and Sergio Pena.
>
>
> Bugs: sentry-2264
> https://issues.apache.org/jira/browse/sentry-2264
>
>
> Repository: sentry
>
>
> Description
> -------
>
> change privilege for table rename
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 4f932ea
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperationsPart1.java 1e72990
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperationsPart2.java cf89b5d
>
>
> Diff: https://reviews.apache.org/r/67539/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 67539: SENTRY-2264: It is possible to elevate
privileges from DROP using alter table rename
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67539/#review205315
-----------------------------------------------------------
Sentry should be able to differenciate if the just table rename or moving table from database to another. Privileges that sentry expects shoud differ based on this. If a table is moved from database to another, the user should have "ALL" permissions on that table to avoid elevation of his privilege.
- kalyan kumar kalvagadda
On June 12, 2018, 8:16 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67539/
> -----------------------------------------------------------
>
> (Updated June 12, 2018, 8:16 p.m.)
>
>
> Review request for sentry, kalyan kumar kalvagadda and Sergio Pena.
>
>
> Bugs: sentry-2264
> https://issues.apache.org/jira/browse/sentry-2264
>
>
> Repository: sentry
>
>
> Description
> -------
>
> change privilege for table rename
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 4f932ea
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperationsPart1.java 1e72990
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperationsPart2.java cf89b5d
>
>
> Diff: https://reviews.apache.org/r/67539/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 67539: SENTRY-2264: It is possible to elevate
privileges from DROP using alter table rename
Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67539/
-----------------------------------------------------------
(Updated June 12, 2018, 8:16 p.m.)
Review request for sentry, kalyan kumar kalvagadda and Sergio Pena.
Bugs: sentry-2264
https://issues.apache.org/jira/browse/sentry-2264
Repository: sentry
Description
-------
change privilege for table rename
Diffs (updated)
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 4f932ea
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperationsPart1.java 1e72990
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperationsPart2.java cf89b5d
Diff: https://reviews.apache.org/r/67539/diff/2/
Changes: https://reviews.apache.org/r/67539/diff/1-2/
Testing
-------
Thanks,
Na Li
Re: Review Request 67539: SENTRY-2264: It is possible to elevate
privileges from DROP using alter table rename
Posted by Na Li via Review Board <no...@reviews.apache.org>.
> On June 12, 2018, 4:27 p.m., kalyan kumar kalvagadda wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java
> > Line 110 (original), 110-111 (patched)
> > <https://reviews.apache.org/r/67539/diff/1/?file=2039246#file2039246line110>
> >
> > Can you explain why this change is needed?
> >
> > having drop on the old database and create on new database good enough?
The problem we had was user does not have privilege to select the data, but can do so with "alter table rename" as what's mentioned in jira description.
The change requires more privileges from user who executes this command to mimic the minimum privilege for someone to export the data, drop the table in original DB, and create table, add data to the new table in destination DB. After introducing FGP, a user with only DROP on a database db1 and at least CREATE on db2 can run
==============================
ALTER TABLE RENAME db1.table1 db2.table2, and thus elevate their privileges. that is why drop on old DB and create on new DB is not enough.
To reproduce:
As admin (e.g. hive):
1. Create db1, db1.table1, db2, role r1.
2. Grant DROP on db1 to role r1.
3. Grant ALL on db2 to role r1
4. Grant role r1 to user testuser1.
As testuser1:
1. use db1; alter table db1.table1 rename to db2.table1
2. select * from db2. table1
Result: the select command succeeds.
- Na
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67539/#review204605
-----------------------------------------------------------
On June 11, 2018, 10:45 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67539/
> -----------------------------------------------------------
>
> (Updated June 11, 2018, 10:45 p.m.)
>
>
> Review request for sentry, kalyan kumar kalvagadda and Sergio Pena.
>
>
> Bugs: sentry-2264
> https://issues.apache.org/jira/browse/sentry-2264
>
>
> Repository: sentry
>
>
> Description
> -------
>
> change privilege for table rename
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 4f932ea
>
>
> Diff: https://reviews.apache.org/r/67539/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 67539: SENTRY-2264: It is possible to elevate
privileges from DROP using alter table rename
Posted by Na Li via Review Board <no...@reviews.apache.org>.
> On June 12, 2018, 4:27 p.m., kalyan kumar kalvagadda wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java
> > Line 110 (original), 110-111 (patched)
> > <https://reviews.apache.org/r/67539/diff/1/?file=2039246#file2039246line110>
> >
> > Can you explain why this change is needed?
> >
> > having drop on the old database and create on new database good enough?
>
> Na Li wrote:
> The problem we had was user does not have privilege to select the data, but can do so with "alter table rename" as what's mentioned in jira description.
>
> The change requires more privileges from user who executes this command to mimic the minimum privilege for someone to export the data, drop the table in original DB, and create table, add data to the new table in destination DB. After introducing FGP, a user with only DROP on a database db1 and at least CREATE on db2 can run
> ==============================
> ALTER TABLE RENAME db1.table1 db2.table2, and thus elevate their privileges. that is why drop on old DB and create on new DB is not enough.
>
> To reproduce:
>
> As admin (e.g. hive):
> 1. Create db1, db1.table1, db2, role r1.
> 2. Grant DROP on db1 to role r1.
> 3. Grant ALL on db2 to role r1
> 4. Grant role r1 to user testuser1.
> As testuser1:
> 1. use db1; alter table db1.table1 rename to db2.table1
> 2. select * from db2. table1
> Result: the select command succeeds.
to avoid user elevate table privlege using destiantion DB, we should require user having all privilege on source table. That is why we have this fix
- Na
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67539/#review204605
-----------------------------------------------------------
On June 26, 2018, 4:22 a.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67539/
> -----------------------------------------------------------
>
> (Updated June 26, 2018, 4:22 a.m.)
>
>
> Review request for sentry, kalyan kumar kalvagadda and Sergio Pena.
>
>
> Bugs: sentry-2264
> https://issues.apache.org/jira/browse/sentry-2264
>
>
> Repository: sentry
>
>
> Description
> -------
>
> change privilege for table rename
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 4f932ea
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperationsPart1.java 1e72990
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperationsPart2.java cf89b5d
>
>
> Diff: https://reviews.apache.org/r/67539/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 67539: SENTRY-2264: It is possible to elevate
privileges from DROP using alter table rename
Posted by Na Li via Review Board <no...@reviews.apache.org>.
- Na
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67539/#review204605
-----------------------------------------------------------
On June 26, 2018, 4:36 a.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67539/
> -----------------------------------------------------------
>
> (Updated June 26, 2018, 4:36 a.m.)
>
>
> Review request for sentry, kalyan kumar kalvagadda and Sergio Pena.
>
>
> Bugs: sentry-2264
> https://issues.apache.org/jira/browse/sentry-2264
>
>
> Repository: sentry
>
>
> Description
> -------
>
> change privilege for table rename
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 4f932ea
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperationsPart1.java 1e72990
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperationsPart2.java cf89b5d
>
>
> Diff: https://reviews.apache.org/r/67539/diff/3/
>
>
> Testing
> -------
>
> tests in TestDbOperationsPart1 and TestDbOperationsPart2
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 67539: SENTRY-2264: It is possible to elevate
privileges from DROP using alter table rename
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67539/#review204605
-----------------------------------------------------------
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java
Line 110 (original), 110-111 (patched)
<https://reviews.apache.org/r/67539/#comment287187>
Can you explain why this change is needed?
having drop on the old database and create on new database good enough?
- kalyan kumar kalvagadda
On June 11, 2018, 10:45 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67539/
> -----------------------------------------------------------
>
> (Updated June 11, 2018, 10:45 p.m.)
>
>
> Review request for sentry, kalyan kumar kalvagadda and Sergio Pena.
>
>
> Bugs: sentry-2264
> https://issues.apache.org/jira/browse/sentry-2264
>
>
> Repository: sentry
>
>
> Description
> -------
>
> change privilege for table rename
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 4f932ea
>
>
> Diff: https://reviews.apache.org/r/67539/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Na Li
>
>