jsr344-experts@javaserverfaces-spec-public.java.net

[jsr344-experts] Re: [869-CSRF] Proposal

From: Blake Sullivan <blake.sullivan_at_oracle.com>
Date: Fri, 22 Jul 2011 17:50:21 -0700

Ed,

It may have been at the 24th hour, but I believe that Alexander is
correct. If we are only applying CSRF protection to form POSTs, why
isn't the view state token completely sufficient? If we are attempting
to apply CSRF protection to GETs, there are other issues that are going
to crop up.

-- Blake Sullivan

On 7/21/11 3:02 PM, Ed Burns wrote:
> http://javaserverfaces-spec-public.java.net/nonav/proposals/JAVASERVERFACES_SPEC_PUBLIC-869-CSRF.txt
>
> ACTION: Please review and respond by 23:59 EDT Tuesday 26 July 2011.
>
> SECTION: Issue Recap
>
>>>>>> On Fri, 29 Apr 2011 10:44:27 -0400, Roger Kitain<roger.kitain_at_oracle.com> said:
> RK> The original CSRF proposal was based off of Kito's proposal. It used two
> RK> algorithms:
> RK> 1. form approach - generated token set as form hidden field and sent
> RK> in request
> RK> 2. url approach - generated token appended in url and sent
>
> RK> 1. Why didn't it make it in to 2.1?
>
> RK> The EG was late in responding with comments. At the 24th hour, Alexandr
> RK> Smirnov
> RK> raised some concerns outlined here:
> RK> http://lists.jboss.org/pipermail/jsr-314-open-mirror/2010-October/000410.html
> RK> In a nutshell, he raised two concerns:
> RK> a. additional "token" parameter increases size of generated html
> RK> code - we already
> RK> have the viewid parameter that is passed.
>
> Alexander suggested:
>
> AS> JSF forms always contain viewId parameter, so adding encoded token
> AS> to that field will have minimal side effects. That token should be
> AS> checked by StateManager before restoring view.
>
> RK> b. For the "URL" approach with token, it protects the whole
> RK> application - so, for example,
> RK> user has to have a secure token - there should be per page
> RK> security configuration.
>
> RK> Blake and Andy also had concerns (although Andy was initially ok with it
> RK> per this email
> RK> thread:
>
> On 10/22/10 10:32 AM, Andy Schwartz wrote:
>
> AS> Thanks for the comments Roger.
>
> On 10/22/10 9:37 AM, Roger Kitain wrote:
>
> RK> Custom forms would need to follow the new rendering requirements for
> RK> CSRF (described in
> RK> renderkit docs).
>
> AS> I reviewed the relevant render kit docs:
>
> RK> Render a hidden field named javax.faces.Token using the
> RK> ResponseStateManager.VIEW_TOKEN_PARAM constant if the configuration
> RK> option javax.faces.CSRF_ALGORITHM is set to either of the key words
> RK> form or all. The name must be namespaced with the enclosing form's
> RK> client identifier. Render the value of this hidden field, known as
> RK> the "token value", as a cryptographically produced random generated
> RK> value (known as the "secret key") retrieved from the session. If the
> RK> "secret key" does not already exist in the session, create the random
> RK> value and store it in the session. Implementations may choose to
> RK> produce a more complex token value by combining the random "secret
> RK> key" with other values. The default value for the
> RK> javax.faces.CSRF_ALGORITHM configuration option is none
>
> AS> This seems okay, however: doesn't the FormRenderer and the
> AS> RestoreViewPhase implementation need to be in sync on what the
> AS> token/secret key is? Is there a way to do this without replacing the
> AS> Lifecycle?
>
> RK> In 2.2, we could further simplify this by providing API "hooks".
> RK> By default, CSRF is not enabled, so existing apps continue to work.
>
> AS> Cool. I still want to make sure that when this is enabled we have
> AS> some way that custom form components can participate (even if this
> AS> means some work for the form component/renderer implementation).
>
> AS> Oh, btw, one thing that I was still unclear on... Are we also solving
> AS> the double-submit problem - ie. do we re-generate the token on each
> AS> submit?
>
> It is very seductive to couple the solutions to these two related
> problems into one solution, but I think we need to just solve the CSRF
> one right now. The need for a status tracking mechanism in the case of
> the double submit problem (as mentioned in this article I found on the
> interwebs<http://www.netlobo.com/form_double_submit.html>) separates
> the two problems very clearly. With a CSRF failure, it is sufficient to
> always send the user to the error-page, but with double submit, it is
> not.
>
> RK> 2. What do we need to do to make it into 2.2?
>
> RK> Possibly look at more fine grained control - maybe at the view level.
>
> SECTION: Rewrite of Roger's 20100930 Proposal
>
> I'd like to reduce the config logic to a simple on/off status. None of
> this "all", "none", "form", or "both" business. If the CSRF protection
> feature is turned on at all, we use "both". If it is not turned on, we
> use "none". I justify this simplification by pointing to our strong
> commitment to supporting portlets.
>
> To that end, consider Neil Griffin's comment from 20100921:
>
> NG> #1 is probably not compatible with portlets. Portals are in full
> NG> control of creation of URLs in general, and it is not possible to
> NG> simply append "&javax.faces.Token=XYZ" to a portal's ActionURL and
> NG> expect it to work.
>
> Because we are always doing both, is it reasonable to specify that a
> portlet bridge implementation must take whatever action is necessary to
> remove or massage the token so that things work as expected?
>
> Also, I propose we use configuration markup in<faces-config-extension>
> instead of introducing a new config parameter.
>
> This proposal does the following:
>
> - Token is generated on the server consisting (minimally) of a randomly
> generated "secret key" (stored in session). The token can be generated
> upon session creation, or, at the time the token hidden field and/or
> token Url parameter is produced.
>
> - Define a<cross-site-request-forgery-protection> element, to be
> declared in the<faces-config-extension> element. If this element
> exists, and contains one or more<web-resource-collection> elements,
> the CSRF protection is said to be enabled for those views that are
> encompassed by the<url-pattern> elements in the
> <web-resource-collection> elements. Otherwise, the CSRF feature is
> not enabled for the application.
>
> - If the CSRF feature is enabled, it must only be applied to those views
> named in the<web-resource-collection> elements. For those views,
> token is produced by both of the following values:
>
> a. "form" : token is produced as the value of the hidden field with the name
> <form client id>:javax.faces.Token where<form client id> is the
> enclosing form's client identifier (produced with getClientId) - i.e.
> the hidden field is namespaced using the form's client identifier.
> b. "url" : token is produced and appended as a parameter to the form's
> action Url with the same name as in [a].
>
> - After render time, a subsequent action will send the token to the server.
> - Restore View Phase processing compares the incoming token request parameter
> value with the token value generated from the secret key in the session.
>
> Spec Document Modifications:
>
> Section 7.5.1:
>
> getActionURL:
>
> If the current view is one of the views to which CSRF protection should
> be applied, the returned URL must contain the parameter with a name
> consisting of the form's fully namespaced client identifier, the
> NamingContainer.SEPARATOR_CHAR and the constant defined by
> ResponseStateManager.VIEW_TOKEN_PARAM. The value of this parameter,
> known as the "token value" must be a cryptographically produced random
> generated value (known as the "secret key") retrieved from the session.
> If the "secret key" does not already exist in the session, create the
> random value and store it in the session. Implementations may choose to
> produce a more complex token value by combining the random "secret key"
> with other values.
>
> Section 2.2.1
>
> If the current view is one of the views to which the CSRF protection
> should be applied, verify the "javax.faces.Token" request parameter
> value is the same as the token value generated from the "secret key"
> stored in the session. If the values do not match, throw a meaningful
> exception. The exception message must not include the token value.
> This enables the error-page mechanism to be employed.
>
> Standard RenderKit Docs
>
> - Form Rendering
>
> If the current view is one of the views to which the CSRF protection
> should be applied, render a hidden field named javax.faces.Token using
> the ResponseStateManager.VIEW_TOKEN_PARAM constant. The name must be
> namespaced with the enclosing form's client identifier. Render the
> value of this hidden field, known as the "token value", as a
> cryptographically produced random generated value (known as the "secret
> key") retrieved from the session. If the "secret key" does not already
> exist in the session, create the random value and store it in the
> session. Implementations may choose to produce a more complex token
> value by combining the random "secret key" with other values.
>
>