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/03 21:35:41 UTC
knox git commit: KNOX-1375 - Dispatch whitelist validation should
decode URLs before testing
Repository: knox
Updated Branches:
refs/heads/master 2717b1574 -> e844eff90
KNOX-1375 - Dispatch whitelist validation should decode URLs before testing
Project: http://git-wip-us.apache.org/repos/asf/knox/repo
Commit: http://git-wip-us.apache.org/repos/asf/knox/commit/e844eff9
Tree: http://git-wip-us.apache.org/repos/asf/knox/tree/e844eff9
Diff: http://git-wip-us.apache.org/repos/asf/knox/diff/e844eff9
Branch: refs/heads/master
Commit: e844eff90b5993da01ab6027a93265d3f1f0077f
Parents: 2717b15
Author: Phil Zampino <pz...@apache.org>
Authored: Tue Jul 3 17:19:47 2018 -0400
Committer: Phil Zampino <pz...@apache.org>
Committed: Tue Jul 3 17:19:47 2018 -0400
----------------------------------------------------------------------
.../gateway/service/knoxsso/WebSSOResource.java | 24 ++++++--
.../service/knoxsso/WebSSOResourceTest.java | 60 ++++++++++++++++++++
.../gateway/dispatch/GatewayDispatchFilter.java | 11 +++-
.../knox/gateway/util/WhitelistUtils.java | 3 +-
.../dispatch/GatewayDispatchFilterTest.java | 39 ++++++++++++-
5 files changed, 129 insertions(+), 8 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/knox/blob/e844eff9/gateway-service-knoxsso/src/main/java/org/apache/knox/gateway/service/knoxsso/WebSSOResource.java
----------------------------------------------------------------------
diff --git a/gateway-service-knoxsso/src/main/java/org/apache/knox/gateway/service/knoxsso/WebSSOResource.java b/gateway-service-knoxsso/src/main/java/org/apache/knox/gateway/service/knoxsso/WebSSOResource.java
index f207432..cda3d48 100644
--- a/gateway-service-knoxsso/src/main/java/org/apache/knox/gateway/service/knoxsso/WebSSOResource.java
+++ b/gateway-service-knoxsso/src/main/java/org/apache/knox/gateway/service/knoxsso/WebSSOResource.java
@@ -18,8 +18,10 @@
package org.apache.knox.gateway.service.knoxsso;
import java.io.IOException;
+import java.io.UnsupportedEncodingException;
import java.net.URI;
import java.net.URISyntaxException;
+import java.net.URLDecoder;
import java.security.Principal;
import java.util.ArrayList;
import java.util.Arrays;
@@ -168,8 +170,8 @@ public class WebSSOResource {
}
private Response getAuthenticationToken(int statusCode) {
- GatewayServices services = (GatewayServices) request.getServletContext()
- .getAttribute(GatewayServices.GATEWAY_SERVICES_ATTRIBUTE);
+ GatewayServices services =
+ (GatewayServices) request.getServletContext().getAttribute(GatewayServices.GATEWAY_SERVICES_ATTRIBUTE);
boolean removeOriginalUrlCookie = true;
String original = getCookieValue((HttpServletRequest) request, ORIGINAL_URL_COOKIE_NAME);
if (original == null) {
@@ -182,11 +184,25 @@ public class WebSSOResource {
throw new WebApplicationException("Original URL not found in the request.", Response.Status.BAD_REQUEST);
}
- boolean validRedirect = (whitelist == null) || RegExUtils.checkWhitelist(whitelist, original);
+ boolean validRedirect = true;
+
+ // If there is a whitelist defined, then the original URL must be validated against it.
+ // If there is no whitelist, then everything is valid.
+ if (whitelist != null) {
+ String decodedOriginal = null;
+ try {
+ decodedOriginal = URLDecoder.decode(original, "UTF-8");
+ } catch (UnsupportedEncodingException e) {
+ //
+ }
+
+ validRedirect = RegExUtils.checkWhitelist(whitelist, (decodedOriginal != null ? decodedOriginal : original));
+ }
+
if (!validRedirect) {
log.whiteListMatchFail(original, whitelist);
throw new WebApplicationException("Original URL not valid according to the configured whitelist.",
- Response.Status.BAD_REQUEST);
+ Response.Status.BAD_REQUEST);
}
}
http://git-wip-us.apache.org/repos/asf/knox/blob/e844eff9/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 58877e4..19a8f03 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
@@ -17,6 +17,7 @@
*/
package org.apache.knox.gateway.service.knoxsso;
+import org.apache.http.HttpStatus;
import org.apache.knox.gateway.config.GatewayConfig;
import org.apache.knox.gateway.util.RegExUtils;
import static org.junit.Assert.assertEquals;
@@ -25,6 +26,7 @@ import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import java.lang.reflect.Field;
+import java.net.URLEncoder;
import java.security.KeyPair;
import java.security.KeyPairGenerator;
import java.security.NoSuchAlgorithmException;
@@ -46,6 +48,7 @@ import javax.servlet.http.Cookie;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.HttpServletResponseWrapper;
+import javax.ws.rs.WebApplicationException;
import org.apache.knox.gateway.services.GatewayServices;
import org.apache.knox.gateway.services.security.token.JWTokenAuthority;
@@ -645,6 +648,63 @@ public class WebSSOResourceTest {
}
@Test
+ public void testWhitelistValidationWithEncodedOriginalURL() throws Exception {
+ GatewayConfig config = EasyMock.createNiceMock(GatewayConfig.class);
+ EasyMock.expect(config.getDispatchWhitelistServices()).andReturn(Collections.emptyList()).anyTimes();
+ EasyMock.expect(config.getDispatchWhitelist()).andReturn(null).anyTimes();
+ EasyMock.replay(config);
+
+ ServletContext context = EasyMock.createNiceMock(ServletContext.class);
+ EasyMock.expect(context.getAttribute(GatewayConfig.GATEWAY_CONFIG_ATTRIBUTE)).andReturn(config).anyTimes();
+ EasyMock.expect(context.getInitParameter("knoxsso.cookie.name")).andReturn(null);
+ EasyMock.expect(context.getInitParameter("knoxsso.cookie.secure.only")).andReturn(null);
+ EasyMock.expect(context.getInitParameter("knoxsso.cookie.max.age")).andReturn(null);
+ EasyMock.expect(context.getInitParameter("knoxsso.cookie.domain.suffix")).andReturn(null);
+ EasyMock.expect(context.getInitParameter("knoxsso.redirect.whitelist.regex")).andReturn(null);
+ EasyMock.expect(context.getInitParameter("knoxsso.token.audiences")).andReturn(null);
+ EasyMock.expect(context.getInitParameter("knoxsso.token.ttl")).andReturn(null);
+ EasyMock.expect(context.getInitParameter("knoxsso.enable.session")).andReturn(null);
+
+ HttpServletRequest request = EasyMock.createNiceMock(HttpServletRequest.class);
+ EasyMock.expect(request.getParameter("originalUrl")).andReturn(URLEncoder.encode("http://disallowedhost:9080/service",
+ "UTF-8"));
+ EasyMock.expect(request.getAttribute("targetServiceRole")).andReturn("KNOXSSO").anyTimes();
+ EasyMock.expect(request.getParameterMap()).andReturn(Collections.<String,String[]>emptyMap());
+ EasyMock.expect(request.getServletContext()).andReturn(context).anyTimes();
+
+ Principal principal = EasyMock.createNiceMock(Principal.class);
+ EasyMock.expect(principal.getName()).andReturn("alice").anyTimes();
+ EasyMock.expect(request.getUserPrincipal()).andReturn(principal).anyTimes();
+
+ GatewayServices services = EasyMock.createNiceMock(GatewayServices.class);
+ EasyMock.expect(context.getAttribute(GatewayServices.GATEWAY_SERVICES_ATTRIBUTE)).andReturn(services);
+
+ JWTokenAuthority authority = new TestJWTokenAuthority(publicKey, privateKey);
+ EasyMock.expect(services.getService(GatewayServices.TOKEN_SERVICE)).andReturn(authority);
+
+ HttpServletResponse response = EasyMock.createNiceMock(HttpServletResponse.class);
+ ServletOutputStream outputStream = EasyMock.createNiceMock(ServletOutputStream.class);
+ CookieResponseWrapper responseWrapper = new CookieResponseWrapper(response, outputStream);
+
+ EasyMock.replay(principal, services, context, request);
+
+ WebSSOResource webSSOResponse = new WebSSOResource();
+ webSSOResponse.request = request;
+ webSSOResponse.response = responseWrapper;
+ webSSOResponse.context = context;
+ webSSOResponse.init();
+
+ try {
+ webSSOResponse.doGet();
+ } catch (WebApplicationException e) {
+ // Expected
+ int status = e.getResponse().getStatus();
+ assertEquals(HttpStatus.SC_BAD_REQUEST, status);
+ }
+ }
+
+
+ @Test
public void testTopologyDefinedWhitelist() throws Exception {
final String testServiceRole = "TEST";
http://git-wip-us.apache.org/repos/asf/knox/blob/e844eff9/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 f4d8383..f23ba55 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
@@ -32,7 +32,9 @@ import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.io.IOException;
+import java.io.UnsupportedEncodingException;
import java.net.URISyntaxException;
+import java.net.URLDecoder;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
@@ -140,7 +142,14 @@ public class GatewayDispatchFilter extends AbstractGatewayFilter {
if (whitelist != null) {
String requestURI = request.getRequestURI();
- isAllowed = RegExUtils.checkWhitelist(whitelist, requestURI);
+ String decodedURL = null;
+ try {
+ decodedURL = URLDecoder.decode(requestURI, "UTF-8");
+ } catch (UnsupportedEncodingException e) {
+ //
+ }
+
+ isAllowed = RegExUtils.checkWhitelist(whitelist, (decodedURL != null ? decodedURL : requestURI));
if (!isAllowed) {
LOG.dispatchDisallowed(requestURI);
http://git-wip-us.apache.org/repos/asf/knox/blob/e844eff9/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 37df2f6..ae5e519 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
@@ -44,7 +44,8 @@ public class WhitelistUtils {
public static String getDispatchWhitelist(HttpServletRequest request) {
String whitelist = null;
- GatewayConfig config = (GatewayConfig) request.getServletContext().getAttribute("org.apache.knox.gateway.config");
+ GatewayConfig config =
+ (GatewayConfig) request.getServletContext().getAttribute(GatewayConfig.GATEWAY_CONFIG_ATTRIBUTE);
if (config != null) {
List<String> whitelistedServiceRoles = new ArrayList<>();
whitelistedServiceRoles.addAll(DEFAULT_SERVICE_ROLES);
http://git-wip-us.apache.org/repos/asf/knox/blob/e844eff9/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 1ad7b18..02f6fa1 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
@@ -26,6 +26,7 @@ import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.io.IOException;
import java.lang.reflect.Method;
+import java.net.URLEncoder;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
@@ -88,7 +89,8 @@ public class GatewayDispatchFilterTest {
doTestServiceDispatchWhitelist(Collections.singletonList(serviceRole),
null,
serviceRole,
- "http://www.notonmylist.org:9999", false);
+ "http://www.notonmylist.org:9999",
+ false);
}
/**
@@ -118,7 +120,8 @@ public class GatewayDispatchFilterTest {
"knoxbox.test.org",
null,
serviceRole,
- "http://www.notonmylist.org:9999", false);
+ "http://www.notonmylist.org:9999",
+ false);
}
/**
@@ -137,6 +140,38 @@ 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 does match the default whitelist, then the dispatch should be
+ * allowed.
+ */
+ @Test
+ public void testServiceDispatchWhitelistNoWhiteListForRole_encodedurl_valid() throws Exception {
+ final String serviceRole = "TESTROLE";
+ doTestServiceDispatchWhitelist(Collections.singletonList(serviceRole),
+ null,
+ serviceRole,
+ URLEncoder.encode("http://localhost:9999", "UTF-8"),
+ true);
+ }
+
+
+ /**
+ * If the dispatch service is configured to honor the whitelist, but DEFAULT 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.
+ */
+ @Test
+ public void testServiceDispatchWhitelistNoWhiteListForRole_encodedurl_invalid() throws Exception {
+ final String serviceRole = "TESTROLE";
+ doTestServiceDispatchWhitelist(Collections.singletonList(serviceRole),
+ "DEFAULT",
+ serviceRole,
+ URLEncoder.encode("http://www.notonmylist.org:9999", "UTF-8"),
+ false);
+ }
+
+
+ /**
* An empty whitelist should be treated as the default whitelist.
*/
@Test