You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2016/05/25 03:41:14 UTC

[2/3] incubator-impala git commit: IMPALA-3143: Fix permission check when user belongs to supergroup

IMPALA-3143: Fix permission check when user belongs to supergroup

This commit fixes an issue where a permission check warning is
incorrectly generated when a user that belongs to the supergroup issues
a create database statement. The fix is to also check if the user
belongs to the supergroup when checking for permissions and ACLs.

Testing:
This change was tested manually by creating an HDFS directory D that
belonged to user/group A and running a CREATE DATABASE foo LOCALTION 'D'
statement as a user B that belongs to the 'supergroup' group; the test
verified that when the supergroup is taken into account, no permision
check warning is issued.

Change-Id: I2bb703ac70280c62db404db2c07d87139aa5ae0d
Reviewed-on: http://gerrit.cloudera.org:8080/3164
Reviewed-by: Alex Behm <al...@cloudera.com>
Tested-by: Internal Jenkins


Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/a0ad1868
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/a0ad1868
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/a0ad1868

Branch: refs/heads/master
Commit: a0ad1868bda902fd914bc2be39eb9629a6eceb76
Parents: 6c8dc1b
Author: Dimitris Tsirogiannis <dt...@cloudera.com>
Authored: Fri May 20 20:49:21 2016 -0700
Committer: Tim Armstrong <ta...@cloudera.com>
Committed: Tue May 24 20:41:09 2016 -0700

----------------------------------------------------------------------
 .../com/cloudera/impala/util/FsPermissionChecker.java  | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/a0ad1868/fe/src/main/java/com/cloudera/impala/util/FsPermissionChecker.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/com/cloudera/impala/util/FsPermissionChecker.java b/fe/src/main/java/com/cloudera/impala/util/FsPermissionChecker.java
index 18d0534..056fdf0 100644
--- a/fe/src/main/java/com/cloudera/impala/util/FsPermissionChecker.java
+++ b/fe/src/main/java/com/cloudera/impala/util/FsPermissionChecker.java
@@ -24,10 +24,10 @@ import java.util.List;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.permission.AclEntry;
 import org.apache.hadoop.fs.permission.AclEntryType;
 import org.apache.hadoop.fs.permission.AclStatus;
-import org.apache.hadoop.fs.permission.AclStatus;
 import org.apache.hadoop.fs.permission.AclEntryScope;
 import org.apache.hadoop.fs.FileStatus;
 import org.apache.hadoop.fs.FileSystem;
@@ -36,7 +36,8 @@ import org.apache.hadoop.fs.permission.FsAction;
 import org.apache.hadoop.fs.permission.FsPermission;
 import org.apache.hadoop.security.UserGroupInformation;
 import org.apache.hadoop.hdfs.protocol.AclException;
-import org.apache.hadoop.ipc.RemoteException;
+import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_PERMISSIONS_SUPERUSERGROUP_DEFAULT;
+import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_PERMISSIONS_SUPERUSERGROUP_KEY;
 
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
@@ -50,10 +51,13 @@ import com.google.common.collect.Lists;
 public class FsPermissionChecker {
   private final static Logger LOG = LoggerFactory.getLogger(FsPermissionChecker.class);
   private final static FsPermissionChecker instance_;
+  private final static Configuration CONF;
   protected final String user_;
   private final Set<String> groups_ = new HashSet<String>();
+  private final String supergroup_;
 
   static {
+    CONF = new Configuration();
     try {
       instance_ = new FsPermissionChecker();
     } catch (IOException e) {
@@ -65,10 +69,12 @@ public class FsPermissionChecker {
   private FsPermissionChecker() throws IOException {
     UserGroupInformation ugi = UserGroupInformation.getCurrentUser();
     groups_.addAll(Arrays.asList(ugi.getGroupNames()));
-    // Todo: should add HDFS supergroup as well
+    supergroup_ = CONF.get(DFS_PERMISSIONS_SUPERUSERGROUP_KEY,
+        DFS_PERMISSIONS_SUPERUSERGROUP_DEFAULT);
     user_ = ugi.getShortUserName();
   }
 
+  private boolean isSuperUser() { return groups_.contains(supergroup_); }
 
   private static List<AclEntryType> ACL_TYPE_PRIORITY =
       ImmutableList.of(AclEntryType.USER, AclEntryType.GROUP, AclEntryType.OTHER);
@@ -198,6 +204,7 @@ public class FsPermissionChecker {
      * permissions.
      */
     public boolean checkPermissions(FsAction action) {
+      if (FsPermissionChecker.this.isSuperUser()) return true;
       Boolean aclPerms = checkAcls(action);
       if (aclPerms != null) return aclPerms;