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

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

From: Ed Burns <edward.burns_at_oracle.com>
Date: Wed, 17 Aug 2011 15:12:16 -0700

Relative to proposal version 20110726 of
<http://javaserverfaces-spec-public.java.net/nonav/proposals/JAVASERVERFACES_SPEC_PUBLIC-869-CSRF.txt>.

This email contains my responses to Blake's helpful 27 July reply.

I will produce a new version of the proposal as time allows this week.

>>>>> On Wed, 27 Jul 2011 10:30:22 -0700, Blake Sullivan <blake.sullivan_at_oracle.com> said:

B> For the GET support:

B> As noted before, the secret key solution is less secure than other
B> alternatives and is also subject to session fixation attacks.

B> Why does the <cross-site-request-forgery-element> live in the
B> <faces-config-element>? Since the JSF specification is adding the new
B> element, this isn't exactly an extension.

Why? Because this was the intent of the faces-config-extension element.
We used this in 2.1 to control the rendering mode. In looking at the
314 archive, I see that there was some relevant content from Andy
Schwartz about this feature.

http://lists.jboss.org/pipermail/jsr-314-open-mirror/2010-October/002951.html

Basically, we created the faces-config-extension element as a place to
put configuration content that is specific to faces, but would otherwise
be in the web.xml. In JSF 2.1, it was the <facelets-processing>
element.

B> What features of <web-resource-collection> were you planning to use? It
B> seems that the <url-pattern> by itself would be sufficient, in which
B> case you could rename <cross-site-request-forgery-element> to something
B> else like <protected-urls>

Yes, that is reasonable.

B> The specification should be specific that these elements will be merged
B> across all faces-configs.

Very good point. I'll add that.

B> There is no runtime access to this information. I suspect that this
B> could be a problem for the portlet bridge.

Yes, I'll add runtime access to the proposal.

B> Solutions that modify the URLs require that every encoded URL be
B> checked. Performance testing needs to be done for realistic cases.
B> This is an example where "referer" checking would be significantly faster.

B> Even if we end up falling back to URL manipulation for GETs, I would
B> prefer that we use referer checking if the referer was present in the
B> HTTP Request that initially created the session, as this doesn't
B> require any URL manipulation.

EB> Yes, using referrer is probably a valid option.

I'll rework the proposal to include this.

B> We no longer need the "form" and "url" sections.

I'll remove those sections.

B> Section 7.5.1:

B> If we are munging the URLs, why would the URL parameter be anything
B> other than ResponseStateManager.VIEW_TOKEN_PARAM?

That's probably a remainder from a previous iteration.

B> Section 2.2.1:

B> Shouldn't we be more specific about the exception thrown? I would think
B> that an IllegalStateException would do the trick, but we might want an
B> explicit exception so that the exception handling system knows what kind
B> of exception this is.

I'll introduce a new FacesException subclass.

B> I don't think that anyone would do this, but why can't the token be
B> in the exception?

I don't want to give any clues at all to how this is implemented.

Ed

-- 
| edward.burns_at_oracle.com | office: +1 407 458 0017
| homepage:               | http://ridingthecrest.com/
| 27 business days until JSF 2.2 Early Draft Review
| 31 business days until JavaOne 2011