сб, 16 нояб. 2019 г. в 18:55, <schu...@apache.org>: > > This is an automated email from the ASF dual-hosted git repository. > > schultz pushed a commit to branch master > in repository https://gitbox.apache.org/repos/asf/tomcat.git > > > The following commit(s) were added to refs/heads/master by this push: > new ff687b5 Improve CSRF prevention filter by exposing the request's > current nonce to the request. > ff687b5 is described below > > commit ff687b5524b2b78c449fe32e8c520da86068cd1a > Author: Christopher Schultz <ch...@christopherschultz.net> > AuthorDate: Sat Nov 16 10:54:36 2019 -0500 > > Improve CSRF prevention filter by exposing the request's current nonce to > the request. > --- > java/org/apache/catalina/filters/Constants.java | 33 > ++++++++++++++++++++++ > .../catalina/filters/CsrfPreventionFilter.java | 5 ++++ > .../catalina/filters/CsrfPreventionFilterBase.java | 10 +++++++ > 3 files changed, 48 insertions(+) > > diff --git a/java/org/apache/catalina/filters/Constants.java > b/java/org/apache/catalina/filters/Constants.java > index 4fe41cd..87dd6c4 100644 > --- a/java/org/apache/catalina/filters/Constants.java > +++ b/java/org/apache/catalina/filters/Constants.java > @@ -25,12 +25,34 @@ package org.apache.catalina.filters; > */ > public final class Constants { > > + /** > + * The session attribute key under which the CSRF nonce > + * cache will be stored. > + */ > public static final String CSRF_NONCE_SESSION_ATTR_NAME = > "org.apache.catalina.filters.CSRF_NONCE"; > > + /** > + * The request attribute key under which the current > + * requests's CSRF nonce can be found. > + */ > + public static final String CSRF_NONCE_REQUEST_ATTR_NAME = > + "org.apache.catalina.filters.CSRF_REQUEST_NONCE"; > + > + /** > + * The name of the request parameter which carries CSRF nonces > + * from the client to the server for validation. > + */ > public static final String CSRF_NONCE_REQUEST_PARAM = > "org.apache.catalina.filters.CSRF_NONCE"; > > + /** > + * The servlet context attribute key under which the > + * CSRF request parameter name can be found. > + */ > + public static final String CSRF_NONCE_REQUEST_PARAM_NAME_KEY = > + "org.apache.catalina.filters.CSRF_NONCE_PARAM_NAME"; > + > public static final String METHOD_GET = "GET"; > > public static final String CSRF_REST_NONCE_HEADER_NAME = "X-CSRF-Token"; > @@ -39,6 +61,17 @@ public final class Constants { > > public static final String CSRF_REST_NONCE_HEADER_REQUIRED_VALUE = > "Required"; > > + /** > + * The session attribute key under which the CSRF REST nonce > + * cache will be stored. > + */ > public static final String CSRF_REST_NONCE_SESSION_ATTR_NAME = > "org.apache.catalina.filters.CSRF_REST_NONCE"; > + > + /** > + * The servlet context attribute key under which the > + * CSRF REST header name can be found. > + */ > + public static final String CSRF_REST_NONCE_HEDAER_NAME_KEY =
1) There is a typo in the name of the constant above, s/HEDAER/HEADER/. 2) This commit is not mentioned in changelog. > + "org.apache.catalina.filters.CSRF_REST_NONCE_HEADER_NAME"; > } > diff --git a/java/org/apache/catalina/filters/CsrfPreventionFilter.java > b/java/org/apache/catalina/filters/CsrfPreventionFilter.java > index fff5c2f..d94cdec 100644 > --- a/java/org/apache/catalina/filters/CsrfPreventionFilter.java > +++ b/java/org/apache/catalina/filters/CsrfPreventionFilter.java > @@ -128,6 +128,11 @@ public class CsrfPreventionFilter extends > CsrfPreventionFilterBase { > > nonceCache.add(newNonce); > > + // Take this request's nonce and put it into the request > + // attributes so pages can make direct use of it, rather than > + // requiring the use of response.encodeURL. > + request.setAttribute(Constants.CSRF_NONCE_REQUEST_ATTR_NAME, > newNonce); > + > wResponse = new CsrfResponseWrapper(res, newNonce); > } else { > wResponse = response; > diff --git a/java/org/apache/catalina/filters/CsrfPreventionFilterBase.java > b/java/org/apache/catalina/filters/CsrfPreventionFilterBase.java > index c0083f0..8d401af 100644 > --- a/java/org/apache/catalina/filters/CsrfPreventionFilterBase.java > +++ b/java/org/apache/catalina/filters/CsrfPreventionFilterBase.java > @@ -78,6 +78,16 @@ public abstract class CsrfPreventionFilterBase extends > FilterBase { > // Set the parameters > super.init(filterConfig); > > + // Put the expected request parameter name into the application scope > + filterConfig.getServletContext().setAttribute( > + Constants.CSRF_NONCE_REQUEST_PARAM_NAME_KEY, > + Constants.CSRF_NONCE_REQUEST_PARAM); > + > + // Put the expected request header name into the application scope > + filterConfig.getServletContext().setAttribute( > + Constants.CSRF_REST_NONCE_HEDAER_NAME_KEY, > + Constants.CSRF_REST_NONCE_HEADER_NAME); The typo that I mentioned above is visible here. 3) Why this code is here, in a *Base class? The CsrfPreventionFilterBase class does not use those constants. They are used by specific subclasses only. 4) I think that one possible improvement to CsrfPreventionFilter may be to make the parameter name configurable. (I think that is implemented by adding a field and public getter/setter to CsrfPreventionFilter class. If so, the base class will have no access to the new field.) 5) I wonder whether it is a good idea to expose those constants as ServletContext attributes, vs attributes of a specific request. We already have nonce cache stored as a session attribute, thus maybe it is not bad. >From other view, Servlet 4.0 spec (chapter 6.2.1 Filter Lifecycle) allows to initialize filters lazily, before processing a request. Currently Tomcat does not do so. It initializes all filters when a web application starts. (Code: - StandardContext.filterStart() -> ApplicationFilterConfig(...) constructor -> ApplicationFilterConfig.initFilter() ) If filters were initialized lazily, presence of those attributes in ServletContext would depend on whether the filter has started yet. > try { > Class<?> clazz = Class.forName(randomClass); > randomSource = (Random) clazz.getConstructor().newInstance(); Best regards, Konstantin Kolinko --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org