curie71 opened a new pull request, #4951:
URL: https://github.com/apache/hbase/pull/4951

   [HBASE-27528](https://issues.apache.org/jira/browse/HBASE-27528) 
MasterRpcServices record audit log in privileged operations **(grant, revoke)**.
   ```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()) {
             String remoteAddress = 
RpcServer.getRemoteAddress().map(InetAddress::toString).orElse("");
             AUDITLOG.trace("User {} (remote address: {}) revoked permission 
{}", caller,
               remoteAddress, userPermission); // duplicate auditlog
           }
           ......
     }
   ```
   but I found a path from `server.cpHost.preRevoke(userPermission);` to 
`AccessChecker audit log`
   (`preRevoke -> MasterCoprocessorHost.preRevoke -> AccessController.preRevoke 
-> preGrantOrRevoke -> accessChecker.requireXXXPermission -> logResult -> 
AUDITLOG.trace...`), which caused **log duplication**: 
   ```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() + ")");
       }
     }
   ```
   the `logResult` auditlog contain all the infomation recorded by 
`MasterRpcServices.revoke` (`user, remote address, permission`) : 
   ```java
     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");
       }
     }
   ```
   Since **AccessChecker** integrates auditlogs for **permission check**, I'll 
delete the log in MasterRpcServices.
   And grant has the same problem.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to