From b36bae5890f26713b43ddf8766bd495d5a410cb4 Mon Sep 17 00:00:00 2001 From: Pankaj Date: Tue, 15 Sep 2015 17:42:11 +0800 Subject: [PATCH] HBASE-14425, In Secure Zookeeper cluster superuser will not have sufficient permission if multiple values are configured in "hbase.superuser" --- .../org/apache/hadoop/hbase/zookeeper/ZKUtil.java | 19 +++++++++++++++--- .../hadoop/hbase/zookeeper/ZooKeeperWatcher.java | 15 ++++++++++++-- .../apache/hadoop/hbase/zookeeper/TestZKUtil.java | 23 +++++++++++++++++++++- 3 files changed, 51 insertions(+), 6 deletions(-) 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 7f8d82a..74a84f8 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; @@ -1025,11 +1026,23 @@ public class ZKUtil { return Ids.OPEN_ACL_UNSAFE; } if (isSecureZooKeeper) { - String superUser = zkw.getConfiguration().get("hbase.superuser"); ArrayList acls = new ArrayList(); // add permission to hbase supper user - if (superUser != null) { - acls.add(new ACL(Perms.ALL, new Id("auth", superUser))); + String[] superUsers = zkw.getConfiguration().getStrings("hbase.superuser"); + if (superUsers != null) { + List groups = new ArrayList(); + 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 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 475e385..29af9da 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,6 +31,7 @@ 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; @@ -255,7 +256,7 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable { * @throws IOException */ private boolean isBaseZnodeAclSetup(List acls) throws IOException { - String superUser = conf.get("hbase.superuser"); + String[] superUsers = conf.getStrings("hbase.superuser"); // this assumes that current authenticated user is the same as zookeeper client user // configured via JAAS @@ -274,7 +275,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; } @@ -289,6 +290,16 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable { return true; } + private 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() { return this.identifier + ", quorum=" + quorum + ", baseZNode=" + baseZNode; 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..8cc9fc5 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,17 @@ */ 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.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 +49,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("hbase.superuser", "user1,@group1,user2,@group2,user3"); + String node = "/hbase/testCreateACL"; + ZooKeeperWatcher watcher = new ZooKeeperWatcher(conf, node, null, false); + List 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")))); + } } -- 1.9.2.msysgit.0