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: danielc Status: patch IP's can change during a session. So, this isn't a good idea. danielc Previous comments: ------------------------------------------------------------------------ April 1, 2005 - 18: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.

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: vauxia Status: patch 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. vauxia Previous comments: ------------------------------------------------------------------------ April 1, 2005 - 17: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 1, 2005 - 18:51 : danielc IP's can change during a session. So, this isn't a good idea. ------------------------------------------------------------------------ April 1, 2005 - 19: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

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: chx Status: patch Attachment: http://drupal.org/files/issues/session.inc_3.patch (686 bytes) So be it. chx 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.

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: kbahey Status: patch 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. kbahey Previous comments: ------------------------------------------------------------------------ April 1, 2005 - 18: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 1, 2005 - 19:51 : danielc IP's can change during a session. So, this isn't a good idea. ------------------------------------------------------------------------ April 1, 2005 - 20: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 1, 2005 - 21: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 - 09: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 - 10:33 : chx Attachment: http://drupal.org/files/issues/session.inc_3.patch (686 bytes) So be it. ------------------------------------------------------------------------ April 2, 2005 - 10:33 : chx Forgot to change the title.

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: kbahey Status: 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. kbahey Previous comments: ------------------------------------------------------------------------ April 1, 2005 - 18: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 1, 2005 - 19:51 : danielc IP's can change during a session. So, this isn't a good idea. ------------------------------------------------------------------------ April 1, 2005 - 20: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 1, 2005 - 21: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 - 09: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 - 10:33 : chx Attachment: http://drupal.org/files/issues/session.inc_3.patch (686 bytes) So be it. ------------------------------------------------------------------------ April 2, 2005 - 10:33 : chx Forgot to change the title. ------------------------------------------------------------------------ April 2, 2005 - 10: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 - 13: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.

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.

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: degerrit Status: patch 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 [1]. 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) [1] http://www.google.com/search?q=cache:DUytxcJp06YJ:www.sitepoint.com/blog-pos... degerrit 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 [2] on this, and they proposed it as optional, but on as default. So, admin/settings? Or -- and I'd prefer this one -- settings.php? [2] 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. ------------------------------------------------------------------------ April 3, 2005 - 12:48 : Carl McDade 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? ------------------------------------------------------------------------ April 3, 2005 - 17:26 : 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.

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: kbahey Status: patch 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. kbahey Previous comments: ------------------------------------------------------------------------ April 1, 2005 - 18: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 1, 2005 - 19:51 : danielc IP's can change during a session. So, this isn't a good idea. ------------------------------------------------------------------------ April 1, 2005 - 20: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 1, 2005 - 21: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 - 09: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 - 10:33 : chx Attachment: http://drupal.org/files/issues/session.inc_3.patch (686 bytes) So be it. ------------------------------------------------------------------------ April 2, 2005 - 10:33 : chx Forgot to change the title. ------------------------------------------------------------------------ April 2, 2005 - 10: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 - 13: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 - 14: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 - 14: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. ------------------------------------------------------------------------ April 3, 2005 - 06:48 : Carl McDade 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? ------------------------------------------------------------------------ April 3, 2005 - 11:26 : 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. ------------------------------------------------------------------------ April 11, 2005 - 18:36 : 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-pos... ------------------------------------------------------------------------ April 12, 2005 - 10:37 : 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.

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