You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@ranger.apache.org by Yan Zhou <yz...@yahoo.com> on 2016/10/27 19:16:33 UTC

Review Request 53240: Ranger-1181: HDFS Plugin does not allow removal of a non-empty directory if the directory is allowed to be removed by HDFS, but the file inside the directory is allowed to be removed by Ranger

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53240/
-----------------------------------------------------------

Review request for ranger.

Repository: ranger

Description
-------

Reproduction Steps:
1. Ranger is installed and HDFS plug-in is enabled.
2. As qaadmin user, create a folder on HDFS with permission 500:
hadoop fs -mkdir /tmp/rangertest1
hadoop fs -chmod 500 /tmp/rangertest1
while the /tmp itself has the 777 as the HDFS bits:
hadoop fs -ls /
drwxrwxrwx - user1 group1 0 2016-10-03 14:54 /tmp
3. Create a Ranger policy p1_1 by granting qaadmin with RWX permission to the folder of /tmp/rangertest1, recursive set to true
4. Wait for around >30 seconds after Policy synced up.
5. Put a file to /tmp/rangertest1 folder:
echo "This is a file2" > /tmp/temp
hadoop fs -put /tmp/temp /tmp/rangertest1
hadoop fs -ls /tmp/rangertest1
Found 1 items
rw-rr- 3 qaadmin hdfs 16 2016-09-21 19:13 /tmp/rangertest1/temp
6. Try to delete the non-empty folder with "-skipTrash" option, but it failed (delete the empty folder could success):
hadoop fs -rm -r -skipTrash /tmp/rangertest1
rm: Permission denied: user=qaadmin, access=ALL, inode="/tmp/rangertest1":qaadmin:hdfs:dr-x------

This happens when the HDFS fallback is enabled. The issue is with the synergy between the Ranger HDFS plugin and the HDFS. The removal of the directory of /tmp/rangertest1 is not covered by a Ranger policy but is allowed by of the HDFS permission for its parent, /tmp. The removal of the file inside /tmp/rangertest1 is, however, not allowed by HDFS for the 500 permission of its parent of /tmp/rangertest1, but allowed by Ranger.
When Ranger finds that the directory of rangertest1 can't be removed by Ranger, it falls back to HDFS policy which does not allow the removal of the file inside rangertest1 even though allowed by Ranger.
We should use HDFS fallback when the checks on ancestor/parent/current/subdir fails before proceed to the next check but with only the failed access check by the HDFS fallback not all of the access checks for ancestor, parent, current and subdir.

Diffs
-----

hdfs-agent/src/main/java/org/apache/ranger/authorization/hadoop/RangerHdfsAuthorizer.java 6f452da

Diff: https://reviews.apache.org/r/53240/diff/

Testing
-------

Unit test works ok.

Thanks,

Yan Zhou

Re: Review Request 53240: Ranger-1181: HDFS Plugin does not allow removal of a non-empty directory if the directory is allowed to be removed by HDFS, but the file inside the directory is allowed to be removed by Ranger

Posted by Madhan Neethiraj <ma...@apache.org>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53240/#review155404
-----------------------------------------------------------



Yan Zhou - the changes look good. Thanks for the fix. Please go through the comments and update patch.


hdfs-agent/src/main/java/org/apache/ranger/authorization/hadoop/RangerHdfsAuthorizer.java (line 254)
<https://reviews.apache.org/r/53240/#comment225316>

    how about calling checkDefaultEnforcer() here, when "authzStatus == NOT_DETERMINED" for ancestor - similar to other cases like parent/access/subAccess?



hdfs-agent/src/main/java/org/apache/ranger/authorization/hadoop/RangerHdfsAuthorizer.java (line 262)
<https://reviews.apache.org/r/53240/#comment225317>

    since checkDefaultEnforcer() is called only when authzStatus is NOT_DETERMINED, it is unnecessary to pass authzStatus to the method. Consider removing this parameter.


- Madhan Neethiraj


On Oct. 27, 2016, 7:16 p.m., Yan Zhou wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53240/
> -----------------------------------------------------------
> 
> (Updated Oct. 27, 2016, 7:16 p.m.)
> 
> 
> Review request for ranger.
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> Reproduction Steps:
> 1. Ranger is installed and HDFS plug-in is enabled.
> 2. As qaadmin user, create a folder on HDFS with permission 500:
> hadoop fs -mkdir /tmp/rangertest1
> hadoop fs -chmod 500 /tmp/rangertest1
> while the /tmp itself has the 777 as the HDFS bits:
> hadoop fs -ls /
> drwxrwxrwx - user1 group1 0 2016-10-03 14:54 /tmp
> 3. Create a Ranger policy p1_1 by granting qaadmin with RWX permission to the folder of /tmp/rangertest1, recursive set to true 
> 4. Wait for around >30 seconds after Policy synced up.
> 5. Put a file to /tmp/rangertest1 folder:
> echo "This is a file2" > /tmp/temp
> hadoop fs -put /tmp/temp /tmp/rangertest1
> hadoop fs -ls /tmp/rangertest1
> Found 1 items
> rw-rr- 3 qaadmin hdfs 16 2016-09-21 19:13 /tmp/rangertest1/temp
> 6. Try to delete the non-empty folder with "-skipTrash" option, but it failed (delete the empty folder could success): 
> hadoop fs -rm -r -skipTrash /tmp/rangertest1
> rm: Permission denied: user=qaadmin, access=ALL, inode="/tmp/rangertest1":qaadmin:hdfs:dr-x------
> 
> 
> This happens when the HDFS fallback is enabled. The issue is with the synergy between the Ranger HDFS plugin and the HDFS. The removal of the directory of /tmp/rangertest1 is not covered by a Ranger policy but is allowed by of the HDFS permission for its parent, /tmp. The removal of the file inside /tmp/rangertest1 is, however, not allowed by HDFS for the 500 permission of its parent of /tmp/rangertest1, but allowed by Ranger.
> When Ranger finds that the directory of rangertest1 can't be removed by Ranger, it falls back to HDFS policy which does not allow the removal of the file inside rangertest1 even though allowed by Ranger.
> We should use HDFS fallback when the checks on ancestor/parent/current/subdir fails before proceed to the next check but with only the failed access check by the HDFS fallback not all of the access checks for ancestor, parent, current and subdir.
> 
> 
> Diffs
> -----
> 
>   hdfs-agent/src/main/java/org/apache/ranger/authorization/hadoop/RangerHdfsAuthorizer.java 6f452da 
> 
> Diff: https://reviews.apache.org/r/53240/diff/
> 
> 
> Testing
> -------
> 
> Unit test works ok.
> 
> 
> Thanks,
> 
> Yan Zhou
> 
>

Re: Review Request 53240: Ranger-1181: HDFS Plugin does not allow removal of a non-empty directory if the directory is allowed to be removed by HDFS, but the file inside the directory is allowed to be removed by Ranger

Posted by Madhan Neethiraj <ma...@apache.org>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53240/#review155546
-----------------------------------------------------------


Ship it!




Ship It!

- Madhan Neethiraj


On Nov. 10, 2016, 1:19 a.m., Yan Zhou wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53240/
> -----------------------------------------------------------
> 
> (Updated Nov. 10, 2016, 1:19 a.m.)
> 
> 
> Review request for ranger.
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> Reproduction Steps:
> 1. Ranger is installed and HDFS plug-in is enabled.
> 2. As qaadmin user, create a folder on HDFS with permission 500:
> hadoop fs -mkdir /tmp/rangertest1
> hadoop fs -chmod 500 /tmp/rangertest1
> while the /tmp itself has the 777 as the HDFS bits:
> hadoop fs -ls /
> drwxrwxrwx - user1 group1 0 2016-10-03 14:54 /tmp
> 3. Create a Ranger policy p1_1 by granting qaadmin with RWX permission to the folder of /tmp/rangertest1, recursive set to true 
> 4. Wait for around >30 seconds after Policy synced up.
> 5. Put a file to /tmp/rangertest1 folder:
> echo "This is a file2" > /tmp/temp
> hadoop fs -put /tmp/temp /tmp/rangertest1
> hadoop fs -ls /tmp/rangertest1
> Found 1 items
> rw-rr- 3 qaadmin hdfs 16 2016-09-21 19:13 /tmp/rangertest1/temp
> 6. Try to delete the non-empty folder with "-skipTrash" option, but it failed (delete the empty folder could success): 
> hadoop fs -rm -r -skipTrash /tmp/rangertest1
> rm: Permission denied: user=qaadmin, access=ALL, inode="/tmp/rangertest1":qaadmin:hdfs:dr-x------
> 
> 
> This happens when the HDFS fallback is enabled. The issue is with the synergy between the Ranger HDFS plugin and the HDFS. The removal of the directory of /tmp/rangertest1 is not covered by a Ranger policy but is allowed by of the HDFS permission for its parent, /tmp. The removal of the file inside /tmp/rangertest1 is, however, not allowed by HDFS for the 500 permission of its parent of /tmp/rangertest1, but allowed by Ranger.
> When Ranger finds that the directory of rangertest1 can't be removed by Ranger, it falls back to HDFS policy which does not allow the removal of the file inside rangertest1 even though allowed by Ranger.
> We should use HDFS fallback when the checks on ancestor/parent/current/subdir fails before proceed to the next check but with only the failed access check by the HDFS fallback not all of the access checks for ancestor, parent, current and subdir.
> 
> 
> Diffs
> -----
> 
>   hdfs-agent/src/main/java/org/apache/ranger/authorization/hadoop/RangerHdfsAuthorizer.java 6f452da 
> 
> Diff: https://reviews.apache.org/r/53240/diff/
> 
> 
> Testing
> -------
> 
> Unit test works ok.
> 
> 
> Thanks,
> 
> Yan Zhou
> 
>

Re: Review Request 53240: Ranger-1181: HDFS Plugin does not allow removal of a non-empty directory if the directory is allowed to be removed by HDFS, but the file inside the directory is allowed to be removed by Ranger

Posted by Yan Zhou via Review Board <no...@reviews.apache.org>.

(Updated April 13, 2017, 3:18 p.m.)

Review request for ranger.

Bugs: RANGER-1181
https://issues.apache.org/jira/browse/RANGER-1181

Repository: ranger

Description
-------

Diffs
-----

hdfs-agent/src/main/java/org/apache/ranger/authorization/hadoop/RangerHdfsAuthorizer.java 6f452da

Diff: https://reviews.apache.org/r/53240/diff/3/

Testing
-------

Unit test works ok.

Thanks,

Yan Zhou

Re: Review Request 53240: Ranger-1181: HDFS Plugin does not allow removal of a non-empty directory if the directory is allowed to be removed by HDFS, but the file inside the directory is allowed to be removed by Ranger

Posted by Yan Zhou <yz...@yahoo.com>.

(Updated Nov. 10, 2016, 1:19 a.m.)

Review request for ranger.

Changes
-------

Adding default check for ancestor permission per the review comment. Sorry for missing that part last night.

Repository: ranger

Description
-------

Diffs (updated)
-----

hdfs-agent/src/main/java/org/apache/ranger/authorization/hadoop/RangerHdfsAuthorizer.java 6f452da

Diff: https://reviews.apache.org/r/53240/diff/

Testing
-------

Unit test works ok.

Thanks,

Yan Zhou

Re: Review Request 53240: Ranger-1181: HDFS Plugin does not allow removal of a non-empty directory if the directory is allowed to be removed by HDFS, but the file inside the directory is allowed to be removed by Ranger

Posted by Madhan Neethiraj <ma...@apache.org>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53240/#review155503
-----------------------------------------------------------




hdfs-agent/src/main/java/org/apache/ranger/authorization/hadoop/RangerHdfsAuthorizer.java (line 253)
<https://reviews.apache.org/r/53240/#comment225472>

    shouldn't fallback to checkDefaultEnforcer() be done for ancestor-access as well? This was one of my comments in the previous review.


- Madhan Neethiraj


On Nov. 9, 2016, 7:16 a.m., Yan Zhou wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53240/
> -----------------------------------------------------------
> 
> (Updated Nov. 9, 2016, 7:16 a.m.)
> 
> 
> Review request for ranger.
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> Reproduction Steps:
> 1. Ranger is installed and HDFS plug-in is enabled.
> 2. As qaadmin user, create a folder on HDFS with permission 500:
> hadoop fs -mkdir /tmp/rangertest1
> hadoop fs -chmod 500 /tmp/rangertest1
> while the /tmp itself has the 777 as the HDFS bits:
> hadoop fs -ls /
> drwxrwxrwx - user1 group1 0 2016-10-03 14:54 /tmp
> 3. Create a Ranger policy p1_1 by granting qaadmin with RWX permission to the folder of /tmp/rangertest1, recursive set to true 
> 4. Wait for around >30 seconds after Policy synced up.
> 5. Put a file to /tmp/rangertest1 folder:
> echo "This is a file2" > /tmp/temp
> hadoop fs -put /tmp/temp /tmp/rangertest1
> hadoop fs -ls /tmp/rangertest1
> Found 1 items
> rw-rr- 3 qaadmin hdfs 16 2016-09-21 19:13 /tmp/rangertest1/temp
> 6. Try to delete the non-empty folder with "-skipTrash" option, but it failed (delete the empty folder could success): 
> hadoop fs -rm -r -skipTrash /tmp/rangertest1
> rm: Permission denied: user=qaadmin, access=ALL, inode="/tmp/rangertest1":qaadmin:hdfs:dr-x------
> 
> 
> This happens when the HDFS fallback is enabled. The issue is with the synergy between the Ranger HDFS plugin and the HDFS. The removal of the directory of /tmp/rangertest1 is not covered by a Ranger policy but is allowed by of the HDFS permission for its parent, /tmp. The removal of the file inside /tmp/rangertest1 is, however, not allowed by HDFS for the 500 permission of its parent of /tmp/rangertest1, but allowed by Ranger.
> When Ranger finds that the directory of rangertest1 can't be removed by Ranger, it falls back to HDFS policy which does not allow the removal of the file inside rangertest1 even though allowed by Ranger.
> We should use HDFS fallback when the checks on ancestor/parent/current/subdir fails before proceed to the next check but with only the failed access check by the HDFS fallback not all of the access checks for ancestor, parent, current and subdir.
> 
> 
> Diffs
> -----
> 
>   hdfs-agent/src/main/java/org/apache/ranger/authorization/hadoop/RangerHdfsAuthorizer.java 6f452da 
> 
> Diff: https://reviews.apache.org/r/53240/diff/
> 
> 
> Testing
> -------
> 
> Unit test works ok.
> 
> 
> Thanks,
> 
> Yan Zhou
> 
>

Re: Review Request 53240: Ranger-1181: HDFS Plugin does not allow removal of a non-empty directory if the directory is allowed to be removed by HDFS, but the file inside the directory is allowed to be removed by Ranger

Posted by Yan Zhou <yz...@yahoo.com>.

(Updated Nov. 9, 2016, 7:16 a.m.)

Review request for ranger.

Changes
-------

Addressing the 2 review comments.

Repository: ranger

Description
-------

Diffs (updated)
-----

hdfs-agent/src/main/java/org/apache/ranger/authorization/hadoop/RangerHdfsAuthorizer.java 6f452da

Diff: https://reviews.apache.org/r/53240/diff/

Testing
-------

Unit test works ok.

Thanks,

Yan Zhou