From 856a3ac154fd55e9de006c97fe419f647dd53810 Mon Sep 17 00:00:00 2001 From: tedyu Date: Tue, 29 May 2018 19:58:32 -0700 Subject: [PATCH] HBASE-20639 Implement permission checking through AccessController instead of RSGroupAdminEndpoint - revert due to pending discussion --- .../hbase/rsgroup/RSGroupAdminEndpoint.java | 7 ++ .../hbase/rsgroup/TestRSGroupsWithACL.java | 82 ++++++++----------- .../security/access/AccessController.java | 43 ---------- 3 files changed, 42 insertions(+), 90 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 8f9319fbf30..a7fcf3d4a7d 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,6 +205,7 @@ 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()); @@ -229,6 +230,7 @@ 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()); @@ -248,6 +250,7 @@ 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()); @@ -268,6 +271,7 @@ 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()); @@ -288,6 +292,7 @@ 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) { @@ -356,6 +361,7 @@ 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, @@ -383,6 +389,7 @@ 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 83d76a415eb..afdff712506 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,7 +23,6 @@ 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; @@ -33,14 +32,9 @@ 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; @@ -100,9 +94,6 @@ 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 { @@ -118,15 +109,8 @@ public class TestRSGroupsWithACL extends SecureTestUtil{ configureRSGroupAdminEndpoint(conf); TEST_UTIL.startMiniCluster(); - 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); + 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); @@ -239,7 +223,9 @@ public class TestRSGroupsWithACL extends SecureTestUtil{ return null; }; - validateAdminPermissions(action); + 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 @@ -249,57 +235,69 @@ public class TestRSGroupsWithACL extends SecureTestUtil{ return null; }; - validateAdminPermissions(action); + 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 = () -> { - accessController.preMoveServers(CTX, null, null); + rsGroupAdminEndpoint.checkPermission("moveServers"); return null; }; - validateAdminPermissions(action); + 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 = () -> { - accessController.preMoveTables(CTX, null, null); + rsGroupAdminEndpoint.checkPermission("moveTables"); return null; }; - validateAdminPermissions(action); + 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 = () -> { - accessController.preAddRSGroup(CTX, null); + rsGroupAdminEndpoint.checkPermission("addRSGroup"); return null; }; - validateAdminPermissions(action); + 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 = () -> { - accessController.preRemoveRSGroup(CTX, null); + rsGroupAdminEndpoint.checkPermission("removeRSGroup"); return null; }; - validateAdminPermissions(action); + 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 = () -> { - accessController.preBalanceRSGroup(CTX, null); + rsGroupAdminEndpoint.checkPermission("balanceRSGroup"); return null; }; - validateAdminPermissions(action); + 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 @@ -309,7 +307,9 @@ public class TestRSGroupsWithACL extends SecureTestUtil{ return null; }; - validateAdminPermissions(action); + 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 @@ -319,30 +319,18 @@ public class TestRSGroupsWithACL extends SecureTestUtil{ return null; }; - validateAdminPermissions(action); + 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 = () -> { - accessController.preMoveServersAndTables(CTX, null, null, null); + rsGroupAdminEndpoint.checkPermission("moveServersAndTables"); 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 8a7a16e65f9..275201c91bd 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,7 +95,6 @@ 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; @@ -1308,48 +1307,6 @@ 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