[drupal-devel] [feature] Avoid session hijacking
nsk
drupal-devel at drupal.org
Mon Aug 1 01:16:50 UTC 2005
Issue status update for
http://drupal.org/node/19845
Post a follow up:
http://drupal.org/project/comments/add/19845
Project: Drupal
Version: cvs
Component: base system
Category: feature requests
Priority: critical
Assigned to: chx
Reported by: chx
Updated by: nsk
Status: patch (code needs review)
Just wanted to point out that it is possible for a user to change their
user agent string, sometimes quite often. I have done this before for
compatibility purposes (Konqueror -> Mozilla or MSIE). Users may also
be logged in from many processes that use the same user agent string,
as I do many times or from many different browsers at the same time (I
do this too).
nsk
Previous comments:
------------------------------------------------------------------------
Fri, 01 Apr 2005 23:50:15 +0000 : chx
Attachment: http://drupal.org/files/issues/bindip.patch (896 bytes)
This would make session hijacking more than a bit harder.
The code can be compacted even more, but I did not dare.
------------------------------------------------------------------------
Sat, 02 Apr 2005 00:51:28 +0000 : danielc
IP's can change during a session. So, this isn't a good idea.
------------------------------------------------------------------------
Sat, 02 Apr 2005 01:23:17 +0000 : chx
I read a Zope coders' thread [1] on this, and they proposed it as
optional, but on as default. So, admin/settings? Or -- and I'd prefer
this one -- settings.php?
[1] http://mail.zope.org/pipermail/zope-coders/2004-October/005239.html
------------------------------------------------------------------------
Sat, 02 Apr 2005 02:18:46 +0000 : Allie Micka
The concern over transient IPs is only going to get worse as time goes
by.
You've got your load-balanced proxy servers, dropped-and-restored
dial-up connections (yes, people still do use dial-up!). plus there
are all those laptops and handheld devices accessing various wireless
and wired networks throughout the day.
On an average day, my laptop accesses the internet from no less than
3-4 different IP addresses, and I would get right feisty if I kept
losing my Drupal session every time. I work with a lot of people who
administer Drupal sites but aren't that technically adept. If they had
a problem with feisty laptop owners I would want them to be able to
change the settings easily, which means that the settings should be in
admin.
Many ISPs and most corporations use some kind of NAT, which means that
binding to IP addresses isn't that effective anyway. True, you limit
the number of clients that can use a session by restricting to IP - but
I'm more concerned about my coworker impersonating me than I am about a
random stranger lucking out with my session_id. So restricting by IP
causes problems without really fixing any real ones.
One thought is to bind the session to USER_AGENT. It is still
guessable and spoofable, and certainly not perfect. But it does not
change for at least as long as the user keeps their browser open and
can vary quite significantly (browser, plug-ins, revision/build, OS,
etc.). It has many benefits over using the IP, with the only real
trade-off being that it is easier to spoof.
------------------------------------------------------------------------
Sat, 02 Apr 2005 14:57:30 +0000 : kbahey
I agree that this should not be included as a standard features.
Entire ISPs and even countries are behind proxies that could change the
IP address within the same session. This would cause havoc for those
behind such proxies. They would not be able to have a meaningful Drupal
session at all.
-1 for that reason.
------------------------------------------------------------------------
Sat, 02 Apr 2005 15:33:11 +0000 : chx
Attachment: http://drupal.org/files/issues/session.inc_3.patch (686 bytes)
So be it.
------------------------------------------------------------------------
Sat, 02 Apr 2005 15:33:50 +0000 : chx
Forgot to change the title.
------------------------------------------------------------------------
Sat, 02 Apr 2005 15:43:32 +0000 : kbahey
That is more like it.
I can't think of a case where the user agent would change between
sessions.
I think some corporations mask the referer as a security/privacy
measure, and perhaps the user agent too. But even if they do so, they
would not change it mid session.
+1 on this feature/patch.
------------------------------------------------------------------------
Sat, 02 Apr 2005 18:41:27 +0000 : chx
Attachment: http://drupal.org/files/issues/session.inc_4.patch (1.14 KB)
For logged in users, SID is changing on every page load. Hijack this.
------------------------------------------------------------------------
Sat, 02 Apr 2005 19:09:10 +0000 : kbahey
The idea is good.
However, this introduces two extra SQL queries per page load per logged
in user.
One is an INSERT and the other is a SELECT.
Is this overhead acceptable for large sites? How would it impact
performance?
Needs to be quantified/benchmarked.
------------------------------------------------------------------------
Sat, 02 Apr 2005 19:41:12 +0000 : chx
Attachment: http://drupal.org/files/issues/session.inc_5.patch (1.91 KB)
You mean, a DELETE and an INSERT? Well, let's see this version... still
a DELETE plus for regged user, but for new anon sessions we have only
one INSERT instead of an INSERT and an UPDATE. In sess_write, UPDATE
happens only for anonymous users who had a session before. For those
without a session, and this includes new anon sessions and every logged
in user, an INSERT happens. This made the INSERT in sess_read
unnecessary.
------------------------------------------------------------------------
Sun, 03 Apr 2005 11:48:40 +0000 : Hivemindz Magazine
It is a good idea. but wouldn't it be easier and more accomadating to
use a an md5 key (base it on something hidden like the email address) ?
Say set a key into the session id and set a cookie with the key in it
and compare them on page load?
------------------------------------------------------------------------
Sun, 03 Apr 2005 16:26:06 +0000 : chx
This patch just makes a bit harder to steal SIDs.
Bart on #drupal said that someone steals my SID, immediately requests a
new page, gets a new SID with my rights and upon the next page load, I
will see that I got logged out -- we could issue a warning but I am not
sure that this always means that my rights are abused.
This can be clearly detected if we change the sess_destroy()<code> call
before the <code>session_regenerate() to a _sess_invalid call and check
whether someone tries to reuse an invalid SID.
Or if someone continously sniffs my traffic, the last SID when I finish
my admin work will be usable for the session cookie's lifetime.
How far do we want to go with this?
One thing is certain -- if you are using trans SID, something like this
patch is a must.
------------------------------------------------------------------------
Mon, 11 Apr 2005 23:36:59 +0000 : degerrit
Attachment: http://drupal.org/files/issues/session.inc_ua_various.patch (1.34 KB)
I was looking for IP address binding to the session after discovering my
site was 'vulnerable' to this (and remembering seeing PHPSESSID revealed
in a URL a while ago), but I now saw this thread above and see IP
addresses change and will cause problems. I don't think we should rely
solely on PHPSESSID though - future bugs could still accidentally
reveal this in a URL.
This is an interesting read BTW: Notes on PHP Session Security [2]. It
seems Drupal probably has an issue with pre-hijacking, which is
obviously fixed by the patches changing SID every page load. I haven't
tested those, but won't they cause problems for the Back/Forward
browser buttons?
My patch is based on the 3rd patch above - "Bind User Agent to session"
but adding a few other user-agent variables for added security (and
because UA is often logged in server logfiles and these variables
usually aren't): HTTP_ACCEPT_CHARSET, HTTP_ACCEPT_LANGUAGE,
HTTP_ACCEPT_ENCODING, HTTP_CONNECTION. I've experienced problems with
IE6 and 'HTTP_ACCEPT' changing, so I removed that one.
Maybe session security should be configurable in a number of levels to
let the admin balance security vs usability:
1) standard security (as is, perhaps generating a new SID after login
though)
2) medium security (only check user agent, should be pretty safe)
3) medium-high security (check user agent & other agent variables)
4) high security (changing SIDs like chx's last patch)
5) paranoid security (also check IP address)
[2]
http://www.google.com/search?q=cache:DUytxcJp06YJ:www.sitepoint.com/blog-post-view.php
------------------------------------------------------------------------
Tue, 12 Apr 2005 15:37:52 +0000 : kbahey
Seems like a good approach to take in general.
Also, session hijacking because of SESSIDs visible in the URLs should
be much less of a problem with 4.6 than it is now, because of the
changes in settings.php, where session.use_only_cookies is set to 1.
------------------------------------------------------------------------
Tue, 12 Apr 2005 15:41:12 +0000 : kbahey
Regarding having a settable security level, I agree with this approach.
Some site admins may have different needs and audience, and hence may
want to set it to a higher or lower level than the rest of us.
Having three settings, with descriptions, should be satisfactory,
rather than finer granularity with more options.
I hope this does not get shot down as 'too many options are confusing'.
If it does, then we can set that in settings.php then.
------------------------------------------------------------------------
Tue, 12 Apr 2005 19:53:57 +0000 : degerrit
This article was linked to from the previous article I posted and is
also very interesting: Chris Shiflett: The Truth about Sessions [3].
The author disagrees that regenerating the session identifier every
page adds security and thinks it should only change "whenever there is
a change in privilege level". I can't say I completely understand it,
but it would solve the performance issue.
[3] http://shiflett.org/articles/the-truth-about-sessions
More information about the drupal-devel
mailing list