Fixing CSRF exploits in Infrastructure

[Till Maas]

On Wed November 26 2008, Toshio Kuratomi wrote:
> Till Maas wrote:
> > On Wed November 26 2008, Toshio Kuratomi wrote:
> >> To be easy to code, require the token for every request of an
> >> authenticated user.
> >
> > If I understand your proposal correctly, a user would need to login again
> > for every link he clicks from his bookmarks or any mail he gets from a
> > Fedora webapplication, e.g. packagedb.

> The con of this is that the consequences of not marking a method are a
> security hole this way.  One that we would have to audit the code to
> discover.  I'm not a big fan of this tradeoff but I'm open to comments
> -- is this something that's so big a regression that we should bite the
> bullet and do it?  Can we come up with another method that protects the
> users from failure by the programs author to properly mark the methods?

How big the regression is if users have to log in for every external link they 
click on, depends on how often this happens. I believe that links to FAS are 
not exchanged very often, therefore it will not hurt very much. I guess there 
is also not so often a need to use FAS with tabs. But maybe there are people 
who have to use FAS more often. With Bodhi it is contrary, because there it 
is normal to get mails with links if someone added a comment to a package or 
for testers to exchange links to Bodhi updates. Also links to Bodhi updates 
are used in Bugzilla comments. There it would have a much bigger impact on 
the efficiency of testing new package updates imho.

Regarding the time needed for auditing applications: There may still be a lot 
of other vulnerabilites in these applications which cannot be fixed 
automatically. Therefore they still need to be written carefully. But maybe a 
compromise would be to require the token for all requests by default and then 
whitelist the ones, that are not meant to change state, e.g. requests like:

https://admin.fedoraproject.org/updates/pstreams-devel-0.6.0-6.fc10

> > And with every login a previous session is
> > invalidated, which also includes links in another open browser tab, where
> > the user logged after he clicked the previous link.
>
> So this is interesting.  This is true since we wanted to remove the need
> to hash the tg-visit in javascript and thus the token is 100% statically
> derived.  We could work around this in several ways:
>
> 1) In addition to checking the hash against the tg-visit, check against
> any non-expired session.  If we did this check in the FAS server (all
> authentication goes back to the FAS server already) then I think that
> would work.  So we'd send the FAS server the tg-visit cookie and the
> token.  Then the FAS server would do the comparison of the token to the
> visit cookie; comparison to the visit to the current active sessions,
> and finally, if the token did not match the cookie, between the token
> and other non-expired sessions owned by the user.

Another way would be to not change the session id if a user needs to supply 
the username/password again only because the token was missing. It would 
probably be enough to ask the user only to click a link that contains the 
matching token in case the token is missing.

Nevertheless it seems to me that securing all requests against CSRF 
automatically makes it a little easier to write a application, because the 
author does not need to care whether a request changes state or not. On the 
downside it has a high impact on usability or makes the automatic CSRF 
protection a lot more complicated. Also securing all requests may cost a lot 
of performance, because more requests need to be made.
Last but not least is always more time spent on using an application than on 
writing it, therefore if the usability of an application is only enhanced a 
little, because of the many times it is used, there will be more manpower 
saved than is used to enhance the application.

Regards,
Till
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 827 bytes
Desc: This is a digitally signed message part.
Url : http://lists.fedoraproject.org/pipermail/infrastructure/attachments/20081126/e84dcbdc/attachment.bin