mbr / flask-kvsession

A drop-in replacement for Flask's session handling using server-side sessions.
http://pythonhosted.org/Flask-KVSession/
MIT License
168 stars 53 forks source link

Security concern of re-using session #31

Closed slick666 closed 9 years ago

slick666 commented 9 years ago

I've implemented this using 0.6.2 and realized that the session is preserved across logins. This is a security concern because over the time that the session is valid the session alone may be valid and re-played.

Ideally I would have a non-repeating key for each logged in instance, or the session would get deleted upon log-out.

mbr commented 9 years ago

I'm a little confused about what you mean here exactly. There is no concept of a login inside flask-kvsession.

Also, I am not sure what "this" is =).

Could you provide some sample code to show what you mean?

slick666 commented 9 years ago

Sorry for my brevity I'll try to expand on it a little more. I see this issue tangled with flask-login so perhaps my question is miss-placed.

After setting up KVSession I have session ids handed out to user's session when the come to the site. when they log in, their session is now authorized to see protected views. When the user logs out they are no longer able to see the protected views but the session is maintained. I can log back in and the session is still the same.

The edge case I'm looking at is if I steal that session id, I can gain access to another machine so long as they are logged in. With the session id being maintained, a malicious user who has that session may gain access if the real user logs back in. The behavior that I want is that if the session id is captured and the user is logged out, that the session id can never be used to log back in again. Once the user logs out that id is never given access again.

My workaround is to completely delete the session upon logout. Ideally I'm thinking I would like to have a non-repeating unique id for the logged in sessions. Almost like having two sessions.I'm not seeing any discussion of this out on the internet so I'm curious if I'm implementing this wrong somehow.

mbr commented 9 years ago

After setting up KVSession I have session ids handed out to user's session when the come to the site. when they log in, their session is now authorized to see protected views. When the user logs out they are no longer able to see the protected views but the session is maintained. I can log back in and the session is still the same.

Flask-Login, Flask-KVSession and your app use slightly different approaches here, you just need to reconcile them.

You are storing authorization ("is allowed to see X") and authentication ("logged in as U") in the same session. As a result, the same information ("X can see U") is found in two different places:

  1. In your database (I assume it comes from there) there's some sort of record of "allow U to see X"
  2. In the session there is now "the owner of this session is allowed to see X".

If you change one or the other, these go out of sync. This is bad. If the administrator revoked U's priviledges to see X, they change would not be effective as long as U is able to hold onto his session.

It works the other way around as well (session gets altered), but that case is not very likely.

A solution to your problem is simply not storing any authorization-information in the session. 9 out of 10 cases you do only want to store one thing inside the session: The username. When a user requests a view that is access restricted, check every time against the database (or whereever the definite location for this piece of information is), not the session.

There are some use cases where you would want to keep extra information inside a session. An example would be a shopping cart - you can fill it up without being logged in and it carries over to your new account. But these are specific cases.

The edge case I'm looking at is if I steal that session id, I can gain access to another machine so long as they are logged in. With the session id being maintained, a malicious user who has that session may gain access if the real user logs back in. The behavior that I want is that if the session id is captured and the user is logged out, that the session id can never be used to log back in again. Once the user logs out that id is never given access again.

You've almost identified the initial sole reason for flask-kvession's existance =). What you're describing is simply stealing a cookie and it is a widely known problem. The issue with Flask's original client-based sessions is that they cannot be invalidated easily - it's the price you pay for the incredible light weight.

The "horror" scenario is: User gets his cookie stolen, now the attacker can do everything the user can do. When using client-side sessions (like Flask does), even if the user logs out, only his cookie is lost, the attacker can still do whatever he wants.

With flask-kvsession, you can actually destroy the session, which you should do upon logout. That removes the session on the server side, it will not be found again and the attacker is logged out.

There is also the danger of session fixation. If the user later on (or initially) logs in and the attacker has already stolen the session id, the attacker's session will get "upgraded" as well. An example:

To rectify this, the session id should be regenerated (see below).

My workaround is to completely delete the session upon logout. Ideally I'm thinking I would like to have a non-repeating unique id for the logged in sessions. Almost like having two sessions.I'm not seeing any discussion of this out on the internet so I'm curious if I'm implementing this wrong somehow.

Your "workaround" is the correct way to use it. You only need to think about these things if you have data other than the username that you need to persist. Normally there should not be any.

Here's how the process is supposed to go (no Flask-Login here at this point):

  1. User visits site, gets a session ID if needs one (usually does not need one, since you're not storing anything but the username in it).
  2. User logs in. You do not know if there has been a session already or not, but it is simple to regenerate is. This will ensure a new session id is created and sent to the client, containing any possibly old session data. Note that if a cookie is stolen here, there is simply nothing you can do.
  3. After a while, the user logs out. destroy the session to make sure potentially stolen sessions are invalid as well.

Here's how it still works if everything goes wrong:

  1. User's browser is buggy and keeps the stale session cookie around. The session is empty (destroyed) on the server side, so if he presents with it, the server can either generate a new one (new session id), which is fine or create a blank session with the old id (bad idea) or just throw an exception (secure, but not user friendly. This used to be a bug, which I hope is fixed now =).

Security wise, generating an empty session is the worst case, so we'll go with that:

  1. The attacker somehow knows the users session ID and now the user logs in with his old session id. But since regenerate is called after login, only the user gets the new session ID.

I hope that clears things up.

And the actual bug here is: destroy() and regenerate() are not documented. They are in the source, but don't show up on the official documentation sites. So, until I fix that, here's


from flask import session

# ...

# after login succeeded:
session.regenerate()
session['username'] = ...

TODO:

slick666 commented 9 years ago

Thanks for your thorough feedback. I did find destroy after some looking but it doesn't seem to be that common. To address some of the points you brought up

You are storing authorization ("is allowed to see X") and authentication ("logged in as U") in the same session. As a result, the same information ("X can see U") is found in two different places:

In your database (I assume it comes from there) there's some sort of record of "allow U to see X" In the session there is now "the owner of this session is allowed to see X".

If you change one or the other, these go out of sync. This is bad. If the administrator revoked U's priviledges to see X, they change would not be effective as long as U is able to hold onto his session.

This is partially true and you raise a valid point. In my particular case I passing my user's credentials to a authentication service and it's returning but my username, user info, and user groups. I've extended the flask-login with some assumptions to be able to pass groups to the @login_required decorator so I can say this view required a login and requires group X. This way the data is only store in memory for the period of time that the session exists

The notion of authentication revocation is there but the API is still in flux so for a workaround I've limited the session lifetime to a period that is acceptable. Ultimately there will be a background process listening and dropping auth sessions should there be a credential change. Thereby closing the loophole where a user's credentials are dropped but the session remains active.

In your answer I see you're calling the destroy function but what is the purpose of the regenerate()? Assuming the user logs out and the session is destroyed when the users comes back the user will have to generate a new session. wouldn't regenerate() be double generating the session id?

mbr commented 9 years ago

In your answer I see you're calling the destroy function but what is the purpose of the regenerate()? Assuming the user logs out and the session is destroyed when the users comes back the user will have to generate a new session. wouldn't regenerate() be double generating the session id?

regenerate() prevents session fixation. This is a larger problem if your sessions are broken in that they allow session IDs to be passed as url parameters. If I remember correctly, this is not possible with Flask, but imagine it would for a second:

  1. Mallory (our attacker) generates a session on yoursite.com, for example by filling his shopping cart with an item. His session ID is MLRY.
  2. He sends Alice (our user) a link: "Checkout the sale on https://mysite.com/widgets?SID=MLRY".
  3. The server now uses the same session for Alice as well, MLRY.
  4. After a while, Alice logs in.

At this point, if we do not regenerate the session, Alice will be elevating Mallory's session with her credentials - he can then use her stored credit card info to order things at his leisure. However, if we regenerate the session ID, we get a new one only sent to Alice (note the use of https).

There are other benefits of regenerating the session id. It is also possible to destroy the session and recreate it after login. The difference is marginal; after all, if nothing is in the session, regenerating an empty session amounts to the same.

mbr commented 9 years ago

I should add that regenerate() effectively generates a new session id and then renames the old session server side. So yes, we're generating 2 sessions IDs.