From 52fb9d7ce227ae2c8296bb8a8f724960fb915bfc Mon Sep 17 00:00:00 2001 From: Takanobu Asanuma Date: Thu, 25 Nov 2021 14:05:04 +0900 Subject: [PATCH] HADOOP-18014. CallerContext should not include some characters. (#3698) Reviewed-by: Viraj Jasani Reviewed-by: Mingliang Liu Reviewed-by: Hui Fei Cherry-picked from 9c887e5b by Owen O'Malley --- .../org/apache/hadoop/ipc/CallerContext.java | 2 - .../hdfs/server/namenode/FSNamesystem.java | 12 +++--- .../hdfs/server/namenode/TestAuditLogger.java | 39 ++++++++++++++++--- 3 files changed, 40 insertions(+), 13 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 9f9c9741f3..ed8dcf2242 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 964879bb99..318442d279 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 @@ -8512,18 +8512,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 681ea78338..45bcf3ecdb 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); } } @@ -599,6 +598,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); + + 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(); + 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(); + + // clear client context + CallerContext.setCurrent(null); + } + } + public static class DummyAuditLogger implements AuditLogger { static boolean initialized;