HDDS-1768. Audit xxxAcl methods in OzoneManager (#1204)

hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java

@@ -248,6 +248,7 @@ public final class OzoneConsts {
   public static final String MAX_KEYS = "maxKeys";
   public static final String PREFIX = "prefix";
   public static final String KEY_PREFIX = "keyPrefix";
+  public static final String ACL = "acl";
   public static final String ACLS = "acls";
   public static final String USER_ACL = "userAcl";
   public static final String ADD_ACLS = "addAcls";
@@ -255,6 +256,7 @@ public final class OzoneConsts {
   public static final String MAX_NUM_OF_BUCKETS = "maxNumOfBuckets";
   public static final String TO_KEY_NAME = "toKeyName";
   public static final String STORAGE_TYPE = "storageType";
+  public static final String RESOURCE_TYPE = "resourceType";
   public static final String IS_VERSION_ENABLED = "isVersionEnabled";
   public static final String CREATION_TIME = "creationTime";
   public static final String DATA_SIZE = "dataSize";

hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/audit/OMAction.java

@@ -58,6 +58,12 @@ public enum OMAction implements AuditAction {
   LIST_MULTIPART_UPLOAD_PARTS,
   ABORT_MULTIPART_UPLOAD,
 
+  //ACL Actions
+  ADD_ACL,
+  GET_ACL,
+  SET_ACL,
+  REMOVE_ACL,
+
   //FS Actions
   GET_FILE_STATUS,
   CREATE_DIRECTORY,

hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/OzoneObj.java

@@ -22,6 +22,8 @@ import org.apache.hadoop.ozone.OzoneConsts;
 import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
 import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OzoneObj.ObjectType;
 
+import java.util.LinkedHashMap;
+import java.util.Map;
 import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OzoneObj.StoreType.*;
 
 /**
@@ -131,4 +133,15 @@ public abstract class OzoneObj implements IOzoneObj {
       value = objType;
     }
   }
+
+  public Map<String, String> toAuditMap() {
+    Map<String, String> auditMap = new LinkedHashMap<>();
+    auditMap.put(OzoneConsts.RESOURCE_TYPE, this.getResourceType().value);
+    auditMap.put(OzoneConsts.STORAGE_TYPE, this.getStoreType().value);
+    auditMap.put(OzoneConsts.VOLUME, this.getVolumeName());
+    auditMap.put(OzoneConsts.BUCKET, this.getBucketName());
+    auditMap.put(OzoneConsts.KEY, this.getKeyName());
+    return auditMap;
+  }
+
 }

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientForAclAuditLog.java

@@ -0,0 +1,284 @@
+package org.apache.hadoop.ozone.client.rpc;
+
+import net.jcip.annotations.NotThreadSafe;
+import org.apache.commons.io.FileUtils;
+import org.apache.commons.lang3.RandomStringUtils;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.scm.protocolPB.StorageContainerLocationProtocolClientSideTranslatorPB;
+import org.apache.hadoop.ozone.MiniOzoneCluster;
+import org.apache.hadoop.ozone.OzoneAcl;
+import org.apache.hadoop.ozone.audit.AuditEventStatus;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.client.ObjectStore;
+import org.apache.hadoop.ozone.client.OzoneClient;
+import org.apache.hadoop.ozone.client.OzoneClientFactory;
+import org.apache.hadoop.ozone.client.OzoneVolume;
+import org.apache.hadoop.ozone.client.VolumeArgs;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.ozone.security.acl.OzoneObjInfo;
+import org.apache.hadoop.security.UserGroupInformation;
+import org.apache.hadoop.test.GenericTestUtils;
+import org.junit.AfterClass;
+import org.junit.Assert;
+import org.junit.BeforeClass;
+import org.junit.FixMethodOrder;
+import org.junit.Test;
+import org.junit.runners.MethodSorters;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.UUID;
+
+import static org.apache.hadoop.ozone.OzoneAcl.AclScope.ACCESS;
+import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ACL_AUTHORIZER_CLASS;
+import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ACL_AUTHORIZER_CLASS_NATIVE;
+import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ACL_ENABLED;
+import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ADMINISTRATORS;
+import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ADMINISTRATORS_WILDCARD;
+import static org.apache.hadoop.ozone.security.acl.OzoneObj.ResourceType.VOLUME;
+import static org.apache.hadoop.ozone.security.acl.OzoneObj.StoreType.OZONE;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * This class is to test audit logs for xxxACL APIs of Ozone Client.
+ * It is annotated as NotThreadSafe intentionally since this test reads from
+ * the generated audit logs to verify the operations. Since the
+ * maven test plugin will trigger parallel test execution, there is a
+ * possibility of other audit events being logged and leading to failure of
+ * all assertion based test in this class.
+ */
+@NotThreadSafe
+@FixMethodOrder(MethodSorters.NAME_ASCENDING)
+public class TestOzoneRpcClientForAclAuditLog {
+
+  private static final Logger LOG =
+      LoggerFactory.getLogger(TestOzoneRpcClientForAclAuditLog.class);
+  private static UserGroupInformation ugi;
+  private static final OzoneAcl USER_ACL =
+      new OzoneAcl(IAccessAuthorizer.ACLIdentityType.USER,
+      "johndoe", IAccessAuthorizer.ACLType.ALL, ACCESS);
+  private static final OzoneAcl USER_ACL_2 =
+      new OzoneAcl(IAccessAuthorizer.ACLIdentityType.USER,
+      "jane", IAccessAuthorizer.ACLType.ALL, ACCESS);
+  private static List<OzoneAcl> aclListToAdd = new ArrayList<>();
+  private static MiniOzoneCluster cluster = null;
+  private static OzoneClient ozClient = null;
+  private static ObjectStore store = null;
+  private static StorageContainerLocationProtocolClientSideTranslatorPB
+      storageContainerLocationClient;
+  private static String scmId = UUID.randomUUID().toString();
+
+
+  /**
+   * Create a MiniOzoneCluster for testing.
+   *
+   * Ozone is made active by setting OZONE_ENABLED = true
+   *
+   * @throws IOException
+   */
+  @BeforeClass
+  public static void init() throws Exception {
+    System.setProperty("log4j.configurationFile", "log4j2.properties");
+    ugi = UserGroupInformation.getCurrentUser();
+    OzoneConfiguration conf = new OzoneConfiguration();
+    conf.setBoolean(OZONE_ACL_ENABLED, true);
+    conf.set(OZONE_ADMINISTRATORS, OZONE_ADMINISTRATORS_WILDCARD);
+    conf.set(OZONE_ACL_AUTHORIZER_CLASS,
+        OZONE_ACL_AUTHORIZER_CLASS_NATIVE);
+    startCluster(conf);
+    aclListToAdd.add(USER_ACL);
+    aclListToAdd.add(USER_ACL_2);
+    emptyAuditLog();
+  }
+
+  /**
+   * Create a MiniOzoneCluster for testing.
+   * @param conf Configurations to start the cluster.
+   * @throws Exception
+   */
+  private static void startCluster(OzoneConfiguration conf) throws Exception {
+    cluster = MiniOzoneCluster.newBuilder(conf)
+        .setNumDatanodes(3)
+        .setScmId(scmId)
+        .build();
+    cluster.waitForClusterToBeReady();
+    ozClient = OzoneClientFactory.getRpcClient(conf);
+    store = ozClient.getObjectStore();
+    storageContainerLocationClient =
+        cluster.getStorageContainerLocationClient();
+  }
+
+  /**
+   * Close OzoneClient and shutdown MiniOzoneCluster.
+   */
+  @AfterClass
+  public static void teardown() throws IOException {
+    shutdownCluster();
+    deleteAuditLog();
+  }
+
+  private static void deleteAuditLog() throws IOException {
+    File file = new File("audit.log");
+    if (FileUtils.deleteQuietly(file)) {
+      LOG.info(file.getName() +
+          " has been deleted.");
+    } else {
+      LOG.info("audit.log could not be deleted.");
+    }
+  }
+
+  private static void emptyAuditLog() throws IOException {
+    File file = new File("audit.log");
+    FileUtils.writeLines(file, new ArrayList<>(), false);
+  }
+
+  /**
+   * Close OzoneClient and shutdown MiniOzoneCluster.
+   */
+  private static void shutdownCluster() throws IOException {
+    if(ozClient != null) {
+      ozClient.close();
+    }
+
+    if (storageContainerLocationClient != null) {
+      storageContainerLocationClient.close();
+    }
+
+    if (cluster != null) {
+      cluster.shutdown();
+    }
+  }
+
+  @Test
+  public void testXXXAclSuccessAudits() throws Exception {
+
+    String userName = ugi.getUserName();
+    String adminName = ugi.getUserName();
+    String volumeName = "volume" + RandomStringUtils.randomNumeric(5);
+
+    VolumeArgs createVolumeArgs = VolumeArgs.newBuilder()
+        .setAdmin(adminName)
+        .setOwner(userName)
+        .build();
+    store.createVolume(volumeName, createVolumeArgs);
+    verifyLog(OMAction.CREATE_VOLUME.name(), volumeName,
+        AuditEventStatus.SUCCESS.name());
+    OzoneVolume retVolumeinfo = store.getVolume(volumeName);
+    verifyLog(OMAction.READ_VOLUME.name(), volumeName,
+        AuditEventStatus.SUCCESS.name());
+    Assert.assertTrue(retVolumeinfo.getName().equalsIgnoreCase(volumeName));
+
+    OzoneObj volObj = new OzoneObjInfo.Builder()
+        .setVolumeName(volumeName)
+        .setResType(VOLUME)
+        .setStoreType(OZONE)
+        .build();
+
+    //Testing getAcl
+    List<OzoneAcl> acls = store.getAcl(volObj);
+    verifyLog(OMAction.GET_ACL.name(), volumeName,
+        AuditEventStatus.SUCCESS.name());
+    Assert.assertTrue(acls.size() > 0);
+
+    //Testing addAcl
+    store.addAcl(volObj, USER_ACL);
+    verifyLog(OMAction.ADD_ACL.name(), volumeName, "johndoe",
+        AuditEventStatus.SUCCESS.name());
+
+    //Testing removeAcl
+    store.removeAcl(volObj, USER_ACL);
+    verifyLog(OMAction.REMOVE_ACL.name(), volumeName, "johndoe",
+        AuditEventStatus.SUCCESS.name());
+
+    //Testing setAcl
+    store.setAcl(volObj, aclListToAdd);
+    verifyLog(OMAction.SET_ACL.name(), volumeName, "johndoe", "jane",
+        AuditEventStatus.SUCCESS.name());
+
+  }
+
+  @Test
+  public void testXXXAclFailureAudits() throws Exception {
+
+    String userName = "bilbo";
+    String adminName = "bilbo";
+    String volumeName = "volume" + RandomStringUtils.randomNumeric(5);
+
+    VolumeArgs createVolumeArgs = VolumeArgs.newBuilder()
+        .setAdmin(adminName)
+        .setOwner(userName)
+        .build();
+    store.createVolume(volumeName, createVolumeArgs);
+    verifyLog(OMAction.CREATE_VOLUME.name(), volumeName,
+        AuditEventStatus.SUCCESS.name());
+
+    OzoneObj volObj = new OzoneObjInfo.Builder()
+        .setVolumeName(volumeName)
+        .setResType(VOLUME)
+        .setStoreType(OZONE)
+        .build();
+
+    // xxxAcl will fail as current ugi user doesn't have the required access
+    // for volume
+    try{
+      List<OzoneAcl> acls = store.getAcl(volObj);
+    } catch (Exception ex) {
+      verifyLog(OMAction.GET_ACL.name(), volumeName,
+          AuditEventStatus.FAILURE.name());
+    }
+
+    try{
+      store.addAcl(volObj, USER_ACL);
+    } catch (Exception ex) {
+      verifyLog(OMAction.ADD_ACL.name(), volumeName,
+          AuditEventStatus.FAILURE.name());
+    }
+
+    try{
+      store.removeAcl(volObj, USER_ACL);
+    } catch (Exception ex) {
+      verifyLog(OMAction.REMOVE_ACL.name(), volumeName,
+          AuditEventStatus.FAILURE.name());
+    }
+
+    try{
+      store.setAcl(volObj, aclListToAdd);
+    } catch (Exception ex) {
+      verifyLog(OMAction.SET_ACL.name(), volumeName, "johndoe", "jane",
+          AuditEventStatus.FAILURE.name());
+    }
+
+  }
+
+  private void verifyLog(String... expected) throws Exception {
+    File file = new File("audit.log");
+    final List<String> lines = FileUtils.readLines(file, (String)null);
+    GenericTestUtils.waitFor(() ->
+        (lines != null) ? true : false, 100, 60000);
+
+    try{
+      // When log entry is expected, the log file will contain one line and
+      // that must be equal to the expected string
+      assertTrue(lines.size() != 0);
+      for(String exp: expected){
+        assertTrue(lines.get(0).contains(exp));
+      }
+    } catch (AssertionError ex){
+      LOG.error("Error occurred in log verification", ex);
+      if(lines.size() != 0){
+        LOG.error("Actual line ::: " + lines.get(0));
+        LOG.error("Expected tokens ::: " + Arrays.toString(expected));
+      }
+      throw ex;
+    } finally {
+      emptyAuditLog();
+    }
+  }
+
+}

hadoop-ozone/integration-test/src/test/resources/log4j2.properties

@@ -0,0 +1,76 @@
+#
+# 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
+# <p>
+# http://www.apache.org/licenses/LICENSE-2.0
+# <p>
+# 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.
+#
+name=PropertiesConfig
+
+# Checks for config change periodically and reloads
+monitorInterval=5
+
+filter=read, write
+# filter.read.onMatch = DENY avoids logging all READ events
+# filter.read.onMatch = ACCEPT permits logging all READ events
+# The above two settings ignore the log levels in configuration
+# filter.read.onMatch = NEUTRAL permits logging of only those READ events
+# which are attempted at log level equal or greater than log level specified
+# in the configuration
+filter.read.type = MarkerFilter
+filter.read.marker = READ
+filter.read.onMatch = NEUTRAL
+filter.read.onMismatch = NEUTRAL
+
+# filter.write.onMatch = DENY avoids logging all WRITE events
+# filter.write.onMatch = ACCEPT permits logging all WRITE events
+# The above two settings ignore the log levels in configuration
+# filter.write.onMatch = NEUTRAL permits logging of only those WRITE events
+# which are attempted at log level equal or greater than log level specified
+# in the configuration
+filter.write.type = MarkerFilter
+filter.write.marker = WRITE
+filter.write.onMatch = NEUTRAL
+filter.write.onMismatch = NEUTRAL
+
+# Log Levels are organized from most specific to least:
+# OFF (most specific, no logging)
+# FATAL (most specific, little data)
+# ERROR
+# WARN
+# INFO
+# DEBUG
+# TRACE (least specific, a lot of data)
+# ALL (least specific, all data)
+
+appenders = console, audit
+appender.console.type = Console
+appender.console.name = STDOUT
+appender.console.layout.type = PatternLayout
+appender.console.layout.pattern = %d{DEFAULT} | %-5level | %c{1} | %msg | %throwable{3} %n
+
+appender.audit.type = File
+appender.audit.name = AUDITLOG
+appender.audit.fileName=audit.log
+appender.audit.layout.type=PatternLayout
+appender.audit.layout.pattern= %d{DEFAULT} | %-5level | %c{1} | %msg | %throwable{3} %n
+
+loggers=audit
+logger.audit.type=AsyncLogger
+logger.audit.name=OMAudit
+logger.audit.level = INFO
+logger.audit.appenderRefs = audit
+logger.audit.appenderRef.file.ref = AUDITLOG
+
+rootLogger.level = INFO
+rootLogger.appenderRefs = stdout
+rootLogger.appenderRef.stdout.ref = STDOUT

hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java

@@ -30,6 +30,7 @@ import java.security.PrivateKey;
 import java.security.PublicKey;
 import java.security.KeyPair;
 import java.security.cert.CertificateException;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Objects;
 
@@ -2989,6 +2990,22 @@ public final class OzoneManager extends ServiceRuntimeInfoImpl
     }
   }
 
+  private void auditAcl(OzoneObj ozoneObj, List<OzoneAcl> ozoneAcl,
+      OMAction omAction, Exception ex) {
+    Map<String, String> auditMap = ozoneObj.toAuditMap();
+    if(ozoneAcl != null) {
+      auditMap.put(OzoneConsts.ACL, ozoneAcl.toString());
+    }
+
+    if(ex == null) {
+      AUDIT.logWriteSuccess(
+          buildAuditMessageForSuccess(omAction, auditMap));
+    } else {
+      AUDIT.logWriteFailure(
+          buildAuditMessageForFailure(omAction, auditMap, ex));
+    }
+  }
+
   /**
    * Add acl for Ozone object. Return true if acl is added successfully else
    * false.
@@ -2999,23 +3016,34 @@ public final class OzoneManager extends ServiceRuntimeInfoImpl
    */
   @Override
   public boolean addAcl(OzoneObj obj, OzoneAcl acl) throws IOException {
-    if(isAclEnabled) {
+    boolean auditSuccess = true;
-      checkAcls(obj.getResourceType(), obj.getStoreType(), ACLType.WRITE_ACL,
+
-          obj.getVolumeName(), obj.getBucketName(), obj.getKeyName());
+    try{
-    }
+      if(isAclEnabled) {
-    // TODO: Audit ACL operation.
+        checkAcls(obj.getResourceType(), obj.getStoreType(), ACLType.WRITE_ACL,
-    switch (obj.getResourceType()) {
+            obj.getVolumeName(), obj.getBucketName(), obj.getKeyName());
-    case VOLUME:
+      }
-      return volumeManager.addAcl(obj, acl);
+      switch (obj.getResourceType()) {
-    case BUCKET:
+      case VOLUME:
-      return bucketManager.addAcl(obj, acl);
+        return volumeManager.addAcl(obj, acl);
-    case KEY:
+      case BUCKET:
-      return keyManager.addAcl(obj, acl);
+        return bucketManager.addAcl(obj, acl);
-    case PREFIX:
+      case KEY:
-      return prefixManager.addAcl(obj, acl);
+        return keyManager.addAcl(obj, acl);
-    default:
+      case PREFIX:
-      throw new OMException("Unexpected resource type: " +
+        return prefixManager.addAcl(obj, acl);
-          obj.getResourceType(), INVALID_REQUEST);
+      default:
+        throw new OMException("Unexpected resource type: " +
+            obj.getResourceType(), INVALID_REQUEST);
+      }
+    } catch(Exception ex) {
+      auditSuccess = false;
+      auditAcl(obj, Arrays.asList(acl), OMAction.ADD_ACL, ex);
+      throw ex;
+    } finally {
+      if(auditSuccess){
+        auditAcl(obj, Arrays.asList(acl), OMAction.ADD_ACL, null);
+      }
     }
   }
 
@@ -3029,24 +3057,35 @@ public final class OzoneManager extends ServiceRuntimeInfoImpl
    */
   @Override
   public boolean removeAcl(OzoneObj obj, OzoneAcl acl) throws IOException {
-    if(isAclEnabled) {
+    boolean auditSuccess = true;
-      checkAcls(obj.getResourceType(), obj.getStoreType(), ACLType.WRITE_ACL,
-          obj.getVolumeName(), obj.getBucketName(), obj.getKeyName());
-    }
-    // TODO: Audit ACL operation.
-    switch (obj.getResourceType()) {
-    case VOLUME:
-      return volumeManager.removeAcl(obj, acl);
-    case BUCKET:
-      return bucketManager.removeAcl(obj, acl);
-    case KEY:
-      return keyManager.removeAcl(obj, acl);
-    case PREFIX:
-      return prefixManager.removeAcl(obj, acl);
 
-    default:
+    try{
-      throw new OMException("Unexpected resource type: " +
+      if(isAclEnabled) {
-          obj.getResourceType(), INVALID_REQUEST);
+        checkAcls(obj.getResourceType(), obj.getStoreType(), ACLType.WRITE_ACL,
+            obj.getVolumeName(), obj.getBucketName(), obj.getKeyName());
+      }
+      switch (obj.getResourceType()) {
+      case VOLUME:
+        return volumeManager.removeAcl(obj, acl);
+      case BUCKET:
+        return bucketManager.removeAcl(obj, acl);
+      case KEY:
+        return keyManager.removeAcl(obj, acl);
+      case PREFIX:
+        return prefixManager.removeAcl(obj, acl);
+
+      default:
+        throw new OMException("Unexpected resource type: " +
+            obj.getResourceType(), INVALID_REQUEST);
+      }
+    } catch(Exception ex) {
+      auditSuccess = false;
+      auditAcl(obj, Arrays.asList(acl), OMAction.REMOVE_ACL, ex);
+      throw ex;
+    } finally {
+      if(auditSuccess){
+        auditAcl(obj, Arrays.asList(acl), OMAction.REMOVE_ACL, null);
+      }
     }
   }
 
@@ -3060,23 +3099,34 @@ public final class OzoneManager extends ServiceRuntimeInfoImpl
    */
   @Override
   public boolean setAcl(OzoneObj obj, List<OzoneAcl> acls) throws IOException {
-    if(isAclEnabled) {
+    boolean auditSuccess = true;
-      checkAcls(obj.getResourceType(), obj.getStoreType(), ACLType.WRITE_ACL,
+
-          obj.getVolumeName(), obj.getBucketName(), obj.getKeyName());
+    try{
-    }
+      if(isAclEnabled) {
-    // TODO: Audit ACL operation.
+        checkAcls(obj.getResourceType(), obj.getStoreType(), ACLType.WRITE_ACL,
-    switch (obj.getResourceType()) {
+            obj.getVolumeName(), obj.getBucketName(), obj.getKeyName());
-    case VOLUME:
+      }
-      return volumeManager.setAcl(obj, acls);
+      switch (obj.getResourceType()) {
-    case BUCKET:
+      case VOLUME:
-      return bucketManager.setAcl(obj, acls);
+        return volumeManager.setAcl(obj, acls);
-    case KEY:
+      case BUCKET:
-      return keyManager.setAcl(obj, acls);
+        return bucketManager.setAcl(obj, acls);
-    case PREFIX:
+      case KEY:
-      return prefixManager.setAcl(obj, acls);
+        return keyManager.setAcl(obj, acls);
-    default:
+      case PREFIX:
-      throw new OMException("Unexpected resource type: " +
+        return prefixManager.setAcl(obj, acls);
-          obj.getResourceType(), INVALID_REQUEST);
+      default:
+        throw new OMException("Unexpected resource type: " +
+            obj.getResourceType(), INVALID_REQUEST);
+      }
+    } catch(Exception ex) {
+      auditSuccess = false;
+      auditAcl(obj, acls, OMAction.SET_ACL, ex);
+      throw ex;
+    } finally {
+      if(auditSuccess){
+        auditAcl(obj, acls, OMAction.SET_ACL, null);
+      }
     }
   }
 
@@ -3088,24 +3138,35 @@ public final class OzoneManager extends ServiceRuntimeInfoImpl
    */
   @Override
   public List<OzoneAcl> getAcl(OzoneObj obj) throws IOException {
-    if(isAclEnabled) {
+    boolean auditSuccess = true;
-      checkAcls(obj.getResourceType(), obj.getStoreType(), ACLType.READ_ACL,
-          obj.getVolumeName(), obj.getBucketName(), obj.getKeyName());
-    }
-    // TODO: Audit ACL operation.
-    switch (obj.getResourceType()) {
-    case VOLUME:
-      return volumeManager.getAcl(obj);
-    case BUCKET:
-      return bucketManager.getAcl(obj);
-    case KEY:
-      return keyManager.getAcl(obj);
-    case PREFIX:
-      return prefixManager.getAcl(obj);
 
-    default:
+    try{
-      throw new OMException("Unexpected resource type: " +
+      if(isAclEnabled) {
-          obj.getResourceType(), INVALID_REQUEST);
+        checkAcls(obj.getResourceType(), obj.getStoreType(), ACLType.READ_ACL,
+            obj.getVolumeName(), obj.getBucketName(), obj.getKeyName());
+      }
+      switch (obj.getResourceType()) {
+      case VOLUME:
+        return volumeManager.getAcl(obj);
+      case BUCKET:
+        return bucketManager.getAcl(obj);
+      case KEY:
+        return keyManager.getAcl(obj);
+      case PREFIX:
+        return prefixManager.getAcl(obj);
+
+      default:
+        throw new OMException("Unexpected resource type: " +
+            obj.getResourceType(), INVALID_REQUEST);
+      }
+    } catch(Exception ex) {
+      auditSuccess = false;
+      auditAcl(obj, null, OMAction.GET_ACL, ex);
+      throw ex;
+    } finally {
+      if(auditSuccess){
+        auditAcl(obj, null, OMAction.GET_ACL, null);
+      }
     }
   }