From 02d0f0ba549e584f98b4606c7cea325c9c1afb6c Mon Sep 17 00:00:00 2001 From: Kihwal Lee Date: Mon, 28 Apr 2014 14:00:32 +0000 Subject: [PATCH] HDFS-6218. Audit log should use true client IP for proxied webhdfs operations. Contributed by Daryn Sharp. git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1590640 13f79535-47bb-0310-9956-ffa450edef68 --- hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt | 3 + .../hadoop/hdfs/server/common/JspHelper.java | 2 +- .../web/resources/NamenodeWebHdfsMethods.java | 20 +++-- .../hdfs/server/namenode/TestAuditLogger.java | 74 ++++++++++++++++++- 4 files changed, 87 insertions(+), 12 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index 9123713b06..7adbd42477 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -409,6 +409,9 @@ Release 2.5.0 - UNRELEASED HDFS-6270. Secondary namenode status page shows transaction count in bytes. (Benoy Antony via wheat9) + HDFS-6218. Audit log should use true client IP for proxied webhdfs + operations. (daryn via kihwal) + Release 2.4.1 - UNRELEASED INCOMPATIBLE CHANGES diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/JspHelper.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/JspHelper.java index 3fd8d8ae04..c47b6826d1 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/JspHelper.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/JspHelper.java @@ -690,7 +690,7 @@ private static UserGroupInformation getTokenUGI(ServletContext context, // honor the X-Forwarded-For header set by a configured set of trusted // proxy servers. allows audit logging and proxy user checks to work // via an http proxy - static String getRemoteAddr(HttpServletRequest request) { + public static String getRemoteAddr(HttpServletRequest request) { String remoteAddr = request.getRemoteAddr(); String proxyHeader = request.getHeader("X-Forwarded-For"); if (proxyHeader != null && ProxyUsers.isProxyServer(remoteAddr)) { diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/web/resources/NamenodeWebHdfsMethods.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/web/resources/NamenodeWebHdfsMethods.java index 5005dfc805..87b64f4434 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/web/resources/NamenodeWebHdfsMethods.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/web/resources/NamenodeWebHdfsMethods.java @@ -162,8 +162,16 @@ private void init(final UserGroupInformation ugi, //clear content type response.setContentType(null); + + // set the remote address, if coming in via a trust proxy server then + // the address with be that of the proxied client + REMOTE_ADDRESS.set(JspHelper.getRemoteAddr(request)); } + private void reset() { + REMOTE_ADDRESS.set(null); + } + private static NamenodeProtocols getRPCServer(NameNode namenode) throws IOException { final NamenodeProtocols np = namenode.getRpcServer(); @@ -394,7 +402,6 @@ public Response put( return ugi.doAs(new PrivilegedExceptionAction() { @Override public Response run() throws IOException, URISyntaxException { - REMOTE_ADDRESS.set(request.getRemoteAddr()); try { return put(ugi, delegation, username, doAsUser, path.getAbsolutePath(), op, destination, owner, group, @@ -402,7 +409,7 @@ public Response run() throws IOException, URISyntaxException { modificationTime, accessTime, renameOptions, createParent, delegationTokenArgument,aclPermission); } finally { - REMOTE_ADDRESS.set(null); + reset(); } } }); @@ -583,12 +590,11 @@ public Response post( return ugi.doAs(new PrivilegedExceptionAction() { @Override public Response run() throws IOException, URISyntaxException { - REMOTE_ADDRESS.set(request.getRemoteAddr()); try { return post(ugi, delegation, username, doAsUser, path.getAbsolutePath(), op, concatSrcs, bufferSize); } finally { - REMOTE_ADDRESS.set(null); + reset(); } } }); @@ -681,12 +687,11 @@ public Response get( return ugi.doAs(new PrivilegedExceptionAction() { @Override public Response run() throws IOException, URISyntaxException { - REMOTE_ADDRESS.set(request.getRemoteAddr()); try { return get(ugi, delegation, username, doAsUser, path.getAbsolutePath(), op, offset, length, renewer, bufferSize); } finally { - REMOTE_ADDRESS.set(null); + reset(); } } }); @@ -889,12 +894,11 @@ public Response delete( return ugi.doAs(new PrivilegedExceptionAction() { @Override public Response run() throws IOException { - REMOTE_ADDRESS.set(request.getRemoteAddr()); try { return delete(ugi, delegation, username, doAsUser, path.getAbsolutePath(), op, recursive); } finally { - REMOTE_ADDRESS.set(null); + reset(); } } }); 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 f20d51a1df..8d1a70ec85 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 @@ -24,7 +24,10 @@ import static org.junit.Assert.fail; import java.io.IOException; +import java.net.HttpURLConnection; import java.net.InetAddress; +import java.net.URI; +import java.net.URISyntaxException; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileStatus; @@ -33,9 +36,11 @@ import org.apache.hadoop.fs.Path; import org.apache.hadoop.hdfs.HdfsConfiguration; import org.apache.hadoop.hdfs.MiniDFSCluster; -import org.apache.hadoop.hdfs.protocol.HdfsFileStatus; +import org.apache.hadoop.hdfs.web.resources.GetOpParam; import org.apache.hadoop.ipc.RemoteException; -import org.apache.hadoop.security.UserGroupInformation; +import org.apache.hadoop.net.NetUtils; +import org.apache.hadoop.security.authorize.ProxyUsers; +import org.junit.Before; import org.junit.Test; /** @@ -45,6 +50,16 @@ public class TestAuditLogger { private static final short TEST_PERMISSION = (short) 0654; + @Before + public void setup() { + DummyAuditLogger.initialized = false; + DummyAuditLogger.logCount = 0; + DummyAuditLogger.remoteAddr = null; + + Configuration conf = new HdfsConfiguration(); + ProxyUsers.refreshSuperUserGroupsConfiguration(conf); + } + /** * Tests that AuditLogger works as expected. */ @@ -69,6 +84,57 @@ public void testAuditLogger() throws IOException { } } + @Test + public void testWebHdfsAuditLogger() throws IOException, URISyntaxException { + Configuration conf = new HdfsConfiguration(); + conf.set(DFS_NAMENODE_AUDIT_LOGGERS_KEY, + DummyAuditLogger.class.getName()); + MiniDFSCluster cluster = new MiniDFSCluster.Builder(conf).build(); + + GetOpParam.Op op = GetOpParam.Op.GETFILESTATUS; + try { + cluster.waitClusterUp(); + assertTrue(DummyAuditLogger.initialized); + URI uri = new URI( + "http", + NetUtils.getHostPortString(cluster.getNameNode().getHttpAddress()), + "/webhdfs/v1/", op.toQueryString(), null); + + // non-proxy request + HttpURLConnection conn = (HttpURLConnection) uri.toURL().openConnection(); + conn.setRequestMethod(op.getType().toString()); + conn.connect(); + assertEquals(200, conn.getResponseCode()); + conn.disconnect(); + assertEquals(1, DummyAuditLogger.logCount); + assertEquals("127.0.0.1", DummyAuditLogger.remoteAddr); + + // non-trusted proxied request + conn = (HttpURLConnection) uri.toURL().openConnection(); + conn.setRequestMethod(op.getType().toString()); + conn.setRequestProperty("X-Forwarded-For", "1.1.1.1"); + conn.connect(); + assertEquals(200, conn.getResponseCode()); + conn.disconnect(); + assertEquals(2, DummyAuditLogger.logCount); + assertEquals("127.0.0.1", DummyAuditLogger.remoteAddr); + + // trusted proxied request + conf.set(ProxyUsers.CONF_HADOOP_PROXYSERVERS, "127.0.0.1"); + ProxyUsers.refreshSuperUserGroupsConfiguration(conf); + conn = (HttpURLConnection) uri.toURL().openConnection(); + conn.setRequestMethod(op.getType().toString()); + conn.setRequestProperty("X-Forwarded-For", "1.1.1.1"); + conn.connect(); + assertEquals(200, conn.getResponseCode()); + conn.disconnect(); + assertEquals(3, DummyAuditLogger.logCount); + assertEquals("1.1.1.1", DummyAuditLogger.remoteAddr); + } finally { + cluster.shutdown(); + } + } + /** * Minor test related to HADOOP-9155. Verify that during a * FileSystem.setPermission() operation, the stat passed in during the @@ -128,7 +194,8 @@ public static class DummyAuditLogger implements AuditLogger { static boolean initialized; static int logCount; static short foundPermission; - + static String remoteAddr; + public void initialize(Configuration conf) { initialized = true; } @@ -140,6 +207,7 @@ public static void resetLogCount() { public void logAuditEvent(boolean succeeded, String userName, InetAddress addr, String cmd, String src, String dst, FileStatus stat) { + remoteAddr = addr.getHostAddress(); logCount++; if (stat != null) { foundPermission = stat.getPermission().toShort();