[HDFS-16868] Fix audit log duplicate issue when an ACE occurs in FSNamesystem.

• Type: Bug
• Status: Resolved
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 3.4.0
• Fix Version/s: 3.4.0
• Component/s: fs
• Labels: pull-request-available
• Target Version/s: 3.4.0
• Hadoop Flags: Reviewed
• Assignee: Beibei Zhao
• Reporter: Beibei Zhao
• Created: 12/Dec/22 06:05
• Updated: 28/Jan/24 07:33
• Resolved: 13/Dec/22 04:26

Description

checkSuperuserPrivilege call logAuditEvent and throw ace when an AccessControlException occurs.

  // This method logs operationName without super user privilege.
  // It should be called without holding FSN lock.
  void checkSuperuserPrivilege(String operationName, String path)
      throws IOException {
    if (isPermissionEnabled) {
      try {
        FSPermissionChecker.setOperationType(operationName);
        FSPermissionChecker pc = getPermissionChecker();
        pc.checkSuperuserPrivilege(path);
      } catch(AccessControlException ace){
        logAuditEvent(false, operationName, path);
        throw ace;
      }
    }
  }

It' s callers like metaSave call it like this:

  /**
   * Dump all metadata into specified file
   * @param filename
   */
  void metaSave(String filename) throws IOException {
    String operationName = "metaSave";
    checkSuperuserPrivilege(operationName);
    ......
    try {
        ......
        metaSave(out);
        ......
      }
    } finally {
      readUnlock(operationName, getLockReportInfoSupplier(null));
    }
    logAuditEvent(true, operationName, null);
  }

but setQuota, addCachePool, modifyCachePool, removeCachePool, createEncryptionZone and reencryptEncryptionZone catch the ace and log the same msg again, it' s a waste of memory I think:

  /**
   * Set the namespace quota and storage space quota for a directory.
   * See {@link ClientProtocol#setQuota(String, long, long, StorageType)} for the
   * contract.
   * 
   * Note: This does not support ".inodes" relative path.
   */
  void setQuota(String src, long nsQuota, long ssQuota, StorageType type)
      throws IOException {
    ......
    try {
      if(!allowOwnerSetQuota) {
        checkSuperuserPrivilege(operationName, src);
      }
     ......
    } catch (AccessControlException ace) {
      logAuditEvent(false, operationName, src);
      throw ace;
    }
    getEditLog().logSync();
    logAuditEvent(true, operationName, src);
  }

Maybe we should move the checkSuperuserPrivilege out of the try block as metaSave and other callers do.

Issue Links

• links to: GitHub Pull Request #5206