Repository: hbase Updated Branches: refs/heads/branch-2 b06aec445 -> ad425e860
HBASE-20185 Fix ACL check for MasterRpcServices#execProcedure Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/ad425e86 Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/ad425e86 Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/ad425e86 Branch: refs/heads/branch-2 Commit: ad425e8603fe07e45f9f00890cc4f063346ddc9f Parents: b06aec4 Author: Apekshit Sharma <[email protected]> Authored: Tue Mar 13 17:13:56 2018 +0530 Committer: Apekshit Sharma <[email protected]> Committed: Wed Mar 14 19:08:17 2018 +0530 ---------------------------------------------------------------------- .../hadoop/hbase/master/MasterRpcServices.java | 4 ++- .../hbase/master/snapshot/SnapshotManager.java | 8 +++++ .../hbase/procedure/MasterProcedureManager.java | 31 ++++++++++++-------- .../flush/MasterFlushTableProcedureManager.java | 10 +++++++ .../hbase/regionserver/RSRpcServices.java | 2 +- .../hbase/security/access/AccessController.java | 3 ++ .../procedure/SimpleMasterProcedureManager.java | 6 ++++ 7 files changed, 49 insertions(+), 15 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/ad425e86/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java index 8f92041..70f4cd4 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java @@ -57,6 +57,7 @@ import org.apache.hadoop.hbase.io.hfile.HFile; import org.apache.hadoop.hbase.ipc.CoprocessorRpcUtils; import org.apache.hadoop.hbase.ipc.PriorityFunction; import org.apache.hadoop.hbase.ipc.QosPriority; +import org.apache.hadoop.hbase.ipc.RpcServer; import org.apache.hadoop.hbase.ipc.RpcServer.BlockingServiceAndInterface; import org.apache.hadoop.hbase.ipc.RpcServerFactory; import org.apache.hadoop.hbase.ipc.RpcServerInterface; @@ -831,8 +832,8 @@ public class MasterRpcServices extends RSRpcServices @Override public ExecProcedureResponse execProcedure(RpcController controller, ExecProcedureRequest request) throws ServiceException { - rpcPreCheck("execProcedure"); try { + master.checkInitialized(); ProcedureDescription desc = request.getProcedure(); MasterProcedureManager mpm = master.getMasterProcedureManagerHost().getProcedureManager( desc.getSignature()); @@ -841,6 +842,7 @@ public class MasterRpcServices extends RSRpcServices + desc.getSignature())); } LOG.info(master.getClientIdAuditPrefix() + " procedure request for: " + desc.getSignature()); + mpm.checkPermissions(desc, accessChecker, RpcServer.getRequestUser().orElse(null)); mpm.execProcedure(desc); // send back the max amount of time the client should wait for the procedure // to complete http://git-wip-us.apache.org/repos/asf/hbase/blob/ad425e86/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java index 3870601..1abd605 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java @@ -63,6 +63,7 @@ import org.apache.hadoop.hbase.procedure.ZKProcedureCoordinator; import org.apache.hadoop.hbase.procedure2.ProcedureExecutor; import org.apache.hadoop.hbase.security.AccessDeniedException; import org.apache.hadoop.hbase.security.User; +import org.apache.hadoop.hbase.security.access.AccessChecker; import org.apache.hadoop.hbase.snapshot.ClientSnapshotDescriptionUtils; import org.apache.hadoop.hbase.snapshot.HBaseSnapshotException; import org.apache.hadoop.hbase.snapshot.RestoreSnapshotException; @@ -1150,6 +1151,13 @@ public class SnapshotManager extends MasterProcedureManager implements Stoppable } @Override + public void checkPermissions(ProcedureDescription desc, AccessChecker accessChecker, User user) + throws IOException { + // Done by AccessController as part of preSnapshot coprocessor hook (legacy code path). + // In future, when we AC is removed for good, that check should be moved here. + } + + @Override public boolean isProcedureDone(ProcedureDescription desc) throws IOException { return isSnapshotDone(toSnapshotDescription(desc)); } http://git-wip-us.apache.org/repos/asf/hbase/blob/ad425e86/hbase-server/src/main/java/org/apache/hadoop/hbase/procedure/MasterProcedureManager.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/procedure/MasterProcedureManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/procedure/MasterProcedureManager.java index 89e55d1..b705157 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/procedure/MasterProcedureManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/procedure/MasterProcedureManager.java @@ -19,6 +19,8 @@ package org.apache.hadoop.hbase.procedure; import java.io.IOException; +import org.apache.hadoop.hbase.security.User; +import org.apache.hadoop.hbase.security.access.AccessChecker; import org.apache.yetus.audience.InterfaceAudience; import org.apache.yetus.audience.InterfaceStability; import org.apache.hadoop.hbase.Stoppable; @@ -34,28 +36,26 @@ import org.apache.zookeeper.KeeperException; * * To implement a custom globally barriered procedure, user needs to extend two classes: * {@link MasterProcedureManager} and {@link RegionServerProcedureManager}. Implementation of -* {@link MasterProcedureManager} is loaded into {@link org.apache.hadoop.hbase.master.HMaster} +* {@link MasterProcedureManager} is loaded into {@link org.apache.hadoop.hbase.master.HMaster} * process via configuration parameter 'hbase.procedure.master.classes', while implementation of -* {@link RegionServerProcedureManager} is loaded into +* {@link RegionServerProcedureManager} is loaded into * {@link org.apache.hadoop.hbase.regionserver.HRegionServer} process via * configuration parameter 'hbase.procedure.regionserver.classes'. * -* An example of globally barriered procedure implementation is +* An example of globally barriered procedure implementation is * {@link org.apache.hadoop.hbase.master.snapshot.SnapshotManager} and * {@link org.apache.hadoop.hbase.regionserver.snapshot.RegionServerSnapshotManager}. * * A globally barriered procedure is identified by its signature (usually it is the name of the * procedure znode). During the initialization phase, the initialize methods are called by both -* {@link org.apache.hadoop.hbase.master.HMaster} -* and {@link org.apache.hadoop.hbase.regionserver.HRegionServer} which create the procedure znode -* and register the listeners. A procedure can be triggered by its signature and an instant name -* (encapsulated in a {@link ProcedureDescription} object). When the servers are shutdown, +* {@link org.apache.hadoop.hbase.master.HMaster} +* and {@link org.apache.hadoop.hbase.regionserver.HRegionServer} which create the procedure znode +* and register the listeners. A procedure can be triggered by its signature and an instant name +* (encapsulated in a {@link ProcedureDescription} object). When the servers are shutdown, * the stop methods on both classes are called to clean up the data associated with the procedure. */ @InterfaceAudience.Private [email protected] -public abstract class MasterProcedureManager extends ProcedureManager implements - Stoppable { +public abstract class MasterProcedureManager extends ProcedureManager implements Stoppable { /** * Initialize a globally barriered procedure for master. * @@ -73,9 +73,7 @@ public abstract class MasterProcedureManager extends ProcedureManager implements * @param desc Procedure description * @throws IOException */ - public void execProcedure(ProcedureDescription desc) throws IOException { - - } + public void execProcedure(ProcedureDescription desc) throws IOException {} /** * Execute a distributed procedure on cluster with return data. @@ -90,6 +88,13 @@ public abstract class MasterProcedureManager extends ProcedureManager implements } /** + * Check for required permissions before executing the procedure. + * @throws IOException if permissions requirements are not met. + */ + public abstract void checkPermissions(ProcedureDescription desc, AccessChecker accessChecker, + User user) throws IOException; + + /** * Check if the procedure is finished successfully * * @param desc Procedure description http://git-wip-us.apache.org/repos/asf/hbase/blob/ad425e86/hbase-server/src/main/java/org/apache/hadoop/hbase/procedure/flush/MasterFlushTableProcedureManager.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/procedure/flush/MasterFlushTableProcedureManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/procedure/flush/MasterFlushTableProcedureManager.java index 75e041a..381e526 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/procedure/flush/MasterFlushTableProcedureManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/procedure/flush/MasterFlushTableProcedureManager.java @@ -41,6 +41,9 @@ import org.apache.hadoop.hbase.procedure.Procedure; import org.apache.hadoop.hbase.procedure.ProcedureCoordinator; import org.apache.hadoop.hbase.procedure.ProcedureCoordinatorRpcs; import org.apache.hadoop.hbase.procedure.ZKProcedureCoordinator; +import org.apache.hadoop.hbase.security.User; +import org.apache.hadoop.hbase.security.access.AccessChecker; +import org.apache.hadoop.hbase.security.access.Permission; import org.apache.hadoop.hbase.util.Pair; import org.apache.hadoop.hbase.zookeeper.MetaTableLocator; import org.apache.yetus.audience.InterfaceAudience; @@ -182,6 +185,13 @@ public class MasterFlushTableProcedureManager extends MasterProcedureManager { } @Override + public void checkPermissions(ProcedureDescription desc, AccessChecker accessChecker, User user) + throws IOException { + // Done by AccessController as part of preTableFlush coprocessor hook (legacy code path). + // In future, when we AC is removed for good, that check should be moved here. + } + + @Override public synchronized boolean isProcedureDone(ProcedureDescription desc) throws IOException { // Procedure instance name is the table name. TableName tableName = TableName.valueOf(desc.getInstance()); http://git-wip-us.apache.org/repos/asf/hbase/blob/ad425e86/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java index acf25cf..5110939 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java @@ -331,7 +331,7 @@ public class RSRpcServices implements HBaseRPCErrorHandler, // Initialized in start() since AccessChecker needs ZKWatcher which is created by HRegionServer // after RSRpcServices constructor and before start() is called. // Initialized only if authorization is enabled, else remains null. - private AccessChecker accessChecker; + protected AccessChecker accessChecker; /** * Services launched in RSRpcServices. By default they are on but you can use the below http://git-wip-us.apache.org/repos/asf/hbase/blob/ad425e86/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java ---------------------------------------------------------------------- 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 b20d7e4..cfc0b91 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 @@ -1136,6 +1136,7 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor, public void preSnapshot(final ObserverContext<MasterCoprocessorEnvironment> ctx, final SnapshotDescription snapshot, final TableDescriptor hTableDescriptor) throws IOException { + // Move this ACL check to SnapshotManager#checkPermissions as part of AC deprecation. requirePermission(ctx, "snapshot " + snapshot.getName(), hTableDescriptor.getTableName(), null, null, Permission.Action.ADMIN); } @@ -1266,6 +1267,8 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor, @Override public void preTableFlush(final ObserverContext<MasterCoprocessorEnvironment> ctx, final TableName tableName) throws IOException { + // Move this ACL check to MasterFlushTableProcedureManager#checkPermissions as part of AC + // deprecation. requirePermission(ctx, "flushTable", tableName, null, null, Action.ADMIN, Action.CREATE); } http://git-wip-us.apache.org/repos/asf/hbase/blob/ad425e86/hbase-server/src/test/java/org/apache/hadoop/hbase/procedure/SimpleMasterProcedureManager.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/procedure/SimpleMasterProcedureManager.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/procedure/SimpleMasterProcedureManager.java index 579fcd3..e1d46ef 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/procedure/SimpleMasterProcedureManager.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/procedure/SimpleMasterProcedureManager.java @@ -29,6 +29,8 @@ import org.apache.hadoop.hbase.errorhandling.ForeignExceptionDispatcher; import org.apache.hadoop.hbase.master.MasterServices; import org.apache.hadoop.hbase.master.MetricsMaster; import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos.ProcedureDescription; +import org.apache.hadoop.hbase.security.User; +import org.apache.hadoop.hbase.security.access.AccessChecker; import org.apache.zookeeper.KeeperException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -114,6 +116,10 @@ public class SimpleMasterProcedureManager extends MasterProcedureManager { } @Override + public void checkPermissions(ProcedureDescription desc, AccessChecker accessChecker, User user) + throws IOException {} + + @Override public boolean isProcedureDone(ProcedureDescription desc) throws IOException { return done; }
