Avoid session hijacking

IP's can change during a session. So, this isn't a good idea.

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.

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.

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.

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.

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?

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. 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)

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.

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.

This article was linked to from the previous article I posted and is also very interesting: Chris Shiflett: The Truth about Sessions. 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.

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).

The discussion here and elsewhere has shown that none of the solutions presented are generally usable. this might be different in a controlled setting, but if you have that you can just encrypt your communication.

While discussing about the recent session fixation fix in the security box, Karoly made me aware of this issue, which I was unable to find. I opologize for that.

I'm reopening the issue because IMO the current method of not providing any kind of protection against session hijacking attacks exposes the majority of Drupal based sites. Even if you encrypt sessions, you are still open to this kind of attacks, if an XSS door is open, specially on sites where non 100% trusted 3rd parties can post content (forums, blogs, comments, etc).

Sooner or later, someone will get hacked. All you need is a PHPSESSID of a logged in user, which can be obtained via methods that Drupal cannot 100% control (bugs in browsers, contrib modules or even in core might be there, or even a simple image posted on the victim site, but hosted where the attacker has access to logs). However, there is something that could be done to minimize the risk.

I think that this issue was on the right track, specially comment #13. If not a forced method, then let site owners choose. SSL might not be an option for many sites, that are otherwise exposed.

That's phpBB2 code. Here's the story:
http://www.phpbb.com/kb/article.php?article_id=54

And here some references on the subject:
http://www.owasp.org/documentation/guide
http://www.hardened-php.net/index.48.html
http://www.hardened-php.net/php_security_guide_considered_harmful.51.html
http://www.acros.si/papers/session_fixation.pdf
http://phpsec.org/projects/guide/4.html
http://www.cgisecurity.com/articles/xss-faq.shtml
http://ha.ckers.org/xss.html

It seems no one cares.

Sorry, maybe I should have commented in the first instance. I try not to throw my newbie opinion around.

I read the first article link (on phpbb.com) and it was very interesting. I have to use Yahoo Groups (don't ask) and they request your password continuously. People are getting used to it (IMO). If I could configure Drupal to request a password every time the first 12 digits of the user's IP changed, I would often do it.

Perhaps 8-16 digit checking should be standard, with no way to simply "switch" it off. If someone didn't like it, they could use a hack to turn it off, but doing so would absolve the Drupal developers of responsibility in this area.
Simon

-1 on all the proposed solutions.

Get a SSL certificate for your server. Make a mod_rewrite condition to redirect all port 80 requests to https://.

RewriteRule ^/?(.*) http://example.com/$1 [R=permanent,L]

...and/or make yourself a contrib module that asks for your password (via SSL) every time you want to POST (think about shopping on Amazon.com). If you're not encrypting all the traffic, eventually you're going to transmit enough information over the open line to get your session hijacked. Any solution to prevent that is going to make using the site undesirably inconvenient.

...and does this really have to be marked critical?

I agree that it's not critical, since right now it's a possible hole, not a gaping hole. However, I do believe we could do more here. Direct IP tracking for the session is too tight for some broken ISPs. Subnet tracking is probably safe. So what's wrong with an optional admin-toggleable feature to check against the first three or first two octets? It seems like a single checkbox would solve the problem here.

Crell: Probably a fine feature for Drupal 4.8.

SSL doesn't prevent cross-site scripting attacks, which is one of the methods that can be used to hijack a session id.

Also, the truth is SSL is not an option for the majority of sites and, while you think about it, Drupal is open to session hijacking. Period.

IMNSHO session hijacking is a general apache/PHP problem.

True, it's a problem. That doesn't mean there is nothing one can do to minimize the risk. See references posted above.

Since this issue has not been closed, yet? ...I'm assuming there is still a chance to get something into core, so...

The current session fixation prevention (added to user module) does not cover contrib modules. ie. if not already fixed, logintoboggan module was vulnerable to this kind of attacks... because it duplicates a lot of code from user module, so it needs to be in sync with any security method implemented in core, but that may not happen during some time...

IMHO, session id regeneration ought to be moved close to session_start(), in bootstrap.inc. So that will cover all possible methods of user login, the one implemented by user module, but also any other contrib module.

In addition to that, it is possible to add a certain level of prevention of session hijacking attacks, by creating a unique fingerprint attached to session data and checked during subsequent requests. This session fingerprint can be built, depending on admin settings, based on user agent, user IP (with the option to choose how many bits to take into account), a unique site identifier and maybe more...

Since it is necessary to, at least, patch bootstrap.inc, it is not possible to implement this kind of protection through a contrib module.

So... anyone interested in something like this?

If you just chose a sensible default (to not introduce yet another mostly useless variable) and the code change is small and it is well tested and it is approved by the security team...

I talked about this issue with the security team since January (through private channels), so while I can assume a certain interest to provide a fix (this issue has not been closed, yet), I do not expect the security team to listen to me much. They (you) know my approach already, but I have not been feeling good with the security team's response, being one of the reasons (there are a couple more), the security team forgot about this thread when I reported this issue to them, until recently. I felt like a dumb when I saw it, making me wonder (once more) why do I use Drupal. Call me paranoid, but I can't believe such a vulnerability has not been fixed already (only a partial patch has been provided since 4.6.6/4.7-beta6, however the security team knew about this issue, at least, since April 2, 2005 when this thread was opened). I'm tired of showing references to stuff related to session hijacking and fixation documents written by security experts and consultants. I've been coding since 1986 with a lot of different enviroments (from mainframes to PCs) and playing with PHP and involved in other open source projects since 2003 (learning everyday, helping other users, coding and sharing). I do not expect for you to take into account my background (this is relative anyway, I agree), however you have made me feel like a Drupal user here, so I expect the security team to know about best practices related to PHP security and provide a fix for this issue, not me.

I still use Drupal, so I've been constantly evolving my own method to prevent from this kind of attacks. When I said "anyone interested in something like this?", it was a question mostly directed to other Drupal users, maybe because I somehow think only more users can convince the security team to spend a bit more time in such a thing.

So, now that you've had your rant, do i get to see a patch or not?

ok, here we go.

You have to upload the attached file to your includes directory. Be sure the file is named 'session_protection.inc'. This is to make it easy to test. The code could be moved to session.inc once it is stable.

Next, you have to modify bootstrap.inc like this.

Open includes/bootstrap.inc and find:

      require_once './includes/session.inc';
      session_set_save_handler("sess_open", "sess_close", "sess_read", "sess_write", "sess_destroy", "sess_gc");
      session_start();

After that, add the following:

      // Protection against session hijacking/fixation attacks.
      require_once './includes/session_protection.inc';
      sess_protect();

I have tried to document all implementation details in the file itself. Check out the @TODO notes as well.

I have been testing this method for some time now with no problems on a box running PHP 4.4.0. It needs someone who could verify it works on PHP 5.

Please, let me know.

BTW, you may wish to remove the following from user.module.

    $old_session_id = session_id();
    session_regenerate_id();
    db_query("UPDATE {sessions} SET sid = '%s' WHERE sid = '%s'", session_id(), $old_session_id);

Read the notes in session_protection.inc. session regeneration is covered on a central place, so module such as logintoboggan won't be broken when details here change (as it has happened already ;-).

Back to my earlier post, I think this is silly. If the data on your server is important enough to protect from session spoofing, you will find a host where you can use SSL. Trying to lock to a user agent, IP block, username, or anything else transmitted in cleartext is pointless as long as it can be sniffed. Don't forget the password can be sniffed at login and bypass the whole session hijack mess, anyway.

At best, you could lock to the IP netblock, but I think you'd have to do it according to IP class (A, B, C. etc.) because of proxies and round-robin NAT, and I would make the case if you're going to get spoofed, the easiest place to snif is on the same network anyway. I think all this type of patch will do is give novice admins a false sense of security and as a result not implement SSL when they should, because the "session protection" is doing everything for them.

First, the ability to choose how many bits of the IP address to take into account to compute the session fingerprint is implemented in the code. Please, check it out.

Second, I'm not an expert in PHP security, but at this point I think I know enough to see that spoofing is a very different thing which is, of course, not covered here. You need something like SSL or better yet, do not use internet. Because even if you use SSL, you're not safe.

Please, take the time to read some of the references I posted before. There are a lot of layers that need some kind of protection. What we're talking here is just a layer, something that, IMHO, is not correctly protected ...but it could be, at the price of a single UPDATE operation for user session. Please, read the comments in the code ;-).

Changing the session id on every page request makes no sense! It is contrary to the way sessions are designed to work and will be at least as bad for performance as before moshe and I fixed Drupal's session creation code in Vancouver.

The fact that session ids may be exposed in URLs is not a problem Drupal can solve.. that is a server configuration issue. Validating the user agent for a session seems OK to me as a general precaution. As has already been mentioned in this thread, if you are really that concerned, run your site through SSL, and please work with PHP, Apache, etc. teams to find a stronger general solution.

I hope no one is suggesting that this issue blocks the release of 4.7.

The code I posted does not change the session id for every page request. It does that only when the user id changes, which is necessary to protect from session fixation attacks. FYI: This is what has been implemented in 4.6.6/4.7-beta6, but it has been left modules such as logintoboggan vulnerable. Doing this on a central place will cover this kind of "problems".

I want argue anymore, unless this is taken seriously. CYa. ;-)

I don't know much about logintoboggan, but it sounds like this bug should be filed against it for the time being if it's so vulnerable.

@markus_petrux: I did read the patch, that's why I said by class. I meant if this patch is going to be committed, it should automatically identify Class A IPs and only filter by the first 8 bits. Setting a 24 bit filter for a class A connection could still break proxy/round-robin stuff. Again, I think the people who'll be most interested in session spoofing will be on the same network as the victim anyway.

@rkerr: +1

deekayen, I'm not sure what patch you're talking about. Just to clarify, the code I'm talking about can be found on comment #37. Read the bottom of the file. There you'll see how to make it check class A, B, C, D or no part of the IP at all.

Regarding logintoboggan, no. I don't use that module. I was just curious to see how it was coded and found that little surprise. I won't report the issue because that won't fix the heart of the problem, which lies in the core.

BTW, deekayen, you've talked again about SSL. Please, read comment #30. Then, if you have the time, please post a reference where it says SSL protects from session hijacking/fixation attacks. Because if it doesn't your comment may give a false sense of security to those who use SSL. So please give a reference. Thanks

Now that I've actually read over the .inc ... it actually looks much more straight forward than this monster of a thread would indicate.

The only things that stand out for me at the moment are:
- lack of ipv6 support
- two anonymous users with the same browser would generate the same signature, if ignoring ip address
- a patch file for bootstrap.inc
- do we want to use md5 or sha1?
- Drupal doesn't already do a register_shutdown_function(register_shutdown_function('session_write_close') elsewhere?

I am warming up to this idea.

@markus_petrux: maybe this will clarify what I mean:

function _sess_fingerprint() {
  global $user;
  $ip = $_SERVER['REMOTE_ADDR'];
  $ip = explode('.', $ip);
  $tmp = array();
  if($ip[0] < 128) {
    $classes = 1;
  }
  elseif($ip[0] < 192) {
    $classes = 2;
  }
  else {
    $classes = 3;
  }
  for ($i=0; $i < $classes; $i++) {
    $tmp[] = $ip[$i];
  }
  $ip = implode('.', $tmp);
  return md5($user->uid . $_SERVER['HTTP_USER_AGENT'] . $ip);
}

@deekayen: Ah, I see. Well, since this code (computing the session fingerprint) would be called for each page request, I opted for a simpler method (where you can choose to take 8, 16, 24 or 32 of the IP, or none at all), so I was not really strict here.

@rkerr: Thanks for reviewing the code.

>> lack of ipv6 support
I couldn't find information about the format of an IPv6 address, as found in $_SERVER['REMOTE_ADDR']. Anyone?

>> two anonymous users with the same browser would generate the same signature, if ignoring ip address
That's why some site admins may prefer to opt for a more secure site by using the IP (or part of it) to compute the fingerprint.

In addition to that, as I wrote in code comments, the ability to choose how many bits of the IP address to use could be placed into the users table. Because this is to protect each user session and users may know if they are behind a proxy farm or not. That, of course, would need a site default and maybe an access control option to allow admins select which roles can choose this level of session security. This option would be really advanced, but it is an implementation issue.

>> a patch file for bootstrap.inc
I would move the functions to session.inc and then provide a patch for bootstrap.inc. For now, I just made the code easier to test, and easier for me to keep updated.

>> do we want to use md5 or sha1?
Is sha1 slower than md5? Otherwise, it doesn't matter much to me.

>> Drupal doesn't already do a register_shutdown_function(register_shutdown_function('session_write_close') elsewhere?
Yes. In the mysqli layer. But that could be moved here, close to the common session management stuff. It is necessary as, in some circumstancies, session data may be lost when using header('Location: ...');

I am against any solution that relies on IP addresses. Remember that many people are behind proxies, such as many corporate networks, AOL users, and even some countries. See comment #4.

We will be causing no end to grief for these people.

I know it is optional (set the define to 0), but it should be moved somewhere else (e.g. admin/settings or settings.php), and not in a define in some file that can get overwritten on an upgrade.

Having a session var to track the visitor by IP class shouldn't break people behind proxies and round-robin NAT. While it wouldn't prevent session hijacking, it would limit it the hijack to the network of the victim. I don't think tracking everyone with a blanket 24 bit filter is right because a 24 bit filter could still break a Class A visitor behind proxy and I don't think most admins are going to understand network classes. Could this all be done as a contrib module using the init() hook?

Of course, there are @TODO notes in the code.

>> Could this all be done as a contrib module using the init() hook?
Nope. hook_init is too far from session_start(). Also, it provides a better protection for session fixation.

* Anyone tested the code?
* Works in PHP5?
* Where should be moved the option to choose how many bits of the IP?
* How to implement this optional feature based on IP classes, or better yet using IP masks or CIDR notation?
* How to provide support for IPv6 (format as present in $_SERVER['REMOTE_ADDR']) ?
* Any other thing not already covered in the @TODO notes?

sorry, I can't restore the assigned field.

ok, I give up

I made a patch for the security module to do some session fingerprinting and SSL login. http://drupal.org/node/55785

Ah, so you really concerned about session hijaking? congrats.

Regarding the use of IP classes, if you want to that strict, FYI:

Classless Inter-Domain Routing (CIDR) is a replacement for the old process of assigning Class A, B and C addresses with a generalized network "prefix". Instead of being limited to network identifiers (or "prefixes") of 8, 16 or 24 bits, CIDR currently uses prefixes anywhere from 13 to 27 bits. Thus, blocks of addresses can be assigned to networks as small as 32 hosts or to those with over 500,000 hosts. This allows for address assignments that much more closely fit an organization's specific needs.

Quoted from:
http://public.pacbell.net/dedicated/cidr.html

Which means, proxy farms probably have IPs assigned using CIDR notation, so that renders your idea about using classes here, pretty useless for most of them.

BTW, deekayen, if you have the time, don't forget #45. ;-)

According to the manual, ini_set('session.use_only_cookies', '1'); is legal, but I can't add it to that patch right now. That could be another config option.

You're absolutely right that CIDR divides up the IP classes, but there's not going to be a way to check who owns what chunk(s) in <.1 sec. I think rudimentary class checking on the basics of subnetting is still a valid, conservative check (unless I'm just really rusty on my Cisco stuff).

[continued from chopped off post] ...less than one second. Rudamentary subnetting rules should still apply as a conservative IP locking method (unless I'm really rusty on my Cisco stuff).

Just an example, XSS can happen, even here at drupal.org
http://drupal.org/node/57942