From 9bd4b04ca81b87c2686dce04e1175b2ac90a2f41 Mon Sep 17 00:00:00 2001 From: Nihal Jain Date: Fri, 25 May 2018 10:29:15 +0530 Subject: [PATCH] HBASE-20639 Implement permission checking through AccessController instead of RSGroupAdminEndpoint Signed-off-by: tedyu --- .../hbase/rsgroup/RSGroupAdminEndpoint.java | 7 -- .../hbase/rsgroup/TestRSGroupsWithACL.java | 82 +++++++++++-------- .../security/access/AccessController.java | 43 ++++++++++ 3 files changed, 90 insertions(+), 42 deletions(-) diff --git a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java index 365082682be..759f3e0b683 100644 --- a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java +++ b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java @@ -205,7 +205,6 @@ public class RSGroupAdminEndpoint implements MasterCoprocessor, MasterObserver { if (master.getMasterCoprocessorHost() != null) { master.getMasterCoprocessorHost().preMoveServers(hostPorts, request.getTargetGroup()); } - checkPermission("moveServers"); groupAdminServer.moveServers(hostPorts, request.getTargetGroup()); if (master.getMasterCoprocessorHost() != null) { master.getMasterCoprocessorHost().postMoveServers(hostPorts, request.getTargetGroup()); @@ -230,7 +229,6 @@ public class RSGroupAdminEndpoint implements MasterCoprocessor, MasterObserver { if (master.getMasterCoprocessorHost() != null) { master.getMasterCoprocessorHost().preMoveTables(tables, request.getTargetGroup()); } - checkPermission("moveTables"); groupAdminServer.moveTables(tables, request.getTargetGroup()); if (master.getMasterCoprocessorHost() != null) { master.getMasterCoprocessorHost().postMoveTables(tables, request.getTargetGroup()); @@ -250,7 +248,6 @@ public class RSGroupAdminEndpoint implements MasterCoprocessor, MasterObserver { if (master.getMasterCoprocessorHost() != null) { master.getMasterCoprocessorHost().preAddRSGroup(request.getRSGroupName()); } - checkPermission("addRSGroup"); groupAdminServer.addRSGroup(request.getRSGroupName()); if (master.getMasterCoprocessorHost() != null) { master.getMasterCoprocessorHost().postAddRSGroup(request.getRSGroupName()); @@ -271,7 +268,6 @@ public class RSGroupAdminEndpoint implements MasterCoprocessor, MasterObserver { if (master.getMasterCoprocessorHost() != null) { master.getMasterCoprocessorHost().preRemoveRSGroup(request.getRSGroupName()); } - checkPermission("removeRSGroup"); groupAdminServer.removeRSGroup(request.getRSGroupName()); if (master.getMasterCoprocessorHost() != null) { master.getMasterCoprocessorHost().postRemoveRSGroup(request.getRSGroupName()); @@ -292,7 +288,6 @@ public class RSGroupAdminEndpoint implements MasterCoprocessor, MasterObserver { if (master.getMasterCoprocessorHost() != null) { master.getMasterCoprocessorHost().preBalanceRSGroup(request.getRSGroupName()); } - checkPermission("balanceRSGroup"); boolean balancerRan = groupAdminServer.balanceRSGroup(request.getRSGroupName()); builder.setBalanceRan(balancerRan); if (master.getMasterCoprocessorHost() != null) { @@ -361,7 +356,6 @@ public class RSGroupAdminEndpoint implements MasterCoprocessor, MasterObserver { master.getMasterCoprocessorHost().preMoveServersAndTables(hostPorts, tables, request.getTargetGroup()); } - checkPermission("moveServersAndTables"); groupAdminServer.moveServersAndTables(hostPorts, tables, request.getTargetGroup()); if (master.getMasterCoprocessorHost() != null) { master.getMasterCoprocessorHost().postMoveServersAndTables(hostPorts, tables, @@ -389,7 +383,6 @@ public class RSGroupAdminEndpoint implements MasterCoprocessor, MasterObserver { if (master.getMasterCoprocessorHost() != null) { master.getMasterCoprocessorHost().preRemoveServers(servers); } - checkPermission("removeServers"); groupAdminServer.removeServers(servers); if (master.getMasterCoprocessorHost() != null) { master.getMasterCoprocessorHost().postRemoveServers(servers); 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 afdff712506..83d76a415eb 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,6 +23,7 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.Coprocessor; import org.apache.hadoop.hbase.HBaseClassTestRule; import org.apache.hadoop.hbase.HBaseTestingUtility; import org.apache.hadoop.hbase.HConstants; @@ -32,9 +33,14 @@ 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.coprocessor.MasterCoprocessorEnvironment; +import org.apache.hadoop.hbase.coprocessor.ObserverContext; +import org.apache.hadoop.hbase.coprocessor.ObserverContextImpl; +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.AccessController; import org.apache.hadoop.hbase.security.access.Permission; import org.apache.hadoop.hbase.security.access.SecureTestUtil; import org.apache.hadoop.hbase.security.access.TableAuthManager; @@ -94,6 +100,9 @@ public class TestRSGroupsWithACL extends SecureTestUtil{ private static byte[] TEST_FAMILY = Bytes.toBytes("f1"); private static RSGroupAdminEndpoint rsGroupAdminEndpoint; + private static AccessController accessController; + private static MasterCoprocessorEnvironment CP_ENV; + private static ObserverContext CTX; @BeforeClass public static void setupBeforeClass() throws Exception { @@ -109,8 +118,15 @@ public class TestRSGroupsWithACL extends SecureTestUtil{ configureRSGroupAdminEndpoint(conf); TEST_UTIL.startMiniCluster(); - rsGroupAdminEndpoint = (RSGroupAdminEndpoint) TEST_UTIL.getMiniHBaseCluster().getMaster(). - getMasterCoprocessorHost().findCoprocessor(RSGroupAdminEndpoint.class.getName()); + MasterCoprocessorHost masterCpHost = + TEST_UTIL.getMiniHBaseCluster().getMaster().getMasterCoprocessorHost(); + rsGroupAdminEndpoint = + (RSGroupAdminEndpoint) masterCpHost.findCoprocessor(RSGroupAdminEndpoint.class.getName()); + accessController = + (AccessController) masterCpHost.findCoprocessor(AccessController.class.getName()); + CP_ENV = + masterCpHost.createEnvironment(accessController, Coprocessor.PRIORITY_HIGHEST, 1, conf); + CTX = ObserverContextImpl.createAndPrepare(CP_ENV); // Wait for the ACL table to become available TEST_UTIL.waitUntilAllRegionsAssigned(AccessControlLists.ACL_TABLE_NAME); @@ -223,9 +239,7 @@ public class TestRSGroupsWithACL extends SecureTestUtil{ return null; }; - 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); + validateAdminPermissions(action); } @Test @@ -235,69 +249,57 @@ public class TestRSGroupsWithACL extends SecureTestUtil{ return null; }; - 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); + validateAdminPermissions(action); } @Test public void testMoveServers() throws Exception { AccessTestAction action = () -> { - rsGroupAdminEndpoint.checkPermission("moveServers"); + accessController.preMoveServers(CTX, null, null); return null; }; - 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); + validateAdminPermissions(action); } @Test public void testMoveTables() throws Exception { AccessTestAction action = () -> { - rsGroupAdminEndpoint.checkPermission("moveTables"); + accessController.preMoveTables(CTX, null, null); return null; }; - 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); + validateAdminPermissions(action); } @Test public void testAddRSGroup() throws Exception { AccessTestAction action = () -> { - rsGroupAdminEndpoint.checkPermission("addRSGroup"); + accessController.preAddRSGroup(CTX, null); return null; }; - 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); + validateAdminPermissions(action); } @Test public void testRemoveRSGroup() throws Exception { AccessTestAction action = () -> { - rsGroupAdminEndpoint.checkPermission("removeRSGroup"); + accessController.preRemoveRSGroup(CTX, null); return null; }; - 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); + validateAdminPermissions(action); } @Test public void testBalanceRSGroup() throws Exception { AccessTestAction action = () -> { - rsGroupAdminEndpoint.checkPermission("balanceRSGroup"); + accessController.preBalanceRSGroup(CTX, null); return null; }; - 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); + validateAdminPermissions(action); } @Test @@ -307,9 +309,7 @@ public class TestRSGroupsWithACL extends SecureTestUtil{ return null; }; - 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); + validateAdminPermissions(action); } @Test @@ -319,18 +319,30 @@ public class TestRSGroupsWithACL extends SecureTestUtil{ return null; }; - 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); + validateAdminPermissions(action); } @Test public void testMoveServersAndTables() throws Exception { AccessTestAction action = () -> { - rsGroupAdminEndpoint.checkPermission("moveServersAndTables"); + accessController.preMoveServersAndTables(CTX, null, null, null); return null; }; + validateAdminPermissions(action); + } + + @Test + public void testRemoveServers() throws Exception { + AccessTestAction action = () -> { + accessController.preRemoveServers(CTX, null); + return null; + }; + + validateAdminPermissions(action); + } + + private void validateAdminPermissions(AccessTestAction action) throws Exception { 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/AccessController.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java index bebf16cadba..8182b4282db 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java @@ -95,6 +95,7 @@ import org.apache.hadoop.hbase.filter.FilterList; import org.apache.hadoop.hbase.io.hfile.HFile; import org.apache.hadoop.hbase.ipc.CoprocessorRpcUtils; import org.apache.hadoop.hbase.ipc.RpcServer; +import org.apache.hadoop.hbase.net.Address; import org.apache.hadoop.hbase.protobuf.ProtobufUtil; import org.apache.hadoop.hbase.protobuf.generated.AccessControlProtos; import org.apache.hadoop.hbase.protobuf.generated.AccessControlProtos.AccessControlService; @@ -1307,6 +1308,48 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor, requirePermission(ctx, "recommissionRegionServers", Action.ADMIN); } + @Override + public void preMoveServersAndTables(final ObserverContext ctx, + Set
servers, Set tables, String targetGroup) throws IOException { + requirePermission(ctx, "moveServersAndTables", Action.ADMIN); + } + + @Override + public void preMoveServers(final ObserverContext ctx, + Set
servers, String targetGroup) throws IOException { + requirePermission(ctx, "moveServers", Action.ADMIN); + } + + @Override + public void preMoveTables(final ObserverContext ctx, + Set tables, String targetGroup) throws IOException { + requirePermission(ctx, "moveTables", Action.ADMIN); + } + + @Override + public void preAddRSGroup(final ObserverContext ctx, String name) + throws IOException { + requirePermission(ctx, "addRSGroup", Action.ADMIN); + } + + @Override + public void preRemoveRSGroup(final ObserverContext ctx, String name) + throws IOException { + requirePermission(ctx, "removeRSGroup", Action.ADMIN); + } + + @Override + public void preBalanceRSGroup(final ObserverContext ctx, + String groupName) throws IOException { + requirePermission(ctx, "balanceRSGroup", Action.ADMIN); + } + + @Override + public void preRemoveServers(final ObserverContext ctx, + Set
servers) throws IOException { + requirePermission(ctx, "removeServers", Action.ADMIN); + } + /* ---- RegionObserver implementation ---- */ @Override