Ww 5083 (#1)

WW-5083: Adds support for Fetch Metadata in Struts2. This commit adds a Fetch Metadata Interceptor to the default stack.

core/src/main/java/org/apache/struts2/interceptor/DefaultResourceIsolationPolicy.java

@@ -0,0 +1,49 @@
+package org.apache.struts2.interceptor;
+
+import org.apache.logging.log4j.util.Strings;
+
+import javax.servlet.http.HttpServletRequest;
+
+/**
+ *
+ * Default resource isolation policy used in {@link FetchMetadataInterceptor} that
+ * implements the {@link ResourceIsolationPolicy} interface. This default policy is based on
+ * <a href="https://web.dev/fetch-metadata/">https://web.dev/fetch-metadata/</a>.
+ *
+ * @see <a href="https://web.dev/fetch-metadata/">https://web.dev/fetch-metadata/</a>
+ *
+ * @author Santiago Diaz - saldiaz@google.com
+ * @author Giannis Chatziveroglou - giannic@google.com
+ **/
+
+public final class DefaultResourceIsolationPolicy implements ResourceIsolationPolicy {
+
+    @Override
+    public boolean isRequestAllowed(HttpServletRequest request) {
+        String site = request.getHeader(SEC_FETCH_SITE_HEADER);
+
+        // Allow requests from browsers which don't send Fetch Metadata
+        if (Strings.isEmpty((site))){
+            return true;
+        }
+
+        // Allow same-site and browser-initiated requests
+        if (SAME_ORIGIN.equals(site) || SAME_SITE.equals(site) || NONE.equals(site)) {
+            return true;
+        }
+
+        // Allow simple top-level navigations except <object> and <embed>
+        return isAllowedTopLevelNavigation(request);
+    }
+
+    private boolean isAllowedTopLevelNavigation(HttpServletRequest request)
+    {
+        String mode = request.getHeader(SEC_FETCH_MODE_HEADER);
+        String dest = request.getHeader(SEC_FETCH_DEST_HEADER);
+
+        boolean isSimpleTopLevelNavigation = MODE_NAVIGATE.equals(mode) || "GET".equals(request.getMethod());
+        boolean isNotObjectOrEmbedRequest = !DEST_EMBED.equals(dest) && !DEST_OBJECT.equals(dest);
+
+        return isSimpleTopLevelNavigation && isNotObjectOrEmbedRequest;
+    }
+}

core/src/main/java/org/apache/struts2/interceptor/FetchMetadataInterceptor.java

@@ -0,0 +1,96 @@
+package org.apache.struts2.interceptor;
+
+import com.opensymphony.xwork2.ActionContext;
+import com.opensymphony.xwork2.ActionInvocation;
+import com.opensymphony.xwork2.interceptor.AbstractInterceptor;
+import com.opensymphony.xwork2.interceptor.PreResultListener;
+import com.opensymphony.xwork2.util.TextParseUtil;
+
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.util.HashSet;
+import java.util.Set;
+
+/**
+ * <!-- START SNIPPET: description -->
+ *
+ *
+ * Interceptor that implements Fetch Metadata policy on incoming requests used to protect against
+ * CSRF, XSSI, and cross-origin information leaks. Uses {@link DefaultResourceIsolationPolicy} to
+ * filter the requests allowed to be processed.
+ *
+ * @see <a href="https://web.dev/fetch-metadata/">https://web.dev/fetch-metadata/</a>
+ *
+ * <!-- END SNIPPET: description -->
+ *
+ * <!-- START SNIPPET: parameters -->
+
+ * <ul>
+ *     <li>exemptedPaths - Set of opt out endpoints that are meant to serve
+ *              cross-site traffic</li>
+ *     <li>resourceIsolationPolicy -Instance of {@link DefaultResourceIsolationPolicy} implementing
+ *              the logic for the requests filtering</li>
+ *      <li>VARY_HEADER_VALUE - static vary header value for each vary response headers</li>
+ * </ul>
+ *
+ * <!-- END SNIPPET: parameters -->
+ *
+ * <!-- START SNIPPET: extending -->
+ *
+ *  No extensions
+ *
+ * <!-- END SNIPPET: extending -->
+ *
+ * <!-- START SNIPPET: example -->
+ *
+ * &lt;action ... &gt;
+ *   &lt;interceptor-ref name="defaultStack"/&gt;
+ *   &lt;interceptor-ref name="fetchMetadata"&gt;
+ *      &lt;param name="exemptedPaths"&gt; path1,path2,path3 &lt;para/&gt;
+ *   &lt;interceptor-ref/&gt;
+ *   ...
+ * &lt;/action&gt;
+ *
+ * <!-- END SNIPPET: example -->
+ **/
+
+public class FetchMetadataInterceptor extends AbstractInterceptor implements PreResultListener {
+
+    private final Set<String> exemptedPaths = new HashSet<String>();
+    private final ResourceIsolationPolicy resourceIsolationPolicy = new DefaultResourceIsolationPolicy();
+    private static final String VARY_HEADER_VALUE = String.format("%s,%s,%s", DefaultResourceIsolationPolicy.SEC_FETCH_DEST_HEADER, DefaultResourceIsolationPolicy.SEC_FETCH_SITE_HEADER, DefaultResourceIsolationPolicy.SEC_FETCH_MODE_HEADER);
+    private static final String SC_FORBIDDEN = String.valueOf(HttpServletResponse.SC_FORBIDDEN);
+
+    public void setExemptedPaths(String paths){
+        this.exemptedPaths.addAll(TextParseUtil.commaDelimitedStringToSet(paths));
+    }
+
+    @Override
+    public void beforeResult(ActionInvocation invocation, String resultCode) {
+        // Add Vary headers
+        HttpServletResponse response = invocation.getInvocationContext().getServletResponse();
+        response.setHeader(DefaultResourceIsolationPolicy.VARY_HEADER, VARY_HEADER_VALUE);
+    }
+
+    @Override
+    public String intercept(ActionInvocation invocation) throws Exception {
+        ActionContext context = invocation.getInvocationContext();
+        HttpServletRequest request = context.getServletRequest();
+
+        // Adds listener that operates between interceptor and result rendering to set Vary headers
+        invocation.addPreResultListener(this);
+
+        // Apply exemptions: paths/endpoints meant to be served cross-origin
+        if (exemptedPaths.contains(request.getContextPath())) {
+            return invocation.invoke();
+        }
+
+        // Check if request is allowed
+        if (resourceIsolationPolicy.isRequestAllowed(request)) {
+            return invocation.invoke();
+        }
+
+        beforeResult(invocation, SC_FORBIDDEN);
+        return SC_FORBIDDEN;
+    }
+}

core/src/main/java/org/apache/struts2/interceptor/ResourceIsolationPolicy.java

@@ -0,0 +1,38 @@
+package org.apache.struts2.interceptor;
+
+import javax.servlet.http.HttpServletRequest;
+
+/**
+ * Interface for the resource isolation policies to be used for fetch metadata checks.
+ *
+ * Resource isolation policies are designed to protect against cross origin attacks and use the
+ * {@code sec-fetch-*} request headers to decide whether to accept or reject a request. Read more
+ * about <a href="https://web.dev/fetch-metadata/">Fetch Metadata.</a>
+ *
+ * See {@link DefaultResourceIsolationPolicy} for the default implementation used.
+ *
+ * @see <a href="https://web.dev/fetch-metadata/">https://web.dev/fetch-metadata/</a>
+ *
+ * @author Santiago Diaz - saldiaz@google.com
+ * @author Giannis Chatziveroglou - giannic@google.com
+ **/
+
+@FunctionalInterface
+public interface ResourceIsolationPolicy {
+    String SEC_FETCH_SITE_HEADER = "sec-fetch-site";
+    String SEC_FETCH_MODE_HEADER = "sec-fetch-mode";
+    String SEC_FETCH_DEST_HEADER = "sec-fetch-dest";
+    String VARY_HEADER = "Vary";
+    String SAME_ORIGIN = "same-origin";
+    String SAME_SITE = "same-site";
+    String NONE = "none";
+    String MODE_NAVIGATE = "navigate";
+    String DEST_OBJECT = "object";
+    String DEST_EMBED = "embed";
+    String CROSS_SITE = "cross-site";
+    String CORS = "cors";
+    String DEST_SCRIPT = "script";
+    String DEST_IMAGE = "image";
+
+    boolean isRequestAllowed(HttpServletRequest request);
+}

core/src/main/resources/struts-default.xml

@@ -273,6 +273,7 @@
             <interceptor name="annotationParameterFilter" class="com.opensymphony.xwork2.interceptor.annotations.AnnotationParameterFilterInterceptor" />
             <interceptor name="multiselect" class="org.apache.struts2.interceptor.MultiselectInterceptor" />
             <interceptor name="noop" class="org.apache.struts2.interceptor.NoOpInterceptor" />
+            <interceptor name="fetchMetadata" class="org.apache.struts2.interceptor.FetchMetadataInterceptor" />
 
             <!-- Empty stack - performs no operations -->
             <interceptor-stack name="emptyStack">
@@ -388,6 +389,7 @@
                 <interceptor-ref name="actionMappingParams"/>
                 <interceptor-ref name="params"/>
                 <interceptor-ref name="conversionError"/>
+                <interceptor-ref name="fetchMetadata"/>
                 <interceptor-ref name="validation">
                     <param name="excludeMethods">input,back,cancel,browse</param>
                 </interceptor-ref>

core/src/test/java/org/apache/struts2/interceptor/FetchMetadataInterceptorTest.java

@@ -0,0 +1,113 @@
+package org.apache.struts2.interceptor;
+
+
+import static org.apache.struts2.interceptor.ResourceIsolationPolicy.SEC_FETCH_DEST_HEADER;
+import static org.apache.struts2.interceptor.ResourceIsolationPolicy.SEC_FETCH_MODE_HEADER;
+import static org.apache.struts2.interceptor.ResourceIsolationPolicy.SEC_FETCH_SITE_HEADER;
+import static org.apache.struts2.interceptor.ResourceIsolationPolicy.VARY_HEADER;
+import static org.junit.Assert.assertNotEquals;
+
+import com.opensymphony.xwork2.ActionContext;
+import com.opensymphony.xwork2.XWorkTestCase;
+import com.opensymphony.xwork2.mock.MockActionInvocation;
+import org.apache.struts2.ServletActionContext;
+import org.springframework.mock.web.MockHttpServletRequest;
+import org.springframework.mock.web.MockHttpServletResponse;
+
+import java.util.Arrays;
+
+public class FetchMetadataInterceptorTest extends XWorkTestCase {
+
+    private final FetchMetadataInterceptor interceptor = new FetchMetadataInterceptor();
+    private final MockActionInvocation mai = new MockActionInvocation();
+    private final MockHttpServletRequest request = new MockHttpServletRequest();
+    private final MockHttpServletResponse response = new MockHttpServletResponse();
+    private static final String VARY_HEADER_VALUE = String.format(
+        "%s,%s,%s",
+        SEC_FETCH_DEST_HEADER,
+        SEC_FETCH_SITE_HEADER,
+        SEC_FETCH_MODE_HEADER
+    );
+
+    @Override
+    protected void setUp() throws Exception {
+        super.setUp();
+        container.inject(interceptor);
+        interceptor.setExemptedPaths("/foo,/bar");
+        ServletActionContext.setRequest(request);
+        ServletActionContext.setResponse(response);
+        ActionContext context = ServletActionContext.getActionContext();
+        mai.setInvocationContext(context);
+    }
+
+    public void testNoSite() throws Exception {
+        request.removeHeader("sec-fetch-site");
+
+        assertNotEquals("Expected interceptor to accept this request", "403",
+            interceptor.intercept(mai));
+    }
+
+    public void testValidSite() throws Exception {
+        for (String header : Arrays.asList("same-origin", "same-site", "none")){
+            request.addHeader("sec-fetch-site", header);
+
+            assertNotEquals("Expected interceptor to accept this request", "403",
+                interceptor.intercept(mai));
+        }
+
+    }
+
+    public void testValidTopLevelNavigation() throws Exception {
+        request.addHeader("sec-fetch-mode", "navigate");
+        request.addHeader("sec-fetch-dest", "script");
+        request.setMethod("GET");
+
+        assertNotEquals("Expected interceptor to accept this request", "403",
+            interceptor.intercept(mai));
+    }
+
+    public void testInvalidTopLevelNavigation() throws Exception {
+        for (String header : Arrays.asList("object", "embed")) {
+            request.addHeader("sec-fetch-site", "foo");
+            request.addHeader("sec-fetch-mode", "navigate");
+            request.addHeader("sec-fetch-dest", header);
+            request.setMethod("GET");
+
+            assertEquals("Expected interceptor to NOT accept this request", "403", interceptor.intercept(mai));
+        }
+    }
+
+    public void testPathInExemptedPaths() throws Exception {
+        request.addHeader("sec-fetch-site", "foo");
+        request.setContextPath("/foo");
+
+        assertNotEquals("Expected interceptor to accept this request", "403",
+            interceptor.intercept(mai));
+    }
+
+    public void testPathNotInExemptedPaths() throws Exception {
+        request.addHeader("sec-fetch-site", "foo");
+        request.setContextPath("/foobar");
+
+        assertEquals("Expected interceptor to NOT accept this request", "403", interceptor.intercept(mai));
+    }
+
+    public void testVaryHeaderAcceptedReq() throws Exception {
+        request.addHeader("sec-fetch-site", "foo");
+        request.setContextPath("/foo");
+
+        interceptor.intercept(mai);
+
+        assertTrue("Expected vary header to be included", response.containsHeader(VARY_HEADER));
+        assertEquals("Expected different vary header value", response.getHeader(VARY_HEADER), VARY_HEADER_VALUE);
+    }
+
+    public void testVaryHeaderRejectedReq() throws Exception {
+        request.addHeader("sec-fetch-site", "foo");
+
+        interceptor.intercept(mai);
+
+        assertTrue("Expected vary header to be included", response.containsHeader(VARY_HEADER));
+        assertEquals("Expected different vary header value", response.getHeader(VARY_HEADER), VARY_HEADER_VALUE);
+    }
+}