[jsr-314-open-mirror] [jsr-314-open] [Spec-869-Specify CSRF Solution] PROPOSAL

[Martin Marinschek]

Hi Andy,

is this the time to finally add a Form interface? please say so ;)

best regards,

Martin

On 10/22/10, Andy Schwartz <andy.schwartz at oracle.com> wrote:
> Another though occurred to me at 3AM while trying to convince a
> screaming baby to go back to sleep...
>
> Several frameworks (Trinidad/ADF Faces as well as others) provide their
> own form components (ie. components that replace h:form).  How do we
> expect these form components to participate in the CSRF solution?  In
> order for this to work, the form components must have access to the
> secret key/token since they must render the hidden field for the token.
> Do we need to provide additional APIs to better support this case?
> Seems like without this our existing form components will not be
> compatible with CSRF_ALGORITHM=form.
>
> Andy
>
> 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.
>>
>> 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.
>>
>> 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.
>>
>> 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.
>>
>> Andy
>>
>
>


-- 

http://www.irian.at

Your JSF powerhouse -
JSF Consulting, Development and
Courses in English and German

Professional Support for Apache MyFaces