Ww 5083 (#1)
WW-5083: Adds support for Fetch Metadata in Struts2. This commit adds a Fetch Metadata Interceptor to the default stack.
diff --git a/core/src/main/java/org/apache/struts2/interceptor/DefaultResourceIsolationPolicy.java b/core/src/main/java/org/apache/struts2/interceptor/DefaultResourceIsolationPolicy.java
new file mode 100644
index 0000000..98c03d8
--- /dev/null
+++ b/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;
+ }
+}
diff --git a/core/src/main/java/org/apache/struts2/interceptor/FetchMetadataInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/FetchMetadataInterceptor.java
new file mode 100644
index 0000000..97a7dcf
--- /dev/null
+++ b/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 -->
+ *
+ * <action ... >
+ * <interceptor-ref name="defaultStack"/>
+ * <interceptor-ref name="fetchMetadata">
+ * <param name="exemptedPaths"> path1,path2,path3 <para/>
+ * <interceptor-ref/>
+ * ...
+ * </action>
+ *
+ * <!-- 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;
+ }
+}
diff --git a/core/src/main/java/org/apache/struts2/interceptor/ResourceIsolationPolicy.java b/core/src/main/java/org/apache/struts2/interceptor/ResourceIsolationPolicy.java
new file mode 100644
index 0000000..74e5457
--- /dev/null
+++ b/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);
+}
diff --git a/core/src/main/resources/struts-default.xml b/core/src/main/resources/struts-default.xml
index 0988d87..8f00459 100644
--- a/core/src/main/resources/struts-default.xml
+++ b/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>
diff --git a/core/src/test/java/org/apache/struts2/interceptor/FetchMetadataInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/FetchMetadataInterceptorTest.java
new file mode 100644
index 0000000..07410e1
--- /dev/null
+++ b/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);
+ }
+}