From e62aa0252dfcf34dff0c3a9c51265b1d0f9dfc9f Mon Sep 17 00:00:00 2001 From: Andy LoPresto Date: Wed, 18 Jul 2018 16:08:31 -0700 Subject: [PATCH] NIFI-5442 Get X-ProxyContextPath value from request attributes rather than directly from headers. NIFI-5442 Populate request contextPath attribute during AccessResource before displaying on message-page.jsp. Refactored shared code from CatchAllFilter to WebUtils. NIFI-5442 Refactored filter and context path code to shared parent filter and subclass. NIFI-5442 Removed unnecessary initParams from nifi-web-ui web.xml. NIFI-5442 Added explicit dispatchers to nifi-web-ui web.xml and removed unnecessary code from AccessResource. This closes #2908 --- .../web/filter/SanitizeContextPathFilter.java | 81 +++++++++++++++++++ .../org/apache/nifi/web/util/WebUtils.java | 25 +++++- .../apache/nifi/web/server/JettyServer.java | 70 ++++++++-------- .../apache/nifi/web/api/AccessResource.java | 8 +- .../nifi/web/filter/CatchAllFilter.java | 52 ++++-------- .../src/main/webapp/WEB-INF/web.xml | 8 ++ .../nifi-web/nifi-web-ui/pom.xml | 5 ++ .../webapp/WEB-INF/pages/message-page.jsp | 17 ++-- .../src/main/webapp/WEB-INF/web.xml | 15 ++++ 9 files changed, 192 insertions(+), 89 deletions(-) create mode 100644 nifi-commons/nifi-web-utils/src/main/java/org/apache/nifi/web/filter/SanitizeContextPathFilter.java diff --git a/nifi-commons/nifi-web-utils/src/main/java/org/apache/nifi/web/filter/SanitizeContextPathFilter.java b/nifi-commons/nifi-web-utils/src/main/java/org/apache/nifi/web/filter/SanitizeContextPathFilter.java new file mode 100644 index 0000000000..00b98d2bc1 --- /dev/null +++ b/nifi-commons/nifi-web-utils/src/main/java/org/apache/nifi/web/filter/SanitizeContextPathFilter.java @@ -0,0 +1,81 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.web.filter; + +import java.io.IOException; +import javax.servlet.Filter; +import javax.servlet.FilterChain; +import javax.servlet.FilterConfig; +import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; +import org.apache.nifi.web.util.WebUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * This filter intercepts a request and populates the {@code contextPath} attribute on the request with a sanitized value (originally) retrieved from {@code nifi.properties}. + */ +public class SanitizeContextPathFilter implements Filter { + private static final Logger logger = LoggerFactory.getLogger(SanitizeContextPathFilter.class); + + private String whitelistedContextPaths = ""; + + + @Override + public void init(FilterConfig filterConfig) throws ServletException { + String providedWhitelist = filterConfig.getServletContext().getInitParameter("whitelistedContextPaths"); + logger.debug("SanitizeContextPathFilter received provided whitelisted context paths from NiFi properties: " + providedWhitelist); + if (providedWhitelist != null) { + whitelistedContextPaths = providedWhitelist; + } + } + + @Override + public void doFilter(ServletRequest request, ServletResponse response, FilterChain filterChain) throws IOException, ServletException { + // Inject the contextPath attribute into the request + injectContextPathAttribute(request); + + // Pass execution to the next filter in the chain + filterChain.doFilter(request, response); + } + + /** + * Determines, sanitizes, and injects the {@code contextPath} attribute into the {@code request}. If not present, an empty string {@code ""} is injected. + * @param request the request + */ + protected void injectContextPathAttribute(ServletRequest request) { + // Capture the provided context path headers and sanitize them before using in the response + String contextPath = WebUtils.sanitizeContextPath(request, whitelistedContextPaths, ""); + request.setAttribute("contextPath", contextPath); + + logger.debug("SanitizeContextPathFilter set contextPath: " + contextPath); + } + + @Override + public void destroy() { + } + + /** + * Getter for whitelistedContextPaths. Cannot be package-private because of an issue where the package is scoped per classloader. + * + * @return the whitelisted context path(s) + */ + protected String getWhitelistedContextPaths() { + return whitelistedContextPaths; + } +} diff --git a/nifi-commons/nifi-web-utils/src/main/java/org/apache/nifi/web/util/WebUtils.java b/nifi-commons/nifi-web-utils/src/main/java/org/apache/nifi/web/util/WebUtils.java index 21a41fdc5b..cbf64e6551 100644 --- a/nifi-commons/nifi-web-utils/src/main/java/org/apache/nifi/web/util/WebUtils.java +++ b/nifi-commons/nifi-web-utils/src/main/java/org/apache/nifi/web/util/WebUtils.java @@ -21,13 +21,12 @@ import java.util.Arrays; import java.util.List; import java.util.concurrent.locks.ReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock; - import javax.net.ssl.SSLContext; +import javax.servlet.ServletRequest; import javax.servlet.http.HttpServletRequest; import javax.ws.rs.client.Client; import javax.ws.rs.client.ClientBuilder; import javax.ws.rs.core.UriBuilderException; - import org.apache.commons.lang3.StringUtils; import org.glassfish.jersey.client.ClientConfig; import org.glassfish.jersey.jackson.internal.jackson.jaxrs.json.JacksonJaxbJsonProvider; @@ -176,6 +175,28 @@ public final class WebUtils { } } + /** + * Returns a "safe" context path value from the request headers to use in a proxy environment. + * This is used on the JSP to build the resource paths for the external resources (CSS, JS, etc.). + * If no headers are present specifying this value, it is an empty string. + * + * @param request the HTTP request + * @return the context path safe to be printed to the page + */ + public static String sanitizeContextPath(ServletRequest request, String whitelistedContextPaths, String jspDisplayName) { + if (StringUtils.isBlank(jspDisplayName)) { + jspDisplayName = "JSP page"; + } + String contextPath = normalizeContextPath(determineContextPath((HttpServletRequest) request)); + try { + verifyContextPath(whitelistedContextPaths, contextPath); + return contextPath; + } catch (UriBuilderException e) { + logger.error("Error determining context path on " + jspDisplayName + ": " + e.getMessage()); + return ""; + } + } + /** * Determines the context path if populated in {@code X-ProxyContextPath} or {@code X-ForwardContext} headers. If not populated, returns an empty string. * diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/JettyServer.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/JettyServer.java index 2f93ec87a9..971353b711 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/JettyServer.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/JettyServer.java @@ -18,6 +18,40 @@ package org.apache.nifi.web.server; import com.google.common.base.Strings; import com.google.common.collect.Lists; +import java.io.BufferedReader; +import java.io.File; +import java.io.FileFilter; +import java.io.IOException; +import java.io.InputStreamReader; +import java.net.InetAddress; +import java.net.NetworkInterface; +import java.net.SocketException; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.EnumSet; +import java.util.Enumeration; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.concurrent.TimeUnit; +import java.util.jar.JarEntry; +import java.util.jar.JarFile; +import java.util.stream.Collectors; +import javax.servlet.DispatcherType; +import javax.servlet.Filter; +import javax.servlet.FilterChain; +import javax.servlet.FilterConfig; +import javax.servlet.ServletContext; +import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServletResponse; import org.apache.commons.collections4.CollectionUtils; import org.apache.commons.lang3.StringUtils; import org.apache.nifi.NiFiServer; @@ -68,41 +102,6 @@ import org.springframework.context.ApplicationContext; import org.springframework.web.context.WebApplicationContext; import org.springframework.web.context.support.WebApplicationContextUtils; -import javax.servlet.DispatcherType; -import javax.servlet.Filter; -import javax.servlet.FilterChain; -import javax.servlet.FilterConfig; -import javax.servlet.ServletContext; -import javax.servlet.ServletException; -import javax.servlet.ServletRequest; -import javax.servlet.ServletResponse; -import javax.servlet.http.HttpServletResponse; -import java.io.BufferedReader; -import java.io.File; -import java.io.FileFilter; -import java.io.IOException; -import java.io.InputStreamReader; -import java.net.InetAddress; -import java.net.NetworkInterface; -import java.net.SocketException; -import java.nio.file.Paths; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.EnumSet; -import java.util.Enumeration; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Objects; -import java.util.Set; -import java.util.concurrent.TimeUnit; -import java.util.jar.JarEntry; -import java.util.jar.JarFile; -import java.util.stream.Collectors; - /** * Encapsulates the Jetty instance. */ @@ -317,6 +316,7 @@ public class JettyServer implements NiFiServer { final WebAppContext webUiContext = loadWar(webUiWar, "/nifi", frameworkClassLoader); webUiContext.getInitParams().put("oidc-supported", String.valueOf(props.isOidcEnabled())); webUiContext.getInitParams().put("knox-supported", String.valueOf(props.isKnoxSsoEnabled())); + webUiContext.getInitParams().put("whitelistedContextPaths", props.getWhitelistedContextPaths()); handlers.addHandler(webUiContext); // load the web api app diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/AccessResource.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/AccessResource.java index 236e14e6f4..f2dd6970f9 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/AccessResource.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/AccessResource.java @@ -340,8 +340,8 @@ public class AccessResource extends ApplicationResource { httpServletResponse.sendRedirect(postLogoutRedirectUri); } else { URI logoutUri = UriBuilder.fromUri(endSessionEndpoint) - .queryParam("post_logout_redirect_uri", postLogoutRedirectUri) - .build(); + .queryParam("post_logout_redirect_uri", postLogoutRedirectUri) + .build(); httpServletResponse.sendRedirect(logoutUri.toString()); } } @@ -765,7 +765,7 @@ public class AccessResource extends ApplicationResource { * Gets the value of a cookie matching the specified name. If no cookie with that name exists, null is returned. * * @param cookies the cookies - * @param name the name of the cookie + * @param name the name of the cookie * @return the value of the corresponding cookie, or null if the cookie does not exist */ private String getCookieValue(final Cookie[] cookies, final String name) { @@ -786,7 +786,7 @@ public class AccessResource extends ApplicationResource { private String getNiFiUri() { final String nifiApiUrl = generateResourceUri(); - final String baseUrl = StringUtils.substringBeforeLast(nifiApiUrl,"/nifi-api"); + final String baseUrl = StringUtils.substringBeforeLast(nifiApiUrl, "/nifi-api"); return baseUrl + "/nifi"; } diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-error/src/main/java/org/apache/nifi/web/filter/CatchAllFilter.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-error/src/main/java/org/apache/nifi/web/filter/CatchAllFilter.java index 71d1a2b46c..fa94e64f65 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-error/src/main/java/org/apache/nifi/web/filter/CatchAllFilter.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-error/src/main/java/org/apache/nifi/web/filter/CatchAllFilter.java @@ -17,65 +17,45 @@ package org.apache.nifi.web.filter; import java.io.IOException; -import javax.servlet.Filter; import javax.servlet.FilterChain; import javax.servlet.FilterConfig; import javax.servlet.ServletException; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; -import javax.servlet.http.HttpServletRequest; -import javax.ws.rs.core.UriBuilderException; -import org.apache.nifi.web.util.WebUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** - * Filter for forward all requests to index.jsp. + * This filter catches all requests and explicitly forwards them to a JSP ({@code index.jsp} injected in + * the filter configuration. This is used to handle all errors (this module is only used for errors). It + * extends {@link SanitizeContextPathFilter} which sanitizes the context path and injects it as a request + * attribute to be used on the page for linking resources without XSS vulnerabilities. */ -public class CatchAllFilter implements Filter { +public class CatchAllFilter extends SanitizeContextPathFilter { private static final Logger logger = LoggerFactory.getLogger(CatchAllFilter.class); - private static String whitelistedContextPaths = ""; + private String forwardPath = ""; + private String displayPath = ""; @Override public void init(FilterConfig filterConfig) throws ServletException { - String providedWhitelist = filterConfig.getServletContext().getInitParameter("whitelistedContextPaths"); - logger.debug("CatchAllFilter received provided whitelisted context paths from NiFi properties: " + providedWhitelist); - if (providedWhitelist != null) { - whitelistedContextPaths = providedWhitelist; - } + // TODO: Perform path validation (against what set of rules)? + forwardPath = filterConfig.getInitParameter("forwardPath"); + displayPath = filterConfig.getInitParameter("displayPath"); + + logger.debug("CatchAllFilter [" + displayPath + "] received provided whitelisted context paths from NiFi properties: " + getWhitelistedContextPaths()); } @Override public void doFilter(ServletRequest request, ServletResponse response, FilterChain filterChain) throws IOException, ServletException { - // Capture the provided context path headers and sanitize them before using in the response - String contextPath = getSanitizedContextPath(request); - request.setAttribute("contextPath", contextPath); + // Inject the contextPath attribute into the request + injectContextPathAttribute(request); - // for all requests to index.jsp - request.getRequestDispatcher("/index.jsp").forward(request, response); + // Forward all requests to index.jsp + request.getRequestDispatcher(forwardPath).forward(request, response); } @Override public void destroy() { } - - /** - * Returns a "safe" context path value from the request headers to use in a proxy environment. - * This is used on the JSP to build the resource paths for the external resources (CSS, JS, etc.). - * If no headers are present specifying this value, it is an empty string. - * - * @param request the HTTP request - * @return the context path safe to be printed to the page - */ - private String getSanitizedContextPath(ServletRequest request) { - String contextPath = WebUtils.normalizeContextPath(WebUtils.determineContextPath((HttpServletRequest) request)); - try { - WebUtils.verifyContextPath(whitelistedContextPaths, contextPath); - return contextPath; - } catch (UriBuilderException e) { - logger.error("Error determining context path on index.jsp: " + e.getMessage()); - return ""; - } - } } diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-error/src/main/webapp/WEB-INF/web.xml b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-error/src/main/webapp/WEB-INF/web.xml index 8921175ff3..3fb27e35b0 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-error/src/main/webapp/WEB-INF/web.xml +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-error/src/main/webapp/WEB-INF/web.xml @@ -18,6 +18,14 @@ catch-all-filter org.apache.nifi.web.filter.CatchAllFilter + + displayPath + index.jsp + + + forwardPath + /index.jsp + catch-all-filter diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/pom.xml b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/pom.xml index c713e41408..abeffa7d25 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/pom.xml +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/pom.xml @@ -923,6 +923,11 @@ nifi-utils provided + + org.apache.nifi + nifi-web-utils + provided + org.apache.commons commons-lang3 diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/src/main/webapp/WEB-INF/pages/message-page.jsp b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/src/main/webapp/WEB-INF/pages/message-page.jsp index 1c82a62fd3..9999ec0bb3 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/src/main/webapp/WEB-INF/pages/message-page.jsp +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/src/main/webapp/WEB-INF/pages/message-page.jsp @@ -17,18 +17,11 @@ <%@ page contentType="text/html" pageEncoding="UTF-8" session="false" %> - <% - String contextPath = request.getHeader("X-ProxyContextPath"); - if (contextPath == null) { - contextPath = request.getHeader("X-Forwarded-Context"); - } - if (contextPath == null) { - contextPath = ""; - } - if (contextPath.endsWith("/")) { - contextPath = contextPath.substring(0, contextPath.length() - 1); - } - %> +<% + // Sanitize the contextPath to ensure it is on this server + // rather than getting it from the header directly + String contextPath = request.getAttribute("contextPath").toString(); +%> <%= request.getAttribute("title") == null ? "" : org.apache.nifi.util.EscapeUtils.escapeHtml(request.getAttribute("title").toString()) %> diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/src/main/webapp/WEB-INF/web.xml b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/src/main/webapp/WEB-INF/web.xml index 3a0ce25dca..0639dd6123 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/src/main/webapp/WEB-INF/web.xml +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/src/main/webapp/WEB-INF/web.xml @@ -146,6 +146,21 @@ /logout + + + SanitizeContextPathFilter + org.apache.nifi.web.filter.SanitizeContextPathFilter + + + SanitizeContextPathFilter + /* + REQUEST + FORWARD + INCLUDE + ERROR + ASYNC + + canvas.jsp /WEB-INF/pages/canvas.jsp