Skip to content

Restrict session

Bengfort requested to merge restrict-session into main

I was thinking about the security impact if cron jobs are not run. This is a part of the system we have little control over so we have to assume this might happen. One cron job that has a direct impact on security is clear_sessions, so I concentrated on that.

We have many restrictions in place for users: In addition to the usual mechanism, a user account in castellum can also expire. On top of that, The user is logged out automatically on inactivity.

It is possible to bypass both of these additional mechanisms by just staying logged in and faking activity with some form of automation. So we added the recommendation to force-logout all users once a day by using the clear_sessions command we provide. If that cron job is missing, the previously described attack is possible.

Django itself provides a command called clearsessions that has a slightly different goal: It clears expired sessions that are no longer needed from the database to reclaim disk (and therefore prevent DoS). Our script deletes all sessions, so it can replace clearsessions. However, it should have a more distinct name to avoid confusion.

My proposal however is to remove our clear_sessions and instead hardcode the desired behavior in the software itself. @goettel do you think this should have a setting?

In addition to that I also reduced SESSION_COOKIE_AGE from two weeks to one day, just as a fallback.

Note that this is a breaking change because the clear_sessions cron job needs to be replaced by clearsessions.

Merge request reports