KNOX-1372 - Default whitelist validation mistreats simple host names
authorPhil Zampino <pzampino@apache.org>
Mon, 2 Jul 2018 17:14:28 +0000 (13:14 -0400)
committerPhil Zampino <pzampino@apache.org>
Mon, 2 Jul 2018 17:14:28 +0000 (13:14 -0400)
gateway-service-knoxsso/src/test/java/org/apache/knox/gateway/service/knoxsso/WebSSOResourceTest.java
gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/GatewayDispatchFilter.java
gateway-spi/src/main/java/org/apache/knox/gateway/util/WhitelistUtils.java
gateway-spi/src/test/java/org/apache/knox/gateway/dispatch/GatewayDispatchFilterTest.java
gateway-spi/src/test/java/org/apache/knox/gateway/util/WhitelistUtilsTest.java

index 65b3a26..58877e4 100644 (file)
@@ -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";
 
index 0a993a0..f4d8383 100644 (file)
@@ -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);
         }
index e1f32be..220e448 100644 (file)
@@ -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;
+  }
 
 }
index 69d2453..1ad7b18 100644 (file)
@@ -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);
   }
 
   /**
index 34a1c6c..1824fe6 100644 (file)
@@ -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;
   }