Apache/struts
1
0
Fork 0
mirror of https://github.com/apache/struts.git synced 2026-08-06 23:27:07 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #593 from apache/WW-5218-disable

Browse Source 
[WW-5218] Allows to disable CSP related interceptors
This commit is contained in:
Lukasz Lenart
2022-09-02 10:19:48 +02:00
committed by GitHub
parent 58a76162a6 16c2948f1a
commit 8c4a90f6e3
9 changed files with 156 additions and 77 deletions
Download Patch File Download Diff File Expand all files Collapse all files
core/src/main/java/org/apache/struts2/interceptor/CoepInterceptor.java
+14 -5
View File
@@ -52,21 +52,30 @@ public class CoepInterceptor extends AbstractInterceptor implements PreResultLis
@Override
public String intercept(ActionInvocation invocation) throws Exception {
invocation.addPreResultListener(this);
if (disabled) {
LOG.trace("COEP interceptor has been disabled");
} else {
invocation.addPreResultListener(this);
}
return invocation.invoke();
}
@Override
public void beforeResult(ActionInvocation invocation, String resultCode) {
if (disabled) {
return;
}
HttpServletRequest req = invocation.getInvocationContext().getServletRequest();
HttpServletResponse res = invocation.getInvocationContext().getServletResponse();
final String path = req.getContextPath();
if (exemptedPaths.contains(path)) {
// no need to add headers
LOG.debug("Skipping COEP header for exempted path {}", path);
} else if (!disabled) {
res.setHeader(header, REQUIRE_COEP_HEADER);
LOG.debug("Skipping COEP header for exempted path: {}", path);
} else {
LOG.trace("Applying COEP header: {} with value: {}", header, REQUIRE_COEP_HEADER);
HttpServletResponse response = invocation.getInvocationContext().getServletResponse();
response.setHeader(header, REQUIRE_COEP_HEADER);
}
}
core/src/main/java/org/apache/struts2/interceptor/CoopInterceptor.java
+15 -7
View File
@@ -49,25 +49,34 @@ public class CoopInterceptor extends AbstractInterceptor implements PreResultLis
private static final String COOP_HEADER = "Cross-Origin-Opener-Policy";
private final Set<String> exemptedPaths = new HashSet<>();
private boolean disabled = false;
private String mode = SAME_ORIGIN;
@Override
public String intercept(ActionInvocation invocation) throws Exception {
invocation.addPreResultListener(this);
if (disabled) {
LOG.trace("COOP interceptor has been disabled");
} else {
invocation.addPreResultListener(this);
}
return invocation.invoke();
}
@Override
public void beforeResult(ActionInvocation invocation, String resultCode) {
if (disabled) {
return;
}
HttpServletRequest request = invocation.getInvocationContext().getServletRequest();
HttpServletResponse response = invocation.getInvocationContext().getServletResponse();
String path = request.getContextPath();
if (isExempted(path)) {
// no need to add headers
LOG.debug("Skipping COOP header for exempted path {}", path);
} else {
response.setHeader(COOP_HEADER, getMode());
LOG.trace("Applying COOP header: {} with value: {}", COOP_HEADER, mode);
HttpServletResponse response = invocation.getInvocationContext().getServletResponse();
response.setHeader(COOP_HEADER, mode);
}
}
@@ -79,10 +88,6 @@ public class CoopInterceptor extends AbstractInterceptor implements PreResultLis
exemptedPaths.addAll(TextParseUtil.commaDelimitedStringToSet(paths));
}
private String getMode() {
return mode;
}
public void setMode(String mode) {
if (!(mode.equals(SAME_ORIGIN) || mode.equals(SAME_ORIGIN_ALLOW_POPUPS) || mode.equals(UNSAFE_NONE))) {
throw new IllegalArgumentException(String.format("Mode '%s' not recognized!", mode));
@@ -90,4 +95,7 @@ public class CoopInterceptor extends AbstractInterceptor implements PreResultLis
this.mode = mode;
}
public void setDisabled(String value) {
this.disabled = Boolean.parseBoolean(value);
}
}
core/src/main/java/org/apache/struts2/interceptor/FetchMetadataInterceptor.java
+39 -28
View File
@@ -18,24 +18,25 @@
*/
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.SEC_FETCH_USER_HEADER;
import static org.apache.struts2.interceptor.ResourceIsolationPolicy.VARY_HEADER;
import com.opensymphony.xwork2.ActionContext;
import com.opensymphony.xwork2.ActionInvocation;
import com.opensymphony.xwork2.inject.Inject;
import com.opensymphony.xwork2.interceptor.AbstractInterceptor;
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;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.util.HashSet;
import java.util.Set;
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.SEC_FETCH_USER_HEADER;
import static org.apache.struts2.interceptor.ResourceIsolationPolicy.VARY_HEADER;
/**
* Interceptor that implements Fetch Metadata policy on incoming requests used to protect against
* CSRF, XSSI, and cross-origin information leaks. Uses {@link StrutsResourceIsolationPolicy} to
@@ -46,6 +47,7 @@ import org.apache.logging.log4j.Logger;
**/
public class FetchMetadataInterceptor extends AbstractInterceptor {
private static final Logger LOG = LogManager.getLogger(FetchMetadataInterceptor.class);
private static final String VARY_HEADER_VALUE = String.format("%s,%s,%s,%s", SEC_FETCH_DEST_HEADER, SEC_FETCH_MODE_HEADER, SEC_FETCH_SITE_HEADER, SEC_FETCH_USER_HEADER);
private static final String SC_FORBIDDEN = String.valueOf(HttpServletResponse.SC_FORBIDDEN);
@@ -53,13 +55,19 @@ public class FetchMetadataInterceptor extends AbstractInterceptor {
private final Set<String> exemptedPaths = new HashSet<>();
private final ResourceIsolationPolicy resourceIsolationPolicy = new StrutsResourceIsolationPolicy();
@Inject (required=false)
private boolean disabled = false;
@Inject(required = false)
public void setExemptedPaths(String paths) {
this.exemptedPaths.addAll(TextParseUtil.commaDelimitedStringToSet(paths));
}
@Override
public String intercept(ActionInvocation invocation) throws Exception {
if (disabled) {
LOG.trace("Fetch Metadata interceptor has been disabled");
return invocation.invoke();
}
ActionContext context = invocation.getInvocationContext();
HttpServletRequest request = context.getServletRequest();
@@ -76,31 +84,34 @@ public class FetchMetadataInterceptor extends AbstractInterceptor {
return invocation.invoke();
}
LOG.info("Fetch metadata rejected cross-origin request to [{}]", contextPath);
LOG.warn("Fetch metadata rejected cross-origin request to: {}", contextPath);
return SC_FORBIDDEN;
}
/**
* Sets {@link SEC_FETCH_DEST_HEADER}, {@link SEC_FETCH_MODE_HEADER}, {@link SEC_FETCH_SITE_HEADER}, and {@link SEC_FETCH_USER_HEADER}
* elements in the provided ActionInvocation's HttpServletResponse {@link VARY_HEADER} response header.
*
* Sets {@link ResourceIsolationPolicy#SEC_FETCH_DEST_HEADER}, {@link ResourceIsolationPolicy#SEC_FETCH_MODE_HEADER},
* {@link ResourceIsolationPolicy#SEC_FETCH_SITE_HEADER}, and {@link ResourceIsolationPolicy#SEC_FETCH_USER_HEADER}
* elements in the provided ActionInvocation's HttpServletResponse {@link ResourceIsolationPolicy#VARY_HEADER} response header.
* <p>
* Note: This method will replace any previous Vary header content already set for the response.
* Note: In order to be effective, the Vary header modification must take place at (or very near) the start of this interceptor's processing.
*
* @param invocation Supplies the HttpServletResponse (if present) to which the SEC_FETCH_* header names are be added to its {@link VARY_HEADER} response header.
* Note: In order to be effective, the Vary header modification must take place at (or very near) the start of this
* interceptor's processing.
*
* @param invocation Supplies the HttpServletResponse (if present) to which the SEC_FETCH_* header names are be added
* to its {@link ResourceIsolationPolicy#VARY_HEADER} response header.
*/
private void addVaryHeaders(ActionInvocation invocation) {
HttpServletResponse response = invocation.getInvocationContext().getServletResponse();
if (response != null) {
// TODO: Whenever servlet 3.x becomes the baseline for Struts, consider revising this method to use
// getHeader(VARY_HEADER) and preserve any VARY_HEADER content already set in the response.
// This will probably require some tokenization logic for the header contents.
if (LOG.isDebugEnabled() && response.containsHeader(VARY_HEADER)) {
LOG.debug("HTTP response already has a [{}] header set, the old value will be overwritten (replaced)", VARY_HEADER);
}
response.setHeader(VARY_HEADER, VARY_HEADER_VALUE);
} else {
LOG.debug("HTTP response is null, cannot add a new [{}] header", VARY_HEADER);
// TODO: Whenever servlet 3.x becomes the baseline for Struts, consider revising this method to use
// getHeader(VARY_HEADER) and preserve any VARY_HEADER content already set in the response.
// This will probably require some tokenization logic for the header contents.
if (LOG.isDebugEnabled() && response.containsHeader(VARY_HEADER)) {
LOG.debug("HTTP response already has header: {} set, the old value will be overwritten (replaced)", VARY_HEADER);
}
response.setHeader(VARY_HEADER, VARY_HEADER_VALUE);
}
public void setDisabled(String value) {
this.disabled = Boolean.parseBoolean(value);
}
}
core/src/main/java/org/apache/struts2/interceptor/csp/CspInterceptor.java
+22 -4
View File
@@ -21,10 +21,13 @@ package org.apache.struts2.interceptor.csp;
import com.opensymphony.xwork2.ActionInvocation;
import com.opensymphony.xwork2.interceptor.AbstractInterceptor;
import com.opensymphony.xwork2.interceptor.PreResultListener;
import java.net.URI;
import java.util.Optional;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.net.URI;
import java.util.Optional;
/**
* Interceptor that implements Content Security Policy on incoming requests used to protect against
@@ -38,15 +41,26 @@ import javax.servlet.http.HttpServletResponse;
**/
public final class CspInterceptor extends AbstractInterceptor implements PreResultListener {
private static final Logger LOG = LogManager.getLogger(CspInterceptor.class);
private final CspSettings settings = new DefaultCspSettings();
private boolean disabled = false;
@Override
public String intercept(ActionInvocation invocation) throws Exception {
invocation.addPreResultListener(this);
if (disabled) {
LOG.trace("CSP interceptor has been disabled");
} else {
invocation.addPreResultListener(this);
}
return invocation.invoke();
}
public void beforeResult(ActionInvocation invocation, String resultCode) {
if (disabled) {
return;
}
HttpServletRequest request = invocation.getInvocationContext().getServletRequest();
HttpServletResponse response = invocation.getInvocationContext().getServletResponse();
settings.addCspHeaders(request, response);
@@ -74,8 +88,12 @@ public final class CspInterceptor extends AbstractInterceptor implements PreResu
return Optional.empty();
}
public void setEnforcingMode(String value){
public void setEnforcingMode(String value) {
boolean enforcingMode = Boolean.parseBoolean(value);
settings.setEnforcingMode(enforcingMode);
}
public void setDisabled(String value) {
this.disabled = Boolean.parseBoolean(value);
}
}
core/src/main/java/org/apache/struts2/interceptor/csp/DefaultCspSettings.java
+2 -2
View File
@@ -53,11 +53,11 @@ public class DefaultCspSettings implements CspSettings {
public void addCspHeaders(HttpServletRequest request, HttpServletResponse response) {
if (isSessionActive(request)) {
LOG.debug("Session is active, applying CSP settings");
LOG.trace("Session is active, applying CSP settings");
associateNonceWithSession(request);
response.setHeader(cspHeader, cratePolicyFormat(request));
} else {
LOG.debug("Session is not active, ignoring CSP settings");
LOG.trace("Session is not active, ignoring CSP settings");
}
}
core/src/main/resources/struts-default.xml
+6 -2
View File
@@ -392,6 +392,7 @@
<interceptor-ref name="servletConfig"/>
<interceptor-ref name="i18n"/>
<interceptor-ref name="cspInterceptor">
<param name="disabled">false</param>
<param name="enforcingMode">false</param>
</interceptor-ref>
<interceptor-ref name="prepare"/>
@@ -407,15 +408,18 @@
<interceptor-ref name="params"/>
<interceptor-ref name="conversionError"/>
<interceptor-ref name="coepInterceptor">
<param name="enforcingMode">false</param>
<param name="disabled">false</param>
<param name="enforcingMode">false</param>
<param name="exemptedPaths"/>
</interceptor-ref>
<interceptor-ref name="coopInterceptor">
<param name="disabled">false</param>
<param name="exemptedPaths"/>
<param name="mode">same-origin</param>
</interceptor-ref>
<interceptor-ref name="fetchMetadata"/>
<interceptor-ref name="fetchMetadata">
<param name="disabled">false</param>
</interceptor-ref>
<interceptor-ref name="validation">
<param name="excludeMethods">input,back,cancel,browse</param>
</interceptor-ref>
core/src/test/java/org/apache/struts2/interceptor/CoopInterceptorTest.java
+10 -2
View File
@@ -34,7 +34,6 @@ public class CoopInterceptorTest extends StrutsInternalTestCase {
private final MockHttpServletResponse response = new MockHttpServletResponse();
String SAME_ORIGIN = "same-origin";
String SAME_SITE = "same-site";
String UNSAFE_NONE = "unsafe-none";
String COOP_HEADER = "Cross-Origin-Opener-Policy";
@@ -65,7 +64,7 @@ public class CoopInterceptorTest extends StrutsInternalTestCase {
assertEquals("Coop header is not same-origin", UNSAFE_NONE, header);
}
public void testErrorNotRecognizedMode() throws Exception {
public void testErrorNotRecognizedMode() {
request.setContextPath("/some");
try{
@@ -76,6 +75,15 @@ public class CoopInterceptorTest extends StrutsInternalTestCase {
}
}
public void testDisabled() throws Exception {
interceptor.setDisabled("true");
interceptor.intercept(mai);
String header = response.getHeader(COOP_HEADER);
assertTrue("COOP is not disabled", Strings.isEmpty(header));
}
@Override
protected void setUp() throws Exception {
super.setUp();
core/src/test/java/org/apache/struts2/interceptor/CspInterceptorTest.java
+15 -4
View File
@@ -121,7 +121,7 @@ public class CspInterceptorTest extends StrutsInternalTestCase {
checkHeader(reportUri, enforcingMode);
}
public void testCannotParseUri() throws Exception {
public void testCannotParseUri() {
String enforcingMode = "false";
interceptor.setEnforcingMode(enforcingMode);
@@ -133,7 +133,7 @@ public class CspInterceptorTest extends StrutsInternalTestCase {
}
}
public void testCannotParseRelativeUri() throws Exception {
public void testCannotParseRelativeUri() {
String enforcingMode = "false";
interceptor.setEnforcingMode(enforcingMode);
@@ -145,8 +145,19 @@ public class CspInterceptorTest extends StrutsInternalTestCase {
}
}
public void testDisabled() throws Exception {
interceptor.setDisabled("true");
interceptor.intercept(mai);
String header = response.getHeader(CSP_ENFORCE_HEADER);
assertTrue("CSP is not disabled", Strings.isEmpty(header));
header = response.getHeader(CSP_REPORT_HEADER);
assertTrue("CSP is not disabled", Strings.isEmpty(header));
}
public void checkHeader(String reportUri, String enforcingMode) {
String expectedCspHeader = "";
String expectedCspHeader;
if (Strings.isEmpty(reportUri)) {
expectedCspHeader = String.format("%s '%s'; %s 'nonce-%s' '%s' %s %s; %s '%s'; ",
OBJECT_SRC, NONE,
@@ -162,7 +173,7 @@ public class CspInterceptorTest extends StrutsInternalTestCase {
);
}
String header = "";
String header;
if (enforcingMode.equals("true")) {
header = response.getHeader(CSP_ENFORCE_HEADER);
} else {
core/src/test/java/org/apache/struts2/interceptor/FetchMetadataInterceptorTest.java
+33 -23
View File
@@ -18,6 +18,27 @@
*/
package org.apache.struts2.interceptor;
import com.opensymphony.xwork2.ActionContext;
import com.opensymphony.xwork2.XWorkTestCase;
import com.opensymphony.xwork2.config.RuntimeConfiguration;
import com.opensymphony.xwork2.config.entities.ActionConfig;
import com.opensymphony.xwork2.config.entities.InterceptorMapping;
import com.opensymphony.xwork2.config.entities.InterceptorStackConfig;
import com.opensymphony.xwork2.config.entities.PackageConfig;
import com.opensymphony.xwork2.config.providers.XmlConfigurationProvider;
import com.opensymphony.xwork2.mock.MockActionInvocation;
import org.apache.logging.log4j.util.Strings;
import org.apache.struts2.ServletActionContext;
import org.apache.struts2.config.StrutsXmlConfigurationProvider;
import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.mock.web.MockHttpServletResponse;
import javax.servlet.http.HttpServletResponse;
import java.util.Arrays;
import java.util.Collection;
import java.util.Iterator;
import java.util.Objects;
import static org.apache.struts2.interceptor.ResourceIsolationPolicy.DEST_EMBED;
import static org.apache.struts2.interceptor.ResourceIsolationPolicy.DEST_OBJECT;
import static org.apache.struts2.interceptor.ResourceIsolationPolicy.DEST_SCRIPT;
@@ -32,25 +53,6 @@ import static org.apache.struts2.interceptor.ResourceIsolationPolicy.SITE_SAME_S
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.config.RuntimeConfiguration;
import com.opensymphony.xwork2.config.entities.ActionConfig;
import com.opensymphony.xwork2.config.entities.InterceptorMapping;
import com.opensymphony.xwork2.config.entities.InterceptorStackConfig;
import com.opensymphony.xwork2.config.entities.PackageConfig;
import com.opensymphony.xwork2.config.providers.XmlConfigurationProvider;
import com.opensymphony.xwork2.mock.MockActionInvocation;
import org.apache.struts2.ServletActionContext;
import org.apache.struts2.config.StrutsXmlConfigurationProvider;
import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.mock.web.MockHttpServletResponse;
import java.util.Arrays;
import java.util.Collection;
import java.util.Iterator;
import javax.servlet.http.HttpServletResponse;
public class FetchMetadataInterceptorTest extends XWorkTestCase {
private final FetchMetadataInterceptor interceptor = new FetchMetadataInterceptor();
@@ -79,7 +81,7 @@ public class FetchMetadataInterceptorTest extends XWorkTestCase {
}
public void testValidSite() throws Exception {
for (String header : Arrays.asList(SITE_SAME_ORIGIN, SITE_SAME_SITE, SITE_NONE)){
for (String header : Arrays.asList(SITE_SAME_ORIGIN, SITE_SAME_SITE, SITE_NONE)) {
request.addHeader(SEC_FETCH_SITE_HEADER, header);
assertNotEquals("Expected interceptor to accept this request", SC_FORBIDDEN, interceptor.intercept(mai));
@@ -147,12 +149,12 @@ public class FetchMetadataInterceptorTest extends XWorkTestCase {
interceptor.intercept(mai);
assertTrue("Expected vary header to be included", response.containsHeader(VARY_HEADER));
assertFalse("Expected original vary header content to be replaced", response.getHeader(VARY_HEADER).contains(ACCEPT_ENCODING_VALUE));
assertTrue("Expected added vary header content to be present", response.getHeader(VARY_HEADER).contains(VARY_HEADER_VALUE));
assertFalse("Expected original vary header content to be replaced", Objects.requireNonNull(response.getHeader(VARY_HEADER)).contains(ACCEPT_ENCODING_VALUE));
assertTrue("Expected added vary header content to be present", Objects.requireNonNull(response.getHeader(VARY_HEADER)).contains(VARY_HEADER_VALUE));
}
public void testSetExemptedPathsInjectionIndirectly() throws Exception {
// Perform a multi-step test to confirm (indirectly) that the method parameter injection of setExemptedPaths() for
// Perform a multistep test to confirm (indirectly) that the method parameter injection of setExemptedPaths() for
// the FetchMetadataInterceptor is functioning as expected, when configured appropriately.
// Ensure we're using the specific test configuration, not the default simple configuration.
XmlConfigurationProvider configurationProvider = new StrutsXmlConfigurationProvider("struts-testing.xml");
@@ -259,4 +261,12 @@ public class FetchMetadataInterceptorTest extends XWorkTestCase {
assertNotEquals("Expected interceptor to accept this request [" + "/" + fetchMetadataExemptedGlobalActionConfig.getName() + "]", SC_FORBIDDEN, configuredFetchMetadataInterceptor.intercept(mai));
}
public void testDisabled() throws Exception {
interceptor.setDisabled("true");
interceptor.intercept(mai);
String header = response.getHeader(VARY_HEADER);
assertTrue("Fetch Metadata is not disabled", Strings.isEmpty(header));
}
}