Should the Django session-id be hashed?

[Rigel]

Hello!

The Django session framework stores session-ids in the database as
plain-text. Unless I'm missing something, these ids could be
vulnerable to SQL injection attacks, if any are discovered or if
developers misuse features like extra(). This vulnerability could be
mitigated if the session-ids were hashed with a secure cryptographic
hash function, like SHA-256, before storing them or querying for them
in the database.

This concern has recently been raised for Joomla! on the Full
Disclosure mailing list:
http://seclists.org/fulldisclosure/2016/Sep/50

What is your opinion on this matter? It could be fairly trivial to
implement, with the only side effect of being computationally
expensive. Still, security is more desirable than efficiency or
performance.

Rigel.

[Curtis Maloney]

On 22/09/16 18:52, Rigel wrote:
> Hello!
>
> The Django session framework stores session-ids in the database as
> plain-text. Unless I'm missing something, these ids could be
> vulnerable to SQL injection attacks, if any are discovered or if
> developers misuse features like extra().

Firstly, extra() is on is way out... being replaced by expressions,
transforms, and so on...

That said, yes, extra() does potentially open you to SQL attacks, but
only if you use it incorrectly. If used as documented -- same goes for
raw() -- you should remain immune to SQL injection.

The only time you're likely to become vulnerable to SQLi is when you do
something as stupid as putting values into SQL commands yourself.

> This vulnerability could be
> mitigated if the session-ids were hashed with a secure cryptographic
> hash function, like SHA-256, before storing them or querying for them
> in the database.

They're just a random string, I don't see how turning them into another
random string will help? Or do you mean to set the original string in
the cookie only, and hash them for the key, and hash them _every_ _time_
you look up the session?

> This concern has recently been raised for Joomla! on the Full
> Disclosure mailing list:
> http://seclists.org/fulldisclosure/2016/Sep/50
>
> What is your opinion on this matter? It could be fairly trivial to
> implement, with the only side effect of being computationally
> expensive. Still, security is more desirable than efficiency or
> performance.

You are right there, security is more important.

However, it's a small overhead for everyone... for a small win for
almost nobody.

Until you can demonstrate how there's any SQLi vulnerability, I'm -1 on
this.

--
Curtis

[Florian Apolloner]

On Thursday, September 22, 2016 at 1:26:19 PM UTC+2, Violet Gibson wrote:

Unless I'm missing something, these ids could be
vulnerable to SQL injection attacks, if any are discovered or if
developers misuse features like extra().

Same is true for literally any field a user can write into.
 

What is your opinion on this matter?

Not worth it imo,

[Alex Gaynor]

If Django were a different framework, I'd probably think this was a reasonable idea. However, Django's ORM is _incredibly_ good at deterring SQL injection. In many many years of using and reviewing Django applications, SQL injection is vanishingly rare in my experience; therefore I think this adds complexity for limited gain. Another relevant factor is that this is only applicable to the database sessions backend.

Alex

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/3c8e5034-b0ed-4dbc-b60c-a2f9b280926e%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--

"I disapprove of what you say, but I will defend to the death your right to say it." -- Evelyn Beatrice Hall (summarizing Voltaire)
"The people's good is the highest law." -- Cicero

GPG Key fingerprint: D1B3 ADC0 E023 8CA6

[Anthony King]

I have noticed that session id's are included in Django debug emails, with no clear way to filter them out. I'm unsure of the behaviour with 1.9+, but this is what I've experienced with 1.8.

The way around that issue though is to sign the cookie, so that people can't just drop the session-id in.

--

You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.

[Rigel]

On Thu, Sep 22, 2016 at 12:31 PM, Curtis Maloney <cur...@tinbrain.net> wrote:
> They're just a random string, I don't see how turning them into another
> random string will help? Or do you mean to set the original string in the
> cookie only, and hash them for the key, and hash them _every_ _time_ you
> look up the session?

I'm an attacker and I've found a way to read the session database
table. I can now impersonate user Bob.

If the session-ids were hashed, I would need still need to know's
Bob's session-id. Django woudn't store it anywhere on the database.

Rigel.

[Erik Cederstrand]

> Den 22. sep. 2016 kl. 13.38 skrev Alex Gaynor <alex....@gmail.com>:
>
> If Django were a different framework, I'd probably think this was a reasonable idea. However, Django's ORM is _incredibly_ good at deterring SQL injection. In many many years of using and reviewing Django applications, SQL injection is vanishingly rare in my experience; therefore I think this adds complexity for limited gain. Another relevant factor is that this is only applicable to the database sessions backend.

The attacker would only need to read access for this to work, not write access. That could possibly be achieved that even without SQL injection. If the attacker can just put another person's session ID in her cookie, then session IDs are basically passwords. Passwords should not be stored clear-text. The only difference is that session IDs are more short-lived than passwords.

It's the same issue with API key authentication for REST APIs. Not many people remember to hash the keys before storing them in the DB.

If the attacker gains write access to the DB, then you're doomed anyway, hashes or not. The attacker just makes up her own session ID, hashes it and writes it to the database. Or makes up her own password and writes it to the Users table.

Erik

[Tim Graham]

The idea of adding an option to store the session ID hash rather than the ID itself was discussed a few years ago on the core team mailing list (see the "Authentication best practices" thread and "Don't store session IDs in the clear" in the security issue tracker). Maybe we can reproduce some of that thread here if it helps. The conclusion was to create this ticket: https://code.djangoproject.com/ticket/21076

[Rigel]

Thanks for ticket link.

Would you mind if I assigned it to myself? I have a few ideas on how
it could be put together, and I'd like to work on it tonight and
submit a proposal.

Rigel.

> --
> You received this message because you are subscribed to the Google Groups
> "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send an

> email to django-develop...@googlegroups.com.
> To post to this group, send email to django-d...@googlegroups.com.

> Visit this group at https://groups.google.com/group/django-developers.
> To view this discussion on the web visit

> https://groups.google.com/d/msgid/django-developers/e74901a8-ba03-4452-b29c-18f95e4e6f67%40googlegroups.com.

[Tim Graham]

Sure, go ahead.

[James Bennett]

For what it's worth, I'm suspicious of threat models which begin with "assume the DB has already been significantly compromised..." simply because there's so much someone can do if they gain even read access that it's not worth expending a ton of effort hardening Django against those cases.

Similarly, "assume a SQL injection vulnerability already exists" doesn't do much for me because for something that lives and dies by the DB, that's very close to "assume some remote-root vuln already exists and has been exploited".

So personally I'd like to hear some more about why this is seen as necessary before I'd endorse work to actually implement it.

--
You received this message because you are subscribed to the Google Groups "Django developers  (Contributions to Django itself)" group.

To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.

Visit this group at https://groups.google.com/group/django-developers.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAD9P0Jr8uKogoeUz%3Dx1CHpC0hKQ4qyt5DVQo7vx-rZka7pYVyQ%40mail.gmail.com.

[Aymeric Augustin]

On 22 Sep 2016, at 20:32, James Bennett <ubern...@gmail.com> wrote:

> So personally I'd like to hear some more about why this is seen as necessary before I'd endorse work to actually implement it.

The reason why I originally filed a security report is that session stores tend to have less focus on security than databases.

Of course this is a moot point when sessions are stored in the database, but I won’t start a debate about why Django still encourages this, this isn’t the point of this thread ;-)

For example Redis is well known for advertising that it has no security and should only be run within a secure network. (Defense in depth, anyone?) Still a bunch of companies provide Redis as a service, usually on random EC2 instances directly reachable from the Internet. The best ones require going through an SSL endpoint and providing a password, but an attacker can still talk directly to Redis, which is concerning given its stance on security.

In contrast, the authors of PostgreSQL have implemented an authentication and authorization framework. I’m not qualified to say if it’s robust, but at least it’s better than shrugging off security entirely.

--
Aymeric.

[Chris Griffin]

Hi Everyone,

I took a stab at implementing this. I'd appreciate any feedback on the PR. The 8tracks leak over the weekend highlights the importance of hashing session ids. The attacker only gained access to backups of the database. AFAIK with the current version of Django, the attacker would've been able to login as any user that hadn't been logged out by simply setting a cookie. By storing the session ids as hashes, we can effectively mitigate this attack vector.

Thanks for the feedback,

Chris