hbase-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From e...@apache.org
Subject [1/3] hbase git commit: HBASE-14425 In Secure Zookeeper cluster superuser will not have sufficient permission if multiple values are configured in hbase.superuser (Pankaj Kumar)
Date Wed, 28 Oct 2015 00:05:52 GMT
Repository: hbase
Updated Branches:
  refs/heads/branch-1 d7c1468ed -> c174a54d8
  refs/heads/branch-1.2 6bd8bf1e2 -> 4a9984da4
  refs/heads/master 007e4dfa1 -> 0e6dd3257


HBASE-14425 In Secure Zookeeper cluster superuser will not have sufficient permission if multiple
values are configured in hbase.superuser (Pankaj Kumar)


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/0e6dd325
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/0e6dd325
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/0e6dd325

Branch: refs/heads/master
Commit: 0e6dd3257b1bebe3e12c84aace59dd9cf0dcac2b
Parents: 007e4df
Author: Enis Soztutar <enis@apache.org>
Authored: Tue Oct 27 16:56:20 2015 -0700
Committer: Enis Soztutar <enis@apache.org>
Committed: Tue Oct 27 16:56:20 2015 -0700

----------------------------------------------------------------------
 .../apache/hadoop/hbase/zookeeper/ZKUtil.java   | 20 +++++++--
 .../hbase/zookeeper/ZooKeeperWatcher.java       | 45 +++++++++++++++++++-
 .../hadoop/hbase/zookeeper/TestZKUtil.java      | 24 ++++++++++-
 .../test/IntegrationTestZKAndFSPermissions.java |  3 +-
 4 files changed, 85 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/0e6dd325/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java
----------------------------------------------------------------------
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java
index 27c3bba..633525f 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java
@@ -39,6 +39,7 @@ import org.apache.commons.lang.StringUtils;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.AuthUtil;
 import org.apache.hadoop.hbase.HBaseConfiguration;
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.ServerName;
@@ -48,6 +49,7 @@ import org.apache.hadoop.hbase.protobuf.ProtobufUtil;
 import org.apache.hadoop.hbase.protobuf.generated.ClusterStatusProtos;
 import org.apache.hadoop.hbase.protobuf.generated.ClusterStatusProtos.RegionStoreSequenceIds;
 import org.apache.hadoop.hbase.protobuf.generated.ZooKeeperProtos;
+import org.apache.hadoop.hbase.security.Superusers;
 import org.apache.hadoop.hbase.util.ByteStringer;
 import org.apache.hadoop.hbase.util.Bytes;
 import org.apache.hadoop.hbase.util.Threads;
@@ -1028,11 +1030,23 @@ public class ZKUtil {
       return Ids.OPEN_ACL_UNSAFE;
     }
     if (isSecureZooKeeper) {
-      String superUser = zkw.getConfiguration().get("hbase.superuser");
       ArrayList<ACL> acls = new ArrayList<ACL>();
       // add permission to hbase supper user
-      if (superUser != null) {
-        acls.add(new ACL(Perms.ALL, new Id("auth", superUser)));
+      String[] superUsers = zkw.getConfiguration().getStrings(Superusers.SUPERUSER_CONF_KEY);
+      if (superUsers != null) {
+        List<String> groups = new ArrayList<String>();
+        for (String user : superUsers) {
+          if (user.startsWith(AuthUtil.GROUP_PREFIX)) {
+            // TODO: Set node ACL for groups when ZK supports this feature
+            groups.add(user);
+          } else {
+            acls.add(new ACL(Perms.ALL, new Id("auth", user)));
+          }
+        }
+        if (!groups.isEmpty()) {
+          LOG.warn("Znode ACL setting for group " + groups
+              + " is skipped, Zookeeper doesn't support this feature presently.");
+        }
       }
       // Certain znodes are accessed directly by the client,
       // so they must be readable by non-authenticated clients

http://git-wip-us.apache.org/repos/asf/hbase/blob/0e6dd325/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
----------------------------------------------------------------------
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
index 220a679..f7a2175 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
@@ -31,10 +31,12 @@ import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.Abortable;
+import org.apache.hadoop.hbase.AuthUtil;
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.HRegionInfo;
 import org.apache.hadoop.hbase.ZooKeeperConnectionException;
 import org.apache.hadoop.hbase.classification.InterfaceAudience;
+import org.apache.hadoop.hbase.security.Superusers;
 import org.apache.hadoop.security.UserGroupInformation;
 import org.apache.zookeeper.KeeperException;
 import org.apache.zookeeper.WatchedEvent;
@@ -257,7 +259,11 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable
{
    * @throws IOException
    */
   private boolean isBaseZnodeAclSetup(List<ACL> acls) throws IOException {
-    String superUser = conf.get("hbase.superuser");
+    String[] superUsers = conf.getStrings(Superusers.SUPERUSER_CONF_KEY);
+    // Check whether ACL set for all superusers
+    if (superUsers != null && !checkACLForSuperUsers(superUsers, acls)) {
+      return false;
+    }
 
     // this assumes that current authenticated user is the same as zookeeper client user
     // configured via JAAS
@@ -276,7 +282,7 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable
{
         if (perms != Perms.READ) {
           return false;
         }
-      } else if (superUser != null && new Id("sasl", superUser).equals(id)) {
+      } else if (superUsers != null && isSuperUserId(superUsers, id)) {
         if (perms != Perms.ALL) {
           return false;
         }
@@ -290,6 +296,41 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable
{
     }
     return true;
   }
+  
+  /*
+   * Validate whether ACL set for all superusers.
+   */
+  private boolean checkACLForSuperUsers(String[] superUsers, List<ACL> acls) {
+    for (String user : superUsers) {
+      boolean hasAccess = false;
+      // TODO: Validate super group members also when ZK supports setting node ACL for groups.
+      if (!user.startsWith(AuthUtil.GROUP_PREFIX)) {
+        for (ACL acl : acls) {
+          if (user.equals(acl.getId().getId()) && acl.getPerms() == Perms.ALL) {
+            hasAccess = true;
+            break;
+          }
+        }
+        if (!hasAccess) {
+          return false;
+        }
+      }
+    }
+    return true;
+  }
+  
+  /*
+   * Validate whether ACL ID is superuser.
+   */
+  public static boolean isSuperUserId(String[] superUsers, Id id) {
+    for (String user : superUsers) {
+      // TODO: Validate super group members also when ZK supports setting node ACL for groups.
+      if (!user.startsWith(AuthUtil.GROUP_PREFIX) && new Id("sasl", user).equals(id))
{
+        return true;
+      }
+    }
+    return false;
+  }
 
   @Override
   public String toString() {

http://git-wip-us.apache.org/repos/asf/hbase/blob/0e6dd325/hbase-client/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKUtil.java
----------------------------------------------------------------------
diff --git a/hbase-client/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKUtil.java
b/hbase-client/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKUtil.java
index 7224507..72de935 100644
--- a/hbase-client/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKUtil.java
+++ b/hbase-client/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKUtil.java
@@ -18,11 +18,18 @@
  */
 package org.apache.hadoop.hbase.zookeeper;
 
-import org.apache.hadoop.conf.Configuration;
+import java.io.IOException;
+import java.util.List;
 
+import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.HBaseConfiguration;
 import org.apache.hadoop.hbase.HConstants;
+import org.apache.hadoop.hbase.ZooKeeperConnectionException;
+import org.apache.hadoop.hbase.security.Superusers;
 import org.apache.hadoop.hbase.testclassification.SmallTests;
+import org.apache.zookeeper.ZooDefs.Perms;
+import org.apache.zookeeper.data.ACL;
+import org.apache.zookeeper.data.Id;
 import org.junit.Assert;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
@@ -43,4 +50,19 @@ public class TestZKUtil {
     Assert.assertTrue(!clusterKey.contains("\t") && !clusterKey.contains("\n"));
     Assert.assertEquals("localhost:3333:hbase,test", clusterKey);
   }
+  
+  @Test
+  public void testCreateACL() throws ZooKeeperConnectionException, IOException {
+    Configuration conf = HBaseConfiguration.create();
+    conf.set(Superusers.SUPERUSER_CONF_KEY, "user1,@group1,user2,@group2,user3");
+    String node = "/hbase/testCreateACL";
+    ZooKeeperWatcher watcher = new ZooKeeperWatcher(conf, node, null, false);
+    List<ACL> aclList = ZKUtil.createACL(watcher, node, true);
+    Assert.assertEquals(aclList.size(), 4); // 3+1, since ACL will be set for the creator
by default
+    Assert.assertTrue(!aclList.contains(new ACL(Perms.ALL, new Id("auth", "@group1")))
+        && !aclList.contains(new ACL(Perms.ALL, new Id("auth", "@group2"))));
+    Assert.assertTrue(aclList.contains(new ACL(Perms.ALL, new Id("auth", "user1")))
+        && aclList.contains(new ACL(Perms.ALL, new Id("auth", "user2")))
+        && aclList.contains(new ACL(Perms.ALL, new Id("auth", "user3"))));
+  }
 }

http://git-wip-us.apache.org/repos/asf/hbase/blob/0e6dd325/hbase-it/src/test/java/org/apache/hadoop/hbase/test/IntegrationTestZKAndFSPermissions.java
----------------------------------------------------------------------
diff --git a/hbase-it/src/test/java/org/apache/hadoop/hbase/test/IntegrationTestZKAndFSPermissions.java
b/hbase-it/src/test/java/org/apache/hadoop/hbase/test/IntegrationTestZKAndFSPermissions.java
index c39056d..3845846 100644
--- a/hbase-it/src/test/java/org/apache/hadoop/hbase/test/IntegrationTestZKAndFSPermissions.java
+++ b/hbase-it/src/test/java/org/apache/hadoop/hbase/test/IntegrationTestZKAndFSPermissions.java
@@ -178,6 +178,7 @@ public class IntegrationTestZKAndFSPermissions extends AbstractHBaseTool
{
       boolean expectedWorldReadable) throws KeeperException, InterruptedException {
     Stat stat = new Stat();
     List<ACL> acls = zk.getZooKeeper().getACL(znode, stat);
+    String[] superUsers = superUser == null ? null : superUser.split(",");
 
     LOG.info("Checking ACLs for znode znode:" + znode + " acls:" + acls);
 
@@ -191,7 +192,7 @@ public class IntegrationTestZKAndFSPermissions extends AbstractHBaseTool
{
         assertTrue(expectedWorldReadable);
         // assert that anyone can only read
         assertEquals(perms, Perms.READ);
-      } else if (superUser != null && new Id("sasl", superUser).equals(id)) {
+      } else if (superUsers != null && ZooKeeperWatcher.isSuperUserId(superUsers,
id)) {
         // assert that super user has all the permissions
         assertEquals(perms, Perms.ALL);
       } else if (new Id("sasl", masterPrincipal).equals(id)) {


Mime
View raw message