Skip to content

Commit

Permalink
Merge pull request apache#426 from salcho/post-ww-5083
Browse files Browse the repository at this point in the history
WW-5083: Adds support for Fetch Metadata in Struts2.
  • Loading branch information
lukaszlenart authored Jul 17, 2020
2 parents c2b09d7 + 8902a9a commit a55e9ed
Show file tree
Hide file tree
Showing 5 changed files with 336 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
/*
* 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.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 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 java.util.HashSet;
import java.util.Set;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

/**
* Interceptor that implements Fetch Metadata policy on incoming requests used to protect against
* CSRF, XSSI, and cross-origin information leaks. Uses {@link StrutsResourceIsolationPolicy} to
* filter the requests allowed to be processed.
*
* @see <a href="https://web.dev/fetch-metadata/">https://web.dev/fetch-metadata/</a>
**/

public class FetchMetadataInterceptor extends AbstractInterceptor {
private static final Logger logger = LogManager.getLogger(FetchMetadataInterceptor.class);
private static final String VARY_HEADER_VALUE = String.format("%s,%s,%s", SEC_FETCH_DEST_HEADER, SEC_FETCH_SITE_HEADER, SEC_FETCH_MODE_HEADER);
private static final String SC_FORBIDDEN = String.valueOf(HttpServletResponse.SC_FORBIDDEN);

private final Set<String> exemptedPaths = new HashSet<>();
private final ResourceIsolationPolicy resourceIsolationPolicy = new StrutsResourceIsolationPolicy();

public void setExemptedPaths(String paths){
this.exemptedPaths.addAll(TextParseUtil.commaDelimitedStringToSet(paths));
}

@Override
public String intercept(ActionInvocation invocation) throws Exception {
ActionContext context = invocation.getInvocationContext();
HttpServletRequest request = context.getServletRequest();

addVaryHeaders(invocation);

String contextPath = request.getContextPath();
// Apply exemptions: paths/endpoints meant to be served cross-origin
if (exemptedPaths.contains(contextPath)) {
return invocation.invoke();
}

// Check if request is allowed
if (resourceIsolationPolicy.isRequestAllowed(request)) {
return invocation.invoke();
}

logger.atDebug().log(
"Fetch metadata rejected cross-origin request to %s",
contextPath
);
return SC_FORBIDDEN;
}

private void addVaryHeaders(ActionInvocation invocation) {
HttpServletResponse response = invocation.getInvocationContext().getServletResponse();
response.setHeader(VARY_HEADER, VARY_HEADER_VALUE);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/*
* 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.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 StrutsResourceIsolationPolicy} for the default implementation used.
*
* @see <a href="https://web.dev/fetch-metadata/">https://web.dev/fetch-metadata/</a>
**/

@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);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
* 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.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>
**/

public final class StrutsResourceIsolationPolicy 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;
}
}
2 changes: 2 additions & 0 deletions core/src/main/resources/struts-default.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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">
Expand Down Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
/*
* 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.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);
}
}

0 comments on commit a55e9ed

Please sign in to comment.