From 4d3cec7b299b5f8598143d930317d1ad0bbba110 Mon Sep 17 00:00:00 2001 From: Takanobu Asanuma Date: Mon, 22 Nov 2021 13:15:03 +0900 Subject: [PATCH 1/3] HADOOP-18014. CallerContext should not include some characters. --- .../org/apache/hadoop/ipc/CallerContext.java | 2 -- .../hdfs/server/namenode/FSNamesystem.java | 12 ++++---- .../hdfs/server/namenode/TestAuditLogger.java | 30 +++++++++++++++++++ 3 files changed, 36 insertions(+), 8 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/CallerContext.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/CallerContext.java index 9f9c9741f36bb..ed8dcf2242f2c 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/CallerContext.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/CallerContext.java @@ -164,8 +164,6 @@ private void checkFieldSeparator(String separator) { /** * Whether the field is valid. - * The field should not contain '\t', '\n', '='. - * Because the context could be written to audit log. * @param field one of the fields in context. * @return true if the field is not null or empty. */ diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java index 27dcf01ee92c5..4b546bdb305f1 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java @@ -8800,18 +8800,18 @@ public void logAuditEvent(boolean succeeded, String userName, callerContext != null && callerContext.isContextValid()) { sb.append("\t").append("callerContext="); - if (callerContext.getContext().length() > callerContextMaxLen) { - sb.append(callerContext.getContext().substring(0, - callerContextMaxLen)); + String context = escapeJava(callerContext.getContext()); + if (context.length() > callerContextMaxLen) { + sb.append(context, 0, callerContextMaxLen); } else { - sb.append(callerContext.getContext()); + sb.append(context); } if (callerContext.getSignature() != null && callerContext.getSignature().length > 0 && callerContext.getSignature().length <= callerSignatureMaxLen) { sb.append(":") - .append(new String(callerContext.getSignature(), - CallerContext.SIGNATURE_ENCODING)); + .append(escapeJava(new String(callerContext.getSignature(), + CallerContext.SIGNATURE_ENCODING))); } } logAuditMessage(sb.toString()); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAuditLogger.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAuditLogger.java index c7e47561dcc7e..6165b90e2b84c 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAuditLogger.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAuditLogger.java @@ -599,6 +599,36 @@ public void testAuditLogWithRemotePort() throws Exception { } } + @Test + public void testCallerContextCharacterEscape() throws IOException { + Configuration conf = new HdfsConfiguration(); + conf.setBoolean(HADOOP_CALLER_CONTEXT_ENABLED_KEY, true); + conf.setInt(HADOOP_CALLER_CONTEXT_MAX_SIZE_KEY, 128); + conf.setInt(HADOOP_CALLER_CONTEXT_SIGNATURE_MAX_SIZE_KEY, 40); + MiniDFSCluster cluster = new MiniDFSCluster.Builder(conf).build(); + LogCapturer auditlog = LogCapturer.captureLogs(FSNamesystem.auditLog); + + try { + cluster.waitClusterUp(); + final FileSystem fs = cluster.getFileSystem(); + final long time = System.currentTimeMillis(); + final Path p = new Path("/"); + + assertNull(CallerContext.getCurrent()); + + CallerContext context = new CallerContext.Builder("c1\nc2").append("c3\tc4") + .setSignature("s1\ns2".getBytes(CallerContext.SIGNATURE_ENCODING)).build(); + CallerContext.setCurrent(context); + LOG.info("Set current caller context as {}", CallerContext.getCurrent()); + fs.setTimes(p, time, time); + assertTrue(auditlog.getOutput().endsWith( + String.format("callerContext=c1\\nc2,c3\\tc4:s1\\ns2%n"))); + auditlog.clearOutput(); + } finally { + cluster.shutdown(); + } + } + public static class DummyAuditLogger implements AuditLogger { static boolean initialized; From 4d1aa672731125e0b0a26c6de42398a891a1ba4e Mon Sep 17 00:00:00 2001 From: Takanobu Asanuma Date: Tue, 23 Nov 2021 01:59:56 +0900 Subject: [PATCH 2/3] use try-with-resources --- .../hadoop/hdfs/server/namenode/TestAuditLogger.java | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAuditLogger.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAuditLogger.java index 6165b90e2b84c..64c1aa9d2a8f6 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAuditLogger.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAuditLogger.java @@ -605,10 +605,9 @@ public void testCallerContextCharacterEscape() throws IOException { conf.setBoolean(HADOOP_CALLER_CONTEXT_ENABLED_KEY, true); conf.setInt(HADOOP_CALLER_CONTEXT_MAX_SIZE_KEY, 128); conf.setInt(HADOOP_CALLER_CONTEXT_SIGNATURE_MAX_SIZE_KEY, 40); - MiniDFSCluster cluster = new MiniDFSCluster.Builder(conf).build(); - LogCapturer auditlog = LogCapturer.captureLogs(FSNamesystem.auditLog); - try { + try (MiniDFSCluster cluster = new MiniDFSCluster.Builder(conf).build()) { + LogCapturer auditlog = LogCapturer.captureLogs(FSNamesystem.auditLog); cluster.waitClusterUp(); final FileSystem fs = cluster.getFileSystem(); final long time = System.currentTimeMillis(); @@ -624,8 +623,6 @@ public void testCallerContextCharacterEscape() throws IOException { assertTrue(auditlog.getOutput().endsWith( String.format("callerContext=c1\\nc2,c3\\tc4:s1\\ns2%n"))); auditlog.clearOutput(); - } finally { - cluster.shutdown(); } } From ff11828179de0fc1556de2a948a9ca7878ff28c4 Mon Sep 17 00:00:00 2001 From: Takanobu Asanuma Date: Tue, 23 Nov 2021 13:28:04 +0900 Subject: [PATCH 3/3] Clear CallerContext at the end of the unit test --- .../hadoop/hdfs/server/namenode/TestAuditLogger.java | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAuditLogger.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAuditLogger.java index 64c1aa9d2a8f6..1cc950723be34 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAuditLogger.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAuditLogger.java @@ -256,10 +256,9 @@ public void testAuditLoggerWithCallContext() throws IOException { conf.setBoolean(HADOOP_CALLER_CONTEXT_ENABLED_KEY, true); conf.setInt(HADOOP_CALLER_CONTEXT_MAX_SIZE_KEY, 128); conf.setInt(HADOOP_CALLER_CONTEXT_SIGNATURE_MAX_SIZE_KEY, 40); - MiniDFSCluster cluster = new MiniDFSCluster.Builder(conf).build(); - LogCapturer auditlog = LogCapturer.captureLogs(FSNamesystem.auditLog); - try { + try (MiniDFSCluster cluster = new MiniDFSCluster.Builder(conf).build()) { + LogCapturer auditlog = LogCapturer.captureLogs(FSNamesystem.auditLog); cluster.waitClusterUp(); final FileSystem fs = cluster.getFileSystem(); final long time = System.currentTimeMillis(); @@ -414,8 +413,8 @@ public void run() { assertFalse(auditlog.getOutput().contains("callerContext=")); auditlog.clearOutput(); - } finally { - cluster.shutdown(); + // clear client context + CallerContext.setCurrent(null); } } @@ -623,6 +622,9 @@ public void testCallerContextCharacterEscape() throws IOException { assertTrue(auditlog.getOutput().endsWith( String.format("callerContext=c1\\nc2,c3\\tc4:s1\\ns2%n"))); auditlog.clearOutput(); + + // clear client context + CallerContext.setCurrent(null); } }