Builtin CSRF protection

[amercader]

While you can add CSRF protection to your site using ckanext-security, we would benefit a much larger number of users if this was built-in and enabled by default. The last time this was discussed the consensus was to aim for a new module on CKAN based on the excellent Django's CSRF protection. To avoid having to maintain a port of Django's code I think we should look at leverage another library. Here are the best contenders IMO (please propose more options if you know some!):

1. Flask-Seasurf is exactly what we were thinking of, a port Django's CSRF module to Flask. It has no dependencies other than Flask and it seems fairly well maintained.
2. The Flask-WTForms CSRF module can be used in any view without using WTForms (if I understood correctly). This is a much more widely used package, but of course adds the WTForms dependency. This might not be a big deal (and even in the future we might want to think about replacing our venerable navl module)

Both libraries offer what I think are our requirements:

• Configure the CSRF via middleware:

  
  from flask_seasurf import SeaSurf

csrf = SeaSurf(app)

or 

```python
from flask_wtf.csrf import CSRFProtect

csrf = CSRFProtect(app)

• A Jinja2 helper function to generate the token:

  <form method="post">
  <input type="hidden" name="csrf_token" value="{{ csrf_token() }}"/>
  </form>

• Support for AJAX requests (Flask-Seasurf / Flask-WTForms)

The implementation plan here would be:

0. Choose a library
1. Add the middleware (TODO do we need to pass any config options?)
2. Update forms in core templates to include the hidden fields with the csrf token, probably with a macro (eg macro.csrf_token())
3. Update the core AJAX calls as needed to pass the token
4. Document steps needed for extensions to apply these changes to their custom forms
5. Update ckanext-scheming (? only if it's overriding the whole core form)

Comments and thoughts welcome

[wardi]

option 1. flask-seasurf sounds like the best approach to me

For our API we then need to start ignoring cookie authentication unless it is accompanied by a csrf token as a header, for both GET and POST requests. This change will most affect users that have built headless front-ends that rely on cookie auth.

[amercader]

A way to ease the transition for core and extension AJAX calls to the API might be to add the necessary parameters or headers to the builtin ckan module used in the JS modules to call the API (like in here. Here's the call() method that could be extended for this. Of course we would need to make the param / header value available to the JS client somehow.

[tino097]

We had some work done at Salsa and using this extension. It is based on @boykoc and @ThrawnCA previous work and we had adapted it for 2.9+ and i guess it could help at least to get some direction

cc @MarkCalvert

[ThrawnCA]

@amercader I'm concerned about an approach that will break pretty much all existing extensions until they change their forms, and doesn't seem to provide a backwards-compatible solution; an extension that adds the CSRF token field to its forms will break on any version of CKAN that doesn't have CSRF support.

That said, if that's the approach, Seasurf seems like a lower-footprint option, and without the...questionable acronym.

Perhaps there could be a CKAN version that adds a helper function for the CSRF token, returning a dummy value at first, to give extensions the chance to update themselves in advance of CKAN implementing full CSRF support? As a bonus, this would be usable immediately by the authors of existing CSRF filters, who could add their own implementations of the helper function.

@tino097 If you're still maintaining the Fortify extension, you might want to look at the latest changes in https://github.com/qld-gov-au/ckanext-csrf-filter/

[ThrawnCA]

Oh - also, ideally there should be a mechanism for extensions to override and provide their own implementations of CSRF token generation and validation. For example, the default Django token implementation does not protect against other subdomains overwriting the token cookie, whereas the HMAC-based approach of ckanext-csrf-filter provides a degree of protection.

I also noticed when implementing ckanext-csrf-filter that there are special considerations around protecting the login form (to prevent Login CSRF attacks). The FriendlyForm Repoze plugin intercepts the traffic before regular CKAN code gets a chance to validate it. Just something to keep in mind when implementing a solution.

[amercader]

@ThrawnCA thanks for the feedback these are all important points.

I'm concerned about an approach that will break pretty much all existing extensions until they change their forms

I agree this will be disruptive in some cases but I still think we should aim to have this behaviour enabled by default in the next CKAN version.

I guess at very minimum we should have a configuration option to disable the CSRF checks, ie keep the current behaviour (ckan.csrf_protection.enabled=False, defaults to True). This would allow sites to transition or use alternative implementations like ckanext-csrf-filter without changes.

an extension that adds the CSRF token field to its forms will break on any version of CKAN that doesn't have CSRF support.

I guess what you mean is that the csrf token field is added to a form of an extension that targets multiple CKAN versions, and this is used in a CKAN version without CSRF support it will get a _crsf_token field that it doesn't know how to handle and could up being stored somewhere (eg when editing a resource). If we use a template helper (h) or a Jinja macro to render the CSRF hidden field we could backport versions of these that render nothing on old CKAN versions.

Oh - also, ideally there should be a mechanism for extensions to override and provide their own implementations of CSRF token generation and validation.

Looking at the Flask-Seasurf source I think the only way to do that is to create a light wrapper around the Seasurf class and override their relevant methods to hook them to a CKAN interface (perhaps involving some monkey-patching).

I also noticed when implementing ckanext-csrf-filter that there are special considerations around protecting the login form

Thanks, that's really good to know. We use our own vendored version of friendlyform so we can add any modifications needed there

[ThrawnCA]

I guess at very minimum we should have a configuration option to disable the CSRF checks, ie keep the current behaviour (ckan.csrf_protection.enabled=False, defaults to True). This would allow sites to transition or use alternative implementations like ckanext-csrf-filter without changes.

That sounds like a low-impact option, yes. It would also help sites that need to keep using legacy extensions.

I guess what you mean is that the csrf token field is added to a form of an extension that targets multiple CKAN versions, and this is used in a CKAN version without CSRF support it will get a _crsf_token field that it doesn't know how to handle and could up being stored somewhere (eg when editing a resource). If we use a template helper (h) or a Jinja macro to render the CSRF hidden field we could backport versions of these that render nothing on old CKAN versions.

I actually meant that there would be a template error when attempting to call a _csrf_token function that doesn't exist. But yes, storing the token as an extra field is a concern too, and making it a helper that can be dummied out is a good solution.

[duttonw]

Just want to link to how java spring-security does it, https://docs.spring.io/spring-security/site/docs/3.2.0.CI-SNAPSHOT/reference/html/csrf.html

All forms should be manually updated to include the _csrf hidden token and should be setup to gracefully be blank if not found.

When enabled it will 'kill' un-updated plugins which should trigger a good hygiene in getting them updated. (if they are not python3 ckan 2.9+ then it needs to get some tlc time etc)

I'm not really a fan of @ThrawnCA auto injection regex, its seems nice, but could possibility be abused compared to adding them directly per form/plugin etc.

I should also mention csp3 which is also should be looked into adding a nonce to javascript and css links which match the header for even better security. (i.e. simliar-ish to csrf needing to alter forms etc)

https://scotthelme.co.uk/csp-nonces-the-easy-way-with-cloudflare-workers/ https://scotthelme.co.uk/protect-site-from-cryptojacking-csp-sri/

[ThrawnCA]

Would it be feasible for an extension to support both old and new CKAN versions with a snippet like this?

{% if csrf_token is defined %}
  {{ csrf_token() }}
{% endif %}