From 81ea657ed14f811c3d1d44aeec82f57786f56689 Mon Sep 17 00:00:00 2001 From: Guangxu Cheng Date: Tue, 9 Jan 2018 17:30:08 +0800 Subject: [PATCH] HBASE-19483 Add proper privilege check for rsgroup commands addendum Signed-off-by: tedyu --- .../hbase/rsgroup/TestRSGroupsWithACL.java | 192 +++++++----------- .../security/access/AccessControlLists.java | 2 + 2 files changed, 74 insertions(+), 120 deletions(-) diff --git a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsWithACL.java b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsWithACL.java index dcc10a3ab1d..a428cfcf7aa 100644 --- a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsWithACL.java +++ b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsWithACL.java @@ -23,8 +23,6 @@ import static org.apache.hadoop.hbase.AuthUtil.toGroupEntry; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; -import java.io.IOException; - import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.HBaseTestingUtility; import org.apache.hadoop.hbase.HConstants; @@ -34,27 +32,27 @@ import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder; import org.apache.hadoop.hbase.client.Connection; import org.apache.hadoop.hbase.client.TableDescriptorBuilder; import org.apache.hadoop.hbase.coprocessor.CoprocessorHost; -import org.apache.hadoop.hbase.master.MasterCoprocessorHost; import org.apache.hadoop.hbase.security.User; import org.apache.hadoop.hbase.security.access.AccessControlClient; import org.apache.hadoop.hbase.security.access.AccessControlLists; import org.apache.hadoop.hbase.security.access.Permission; import org.apache.hadoop.hbase.security.access.SecureTestUtil; -import org.apache.hadoop.hbase.security.access.SecureTestUtil.AccessTestAction; import org.apache.hadoop.hbase.security.access.TableAuthManager; import org.apache.hadoop.hbase.testclassification.SecurityTests; import org.apache.hadoop.hbase.util.Bytes; import org.junit.AfterClass; import org.junit.BeforeClass; -import org.junit.Rule; import org.junit.Test; import org.junit.experimental.categories.Category; -import org.junit.rules.TestName; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +/** + * Performs authorization checks for rsgroup operations, according to different + * levels of authorized users. + */ @Category({SecurityTests.class}) -public class TestRSGroupsWithACL { +public class TestRSGroupsWithACL extends SecureTestUtil{ private static final Logger LOG = LoggerFactory.getLogger(TestRSGroupsWithACL.class); private static TableName TEST_TABLE = TableName.valueOf("testtable1"); private static final HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility(); @@ -75,8 +73,6 @@ public class TestRSGroupsWithACL { private static User USER_CREATE; // user with no permissions private static User USER_NONE; - // user with admin rights on the column family - private static User USER_ADMIN_CF; private static final String GROUP_ADMIN = "group_admin"; private static final String GROUP_CREATE = "group_create"; @@ -92,9 +88,6 @@ public class TestRSGroupsWithACL { private static RSGroupAdminEndpoint rsGroupAdminEndpoint; - @Rule - public TestName name = new TestName(); - @BeforeClass public static void setupBeforeClass() throws Exception { // setup configuration @@ -102,19 +95,15 @@ public class TestRSGroupsWithACL { conf.set(HConstants.HBASE_MASTER_LOADBALANCER_CLASS, RSGroupBasedLoadBalancer.class.getName()); // Enable security - SecureTestUtil.enableSecurity(conf); + enableSecurity(conf); // Verify enableSecurity sets up what we require - SecureTestUtil.verifyConfiguration(conf); - + verifyConfiguration(conf); + // Enable rsgroup configureRSGroupAdminEndpoint(conf); TEST_UTIL.startMiniCluster(); - MasterCoprocessorHost cpHost = - TEST_UTIL.getMiniHBaseCluster().getMaster().getMasterCoprocessorHost(); - rsGroupAdminEndpoint = (RSGroupAdminEndpoint) - cpHost.findCoprocessor(RSGroupAdminEndpoint.class.getName()); - - + rsGroupAdminEndpoint = (RSGroupAdminEndpoint) TEST_UTIL.getMiniHBaseCluster().getMaster(). + getMasterCoprocessorHost().findCoprocessor(RSGroupAdminEndpoint.class.getName()); // Wait for the ACL table to become available TEST_UTIL.waitUntilAllRegionsAssigned(AccessControlLists.ACL_TABLE_NAME); @@ -126,7 +115,6 @@ public class TestRSGroupsWithACL { USER_OWNER = User.createUserForTesting(conf, "owner", new String[0]); USER_CREATE = User.createUserForTesting(conf, "tbl_create", new String[0]); USER_NONE = User.createUserForTesting(conf, "nouser", new String[0]); - USER_ADMIN_CF = User.createUserForTesting(conf, "col_family_admin", new String[0]); USER_GROUP_ADMIN = User.createUserForTesting(conf, "user_group_admin", new String[] { GROUP_ADMIN }); @@ -147,40 +135,36 @@ public class TestRSGroupsWithACL { cfd.setMaxVersions(100); tableBuilder.addColumnFamily(cfd.build()); tableBuilder.setValue(TableDescriptorBuilder.OWNER, USER_OWNER.getShortName()); - SecureTestUtil.createTable(TEST_UTIL, tableBuilder.build(), + createTable(TEST_UTIL, tableBuilder.build(), new byte[][] { Bytes.toBytes("s") }); // Set up initial grants - SecureTestUtil.grantGlobal(TEST_UTIL, USER_ADMIN.getShortName(), + grantGlobal(TEST_UTIL, USER_ADMIN.getShortName(), Permission.Action.ADMIN, Permission.Action.CREATE, Permission.Action.READ, Permission.Action.WRITE); - SecureTestUtil.grantOnTable(TEST_UTIL, USER_RW.getShortName(), + grantOnTable(TEST_UTIL, USER_RW.getShortName(), TEST_TABLE, TEST_FAMILY, null, Permission.Action.READ, Permission.Action.WRITE); // USER_CREATE is USER_RW plus CREATE permissions - SecureTestUtil.grantOnTable(TEST_UTIL, USER_CREATE.getShortName(), + grantOnTable(TEST_UTIL, USER_CREATE.getShortName(), TEST_TABLE, null, null, Permission.Action.CREATE, Permission.Action.READ, Permission.Action.WRITE); - SecureTestUtil.grantOnTable(TEST_UTIL, USER_RO.getShortName(), + grantOnTable(TEST_UTIL, USER_RO.getShortName(), TEST_TABLE, TEST_FAMILY, null, Permission.Action.READ); - SecureTestUtil.grantOnTable(TEST_UTIL, USER_ADMIN_CF.getShortName(), - TEST_TABLE, TEST_FAMILY, - null, Permission.Action.ADMIN, Permission.Action.CREATE); - - SecureTestUtil.grantGlobal(TEST_UTIL, toGroupEntry(GROUP_ADMIN), Permission.Action.ADMIN); - SecureTestUtil.grantGlobal(TEST_UTIL, toGroupEntry(GROUP_CREATE), Permission.Action.CREATE); - SecureTestUtil.grantGlobal(TEST_UTIL, toGroupEntry(GROUP_READ), Permission.Action.READ); - SecureTestUtil.grantGlobal(TEST_UTIL, toGroupEntry(GROUP_WRITE), Permission.Action.WRITE); + grantGlobal(TEST_UTIL, toGroupEntry(GROUP_ADMIN), Permission.Action.ADMIN); + grantGlobal(TEST_UTIL, toGroupEntry(GROUP_CREATE), Permission.Action.CREATE); + grantGlobal(TEST_UTIL, toGroupEntry(GROUP_READ), Permission.Action.READ); + grantGlobal(TEST_UTIL, toGroupEntry(GROUP_WRITE), Permission.Action.WRITE); assertEquals(5, AccessControlLists.getTablePermissions(conf, TEST_TABLE).size()); try { @@ -194,16 +178,14 @@ public class TestRSGroupsWithACL { private static void cleanUp() throws Exception { // Clean the _acl_ table try { - SecureTestUtil.deleteTable(TEST_UTIL, TEST_TABLE); + deleteTable(TEST_UTIL, TEST_TABLE); } catch (TableNotFoundException ex) { // Test deleted the table, no problem LOG.info("Test deleted table " + TEST_TABLE); } // Verify all table/namespace permissions are erased assertEquals(0, AccessControlLists.getTablePermissions(conf, TEST_TABLE).size()); - assertEquals( - 0, - AccessControlLists.getNamespacePermissions(conf, + assertEquals(0, AccessControlLists.getNamespacePermissions(conf, TEST_TABLE.getNamespaceAsString()).size()); } @@ -215,7 +197,7 @@ public class TestRSGroupsWithACL { assertTrue("Unexpected reference count: " + total, total == 0); } - private static void configureRSGroupAdminEndpoint(Configuration conf) throws IOException { + private static void configureRSGroupAdminEndpoint(Configuration conf) { String currentCoprocessors = conf.get(CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY); String coprocessors = RSGroupAdminEndpoint.class.getName(); if (currentCoprocessors != null) { @@ -228,151 +210,121 @@ public class TestRSGroupsWithACL { @Test public void testGetRSGroupInfo() throws Exception { - AccessTestAction action = new AccessTestAction() { - @Override - public Object run() throws Exception { - rsGroupAdminEndpoint.checkPermission("getRSGroupInfo"); - return null; - } + AccessTestAction action = () -> { + rsGroupAdminEndpoint.checkPermission("getRSGroupInfo"); + return null; }; - SecureTestUtil.verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN); - SecureTestUtil.verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, + verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN); + verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ, USER_GROUP_WRITE, USER_GROUP_CREATE); } @Test public void testGetRSGroupInfoOfTable() throws Exception { - AccessTestAction action = new AccessTestAction() { - @Override - public Object run() throws Exception { - rsGroupAdminEndpoint.checkPermission("getRSGroupInfoOfTable"); - return null; - } + AccessTestAction action = () -> { + rsGroupAdminEndpoint.checkPermission("getRSGroupInfoOfTable"); + return null; }; - SecureTestUtil.verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN); - SecureTestUtil.verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, + verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN); + verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ, USER_GROUP_WRITE, USER_GROUP_CREATE); } @Test public void testMoveServers() throws Exception { - AccessTestAction action = new AccessTestAction() { - @Override - public Object run() throws Exception { - rsGroupAdminEndpoint.checkPermission("moveServers"); - return null; - } + AccessTestAction action = () -> { + rsGroupAdminEndpoint.checkPermission("moveServers"); + return null; }; - SecureTestUtil.verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN); - SecureTestUtil.verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, + verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN); + verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ, USER_GROUP_WRITE, USER_GROUP_CREATE); } @Test public void testMoveTables() throws Exception { - AccessTestAction action = new AccessTestAction() { - @Override - public Object run() throws Exception { - rsGroupAdminEndpoint.checkPermission("moveTables"); - return null; - } + AccessTestAction action = () -> { + rsGroupAdminEndpoint.checkPermission("moveTables"); + return null; }; - SecureTestUtil.verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN); - SecureTestUtil.verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, + verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN); + verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ, USER_GROUP_WRITE, USER_GROUP_CREATE); } @Test public void testAddRSGroup() throws Exception { - AccessTestAction action = new AccessTestAction() { - @Override - public Object run() throws Exception { - rsGroupAdminEndpoint.checkPermission("addRSGroup"); - return null; - } + AccessTestAction action = () -> { + rsGroupAdminEndpoint.checkPermission("addRSGroup"); + return null; }; - SecureTestUtil.verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN); - SecureTestUtil.verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, + verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN); + verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ, USER_GROUP_WRITE, USER_GROUP_CREATE); } @Test public void testRemoveRSGroup() throws Exception { - AccessTestAction action = new AccessTestAction() { - @Override - public Object run() throws Exception { - rsGroupAdminEndpoint.checkPermission("removeRSGroup"); - return null; - } + AccessTestAction action = () -> { + rsGroupAdminEndpoint.checkPermission("removeRSGroup"); + return null; }; - SecureTestUtil.verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN); - SecureTestUtil.verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, + verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN); + verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ, USER_GROUP_WRITE, USER_GROUP_CREATE); } @Test public void testBalanceRSGroup() throws Exception { - AccessTestAction action = new AccessTestAction() { - @Override - public Object run() throws Exception { - rsGroupAdminEndpoint.checkPermission("balanceRSGroup"); - return null; - } + AccessTestAction action = () -> { + rsGroupAdminEndpoint.checkPermission("balanceRSGroup"); + return null; }; - SecureTestUtil.verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN); - SecureTestUtil.verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, + verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN); + verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ, USER_GROUP_WRITE, USER_GROUP_CREATE); } @Test public void testListRSGroup() throws Exception { - AccessTestAction action = new AccessTestAction() { - @Override - public Object run() throws Exception { - rsGroupAdminEndpoint.checkPermission("listRSGroup"); - return null; - } + AccessTestAction action = () -> { + rsGroupAdminEndpoint.checkPermission("listRSGroup"); + return null; }; - SecureTestUtil.verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN); - SecureTestUtil.verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, + verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN); + verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ, USER_GROUP_WRITE, USER_GROUP_CREATE); } @Test public void testGetRSGroupInfoOfServer() throws Exception { - AccessTestAction action = new AccessTestAction() { - @Override - public Object run() throws Exception { - rsGroupAdminEndpoint.checkPermission("getRSGroupInfoOfServer"); - return null; - } + AccessTestAction action = () -> { + rsGroupAdminEndpoint.checkPermission("getRSGroupInfoOfServer"); + return null; }; - SecureTestUtil.verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN); - SecureTestUtil.verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, + verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN); + verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ, USER_GROUP_WRITE, USER_GROUP_CREATE); } @Test public void testMoveServersAndTables() throws Exception { - AccessTestAction action = new AccessTestAction() { - @Override - public Object run() throws Exception { - rsGroupAdminEndpoint.checkPermission("moveServersAndTables"); - return null; - } + AccessTestAction action = () -> { + rsGroupAdminEndpoint.checkPermission("moveServersAndTables"); + return null; }; - SecureTestUtil.verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN); - SecureTestUtil.verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, + verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN); + verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ, USER_GROUP_WRITE, USER_GROUP_CREATE); } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlLists.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlLists.java index f5f5d14601c..b0f33bdb461 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlLists.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlLists.java @@ -63,6 +63,7 @@ import org.apache.hadoop.hbase.protobuf.generated.AccessControlProtos; import org.apache.hadoop.hbase.regionserver.InternalScanner; import org.apache.hadoop.hbase.regionserver.Region; import org.apache.hadoop.hbase.security.User; +import org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting; import org.apache.hbase.thirdparty.com.google.common.collect.ArrayListMultimap; import org.apache.hbase.thirdparty.com.google.common.collect.ListMultimap; import org.apache.hbase.thirdparty.com.google.common.collect.Lists; @@ -490,6 +491,7 @@ public class AccessControlLists { return getPermissions(conf, tableName != null ? tableName.getName() : null, null); } + @VisibleForTesting public static ListMultimap getNamespacePermissions(Configuration conf, String namespace) throws IOException { return getPermissions(conf, Bytes.toBytes(toNamespaceEntry(namespace)), null);