Homec4science

Fix conservative CSRF token cycling limit

Authored by epriestley <git@epriestley.com> on Jul 13 2011, 23:05.

Description

Fix conservative CSRF token cycling limit

Summary:
We currently cycle CSRF tokens every hour and check for the last two valid ones.
This means that a form could go stale in as little as an hour, and is certainly
stale after two.

When a stale form is submitted, you basically get a terrible heisen-state where
some of your data might persist if you're lucky but more likely it all just
vanishes. The .js file below outlines some more details.

This is a pretty terrible UX and we don't need to be as conservative about CSRF
validation as we're being. Remedy this problem by:

  • Accepting the last 6 CSRF tokens instead of the last 1 (i.e., pages are

valid for at least 6 hours, and for as long as 7).

  • Using JS to refresh the CSRF token every 55 minutes (i.e., pages connected

to the internet are valid indefinitely).

  • Showing the user an explicit message about what went wrong when CSRF

validation fails so the experience is less bewildering.

They should now only be able to submit with a bad CSRF token if:

  • They load a page, disconnect from the internet for 7 hours, reconnect, and

submit the form within 55 minutes; or

  • They are actually the victim of a CSRF attack.

We could eventually fix the first one by tracking reconnects, which might be
"free" once the notification server gets built. It will probably never be an
issue in practice.

Test Plan:

  • Reduced CSRF cycle frequency to 2 seconds, submitted a form after 15

seconds, got the CSRF exception.

  • Reduced csrf-refresh cycle frequency to 3 seconds, submitted a form after 15

seconds, got a clean form post.

  • Added debugging code the the csrf refresh to make sure it was doing sensible

things (pulling different tokens, finding all the inputs).

Reviewed By: aran
Reviewers: tuomaspelkonen, jungejason, aran
CC: aran, epriestley
Differential Revision: 660

Details

Committed
epriestley <git@epriestley.com>Jul 14 2011, 17:09
Pushed
aubortJan 31 2017, 17:16
Parents
rPH2a73e7eded06: Merge pull request #36 from CodeBlock/master
Branches
Unknown
Tags
Unknown

Event Timeline

epriestley <git@epriestley.com> committed rPH15ef2fced0f7: Fix conservative CSRF token cycling limit (authored by epriestley <git@epriestley.com>).Jul 14 2011, 17:09