[ 
https://issues.apache.org/jira/browse/HDFS-16945?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17708586#comment-17708586
 ] 

ASF GitHub Bot commented on HDFS-16945:
---------------------------------------

simbadzina commented on code in PR #5468:
URL: https://github.com/apache/hadoop/pull/5468#discussion_r1157760199


##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/security/RouterSecurityAuditLogger.java:
##########
@@ -0,0 +1,109 @@
+/**
+ * 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
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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.
+ */
+
+package org.apache.hadoop.hdfs.server.federation.router.security;
+
+import org.apache.hadoop.classification.VisibleForTesting;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.ipc.CallerContext;
+import org.apache.hadoop.ipc.Server;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.net.InetAddress;
+
+import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.*;
+import static 
org.apache.hadoop.fs.CommonConfigurationKeysPublic.HADOOP_CALLER_CONTEXT_SIGNATURE_MAX_SIZE_DEFAULT;
+
+public class RouterSecurityAuditLogger {
+
+  public static final Logger AUDIT_LOG = LoggerFactory.getLogger(
+      RouterSecurityManager.class.getName() + ".audit");
+
+  private static final ThreadLocal<StringBuilder> STRING_BUILDER =
+      new ThreadLocal<StringBuilder>() {
+        @Override
+        protected StringBuilder initialValue() {
+          return new StringBuilder();
+        }
+      };
+
+  private int callerContextMaxLen;
+  private int callerSignatureMaxLen;
+
+  public RouterSecurityAuditLogger(Configuration conf) {
+    callerContextMaxLen = conf.getInt(
+        HADOOP_CALLER_CONTEXT_MAX_SIZE_KEY,
+        HADOOP_CALLER_CONTEXT_MAX_SIZE_DEFAULT);
+    callerSignatureMaxLen = conf.getInt(
+        HADOOP_CALLER_CONTEXT_SIGNATURE_MAX_SIZE_KEY,
+        HADOOP_CALLER_CONTEXT_SIGNATURE_MAX_SIZE_DEFAULT);
+  }
+
+  public void logAuditEvent(boolean succeeded, String userName,
+                            InetAddress addr, String cmd,
+                            CallerContext callerContext, String tokenId) {
+    if (AUDIT_LOG.isDebugEnabled() || AUDIT_LOG.isInfoEnabled()) {
+      logAuditMessage(
+          creatAuditLog(succeeded, userName, addr, cmd, callerContext,
+              tokenId));
+    }
+  }
+
+  @VisibleForTesting
+  public String creatAuditLog(boolean succeeded, String userName,

Review Comment:
   Typo `createAuditLog`



##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/security/RouterSecurityManager.java:
##########
@@ -152,7 +160,8 @@ public Token<DelegationTokenIdentifier> 
getDelegationToken(Text renewer)
       tokenId = dtId.toStringStable();
       success = true;
     } finally {
-      logAuditEvent(success, operationName, tokenId);
+      logAuditEvent(success, user, Server.getRemoteIp(), operationName,

Review Comment:
   The remote address should be part of the CallerContext as well after 
HDFS-13248.



##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/security/RouterSecurityAuditLogger.java:
##########
@@ -0,0 +1,109 @@
+/**
+ * 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
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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.
+ */
+
+package org.apache.hadoop.hdfs.server.federation.router.security;
+
+import org.apache.hadoop.classification.VisibleForTesting;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.ipc.CallerContext;
+import org.apache.hadoop.ipc.Server;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.net.InetAddress;
+
+import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.*;
+import static 
org.apache.hadoop.fs.CommonConfigurationKeysPublic.HADOOP_CALLER_CONTEXT_SIGNATURE_MAX_SIZE_DEFAULT;
+
+public class RouterSecurityAuditLogger {
+
+  public static final Logger AUDIT_LOG = LoggerFactory.getLogger(
+      RouterSecurityManager.class.getName() + ".audit");
+
+  private static final ThreadLocal<StringBuilder> STRING_BUILDER =
+      new ThreadLocal<StringBuilder>() {
+        @Override
+        protected StringBuilder initialValue() {
+          return new StringBuilder();
+        }
+      };

Review Comment:
   The Java8 construct is a bit more readable.
   ```
   ThreadLocal.withInitial(() -> new StringBuilder());
   ```
   



##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/security/RouterSecurityAuditLogger.java:
##########
@@ -0,0 +1,109 @@
+/**
+ * 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
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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.
+ */
+
+package org.apache.hadoop.hdfs.server.federation.router.security;
+
+import org.apache.hadoop.classification.VisibleForTesting;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.ipc.CallerContext;
+import org.apache.hadoop.ipc.Server;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.net.InetAddress;
+
+import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.*;
+import static 
org.apache.hadoop.fs.CommonConfigurationKeysPublic.HADOOP_CALLER_CONTEXT_SIGNATURE_MAX_SIZE_DEFAULT;
+
+public class RouterSecurityAuditLogger {
+
+  public static final Logger AUDIT_LOG = LoggerFactory.getLogger(
+      RouterSecurityManager.class.getName() + ".audit");
+
+  private static final ThreadLocal<StringBuilder> STRING_BUILDER =
+      new ThreadLocal<StringBuilder>() {
+        @Override
+        protected StringBuilder initialValue() {
+          return new StringBuilder();
+        }
+      };
+
+  private int callerContextMaxLen;
+  private int callerSignatureMaxLen;
+
+  public RouterSecurityAuditLogger(Configuration conf) {
+    callerContextMaxLen = conf.getInt(
+        HADOOP_CALLER_CONTEXT_MAX_SIZE_KEY,
+        HADOOP_CALLER_CONTEXT_MAX_SIZE_DEFAULT);
+    callerSignatureMaxLen = conf.getInt(
+        HADOOP_CALLER_CONTEXT_SIGNATURE_MAX_SIZE_KEY,
+        HADOOP_CALLER_CONTEXT_SIGNATURE_MAX_SIZE_DEFAULT);
+  }
+
+  public void logAuditEvent(boolean succeeded, String userName,
+                            InetAddress addr, String cmd,
+                            CallerContext callerContext, String tokenId) {
+    if (AUDIT_LOG.isDebugEnabled() || AUDIT_LOG.isInfoEnabled()) {
+      logAuditMessage(
+          creatAuditLog(succeeded, userName, addr, cmd, callerContext,
+              tokenId));
+    }
+  }
+
+  @VisibleForTesting
+  public String creatAuditLog(boolean succeeded, String userName,
+                              InetAddress addr, String cmd,
+                              CallerContext callerContext, String tokenId) {
+    final StringBuilder sb = STRING_BUILDER.get();
+    sb.setLength(0);
+    sb.append("allowed=").append(succeeded).append("\t");
+    sb.append("ugi=").append(userName).append("\t");
+    sb.append("ip=").append(addr).append("\t");
+    sb.append("cmd=").append(cmd).append("\t");
+
+    sb.append("\t").append("toeknId=");

Review Comment:
   Typo `tokenId`





> RBF: add RouterSecurityAuditLogger for router security manager
> --------------------------------------------------------------
>
>                 Key: HDFS-16945
>                 URL: https://issues.apache.org/jira/browse/HDFS-16945
>             Project: Hadoop HDFS
>          Issue Type: New Feature
>          Components: rbf
>    Affects Versions: 3.4.0
>            Reporter: Max  Xie
>            Assignee: Max  Xie
>            Priority: Minor
>              Labels: pull-request-available
>
> we should add audit log for router security manager for  token APIs. For 
> examples,
> ```
>  
> {{2023-03-02 20:53:02,712 INFO 
> org.apache.hadoop.hdfs.server.federation.router.security.RouterSecurityManager.audit:
>  allowed=true ugi=hadoop      ip=localhost/127.0.0.1  cmd=getDelegationToken  
>         toeknId=HDFS_DELEGATION_TOKEN token 18359 for hadoop with renewer 
> hadoop        proto=webhdfs}}
> ```



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

---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org

Reply via email to