Run the security filters on every dispatch type

openam-core/src/main/java/com/sun/identity/jaxrpc/JAXRPCRequestFilter.java

@@ -126,16 +126,22 @@ public void init(FilterConfig filterConfig) throws ServletException {
     @Override
     public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
             throws IOException, ServletException {
-        boolean bound = false;
-        if (request instanceof HttpServletRequest) {
-            CURRENT_REQUEST.set((HttpServletRequest) request);
-            bound = true;
+        if (!(request instanceof HttpServletRequest)) {
+            chain.doFilter(request, response);
+            return;
         }
+        // The filter is mapped for FORWARD and INCLUDE dispatches as well, so a nested
+        // dispatch into /jaxrpc/* re-enters it on the same thread: restore the outer binding
+        // on exit rather than clearing it.
+        HttpServletRequest previous = CURRENT_REQUEST.get();
+        CURRENT_REQUEST.set((HttpServletRequest) request);
         try {
             chain.doFilter(request, response);
         } finally {
-            if (bound) {
+            if (previous == null) {
                 CURRENT_REQUEST.remove();
+            } else {
+                CURRENT_REQUEST.set(previous);
             }
         }
     }

openam-core/src/test/java/com/sun/identity/jaxrpc/JAXRPCRequestFilterTest.java

@@ -72,4 +72,27 @@ public void filterBindsRequestForTheChainAndClearsItAfterwards() throws Exceptio
         assertNull(JAXRPCRequestFilter.getCurrentRequest(),
                 "request must be cleared once the filter chain completes");
     }
+
+    @Test
+    public void nestedDispatchRestoresTheOuterRequestOnExit() throws Exception {
+        // The filter is mapped for FORWARD and INCLUDE dispatches as well, so a nested
+        // dispatch into /jaxrpc/* re-enters it on the same thread; the outer request
+        // must be bound again once the nested chain returns.
+        HttpServletRequest outer = mock(HttpServletRequest.class);
+        HttpServletRequest inner = mock(HttpServletRequest.class);
+        HttpServletResponse response = mock(HttpServletResponse.class);
+        JAXRPCRequestFilter filter = new JAXRPCRequestFilter();
+        final HttpServletRequest[] seen = new HttpServletRequest[2];
+        FilterChain outerChain = (req, res) -> {
+            filter.doFilter(inner, res, (r, s) -> seen[0] = JAXRPCRequestFilter.getCurrentRequest());
+            seen[1] = JAXRPCRequestFilter.getCurrentRequest();
+        };
+
+        filter.doFilter(outer, response, outerChain);
+
+        assertSame(seen[0], inner, "the nested request must be bound during the nested chain");
+        assertSame(seen[1], outer, "the outer request must be bound again after the nested chain");
+        assertNull(JAXRPCRequestFilter.getCurrentRequest(),
+                "nothing must stay bound once the outer chain completes");
+    }
 }

openam-federation/openam-federation-library/src/main/java/com/sun/identity/federation/common/FSUtils.java

@@ -361,8 +361,10 @@ public static void forwardRequest(
                     index + deploymentURI.length());
                 // The target comes from the request (goto/RelayState). A forward
                 // bypasses the web.xml filters and can reach /WEB-INF, so refuse
-                // anything that is not a plain in-app path.
-                if (!ForwardPathValidator.isSafeForwardPath(resource)) {
+                // anything that is not a plain in-app path and dispatch the form
+                // the container maps.
+                String dispatchPath = ForwardPathValidator.forwardTarget(resource);
+                if (dispatchPath == null) {
                     FSUtils.debug.warning("FSUtils.forwardRequest: refusing to "
                         + "forward to a path with traversal or a reserved "
                         + "directory");
@@ -371,16 +373,16 @@ public static void forwardRequest(
                 }
                 if (FSUtils.debug.messageEnabled()) {
                     FSUtils.debug.message(
-                        "FSUtils.forwardRequest: Forwarding to :" + resource);
-                }  
-                RequestDispatcher dispatcher = 
-                    request.getRequestDispatcher(resource);
+                        "FSUtils.forwardRequest: Forwarding to :" + dispatchPath);
+                }
+                RequestDispatcher dispatcher =
+                    request.getRequestDispatcher(dispatchPath);
                 try {
                     dispatcher.forward(request, response);
                 } catch (Exception e) {
                     FSUtils.debug.error("FSUtils.forwardRequest: Exception "
                         + "occured while trying to forward to resource:" +
-                        resource , e);
+                        dispatchPath, e);
                 }
             } 
         } catch (Exception ex) {

openam-federation/openam-federation-library/src/main/java/com/sun/identity/federation/common/ForwardPathValidator.java

@@ -16,6 +16,7 @@
 package com.sun.identity.federation.common;
 
 import java.io.ByteArrayOutputStream;
+import java.net.URLDecoder;
 import java.nio.ByteBuffer;
 import java.nio.charset.CharacterCodingException;
 import java.nio.charset.CodingErrorAction;
@@ -78,6 +79,46 @@ public static boolean isSafeForwardPath(String path) {
         return true;
     }
 
+    /**
+     * The path to hand to a {@code RequestDispatcher} for {@code path}, or {@code null} when
+     * {@code path} must not be dispatched to. The result is the form the container maps -
+     * path parameters stripped and percent-escapes decoded once, {@code +} kept literal - with
+     * the query string as it was given.
+     * <p>
+     * Beyond {@link #isSafeForwardPath} the dispatcher form is refused when it carries a
+     * {@code WEB-INF} segment anywhere, not only at the root: no in-app path of the product
+     * does. The decoded form is derived and re-checked here in the shape static analysis
+     * recognises as a sanitised forward target, so the checks read as a repetition of what
+     * {@code isSafeForwardPath} established.
+     *
+     * @param path a context-relative path, optionally with a query string
+     * @return the path to dispatch to, or {@code null} if it is not a plain in-app path
+     */
+    public static String forwardTarget(String path) {
+        if (!isSafeForwardPath(path)) {
+            return null;
+        }
+        int query = path.indexOf('?');
+        String uri = query == -1 ? path : path.substring(0, query);
+        // isSafeForwardPath refused every escape that decodes to a delimiter, %25 included,
+        // so one round decodes everything; '+' is protected from URLDecoder's form decoding.
+        String decoded = stripPathParams(uri).replace("+", "%2B");
+        try {
+            while (decoded.contains("%")) {
+                decoded = URLDecoder.decode(decoded, StandardCharsets.UTF_8);
+            }
+        } catch (IllegalArgumentException e) {
+            return null;
+        }
+        // Only a segment that is exactly ".." is traversal; every other segment becomes "x".
+        String dotSegments = decoded.replaceAll("(?<=^|/)(?!\\.\\.(?=/|$))[^/]+", "x");
+        if ((decoded + "/").toUpperCase(Locale.ROOT).contains("/WEB-INF/")
+                || dotSegments.contains("..")) {
+            return null;
+        }
+        return query == -1 ? decoded : decoded + "?" + path.substring(query + 1);
+    }
+
     private static boolean isReserved(String collapsedPath) {
         String lower = collapsedPath.toLowerCase(Locale.ROOT);
         return lower.startsWith("/web-inf/") || lower.equals("/web-inf")

openam-federation/openam-federation-library/src/test/java/com/sun/identity/federation/common/FSUtilsForwardPathTest.java

@@ -70,6 +70,20 @@ public void forwardsAnInAppTargetOnTheSameHost() throws Exception {
         verify(response, never()).sendError(anyInt());
     }
 
+    @Test
+    public void dispatchesTheFormTheContainerMaps() throws Exception {
+        // Path parameters stripped and escapes decoded once, the query string as given.
+        RequestDispatcher dispatcher = mock(RequestDispatcher.class);
+        when(request.getRequestDispatcher("/console/my page.jsp?goto=%2Fopenam%2Fconsole"))
+            .thenReturn(dispatcher);
+
+        FSUtils.forwardRequest(request, response,
+            BASE + "/console;x%2Fy/my%20page.jsp?goto=%2Fopenam%2Fconsole");
+
+        verify(dispatcher).forward(request, response);
+        verify(response, never()).sendError(anyInt());
+    }
+
     @DataProvider
     public Object[][] traversingTargets() {
         return new Object[][] {

openam-federation/openam-federation-library/src/test/java/com/sun/identity/federation/common/ForwardPathValidatorTest.java

@@ -121,6 +121,56 @@ public void rejectsTraversalAndReservedDirectories(String path) {
         assertThat(ForwardPathValidator.isSafeForwardPath(path)).as(path).isFalse();
     }
 
+    @DataProvider
+    public Object[][] forwardTargets() {
+        return new Object[][] {
+            // {path as received, the form handed to the RequestDispatcher}
+            {"/idpSSOInit.jsp", "/idpSSOInit.jsp"},
+            {"/console/my%20page.jsp", "/console/my page.jsp"},   // decoded once, as the container maps it
+            {"/console/a+b.jsp", "/console/a+b.jsp"},             // '+' stays literal
+            {"/console/a+b%20c.jsp", "/console/a+b c.jsp"},
+            {"/console/base;jsessionid=1/AMAdminFrame", "/console/base/AMAdminFrame"}, // parameter stripped
+            {"/console;x%2Fy/base/AMAdminFrame", "/console/base/AMAdminFrame"},      // encoded slash goes with it
+            {"/console//base/./AMAdminFrame", "/console//base/./AMAdminFrame"},      // the container collapses
+            {"/console/base/", "/console/base/"},                 // a trailing slash is a different mapping
+            {"/a/b..c/d", "/a/b..c/d"},                           // ".." inside a segment is an ordinary name
+            {"/x/.../y", "/x/.../y"},
+            {"/web-info/page.jsp", "/web-info/page.jsp"},
+            {"/UI/Login?goto=%2Fopenam%2Fconsole", "/UI/Login?goto=%2Fopenam%2Fconsole"}, // query kept as given
+            {"/saml2/jsp/idpSSOInit.jsp?metaAlias=%2Fidp&goto=%2F..%2FWEB-INF",
+                "/saml2/jsp/idpSSOInit.jsp?metaAlias=%2Fidp&goto=%2F..%2FWEB-INF"},
+            {"/console/my%20page.jsp?", "/console/my page.jsp?"},
+            {"/XUI/#login/", "/XUI/#login/"},
+        };
+    }
+
+    @Test(dataProvider = "forwardTargets")
+    public void forwardTargetIsTheFormTheContainerMaps(String path, String expected) {
+        assertThat(ForwardPathValidator.forwardTarget(path)).as(path).isEqualTo(expected);
+    }
+
+    @Test(dataProvider = "unsafeForwardPaths")
+    public void forwardTargetIsNullForAnUnsafePath(String path) {
+        assertThat(ForwardPathValidator.forwardTarget(path)).as(path).isNull();
+    }
+
+    @DataProvider
+    public Object[][] reservedSegmentsBelowTheRoot() {
+        return new Object[][] {
+            {"/x/WEB-INF/y.jsp"},
+            {"/x/web-inf/"},
+            {"/x/WEB-INF"},
+            {"/x;a/%57EB-INF/y.jsp"},
+        };
+    }
+
+    @Test(dataProvider = "reservedSegmentsBelowTheRoot")
+    public void forwardTargetRefusesAReservedSegmentAnywhere(String path) {
+        // isSafeForwardPath reserves the directory at the root only; no in-app path carries
+        // the name below it either, and the dispatcher form is refused when one does.
+        assertThat(ForwardPathValidator.forwardTarget(path)).as(path).isNull();
+    }
+
     @DataProvider
     public Object[][] safeMetaAliases() {
         return new Object[][] {

openam-server-only/pom.xml

@@ -973,5 +973,12 @@
             <groupId>org.openidentityplatform.openam</groupId>
             <artifactId>openam-cassandra-cts</artifactId>
         </dependency>
+
+        <!-- Test dependencies -->
+        <dependency>
+            <groupId>org.testng</groupId>
+            <artifactId>testng</artifactId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 </project>

openam-server-only/src/main/webapp/WEB-INF/web.xml

@@ -1,4 +1,20 @@
 <?xml version="1.0" encoding="UTF-8"?>
+<!--
+ * The contents of this file are subject to the terms of the Common Development and
+ * Distribution License (the License). You may not use this file except in compliance with the
+ * License.
+ *
+ * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the
+ * specific language governing permission and limitations under the License.
+ *
+ * When distributing Covered Software, include this CDDL Header Notice in each file and include
+ * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL
+ * Header, with the fields enclosed by brackets [] replaced by your own identifying
+ * information: "Portions copyright [year] [name of copyright owner]".
+ *
+ * Copyright 2012-2016 ForgeRock AS.
+ * Portions Copyright 2017-2026 3A Systems, LLC.
+-->
 <web-app version="3.0" xmlns="http://java.sun.com/xml/ns/javaee" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
          xsi:schemaLocation="http://java.sun.com/xml/ns/javaee http://java.sun.com/xml/ns/javaee/web-app_3_0.xsd">
     <display-name>OpenAM Services</display-name>
@@ -226,21 +242,45 @@
         <url-pattern>/oauth2/*</url-pattern>
     </filter-mapping>
 
+    <!-- A filter-mapping that names no <dispatcher> applies to REQUEST only, so a
+         RequestDispatcher forward or include, an async dispatch or an error-page dispatch
+         reaches the endpoint with the filter skipped. Naming any type replaces that default,
+         so the security filters below list every type. -->
     <filter-mapping>
         <filter-name>AuthNFilter</filter-name>
         <url-pattern>/ws/*</url-pattern>
+        <dispatcher>REQUEST</dispatcher>
+        <dispatcher>FORWARD</dispatcher>
+        <dispatcher>INCLUDE</dispatcher>
+        <dispatcher>ASYNC</dispatcher>
+        <dispatcher>ERROR</dispatcher>
     </filter-mapping>
     <filter-mapping>
         <filter-name>JAXRPCRequestFilter</filter-name>
         <url-pattern>/jaxrpc/*</url-pattern>
+        <dispatcher>REQUEST</dispatcher>
+        <dispatcher>FORWARD</dispatcher>
+        <dispatcher>INCLUDE</dispatcher>
+        <dispatcher>ASYNC</dispatcher>
+        <dispatcher>ERROR</dispatcher>
     </filter-mapping>
     <filter-mapping>
         <filter-name>AuthZFilter</filter-name>
         <url-pattern>/ws/*</url-pattern>
+        <dispatcher>REQUEST</dispatcher>
+        <dispatcher>FORWARD</dispatcher>
+        <dispatcher>INCLUDE</dispatcher>
+        <dispatcher>ASYNC</dispatcher>
+        <dispatcher>ERROR</dispatcher>
     </filter-mapping>
     <filter-mapping>
         <filter-name>NotificationsWebSocketFilter</filter-name>
         <url-pattern>/notifications</url-pattern>
+        <dispatcher>REQUEST</dispatcher>
+        <dispatcher>FORWARD</dispatcher>
+        <dispatcher>INCLUDE</dispatcher>
+        <dispatcher>ASYNC</dispatcher>
+        <dispatcher>ERROR</dispatcher>
     </filter-mapping>
 
     <!-- filter mapping -->