Expired sessions create NullSession

[USSRLivesOn]

According to the Flask SessionInterface documentation, a NullSession is created if open_session returns None. However, KVSessionInterface returns None for expired sessions. Shouldn't it return a new instance of KVSession instead? Currently, it seems like an expired session results in NullSession being created.

[USSRLivesOn]

Also (and I may be wrong on this), shouldn't session expiration be calculated against last modified datetime and not original creation datetime? Currently, it seems like all sessions will expire PERMANENT_SESSION_LIFETIME from creation, even if they were modified more recently.

[mbr]

Woups, I sort of lost this issue in a sea of email. Sorry. I do think you're right though:

• From glancing over it, this seems like a bug, in some circumstances a quite severe one. If a users session expires, a NullSession will be created which will cause a server error (since it just complains about the missing SECRET_KEY). However, this might be well masked by the fact that the cookie with the session id will expire at the same time, so the only way to trigger this error right now is when there is a significant time difference between the users system clock and the server time.
• The fact that session expiration is not refreshed is "by design". It's not the optimal course, but it is a trade-off: To refresh the session, there would have to be a new session written every time the user makes a request (since it is encoded in the key). I've played around with an implementation that refreshed the session once half the expiration time had passed to limit this cost, but I'm not sure if it made it into the code at the end. Finally, "losing" a session after the expiration should not be the end of the world, usually, a user would simply have to login again. When doing something "critical", you can manually call regenerate() to fix this issue - or call it on every request with a session.

If you want to write patches for some of these issues, you're welcome. The first issue is not that severe as it will affect only malicious users, users with a broken clock or very, very unlucky people. However, for the latter two it's harsh, as they won't be able to do anything anymore until their clock catches up. Writing a patch here is somewhat simple, but needs to be accompied by tests, as its pretty core to the extension.

The second issue is less easily solved. It always involves the trade-off of writing the session many times over without need - especially if it's only used for a login token.

[sharathpuranik]

One simple solution here would be to regenerate the session id since that is used to calculate the expiration time and maintain the same session values as earlier. I have written a patch for this, but its very annoying to reproduce this bug, hence no test case as of yet.

Setting session.permanent on an expired session also causes the same behaviour, where in NullSession is returned and everything breaks. Which means that any kind of manipulation of an expired session will break the flask application because open_session module (called on every modification) has the expiry check inside.

Here is a simple stack trace of what happens when an expired session is made permanent.

session.permanent = True

werkzeug/local.py in <lambda>
    __setattr__ = lambda x, n, v: setattr(x._get_current_object(), n, v)
flask/sessions.py in _set_permanent
        self['_permanent'] = bool(value)
flask/sessions.py in _fail
        raise RuntimeError('the session is unavailable because no secret key was set.  Set the secret_key on the application to something unique and secret.')

[mbr]

I'm still on the fence whether or not it is desirable to be able to make an expired session permanent (and valid again) or allow any kind of manipulation. Under bad circumstances, this could result in a security risk (i.e. a possibly stolen, expired session suddenly becomes valid again).

Maybe a better Exception is the way to go? Usually I'm also okay with things blowing up for a user that managed to trigger this bug, if that means more security for everyone else, as long as it only happens once in a blue moon. How are you triggering this on a regular basis?

[sharathpuranik]

How are you triggering this on a regular basis?

I had gone through the code and had seen that session expiration would cause a major problem for users. So what I did was I set the PERMANENT_SESSION_LIFETIME for my Flask app to 3 days. Then, I created around 10 different sessions using multiple machines and browsers and set all of these sessions to permanent causing them to set expiry at three days. Then, I waited... I tried to manipulate all of these after 3 days and every single one of them now started giving me an error saying session is not available because secret key is not set or something similar.

According to me there are three solutions to this which do not blow up in the users browser.

• Create a completely new session and deleting the current expired one entirely to prevent any kind of security leak from happening.
• Regenerate the session with a different ID but maintain the same values stored in the session (Like you mentioned this can have security implications in case of stolen and expired sessions)
• Change the way session expiry is calculated. Just using the creation time is a not a good technique like what USSRLivesOn has mentioned previously in this thread.

Personally, I think the best way to resolve this problem is to change the way expiry of session is checked. Last modified time or last access time would be a better solution.

[mbr]

I just double checked what Flask's own session implementation does, and it seems to simply create a new session if a permanent session expires. It does now, however, auto-refresh sessions by regenerating them, if they are not modified, but does this when they are altered (as the hash has to be recalculated anyway).

These are two different issues. The creation of an empty session if an expired session is encountered, i.e. treating an invalid session like "no session", is how it should be, this is clearly a bug that I'll fix right now.

Whether or not sessions are refreshed upon alteration is another matter, I'll look into that again later on.

[mbr]

There's a fix for what I believe to be your issue in 0d9c776380f49008a2042e6c644aed5b3c036938

The main problem seems to be that you cannot treat any session in Flask-KVSession as limited to a browser session. There simply isn't any way for the server to know if the user has closed the browser, so every session gets an expiry timestamp on the server. The assumption is simply to hope that the user closes his browser before the permanent session lifetime ends.

Similiar behavior can also be triggered by clock mismatches. However, I've made it so that these errors are silently dropped now, replaced by a new session.

Please check if you run into the same problems with the latest master branch commit.