This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push:
new 2da3fa89e4 Fix BZ 65853 - refactor CsrfPreventionFilter to make
extension easier
2da3fa89e4 is described below
commit 2da3fa89e47694260625df1548ab834823fee080
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Mon May 9 18:34:52 2022 +0100
Fix BZ 65853 - refactor CsrfPreventionFilter to make extension easier
---
.../catalina/filters/CsrfPreventionFilter.java | 93 +++++++++++++++++-----
.../catalina/filters/CsrfPreventionFilterBase.java | 19 +++++
.../catalina/filters/RestCsrfPreventionFilter.java | 2 +-
webapps/docs/changelog.xml | 5 ++
4 files changed, 97 insertions(+), 22 deletions(-)
diff --git a/java/org/apache/catalina/filters/CsrfPreventionFilter.java
b/java/org/apache/catalina/filters/CsrfPreventionFilter.java
index fe7a27f92d..6e45f3d111 100644
--- a/java/org/apache/catalina/filters/CsrfPreventionFilter.java
+++ b/java/org/apache/catalina/filters/CsrfPreventionFilter.java
@@ -118,25 +118,11 @@ public class CsrfPreventionFilter extends
CsrfPreventionFilterBase {
HttpServletRequest req = (HttpServletRequest) request;
HttpServletResponse res = (HttpServletResponse) response;
- boolean skipNonceCheck = false;
-
- if (Constants.METHOD_GET.equals(req.getMethod())
- && entryPoints.contains(getRequestedPath(req))) {
- if(log.isTraceEnabled()) {
- log.trace("Skipping CSRF nonce-check for GET request to
entry point " + getRequestedPath(req));
- }
-
- skipNonceCheck = true;
- }
-
HttpSession session = req.getSession(false);
- @SuppressWarnings("unchecked")
- LruCache<String> nonceCache = (session == null) ? null
- : (LruCache<String>) session.getAttribute(
- Constants.CSRF_NONCE_SESSION_ATTR_NAME);
+ NonceCache<String> nonceCache = (session == null) ? null :
getNonceCache(req, session);
- if (!skipNonceCheck) {
+ if (!skipNonceCheck(req)) {
String previousNonce =
req.getParameter(nonceRequestParameterName);
@@ -182,7 +168,6 @@ public class CsrfPreventionFilter extends
CsrfPreventionFilterBase {
log.debug("Creating new CSRF nonce cache with size=" +
nonceCacheSize + " for session " + (null == session ? "(will create)" :
session.getId()));
}
- nonceCache = new LruCache<>(nonceCacheSize);
if (session == null) {
if(log.isDebugEnabled()) {
log.debug("Creating new session to store CSRF nonce
cache");
@@ -190,11 +175,11 @@ public class CsrfPreventionFilter extends
CsrfPreventionFilterBase {
session = req.getSession(true);
}
- session.setAttribute(
- Constants.CSRF_NONCE_SESSION_ATTR_NAME, nonceCache);
+
+ nonceCache = createNonceCache(req, session);
}
- String newNonce = generateNonce();
+ String newNonce = generateNonce(req);
nonceCache.add(newNonce);
@@ -212,6 +197,64 @@ public class CsrfPreventionFilter extends
CsrfPreventionFilterBase {
}
+ protected boolean skipNonceCheck(HttpServletRequest request) {
+ if (!Constants.METHOD_GET.equals(request.getMethod())) {
+ return false;
+ }
+
+ String requestedPath = getRequestedPath(request);
+
+ if (!entryPoints.contains(requestedPath)) {
+ return false;
+ }
+
+ if (log.isTraceEnabled()) {
+ log.trace("Skipping CSRF nonce-check for GET request to entry
point " + requestedPath);
+ }
+
+ return true;
+ }
+
+
+ /**
+ * Create a new {@link NonceCache} and store in the {@link HttpSession}.
+ * This method is provided primarily for the benefit of sub-classes that
+ * wish to customise this behaviour.
+ *
+ * @param request The request that triggered the need to create the nonce
+ * cache. Unused by the default implementation.
+ * @param session The session associated with the request.
+ *
+ * @return A newly created {@link NonceCache}
+ */
+ protected NonceCache<String> createNonceCache(HttpServletRequest request,
HttpSession session) {
+
+ NonceCache<String> nonceCache = new LruCache<>(nonceCacheSize);
+
+ session.setAttribute(Constants.CSRF_NONCE_SESSION_ATTR_NAME,
nonceCache);
+
+ return nonceCache;
+ }
+
+
+ /**
+ * Obtain the {@link NonceCache} associated with the request and/or
session.
+ * This method is provided primarily for the benefit of sub-classes that
+ * wish to customise this behaviour.
+ *
+ * @param request The request that triggered the need to obtain the nonce
+ * cache. Unused by the default implementation.
+ * @param session The session associated with the request.
+ *
+ * @return A newly created {@link NonceCache}
+ */
+ protected NonceCache<String> getNonceCache(HttpServletRequest request,
HttpSession session) {
+ @SuppressWarnings("unchecked")
+ NonceCache<String> nonceCache =
+ (NonceCache<String>)
session.getAttribute(Constants.CSRF_NONCE_SESSION_ATTR_NAME);
+ return nonceCache;
+ }
+
protected static class CsrfResponseWrapper
extends HttpServletResponseWrapper {
@@ -285,7 +328,15 @@ public class CsrfPreventionFilter extends
CsrfPreventionFilterBase {
}
}
- protected static class LruCache<T> implements Serializable {
+
+ protected static interface NonceCache<T> extends Serializable {
+ void add(T nonce);
+
+ boolean contains(T nonce);
+ }
+
+
+ protected static class LruCache<T> implements NonceCache<T> {
private static final long serialVersionUID = 1L;
diff --git a/java/org/apache/catalina/filters/CsrfPreventionFilterBase.java
b/java/org/apache/catalina/filters/CsrfPreventionFilterBase.java
index 1f51fa6b58..ba35ec42a5 100644
--- a/java/org/apache/catalina/filters/CsrfPreventionFilterBase.java
+++ b/java/org/apache/catalina/filters/CsrfPreventionFilterBase.java
@@ -93,13 +93,32 @@ public abstract class CsrfPreventionFilterBase extends
FilterBase {
return true;
}
+
+ /**
+ * Generate a once time token (nonce) for authenticating subsequent
+ * requests. The nonce generation is a simplified version of
+ * ManagerBase.generateSessionId().
+ *
+ * @param request The request. Unused in this method but present for the
+ * the benefit of sub-classes.
+ *
+ * @return the generated nonce
+ */
+ protected String generateNonce(HttpServletRequest request) {
+ return generateNonce();
+ }
+
/**
* Generate a once time token (nonce) for authenticating subsequent
* requests. The nonce generation is a simplified version of
* ManagerBase.generateSessionId().
*
* @return the generated nonce
+ *
+ * @deprecated Use {@link #generateNonce(HttpServletRequest)} instead. This
+ * method will be removed in Apache Tomcat 10.1.x onwards.
*/
+ @Deprecated
protected String generateNonce() {
byte random[] = new byte[16];
diff --git a/java/org/apache/catalina/filters/RestCsrfPreventionFilter.java
b/java/org/apache/catalina/filters/RestCsrfPreventionFilter.java
index 66d3ea98f7..eb4afa3a70 100644
--- a/java/org/apache/catalina/filters/RestCsrfPreventionFilter.java
+++ b/java/org/apache/catalina/filters/RestCsrfPreventionFilter.java
@@ -221,7 +221,7 @@ public class RestCsrfPreventionFilter extends
CsrfPreventionFilterBase {
String nonceFromSessionStr =
extractNonceFromSession(request.getSession(false),
Constants.CSRF_REST_NONCE_SESSION_ATTR_NAME);
if (nonceFromSessionStr == null) {
- nonceFromSessionStr = generateNonce();
+ nonceFromSessionStr = generateNonce(request);
storeNonceToSession(Objects.requireNonNull(request.getSession(true)),
Constants.CSRF_REST_NONCE_SESSION_ATTR_NAME,
nonceFromSessionStr);
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 27873e6402..743cdc76bd 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -107,6 +107,11 @@
<section name="Tomcat 8.5.79 (schultz)" rtext="in development">
<subsection name="Catalina">
<changelog>
+ <scode>
+ <bug>65853</bug>: Refactor the <code>CsrfPreventionFilter</code> to
make
+ it easier for sub-classes to modify the nonce generation and storage.
+ Based on suggestions by Marvin Fröhlich. (markt)
+ </scode>
<fix>
<bug>65991</bug>: Avoid NPE with <code>SSLAuthenticator</code> when
<code>boundOnInit</code> is used on a connector, during the check
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
