[Owncloud] Cross-site request forgery protection

[Matthew Dawson]

On June 9, 2012 11:17:24 PM Frank Karlitschek wrote:
> 
> On 09.06.2012, at 19:11, Matthew Dawson <matthew at mjdsystems.ca> wrote:
> 
> > On June 9, 2012 03:16:43 PM you wrote:
> >>>> I thought about simpler solutions but they all have problems so I think we
> >>>> have to do the real thing which means a lot of changes in ownCloud.
> >>> Well true, maybe there could be some generic measures taken to help protect 
> >>> against simple mistakes?  Things like checking the Referrer header would help 
> >>> prevent several drive-by attacks (ie using images).  Its not perfect, but it 
> >>> would be a good to have for "defence in depth".
> >> 
> >> I played with this idea too. The problem is that the referrer can be faked if
> >> the attack comes from a flash applet.
> > This is true, however there are several other attacks (ie using image tags on forum and such) that could make the request.  These can not fake the referrer.  So while this wouldn't prevent a determined attacker, and it would help prevent simple attacks.  When it comes to security, having multiple barriers is a good thing.  I wouldn't rely upon this one, but it does help.  Its all about defence in depth.
> 
> This sounds like a 90% solution to me. I agree with you generally. But we have to do the 100% solution here.
I think we agree here.  The stuff below matters more, and this would just be the nice fancy icing on the cake.

> 
> 
> 
> >>>> 
> >>>> We have to register every possible ajax call or form submit or button press
> >>>> on the page where this call could happen with a special function. This
> >>>> functions returns a token. This token has to be included in the GET or POST
> >>>> request.
> >>> <snip functions>
> >>> Looks good.  It would work for the base pieces.  It might be nice to add one 
> >>> other function to easily output a hidden input element with the name/value 
> >>> set, to allow easy integration.
> >> 
> >> good idea.
> >> This depends on the way it is integrated. It´s sometimes in JS, sometimes in a template, ..
> >> So I´m mot sure if it´s worth the effort.
> > Something like:
> > function getInputElement(){
> > 	return '<input type="hidden" name="token" value="' . callRegister() .'" />';
> > }
> > 
> > For more complicated cases, you fall back on the callRegister function.  But if I'm building a simple form this function hides some of those details and standardizes the implementation.
> 
> Good idea.
> 
> >> 
> >>> 
> >>>> 
> >>>> Opinions? Does this make sense?
> >>> I haven't looked at OwnCloud's ajax system, so I'm sorry if this isn't easily 
> >>> implemented, but the javascript double-cookie check (mentioned in the 
> >>> Wikipedia article) would be good to use too.  If you can intercept the ajax 
> >>> outgoing calls, you can restash the PHP session id into the request as a 
> >>> get/post parameter, and then re-check that the session is correct.  This could 
> >>> be largely automated, making the protection that much more effective.
> >>> 
> >> 
> >> Hmm. not sure I understood this completely. I think the only really save way is to store the token in the session and also transmit it via GET/POST and compare it after the request.
> > The trick to remember is that CSRFs rely on the fact browsers always send cookies along with a request, no matter what the origin of the request.  However, accessing these cookies, except from the original source, in javascript is impossible.
> > 
> > Thus, this defence works because in javascript, you send your session id in the get/post request.  Only the original site's javascript could do that.  You then verify that this is indeed the correct session id, and you have proof that it was your javascript that did it.
> > 
> > Of course, sending it in the GET request carries its own risks.  But all ajax requests can send stuff as a POST request, so its not too big of an issue.
> > 
> > This is again a defence in depth.  By adding this layer you again prevent people making mistakes.  With the token system, this is totally optional.
> 
> Hmmm. Interesting idea.
> What do the experts think about this? :-)
Both the wikipedia article and the links posted by Thomas Muller mention this method.  So it seems they generally like it.  Its also from where I suggested it.  Look for "Double Submit Cookies".

> 
> > 
> >> 
> >> 
> >> I added two helper calls.
> >> https://gitorious.org/owncloud/owncloud/commit/344299a074e135140262d051531f723be69c786f
> >> 
> >> Does this make sense?
> >> 
> >> Feedback is very welcome. And help to port the existing ajax call of course. ;-)
> > That looks like a good start.  Is there any worry about the session space exploding as more tokens are generated?  For long running sessions, the amount of space used may start to explode.
> > 
> > Also, for long running apps (like the music player, for instance), is 1 hour enough time?  Maybe there should be a way to refresh tokens.  Sort of like:
> > 
> > function refreshToken(){
> > 	callCheck();
> > 	return callRegister();
> > }
> 
> Both are good points. I think we have to see how it works in reality.
> 
True.  I think what you have so far is a good start for moving forward.  Refreshing tokens is a separate issue that be dealt with later, and that session space explosion just needs some sort of pruning system.  Neither is critical for getting this system out there.

Maybe for long running things, it just falls back onto the method above and skip tokens.  That would help.


Matthew