[jsr-314-open-mirror] [jsr-314-open] [Spec-869-Specify CSRF Solution] PROPOSAL
Roger Kitain
roger.kitain at oracle.com
Fri Oct 22 09:26:16 EDT 2010
Hey Andy, all -
On 10/21/10 9:53 PM, Andy Schwartz wrote:
> Gang -
>
> Apologies for being so amazingly late in reviewing this - just haven't
> had a chance to look until now.
Understood.
>
> Regarding:
>
>> For token encoded as url parameter this proposal protects whole
>> application, so no one can either got logged in to protected site
>> because of circular dependencies: to open login page, visitor has to
>> have secure token, which one he can get only from JSF login page...
>
> I think that this assumes that:
>
> 1. We are checking GET requests for the token.
> 2. We need a token for the initial request into the application.
That is a correct assumption.
>
> Looking at Mojarra's current implementation, these assumptions are not
> true. From RestoreViewPhase.execute():
>
>> if (isPostBack) {
>> if (isCSRFOptionEnabled(facesContext)) {
>> String convertedViewId =
>> viewHandler.deriveViewId(facesContext, viewId);
>> if (!TokenHelper.verifyToken(facesContext,
>> convertedViewId)) {
>> throw new FacesException("Token verification
>> failed.");
>> }
>
> Since we only verify the token for POST requests, if we arrive on the
> login page via a GET, we should be fine.
>
> However, if we only verify POST requests, I am not sure that I
> understand the value of CSRF_ALGORITHM=URL. Any POSTs from an h:form
> will already contain the token from the hidden form field. Including
> a query parameter in the form's action seems redundant/unnecessary.
>
> Speciyfing that ViewHandler.getActionURL() must include the token
> query parameter has other undesirable side effects. Both
> getRedirectURL() and getBookmarkableURL() call getActionURL(). As
> such, these URLs will end up with the query parameter even though we
> won't use this query parameter when handling the GET. We end up with
> noticeably uglier URLs with no gain in security.
>
> Earlier Kito mentioned the following justification for supporting the
> CSRF=URL approach:
>
>> I think it's actually safer to allow both:
>> http://www.owasp.org/index.php/Cross-Site_Request_Forgery_%28CSRF%29_Prevention_Cheat_Sheet#Disclosure_of_Token_in_URL.
>> According to this, appending the token works better when you can't
>> guarantee that a server-side operation is occurring only through
>> POST. Especially with the new capabilities of JSF 2 for parsing view
>> parameters, I don't think we can make this guarantee.
>
> I think I see Kito's point. However, if we are only verifying the
> token on POST requests, the CSRF=URL approach doesn't address the view
> parameter use case. Again, we don't gain anything here.
>
> I get the feeling from both Alexander and Kito's comments that there
> was an assumption that we would be performing token verification for
> both POST and GET requests. I suppose we can consider doing this.
> However, as Alexander hinted, this gets tricky fast. If we only
> expose an option for globally securing all GETs, this means that the
> application can no longer support bookmarkable URLs (since every URL
> must contain a token which is only valid for a very limited
> lifetime). Any attempt to access the application without a valid
> token (such as a bookmark to the login page) will fail. As such, I
> agree with Alexander's comment:
>
>> There should be per-page security configuration.
Based on the circular dependency "chicken and egg" problem for GET
requests, I agree.
>
> At least when it comes to the URL/GET-based solution, I don't see how
> enabling this globally is a useful solution.
>
> Based on the current implementation I fail to see the benefits of the
> CSRF=URL approach. I see at least one fairly significant drawback:
> unnecessarily uglier redirect/bookmarkable URLs. Unless I am missing
> something (which might well be the case), I am -1 on including
> CSRF=URL in the specification.
Based on this feedback I'm ok with rolling back the URL specification
and implementation for CSRF.
>
> Andy
>
>
>
> On 10/1/10 1:22 PM, Alexander Smirnov wrote:
>> I disagree with suggested proposal. There are two pitfalls:
>> a) additional parameter just increases size of generated html code and
>> number of request parameters. JSF forms always contain viewId parameter,
>> so adding encoded token to that field will have minimal side effects.
>> That token should be checked by StateManager before restoring view.
>> b) URL parameter does not make sense for forms, but more important for
>> 'GET' access. To make this parameter compatible with portal environment,
>> it can be passed trough ExternalContext.encodeNamespace method ( while I
>> think that it should be portal vendor concern, where secure token has to
>> protect whole portal page ).
>> c) For token encoded as url parameter this proposal protects whole
>> application, so no one can either got logged in to protected site
>> because of circular dependencies: to open login page, visitor has to
>> have secure token, which one he can get only from JSF login page...
>> There should be per-page security configuration.
>>
>> On 09/30/2010 07:59 PM, Roger Kitain wrote:
>>> Folks -
>>>
>>> I have combined the previous two proposals into one based on valuable
>>> feed back from
>>> Kito Mann and Neil Griffin:
>>>
>>> On 9/27/10 5:12 PM, Kito Mann wrote:
>>>> On Tue, Sep 21, 2010 at 1:15 PM, Roger Kitain <roger.kitain at oracle.com
>>>> <mailto:roger.kitain at oracle.com>> wrote:
>>>>
>>>>
>>>> There are two proposals for enhancing CSRF attacks in JSF. We
>>>> need to pick one.
>>>>
>>>>
>>>> Why? I think it's actually safer to allow both:
>>>> http://www.owasp.org/index.php/Cross-Site_Request_Forgery_%28CSRF%29_Prevention_Cheat_Sheet#Disclosure_of_Token_in_URL.
>>>>
>>>> According to this, appending the token works better when you can't
>>>> guarantee that a server-side operation is occurring only through POST.
>>>> Especially with the new capabilities of JSF 2 for parsing view
>>>> parameters, I don't think we can make this guarantee.
>>>>
>>>> Also, the rewriting approach is what they're using for the generic
>>>> filter in Tomcat 7:
>>>> http://www.owasp.org/index.php/Cross-Site_Request_Forgery_%28CSRF%29_Prevention_Cheat_Sheet#Disclosure_of_Token_in_URL.
>>>>
>>>> I was talking to Mark Thomas, the guy working on this feature, about
>>>> this at JavaOne. As you can see, it doesn't even look like they're
>>>> using the hidden field approach (probably because it'd be very hard to
>>>> implement in a generic fashion).
>>>>
>>>> So, I think we should provide both features, but give the users to
>>>> turn off either algorithm, with a context parameter such as:
>>>>
>>>> javax.faces.CSRF_ALGORITHM=NONE | FORM | URL | BOTH (or ALL)
>>>>
>>> On 9/21/10 4:20 PM, Neil Griffin wrote:
>>>> #1 is probably not compatible with portlets. Portals are in full
>>>> control of creation of URLs in general, and it is not possible to
>>>> simply append "&javax.faces.Token=XYZ" to a portal's ActionURL and
>>>> expect it to work.
>>>>
>>>> #2 is compatible with portlets, but it would be best to have the
>>>> hidden field namespaced. Otherwise it would be like
>>>> javax.faces.VIEW_ID which is not namespaced, and can cause problems in
>>>> portals.
>>>> Neil
>>> ==============================================
>>>
>>> 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.
>>>
>>> - Standard context parameter javax.faces.CSRF_ALGORITHM controls
>>> where the token is produced by 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].
>>> c. "all" : token is produced as in [a] *and* [b]
>>> d. "none" : no token is produced The default is [a] "form".
>>>
>>> - 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:
>>>
>>> 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,
>>> if the configuration option javax.faces.CSRF_ALGORITHM
>>> is set to "url" or "all". 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 value of the configuration option javax.faces.CSRF_ALGORITHM
>>> is not "none" 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."
>>>
>>> Standard RenderKit Docs
>>>
>>> - Form Rendering
>>>
>>> Render a hidden field named javax.faces.Token using the
>>> ResponseStateManager.VIEW_TOKEN_PARAM constant if the configuration
>>> option javax.faces.CSRF_ALGORITHM is set to the key words "form" or
>>> "all". 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.
>>> Implementation Changes are attached to this issue:
>>>
>>> [1]
>>> https://javaserverfaces-spec-public.dev.java.net/issues/show_bug.cgi?id=869
>>>
>>>
>>> -roger
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>> --
>>> roger.kitain at oracle.com
>>> https://twitter.com/rogerk09
>>> http://www.java.net/blogs/rogerk
>
--
roger.kitain at oracle.com
https://twitter.com/rogerk09
http://www.java.net/blogs/rogerk
More information about the jsr-314-open-mirror
mailing list