Skip to content

KNOX-3132: Improve URL checks for originalUrl #1027

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

KNOX-3132: Improve URL checks for originalUrl #1027

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions gateway-applications/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@
<groupId>org.apache.knox</groupId>
<artifactId>knox-webshell-ui</artifactId>
</dependency>
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-text</artifactId>
</dependency>
</dependencies>

<build>
Expand Down
6 changes: 5 additions & 1 deletion gateway-applications/src/main/resources/applications/knoxauth/app/logout.jsp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
<%@ page import="org.apache.knox.gateway.config.GatewayConfig" %>
<%@ page import="java.net.MalformedURLException" %>
<%@ page import="org.apache.knox.gateway.util.Urls" %>
<%@ page import="org.apache.commons.text.StringEscapeUtils" %>

<!DOCTYPE html>
<!--[if lt IE 7]><html class="no-js lt-ie9 lt-ie8 lt-ie7"><![endif]-->
Expand Down Expand Up @@ -93,6 +94,9 @@
if (origUrl != null) {
validRedirect = RegExUtils.checkWhitelist(whitelist, origUrl);
}
if (validRedirect) {
validRedirect = Urls.isValidURL(originalUrl);
}
if (("1".equals(request.getParameter("returnToApp")))) {
if (validRedirect) {
response.setStatus(HttpServletResponse.SC_MOVED_PERMANENTLY);
Expand Down Expand Up @@ -178,7 +182,7 @@
the application. If your previously established SSO session is still valid then
you will likely be automatically logged into your application. Otherwise, you
will be required to login again.
<a href="?returnToApp=1&originalUrl=<%= originalUrl %>" >Return to Application</a>
<a href="?returnToApp=1&originalUrl=<%= StringEscapeUtils.escapeHtml4(originalUrl) %>" >Return to Application</a>
</p>
<%
if (globalLogoutPageURL != null && !globalLogoutPageURL.isEmpty()) {
Expand Down
8 changes: 6 additions & 2 deletions gateway-applications/src/main/resources/applications/knoxauth/app/redirecting.jsp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
<%@ page import="org.apache.knox.gateway.util.RegExUtils" %>
<%@ page import="org.apache.knox.gateway.util.Urls" %>
<%@ page import="org.apache.knox.gateway.util.WhitelistUtils" %>
<%@ page import="org.apache.commons.text.StringEscapeUtils" %>

<!DOCTYPE html>
<!--[if lt IE 7]><html class="no-js lt-ie9 lt-ie8 lt-ie7"><![endif]-->
Expand Down Expand Up @@ -57,6 +58,9 @@
// if not a well formed URL then not a valid redirect
validRedirect = false;
}
if (validRedirect) {
validRedirect = Urls.isValidURL(originalUrl);
}
if (validRedirect) {
Topology topology = (Topology)request.getSession().getServletContext().getAttribute("org.apache.knox.gateway.topology");
String whitelist = null;
Expand All @@ -82,7 +86,7 @@
document.addEventListener("load", redirectOnLoad());

function redirectOnLoad() {
var originalUrl = "<%= originalUrl %>";
var originalUrl = "<%= StringEscapeUtils.escapeEcmaScript(originalUrl) %>";
if (originalUrl != null) {
redirect(originalUrl);
}
Expand All @@ -103,7 +107,7 @@
<div style="background: white;" class="l-logo">
<img src="images/loading.gif" alt="Knox logo" style="text-align:center;width: 2%; height: 2%">
</div>
<p style="color: white;display: block">Loading should complete in few a seconds. If not, click <a href="<%= originalUrl %>">here</a></p>
<p style="color: white;display: block">Loading should complete in few a seconds. If not, click <a href="<%= StringEscapeUtils.escapeHtml4(originalUrl) %>">here</a></p>
<%
} else {
%>
Expand Down
15 changes: 15 additions & 0 deletions gateway-util-common/src/main/java/org/apache/knox/gateway/util/Urls.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import java.io.UnsupportedEncodingException;
import java.net.MalformedURLException;
import java.net.URISyntaxException;
import java.net.URL;
import java.nio.charset.StandardCharsets;
import java.util.regex.Matcher;
Expand Down Expand Up @@ -81,6 +82,20 @@ public static boolean containsUserInfo(String url) throws MalformedURLException
return (new URL(url).getUserInfo() != null);
}

/**
* Checks if a given URL is valid according to RFC2396.
* @param url a URL
* @return true if URL conforms to RFC2396, false otherwise.
*/
public static boolean isValidURL(String url) {
try {
new URL(url).toURI();
return true;
} catch (MalformedURLException | URISyntaxException e) {
return false;
}
}

/**
* Compute the domain name from an URL.
*
Expand Down
20 changes: 20 additions & 0 deletions gateway-util-common/src/test/java/org/apache/knox/gateway/util/UrlsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,14 @@

public class UrlsTest {

private static final String INVALID_ORIGINAL_URL = "https://localhost:8443/gateway/homepage/homeisyr\">";
private static final String VALID_LOCAL_ORIGINAL_URL = "https://localhost:8443/gateway/homepage/home";
private static final String VALID_ORIGINAL_URL = "https://knoxhost.site:8443/gateway/homepage/home";
private static final String VALID_ORIGINAL_URL_SOLR = "https://knoxhost.site:8443/gateway/cdp-proxy/solr/";
private static final String LOGOUT_LOCAL_ORIGINAL_URL = "https://localhost:8443/gateway/homepage/home?profile=token";
private static final String LOGOUT_ORIGINAL_URL_WITH_PARAMETERS = "https://knoxhost.site:8443/gateway/homepage/session/api/v1/sessioninfo?logoutPageProfile=token&logoutPageTopologies=cdp-proxy-token";
private static final String LOGOUT_ORIGINAL_URL_WITH_PARAMETERS_ENCODED = "https://knoxhost.site:8443/gateway/homepage/session/api/v1/sessioninfo%3FlogoutPageProfile=token%26logoutPageTopologies=cdp-proxy-token";

/*
* Domain name creation follows the following algorithm:
* 1. if the incoming request hostname endsWith a configured domain suffix return the suffix - with prefixed dot
Expand Down Expand Up @@ -101,4 +109,16 @@ public void testContainsUserInfo() throws Exception {
assertTrue(Urls.containsUserInfo( "https://www.local.com:[email protected]"));
assertFalse(Urls.containsUserInfo( "https://www.local.com:8443/google.com"));
}

@Test
public void testValidUrl() throws Exception {
assertFalse(Urls.isValidURL(INVALID_ORIGINAL_URL));
assertTrue(Urls.isValidURL(VALID_LOCAL_ORIGINAL_URL));
assertTrue(Urls.isValidURL(VALID_ORIGINAL_URL));
assertTrue(Urls.isValidURL(VALID_ORIGINAL_URL_SOLR));
assertTrue(Urls.isValidURL(LOGOUT_LOCAL_ORIGINAL_URL));
assertTrue(Urls.isValidURL(LOGOUT_ORIGINAL_URL_WITH_PARAMETERS));
assertTrue(Urls.isValidURL(LOGOUT_ORIGINAL_URL_WITH_PARAMETERS_ENCODED));
}

}