----------------------------------------------------------- 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 > >
