[jira] [Updated] (HBASE-27528) log duplication issues in MasterRpcServices

2023-03-17 Thread Beibei Zhao (Jira)


 [ 
https://issues.apache.org/jira/browse/HBASE-27528?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Beibei Zhao updated HBASE-27528:

Description: 
MasterRpcServices record audit log in privileged operations (grant, revoke) and 
vital apis like "execMasterService".
{code:java}
  public RevokeResponse revoke(RpcController controller, RevokeRequest request)
throws ServiceException {
try {
  ..
server.cpHost.preRevoke(userPermission); // has audit log in 
AccessChecker
   .. // removeUserPermission
User caller = RpcServer.getRequestUser().orElse(null);
if (AUDITLOG.isTraceEnabled()) {
  // audit log should record all permission changes
  String remoteAddress = 
RpcServer.getRemoteAddress().map(InetAddress::toString).orElse("");
  AUDITLOG.trace("User {} (remote address: {}) revoked permission {}", 
caller,
remoteAddress, userPermission);
}
..
  }
{code}
but I found a path from *server.cpHost.preRevoke(userPermission);* to 
{*}AccessChecker audit log{*}, which caused {*}log duplication{*}.
*_+grant/revoke -> AccessController.preGrant/Revoke -> preGrantOrRevoke -> 
AccessChecker.requireGlobalPermission/... -> logResult+_*
{code:java}
public void requireGlobalPermission(User user, String request, Action perm, 
String namespace)
throws IOException {
AuthResult authResult;
if (authManager.authorizeUserGlobal(user, perm)) {
  authResult = AuthResult.allow(request, "Global check allowed", user, 
perm, null);
  authResult.getParams().setNamespace(namespace);
  logResult(authResult);
} else {
  authResult = AuthResult.deny(request, "Global check failed", user, perm, 
null);
  authResult.getParams().setNamespace(namespace);
  logResult(authResult);
  throw new AccessDeniedException(
"Insufficient permissions for user '" + (user != null ? 
user.getShortName() : "null")
  + "' (global, action=" + perm.toString() + ")");
}
  }

  public static void logResult(AuthResult result) {
if (AUDITLOG.isTraceEnabled()) {
  User user = result.getUser();
  UserGroupInformation ugi = user != null ? user.getUGI() : null;
  AUDITLOG.trace(
"Access {} for user {}; reason: {}; remote address: {}; request: {}; 
context: {};"
  + "auth method: {}",
(result.isAllowed() ? "allowed" : "denied"),
(user != null ? user.getShortName() : "UNKNOWN"), result.getReason(),
RpcServer.getRemoteAddress().map(InetAddress::toString).orElse(""), 
result.getRequest(),
result.toContextString(), ugi != null ? ugi.getAuthenticationMethod() : 
"UNKNOWN");
}
  }
{code}
Since *AccessChecker* integrates auditlogs for permission check, I'll delete 
the log in MasterRpcServices.

There must be more problems like this, I' ll check it later and commit the code.

-There are many "write" operations like "deleteTable", which may cause security 
problems, should also record an audit log.-

  was:
MasterRpcServices record audit log in privileged operations (grant, revoke) and 
vital apis like "execMasterService".
{code:java}
  public RevokeResponse revoke(RpcController controller, RevokeRequest request)
throws ServiceException {
try {
  ..
server.cpHost.preRevoke(userPermission); // has audit log in 
AccessChecker
   .. // removeUserPermission
User caller = RpcServer.getRequestUser().orElse(null);
if (AUDITLOG.isTraceEnabled()) {
  // audit log should record all permission changes
  String remoteAddress = 
RpcServer.getRemoteAddress().map(InetAddress::toString).orElse("");
  AUDITLOG.trace("User {} (remote address: {}) revoked permission {}", 
caller,
remoteAddress, userPermission);
}
..
  }
{code}
but I found a path from *server.cpHost.preRevoke(userPermission);* to 
{*}AccessChecker audit log{*}, which caused {*}log duplication{*}.
{code:java}
public void requireGlobalPermission(User user, String request, Action perm, 
String namespace)
throws IOException {
AuthResult authResult;
if (authManager.authorizeUserGlobal(user, perm)) {
  authResult = AuthResult.allow(request, "Global check allowed", user, 
perm, null);
  authResult.getParams().setNamespace(namespace);
  logResult(authResult);
} else {
  authResult = AuthResult.deny(request, "Global check failed", user, perm, 
null);
  authResult.getParams().setNamespace(namespace);
  logResult(authResult);
  throw new AccessDeniedException(
"Insufficient permissions for user '" + (user != null ? 
user.getShortName() : "null")
  + "' (global, action=" + perm.toString() + ")");
}
  }

  public static void logResult(AuthResult result) {
if (AUDITLOG.isTraceEnabled()) {
  User user = result.getUser();
  UserGroupInformation ugi = user != null ? user.getUGI() 

[jira] [Updated] (HBASE-27528) log duplication issues in MasterRpcServices

2023-01-08 Thread Beibei Zhao (Jira)


 [ 
https://issues.apache.org/jira/browse/HBASE-27528?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Beibei Zhao updated HBASE-27528:

Description: 
MasterRpcServices record audit log in privileged operations (grant, revoke) and 
vital apis like "execMasterService".
{code:java}
  public RevokeResponse revoke(RpcController controller, RevokeRequest request)
throws ServiceException {
try {
  ..
server.cpHost.preRevoke(userPermission); // has audit log in 
AccessChecker
   .. // removeUserPermission
User caller = RpcServer.getRequestUser().orElse(null);
if (AUDITLOG.isTraceEnabled()) {
  // audit log should record all permission changes
  String remoteAddress = 
RpcServer.getRemoteAddress().map(InetAddress::toString).orElse("");
  AUDITLOG.trace("User {} (remote address: {}) revoked permission {}", 
caller,
remoteAddress, userPermission);
}
..
  }
{code}
but I found a path from *server.cpHost.preRevoke(userPermission);* to 
{*}AccessChecker audit log{*}, which caused {*}log duplication{*}.
{code:java}
public void requireGlobalPermission(User user, String request, Action perm, 
String namespace)
throws IOException {
AuthResult authResult;
if (authManager.authorizeUserGlobal(user, perm)) {
  authResult = AuthResult.allow(request, "Global check allowed", user, 
perm, null);
  authResult.getParams().setNamespace(namespace);
  logResult(authResult);
} else {
  authResult = AuthResult.deny(request, "Global check failed", user, perm, 
null);
  authResult.getParams().setNamespace(namespace);
  logResult(authResult);
  throw new AccessDeniedException(
"Insufficient permissions for user '" + (user != null ? 
user.getShortName() : "null")
  + "' (global, action=" + perm.toString() + ")");
}
  }

  public static void logResult(AuthResult result) {
if (AUDITLOG.isTraceEnabled()) {
  User user = result.getUser();
  UserGroupInformation ugi = user != null ? user.getUGI() : null;
  AUDITLOG.trace(
"Access {} for user {}; reason: {}; remote address: {}; request: {}; 
context: {};"
  + "auth method: {}",
(result.isAllowed() ? "allowed" : "denied"),
(user != null ? user.getShortName() : "UNKNOWN"), result.getReason(),
RpcServer.getRemoteAddress().map(InetAddress::toString).orElse(""), 
result.getRequest(),
result.toContextString(), ugi != null ? ugi.getAuthenticationMethod() : 
"UNKNOWN");
}
  }
{code}
Since *AccessChecker* integrates auditlogs for permission check, I'll delete 
the log in MasterRpcServices.

There must be more problems like this, I' ll check it later and commit the code.

-There are many "write" operations like "deleteTable", which may cause security 
problems, should also record an audit log.-

  was:
MasterRpcServices record audit log in privileged operations (grant, revoke) and 
vital apis like "execMasterService".

 
{code:java}
public ClientProtos.CoprocessorServiceResponse execMasterService(final 
RpcController controller,
..
  String remoteAddress = 
RpcServer.getRemoteAddress().map(InetAddress::toString).orElse("");
  User caller = RpcServer.getRequestUser().orElse(null);
  AUDITLOG.info("User {} (remote address: {}) master service request for 
{}.{}", caller,
remoteAddress, serviceName, methodName);

  return CoprocessorRpcUtils.getResponse(execResult, 
HConstants.EMPTY_BYTE_ARRAY);
} catch (IOException ie) {
  throw new ServiceException(ie);
}
  }
{code}

There are many "write" operations like "deleteTable", which may cause security 
problems, should also record an audit log.

{code:java}
  public DeleteTableResponse deleteTable(RpcController controller, 
DeleteTableRequest request)
throws ServiceException {
try {
  long procId = 
server.deleteTable(ProtobufUtil.toTableName(request.getTableName()),
request.getNonceGroup(), request.getNonce());
  // an audit log is required here.
  return DeleteTableResponse.newBuilder().setProcId(procId).build();
} catch (IOException ioe) {
  throw new ServiceException(ioe);
}
  }
{code}



> log duplication issues in MasterRpcServices
> ---
>
> Key: HBASE-27528
> URL: https://issues.apache.org/jira/browse/HBASE-27528
> Project: HBase
>  Issue Type: Bug
>  Components: logging, master, rpc, security
>Reporter: Beibei Zhao
>Priority: Major
>
> MasterRpcServices record audit log in privileged operations (grant, revoke) 
> and vital apis like "execMasterService".
> {code:java}
>   public RevokeResponse revoke(RpcController controller, RevokeRequest 
> request)
> throws ServiceException {
> try {
>   ..
> server.cpHost.preRevoke(userPermission); // 

[jira] [Updated] (HBASE-27528) log duplication issues in MasterRpcServices

2023-01-08 Thread Beibei Zhao (Jira)


 [ 
https://issues.apache.org/jira/browse/HBASE-27528?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Beibei Zhao updated HBASE-27528:

Issue Type: Bug  (was: Improvement)

> log duplication issues in MasterRpcServices
> ---
>
> Key: HBASE-27528
> URL: https://issues.apache.org/jira/browse/HBASE-27528
> Project: HBase
>  Issue Type: Bug
>  Components: logging, master, rpc, security
>Reporter: Beibei Zhao
>Priority: Major
>
> MasterRpcServices record audit log in privileged operations (grant, revoke) 
> and vital apis like "execMasterService".
>  
> {code:java}
> public ClientProtos.CoprocessorServiceResponse execMasterService(final 
> RpcController controller,
> ..
>   String remoteAddress = 
> RpcServer.getRemoteAddress().map(InetAddress::toString).orElse("");
>   User caller = RpcServer.getRequestUser().orElse(null);
>   AUDITLOG.info("User {} (remote address: {}) master service request for 
> {}.{}", caller,
> remoteAddress, serviceName, methodName);
>   return CoprocessorRpcUtils.getResponse(execResult, 
> HConstants.EMPTY_BYTE_ARRAY);
> } catch (IOException ie) {
>   throw new ServiceException(ie);
> }
>   }
> {code}
> There are many "write" operations like "deleteTable", which may cause 
> security problems, should also record an audit log.
> {code:java}
>   public DeleteTableResponse deleteTable(RpcController controller, 
> DeleteTableRequest request)
> throws ServiceException {
> try {
>   long procId = 
> server.deleteTable(ProtobufUtil.toTableName(request.getTableName()),
> request.getNonceGroup(), request.getNonce());
>   // an audit log is required here.
>   return DeleteTableResponse.newBuilder().setProcId(procId).build();
> } catch (IOException ioe) {
>   throw new ServiceException(ioe);
> }
>   }
> {code}



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


[jira] [Updated] (HBASE-27528) log duplication issues in MasterRpcServices

2023-01-08 Thread Beibei Zhao (Jira)


 [ 
https://issues.apache.org/jira/browse/HBASE-27528?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Beibei Zhao updated HBASE-27528:

Summary: log duplication issues in MasterRpcServices  (was: Add audit logs 
in MasterRpcServices)

> log duplication issues in MasterRpcServices
> ---
>
> Key: HBASE-27528
> URL: https://issues.apache.org/jira/browse/HBASE-27528
> Project: HBase
>  Issue Type: Improvement
>  Components: logging, master, rpc, security
>Reporter: Beibei Zhao
>Priority: Major
>
> MasterRpcServices record audit log in privileged operations (grant, revoke) 
> and vital apis like "execMasterService".
>  
> {code:java}
> public ClientProtos.CoprocessorServiceResponse execMasterService(final 
> RpcController controller,
> ..
>   String remoteAddress = 
> RpcServer.getRemoteAddress().map(InetAddress::toString).orElse("");
>   User caller = RpcServer.getRequestUser().orElse(null);
>   AUDITLOG.info("User {} (remote address: {}) master service request for 
> {}.{}", caller,
> remoteAddress, serviceName, methodName);
>   return CoprocessorRpcUtils.getResponse(execResult, 
> HConstants.EMPTY_BYTE_ARRAY);
> } catch (IOException ie) {
>   throw new ServiceException(ie);
> }
>   }
> {code}
> There are many "write" operations like "deleteTable", which may cause 
> security problems, should also record an audit log.
> {code:java}
>   public DeleteTableResponse deleteTable(RpcController controller, 
> DeleteTableRequest request)
> throws ServiceException {
> try {
>   long procId = 
> server.deleteTable(ProtobufUtil.toTableName(request.getTableName()),
> request.getNonceGroup(), request.getNonce());
>   // an audit log is required here.
>   return DeleteTableResponse.newBuilder().setProcId(procId).build();
> } catch (IOException ioe) {
>   throw new ServiceException(ioe);
> }
>   }
> {code}



--
This message was sent by Atlassian Jira
(v8.20.10#820010)