You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@knox.apache.org by pz...@apache.org on 2018/07/09 20:03:05 UTC

knox git commit: KNOX-1379 - Default dispatch whitelist should not include localhost when the Knox host domain can be determined

Repository: knox
Updated Branches:
  refs/heads/master c555bafbd -> 4729799d6


KNOX-1379 - Default dispatch whitelist should not include localhost when the Knox host domain can be determined


Project: http://git-wip-us.apache.org/repos/asf/knox/repo
Commit: http://git-wip-us.apache.org/repos/asf/knox/commit/4729799d
Tree: http://git-wip-us.apache.org/repos/asf/knox/tree/4729799d
Diff: http://git-wip-us.apache.org/repos/asf/knox/diff/4729799d

Branch: refs/heads/master
Commit: 4729799d6cc7fedef378e6f7fe3ac8409754f342
Parents: c555baf
Author: Phil Zampino <pz...@apache.org>
Authored: Mon Jul 9 13:36:08 2018 -0400
Committer: Phil Zampino <pz...@apache.org>
Committed: Mon Jul 9 13:36:08 2018 -0400

----------------------------------------------------------------------
 .../knox/gateway/util/WhitelistUtils.java       |  2 +-
 .../knox/gateway/util/WhitelistUtilsTest.java   | 30 +++++++++++++++++---
 2 files changed, 27 insertions(+), 5 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/knox/blob/4729799d/gateway-spi/src/main/java/org/apache/knox/gateway/util/WhitelistUtils.java
----------------------------------------------------------------------
diff --git a/gateway-spi/src/main/java/org/apache/knox/gateway/util/WhitelistUtils.java b/gateway-spi/src/main/java/org/apache/knox/gateway/util/WhitelistUtils.java
index ae5e519..4f7d34f 100644
--- a/gateway-spi/src/main/java/org/apache/knox/gateway/util/WhitelistUtils.java
+++ b/gateway-spi/src/main/java/org/apache/knox/gateway/util/WhitelistUtils.java
@@ -104,7 +104,7 @@ public class WhitelistUtils {
       String domain = hostname.substring(hostname.indexOf('.'));
       String domainPattern = ".+" + domain.replaceAll("\\.", "\\\\.");
       whitelist =
-              String.format(DEFAULT_DISPATCH_WHITELIST_TEMPLATE, LOCALHOST_REGEXP_SEGMENT + "|(" + domainPattern + ")");
+              String.format(DEFAULT_DISPATCH_WHITELIST_TEMPLATE, "(" + domainPattern + ")");
     }
     return whitelist;
   }

http://git-wip-us.apache.org/repos/asf/knox/blob/4729799d/gateway-spi/src/test/java/org/apache/knox/gateway/util/WhitelistUtilsTest.java
----------------------------------------------------------------------
diff --git a/gateway-spi/src/test/java/org/apache/knox/gateway/util/WhitelistUtilsTest.java b/gateway-spi/src/test/java/org/apache/knox/gateway/util/WhitelistUtilsTest.java
index 272a35d..7fce2cc 100644
--- a/gateway-spi/src/test/java/org/apache/knox/gateway/util/WhitelistUtilsTest.java
+++ b/gateway-spi/src/test/java/org/apache/knox/gateway/util/WhitelistUtilsTest.java
@@ -20,9 +20,12 @@ import org.apache.knox.gateway.config.GatewayConfig;
 import org.easymock.EasyMock;
 import org.junit.Test;
 
+import javax.annotation.RegEx;
 import javax.servlet.ServletContext;
 import javax.servlet.http.HttpServletRequest;
 import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 
@@ -33,6 +36,8 @@ import static org.junit.Assert.assertTrue;
 
 public class WhitelistUtilsTest {
 
+  private static final List<String> LOCALHOST_NAMES = Arrays.asList("localhost", "127.0.0.1", "0:0:0:0:0:0:0:1", "::1");
+
   @Test
   public void testDefault() throws Exception {
     String whitelist = doTestGetDispatchWhitelist(createMockGatewayConfig(Collections.emptyList(), null), "TEST");
@@ -79,6 +84,16 @@ public class WhitelistUtilsTest {
   }
 
   @Test
+  public void testDefaultDomainWhitelistLocalhostDisallowed() throws Exception {
+    String whitelist = doTestDeriveDomainBasedWhitelist("host.test.org");
+    assertNotNull(whitelist);
+    // localhost names should be excluded from the whitelist when the Knox host domain can be determined
+    for (String name : LOCALHOST_NAMES) {
+      assertFalse(RegExUtils.checkWhitelist(whitelist, name));
+    }
+  }
+
+  @Test
   public void testDefaultDomainWhitelistWithXForwardedHost() throws Exception {
     final String serviceRole = "TEST";
 
@@ -158,11 +173,9 @@ public class WhitelistUtilsTest {
     EasyMock.replay(request);
 
     String result = null;
-    if (serverName != null && !serverName.isEmpty() && !serverName.equalsIgnoreCase("localhost") && xForwardedHost == null) {
+    if (serverName != null && !serverName.isEmpty() && !isLocalhostServerName(serverName) && xForwardedHost == null) {
       try {
-        Method method = WhitelistUtils.class.getDeclaredMethod("deriveDomainBasedWhitelist", String.class);
-        method.setAccessible(true);
-        result = (String) method.invoke(null, serverName);
+        result = doTestDeriveDomainBasedWhitelist(serverName);
       } catch (Exception e) {
         e.printStackTrace();
       }
@@ -181,6 +194,15 @@ public class WhitelistUtilsTest {
     return result;
   }
 
+  private static String doTestDeriveDomainBasedWhitelist(final String serverName) throws Exception {
+    Method method = WhitelistUtils.class.getDeclaredMethod("deriveDomainBasedWhitelist", String.class);
+    method.setAccessible(true);
+    return (String) method.invoke(null, serverName);
+  }
+
+  private static boolean isLocalhostServerName(final String serverName) {
+    return LOCALHOST_NAMES.contains(serverName.toLowerCase());
+  }
 
   private static GatewayConfig createMockGatewayConfig(final List<String> serviceRoles, final String whitelist) {
     GatewayConfig config = EasyMock.createNiceMock(GatewayConfig.class);