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

Martin Marinschek mmarinschek at apache.org
Wed Oct 27 19:31:52 EDT 2010


Hi Blake,

>>> By double submit, which cases:
>>> 1) User accidentally presses the button twice
>>> 2) User hits back to a POST
>>> 3) User hits reload on a POST
>> All of them.
> OK.  Though the first of these is better addressed (or additionally
> addressed) through the user interface--for example, disabling the button
> and potentially showing some kind of busy indicator on it.

Oh, certainly. The keyword is additionally here - we never trust the
client to do this properly in server-side web-programming. Or are you
also proposing to move our validation logic to the client ;) ?

>>> For refresh handling, what is the use case where you want to deal with
>>> reload differently than the initial render?
>> That would be my default implementation - but I would still let the
>> application decide how to handle this. An example: some applications I
>> have seen display some kind of message alongside the re-rendering.
> I'm just curious.  What kind of message?

A message along the lines that content has been redisplayed, but no
actions have been executed. If you want the exact message, I can try
to dig it up for you.

>>>> Problem 2: what happens if (in server-side state-saving) the
>>>> view-states run out while you go back through the history? JSF does
>>>> anything (depending on the release), but never something which is a)
>>>> the same across all versions b) makes sense c) is influencable by the
>>>> user.
>>> I agree that we need to decide how this should work as well as what
>>> should happen if we receive a token we've never seen and that this needs
>>> to be part of the 2.1 release.
>> If we do the token handling at all - yes, it should probably be, but I
>> don't see proper  handling of these in for 2.1. It's gonna take us
>> until 2.2.
> We already have the issue that the specification should be saying how we
> handle a view state token for which the server has no view state.  If we
> are saying that the view state token is also protecting POSTs from CSRF
> attacks, I think we need to make sure that our handling of the invalid
> token isn't too nice.  We can be pretty nice in the case where it is a
> token that was once valid, but if it is a token we've never seen, we
> might want to go medieval on the response--for example send back a 400
> error.

Yes - this ties in with all the other problems we have with back
button handling right now (memory-issues, at least in the JSF impls
out of the box). I know Trinidad has a different way of handling
things, but we are not talking component libs here. If we can store a
list of tokens long enough, we are safe to not throw a 400 at people
who just happened to press the back button too often. If the token is
the server-side view-state id, we have the whole view-state dangling
of the token - with the associated memory problems, we certainly don't
want to make the list any longer then.

>>>> Problem 3: Treating browser back as applicatory back: as an
>>>> application developer, I would love to have a way to treat the browser
>>>> back as an applicatory back instead of a mere browsing in the history
>>>> of pages displayed.
>>> Why can't you do that today?
>> Well, if I save the request tokens myself, than certainly, I can. I
>> just don't want to take care of such things as an application
>> developer. I hear missing back-button handling as one of the remaining
>> complaints about JSF all the time, I wonder why you don't hear
>> anything about this? Are the agonized cries of project-developers so
>> muted down at the Oracle holy-halls ;) ?
> They have been beaten into submission.
>
> But also the problem is hard for complicated applications.  At a
> framework level we can tell the application to push undo/redo objects
> during the request, but I don't think that quite does the job in some of
> the weird cases.

Ah well, you are jumping my boat: why not provide an API which allows
application developers to easily hook into this handling for complex
applications? The question is only if the API should be there on an
application level, or on a per-page basis. For me, application level
would be enough.

>>>>>> I agree with other commenters that the CSRF protection really makes
>>>>>> only sense per view - or actually, and this is hopefully a right
>>>>>> assumption, per what the view does.
>>>>> But I don't think it is a per-view.  Isn't it potentially per action?
>>>> Yes - but, if we consider the post case covered, no actions are gonna
>>>> be executed on a CSRF-call, except for view-actions or pre-render
>>>> callbacks. So its more a view-level action I am talking about, that's
>>>> what I meant with "what the view does".
>>> I must admit that I haven't looked at the view-actions enough, but can't
>>> I have some view actions that are idempotent and others that aren't.
>>> I'm mostly worried about squeezing in something that we haven't fully
>>> thought out.
>> Yes - some view-actions are certainly idempotent, some aren't. We
>> should probably put the  configuration on these artefacts.
>>
>>>> In any case, I am not advocating that this needs to be in right away
>>>> and is absolutely necessary, I just advocate that if we add something
>>>> for CSRF protection which necessitates a new token-API, we might want
>>>> to do it proper and cover these other issues as well.
>>> I agree that if we added a new per-request token we should look at the
>>> rest of the cases.  I just don't think we have time to do everything
>>> that we would need to for this release.
>> Then I would rather fix the issue of an encrypted view-state token
>> only, and defer the rest to 2.2 proper, just like you originally
>> suggested. Which doesn't mean we couldn't continue our discussion and
>> reach an agreement right now which we can re-use for 2.2 development.
> Yep.  I agree.  I fall on the "do no harm" side of the specification.
> When in doubt, do what you think is definitely right to make user's
> lives better.  Keep track of the feedback for the next version.
> Hopefully get even better use cases for the next version and then get
> new versions out on a predictable and timely basis.

Certainly, I agree - as I said, let's work on a proposal for 2.2.

One more issue which needs to be solved: when a new window is opened
(with a target="..." attribute), we do not want to prevent double
submits of the previous page. So we need to tie into the
commands-handling as well.

best regards,

Martin

>>>>>> On 10/22/10, Blake Sullivan<blake.sullivan at oracle.com>    wrote:
>>>>>>>      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
>>>
>>
>
>


-- 

http://www.irian.at

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

Professional Support for Apache MyFaces



More information about the jsr-314-open-mirror mailing list