Comment 25 for bug 1394370

Revision history for this message
Tihomir Trifonov (ttrifonov) wrote : Re: horizon login page is vulnerable to DOS attack

Here is the updated patch, containing the request.session.delete(request.session.session_key) code in process_response, but in a different middleware class.

Since we have this code in Django: https://github.com/django/django/blob/master/django/contrib/sessions/middleware.py#L49 the SessionMiddleware tries to save the session in case it is modified, and this is the case when logout is called for a logged in session. Since the user was logged in, then logged out, the session is marked as modified. The problem here is the order of execution of the middlewares - the SessionMiddleware is being executed after the HorizonMiddleware, so if we first try to clear/delete the session in HorizonMiddleware, then the SessionMiddleware processes the response, detects that is has been modified and tries to save it. This creates a new record in the DB with the sessionId of the logged-out user. So we need to process the response after the SessionMiddleware, so the proposed change is to add a CleanupMiddleware, that does the cleanup.

I've tested with signed_cookies and db - no visible drawbacks or problems. The tables seems clear after several logins, logouts, clean browser sessions.

A positive side-effect of the patch: no session cookie is being set in the response for anonymous users. In the current codebase - after an unsuccessful login, a cookie with the session_id is being returned to the users, thus exposing the SESSION_COOKIE_NAME to the user even if they don't have a valid account for the system. Now - until the user logs in for first time - no session cookie is being returned. When the users logs out - the Session cookie is being removed from response, so browser seems clear.