From ef68759382e4f1cf26316fb9b98c31fca40eef3b Mon Sep 17 00:00:00 2001 From: Kihwal Lee Date: Wed, 16 Apr 2014 16:22:36 +0000 Subject: [PATCH] HDFS-6219. Proxy superuser configuration should use true client IP for address checks. Contributed by Daryn Sharp. git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1587962 13f79535-47bb-0310-9956-ffa450edef68 --- hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt | 3 ++ .../hadoop/hdfs/server/common/JspHelper.java | 19 ++++++- .../hdfs/server/common/TestJspHelper.java | 53 ++++++++++++++++++- 3 files changed, 73 insertions(+), 2 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index d276c8de986..e98d4e1c625 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -125,6 +125,9 @@ Trunk (Unreleased) HDFS-6228. comments typo fix for FsDatasetImpl.java (zhaoyunjiong via umamahesh) + HDFS-6219. Proxy superuser configuration should use true client IP for + address checks. (daryn via kihwal) + OPTIMIZATIONS BUG FIXES 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 615543ec16d..0e474b1bb84 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 @@ -25,6 +25,7 @@ import java.io.ByteArrayInputStream; import java.io.DataInputStream; import java.io.IOException; import java.io.UnsupportedEncodingException; +import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.Socket; import java.net.URL; @@ -646,7 +647,7 @@ public class JspHelper { if (doAsUserFromQuery != null) { // create and attempt to authorize a proxy user ugi = UserGroupInformation.createProxyUser(doAsUserFromQuery, ugi); - ProxyUsers.authorize(ugi, request.getRemoteAddr(), conf); + ProxyUsers.authorize(ugi, getRemoteAddr(request), conf); } } @@ -686,6 +687,22 @@ public class JspHelper { return ugi; } + // 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) { + String remoteAddr = request.getRemoteAddr(); + String proxyHeader = request.getHeader("X-Forwarded-For"); + if (proxyHeader != null && ProxyUsers.isProxyServer(remoteAddr)) { + final String clientAddr = proxyHeader.split(",")[0].trim(); + if (!clientAddr.isEmpty()) { + remoteAddr = clientAddr; + } + } + return remoteAddr; + } + + /** * Expected user name should be a short name. */ diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/common/TestJspHelper.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/common/TestJspHelper.java index 69ed4666b46..0906ceec7b0 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/common/TestJspHelper.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/common/TestJspHelper.java @@ -29,7 +29,6 @@ import static org.mockito.Mockito.when; import java.io.IOException; import java.io.StringReader; import java.net.InetSocketAddress; -import java.text.MessageFormat; import java.util.ArrayList; import javax.servlet.ServletContext; @@ -62,6 +61,7 @@ import org.apache.hadoop.security.token.Token; import org.apache.hadoop.security.token.TokenIdentifier; import org.apache.hadoop.security.token.delegation.AbstractDelegationTokenSecretManager; import org.junit.Assert; +import org.junit.BeforeClass; import org.junit.Test; import org.mockito.ArgumentCaptor; import org.mockito.Mockito; @@ -78,6 +78,13 @@ public class TestJspHelper { private final Configuration conf = new HdfsConfiguration(); private String jspWriterOutput = ""; + // allow user with TGT to run tests + @BeforeClass + public static void setupKerb() { + System.setProperty("java.security.krb5.kdc", ""); + System.setProperty("java.security.krb5.realm", "NONE"); + } + public static class DummySecretManager extends AbstractDelegationTokenSecretManager { @@ -630,5 +637,49 @@ public class TestJspHelper { 50020, 50075, 50076, 50010); assertNotNull(JspHelper.Url.authority("http", dnWithEmptyIp)); } + + private static String clientAddr = "1.1.1.1"; + private static String chainedClientAddr = clientAddr+", 2.2.2.2"; + private static String proxyAddr = "3.3.3.3"; + + @Test + public void testRemoteAddr() { + assertEquals(clientAddr, getRemoteAddr(clientAddr, null, false)); + } + + @Test + public void testRemoteAddrWithUntrustedProxy() { + assertEquals(proxyAddr, getRemoteAddr(clientAddr, proxyAddr, false)); + } + + @Test + public void testRemoteAddrWithTrustedProxy() { + assertEquals(clientAddr, getRemoteAddr(clientAddr, proxyAddr, true)); + assertEquals(clientAddr, getRemoteAddr(chainedClientAddr, proxyAddr, true)); + } + + @Test + public void testRemoteAddrWithTrustedProxyAndEmptyClient() { + assertEquals(proxyAddr, getRemoteAddr(null, proxyAddr, true)); + assertEquals(proxyAddr, getRemoteAddr("", proxyAddr, true)); + } + + private String getRemoteAddr(String clientAddr, String proxyAddr, boolean trusted) { + HttpServletRequest req = mock(HttpServletRequest.class); + when(req.getRemoteAddr()).thenReturn("1.2.3.4"); + + Configuration conf = new Configuration(); + if (proxyAddr == null) { + when(req.getRemoteAddr()).thenReturn(clientAddr); + } else { + when(req.getRemoteAddr()).thenReturn(proxyAddr); + when(req.getHeader("X-Forwarded-For")).thenReturn(clientAddr); + if (trusted) { + conf.set(ProxyUsers.CONF_HADOOP_PROXYSERVERS, proxyAddr); + } + } + ProxyUsers.refreshSuperUserGroupsConfiguration(conf); + return JspHelper.getRemoteAddr(req); + } }