сб, 16 нояб. 2019 г. в 18:55, <[email protected]>:
>
> 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 <[email protected]>
> 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: [email protected]
For additional commands, e-mail: [email protected]
