Stripes

The approach for frustrating XSRF attacks (aka CRSF) I've seen is similar to what you've described here: generate a "transaction token" (nonce); embed it into a form returned by the display JSP (obtained by the initial GET); stash a copy of the token in the HttpSession; then, when the POST is processed, compare the submitted nonce with the stashed copy.

I've been thinking about this issue in relation to solving a particular spam-bot problem we have with JSPWiki, the display tier of which is being migrated to Stripes. Like you, I'd like to solve this in a Stripes-friendly way. There are two issues that need solving: (1) generating and returning the token (I prefer the word "ticket" here) to the user, and (2) processing the submitted ticket. And of course, it needs to be done in a simple and elegant way that is appropriate for Stripes.

I think I have an idea on how to do this. Here's the approach.

First, there's the issue of the ticket. As for (1), this could be done two ways.

a. The Stripes FormTag could be extended to write the ticket name and value out as a hidden field, similar to how the source page is written out today when someone uses the stripes:form tag, or....

b. The ticket could be written out as a cookie with a unique name, and value equal to the ticket value. (It should be possible for the user to have multiple 'transactions' going at once).

In both cases, the ticket name and value would be stashed in the HttpSession. The processing step at time of POST processing (2) would involve checking for the correct value, and if found, removing it from the list of stashed tickets. Failure to find the ticket would cause a checked Exception to be thrown, which could be picked up by an exception handler.

I haven't seen the cookie option discussed in the security community before, so I'm going to solicit some opinions from some security researchers about the viability of this approach. The advantage of using cookies is that no form-rewriting is needed, so you wouldn't need to modify the Stripes FormTag. It's also more "portable" in the sense that it doesn't require developers to use Stripes form tags The disadvantage is that the ticket doesn't travel with the form. I am not sure how big a deal this is, though, because the point of the ticket is that you must have one and it expires.

Now, here's how to wrap everything up with some nice Stripes-like simplicity: use annotations to simply indicate which event methods require tickets. Here's the idea:

@PredecessorEvent(beanclass="com.example.MyActionBean" event="bar")
@HandlesEvent("foo")
public Resolution foo() { ...}

The PredecessorEvent annotation indicates that event handler method "foo" requires the event handler method "bar" of MyActionBean to have previously executed before method "foo" can run. We assume that method foo() forwards to a display JSP that contains the form, though it need not. We can guarantee that "bar" runs before "foo" by using our CRSF tickets. Simple introspection at startup time would determine that there is a predecessor/successor relationship between method "bar" and method "foo," so when "bar" runs we know we need to inject a CRSF ticket.

What would make this all work is a "TransactionInterceptor" that injects the initial ticket for the first method (the GET of the display JSP), and inspects the ticket needed to the execute second method (the POST).

A nice side benefit to this approach is the ability to enforce specific paths through the webapp (you could chain @PredecessorEvent annotations across multiple ActionBeans).

I'd like to get some comments from the Stripes committers about this. I'm happy to work something up as a proof of concept if the approach seems viable.

After having built some proofs-of-concept, I have been re-thinking the approach to this, for two reasons:

• the @PredecessorEvent idea was a decent first stab, but it's too rigid. Because the same annotation type cannot mark up a method more than once, there is no way to specify multiple predecessor events
• The cookie-based approach has some potential security issues that are harder to exploit with form-generated tickets

Here's a revised approach. FormTag would write out a hidden parameter with a unique "ticket" value, if all of the following conditions hold:

• the form's beanclass and event properties are set
• the target event handler method contains an annotation indicating that it requires a ticket to process the event

That's the ticket-generation part. The ticket-inspection part would work similarly to the way described above.

Thus, if we had a stripes:form element that looks like this:

<stripes:form beanclass="com.example.MyActionBean" event="bar">

...and the "bar" event handler contains (say) a @TicketRequired annotation, like so...

@TicketRequired
@HandlesEvent("foo")
public Resolution foo() { ...}

...then the hidden param would be written.

That is much nicer and simpler. The annotation could also specify an "expires" attribute that sets a timeout threshold for the ticket.

Sample implementation. No test cases in the patch; if the approximate direction seems decent, I can make some. I've manually verified that it works.

Uploaded an improved patch, which includes a battery of TestNG tests, better commenting, and some light refactoring. This is in good shape. Tim, Ben, Frederic: if this patch proves worthy, any chance we could get it into 1.6?

we did something like this pretty quickly by subclassing the stripes form tag, overriding writeFieldsPresentHiddenField (a predefined hook for what you really need doesn't exist and should be added, lucky for us this method worked out ok by coincidence).

As a quick aside I'd like to complain that any code that is possibly intended to be subclassed should use protected variables, or better yet private variables with protected getter methods! It sucks when the hook you need isn't there, and you have to override an outer method, duplicate the code and make your small change. But there's nothing worse than having the ideal hook but not being able to read private variable in there! I think I've ran into a few instances of this in stripes, as a potential java rule I'd argue that "Any field marked private should provide a protected getter unless the class is marked final."

Anyway, in the overriden method, we wrote out a hidden field <input type="hidden" name="__token" value=" + getTokenSessionAttribute()

The token session attribute is generated once in a http session listener (on create), it's the session id + system.currentTimeMillis(). There's a stripes interceptor to run before binding, and if the method is "POST" and if the posted token does not exist or does not match what is saved in the session we know it's either a session timeout, or app restart.

If the token doesn't match it forwards (or streams json for ajax event) to a warning page, with a link to <a href="" onclick="window.location=window.location">refresh</a>

I'm not too familiar with how xss is done, does this approach prevent that as well?

John, good comments. Your approach sounds good too. In my implementation, I chose to throw a checked exception rather than hard-code a forward to an error page. The exception could be then be caught by the various Stripes exception classes and forwarded however the deployer wants. This is probably a little more portable than what you did, but only because I want this patch included in the core.

FYI, my proposed "ticketing" system for forms won't help with cross-site scripting (XSS) per se. The best defense against XSS is to (1) ruthlessly filter input (remove/prohibit angle brackets etc) and (2) sanitize/escape all output of user-submitted content. The JSTP <c:out> tag does this by default automatically, and should be used in preference to plain EL (${}) expressions. The OWASP folks have a great guide on how to do this.

Are there any thoughts to bring this into Stripes? It would be really useful, especially for some "mission critical" events. I already use JavaScript to prevent multiple submits, but this one would enforce single calls server-side which would let me sleep much better

Stripes should provide built-in CSRF protection.

• Django: https://docs.djangoproject.com/en/dev/ref/contrib/csrf/
• Grails: http://grails.org/doc/latest/guide/single.html#6.1.11%20Handling+Duplicate+Form+Submissions
• HDIV: http://www.hdiv.org/
• Ruby on Rails: http://guides.rubyonrails.org/security.html#cross-site-request-forgery-csrf
• Seam: http://docs.jboss.org/seam/2.2.2.Final/reference/en-US/html/controls.html#d0e29274