Author: violetagg
Date: Sun Oct 18 18:05:06 2015
New Revision: 1709295
URL: http://svn.apache.org/viewvc?rev=1709295&view=rev
Log:
There are use cases when a nonce information cannot be provided via header.
This commit introduces a mechanism to provide it via request parameters.
If there is a X-CSRF-Token header, it will be taken with preference over any
parameter with the same name in the request.
Request parameters cannot be used to fetch new nonce, only header.
Only configured paths can accept such request parameters with nonce
information.
Modified:
tomcat/trunk/java/org/apache/catalina/filters/CsrfPreventionFilter.java
tomcat/trunk/java/org/apache/catalina/filters/CsrfPreventionFilterBase.java
tomcat/trunk/java/org/apache/catalina/filters/RestCsrfPreventionFilter.java
tomcat/trunk/test/org/apache/catalina/filters/TestRestCsrfPreventionFilter.java
Modified:
tomcat/trunk/java/org/apache/catalina/filters/CsrfPreventionFilter.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/filters/CsrfPreventionFilter.java?rev=1709295&r1=1709294&r2=1709295&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/filters/CsrfPreventionFilter.java
(original)
+++ tomcat/trunk/java/org/apache/catalina/filters/CsrfPreventionFilter.java Sun
Oct 18 18:05:06 2015
@@ -92,15 +92,9 @@ public class CsrfPreventionFilter extend
boolean skipNonceCheck = false;
- if (Constants.METHOD_GET.equals(req.getMethod())) {
- String path = req.getServletPath();
- if (req.getPathInfo() != null) {
- path = path + req.getPathInfo();
- }
-
- if (entryPoints.contains(path)) {
- skipNonceCheck = true;
- }
+ if (Constants.METHOD_GET.equals(req.getMethod())
+ && entryPoints.contains(getRequestedPath(req))) {
+ skipNonceCheck = true;
}
HttpSession session = req.getSession(false);
Modified:
tomcat/trunk/java/org/apache/catalina/filters/CsrfPreventionFilterBase.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/filters/CsrfPreventionFilterBase.java?rev=1709295&r1=1709294&r2=1709295&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/filters/CsrfPreventionFilterBase.java
(original)
+++ tomcat/trunk/java/org/apache/catalina/filters/CsrfPreventionFilterBase.java
Sun Oct 18 18:05:06 2015
@@ -21,6 +21,7 @@ import java.util.Random;
import javax.servlet.FilterConfig;
import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.apache.juli.logging.Log;
@@ -121,4 +122,11 @@ public abstract class CsrfPreventionFilt
return buffer.toString();
}
+ protected String getRequestedPath(HttpServletRequest request) {
+ String path = request.getServletPath();
+ if (request.getPathInfo() != null) {
+ path = path + request.getPathInfo();
+ }
+ return path;
+ }
}
Modified:
tomcat/trunk/java/org/apache/catalina/filters/RestCsrfPreventionFilter.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/filters/RestCsrfPreventionFilter.java?rev=1709295&r1=1709294&r2=1709295&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/filters/RestCsrfPreventionFilter.java
(original)
+++ tomcat/trunk/java/org/apache/catalina/filters/RestCsrfPreventionFilter.java
Sun Oct 18 18:05:06 2015
@@ -17,7 +17,10 @@
package org.apache.catalina.filters;
import java.io.IOException;
+import java.util.Arrays;
+import java.util.HashSet;
import java.util.Objects;
+import java.util.Set;
import java.util.function.Predicate;
import java.util.regex.Pattern;
@@ -30,12 +33,9 @@ import javax.servlet.http.HttpServletRes
import javax.servlet.http.HttpSession;
/**
- * Provides basic CSRF protection for REST APIs.
- * The filter assumes that:
- * <ul>
- * <li>The filter is mapped to /*</li>
- * <li>The clients have adapted the transfer of the nonce through the
'X-CSRF-Token' header.</li>
- * </ul>
+ * Provides basic CSRF protection for REST APIs. The filter assumes that the
+ * clients have adapted the transfer of the nonce through the 'X-CSRF-Token'
+ * header.
*
* <pre>
* Positive scenario:
@@ -81,9 +81,13 @@ public class RestCsrfPreventionFilter ex
}
private static final Pattern NON_MODIFYING_METHODS_PATTERN =
Pattern.compile("GET|HEAD|OPTIONS");
- private static final Predicate<String> nonModifyingMethods = m -> m !=
null &&
+ private static final Predicate<String> nonModifyingMethods = m ->
Objects.nonNull(m) &&
NON_MODIFYING_METHODS_PATTERN.matcher(m).matches();
+ private Set<String> pathsAcceptingParams = new HashSet<>();
+
+ private String pathsDelimiter = ",";
+
@Override
public void doFilter(ServletRequest request, ServletResponse response,
FilterChain chain)
throws IOException, ServletException {
@@ -113,9 +117,12 @@ public class RestCsrfPreventionFilter ex
}
private static interface RestCsrfPreventionStrategy {
- static final NonceSupplier<HttpServletRequest> nonceFromRequest = (r,
k) -> r.getHeader(k);
- static final NonceSupplier<HttpSession> nonceFromSession = (s, k) ->
Objects.isNull(s) ? null
- : (String) s.getAttribute(k);
+ static final NonceSupplier<HttpServletRequest, String>
nonceFromRequestHeader = (r, k) -> r
+ .getHeader(k);
+ static final NonceSupplier<HttpServletRequest, String[]>
nonceFromRequestParams = (r, k) -> r
+ .getParameterValues(k);
+ static final NonceSupplier<HttpSession, String> nonceFromSession = (s,
k) -> Objects
+ .isNull(s) ? null : (String) s.getAttribute(k);
static final NonceConsumer<HttpServletResponse> nonceToResponse = (r,
k, v) -> r.setHeader(
k, v);
@@ -130,7 +137,7 @@ public class RestCsrfPreventionFilter ex
public boolean apply(HttpServletRequest request, HttpServletResponse
response)
throws IOException {
if (isValidStateChangingRequest(
- nonceFromRequest.getNonce(request,
Constants.CSRF_REST_NONCE_HEADER_NAME),
+ extractNonceFromRequest(request),
nonceFromSession.getNonce(request.getSession(false),
Constants.CSRF_REST_NONCE_SESSION_ATTR_NAME))) {
return true;
}
@@ -146,6 +153,32 @@ public class RestCsrfPreventionFilter ex
return Objects.nonNull(reqNonce) && Objects.nonNull(sessionNonce)
&& Objects.equals(reqNonce, sessionNonce);
}
+
+ private String extractNonceFromRequest(HttpServletRequest request) {
+ String nonceFromRequest = nonceFromRequestHeader.getNonce(request,
+ Constants.CSRF_REST_NONCE_HEADER_NAME);
+ if ((Objects.isNull(nonceFromRequest) || Objects.equals("",
nonceFromRequest))
+ && !getPathsAcceptingParams().isEmpty()
+ &&
getPathsAcceptingParams().contains(getRequestedPath(request))) {
+ nonceFromRequest = extractNonceFromRequestParams(request);
+ }
+ return nonceFromRequest;
+ }
+
+ private String extractNonceFromRequestParams(HttpServletRequest
request) {
+ String[] params = nonceFromRequestParams.getNonce(request,
+ Constants.CSRF_REST_NONCE_HEADER_NAME);
+ if (Objects.nonNull(params) && params.length > 0) {
+ String nonce = params[0];
+ for (String param : params) {
+ if (!Objects.equals(param, nonce)) {
+ return null;
+ }
+ }
+ return nonce;
+ }
+ return null;
+ }
}
private class FetchRequest implements RestCsrfPreventionStrategy {
@@ -155,7 +188,7 @@ public class RestCsrfPreventionFilter ex
@Override
public boolean apply(HttpServletRequest request, HttpServletResponse
response) {
if (fetchRequest.test(
- nonceFromRequest.getNonce(request,
Constants.CSRF_REST_NONCE_HEADER_NAME))) {
+ nonceFromRequestHeader.getNonce(request,
Constants.CSRF_REST_NONCE_HEADER_NAME))) {
String nonceFromSessionStr =
nonceFromSession.getNonce(request.getSession(false),
Constants.CSRF_REST_NONCE_SESSION_ATTR_NAME);
if (nonceFromSessionStr == null) {
@@ -172,12 +205,36 @@ public class RestCsrfPreventionFilter ex
}
@FunctionalInterface
- private static interface NonceSupplier<T> {
- String getNonce(T supplier, String key);
+ private static interface NonceSupplier<T, R> {
+ R getNonce(T supplier, String key);
}
@FunctionalInterface
private static interface NonceConsumer<T> {
void setNonce(T consumer, String key, String value);
}
+
+ /**
+ * Paths accepting request parameters with nonce information are URLs that
+ * can supply nonces via request parameter 'X-CSRF-Token'. For use cases
+ * when a nonce information cannot be provided via header, one can provide
+ * it via request parameters. If there is a X-CSRF-Token header, it will be
+ * taken with preference over any parameter with the same name in the
+ * request. Request parameters cannot be used to fetch new nonce, only
+ * header.
+ *
+ * @param pathsList
+ * Comma separated list of URLs to be configured as paths
+ * accepting request parameters with nonce information.
+ */
+ public void setPathsAcceptingParams(String pathsList) {
+ if (Objects.nonNull(pathsList)) {
+ Arrays.asList(pathsList.split(pathsDelimiter)).forEach(
+ e -> pathsAcceptingParams.add(e.trim()));
+ }
+ }
+
+ public Set<String> getPathsAcceptingParams() {
+ return pathsAcceptingParams;
+ }
}
Modified:
tomcat/trunk/test/org/apache/catalina/filters/TestRestCsrfPreventionFilter.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/filters/TestRestCsrfPreventionFilter.java?rev=1709295&r1=1709294&r2=1709295&view=diff
==============================================================================
---
tomcat/trunk/test/org/apache/catalina/filters/TestRestCsrfPreventionFilter.java
(original)
+++
tomcat/trunk/test/org/apache/catalina/filters/TestRestCsrfPreventionFilter.java
Sun Oct 18 18:05:06 2015
@@ -25,6 +25,7 @@ import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.HttpSession;
+import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import org.junit.Before;
@@ -42,6 +43,12 @@ public class TestRestCsrfPreventionFilte
private static final String POST_METHOD = "POST";
+ public static final String ACCEPTED_PATH1 = "/accepted/index1.jsp";
+
+ public static final String ACCEPTED_PATH2 = "/accepted/index2.jsp";
+
+ public static final String ACCEPTED_PATHS = ACCEPTED_PATH1 + "," +
ACCEPTED_PATH2;
+
private RestCsrfPreventionFilter filter;
private TesterRequest request;
@@ -83,45 +90,25 @@ public class TestRestCsrfPreventionFilte
@Test
public void testPostRequestSessionNoNonce1() throws Exception {
setRequestExpectations(POST_METHOD, session, null);
-
EasyMock.expect(session.getAttribute(Constants.CSRF_REST_NONCE_SESSION_ATTR_NAME))
- .andReturn(null);
- EasyMock.replay(session);
- filter.doFilter(request, response, filterChain);
- verifyDenyResponse(HttpServletResponse.SC_FORBIDDEN);
- EasyMock.verify(session);
+ testPostRequestHeaderScenarios(null, true);
}
@Test
public void testPostRequestSessionNoNonce2() throws Exception {
setRequestExpectations(POST_METHOD, session, null);
-
EasyMock.expect(session.getAttribute(Constants.CSRF_REST_NONCE_SESSION_ATTR_NAME))
- .andReturn(NONCE);
- EasyMock.replay(session);
- filter.doFilter(request, response, filterChain);
- verifyDenyResponse(HttpServletResponse.SC_FORBIDDEN);
- EasyMock.verify(session);
+ testPostRequestHeaderScenarios(NONCE, true);
}
@Test
public void testPostRequestSessionInvalidNonce() throws Exception {
setRequestExpectations(POST_METHOD, session, INVALID_NONCE);
-
EasyMock.expect(session.getAttribute(Constants.CSRF_REST_NONCE_SESSION_ATTR_NAME))
- .andReturn(NONCE);
- EasyMock.replay(session);
- filter.doFilter(request, response, filterChain);
- verifyDenyResponse(HttpServletResponse.SC_FORBIDDEN);
- EasyMock.verify(session);
+ testPostRequestHeaderScenarios(NONCE, true);
}
@Test
public void testPostRequestSessionValidNonce() throws Exception {
setRequestExpectations(POST_METHOD, session, NONCE);
-
EasyMock.expect(session.getAttribute(Constants.CSRF_REST_NONCE_SESSION_ATTR_NAME))
- .andReturn(NONCE);
- EasyMock.replay(session);
- filter.doFilter(request, response, filterChain);
- verifyContinueChain();
- EasyMock.verify(session);
+ testPostRequestHeaderScenarios(NONCE, false);
}
@Test
@@ -140,12 +127,7 @@ public class TestRestCsrfPreventionFilte
@Test
public void testPostFetchRequestSessionNoNonce() throws Exception {
setRequestExpectations(POST_METHOD, session,
Constants.CSRF_REST_NONCE_HEADER_FETCH_VALUE);
-
EasyMock.expect(session.getAttribute(Constants.CSRF_REST_NONCE_SESSION_ATTR_NAME))
- .andReturn(null);
- EasyMock.replay(session);
- filter.doFilter(request, response, filterChain);
- verifyDenyResponse(HttpServletResponse.SC_FORBIDDEN);
- EasyMock.verify(session);
+ testPostRequestHeaderScenarios(null, true);
}
@Test
@@ -162,12 +144,7 @@ public class TestRestCsrfPreventionFilte
@Test
public void testPostFetchRequestSessionNonce() throws Exception {
setRequestExpectations(POST_METHOD, session,
Constants.CSRF_REST_NONCE_HEADER_FETCH_VALUE);
-
EasyMock.expect(session.getAttribute(Constants.CSRF_REST_NONCE_SESSION_ATTR_NAME))
- .andReturn(NONCE);
- EasyMock.replay(session);
- filter.doFilter(request, response, filterChain);
- verifyDenyResponse(HttpServletResponse.SC_FORBIDDEN);
- EasyMock.verify(session);
+ testPostRequestHeaderScenarios(NONCE, true);
}
@Test
@@ -178,10 +155,120 @@ public class TestRestCsrfPreventionFilte
verifyDenyResponse(HttpServletResponse.SC_BAD_REQUEST);
}
+ @Test
+ public void testPostRequestValidNonceAsParameterValidPath1() throws
Exception {
+ setRequestExpectations(POST_METHOD, session, null, new String[] {
NONCE }, ACCEPTED_PATH1);
+ testPostRequestParamsScenarios(NONCE, false, true);
+ }
+
+ @Test
+ public void testPostRequestValidNonceAsParameterValidPath2() throws
Exception {
+ setRequestExpectations(POST_METHOD, session, null, new String[] {
NONCE }, ACCEPTED_PATH2);
+ testPostRequestParamsScenarios(NONCE, false, true);
+ }
+
+ @Test
+ public void testPostRequestInvalidNonceAsParameterValidPath() throws
Exception {
+ setRequestExpectations(POST_METHOD, session, null, new String[] {
INVALID_NONCE },
+ ACCEPTED_PATH1);
+ testPostRequestParamsScenarios(NONCE, true, true);
+ }
+
+ @Test
+ public void testPostRequestValidNonceAsParameterInvalidPath() throws
Exception {
+ setRequestExpectations(POST_METHOD, session, null, new String[] {
NONCE }, ACCEPTED_PATH1
+ + "blah");
+ testPostRequestParamsScenarios(NONCE, true, true);
+ }
+
+ @Test
+ public void testPostRequestValidNonceAsParameterNoPath() throws Exception {
+ setRequestExpectations(POST_METHOD, session, null, new String[] {
NONCE }, ACCEPTED_PATH1);
+ testPostRequestParamsScenarios(NONCE, true, false);
+ }
+
+ @Test
+ public void testPostRequestValidNonceAsParameterNoNonceInSession() throws
Exception {
+ setRequestExpectations(POST_METHOD, session, null, new String[] {
NONCE }, ACCEPTED_PATH1);
+ testPostRequestParamsScenarios(null, true, true);
+ }
+
+ @Test
+ public void testPostRequestValidNonceAsParameterInvalidNonceAsHeader()
throws Exception {
+ setRequestExpectations(POST_METHOD, session, INVALID_NONCE, new
String[] { NONCE },
+ ACCEPTED_PATH1);
+ testPostRequestParamsScenarios(NONCE, true, true);
+ }
+
+ @Test
+ public void testPostRequestNoNonceAsParameterAndHeaderValidPath() throws
Exception {
+ setRequestExpectations(POST_METHOD, session, null, null,
ACCEPTED_PATH1);
+ testPostRequestParamsScenarios(NONCE, true, true);
+ }
+
+ @Test
+ public void testPostRequestMultipleValidNoncesAsParameterValidPath()
throws Exception {
+ setRequestExpectations(POST_METHOD, session, null, new String[] {
NONCE, NONCE },
+ ACCEPTED_PATH1);
+ testPostRequestParamsScenarios(NONCE, false, true);
+ }
+
+ @Test
+ public void testPostRequestMultipleNoncesAsParameterValidPath() throws
Exception {
+ setRequestExpectations(POST_METHOD, session, null, new String[] {
NONCE, INVALID_NONCE },
+ ACCEPTED_PATH1);
+ testPostRequestParamsScenarios(NONCE, true, true);
+ }
+
+ @Test
+ public void testPostRequestMultipleInvalidNoncesAsParameterValidPath()
throws Exception {
+ setRequestExpectations(POST_METHOD, session, null, new String[] {
INVALID_NONCE,
+ INVALID_NONCE }, ACCEPTED_PATH1);
+ testPostRequestParamsScenarios(NONCE, true, true);
+ }
+
+ @Test
+ public void testGETRequestFetchNonceAsParameter() throws Exception {
+ setRequestExpectations(GET_METHOD, null, null,
+ new String[] { Constants.CSRF_REST_NONCE_HEADER_FETCH_VALUE },
ACCEPTED_PATH1);
+ filter.setPathsAcceptingParams(ACCEPTED_PATHS);
+ filter.doFilter(request, response, filterChain);
+ verifyContinueChainNonceNotAvailable();
+ }
+
+ private void testPostRequestHeaderScenarios(String sessionAttr, boolean
denyResponse)
+ throws Exception {
+ testPostRequestParamsScenarios(sessionAttr, denyResponse, false);
+ }
+
+ private void testPostRequestParamsScenarios(String sessionAttr, boolean
denyResponse,
+ boolean configurePaths) throws Exception {
+
EasyMock.expect(session.getAttribute(Constants.CSRF_REST_NONCE_SESSION_ATTR_NAME))
+ .andReturn(sessionAttr);
+ EasyMock.replay(session);
+ if (configurePaths) {
+ filter.setPathsAcceptingParams(ACCEPTED_PATHS);
+ }
+ filter.doFilter(request, response, filterChain);
+ if (denyResponse) {
+ verifyDenyResponse(HttpServletResponse.SC_FORBIDDEN);
+ } else {
+ verifyContinueChain();
+ }
+ EasyMock.verify(session);
+ }
+
private void setRequestExpectations(String method, HttpSession session,
String headerValue) {
+ setRequestExpectations(method, session, headerValue, null, null);
+ }
+
+ private void setRequestExpectations(String method, HttpSession session,
String headerValue,
+ String[] paramValues, String servletPath) {
request.setMethod(method);
request.setSession(session);
request.setHeader(Constants.CSRF_REST_NONCE_HEADER_NAME, headerValue);
+ request.setParameterValues(paramValues);
+ request.setServletPath(servletPath);
}
private void verifyContinueChain() {
@@ -193,6 +280,11 @@ public class TestRestCsrfPreventionFilte
verifyContinueChain();
}
+ private void verifyContinueChainNonceNotAvailable() {
+ assertNull(response.getHeader(Constants.CSRF_REST_NONCE_HEADER_NAME));
+ verifyContinueChain();
+ }
+
private void verifyDenyResponse(int statusCode) {
assertTrue(Constants.CSRF_REST_NONCE_HEADER_REQUIRED_VALUE.equals(response
.getHeader(Constants.CSRF_REST_NONCE_HEADER_NAME)));
@@ -216,6 +308,8 @@ public class TestRestCsrfPreventionFilte
private static class TesterRequest extends TesterHttpServletRequest {
private HttpSession session;
+ private String[] paramValues;
+ private String servletPath;
void setSession(HttpSession session) {
this.session = session;
@@ -225,6 +319,29 @@ public class TestRestCsrfPreventionFilte
public HttpSession getSession(boolean create) {
return session;
}
+
+ void setParameterValues(String[] paramValues) {
+ this.paramValues = paramValues;
+ }
+
+ @Override
+ public String[] getParameterValues(String name) {
+ return paramValues;
+ }
+
+ void setServletPath(String servletPath) {
+ this.servletPath = servletPath;
+ }
+
+ @Override
+ public String getServletPath() {
+ return servletPath;
+ }
+
+ @Override
+ public String getPathInfo() {
+ return "";
+ }
}
private static class TesterResponse extends TesterHttpServletResponse {
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
