Cwikius-Spring/spring-security
1
0
Fork 0
mirror of synced 2026-08-05 17:57:15 +00:00
Code Issues Packages Projects Releases Wiki Activity

Add Cross Site Tracing (XST) & HTTP Method Tampering Protection

Browse Source 
Fixes: gh-5377
This commit is contained in:
Rob Winch
2018-05-24 09:35:27 -05:00
parent 2c92496911
commit 73345e7434
35 changed files with 276 additions and 87 deletions
Download Patch File Download Diff File Expand all files Collapse all files
web/src/main/java/org/springframework/security/web/FilterChainProxy.java
+1 -1
View File
@@ -238,7 +238,7 @@ public class FilterChainProxy extends GenericFilterBean {
* @return matching filter list
*/
public List<Filter> getFilters(String url) {
return getFilters(firewall.getFirewalledRequest((new FilterInvocation(url, null)
return getFilters(firewall.getFirewalledRequest((new FilterInvocation(url, "GET")
.getRequest())));
}
web/src/main/java/org/springframework/security/web/firewall/StrictHttpFirewall.java
+72
View File
@@ -16,6 +16,8 @@
package org.springframework.security.web.firewall;
import org.springframework.http.HttpMethod;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.util.Arrays;
@@ -35,6 +37,11 @@ import java.util.Set;
* </p>
* <ul>
* <li>
* Rejects HTTP methods that are not allowed. This specified to block
* <a href="https://www.owasp.org/index.php/Test_HTTP_Methods_(OTG-CONFIG-006)">HTTP Verb tampering and XST attacks</a>.
* See {@link #setAllowedHttpMethods(Collection)}
* </li>
* <li>
* Rejects URLs that are not normalized to avoid bypassing security constraints. There is
* no way to disable this as it is considered extremely risky to disable this constraint.
* A few options to allow this behavior is to normalize the request prior to the firewall
@@ -66,6 +73,11 @@ import java.util.Set;
* @since 4.2.4
*/
public class StrictHttpFirewall implements HttpFirewall {
/**
* Used to specify to {@link #setAllowedHttpMethods(Collection)} that any HTTP method should be allowed.
*/
private static final Set<String> ALLOW_ANY_HTTP_METHOD = Collections.unmodifiableSet(Collections.emptySet());
private static final String ENCODED_PERCENT = "%25";
private static final String PERCENT = "%";
@@ -82,6 +94,8 @@ public class StrictHttpFirewall implements HttpFirewall {
private Set<String> decodedUrlBlacklist = new HashSet<String>();
private Set<String> allowedHttpMethods = createDefaultAllowedHttpMethods();
public StrictHttpFirewall() {
urlBlacklistsAddAll(FORBIDDEN_SEMICOLON);
urlBlacklistsAddAll(FORBIDDEN_FORWARDSLASH);
@@ -92,6 +106,39 @@ public class StrictHttpFirewall implements HttpFirewall {
this.decodedUrlBlacklist.add(PERCENT);
}
/**
* Sets if any HTTP method is allowed. If this set to true, then no validation on the HTTP method will be performed.
* This can open the application up to <a href="https://www.owasp.org/index.php/Test_HTTP_Methods_(OTG-CONFIG-006)">
* HTTP Verb tampering and XST attacks</a>
* @param unsafeAllowAnyHttpMethod if true, disables HTTP method validation, else resets back to the defaults. Default is false.
* @see #setAllowedHttpMethods(Collection)
* @since 5.1
*/
public void setUnsafeAllowAnyHttpMethod(boolean unsafeAllowAnyHttpMethod) {
this.allowedHttpMethods = unsafeAllowAnyHttpMethod ? ALLOW_ANY_HTTP_METHOD : createDefaultAllowedHttpMethods();
}
/**
* <p>
* Determines which HTTP methods should be allowed. The default is to allow "DELETE", "GET", "HEAD", "OPTIONS",
* "PATCH", "POST", and "PUT".
* </p>
*
* @param allowedHttpMethods the case-sensitive collection of HTTP methods that are allowed.
* @see #setUnsafeAllowAnyHttpMethod(boolean)
* @since 5.1
*/
public void setAllowedHttpMethods(Collection<String> allowedHttpMethods) {
if (allowedHttpMethods == null) {
throw new IllegalArgumentException("allowedHttpMethods cannot be null");
}
if (allowedHttpMethods == ALLOW_ANY_HTTP_METHOD) {
this.allowedHttpMethods = ALLOW_ANY_HTTP_METHOD;
} else {
this.allowedHttpMethods = new HashSet<>(allowedHttpMethods);
}
}
/**
* <p>
* Determines if semicolon is allowed in the URL (i.e. matrix variables). The default
@@ -242,6 +289,7 @@ public class StrictHttpFirewall implements HttpFirewall {
@Override
public FirewalledRequest getFirewalledRequest(HttpServletRequest request) throws RequestRejectedException {
rejectForbiddenHttpMethod(request);
rejectedBlacklistedUrls(request);
if (!isNormalized(request)) {
@@ -259,6 +307,18 @@ public class StrictHttpFirewall implements HttpFirewall {
};
}
private void rejectForbiddenHttpMethod(HttpServletRequest request) {
if (this.allowedHttpMethods == ALLOW_ANY_HTTP_METHOD) {
return;
}
if (!this.allowedHttpMethods.contains(request.getMethod())) {
throw new RequestRejectedException("The request was rejected because the HTTP method \"" +
request.getMethod() +
"\" was not included within the whitelist " +
this.allowedHttpMethods);
}
}
private void rejectedBlacklistedUrls(HttpServletRequest request) {
for (String forbidden : this.encodedUrlBlacklist) {
if (encodedUrlContains(request, forbidden)) {
@@ -277,6 +337,18 @@ public class StrictHttpFirewall implements HttpFirewall {
return new FirewalledResponse(response);
}
private static Set<String> createDefaultAllowedHttpMethods() {
Set<String> result = new HashSet<>();
result.add(HttpMethod.DELETE.name());
result.add(HttpMethod.GET.name());
result.add(HttpMethod.HEAD.name());
result.add(HttpMethod.OPTIONS.name());
result.add(HttpMethod.PATCH.name());
result.add(HttpMethod.POST.name());
result.add(HttpMethod.PUT.name());
return result;
}
private static boolean isNormalized(HttpServletRequest request) {
if (!isNormalized(request.getRequestURI())) {
return false;
web/src/test/java/org/springframework/security/web/FilterChainProxyTests.java
+1 -1
View File
@@ -69,7 +69,7 @@ public class FilterChainProxyTests {
fcp = new FilterChainProxy(new DefaultSecurityFilterChain(matcher,
Arrays.asList(filter)));
fcp.setFilterChainValidator(mock(FilterChainProxy.FilterChainValidator.class));
request = new MockHttpServletRequest();
request = new MockHttpServletRequest("GET", "");
request.setServletPath("/path");
response = new MockHttpServletResponse();
chain = mock(FilterChain.class);
web/src/test/java/org/springframework/security/web/firewall/StrictHttpFirewallTests.java
+65 -10
View File
@@ -16,11 +16,17 @@
package org.springframework.security.web.firewall;
import org.junit.Test;
import org.springframework.mock.web.MockHttpServletRequest;
import static org.assertj.core.api.Assertions.assertThatCode;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.assertj.core.api.Assertions.fail;
import java.util.Arrays;
import java.util.List;
import org.junit.Test;
import org.springframework.http.HttpMethod;
import org.springframework.mock.web.MockHttpServletRequest;
/**
* @author Rob Winch
*/
@@ -31,12 +37,61 @@ public class StrictHttpFirewallTests {
private StrictHttpFirewall firewall = new StrictHttpFirewall();
private MockHttpServletRequest request = new MockHttpServletRequest();
private MockHttpServletRequest request = new MockHttpServletRequest("GET", "");
@Test
public void getFirewalledRequestWhenInvalidMethodThenThrowsRequestRejectedException() {
this.request.setMethod("INVALID");
assertThatThrownBy(() -> this.firewall.getFirewalledRequest(this.request))
.isInstanceOf(RequestRejectedException.class);
}
// blocks XST attacks
@Test
public void getFirewalledRequestWhenTraceMethodThenThrowsRequestRejectedException() {
this.request.setMethod(HttpMethod.TRACE.name());
assertThatThrownBy(() -> this.firewall.getFirewalledRequest(this.request))
.isInstanceOf(RequestRejectedException.class);
}
@Test
// blocks XST attack if request is forwarded to a Microsoft IIS web server
public void getFirewalledRequestWhenTrackMethodThenThrowsRequestRejectedException() {
this.request.setMethod("TRACK");
assertThatThrownBy(() -> this.firewall.getFirewalledRequest(this.request))
.isInstanceOf(RequestRejectedException.class);
}
@Test
// HTTP methods are case sensitive
public void getFirewalledRequestWhenLowercaseGetThenThrowsRequestRejectedException() {
this.request.setMethod("get");
assertThatThrownBy(() -> this.firewall.getFirewalledRequest(this.request))
.isInstanceOf(RequestRejectedException.class);
}
@Test
public void getFirewalledRequestWhenAllowedThenNoException() {
List<String> allowedMethods = Arrays.asList("DELETE", "GET", "HEAD", "OPTIONS", "PATCH", "POST", "PUT");
for (String allowedMethod : allowedMethods) {
this.request = new MockHttpServletRequest(allowedMethod, "");
assertThatCode(() -> this.firewall.getFirewalledRequest(this.request))
.doesNotThrowAnyException();
}
}
@Test
public void getFirewalledRequestWhenInvalidMethodAndAnyMethodThenNoException() {
this.firewall.setUnsafeAllowAnyHttpMethod(true);
this.request.setMethod("INVALID");
assertThatCode(() -> this.firewall.getFirewalledRequest(this.request))
.doesNotThrowAnyException();
}
@Test
public void getFirewalledRequestWhenRequestURINotNormalizedThenThrowsRequestRejectedException() throws Exception {
for (String path : this.unnormalizedPaths) {
this.request = new MockHttpServletRequest();
this.request = new MockHttpServletRequest("GET", "");
this.request.setRequestURI(path);
try {
this.firewall.getFirewalledRequest(this.request);
@@ -49,7 +104,7 @@ public class StrictHttpFirewallTests {
@Test
public void getFirewalledRequestWhenContextPathNotNormalizedThenThrowsRequestRejectedException() throws Exception {
for (String path : this.unnormalizedPaths) {
this.request = new MockHttpServletRequest();
this.request = new MockHttpServletRequest("GET", "");
this.request.setContextPath(path);
try {
this.firewall.getFirewalledRequest(this.request);
@@ -62,7 +117,7 @@ public class StrictHttpFirewallTests {
@Test
public void getFirewalledRequestWhenServletPathNotNormalizedThenThrowsRequestRejectedException() throws Exception {
for (String path : this.unnormalizedPaths) {
this.request = new MockHttpServletRequest();
this.request = new MockHttpServletRequest("GET", "");
this.request.setServletPath(path);
try {
this.firewall.getFirewalledRequest(this.request);
@@ -75,7 +130,7 @@ public class StrictHttpFirewallTests {
@Test
public void getFirewalledRequestWhenPathInfoNotNormalizedThenThrowsRequestRejectedException() throws Exception {
for (String path : this.unnormalizedPaths) {
this.request = new MockHttpServletRequest();
this.request = new MockHttpServletRequest("GET", "");
this.request.setPathInfo(path);
try {
this.firewall.getFirewalledRequest(this.request);
@@ -352,7 +407,7 @@ public class StrictHttpFirewallTests {
public void getFirewalledRequestWhenAllowUrlEncodedSlashAndLowercaseEncodedPathThenNoException() {
this.firewall.setAllowUrlEncodedSlash(true);
this.firewall.setAllowSemicolon(true);
MockHttpServletRequest request = new MockHttpServletRequest();
MockHttpServletRequest request = new MockHttpServletRequest("GET", "");
request.setRequestURI("/context-root/a/b;%2f1/c");
request.setContextPath("/context-root");
request.setServletPath("");
@@ -365,7 +420,7 @@ public class StrictHttpFirewallTests {
public void getFirewalledRequestWhenAllowUrlEncodedSlashAndUppercaseEncodedPathThenNoException() {
this.firewall.setAllowUrlEncodedSlash(true);
this.firewall.setAllowSemicolon(true);
MockHttpServletRequest request = new MockHttpServletRequest();
MockHttpServletRequest request = new MockHttpServletRequest("GET", "");
request.setRequestURI("/context-root/a/b;%2F1/c");
request.setContextPath("/context-root");
request.setServletPath("");