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 <[email protected]> Authored: Mon Jul 9 13:36:08 2018 -0400 Committer: Phil Zampino <[email protected]> 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);
