[Owncloud] Cross-site request forgery protection

[Frank Karlitschek]

On 11.06.2012, at 05:15, Matthew Dawson <matthew at mjdsystems.ca> wrote:

> On June 10, 2012 09:44:24 PM Florian Rüchel wrote:
>> Hi Frank,
>> 
>> I thought about CSRF protection and the general idea already stands. We
>> should now figure out how we want to have it implemented and then I will
>> start working on it.
>> 
>> What we need for it would first be a good PRNG (pseudo random number
>> generator). I dug up some code from here:
>> http://forums.thedailywtf.com/forums/t/16453.aspx
>> I looked through it and it seems like a reasonable approach though its
>> fallback is silent and we should think about any kind of user
>> notification or removing the fallback. The important thing here is the
>> seed and this needs to be as random as possible and no microtime or PID
>> stuff will be random enough.
> Hmm ... Well I agree better stronger seeds are good, microtime + PID is pretty difficult to guess.  Especially if you stick a lockout system on it, there is probably enough entropy.  I don't think that fallback is a weakness.  If a user wishes to audit their setup, a page could be created.
> 
>> The solution basically tries:
>> - OpenSSL (very good!)
>> - /dev/urandom (nice one as long as the maintainers keep it random,
>> Debian already did screw up, but random enough for our needs, Unix only
>> though)
> As far as I remember, /dev/urandom wasn't broken, but openssl.  /dev/urandom's only issue is that the entropy is not checked.  For better random numbers, /dev/random is the place to look.  And by itself, /dev/urandom is good enough for our needs as its not a PRNG, it instead uses randomness induced by the computer (thinks like hard drive speeds, which deviate based upon the head's location.)
> 
>> - CAPICOM (Windows, uses CryptoAPI, also not bad if we are on Windows)
>> - Fallback to crappy microtime stuff
>> 
>> So by this we get a rood new random number for the seed which will then
>> be used by the wrapper for PHPs random function. I stuffed this into the
>> OC_Util class but we could also think of some place else.
>> 
>> The next thing will now be the question of how to keep track of the
>> tokens generated. The general suggestion here that keeps balance between
>> usability and security is a per-session token. My knowledge on the
>> database and such is limited so I need to know where we could store such
>> session information.
> Those can be stored in PHP's session system ($_SESSION).  Frank already implemented two functions doing this.  The only difficulty I see is that the session state space explodes with several calls.

I added a simple garbage collector to the call. I think this should solve the problem for now.

> PHP's session system doesn't send the stored values over HTTP, so it should be secure enough for our uses.
> 
>> 
>> Then I will create simple functions allowing for a token check and of
>> course the generation. Also a wrapper will be useful that inserts a form
>> element (so that does not get passed to the user).
> There are a couple of ideas/function kicking around in this thread.
> 
>> 
>> Another thing that needs to be discussed is in which way we want to
>> inform developers about the existence (and importance) of such tokens,
>> i.e. if noone knows about them we did not fix anything.
>> 
>> The basic approach would be to write a good documentation and just leave
>> it to the developers.
>> Another approach would be to fail any requests if neither a token was
>> added nor was the request marked is non-critical (in some way).
>> I leave that decision up to you guys, I have a hard time deciding for
>> either solution but I guess for compatibility you should just take the
>> first approach and just spread the word.
> This is why I suggested the referrer check.  Then if a developer doesn't know/ignores this issue, the user does get some protection.  I think it should be up to the developer to implement, but it should be as easy as possible to do so.
> 
> Matthew


It would be great if developers try the 2 functions and see if it works. Then we have to port all the important calls to it.
We also have to backport it to the stable4 branch because this is a security issue.



Frank