search

pallets-eco / flask-session

Server side session extension for Flask
https://flask-session.readthedocs.io
BSD 3-Clause "New" or "Revised" License
490 stars 236 forks source link

Increase session identifier entropy & reduce bandwidth a little bit by using secrets module #197

Closed yrro closed 5 months ago

yrro commented 8 months ago

In reviews of a number of systems we have that use Flask-Session the way that session IDs are generated has been raised as an issue.

I'm asked for evidence that the session ID is generated with "sufficient randomness".

Here's what Flask-Session does:

https://github.com/pallets-eco/flask-session/blob/09094d30cfa1aace46f93a192260940bf95cf97b/src/flask_session/sessions.py#L62-L63

  1. v4 UUIDs have 122 bits of randomness.
    OWASP says "session identifiers should be at least 128 bits long to prevent brute-force session guessing attacks".

  2. Does uuid.uuid4() use a cryptographically secure PRNG?
    In Python 3.11 it uses os.urandom which is documented to be "suitable for cryptographic use".
    Sufficient, but this explanation tends to generate a lot of paperwork and time spent reviewing my explanation.

Both of these points can be addressed by replacing the call to uuid.uuid4() with a call to secrets.token_urlsafe(128//8), which returns a string in modified base64 encoding such as "u2sDdiy9v9ZdCrwb3-b3rb5m5o4C01JYgnS72biDcCA". This is 22 characters long, so we also save a bit of bandwidth by moving away from the 36-character long string representation of a UUID.

Lxstr commented 8 months ago

Hi @yrro thanks for the issue. I'm a junior contributor here, but this seems like a no brainer for my eyes. I'll try to get this one through with the team.

Just for background, we're going through a bit of a drop in momentum. A few months ago we managed to get this repo over from somewhere unmaintained and still need to do a lot of work merging a previously popular fork. It's a pretty huge task.

yrro commented 8 months ago

Thanks for taking a look. I opened a pull request with the proposed change at https://github.com/pallets-eco/flask-session/pull/198 if that's helpful. With the Flask-Session quick start demo it gives the result:

$ curl -i localhost:5000/get/
HTTP/1.1 200 OK
Server: Werkzeug/3.0.1 Python/3.11.2
Date: Fri, 03 Nov 2023 12:29:11 GMT
Content-Type: text/html; charset=utf-8
Content-Length: 7
Set-Cookie: session=FXRQglhzkeIFPon8Y80apw; Expires=Mon, 04 Dec 2023 12:29:11 GMT; HttpOnly; Path=/
Connection: close

not set

session cookie value looks good to me.

Lxstr commented 5 months ago

Thanks again for the issue, updated in 0.6.0.