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

[Blake Sullivan]

  On 10/22/10 11:10 AM, Roger Kitain wrote:
>  Hey Blake -
>
> Thanks for responses..
>
> On 10/22/10 12:51 PM, Blake Sullivan wrote:
>>  Roger,
>>
>> Di you answer Alexander's question regarding whether this feature is 
>> even necessary?  In the current releases, with no explicit CSRF 
>> defenses, we have attacks against:
>> 1) GET attack
>> 2) POST attack against a page with token state saving
>> 3) POST attack against a page with client state saving
> Yes.  By number 2 - I presume you are talking about server side state 
> saving.
>>
>> 1) For GET attacks, it appears that we are saying that we aren't 
>> going to defend against these anyway for this release.
> Yes. It's a bit trickier as we may need to do this on a page by page 
> basis.
I think that it is trickier than that--you need to worry about things 
like your parameter leaking out through the referer header.  At this 
point, I don't see how you would get the spec work done in time.  Plus, 
a well written web application won't have side-effects on the GET 
anyway.  The CSRF attacker can GET all day and not change the 
application state.
>>
>> 2) For POST attacks against a page with token state saving.  As long 
>> as the view state token is cryptographically strong, the attacker 
>> can't guess the token anyway.  At that point, it comes down to what 
>> the behavior is for a request that has an invalid token--we can 
>> either return an error page or we can treat the request as a GET.  If 
>> we treat the request as a GET, we are in 1
> For server side state saving by default, the view state is not as strong.
>   So we were looking at
> this CSRF "extra token" approach as facilitating the view state for 
> CSRF solution.  However, there are ways to make the server side view 
> state stronger - in which case there may not
> be a need for an extra token to augment the view state.  For now it 
> may be as simple as a clarification
> (recommendation) in the spec about the server side view state being 
> cryptographically strong - I'd have
> to check the spec to see what is there.
Yep.  Make it cryptographically strong, as it is in Trinidad and you are 
done with the server-side storage POST except for figuring out what to 
do if the token isn't valid.  I see no need to add yet another token 
when the existing one will do.
>>
>> 3) For POST attacks against a page with client state saving, as long 
>> as attackers can't forge the client state, they can't do anything.  
>> Since, they can't, we're safe here as well.
> Yes, client side view state is cryptographically strong.
Yep
>>
>> Therefore, it isn't clear what the feature is buying us.  If there is 
>> no clear need for the feature, it should not be added to the 
>> specification.
> If we are deferring GET CSRF handling, and possibly adding a spec 
> clarification for server side view state, then I agree.
Cool

-- Blake Sullivan

>>
>> -- Blake Sullivan
>>
>>
>> On 9/21/10 10:15 AM, Roger Kitain wrote:
>>>
>>> There are two proposals for enhancing CSRF attacks in JSF.  We need 
>>> to pick one.
>>>
>>> Proposal 1: Form Action URL Approach (Approach provided by Kito Mann)
>>>
>>> This approach does the following:      - Token is generated on the 
>>> server consisting (minimally) of a randomly generated "secret key
>>>      (stored in session).
>>>    - ViewHandler.getActionURL method must include the token parameter
>>>      named "javax.faces.Token", and whose value is the token value.
>>>    - At render time this token will be included in Form's action URL 
>>> - and it will be
>>>      posted back 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 URL must contain the parameter constant defined by 
>>> ResponseStateManager.VIEW_TOKEN_PARAM
>>> The value of this parameter must be a cryptographically produced 
>>> value minimally consisting
>>> of a "secret key". The "secret key" is a random generated value that 
>>> was stored in the session
>>> (preferably around session creation time).  Implementations may also 
>>> choose to combine other
>>> values with the secret key to produce a more complex token."
>>>
>>> Section 2.2.1
>>>
>>>  "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."
>>>
>>>
>>> Proposal 2: Form Hidden Field Approach
>>>
>>> This approach is similar to Approach 1, except a Form hidden field 
>>> "javax.faces.Token"
>>> is used instead of appending to the Form's Action URL.
>>>
>>> Spec Document Modifications:
>>>
>>> Standard RenderKit Docs
>>>
>>> - Form Rendering
>>>
>>> "Render a hidden field named "javax.faces.Token" using the 
>>> ResponseStateManager.VIEW_TOKEN_PARAM
>>>  constant.  The value of this hidden field is a cryptographically 
>>> produced value that must at least
>>>  consist of a "secret key".  The "secret key" is a random generated 
>>> value that was stored in the
>>>  session (preferably around session creation time).  Implementations 
>>> may also choose to combine
>>>  other values with the secret key to produce a more complex token."
>>>
>>> Specification Document
>>>
>>> Section 2.2.1
>>>   "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 FacesException.
>>>
>>>
>>> For both approaches see:
>>>
>>>
>>> [1] 
>>> https://javaserverfaces-spec-public.dev.java.net/issues/show_bug.cgi?id=869
>>>
>>> Look at the two latest change bundles attached to the issue.
>>>
>>> Please review by COB Friday as we have no time left for 2.1.
>>>
>>> Kudos to Kito Mann for helping out with the implementation.
>>>
>>> -roger
>>
>
>