[CKAN-Security] (no subject)

[Thrawn]

Hi, David.

Thanks for the feedback :). Yes, custom extensions could well encounter some problems.

I'm not entirely certain of the Django anti-CSRF implementation, but if it doesn't use cookies, then I suspect that it stores the token in the user session. Definitely feasible, but it means more state on the server, whereas the double-submit implementation is stateless. It might also mean more changes to the code; I'm not sure. OWASP includes both as feasible options (https://www.owasp.org/index.php/Cross-Site_Request_Forgery_%28CSRF%29_Prevention_Cheat_Sheet), along with the Encrypted Token pattern, making a token by encrypting the user id, timestamp, nonce, and a secret, similarly to how auth_tkt cookies are generated.

There is definitely room for improvement in the insertion of the token. To keep the regex simple (single line), I had to look for the form close tag, not the opening tag - so I couldn't distinguish GET from POST. And yes, for transitional purposes, it would be advantageous to specify which forms should/n't be protected.

Regards

Thrawn

------------------------------
On Mon, Nov 24, 2014 10:58 PM AEDT David Read wrote:

>Thanks very much for this Thrawn! This code is very useful. I've had a
>bit of a think in how to implement this in core and take account of
>various people's set-ups and customizations.
>
>Having a token is v. bad for caching - for DGU we'd not want any token
>in the form or cookie headers until the user is logged in, when we
>don't cache.
>e.g. DGU use POST requests for drop-downs lists that auto-submit (e.g.
>language selector, 'sort-by' drop-down etc) - we don't want CSRF
>protection for this.
>
>It is recommended to require a token in all form POSTs apart from a
>specific list of unchecked forms. That way when someone adds a new
>form, POSTs will not work until the form is CSRF protected or
>whitelisted - it does not become a security hole by default. However
>as Ross says, this could disrupt extensions that have POSTs.
>
>So I suggest a smooth transition over two CKAN releases: in the next
>release we have a list of forms to check and a list of forms to not
>check, and the remaining forms (extensions) are not checked but have
>warning log messages. And in the following release we switch the
>default so that those non-listed forms are checked. We could add a
>note in the CHANGELOG for each release saying that every form needs a
>csrf token adding, or the form is added to the 'not-checked' list.
>
>I don't like adding cookies as it is one more thing to ignore in a
>cache and explain in the site's privacy page. I prefer Django's way of
>simply using a hidden form field containing the token. The Django page
>suggests that cookies are useful for AJAX that needs protection, but I
>don't think we have any AJAX to edit things the moment (do we?). We
>could always simply add cookies if they are really necessary.
>
>Yes we need to stop the ckan cookie being allowable authentication
>when doing non-GETs to the API. As a sysadmin logged into ckan in a
>browser, it can be useful to GET a group and see its users. However I
>don't think people would use a browser to create or update objects. So
>we could only allow API GETs to use the browser cookie.
>
>David