Filter requests to a list of Spring-managed beans.
- * As of version 2.0, you shouldn't need to explicitly configure a FilterChainProxy bean in your application
+ * Delegates {@code Filter} requests to a list of Spring-managed filter beans.
+ * As of version 2.0, you shouldn't need to explicitly configure a {@code FilterChainProxy} bean in your application
* context unless you need very fine control over the filter chain contents. Most cases should be adequately covered
- * by the default <security:http /> namespace configuration options.
+ * by the default <security:http /> namespace configuration options.
*
* The FilterChainProxy is loaded via a standard Spring {@link DelegatingFilterProxy} declaration in
* web.xml. FilterChainProxy will then pass {@link #init(FilterConfig)}, {@link #destroy()}
@@ -71,7 +71,7 @@ import org.springframework.web.filter.GenericFilterBean;
</bean>
*
*
- * The names "filter1", "filter2", "filter3" should be the bean names of Filter instances defined in the
+ * The names "filter1", "filter2", "filter3" should be the bean names of {@code Filter} instances defined in the
* application context. The order of the names defines the order in which the filters will be applied. As shown above,
* use of the value "none" for the "filters" can be used to exclude
* Please consult the security namespace schema file for a full list of available configuration options.
@@ -86,17 +86,32 @@ import org.springframework.web.filter.GenericFilterBean;
*
*
FilterChainProxy respects normal handling of Filters that elect not to call {@link
* javax.servlet.Filter#doFilter(javax.servlet.ServletRequest, javax.servlet.ServletResponse,
- * javax.servlet.FilterChain)}, in that the remainder of the original or FilterChainProxy-declared filter
+ * javax.servlet.FilterChain)}, in that the remainder of the original or {@code FilterChainProxy}-declared filter
* chain will not be called.
*
- *
Note the Filter lifecycle mismatch between the servlet container and IoC
- * container. As described in the {@link DelegatingFilterProxy} JavaDocs, we recommend you allow the IoC
- * container to manage the lifecycle instead of the servlet container. By default the DelegatingFilterProxy
- * will never call this class' {@link #init(FilterConfig)} and {@link #destroy()} methods, which in turns means that
- * the corresponding methods on the filter beans managed by this class will never be called. If you do need your filters to be
- * initialized and destroyed, please set the targetFilterLifecycle initialization parameter against the
- * DelegatingFilterProxy to specify that servlet container lifecycle management should be used. You don't
- * need to worry about this in most cases.
+ *
+ * Note that this means that you must use the Spring Security filters in combination with a {@code FilterChainProxy} + * if you want this protection. Don't define them explicitly in your {@code web.xml} file. + *
+ * {@code FilterChainProxy} will use the firewall instance to obtain both request and response objects which will be + * fed down the filter chain, so it is also possible to use this functionality to control the functionality of the + * response. When the request has passed through the security filter chain, the {@code reset} method will be called. + * With the default implementation this means that the original values of {@code servletPath} and {@code pathInfo} will + * be returned thereafter, instead of the modified ones used for security pattern matching. + * + *
+ * Note the {@code Filter} lifecycle mismatch between the servlet container and IoC
+ * container. As described in the {@link DelegatingFilterProxy} Javadocs, we recommend you allow the IoC
+ * container to manage the lifecycle instead of the servlet container. {@code FilterChainProxy} does not invoke the
+ * standard filter lifecycle methods on any filter beans that you add to the application context.
*
* @author Carlos Sanchez
* @author Ben Alex
@@ -118,6 +133,7 @@ public class FilterChainProxy extends GenericFilterBean {
private Map
- * This
+ * If any un-normalized paths are found (containing directory-traversal character sequences), the request will be
+ * rejected immediately. Most containers normalize the paths before performing the servlet-mapping, but again this is
+ * not guaranteed by the servlet spec.
+ *
+ * @author Luke Taylor
+ */
+public class DefaultHttpFirewall implements HttpFirewall {
+
+ public FirewalledRequest getFirewalledRequest(HttpServletRequest request) throws RequestRejectedException {
+ FirewalledRequest fwr = new RequestWrapper(request);
+
+ if (!isNormalized(fwr.getServletPath()) || !isNormalized(fwr.getPathInfo())) {
+ throw new RequestRejectedException("Un-normalized paths are not supported: " + fwr.getServletPath() +
+ (fwr.getPathInfo() != null ? fwr.getPathInfo() : ""));
+ }
+
+ return fwr;
+ }
+
+ public HttpServletResponse getFirewalledResponse(HttpServletResponse response) {
+ return response;
+ }
+
+ /**
+ * Checks whether a path is normalized (doesn't contain path traversal sequences like "./", "/../" or "/.")
+ *
+ * @param path the path to test
+ * @return true if the path doesn't contain any path-traversal character sequences.
+ */
+ private boolean isNormalized(String path) {
+ if (path == null) {
+ return true;
+ }
+
+ for (int j = path.length(); j > 0;) {
+ int i = path.lastIndexOf('/', j - 1);
+ int gap = j - i;
+
+ if (gap == 2 && path.charAt(i+1) == '.') {
+ // ".", "/./" or "/."
+ return false;
+ } else if (gap == 3 && path.charAt(i+1) == '.'&& path.charAt(i+2) == '.') {
+ return false;
+ }
+
+ j = i;
+ }
+
+ return true;
+ }
+
+}
diff --git a/web/src/main/java/org/springframework/security/web/firewall/FirewalledRequest.java b/web/src/main/java/org/springframework/security/web/firewall/FirewalledRequest.java
new file mode 100644
index 0000000000..48cf40acc2
--- /dev/null
+++ b/web/src/main/java/org/springframework/security/web/firewall/FirewalledRequest.java
@@ -0,0 +1,34 @@
+package org.springframework.security.web.firewall;
+
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletRequestWrapper;
+
+/**
+ * Request wrapper which is returned by the {@code HttpFirewall} interface.
+ *
+ * The only difference is the {@code reset} method which allows some
+ * or all of the state to be reset by the {@code FilterChainProxy} when the
+ * request leaves the security filter chain.
+ *
+ * @author Luke Taylor
+ */
+public abstract class FirewalledRequest extends HttpServletRequestWrapper {
+ /**
+ * Constructs a request object wrapping the given request.
+ *
+ * @throws IllegalArgumentException if the request is null
+ */
+ public FirewalledRequest(HttpServletRequest request) {
+ super(request);
+ }
+
+ /**
+ * This method will be called once the request has passed through the
+ * security filter chain, when it is about to proceed to the application
+ * proper.
+ *
+ * An implementation can thus choose to modify the state of the request
+ * for the security infrastructure, while still maintaining the
+ */
+ public abstract void reset();
+}
diff --git a/web/src/main/java/org/springframework/security/web/firewall/HttpFirewall.java b/web/src/main/java/org/springframework/security/web/firewall/HttpFirewall.java
new file mode 100644
index 0000000000..de7b9af17e
--- /dev/null
+++ b/web/src/main/java/org/springframework/security/web/firewall/HttpFirewall.java
@@ -0,0 +1,32 @@
+package org.springframework.security.web.firewall;
+
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+/**
+ * Interface which can be used to reject potentially dangerous requests and/or wrap them to
+ * control their behaviour.
+ *
+ * The implementation is injected into the {@code FilterChainProxy} and will be invoked before
+ * sending any request through the filter chain. It can also provide a response wrapper if the response
+ * behaviour should also be restricted.
+ *
+ * @author Luke Taylor
+ */
+public interface HttpFirewall {
+
+ /**
+ * Provides the request object which will be passed through the filter chain.
+ *
+ * @throws RequestRejectedException if the request should be rejected immediately
+ */
+ FirewalledRequest getFirewalledRequest(HttpServletRequest request) throws RequestRejectedException;
+
+ /**
+ * Provides the response which will be passed through the filter chain.
+ *
+ * @param response the original response
+ * @return either the original response or a replacement/wrapper.
+ */
+ HttpServletResponse getFirewalledResponse(HttpServletResponse response);
+}
diff --git a/web/src/main/java/org/springframework/security/web/firewall/RequestRejectedException.java b/web/src/main/java/org/springframework/security/web/firewall/RequestRejectedException.java
new file mode 100644
index 0000000000..fe334219a0
--- /dev/null
+++ b/web/src/main/java/org/springframework/security/web/firewall/RequestRejectedException.java
@@ -0,0 +1,10 @@
+package org.springframework.security.web.firewall;
+
+/**
+ * @author Luke Taylor
+ */
+public class RequestRejectedException extends RuntimeException {
+ public RequestRejectedException(String message) {
+ super(message);
+ }
+}
diff --git a/web/src/main/java/org/springframework/security/web/firewall/RequestWrapper.java b/web/src/main/java/org/springframework/security/web/firewall/RequestWrapper.java
new file mode 100644
index 0000000000..1c6e25de8e
--- /dev/null
+++ b/web/src/main/java/org/springframework/security/web/firewall/RequestWrapper.java
@@ -0,0 +1,100 @@
+package org.springframework.security.web.firewall;
+
+import javax.servlet.http.HttpServletRequest;
+import java.util.*;
+
+/**
+ * Request wrapper which ensures values of {@code servletPath} and {@code pathInfo} are returned which are suitable for
+ * pattern matching against. It strips out path parameters and extra consecutive '/' characters.
+ *
+ *
+ * The parameter sequence is demarcated by a semi-colon, so each segment is checked for the occurrence of a ";"
+ * character and truncated at that point if it is present.
+ *
+ * The behaviour differs between servlet containers in how they interpret the servlet spec, which
+ * does not clearly state what the behaviour should be. For consistency, we make sure they are always removed, to
+ * avoid the risk of URL matching rules being bypassed by the malicious addition of parameters to the path component.
+ *
+ * @author Luke Taylor
+ */
+final class RequestWrapper extends FirewalledRequest {
+ private final String strippedServletPath;
+ private final String strippedPathInfo;
+ private boolean stripPaths = true;
+
+ public RequestWrapper(HttpServletRequest request) {
+ super(request);
+ strippedServletPath = strip(request.getServletPath());
+ String pathInfo = strip(request.getPathInfo());
+ if (pathInfo != null && pathInfo.length() == 0) {
+ pathInfo = null;
+ }
+ strippedPathInfo = pathInfo;
+ }
+
+ /**
+ * Removes path parameters from each path segment in the supplied path and truncates sequences of multiple '/'
+ * characters to a single '/'.
+ *
+ * @param path either the {@code servletPath} and {@code pathInfo} from the original request
+ *
+ * @return the supplied value, with path parameters removed and sequences of multiple '/' characters truncated,
+ * or null if the supplied path was null.
+ */
+ private String strip(String path) {
+ if (path == null) {
+ return null;
+ }
+
+ int scIndex = path.indexOf(';');
+
+ if (scIndex < 0) {
+ int doubleSlashIndex = path.indexOf("//");
+ if (doubleSlashIndex < 0) {
+ // Most likely case, no parameters in any segment and no '//', so no stripping required
+ return path;
+ }
+ }
+
+ StringTokenizer st = new StringTokenizer(path, "/");
+ StringBuilder stripped = new StringBuilder(path.length());
+
+ if (path.charAt(0) == '/') {
+ stripped.append('/');
+ }
+
+ while(st.hasMoreTokens()) {
+ String segment = st.nextToken();
+ scIndex = segment.indexOf(';');
+
+ if (scIndex >= 0) {
+ segment = segment.substring(0, scIndex);
+ }
+ stripped.append(segment).append('/');
+ }
+
+ // Remove the trailing slash if the original path didn't have one
+ if (path.charAt(path.length() - 1) != '/') {
+ stripped.deleteCharAt(stripped.length() - 1);
+ }
+
+ return stripped.toString();
+ }
+
+ @Override
+ public String getPathInfo() {
+ return stripPaths ? strippedPathInfo : super.getPathInfo();
+ }
+
+ @Override
+ public String getServletPath() {
+ return stripPaths ? strippedServletPath : super.getServletPath();
+ }
+
+ public void reset() {
+ this.stripPaths = false;
+ }
+}
diff --git a/web/src/test/java/org/springframework/security/web/FilterChainProxyTests.java b/web/src/test/java/org/springframework/security/web/FilterChainProxyTests.java
new file mode 100644
index 0000000000..a381b44f09
--- /dev/null
+++ b/web/src/test/java/org/springframework/security/web/FilterChainProxyTests.java
@@ -0,0 +1,103 @@
+package org.springframework.security.web;
+
+import static org.junit.Assert.assertEquals;
+import static org.mockito.Mockito.*;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
+import org.springframework.mock.web.MockHttpServletRequest;
+import org.springframework.mock.web.MockHttpServletResponse;
+import org.springframework.security.web.firewall.FirewalledRequest;
+
+import javax.servlet.Filter;
+import javax.servlet.FilterChain;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.util.*;
+
+/**
+ * @author Luke Taylor
+ */
+@SuppressWarnings({"unchecked"})
+public class FilterChainProxyTests {
+ private FilterChainProxy fcp;
+ private MockHttpServletRequest request;
+ private MockHttpServletResponse response;
+ private FilterChain chain;
+ private Filter filter;
+
+ @Before
+ public void setup() throws Exception {
+ fcp = new FilterChainProxy();
+ filter = mock(Filter.class);
+ doAnswer(new Answer() {
+ public Object answer(InvocationOnMock inv) throws Throwable {
+ Object[] args = inv.getArguments();
+ FilterChain fc = (FilterChain) args[2];
+ fc.doFilter((HttpServletRequest) args[0], (HttpServletResponse) args[1]);
+ return null;
+ }
+ }).when(filter).doFilter(any(HttpServletRequest.class), any(HttpServletResponse.class), any(FilterChain.class));
+ LinkedHashMap map = new LinkedHashMap();
+ map.put("/match", Arrays.asList(filter));
+ fcp.setFilterChainMap(map);
+ request = new MockHttpServletRequest();
+ request.setServletPath("/match");
+ response = new MockHttpServletResponse();
+ chain = mock(FilterChain.class);
+ }
+
+ @Test
+ public void toStringCallSucceeds() throws Exception {
+ fcp.afterPropertiesSet();
+ fcp.toString();
+ }
+
+ @Test
+ public void securityFilterChainIsNotInvokedIfMatchFails() throws Exception {
+ request.setServletPath("/nomatch");
+ fcp.doFilter(request, response, chain);
+ assertEquals(1, fcp.getFilterChainMap().size());
+
+ verifyZeroInteractions(filter);
+ // The actual filter chain should be invoked though
+ verify(chain).doFilter(any(HttpServletRequest.class), any(HttpServletResponse.class));
+ }
+
+ @Test
+ public void originalChainIsInvokedAfterSecurityChainIfMatchSucceeds() throws Exception {
+ fcp.doFilter(request, response, chain);
+
+ verify(filter).doFilter(any(HttpServletRequest.class), any(HttpServletResponse.class), any(FilterChain.class));
+ verify(chain).doFilter(any(HttpServletRequest.class), any(HttpServletResponse.class));
+ }
+
+ @Test
+ public void originalFilterChainIsInvokedIfMatchingSecurityChainIsEmpty() throws Exception {
+ LinkedHashMap map = new LinkedHashMap();
+ map.put("/match", Collections.emptyList());
+ fcp.setFilterChainMap(map);
+
+ fcp.doFilter(request, response, chain);
+
+ verify(chain).doFilter(any(HttpServletRequest.class), any(HttpServletResponse.class));
+ }
+
+ @Test
+ public void requestIsWrappedForFilteringWhenMatchIsFound() throws Exception {
+ fcp.doFilter(request, response, chain);
+ verify(filter).doFilter(any(FirewalledRequest.class), any(HttpServletResponse.class), any(FilterChain.class));
+ verify(chain).doFilter(any(FirewalledRequest.class), any(HttpServletResponse.class));
+ }
+
+ @Test
+ public void requestIsWrappedForFilteringWhenMatchIsNotFound() throws Exception {
+ request.setServletPath("/nomatch");
+ fcp.doFilter(request, response, chain);
+ verifyZeroInteractions(filter);
+ verify(chain).doFilter(any(FirewalledRequest.class), any(HttpServletResponse.class));
+ }
+
+}
diff --git a/web/src/test/java/org/springframework/security/web/firewall/DefaultHttpFirewallTests.java b/web/src/test/java/org/springframework/security/web/firewall/DefaultHttpFirewallTests.java
new file mode 100644
index 0000000000..8b3df587e9
--- /dev/null
+++ b/web/src/test/java/org/springframework/security/web/firewall/DefaultHttpFirewallTests.java
@@ -0,0 +1,44 @@
+package org.springframework.security.web.firewall;
+
+import static org.junit.Assert.fail;
+
+import org.junit.Test;
+import org.springframework.mock.web.MockHttpServletRequest;
+
+/**
+ * @author Luke Taylor
+ */
+public class DefaultHttpFirewallTests {
+ public String[] unnormalizedPaths = {
+ "/..",
+ "/./path/",
+ "/path/path/.",
+ "/path/path//.",
+ "./path/../path//.",
+ "./path",
+ ".//path",
+ "."
+ };
+
+ @Test
+ public void unnormalizedPathsAreRejected() throws Exception {
+ DefaultHttpFirewall fw = new DefaultHttpFirewall();
+
+ MockHttpServletRequest request;
+ for (String path : unnormalizedPaths) {
+ request = new MockHttpServletRequest();
+ request.setServletPath(path);
+ try {
+ fw.getFirewalledRequest(request);
+ fail(path + " is un-normalized");
+ } catch (RequestRejectedException expected) {
+ }
+ request.setPathInfo(path);
+ try {
+ fw.getFirewalledRequest(request);
+ fail(path + " is un-normalized");
+ } catch (RequestRejectedException expected) {
+ }
+ }
+ }
+}
diff --git a/web/src/test/java/org/springframework/security/web/firewall/RequestWrapperTests.java b/web/src/test/java/org/springframework/security/web/firewall/RequestWrapperTests.java
new file mode 100644
index 0000000000..b462251440
--- /dev/null
+++ b/web/src/test/java/org/springframework/security/web/firewall/RequestWrapperTests.java
@@ -0,0 +1,62 @@
+package org.springframework.security.web.firewall;
+
+import static org.junit.Assert.assertEquals;
+
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.springframework.mock.web.MockHttpServletRequest;
+
+import java.util.*;
+
+/**
+ * @author Luke Taylor
+ */
+public class RequestWrapperTests {
+ private static MapFilterChain that records whether or not {@link
- * FilterChain#doFilter(javax.servlet.ServletRequest, javax.servlet.ServletResponse)} is called.
- * FilterChain is used by FilterChainProxy to determine if the next
- * Filter should be called or not.Path Parameters
+ * Parameters (as defined in RFC 2396) are stripped from the path
+ * segments of the {@code servletPath} and {@code pathInfo} values of the request.
+ *