data-govt-nz / ckanext-security

A CKAN extension to hold various security improvements for CKAN
GNU Affero General Public License v3.0
25 stars 32 forks source link

Memcached get fails randomly for CSRF tokens #6

Closed bzar closed 5 years ago

bzar commented 7 years ago

Running CKAN with multiple processes+threads seems to cause (or make more probable) some get requests to memcached return empty strings for CSRF tokens. Adding a loop in middleware.py to fetch the token until a non-empty string is returned "fixes" the issue, but is not (of course) a viable alternative. Sometimes it takes 5 tries until the token is retrieved.

I've tried to replicate the issue by running pylibmc scripts in the same environment that set/get the same token in parallel, but haven't been able to get a single empty string. It doesn't make sense an empty string would be stored in memcached, so I'm assuming it's some error handling mechanism.

With the bug in play, I'm getting a CSRF error roughly 1/3 of the time when submitting forms.

camfindlay commented 7 years ago

Thanks for this @bzar would be good to know more about your setup to make sure we can replicate the issue. What version of CKAN are you applying this extension to as a starter? 👍

bzar commented 7 years ago

Sure! We're using CKAN 2.6.2 at the moment, but in the process of moving to 2.7.0.

camfindlay commented 6 years ago

Does #5 fix this?

bzar commented 6 years ago

As I recall, it should not. This is a separate issue that is only visible after fixing #5 .

camfindlay commented 5 years ago

Is this still a known issue?

bzar commented 5 years ago

No idea, we had to switch to our own implementation of CSRF due to this though.

camfindlay commented 5 years ago

See our new proposed way of doing CSRF which we'll look to make a tagged release of soon #24

ebuckley commented 5 years ago

As per cams comment we have switched to a stateless method of checking CSRF tokens, this is released in 1.1.0 https://github.com/data-govt-nz/ckanext-security/releases/tag/1.1.0