From c70b6a8ff95b21904a8fbb772ff1f5abdcc81c08 Mon Sep 17 00:00:00 2001 From: Duo Zhang Date: Sat, 22 Aug 2026 17:10:37 +0800 Subject: [PATCH] HBASE-30337 Miscellaneous improvements on rpc checks --- .../hbase/security/access/Permission.java | 5 +- .../access/ShadedAccessControlUtil.java | 16 ++ .../hadoop/hbase/HBaseRpcServicesBase.java | 55 ----- .../hbase/coprocessor/MasterObserver.java | 52 +++- .../hbase/master/MasterCoprocessorHost.java | 32 ++- .../hbase/master/MasterRpcServices.java | 17 +- .../hbase/regionserver/RSRpcServices.java | 56 +++++ .../security/access/AccessController.java | 33 ++- .../access/MasterReadOnlyController.java | 4 +- .../security/access/TestAccessController.java | 62 +++++ .../TestAccessControllerObserverCoverage.java | 222 ++++++++++++++++++ 11 files changed, 480 insertions(+), 74 deletions(-) create mode 100644 hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessControllerObserverCoverage.java diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/Permission.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/Permission.java index ace6f302a198..78a53f725ecf 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/Permission.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/Permission.java @@ -26,6 +26,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import org.apache.hadoop.hbase.HBaseInterfaceAudience; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.io.VersionedWritable; @@ -62,8 +63,8 @@ public byte code() { } } - @InterfaceAudience.Private - protected enum Scope { + @InterfaceAudience.LimitedPrivate(HBaseInterfaceAudience.COPROC) + public enum Scope { GLOBAL('G'), NAMESPACE('N'), TABLE('T'), diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/ShadedAccessControlUtil.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/ShadedAccessControlUtil.java index cf7c797c98ac..740c80dbceb4 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/ShadedAccessControlUtil.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/ShadedAccessControlUtil.java @@ -328,4 +328,20 @@ public static HasUserPermissionsRequest buildHasUserPermissionsRequest(String us } return builder.build(); } + + public static Permission.Scope toPermissionScope(AccessControlProtos.Permission.Type protoType) { + if (protoType == null) { + return null; + } + switch (protoType) { + case Global: + return Permission.Scope.GLOBAL; + case Namespace: + return Permission.Scope.NAMESPACE; + case Table: + return Permission.Scope.TABLE; + default: + return null; + } + } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/HBaseRpcServicesBase.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/HBaseRpcServicesBase.java index d6d277808838..13c06809edac 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/HBaseRpcServicesBase.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/HBaseRpcServicesBase.java @@ -20,10 +20,8 @@ import com.google.errorprone.annotations.RestrictedApi; import java.io.IOException; import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; import java.net.BindException; import java.net.InetSocketAddress; -import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -43,9 +41,6 @@ import org.apache.hadoop.hbase.ipc.RpcServerInterface; import org.apache.hadoop.hbase.namequeues.NamedQueuePayload; import org.apache.hadoop.hbase.namequeues.NamedQueueRecorder; -import org.apache.hadoop.hbase.namequeues.RpcLogDetails; -import org.apache.hadoop.hbase.namequeues.request.NamedQueueGetRequest; -import org.apache.hadoop.hbase.namequeues.response.NamedQueueGetResponse; import org.apache.hadoop.hbase.net.Address; import org.apache.hadoop.hbase.regionserver.RpcSchedulerFactory; import org.apache.hadoop.hbase.security.User; @@ -62,7 +57,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.apache.hbase.thirdparty.com.google.protobuf.ByteString; import org.apache.hbase.thirdparty.com.google.protobuf.Message; import org.apache.hbase.thirdparty.com.google.protobuf.RpcController; import org.apache.hbase.thirdparty.com.google.protobuf.ServiceException; @@ -71,11 +65,8 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.AdminProtos.AdminService; import org.apache.hadoop.hbase.shaded.protobuf.generated.AdminProtos.ClearSlowLogResponseRequest; import org.apache.hadoop.hbase.shaded.protobuf.generated.AdminProtos.ClearSlowLogResponses; -import org.apache.hadoop.hbase.shaded.protobuf.generated.AdminProtos.SlowLogResponseRequest; -import org.apache.hadoop.hbase.shaded.protobuf.generated.AdminProtos.SlowLogResponses; import org.apache.hadoop.hbase.shaded.protobuf.generated.AdminProtos.UpdateConfigurationRequest; import org.apache.hadoop.hbase.shaded.protobuf.generated.AdminProtos.UpdateConfigurationResponse; -import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos; import org.apache.hadoop.hbase.shaded.protobuf.generated.RPCProtos.RequestHeader; import org.apache.hadoop.hbase.shaded.protobuf.generated.RegistryProtos.ClientMetaService; import org.apache.hadoop.hbase.shaded.protobuf.generated.RegistryProtos.GetActiveMasterRequest; @@ -89,7 +80,6 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.RegistryProtos.GetMastersResponseEntry; import org.apache.hadoop.hbase.shaded.protobuf.generated.RegistryProtos.GetMetaRegionLocationsRequest; import org.apache.hadoop.hbase.shaded.protobuf.generated.RegistryProtos.GetMetaRegionLocationsResponse; -import org.apache.hadoop.hbase.shaded.protobuf.generated.TooSlowLog.SlowLogPayload; /** * Base class for Master and RegionServer RpcServices. @@ -414,49 +404,4 @@ public ClearSlowLogResponses clearSlowLogsResponses(final RpcController controll ClearSlowLogResponses.newBuilder().setIsCleaned(slowLogsCleaned).build(); return clearSlowLogResponses; } - - private List getSlowLogPayloads(SlowLogResponseRequest request, - NamedQueueRecorder namedQueueRecorder) { - if (namedQueueRecorder == null) { - return Collections.emptyList(); - } - List slowLogPayloads; - NamedQueueGetRequest namedQueueGetRequest = new NamedQueueGetRequest(); - namedQueueGetRequest.setNamedQueueEvent(RpcLogDetails.SLOW_LOG_EVENT); - namedQueueGetRequest.setSlowLogResponseRequest(request); - NamedQueueGetResponse namedQueueGetResponse = - namedQueueRecorder.getNamedQueueRecords(namedQueueGetRequest); - slowLogPayloads = namedQueueGetResponse != null - ? namedQueueGetResponse.getSlowLogPayloads() - : Collections.emptyList(); - return slowLogPayloads; - } - - @Override - @QosPriority(priority = HConstants.ADMIN_QOS) - public HBaseProtos.LogEntry getLogEntries(RpcController controller, - HBaseProtos.LogRequest request) throws ServiceException { - try { - final String logClassName = request.getLogClassName(); - Class logClass = Class.forName(logClassName).asSubclass(Message.class); - Method method = logClass.getMethod("parseFrom", ByteString.class); - if (logClassName.contains("SlowLogResponseRequest")) { - SlowLogResponseRequest slowLogResponseRequest = - (SlowLogResponseRequest) method.invoke(null, request.getLogMessage()); - final NamedQueueRecorder namedQueueRecorder = this.server.getNamedQueueRecorder(); - final List slowLogPayloads = - getSlowLogPayloads(slowLogResponseRequest, namedQueueRecorder); - SlowLogResponses slowLogResponses = - SlowLogResponses.newBuilder().addAllSlowLogPayloads(slowLogPayloads).build(); - return HBaseProtos.LogEntry.newBuilder() - .setLogClassName(slowLogResponses.getClass().getName()) - .setLogMessage(slowLogResponses.toByteString()).build(); - } - } catch (ClassNotFoundException | NoSuchMethodException | IllegalAccessException - | InvocationTargetException e) { - LOG.error("Error while retrieving log entries.", e); - throw new ServiceException(e); - } - throw new ServiceException("Invalid request params"); - } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java index d0e451508b43..5cfa7fa1a3cc 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java @@ -593,7 +593,7 @@ default void preSplitRegionAction(final ObserverContext c, - final RegionInfo regionInfo) { + final RegionInfo regionInfo) throws IOException { } /** @@ -603,7 +603,7 @@ default void preTruncateRegionAction(final ObserverContext c, - RegionInfo regionInfo) { + RegionInfo regionInfo) throws IOException { } /** @@ -613,7 +613,7 @@ default void preTruncateRegion(final ObserverContext c, - RegionInfo regionInfo) { + RegionInfo regionInfo) throws IOException { } /** @@ -623,7 +623,7 @@ default void postTruncateRegion(final ObserverContext c, - final RegionInfo regionInfo) { + final RegionInfo regionInfo) throws IOException { } /** @@ -1834,12 +1834,34 @@ default void postRevoke(ObserverContext ctx, * @param family the table column family, null if don't get table family permission * @param qualifier the table column qualifier, null if don't get table qualifier permission * @throws IOException if something went wrong + * @deprecated Since 2.5.17,2.6.8, 2.7.0, 3.0.1 and 3.10, will be removed in 4.0.0. Use + * {@link #preGetUserPermissions(ObserverContext, String, String, TableName, byte[], byte[], Permission.Scope)} + * instead. */ + @Deprecated default void preGetUserPermissions(ObserverContext ctx, String userName, String namespace, TableName tableName, byte[] family, byte[] qualifier) throws IOException { } + /** + * Called before getting user permissions. + * @param ctx the coprocessor instance's environment + * @param userName the user name, null if get all user permissions + * @param namespace the namespace, null if don't get namespace permission + * @param tableName the table name, null if don't get table permission + * @param family the table column family, null if don't get table family permission + * @param qualifier the table column qualifier, null if don't get table qualifier permission + * @param permissionScope the scope of permission being requested (GLOBAL, NAMESPACE, or TABLE), + * may be null for backward compatibility + * @throws IOException if something went wrong + */ + default void preGetUserPermissions(ObserverContext ctx, + String userName, String namespace, TableName tableName, byte[] family, byte[] qualifier, + Permission.Scope permissionScope) throws IOException { + preGetUserPermissions(ctx, userName, namespace, tableName, family, qualifier); + } + /** * Called after getting user permissions. * @param ctx the coprocessor instance's environment @@ -1849,12 +1871,34 @@ default void preGetUserPermissions(ObserverContext * @param family the table column family, null if don't get table family permission * @param qualifier the table column qualifier, null if don't get table qualifier permission * @throws IOException if something went wrong + * @deprecated Since 2.5.17,2.6.8, 2.7.0, 3.0.1 and 3.10, will be removed in 4.0.0. Use + * {@link #postGetUserPermissions(ObserverContext, String, String, TableName, byte[], byte[], Permission.Scope)} + * instead. */ + @Deprecated default void postGetUserPermissions(ObserverContext ctx, String userName, String namespace, TableName tableName, byte[] family, byte[] qualifier) throws IOException { } + /** + * Called after getting user permissions. + * @param ctx the coprocessor instance's environment + * @param userName the user name, null if get all user permissions + * @param namespace the namespace, null if don't get namespace permission + * @param tableName the table name, null if don't get table permission + * @param family the table column family, null if don't get table family permission + * @param qualifier the table column qualifier, null if don't get table qualifier permission + * @param permissionScope the scope of permission being requested (GLOBAL, NAMESPACE, or TABLE), + * may be null for backward compatibility + * @throws IOException if something went wrong + */ + default void postGetUserPermissions(ObserverContext ctx, + String userName, String namespace, TableName tableName, byte[] family, byte[] qualifier, + Permission.Scope permissionScope) throws IOException { + postGetUserPermissions(ctx, userName, namespace, tableName, family, qualifier); + } + /* * Called before checking if user has permissions. * @param ctx the coprocessor instance's environment diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java index 00b3e714d1dd..53c4f41c7bc3 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java @@ -873,7 +873,7 @@ public void call(MasterObserver observer) throws IOException { public void preTruncateRegion(RegionInfo regionInfo) throws IOException { execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { @Override - public void call(MasterObserver observer) { + public void call(MasterObserver observer) throws IOException { observer.preTruncateRegion(this, regionInfo); } }); @@ -886,7 +886,7 @@ public void call(MasterObserver observer) { public void postTruncateRegion(RegionInfo regionInfo) throws IOException { execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { @Override - public void call(MasterObserver observer) { + public void call(MasterObserver observer) throws IOException { observer.postTruncateRegion(this, regionInfo); } }); @@ -2085,22 +2085,46 @@ public void call(MasterObserver observer) throws IOException { }); } + /** + * @deprecated Since 2.5.17,2.6.8, 2.7.0, 3.0.1 and 3.10, will be removed in 4.0.0.. Use + * {@link #preGetUserPermissions(String, String, TableName, byte[], byte[], Permission.Scope)} + * instead. + */ + @Deprecated public void preGetUserPermissions(String userName, String namespace, TableName tableName, byte[] family, byte[] qualifier) throws IOException { + preGetUserPermissions(userName, namespace, tableName, family, qualifier, null); + } + + public void preGetUserPermissions(String userName, String namespace, TableName tableName, + byte[] family, byte[] qualifier, Permission.Scope permissionScope) throws IOException { execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { @Override public void call(MasterObserver observer) throws IOException { - observer.preGetUserPermissions(this, userName, namespace, tableName, family, qualifier); + observer.preGetUserPermissions(this, userName, namespace, tableName, family, qualifier, + permissionScope); } }); } + /** + * @deprecated Since 2.5.17,2.6.8, 2.7.0, 3.0.1 and 3.10, will be removed in 4.0.0. Use + * {@link #postGetUserPermissions(String, String, TableName, byte[], byte[], Permission.Scope)} + * instead. + */ + @Deprecated public void postGetUserPermissions(String userName, String namespace, TableName tableName, byte[] family, byte[] qualifier) throws IOException { + postGetUserPermissions(userName, namespace, tableName, family, qualifier, null); + } + + public void postGetUserPermissions(String userName, String namespace, TableName tableName, + byte[] family, byte[] qualifier, Permission.Scope permissionScope) throws IOException { execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { @Override public void call(MasterObserver observer) throws IOException { - observer.postGetUserPermissions(this, userName, namespace, tableName, family, qualifier); + observer.postGetUserPermissions(this, userName, namespace, tableName, family, qualifier, + permissionScope); } }); } 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 bb0e14a5189e..924013d159dc 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 @@ -2770,6 +2770,7 @@ private void checkMasterProcedureExecutor() throws ServiceException { @Override public MasterProtos.AssignsResponse assigns(RpcController controller, MasterProtos.AssignsRequest request) throws ServiceException { + rpcPreCheck("assigns"); checkMasterProcedureExecutor(); final ProcedureExecutor pe = server.getMasterProcedureExecutor(); final AssignmentManager am = server.getAssignmentManager(); @@ -2798,6 +2799,7 @@ public MasterProtos.AssignsResponse assigns(RpcController controller, @Override public MasterProtos.UnassignsResponse unassigns(RpcController controller, MasterProtos.UnassignsRequest request) throws ServiceException { + rpcPreCheck("unassigns"); checkMasterProcedureExecutor(); final ProcedureExecutor pe = server.getMasterProcedureExecutor(); final AssignmentManager am = server.getAssignmentManager(); @@ -2830,6 +2832,7 @@ public MasterProtos.UnassignsResponse unassigns(RpcController controller, @Override public MasterProtos.BypassProcedureResponse bypassProcedure(RpcController controller, MasterProtos.BypassProcedureRequest request) throws ServiceException { + rpcPreCheck("bypassProcedure"); try { LOG.info("{} bypass procedures={}, waitTime={}, override={}, recursive={}", server.getClientIdAuditPrefix(), request.getProcIdList(), request.getWaitTime(), @@ -2847,6 +2850,7 @@ public MasterProtos.BypassProcedureResponse bypassProcedure(RpcController contro public MasterProtos.ScheduleServerCrashProcedureResponse scheduleServerCrashProcedure( RpcController controller, MasterProtos.ScheduleServerCrashProcedureRequest request) throws ServiceException { + rpcPreCheck("scheduleServerCrashProcedure"); List pids = new ArrayList<>(); for (HBaseProtos.ServerName sn : request.getServerNameList()) { ServerName serverName = ProtobufUtil.toServerName(sn); @@ -2865,6 +2869,7 @@ public MasterProtos.ScheduleServerCrashProcedureResponse scheduleServerCrashProc public MasterProtos.ScheduleSCPsForUnknownServersResponse scheduleSCPsForUnknownServers( RpcController controller, MasterProtos.ScheduleSCPsForUnknownServersRequest request) throws ServiceException { + rpcPreCheck("scheduleSCPsForUnknownServers"); List pids = new ArrayList<>(); final Set serverNames = server.getAssignmentManager().getRegionStates() .getRegionStates().stream().map(RegionState::getServerName).collect(Collectors.toSet()); @@ -2993,7 +2998,10 @@ public GetUserPermissionsResponse getUserPermissions(RpcController controller, byte[] cq = request.hasColumnQualifier() ? request.getColumnQualifier().toByteArray() : null; Type permissionType = request.hasType() ? request.getType() : null; - server.getMasterCoprocessorHost().preGetUserPermissions(userName, namespace, table, cf, cq); + Permission.Scope permissionScope = + ShadedAccessControlUtil.toPermissionScope(permissionType); + server.getMasterCoprocessorHost().preGetUserPermissions(userName, namespace, table, cf, cq, + permissionScope); List perms = null; if (permissionType == Type.Table) { @@ -3018,8 +3026,8 @@ public GetUserPermissionsResponse getUserPermissions(RpcController controller, } } - server.getMasterCoprocessorHost().postGetUserPermissions(userName, namespace, table, cf, - cq); + server.getMasterCoprocessorHost().postGetUserPermissions(userName, namespace, table, cf, cq, + permissionScope); AccessControlProtos.GetUserPermissionsResponse response = ShadedAccessControlUtil.buildGetUserPermissionsResponse(perms); return response; @@ -3436,6 +3444,7 @@ public UpdateRSGroupConfigResponse updateRSGroupConfig(RpcController controller, public HBaseProtos.LogEntry getLogEntries(RpcController controller, HBaseProtos.LogRequest request) throws ServiceException { try { + requirePermission("getLogEntries", Permission.Action.ADMIN); final String logClassName = request.getLogClassName(); Class logClass = Class.forName(logClassName).asSubclass(Message.class); Method method = logClass.getMethod("parseFrom", ByteString.class); @@ -3457,7 +3466,7 @@ public HBaseProtos.LogEntry getLogEntries(RpcController controller, .setLogMessage(balancerRejectionsResponse.toByteString()).build(); } } catch (ClassNotFoundException | NoSuchMethodException | IllegalAccessException - | InvocationTargetException e) { + | InvocationTargetException | IOException e) { LOG.error("Error while retrieving log entries.", e); throw new ServiceException(e); } 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 39968843c4ab..c45e1a36fafe 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 @@ -21,6 +21,8 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.io.UncheckedIOException; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; import java.net.BindException; import java.net.InetAddress; import java.net.InetSocketAddress; @@ -104,6 +106,10 @@ import org.apache.hadoop.hbase.ipc.ServerNotRunningYetException; import org.apache.hadoop.hbase.ipc.ServerRpcController; import org.apache.hadoop.hbase.monitoring.ThreadLocalServerSideScanMetrics; +import org.apache.hadoop.hbase.namequeues.NamedQueueRecorder; +import org.apache.hadoop.hbase.namequeues.RpcLogDetails; +import org.apache.hadoop.hbase.namequeues.request.NamedQueueGetRequest; +import org.apache.hadoop.hbase.namequeues.response.NamedQueueGetResponse; import org.apache.hadoop.hbase.net.Address; import org.apache.hadoop.hbase.procedure2.RSProcedureCallable; import org.apache.hadoop.hbase.quotas.ActivePolicyEnforcement; @@ -195,6 +201,8 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.AdminProtos.ReplicateWALEntryResponse; import org.apache.hadoop.hbase.shaded.protobuf.generated.AdminProtos.RollWALWriterRequest; import org.apache.hadoop.hbase.shaded.protobuf.generated.AdminProtos.RollWALWriterResponse; +import org.apache.hadoop.hbase.shaded.protobuf.generated.AdminProtos.SlowLogResponseRequest; +import org.apache.hadoop.hbase.shaded.protobuf.generated.AdminProtos.SlowLogResponses; import org.apache.hadoop.hbase.shaded.protobuf.generated.AdminProtos.StopServerRequest; import org.apache.hadoop.hbase.shaded.protobuf.generated.AdminProtos.StopServerResponse; import org.apache.hadoop.hbase.shaded.protobuf.generated.AdminProtos.UpdateFavoredNodesRequest; @@ -234,6 +242,7 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.ClientProtos.ScanResponse; import org.apache.hadoop.hbase.shaded.protobuf.generated.ClusterStatusProtos; import org.apache.hadoop.hbase.shaded.protobuf.generated.ClusterStatusProtos.RegionLoad; +import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos; import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos.BooleanMsg; import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos.EmptyMsg; import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos.ManagedKeyEntryRequest; @@ -246,6 +255,7 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.QuotaProtos.GetSpaceQuotaSnapshotsResponse; import org.apache.hadoop.hbase.shaded.protobuf.generated.QuotaProtos.GetSpaceQuotaSnapshotsResponse.TableQuotaSnapshot; import org.apache.hadoop.hbase.shaded.protobuf.generated.RegistryProtos.ClientMetaService; +import org.apache.hadoop.hbase.shaded.protobuf.generated.TooSlowLog.SlowLogPayload; import org.apache.hadoop.hbase.shaded.protobuf.generated.WALProtos.BulkLoadDescriptor; import org.apache.hadoop.hbase.shaded.protobuf.generated.WALProtos.CompactionDescriptor; import org.apache.hadoop.hbase.shaded.protobuf.generated.WALProtos.FlushDescriptor; @@ -4118,4 +4128,50 @@ RegionScannerContext checkQuotaAndGetRegionScannerContext(ScanRequest request, Pair pair = newRegionScanner(request, region, builder); return new RegionScannerContext(pair.getFirst(), pair.getSecond(), quota); } + + private List getSlowLogPayloads(SlowLogResponseRequest request, + NamedQueueRecorder namedQueueRecorder) { + if (namedQueueRecorder == null) { + return Collections.emptyList(); + } + List slowLogPayloads; + NamedQueueGetRequest namedQueueGetRequest = new NamedQueueGetRequest(); + namedQueueGetRequest.setNamedQueueEvent(RpcLogDetails.SLOW_LOG_EVENT); + namedQueueGetRequest.setSlowLogResponseRequest(request); + NamedQueueGetResponse namedQueueGetResponse = + namedQueueRecorder.getNamedQueueRecords(namedQueueGetRequest); + slowLogPayloads = namedQueueGetResponse != null + ? namedQueueGetResponse.getSlowLogPayloads() + : Collections.emptyList(); + return slowLogPayloads; + } + + @Override + @QosPriority(priority = HConstants.ADMIN_QOS) + public HBaseProtos.LogEntry getLogEntries(RpcController controller, + HBaseProtos.LogRequest request) throws ServiceException { + try { + requirePermission("getLogEntries", Permission.Action.ADMIN); + final String logClassName = request.getLogClassName(); + Class logClass = Class.forName(logClassName).asSubclass(Message.class); + Method method = logClass.getMethod("parseFrom", ByteString.class); + if (logClassName.contains("SlowLogResponseRequest")) { + SlowLogResponseRequest slowLogResponseRequest = + (SlowLogResponseRequest) method.invoke(null, request.getLogMessage()); + final NamedQueueRecorder namedQueueRecorder = this.server.getNamedQueueRecorder(); + final List slowLogPayloads = + getSlowLogPayloads(slowLogResponseRequest, namedQueueRecorder); + SlowLogResponses slowLogResponses = + SlowLogResponses.newBuilder().addAllSlowLogPayloads(slowLogPayloads).build(); + return HBaseProtos.LogEntry.newBuilder() + .setLogClassName(slowLogResponses.getClass().getName()) + .setLogMessage(slowLogResponses.toByteString()).build(); + } + } catch (ClassNotFoundException | NoSuchMethodException | IllegalAccessException + | InvocationTargetException | IOException e) { + LOG.error("Error while retrieving log entries.", e); + throw new ServiceException(e); + } + throw new ServiceException("Invalid request params"); + } } 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 3b45e0175cc3..13c8639c33d8 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 @@ -874,6 +874,13 @@ public Void run() throws Exception { }); } + @Override + public void preTruncateRegion(final ObserverContext ctx, + final RegionInfo regionInfo) throws IOException { + requirePermission(ctx, "truncateRegion", regionInfo.getTable(), null, null, Action.ADMIN, + Action.CREATE); + } + @Override public TableDescriptor preModifyTable(ObserverContext c, TableName tableName, TableDescriptor currentDesc, TableDescriptor newDesc) throws IOException { @@ -1990,7 +1997,10 @@ public void getUserPermissions(RpcController controller, request.hasColumnFamily() ? request.getColumnFamily().toByteArray() : null; final byte[] cq = request.hasColumnQualifier() ? request.getColumnQualifier().toByteArray() : null; - preGetUserPermissions(caller, userName, namespace, table, cf, cq); + AccessControlProtos.Permission.Type protoType = + request.hasType() ? request.getType() : null; + Permission.Scope permissionScope = ShadedAccessControlUtil.toPermissionScope(protoType); + preGetUserPermissions(caller, userName, namespace, table, cf, cq, permissionScope); GetUserPermissionsRequest getUserPermissionsRequest = null; if (request.getType() == AccessControlProtos.Permission.Type.Table) { getUserPermissionsRequest = GetUserPermissionsRequest.newBuilder(table).withFamily(cf) @@ -2418,9 +2428,26 @@ private void preGrantOrRevoke(User caller, String request, UserPermission userPe @Override public void preGetUserPermissions(ObserverContext ctx, - String userName, String namespace, TableName tableName, byte[] family, byte[] qualifier) + String userName, String namespace, TableName tableName, byte[] family, byte[] qualifier, + Permission.Scope permissionScope) throws IOException { + preGetUserPermissions(getActiveUser(ctx), userName, namespace, tableName, family, qualifier, + permissionScope); + } + + private void preGetUserPermissions(User caller, String userName, String namespace, + TableName tableName, byte[] family, byte[] qualifier, Permission.Scope permissionScope) throws IOException { - preGetUserPermissions(getActiveUser(ctx), userName, namespace, tableName, family, qualifier); + if (permissionScope == Permission.Scope.TABLE) { + accessChecker.requirePermission(caller, "getUserPermissions", tableName, family, qualifier, + userName, Action.ADMIN); + } else if (permissionScope == Permission.Scope.NAMESPACE) { + accessChecker.requireNamespacePermission(caller, "getUserPermissions", namespace, userName, + Action.ADMIN); + } else if (permissionScope == Permission.Scope.GLOBAL) { + accessChecker.requirePermission(caller, "getUserPermissions", userName, Action.ADMIN); + } else { + preGetUserPermissions(caller, userName, namespace, tableName, family, qualifier); + } } private void preGetUserPermissions(User caller, String userName, String namespace, diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/MasterReadOnlyController.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/MasterReadOnlyController.java index e6efcac856e7..a1feceb4d8ec 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/MasterReadOnlyController.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/MasterReadOnlyController.java @@ -169,7 +169,7 @@ public void preSplitRegionAfterMETAAction(ObserverContext c, - RegionInfo regionInfo) { + RegionInfo regionInfo) throws IOException { try { internalReadOnlyGuard(); } catch (IOException e) { @@ -181,7 +181,7 @@ public void preTruncateRegion(ObserverContext c, @Override public void preTruncateRegionAction(ObserverContext c, - RegionInfo regionInfo) { + RegionInfo regionInfo) throws IOException { try { internalReadOnlyGuard(); } catch (IOException e) { 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 8a32ca8c0d76..2935f5565bc9 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 @@ -3418,6 +3418,68 @@ public void testGetUserPermissions() throws Throwable { } } + @Test + public void testGetUserPermissionsScopeBasedAuthorization() throws Throwable { + // Regression test: authorization must be based on permissionScope (derived from + // the type field), not on the tableName field. A user with only table-level ADMIN + // should not be able to request global permissions by passing tableName with + // permissionScope=GLOBAL. + AccessTestAction tablePermWithGlobalScope = new AccessTestAction() { + @Override + public Object run() throws Exception { + ACCESS_CONTROLLER.preGetUserPermissions(ObserverContextImpl.createAndPrepare(CP_ENV), null, + null, TEST_TABLE, null, null, Permission.Scope.GLOBAL); + return null; + } + }; + + // USER_ADMIN has global ADMIN, should be allowed + verifyAllowed(tablePermWithGlobalScope, SUPERUSER, USER_ADMIN); + // Users with only table-level ADMIN should be denied + verifyDenied(tablePermWithGlobalScope, USER_OWNER, USER_ADMIN_CF, USER_CREATE, USER_RW, USER_RO, + USER_NONE); + + AccessTestAction tablePermWithNamespaceScope = new AccessTestAction() { + @Override + public Object run() throws Exception { + ACCESS_CONTROLLER.preGetUserPermissions(ObserverContextImpl.createAndPrepare(CP_ENV), null, + TEST_TABLE.getNamespaceAsString(), TEST_TABLE, null, null, Permission.Scope.NAMESPACE); + return null; + } + }; + + // Users without namespace ADMIN should be denied + verifyDenied(tablePermWithNamespaceScope, USER_OWNER, USER_ADMIN_CF, USER_CREATE, USER_RW, + USER_RO, USER_NONE); + + AccessTestAction tablePermWithTableScope = new AccessTestAction() { + @Override + public Object run() throws Exception { + ACCESS_CONTROLLER.preGetUserPermissions(ObserverContextImpl.createAndPrepare(CP_ENV), null, + null, TEST_TABLE, null, null, Permission.Scope.TABLE); + return null; + } + }; + + // Users with table ADMIN should be allowed + verifyAllowed(tablePermWithTableScope, SUPERUSER, USER_ADMIN, USER_OWNER); + verifyDenied(tablePermWithTableScope, USER_ADMIN_CF, USER_CREATE, USER_RW, USER_RO, USER_NONE); + + AccessTestAction globalPermWithNullScope = new AccessTestAction() { + @Override + public Object run() throws Exception { + ACCESS_CONTROLLER.preGetUserPermissions(ObserverContextImpl.createAndPrepare(CP_ENV), null, + null, null, null, null, null); + return null; + } + }; + + // null scope falls back to old behavior (no tableName -> require global ADMIN) + verifyAllowed(globalPermWithNullScope, SUPERUSER, USER_ADMIN); + verifyDenied(globalPermWithNullScope, USER_OWNER, USER_ADMIN_CF, USER_CREATE, USER_RW, USER_RO, + USER_NONE); + } + @Test public void testHasPermission() throws Throwable { Connection conn = null; diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessControllerObserverCoverage.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessControllerObserverCoverage.java new file mode 100644 index 000000000000..dafe9cf065ee --- /dev/null +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessControllerObserverCoverage.java @@ -0,0 +1,222 @@ +/* + * 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 + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hbase.security.access; + +import static org.junit.jupiter.api.Assertions.fail; + +import java.lang.reflect.Method; +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; +import java.util.TreeSet; +import java.util.stream.Collectors; +import org.apache.hadoop.hbase.coprocessor.BulkLoadObserver; +import org.apache.hadoop.hbase.coprocessor.EndpointObserver; +import org.apache.hadoop.hbase.coprocessor.MasterObserver; +import org.apache.hadoop.hbase.coprocessor.RegionObserver; +import org.apache.hadoop.hbase.coprocessor.RegionServerObserver; +import org.apache.hadoop.hbase.testclassification.SecurityTests; +import org.apache.hadoop.hbase.testclassification.SmallTests; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; + +/** + * Verifies that AccessController implements every security-relevant method declared in the five + * observer interfaces it claims to implement: MasterObserver, RegionObserver, RegionServerObserver, + * EndpointObserver, BulkLoadObserver. + *

+ * If a new hook is added to any of these interfaces and AccessController does not override it, the + * default no-op implementation will silently skip the permission check — a potential privilege + * escalation. This test catches that at build time. + *

+ * Skipped methods are determined by two mechanisms: + *

    + *
  • Pattern rules — methods matching these patterns are always safe to skip: + *
      + *
    • {@code post*} — post-operation notifications; the operation has already been authorized and + * executed.
      • + *
    • {@code pre*Action} — procedure-level action hooks; authorization happens at the RPC layer in + * the corresponding {@code pre*} hook.
      • + *
    + *
    • + *
  • Explicit whitelist — methods that don't match the above rules but are still safe to + * skip (internal lifecycle hooks, deprecated overloads with default delegation, read-only queries). + * Each entry has a justification comment.
    • + *
+ */ +@Tag(SecurityTests.TAG) +@Tag(SmallTests.TAG) +public class TestAccessControllerObserverCoverage { + + /** + * Explicit whitelist for methods that don't match the pattern rules but are intentionally not + * overridden in AccessController. + *

+ * Use simple method name (covers all overloads) or full signature key + * "methodName(ParamType1,ParamType2,..." using simple class names. + */ + private static final Set WHITELIST = new HashSet<>(Arrays.asList( + + // --- Internal lifecycle hooks (not client-facing RPCs) --- + // Store file / WAL internal hooks + "preStoreFileReaderOpen", "preStoreScannerOpen", "preCommitStoreFile", "preReplayWALs", + "preWALRestore", + // Master internal + "preMasterStoreFlush", + // Lifecycle markers (not triggered by client RPC) + "preMasterInitialization", "preCreateTableRegionsInfos", + // --- Read-only query hooks (no mutation, no authorization needed) --- + "preGetClusterMetrics", "preGetTableNames", "preListNamespaceDescriptors", "preListNamespaces", + + // --- Deprecated overloads: interface default delegates to non-deprecated --- + // prePut(3-arg) delegates to prePut(4-arg Durability) + "prePut(org.apache.hadoop.hbase.coprocessor.ObserverContext,org.apache.hadoop.hbase.client.Put,org.apache.hadoop.hbase.wal.WALEdit)", + // preDelete(3-arg) delegates to preDelete(4-arg Durability) + "preDelete(org.apache.hadoop.hbase.coprocessor.ObserverContext,org.apache.hadoop.hbase.client.Delete,org.apache.hadoop.hbase.wal.WALEdit)", + // preAppend(3-arg) delegates to preAppend(2-arg deprecated) + "preAppend(org.apache.hadoop.hbase.coprocessor.ObserverContext,org.apache.hadoop.hbase.client.Append,org.apache.hadoop.hbase.wal.WALEdit)", + // preAppendAfterRowLock(2-arg) delegates to preAppendAfterRowLock(1-arg deprecated) + "preAppendAfterRowLock(org.apache.hadoop.hbase.coprocessor.ObserverContext,org.apache.hadoop.hbase.client.Append)", + // preIncrement(3-arg) delegates to preIncrement(2-arg deprecated) + "preIncrement(org.apache.hadoop.hbase.coprocessor.ObserverContext,org.apache.hadoop.hbase.client.Increment,org.apache.hadoop.hbase.wal.WALEdit)", + // preIncrementAfterRowLock(2-arg) delegates to preIncrementAfterRowLock(1-arg deprecated) + "preIncrementAfterRowLock(org.apache.hadoop.hbase.coprocessor.ObserverContext,org.apache.hadoop.hbase.client.Increment)", + // preCheckAndMutate delegates to preCheckAndPut/preCheckAndDelete + "preCheckAndMutate(org.apache.hadoop.hbase.coprocessor.ObserverContext,org.apache.hadoop.hbase.client.CheckAndMutate,org.apache.hadoop.hbase.client.CheckAndMutateResult)", + // preCheckAndMutateAfterRowLock delegates to + // preCheckAndPutAfterRowLock/preCheckAndDeleteAfterRowLock + "preCheckAndMutateAfterRowLock(org.apache.hadoop.hbase.coprocessor.ObserverContext,org.apache.hadoop.hbase.client.CheckAndMutate,org.apache.hadoop.hbase.client.CheckAndMutateResult)", + // Filter-based CheckAnd* are deprecated; framework uses byte[] overloads + // Note: Class.getName() returns "[B" for byte[], not "byte[]" + "preCheckAndPut(org.apache.hadoop.hbase.coprocessor.ObserverContext,[B,org.apache.hadoop.hbase.filter.Filter,org.apache.hadoop.hbase.client.Put,boolean)", + "preCheckAndPutAfterRowLock(org.apache.hadoop.hbase.coprocessor.ObserverContext,[B,org.apache.hadoop.hbase.filter.Filter,org.apache.hadoop.hbase.client.Put,boolean)", + "preCheckAndDelete(org.apache.hadoop.hbase.coprocessor.ObserverContext,[B,org.apache.hadoop.hbase.filter.Filter,org.apache.hadoop.hbase.client.Delete,boolean)", + "preCheckAndDeleteAfterRowLock(org.apache.hadoop.hbase.coprocessor.ObserverContext,[B,org.apache.hadoop.hbase.filter.Filter,org.apache.hadoop.hbase.client.Delete,boolean)", + // Deprecated preGetUserPermissions(6-arg) delegates to preGetUserPermissions(7-arg with Scope) + "preGetUserPermissions(ObserverContext,String,String,TableName,byte[],byte[])", + // Deprecated WAL-append / timestamp hooks + "prePrepareTimeStampForDeleteVersion", "preWALAppend", + // --- Replication sink is a trusted internal cluster-to-cluster operation --- + "preReplicationSinkBatchMutate")); + + private static final Class[] OBSERVER_INTERFACES = + { MasterObserver.class, RegionObserver.class, RegionServerObserver.class, + EndpointObserver.class, BulkLoadObserver.class }; + + /** + * Returns true if the method matches a pattern rule that makes it safe to skip without an + * explicit whitelist entry. + */ + private static boolean matchesSkipPattern(Class iface, Method m) { + String name = m.getName(); + // All post* methods are post-operation notifications. + // Permission checks must happen before the operation, not after. + if (name.startsWith("post")) { + return true; + } + // *Action suffix on pre* hooks are procedure-level callbacks. + // Authorization is done at the RPC layer in the corresponding pre* hook. + if (name.endsWith("Action")) { + return true; + } + // RegionObserver internal storage hooks: flush, compaction, in-memory + // compaction. These are sub-step callbacks within a region storage + // operation. The region-level entry hook (preFlush, preCompact) already + // handles authorization in AccessController. + if ( + iface == RegionObserver.class && (name.startsWith("preFlush") || name.startsWith("preCompact") + || name.startsWith("preMemStore")) + ) { + return true; + } + return false; + } + + private static String methodSignatureKey(Method m) { + String paramTypes = Arrays.stream(m.getParameterTypes()).map(Class::getSimpleName) + .collect(Collectors.joining(",")); + return m.getName() + "(" + paramTypes + ")"; + } + + private static String fullMethodSignatureKey(Method m) { + String paramTypes = + Arrays.stream(m.getParameterTypes()).map(Class::getName).collect(Collectors.joining(",")); + return m.getName() + "(" + paramTypes + ")"; + } + + private static boolean isMethodImplemented(Class implClass, Method ifaceMethod) { + Class clazz = implClass; + while (clazz != null) { + for (Method m : clazz.getDeclaredMethods()) { + if ( + m.getName().equals(ifaceMethod.getName()) + && Arrays.equals(m.getParameterTypes(), ifaceMethod.getParameterTypes()) + ) { + return true; + } + } + clazz = clazz.getSuperclass(); + } + return false; + } + + @Test + public void testAllObserverMethodsAreImplemented() { + Set missing = new TreeSet<>(); + + for (Class iface : OBSERVER_INTERFACES) { + for (Method m : iface.getMethods()) { + if (m.getDeclaringClass() == Object.class) { + continue; + } + if (!m.getDeclaringClass().equals(iface)) { + continue; + } + + if (matchesSkipPattern(iface, m)) { + continue; + } + + String simpleName = m.getName(); + String simpleKey = methodSignatureKey(m); + String fullKey = fullMethodSignatureKey(m); + + if ( + WHITELIST.contains(simpleName) || WHITELIST.contains(simpleKey) + || WHITELIST.contains(fullKey) + ) { + continue; + } + + if (!isMethodImplemented(AccessController.class, m)) { + missing.add(" " + iface.getSimpleName() + "." + simpleKey); + } + } + } + + if (!missing.isEmpty()) { + StringBuilder sb = new StringBuilder(); + sb.append("AccessController does not implement the following observer methods.\n"); + sb.append("Either override them in AccessController (with permission checks),\n"); + sb.append("or add them to the WHITELIST with a justification comment.\n\n"); + sb.append("Missing methods:\n"); + missing.forEach(m -> sb.append(m).append("\n")); + fail(sb.toString()); + } + } +}