Issue status update for http://drupal.org/node/19845 Project: Drupal Version: cvs Component: base system Category: feature requests Priority: critical Assigned to: chx Reported by: chx Updated by: Carl McDade Status: patch 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? Carl McDade Previous comments: ------------------------------------------------------------------------ April 2, 2005 - 00:50 : 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. ------------------------------------------------------------------------ April 2, 2005 - 01:51 : danielc IP's can change during a session. So, this isn't a good idea. ------------------------------------------------------------------------ April 2, 2005 - 02:23 : 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 ------------------------------------------------------------------------ April 2, 2005 - 03:18 : vauxia 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. ------------------------------------------------------------------------ April 2, 2005 - 15:57 : 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. ------------------------------------------------------------------------ April 2, 2005 - 16:33 : chx Attachment: http://drupal.org/files/issues/session.inc_3.patch (686 bytes) So be it. ------------------------------------------------------------------------ April 2, 2005 - 16:33 : chx Forgot to change the title. ------------------------------------------------------------------------ April 2, 2005 - 16:43 : 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. ------------------------------------------------------------------------ April 2, 2005 - 19:41 : 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. ------------------------------------------------------------------------ April 2, 2005 - 20:09 : 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. ------------------------------------------------------------------------ April 2, 2005 - 20:41 : 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.