Repository: knox Updated Branches: refs/heads/master 96728794b -> b2ec86f71
KNOX-1372 - Default whitelist validation mistreats simple host names Project: http://git-wip-us.apache.org/repos/asf/knox/repo Commit: http://git-wip-us.apache.org/repos/asf/knox/commit/b2ec86f7 Tree: http://git-wip-us.apache.org/repos/asf/knox/tree/b2ec86f7 Diff: http://git-wip-us.apache.org/repos/asf/knox/diff/b2ec86f7 Branch: refs/heads/master Commit: b2ec86f71046867d7798635853a9c7076f6e3a20 Parents: 9672879 Author: Phil Zampino <pzamp...@apache.org> Authored: Mon Jul 2 13:14:28 2018 -0400 Committer: Phil Zampino <pzamp...@apache.org> Committed: Mon Jul 2 13:14:28 2018 -0400 ---------------------------------------------------------------------- .../service/knoxsso/WebSSOResourceTest.java | 12 ----- .../gateway/dispatch/GatewayDispatchFilter.java | 10 +++- .../knox/gateway/util/WhitelistUtils.java | 38 +++++++------- .../dispatch/GatewayDispatchFilterTest.java | 18 +------ .../knox/gateway/util/WhitelistUtilsTest.java | 54 ++++++-------------- 5 files changed, 43 insertions(+), 89 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/knox/blob/b2ec86f7/gateway-service-knoxsso/src/test/java/org/apache/knox/gateway/service/knoxsso/WebSSOResourceTest.java ---------------------------------------------------------------------- diff --git a/gateway-service-knoxsso/src/test/java/org/apache/knox/gateway/service/knoxsso/WebSSOResourceTest.java b/gateway-service-knoxsso/src/test/java/org/apache/knox/gateway/service/knoxsso/WebSSOResourceTest.java index 65b3a26..58877e4 100644 --- a/gateway-service-knoxsso/src/test/java/org/apache/knox/gateway/service/knoxsso/WebSSOResourceTest.java +++ b/gateway-service-knoxsso/src/test/java/org/apache/knox/gateway/service/knoxsso/WebSSOResourceTest.java @@ -583,23 +583,11 @@ public class WebSSOResourceTest { doTestDefaultLocalhostWhitelist("localhost"); } - @Test - public void testDefaultDomainWhitelist() throws Exception { - doTestDefaultDomainWhitelist("knox.test.org"); - doTestDefaultDomainWhitelist("knox.test.com"); - } - private void doTestDefaultLocalhostWhitelist(String localhostId) throws Exception { String whitelistValue = doTestDefaultWhitelist(localhostId); assertTrue(whitelistValue.contains("localhost")); } - private void doTestDefaultDomainWhitelist(String hostname) throws Exception { - String whitelistValue = doTestDefaultWhitelist(hostname); - assertTrue(whitelistValue.contains(hostname.substring(hostname.indexOf('.')).replaceAll("\\.", "\\\\."))); - } - - private String doTestDefaultWhitelist(String hostname) throws Exception { final String testServiceRole = "TEST"; http://git-wip-us.apache.org/repos/asf/knox/blob/b2ec86f7/gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/GatewayDispatchFilter.java ---------------------------------------------------------------------- diff --git a/gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/GatewayDispatchFilter.java b/gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/GatewayDispatchFilter.java index 0a993a0..f4d8383 100644 --- a/gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/GatewayDispatchFilter.java +++ b/gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/GatewayDispatchFilter.java @@ -45,6 +45,8 @@ public class GatewayDispatchFilter extends AbstractGatewayFilter { private final Object lock = new Object(); + private String whitelist = null; + private Dispatch dispatch; private HttpClient httpClient; @@ -130,12 +132,16 @@ public class GatewayDispatchFilter extends AbstractGatewayFilter { private boolean isDispatchAllowed(HttpServletRequest request) { boolean isAllowed = true; - String whitelist = WhitelistUtils.getDispatchWhitelist(request); - if (whitelist != null) { + // Initialize the white list if it has not yet been initialized + if (whitelist == null) { + whitelist = WhitelistUtils.getDispatchWhitelist(request); + } + if (whitelist != null) { String requestURI = request.getRequestURI(); isAllowed = RegExUtils.checkWhitelist(whitelist, requestURI); + if (!isAllowed) { LOG.dispatchDisallowed(requestURI); } http://git-wip-us.apache.org/repos/asf/knox/blob/b2ec86f7/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 e1f32be..220e448 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 @@ -21,6 +21,8 @@ import org.apache.knox.gateway.config.GatewayConfig; import org.apache.knox.gateway.i18n.messages.MessagesFactory; import javax.servlet.http.HttpServletRequest; +import java.net.InetAddress; +import java.net.UnknownHostException; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -39,7 +41,6 @@ public class WhitelistUtils { private static final List<String> DEFAULT_SERVICE_ROLES = Arrays.asList("KNOXSSO"); - public static String getDispatchWhitelist(HttpServletRequest request) { String whitelist = null; @@ -66,27 +67,13 @@ public class WhitelistUtils { private static String deriveDefaultDispatchWhitelist(HttpServletRequest request) { String defaultWhitelist = null; - String thisHost = request.getHeader("X-Forwarded-Host"); - if (thisHost == null) { - thisHost = request.getServerName(); - } - - // If the host is not some form of localhost, try to determine its domain - if (!thisHost.matches(LOCALHOST_REGEXP)) { - int domainIndex = thisHost.indexOf('.'); - if (domainIndex > 0) { - String domain = thisHost.substring(thisHost.indexOf('.')); - // Sometimes, the server name includes port details, which need to be stripped - int portIndex = domain.indexOf(":"); - if (portIndex > 0) { - domain = domain.substring(0, portIndex); - } - String domainPattern = ".+" + domain.replaceAll("\\.", "\\\\."); - defaultWhitelist = String.format(DEFAULT_DISPATCH_WHITELIST_TEMPLATE, domainPattern); - } + try { + defaultWhitelist = deriveDomainBasedWhitelist(InetAddress.getLocalHost().getCanonicalHostName()); + } catch (UnknownHostException e) { + // } - // If the host is localhost or the domain could not be determined, default to the local/relative whitelist + // If the domain could not be determined, default to just the local/relative whitelist if (defaultWhitelist == null) { defaultWhitelist = String.format(DEFAULT_DISPATCH_WHITELIST_TEMPLATE, LOCALHOST_REGEXP_SEGMENT); } @@ -94,5 +81,16 @@ public class WhitelistUtils { return defaultWhitelist; } + private static String deriveDomainBasedWhitelist(String hostname) { + String whitelist = null; + int domainIndex = hostname.indexOf('.'); + if (domainIndex > 0) { + String domain = hostname.substring(hostname.indexOf('.')); + String domainPattern = ".+" + domain.replaceAll("\\.", "\\\\."); + whitelist = + String.format(DEFAULT_DISPATCH_WHITELIST_TEMPLATE, LOCALHOST_REGEXP_SEGMENT + "|(" + domainPattern + ")"); + } + return whitelist; + } } http://git-wip-us.apache.org/repos/asf/knox/blob/b2ec86f7/gateway-spi/src/test/java/org/apache/knox/gateway/dispatch/GatewayDispatchFilterTest.java ---------------------------------------------------------------------- diff --git a/gateway-spi/src/test/java/org/apache/knox/gateway/dispatch/GatewayDispatchFilterTest.java b/gateway-spi/src/test/java/org/apache/knox/gateway/dispatch/GatewayDispatchFilterTest.java index 69d2453..1ad7b18 100644 --- a/gateway-spi/src/test/java/org/apache/knox/gateway/dispatch/GatewayDispatchFilterTest.java +++ b/gateway-spi/src/test/java/org/apache/knox/gateway/dispatch/GatewayDispatchFilterTest.java @@ -123,21 +123,6 @@ public class GatewayDispatchFilterTest { /** * If the dispatch service is configured to honor the whitelist, but no whitelist is configured, then the default - * whitelist should be applied. If the dispatch URL matches the default domain-based whitelist, then the dispatch - * should be permitted. - */ - @Test - public void testServiceDispatchWhitelistNoWhiteListForRole_valid_domain() throws Exception { - final String serviceRole = "TESTROLE"; - doTestServiceDispatchWhitelist(Collections.singletonList(serviceRole), - "knoxbox.test.org", - null, - serviceRole, - "http://onmylist.test.org:9999", true); - } - - /** - * If the dispatch service is configured to honor the whitelist, but no whitelist is configured, then the default * whitelist should be applied. If the dispatch URL does match the default whitelist, then the dispatch should be * allowed. */ @@ -147,7 +132,8 @@ public class GatewayDispatchFilterTest { doTestServiceDispatchWhitelist(Collections.singletonList(serviceRole), null, serviceRole, - "http://localhost:9999", true); + "http://localhost:9999", + true); } /** http://git-wip-us.apache.org/repos/asf/knox/blob/b2ec86f7/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 34a1c6c..1824fe6 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 @@ -22,6 +22,7 @@ import org.junit.Test; import javax.servlet.ServletContext; import javax.servlet.http.HttpServletRequest; +import java.lang.reflect.Method; import java.util.Collections; import java.util.List; @@ -65,22 +66,6 @@ public class WhitelistUtilsTest { assertTrue(whitelist.contains("localhost")); } - /** - * KNOX-1369 - */ - @Test - public void testDomainBasedDefaultForAffectedServiceRoleWhenServerNameIncludesPort() throws Exception { - final String serviceRole = "TEST"; - - GatewayConfig config = createMockGatewayConfig(Collections.singletonList(serviceRole), null); - - // Check localhost by loopback address - String whitelist = doTestGetDispatchWhitelist(config, "host.test.com:1234", serviceRole); - assertNotNull(whitelist); - assertTrue(whitelist.contains(".+\\.test\\.com")); - assertFalse(whitelist.contains(":1234")); - } - @Test public void testDefaultDomainWhitelist() throws Exception { final String serviceRole = "TEST"; @@ -94,19 +79,6 @@ public class WhitelistUtilsTest { } @Test - public void testDefaultProxiedDomainWhitelist() throws Exception { - final String serviceRole = "TEST"; - - String whitelist = - doTestGetDispatchWhitelist(createMockGatewayConfig(Collections.singletonList(serviceRole), null), - "host0.test.org", - "forwarded-host.proxy.org", - serviceRole); - assertNotNull(whitelist); - assertTrue(whitelist.contains("\\.proxy\\.org")); - } - - @Test public void testConfiguredWhitelist() throws Exception { final String serviceRole = "TEST"; final String WHITELIST = "^.*\\.my\\.domain\\.com.*$"; @@ -139,25 +111,29 @@ public class WhitelistUtilsTest { private String doTestGetDispatchWhitelist(GatewayConfig config, String serverName, String serviceRole) { - return doTestGetDispatchWhitelist(config, serverName, null, serviceRole); - } - - private String doTestGetDispatchWhitelist(GatewayConfig config, - String serverName, - String xForwardedHost, - String serviceRole) { ServletContext sc = EasyMock.createNiceMock(ServletContext.class); EasyMock.expect(sc.getAttribute("org.apache.knox.gateway.config")).andReturn(config).anyTimes(); EasyMock.replay(sc); HttpServletRequest request = EasyMock.createNiceMock(HttpServletRequest.class); - EasyMock.expect(request.getServerName()).andReturn(serverName).anyTimes(); - EasyMock.expect(request.getHeader("X-Forwarded-Host")).andReturn(xForwardedHost).anyTimes(); EasyMock.expect(request.getAttribute("targetServiceRole")).andReturn(serviceRole).anyTimes(); EasyMock.expect(request.getServletContext()).andReturn(sc).anyTimes(); EasyMock.replay(request); - return WhitelistUtils.getDispatchWhitelist(request); + String result = null; + if (serverName != null && !serverName.isEmpty() && !serverName.equalsIgnoreCase("localhost")) { + try { + Method method = WhitelistUtils.class.getDeclaredMethod("deriveDomainBasedWhitelist", String.class); + method.setAccessible(true); + result = (String) method.invoke(null, serverName); + } catch (Exception e) { + e.printStackTrace(); + } + } else { + result = WhitelistUtils.getDispatchWhitelist(request); + } + + return result; }