From 5a81043bcb7f94092b29ae75f93f918f5d93aebb Mon Sep 17 00:00:00 2001 From: Dinesh Chitlangia Date: Mon, 1 Jun 2020 14:03:52 -0400 Subject: [PATCH 1/3] HDDS-3694. Refactor dn-audit log --- .../container/common/impl/HddsDispatcher.java | 38 ++++++++++--------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java index c998f899fe46..512c215512ca 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java @@ -40,6 +40,7 @@ import org.apache.hadoop.hdds.security.token.TokenVerifier; import org.apache.hadoop.hdds.tracing.TracingUtil; import org.apache.hadoop.hdds.utils.ProtocolMessageMetrics; +import org.apache.hadoop.ipc.Server; import org.apache.hadoop.ozone.audit.AuditAction; import org.apache.hadoop.ozone.audit.AuditEventStatus; import org.apache.hadoop.ozone.audit.AuditLogger; @@ -65,6 +66,8 @@ import io.opentracing.util.GlobalTracer; import static org.apache.hadoop.hdds.scm.protocolPB.ContainerCommandResponseBuilders.malformedRequest; import static org.apache.hadoop.hdds.scm.protocolPB.ContainerCommandResponseBuilders.unsupportedRequest; +import static org.apache.hadoop.hdds.server.ServerUtils.getRemoteUserName; + import org.apache.ratis.thirdparty.com.google.protobuf.ProtocolMessageEnum; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -113,7 +116,7 @@ public HddsDispatcher(ConfigurationSource config, ContainerSet contSet, this.tokenVerifier = tokenVerifier; protocolMetrics = - new ProtocolMessageMetrics( + new ProtocolMessageMetrics<>( "HddsDispatcher", "HDDS dispatcher metrics", ContainerProtos.Type.values()); @@ -175,7 +178,7 @@ private ContainerCommandResponseProto dispatchRequest( ContainerCommandRequestProto msg, DispatcherContext dispatcherContext) { Preconditions.checkNotNull(msg); if (LOG.isTraceEnabled()) { - LOG.trace("Command {}, trace ID: {} ", msg.getCmdType().toString(), + LOG.trace("Command {}, trace ID: {} ", msg.getCmdType(), msg.getTraceID()); } @@ -490,10 +493,9 @@ public void validateContainerCommand( try { validateBlockToken(msg); } catch (IOException ioe) { - StorageContainerException sce = new StorageContainerException( + throw new StorageContainerException( "Block token verification failed. " + ioe.getMessage(), ioe, ContainerProtos.Result.BLOCK_TOKEN_VERIFICATION_FAILED); - throw sce; } } @@ -583,14 +585,16 @@ private void audit(AuditAction action, EventType eventType, AuditMessage amsg; switch (result) { case SUCCESS: - if(eventType == EventType.READ && - AUDIT.getLogger().isInfoEnabled(AuditMarker.READ.getMarker())) { - amsg = buildAuditMessageForSuccess(action, params); - AUDIT.logReadSuccess(amsg); - } else if(eventType == EventType.WRITE && - AUDIT.getLogger().isInfoEnabled(AuditMarker.WRITE.getMarker())) { - amsg = buildAuditMessageForSuccess(action, params); - AUDIT.logWriteSuccess(amsg); + if(action.getAction().contains("CONTAINER")) { + if(eventType == EventType.READ && + AUDIT.getLogger().isInfoEnabled(AuditMarker.READ.getMarker())) { + amsg = buildAuditMessageForSuccess(action, params); + AUDIT.logReadSuccess(amsg); + } else if(eventType == EventType.WRITE && + AUDIT.getLogger().isInfoEnabled(AuditMarker.WRITE.getMarker())) { + amsg = buildAuditMessageForSuccess(action, params); + AUDIT.logWriteSuccess(amsg); + } } break; @@ -613,28 +617,26 @@ private void audit(AuditAction action, EventType eventType, } } - //TODO: use GRPC to fetch user and ip details @Override public AuditMessage buildAuditMessageForSuccess(AuditAction op, Map auditMap) { return new AuditMessage.Builder() - .setUser(null) - .atIp(null) + .setUser(getRemoteUserName()) + .atIp(Server.getRemoteAddress()) .forOperation(op) .withParams(auditMap) .withResult(AuditEventStatus.SUCCESS) .build(); } - //TODO: use GRPC to fetch user and ip details @Override public AuditMessage buildAuditMessageForFailure(AuditAction op, Map auditMap, Throwable throwable) { return new AuditMessage.Builder() - .setUser(null) - .atIp(null) + .setUser(getRemoteUserName()) + .atIp(Server.getRemoteAddress()) .forOperation(op) .withParams(auditMap) .withResult(AuditEventStatus.FAILURE) From d2753137cd213dcc5b8cb6413fc56f449f120375 Mon Sep 17 00:00:00 2001 From: Dinesh Chitlangia Date: Tue, 2 Jun 2020 12:00:34 -0400 Subject: [PATCH 2/3] addressed review comment. --- .../container/common/impl/HddsDispatcher.java | 30 +++++++++++++++---- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java index 512c215512ca..62667517cb58 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java @@ -585,7 +585,7 @@ private void audit(AuditAction action, EventType eventType, AuditMessage amsg; switch (result) { case SUCCESS: - if(action.getAction().contains("CONTAINER")) { + if(isAllowed(action.getAction())) { if(eventType == EventType.READ && AUDIT.getLogger().isInfoEnabled(AuditMarker.READ.getMarker())) { amsg = buildAuditMessageForSuccess(action, params); @@ -617,13 +617,14 @@ private void audit(AuditAction action, EventType eventType, } } + //TODO: use GRPC to fetch user and ip details @Override public AuditMessage buildAuditMessageForSuccess(AuditAction op, Map auditMap) { return new AuditMessage.Builder() - .setUser(getRemoteUserName()) - .atIp(Server.getRemoteAddress()) + .setUser(null) + .atIp(null) .forOperation(op) .withParams(auditMap) .withResult(AuditEventStatus.SUCCESS) @@ -635,8 +636,8 @@ public AuditMessage buildAuditMessageForFailure(AuditAction op, Map auditMap, Throwable throwable) { return new AuditMessage.Builder() - .setUser(getRemoteUserName()) - .atIp(Server.getRemoteAddress()) + .setUser(null) + .atIp(null) .forOperation(op) .withParams(auditMap) .withResult(AuditEventStatus.FAILURE) @@ -648,4 +649,23 @@ enum EventType { READ, WRITE } + + /** + * Checks if the action is allowed for audit. + * @param action + * @return true or false accordingly. + */ + private boolean isAllowed(String action) { + switch(action) { + case "CLOSE_CONTAINER": + case "CREATE_CONTAINER": + case "LIST_CONTAINER": + case "DELETE_CONTAINER": + case "READ_CONTAINER": + case "UPDATE_CONTAINER": + case "DELETE_BLOCK": + return true; + default: return false; + } + } } From 9c7857a9eddc845cd45206544c7f82aebba66e9d Mon Sep 17 00:00:00 2001 From: Dinesh Chitlangia Date: Tue, 2 Jun 2020 12:35:36 -0400 Subject: [PATCH 3/3] addressed checkstyle --- .../hadoop/ozone/container/common/impl/HddsDispatcher.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java index 62667517cb58..ab65805b3b85 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java @@ -40,7 +40,6 @@ import org.apache.hadoop.hdds.security.token.TokenVerifier; import org.apache.hadoop.hdds.tracing.TracingUtil; import org.apache.hadoop.hdds.utils.ProtocolMessageMetrics; -import org.apache.hadoop.ipc.Server; import org.apache.hadoop.ozone.audit.AuditAction; import org.apache.hadoop.ozone.audit.AuditEventStatus; import org.apache.hadoop.ozone.audit.AuditLogger; @@ -66,7 +65,6 @@ import io.opentracing.util.GlobalTracer; import static org.apache.hadoop.hdds.scm.protocolPB.ContainerCommandResponseBuilders.malformedRequest; import static org.apache.hadoop.hdds.scm.protocolPB.ContainerCommandResponseBuilders.unsupportedRequest; -import static org.apache.hadoop.hdds.server.ServerUtils.getRemoteUserName; import org.apache.ratis.thirdparty.com.google.protobuf.ProtocolMessageEnum; import org.slf4j.Logger;