From 7fde346b32df769f7099438c40713f5a49ee28cc Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Fri, 29 Nov 2013 07:12:14 +0000 Subject: [PATCH] WW-4227 Temporally reverts changes related to SecurityGate to allow prepare new release without introducing new API git-svn-id: https://svn.apache.org/repos/asf/struts/struts2/trunk@1546514 13f79535-47bb-0310-9956-ffa450edef68 --- .../org/apache/struts2/StrutsConstants.java | 3 - .../apache/struts2/dispatcher/Dispatcher.java | 17 ------ .../dispatcher/ng/PrepareOperations.java | 1 - .../struts2/security/DefaultSecurityGate.java | 56 ------------------- .../security/ParameterNameSecurityGuard.java | 14 ----- .../security/ParameterValueSecurityGuard.java | 14 ----- .../apache/struts2/security/SecurityGate.java | 12 ---- .../struts2/security/SecurityGuard.java | 12 ---- .../apache/struts2/security/SecurityPass.java | 36 ------------ .../security/StrutsSecurityException.java | 14 ----- core/src/main/resources/struts-default.xml | 4 -- .../ParameterNameSecurityGuardTest.java | 28 ---------- .../ParameterValueSecurityGuardTest.java | 28 ---------- 13 files changed, 239 deletions(-) delete mode 100644 core/src/main/java/org/apache/struts2/security/DefaultSecurityGate.java delete mode 100644 core/src/main/java/org/apache/struts2/security/ParameterNameSecurityGuard.java delete mode 100644 core/src/main/java/org/apache/struts2/security/ParameterValueSecurityGuard.java delete mode 100644 core/src/main/java/org/apache/struts2/security/SecurityGate.java delete mode 100644 core/src/main/java/org/apache/struts2/security/SecurityGuard.java delete mode 100644 core/src/main/java/org/apache/struts2/security/SecurityPass.java delete mode 100644 core/src/main/java/org/apache/struts2/security/StrutsSecurityException.java delete mode 100644 core/src/test/java/org/apache/struts2/security/ParameterNameSecurityGuardTest.java delete mode 100644 core/src/test/java/org/apache/struts2/security/ParameterValueSecurityGuardTest.java diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java b/core/src/main/java/org/apache/struts2/StrutsConstants.java index 732b96046..1cb4ab578 100644 --- a/core/src/main/java/org/apache/struts2/StrutsConstants.java +++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java @@ -271,9 +271,6 @@ public final class StrutsConstants { /** actions names' whitelist **/ public static final String STRUTS_ALLOWED_ACTION_NAMES = "struts.allowed.action.names"; - /** Security firewall **/ - public static final String STRUTS_SECURITY_GATE = "struts.securityGate"; - /** enables action: prefix **/ public static final String STRUTS_MAPPER_ACTION_PREFIX_ENABLED = "struts.mapper.action.prefix.enabled"; diff --git a/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java b/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java index 588fedc8a..df4ba8382 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java @@ -65,7 +65,6 @@ import org.apache.struts2.config.StrutsXmlConfigurationProvider; import org.apache.struts2.dispatcher.mapper.ActionMapping; import org.apache.struts2.dispatcher.multipart.MultiPartRequest; import org.apache.struts2.dispatcher.multipart.MultiPartRequestWrapper; -import org.apache.struts2.security.SecurityGate; import org.apache.struts2.util.AttributeMap; import org.apache.struts2.util.ObjectFactoryDestroyable; import org.apache.struts2.util.fs.JBossFileManager; @@ -210,8 +209,6 @@ public class Dispatcher { private ValueStackFactory valueStackFactory; - private SecurityGate securityGate; - /** * Create the Dispatcher instance for a given ServletContext and set of initialization parameters. * @@ -283,11 +280,6 @@ public class Dispatcher { this.handleException = Boolean.parseBoolean(handleException); } - @Inject - public void setSecurityGate(SecurityGate securityGate) { - this.securityGate = securityGate; - } - /** * Releases all instances bound to this dispatcher instance. */ @@ -936,15 +928,6 @@ public class Dispatcher { ContainerHolder.clear(); } - /** - * Checks if request doesn't contain suspicious values - * - * @param request current {@link HttpServletRequest} - */ - public void checkRequest(HttpServletRequest request) { - securityGate.check(request); - } - /** * Provide an accessor class for static XWork utility. */ diff --git a/core/src/main/java/org/apache/struts2/dispatcher/ng/PrepareOperations.java b/core/src/main/java/org/apache/struts2/dispatcher/ng/PrepareOperations.java index 0a055ea9e..93e105510 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/ng/PrepareOperations.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/ng/PrepareOperations.java @@ -158,7 +158,6 @@ public class PrepareOperations { ActionMapping mapping = (ActionMapping) request.getAttribute(STRUTS_ACTION_MAPPING_KEY); if (mapping == null || forceLookup) { try { - dispatcher.checkRequest(request); mapping = dispatcher.getContainer().getInstance(ActionMapper.class).getMapping(request, dispatcher.getConfigurationManager()); if (mapping != null) { request.setAttribute(STRUTS_ACTION_MAPPING_KEY, mapping); diff --git a/core/src/main/java/org/apache/struts2/security/DefaultSecurityGate.java b/core/src/main/java/org/apache/struts2/security/DefaultSecurityGate.java deleted file mode 100644 index 35e3fa48e..000000000 --- a/core/src/main/java/org/apache/struts2/security/DefaultSecurityGate.java +++ /dev/null @@ -1,56 +0,0 @@ -package org.apache.struts2.security; - -import com.opensymphony.xwork2.inject.Container; -import com.opensymphony.xwork2.inject.Inject; -import com.opensymphony.xwork2.util.logging.Logger; -import com.opensymphony.xwork2.util.logging.LoggerFactory; -import org.apache.struts2.StrutsConstants; - -import javax.servlet.http.HttpServletRequest; -import java.util.ArrayList; -import java.util.List; -import java.util.Set; - -/** - * Default implementation of {@link org.apache.struts2.security.SecurityGate} - * just examines all the defined {@link org.apache.struts2.security.SecurityGuard}'s - */ -public class DefaultSecurityGate implements SecurityGate { - - private static final Logger LOG = LoggerFactory.getLogger(DefaultSecurityGate.class); - - private List guards; - private boolean devMode; - - @Inject(StrutsConstants.STRUTS_DEVMODE) - public void setDevMode(String devMode) { - this.devMode = "true".equalsIgnoreCase(devMode); - } - - @Inject - public void setContainer(Container container) { - guards = new ArrayList(); - Set guardNames = container.getInstanceNames(SecurityGate.class); - for (String guardName : guardNames) { - SecurityGuard guard = container.getInstance(SecurityGuard.class, guardName); - if (guard != null) { - guards.add(guard); - } else if (devMode) { - LOG.debug("Got null instance of [#0] for name [#1]", SecurityGuard.class.getSimpleName(), guardName); - } - } - } - - public void check(HttpServletRequest request) { - for (SecurityGuard guard : guards) { - SecurityPass pass = guard.accept(request); - if (pass.isNotAccepted()) { - if (LOG.isDebugEnabled()) { - LOG.debug("[#0] didn't accept the request!", guard.getClass().getName()); - } - throw new StrutsSecurityException(pass.getGuardMessage()); - } - } - } - -} diff --git a/core/src/main/java/org/apache/struts2/security/ParameterNameSecurityGuard.java b/core/src/main/java/org/apache/struts2/security/ParameterNameSecurityGuard.java deleted file mode 100644 index 8f738cdb1..000000000 --- a/core/src/main/java/org/apache/struts2/security/ParameterNameSecurityGuard.java +++ /dev/null @@ -1,14 +0,0 @@ -package org.apache.struts2.security; - -import javax.servlet.http.HttpServletRequest; - -/** - * Checks if parameter name is valida and it doesn't contain vulnerable code - */ -public class ParameterNameSecurityGuard implements SecurityGuard { - - public SecurityPass accept(HttpServletRequest request) { - return SecurityPass.accepted(); - } - -} diff --git a/core/src/main/java/org/apache/struts2/security/ParameterValueSecurityGuard.java b/core/src/main/java/org/apache/struts2/security/ParameterValueSecurityGuard.java deleted file mode 100644 index be463cbd5..000000000 --- a/core/src/main/java/org/apache/struts2/security/ParameterValueSecurityGuard.java +++ /dev/null @@ -1,14 +0,0 @@ -package org.apache.struts2.security; - -import javax.servlet.http.HttpServletRequest; - -/** - * Checks if parameter's value doesn't contain vulnerable code - */ -public class ParameterValueSecurityGuard implements SecurityGuard { - - public SecurityPass accept(HttpServletRequest request) { - return SecurityPass.accepted(); - } - -} diff --git a/core/src/main/java/org/apache/struts2/security/SecurityGate.java b/core/src/main/java/org/apache/struts2/security/SecurityGate.java deleted file mode 100644 index 21a793a4c..000000000 --- a/core/src/main/java/org/apache/struts2/security/SecurityGate.java +++ /dev/null @@ -1,12 +0,0 @@ -package org.apache.struts2.security; - -import javax.servlet.http.HttpServletRequest; - -/** - * Main - */ -public interface SecurityGate { - - void check(HttpServletRequest request); - -} diff --git a/core/src/main/java/org/apache/struts2/security/SecurityGuard.java b/core/src/main/java/org/apache/struts2/security/SecurityGuard.java deleted file mode 100644 index 942dd3820..000000000 --- a/core/src/main/java/org/apache/struts2/security/SecurityGuard.java +++ /dev/null @@ -1,12 +0,0 @@ -package org.apache.struts2.security; - -import javax.servlet.http.HttpServletRequest; - -/** - * TODO lukaszlenart: write a JavaDoc - */ -public interface SecurityGuard { - - SecurityPass accept(HttpServletRequest request); - -} diff --git a/core/src/main/java/org/apache/struts2/security/SecurityPass.java b/core/src/main/java/org/apache/struts2/security/SecurityPass.java deleted file mode 100644 index 0993c2fbc..000000000 --- a/core/src/main/java/org/apache/struts2/security/SecurityPass.java +++ /dev/null @@ -1,36 +0,0 @@ -package org.apache.struts2.security; - -/** - * TODO lukaszlenart: write a JavaDoc - */ -public class SecurityPass { - - private Boolean accepted; - private final String message; - - public static SecurityPass accepted() { - return new SecurityPass(true, null); - } - - public static SecurityPass notAccepted(String message) { - return new SecurityPass(false, message); - } - - private SecurityPass(boolean accepted, String message) { - this.accepted = accepted; - this.message = message; - } - - public String getGuardMessage() { - return message; - } - - public boolean isAccepted() { - return accepted; - } - - public boolean isNotAccepted() { - return !accepted; - } - -} diff --git a/core/src/main/java/org/apache/struts2/security/StrutsSecurityException.java b/core/src/main/java/org/apache/struts2/security/StrutsSecurityException.java deleted file mode 100644 index 9e17dcc50..000000000 --- a/core/src/main/java/org/apache/struts2/security/StrutsSecurityException.java +++ /dev/null @@ -1,14 +0,0 @@ -package org.apache.struts2.security; - -import org.apache.struts2.StrutsException; - -/** - * Exception indicates possible security breach - */ -public class StrutsSecurityException extends StrutsException { - - public StrutsSecurityException(String message) { - super(message); - } - -} diff --git a/core/src/main/resources/struts-default.xml b/core/src/main/resources/struts-default.xml index 572ce5ebe..635b241cb 100644 --- a/core/src/main/resources/struts-default.xml +++ b/core/src/main/resources/struts-default.xml @@ -137,10 +137,6 @@ - - - - diff --git a/core/src/test/java/org/apache/struts2/security/ParameterNameSecurityGuardTest.java b/core/src/test/java/org/apache/struts2/security/ParameterNameSecurityGuardTest.java deleted file mode 100644 index 2ffae0ed8..000000000 --- a/core/src/test/java/org/apache/struts2/security/ParameterNameSecurityGuardTest.java +++ /dev/null @@ -1,28 +0,0 @@ -package org.apache.struts2.security; - -import com.mockobjects.servlet.MockHttpServletRequest; -import org.junit.Test; - -import javax.servlet.http.HttpServletRequest; - -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; - -public class ParameterNameSecurityGuardTest { - - @Test - public void shouldPass() throws Exception { - // given - SecurityGuard guard = new ParameterNameSecurityGuard(); - - HttpServletRequest request = new MockHttpServletRequest(); - - // when - SecurityPass pass = guard.accept(request); - - // then - assertTrue(pass.isAccepted()); - assertNull(pass.getGuardMessage()); - } - -} diff --git a/core/src/test/java/org/apache/struts2/security/ParameterValueSecurityGuardTest.java b/core/src/test/java/org/apache/struts2/security/ParameterValueSecurityGuardTest.java deleted file mode 100644 index 8203d4d6e..000000000 --- a/core/src/test/java/org/apache/struts2/security/ParameterValueSecurityGuardTest.java +++ /dev/null @@ -1,28 +0,0 @@ -package org.apache.struts2.security; - -import com.mockobjects.servlet.MockHttpServletRequest; -import org.junit.Test; - -import javax.servlet.http.HttpServletRequest; - -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; - -public class ParameterValueSecurityGuardTest { - - @Test - public void shouldPass() throws Exception { - // given - SecurityGuard guard = new ParameterValueSecurityGuard(); - - HttpServletRequest request = new MockHttpServletRequest(); - - // when - SecurityPass pass = guard.accept(request); - - // then - assertTrue(pass.isAccepted()); - assertNull(pass.getGuardMessage()); - } - -}