From 323f6c556b5fff105568967e142e2959c8c44476 Mon Sep 17 00:00:00 2001 From: zhangduo Date: Tue, 2 Jan 2018 18:04:17 +0800 Subject: [PATCH] HBASE-19634 Add permission check for executeProcedures in AccessController --- .../hbase/coprocessor/RegionServerObserver.java | 14 ++++++ .../hadoop/hbase/regionserver/RSRpcServices.java | 54 ++++++++++++---------- .../regionserver/RegionServerCoprocessorHost.java | 18 ++++++++ .../hbase/security/access/AccessController.java | 26 ++++++----- .../security/access/TestAccessController.java | 18 ++++++-- 5 files changed, 91 insertions(+), 39 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/RegionServerObserver.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/RegionServerObserver.java index c1af3fb..5b751df 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/RegionServerObserver.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/RegionServerObserver.java @@ -126,4 +126,18 @@ public interface RegionServerObserver { default void postClearCompactionQueues( final ObserverContext ctx) throws IOException {} + + /** + * This will be called before executing procedures + * @param ctx the environment to interact with the framework and region server. + */ + default void preExecuteProcedures(ObserverContext ctx) + throws IOException {} + + /** + * This will be called after executing procedures + * @param ctx the environment to interact with the framework and region server. + */ + default void postExecuteProcedures(ObserverContext ctx) + throws IOException {} } 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 e88f70e..695b859 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 @@ -41,7 +41,6 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.LongAdder; - import org.apache.commons.lang3.mutable.MutableObject; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.Path; @@ -142,6 +141,7 @@ import org.apache.hbase.thirdparty.com.google.protobuf.RpcController; import org.apache.hbase.thirdparty.com.google.protobuf.ServiceException; import org.apache.hbase.thirdparty.com.google.protobuf.TextFormat; import org.apache.hbase.thirdparty.com.google.protobuf.UnsafeByteOperations; + import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil; import org.apache.hadoop.hbase.shaded.protobuf.RequestConverter; import org.apache.hadoop.hbase.shaded.protobuf.ResponseConverter; @@ -3454,36 +3454,40 @@ public class RSRpcServices implements HBaseRPCErrorHandler, } @Override + @QosPriority(priority = HConstants.ADMIN_QOS) public ExecuteProceduresResponse executeProcedures(RpcController controller, ExecuteProceduresRequest request) throws ServiceException { - if (request.getOpenRegionCount() > 0) { - for (OpenRegionRequest req : request.getOpenRegionList()) { - openRegion(controller, req); + try { + checkOpen(); + regionServer.getRegionServerCoprocessorHost().preExecuteProcedures(); + if (request.getOpenRegionCount() > 0) { + for (OpenRegionRequest req : request.getOpenRegionList()) { + openRegion(controller, req); + } } - } - if (request.getCloseRegionCount() > 0) { - for (CloseRegionRequest req : request.getCloseRegionList()) { - closeRegion(controller, req); + if (request.getCloseRegionCount() > 0) { + for (CloseRegionRequest req : request.getCloseRegionList()) { + closeRegion(controller, req); + } } - } - if (request.getProcCount() > 0) { - for (RemoteProcedureRequest req : request.getProcList()) { - RSProcedureCallable callable; - try { - callable = - Class.forName(req.getProcClass()).asSubclass(RSProcedureCallable.class).newInstance(); - } catch (Exception e) { - // here we just ignore the error as this should not happen and we do not provide a general - // way to report errors for all types of remote procedure. The procedure will hang at - // master side but after you solve the problem and restart master it will be executed - // again and pass. - LOG.warn("create procedure of type " + req.getProcClass() + " failed, give up", e); - continue; + if (request.getProcCount() > 0) { + for (RemoteProcedureRequest req : request.getProcList()) { + RSProcedureCallable callable; + try { + callable = + Class.forName(req.getProcClass()).asSubclass(RSProcedureCallable.class).newInstance(); + } catch (Exception e) { + regionServer.remoteProcedureComplete(req.getProcId(), e); + continue; + } + callable.init(req.getProcData().toByteArray(), regionServer); + regionServer.executeProcedure(req.getProcId(), callable); } - callable.init(req.getProcData().toByteArray(), regionServer); - regionServer.executeProcedure(req.getProcId(), callable); } + regionServer.getRegionServerCoprocessorHost().postExecuteProcedures(); + return ExecuteProceduresResponse.getDefaultInstance(); + } catch (IOException e) { + throw new ServiceException(e); } - return ExecuteProceduresResponse.getDefaultInstance(); } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerCoprocessorHost.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerCoprocessorHost.java index dc1708c..09617c4 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerCoprocessorHost.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerCoprocessorHost.java @@ -205,6 +205,24 @@ public class RegionServerCoprocessorHost extends }); } + public void preExecuteProcedures() throws IOException { + execOperation(coprocEnvironments.isEmpty() ? null : new RegionServerObserverOperation() { + @Override + public void call(RegionServerObserver observer) throws IOException { + observer.preExecuteProcedures(this); + } + }); + } + + public void postExecuteProcedures() throws IOException { + execOperation(coprocEnvironments.isEmpty() ? null : new RegionServerObserverOperation() { + @Override + public void call(RegionServerObserver observer) throws IOException { + observer.postExecuteProcedures(this); + } + }); + } + /** * Coprocessor environment extension providing access to region server * related services. 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 4e1924f..b29f3e8 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 @@ -1,4 +1,4 @@ -/* +/** * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file * distributed with this work for additional information @@ -22,7 +22,6 @@ import com.google.protobuf.Message; import com.google.protobuf.RpcCallback; import com.google.protobuf.RpcController; import com.google.protobuf.Service; - import java.io.IOException; import java.net.InetAddress; import java.security.PrivilegedExceptionAction; @@ -38,7 +37,6 @@ import java.util.Optional; import java.util.Set; import java.util.TreeMap; import java.util.TreeSet; - import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.ArrayBackedTag; import org.apache.hadoop.hbase.Cell; @@ -121,13 +119,6 @@ import org.apache.hadoop.hbase.security.Superusers; import org.apache.hadoop.hbase.security.User; import org.apache.hadoop.hbase.security.UserProvider; import org.apache.hadoop.hbase.security.access.Permission.Action; -import org.apache.hbase.thirdparty.com.google.common.collect.ArrayListMultimap; -import org.apache.hbase.thirdparty.com.google.common.collect.ImmutableSet; -import org.apache.hbase.thirdparty.com.google.common.collect.ListMultimap; -import org.apache.hbase.thirdparty.com.google.common.collect.Lists; -import org.apache.hbase.thirdparty.com.google.common.collect.MapMaker; -import org.apache.hbase.thirdparty.com.google.common.collect.Maps; -import org.apache.hbase.thirdparty.com.google.common.collect.Sets; import org.apache.hadoop.hbase.snapshot.SnapshotDescriptionUtils; import org.apache.hadoop.hbase.util.ByteRange; import org.apache.hadoop.hbase.util.Bytes; @@ -140,6 +131,14 @@ import org.apache.yetus.audience.InterfaceAudience; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.apache.hbase.thirdparty.com.google.common.collect.ArrayListMultimap; +import org.apache.hbase.thirdparty.com.google.common.collect.ImmutableSet; +import org.apache.hbase.thirdparty.com.google.common.collect.ListMultimap; +import org.apache.hbase.thirdparty.com.google.common.collect.Lists; +import org.apache.hbase.thirdparty.com.google.common.collect.MapMaker; +import org.apache.hbase.thirdparty.com.google.common.collect.Maps; +import org.apache.hbase.thirdparty.com.google.common.collect.Sets; + /** * Provides basic authorization checks for data access and administrative * operations. @@ -430,7 +429,6 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor, private User getActiveUser(ObserverContext ctx) throws IOException { // for non-rpc handling, fallback to system user Optional optionalUser = ctx.getCaller(); - User user; if (optionalUser.isPresent()) { return optionalUser.get(); } @@ -2651,6 +2649,12 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor, } @Override + public void preExecuteProcedures(ObserverContext ctx) + throws IOException { + requirePermission(getActiveUser(ctx), "preExecuteProcedures", Permission.Action.ADMIN); + } + + @Override public void preMoveServersAndTables(ObserverContext ctx, Set
servers, Set tables, String targetGroup) throws IOException { requirePermission(getActiveUser(ctx), "moveServersAndTables", Action.ADMIN); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java index a6ccd59..e4fda6c 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java @@ -1,4 +1,4 @@ -/* +/** * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file * distributed with this work for additional information @@ -30,7 +30,6 @@ import com.google.protobuf.RpcCallback; import com.google.protobuf.RpcController; import com.google.protobuf.Service; import com.google.protobuf.ServiceException; - import java.io.IOException; import java.security.PrivilegedAction; import java.util.ArrayList; @@ -38,7 +37,6 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.List; - import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.FileSystem; @@ -3139,4 +3137,18 @@ public class TestAccessController extends SecureTestUtil { verifyAllowed(action, SUPERUSER, USER_ADMIN); verifyDenied(action, USER_CREATE, USER_RW, USER_RO, USER_NONE, USER_OWNER); } + + @Test + public void testExecuteProcedures() throws Exception { + AccessTestAction action = new AccessTestAction() { + @Override + public Object run() throws Exception { + ACCESS_CONTROLLER.preExecuteProcedures(ObserverContextImpl.createAndPrepare(RSCP_ENV)); + return null; + } + }; + + verifyAllowed(action, SUPERUSER, USER_ADMIN); + verifyDenied(action, USER_CREATE, USER_RW, USER_RO, USER_NONE, USER_OWNER); + } } -- 2.7.4