duplicate session cookies prevent login

crunchywelch - October 4, 2006 - 07:30
Project:Drupal
Version:4.7.5
Component:base system
Category:bug report
Priority:critical
Assigned:crunchywelch
Status:closed
Description

Was the popular but sadly abused issue: http://drupal.org/node/60584

This is against cvs, and moved to session.inc where the new session regeneration code was moved.

AttachmentSize
session.inc.patch.txt759 bytes

#1

chx - October 4, 2006 - 07:31

Sure. Simple but necessary fix for any PHP versions before 4.4.0 http://bugs.php.net/bug.php?id=32802

#2

crunchywelch - October 4, 2006 - 07:34

Lets add that to the patch for curious future code reviewers and for future removal once its not needed

AttachmentSize
session.inc.patch_0.txt846 bytes

#3

Dries - October 4, 2006 - 14:50
Status:patch (reviewed & tested by the community)» patch (code needs work)

Coding style and documentation style need work. Punctuation, spaces, etc, ...

#4

crunchywelch - October 4, 2006 - 16:12

Cleaned uop, documentation rewritten

AttachmentSize
session.inc.patch_1.txt951 bytes

#5

chx - October 4, 2006 - 16:14
Status:patch (code needs work)» patch (reviewed & tested by the community)

I still like it...

#6

drumm - October 17, 2006 - 09:04
Status:patch (reviewed & tested by the community)» patch (code needs work)

patching file ./includes/session.inc
patch: **** malformed patch at line 22:

#7

chx - October 17, 2006 - 18:57
Status:patch (code needs work)» patch (reviewed & tested by the community)

Really? It needs a bzr patch then :)

AttachmentSize
session.inc.patch_1_0.txt953 bytes

#8

chx - October 17, 2006 - 18:59

proper file.

AttachmentSize
session.inc.patch_2.txt993 bytes

#9

Dries - October 18, 2006 - 11:21
Status:patch (reviewed & tested by the community)» patch (code needs work)

In the code comments, please clarify 'certain drupal configurations'. Also, we write 'Drupal', not 'drupal'. Maybe add a TODO so we don't forget to remove this cruft once we drop support for PHP 4.4.

#10

chx - October 21, 2006 - 15:43
Status:patch (code needs work)» patch (reviewed & tested by the community)

Dries, detailing "certain Drupal configurations" is WAY beyond the scope of a code comment. Especially because I surely would forget one or two.

AttachmentSize
session.inc_7.patch2.93 KB

#11

chx - October 21, 2006 - 15:44

Grrr.. wrong file. Have not happend since long. Sorry.

AttachmentSize
session.inc_8.patch1.03 KB

#12

Dries - October 23, 2006 - 06:28

You don't have to be super-specific when you describe the configurations. But surely, there must be something generic that triggers the problem? (Code comments normally wrap at line width 78.)

#13

crunchywelch - October 23, 2006 - 20:05

'certain Drupal configurations' is misleading. I have reproduced this on normal and multisite installs on both 4.6 and 4.7. The root cause is a php bug. This is evidenced by the similar workarounds being done in other projects like gallery2. I have changed the comments to reflect this, and reformatted the comment line breaks.

AttachmentSize
session.inc.patch_3.txt1.04 KB

#14

Dries - October 23, 2006 - 20:47
Status:patch (reviewed & tested by the community)» patch (code needs work)

Patch does not apply, and line breaks are not consistent.

#15

chx - October 23, 2006 - 21:03
Status:patch (code needs work)» patch (reviewed & tested by the community)
AttachmentSize
session.inc_9.patch995 bytes

#16

Dries - October 24, 2006 - 15:31
Status:patch (reviewed & tested by the community)» fixed

Committed to CVS HEAD. Thanks.

#17

webernet - October 24, 2006 - 16:14
Status:fixed» patch (code needs work)

This seems to have broken login for PHP 5.1.2 - the session is opened, then the cookie is destroyed resulting in being unable to login.

#18

webernet - October 24, 2006 - 17:42

Testing under PHP 4.3.3 reveals the same login problem.

#19

webernet - October 24, 2006 - 18:04
Status:patch (code needs work)» active

Until a better fix can be found - this patch needs to be rolled back.

#20

Thomas Ilsche - October 24, 2006 - 18:18

While I cannot confirm that it is due to this patch, I can confirm that login in HEAD is broken. The state of beeing logged in however keeps working if the login happened before update (of course just until logout).

#21

rkerr - October 24, 2006 - 18:20

Confirmed to be broken on PHP 5.2.1 (OpenSUSE 10.1) with a fresh HEAD install and database.

Logging in as uid 1 through the block returns to without being logged in.
Logging in as uid 1 through user/login returns to user/1 with "access denied" message.

#22

chx - October 24, 2006 - 19:34

Crunchywelch told me that he ran this patch for years on several installs. He is looking into the problem.

#23

hunmonk - October 24, 2006 - 23:21

i can confirm that this patch is causing the problem. i removed the offending lines and logins now work fine. please roll this back until we find a workable solution for the problem it was trying to solve.

#24

drumm - October 25, 2006 - 03:47
Status:active» patch (code needs review)

I'll explain this when I'm not boardinfg an airplabne

AttachmentSize
session.inc.diff.txt920 bytes

#25

chx - October 25, 2006 - 07:04
Status:patch (code needs review)» patch (reviewed & tested by the community)

We understand. The current code invalidates the new session cookie session_regenerate_id sends since 4.3.3 instead of invalidating the old one. Good catch.

#26

Kjartan - October 25, 2006 - 09:53
Status:patch (reviewed & tested by the community)» fixed

Committed.

#27

moshe weitzman - November 7, 2006 - 18:36

does drumm's patch need to be backported?

#28

Anonymous - November 21, 2006 - 18:45
Status:fixed» closed

#29

ajwwong - December 21, 2006 - 01:21
Version:x.y.z» 4.7.x-dev
Status:closed» patch (to be ported)

This is a backport attempt of drumm's patch for 4.7. I hope I did this right. I just wanted to try to make a small contribution to this dialogue.

So... as far as I can tell, the patch requires a modification to user.module [where the session_regeneration function *used* to live] as well as session.inc, I think....

So here is the patch modification to user.module of 4.7 HEAD.

AttachmentSize
user_77.patch446 bytes

#30

ajwwong - December 21, 2006 - 01:22

And this is the patch modification for sessions.inc.

Thanks to everyone for your hard work here...

Much love,
Albert

www.ithou.org

AttachmentSize
session_4.patch964 bytes

#31

ajwwong - December 21, 2006 - 01:31
Priority:critical» normal
Status:patch (to be ported)» patch (code needs review)

#32

Caleb G - oldacct - December 26, 2006 - 21:44

No problems logging in in IE/FF for windows, or Safari/FF for Mac with latest 4.7 head. Seems good. This is good commit, because people have definitely had problems with login. (used to myself until fixed them with patches and such)

#33

killes@www.drop.org - January 1, 2007 - 18:25
Status:patch (code needs review)» fixed

applied

#34

Chill35 - January 7, 2007 - 22:11
Version:4.7.x-dev» 4.7.5
Priority:normal» critical

Problem persists for me with Drupal 4.7.5.

#35

Chill35 - January 7, 2007 - 22:12
Status:fixed» active

#36

Chill35 - January 7, 2007 - 22:14

Details :

1. You log in once, nothing happens, you log in a second time, you're logged in.

2. Sometimes you have to use a different browser than IE because you end up always remaining logged out in it.

There are no bad logs in my Drupal Admin Panel about it.

On my host I checked which version of Apache I am using...

(It happens all the time for me now, on my remote site.)

PHP Version 5.1.2
Apache/2.0.54

I upgraded Drupal to 4.7.5 from 4.7.4 (and ran update.php). Same problem.

The exact same site on my local machine works like a charm.

Here are some more info about my remote server :

Session Support enabled
Registered save handlers files user sqlite
Registered serializer handlers php php_binary

Directive Local Value Versus Master Value
session.auto_start Off Off
session.bug_compat_42 Off Off
session.bug_compat_warn On On
session.cache_expire 180 180
session.cache_limiter nocache nocache
session.cookie_domain no value no value
session.cookie_lifetime 0 0
session.cookie_path / /
session.cookie_secure Off Off
session.entropy_file no value no value
session.entropy_length 0 0
session.gc_divisor 1000 1000
session.gc_maxlifetime 1440 1440
session.gc_probability 1 1
session.hash_bits_per_character 5 5
session.hash_function 0 0
session.name PHPSESSID PHPSESSID
session.referer_check no value no value
session.save_handler files files
session.save_path /tmp /tmp
session.serialize_handler php php
session.use_cookies On On
session.use_only_cookies Off Off
session.use_trans_sid 0 0

#37

Chill35 - January 7, 2007 - 22:20

My session.inc files says (the file installed with drupal 4.7.5) :

function sess_regenerate() {
  $old_session_id = session_id();

  // We code around http://bugs.php.net/bug.php?id=32802 by destroying
  // the session cookie by setting expiration in the past (a negative
  // value).  This issue only arises in PHP versions before 4.4.0,
  // regardless of the Drupal configuration.
  // TODO: remove this when we require at least PHP 4.4.0
  if (isset($_COOKIE[session_name()])) {
    setcookie(session_name(), '', time() - 42000, '/');
  }

  session_regenerate_id();

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

My question is :

Is this patch causing me a problem ? Should I do what the comment is telling me :

TODO: remove this when we require at least PHP 4.4.0

My remote server is runnning PHP 5.

#38

salvis - January 21, 2007 - 20:52

I don't know if I'm seeing the same problem — I'm using 4.7.5 with PHP four.three.ten and none of the patches in this thread applied yet. In the middle of a session I'm suddenly logged out, and any log in attempts just won't catch on. Logging in from another computer works just fine. In the log I see successful log-ins.

However, this is at least in part caused by ZoneAlarm. When it happens, it usually takes a day or so until I can log in again successfully, but I've had immediate success removing and recreating the ZA Privacy record for my site. Another user has reported the same problem, and he's also using ZA.

If I install the httpauth module, I can force a successful login by appending ?authenticate to the URL, however, with the next click on a link I get an empty page and the address bar shows ?PHPSESSID=sessionid appended to the URL. This happens even though I have session.use_only_cookies On and session.use_trans_sid Off! I verified these settings in phpInfo() inside the devel module — his baffles me completely! If I cut off the session ID and re-get the page, I get the requested page, i.e. I'm still logged in.

Is this the effect discussed in this thread, or can anyone point me in the right direction?

#39

ajwwong - January 31, 2007 - 21:23

Running php 4.4.2
MySQL 4.1
Apache 2.2
FreeBSD 6.0
Drupal 4.7.5
dedicated server

Just fyi:

running drupal with patch above applied:

nevertheless -- i am still observing duplicate session entries for particular users [approximately 5% of the user base -- all of whom use internet explorer] in spite of clearing cookies:

For the user who cannot login, the sessions table data looks like this: **duplicate session entries** with same timestamp:

uid  sessid                       ip address      timestamp 
242 c40a409fef19axxxxxxxxxx      68.7.124.128       1170270738
0    59417fb4a205xxxxxxxxxxx      68.7.124.128       1170270738

They have been asked to clear cookies, sessions table has been truncated, and the .htaccess file now contains the following:

   RewriteCond %{HTTP_HOST} !^www\.ithou\.org$ [NC]
   RewriteRule .* http://www.ithou.org/ [L,R=301]

and settings.php contains the following line:

ini_set('session.cookie_domain', '.ithou.org');

Nevertheless duplicate session id's appear. I will continue to work with the users to try and track this down.

If anyone wants access to the settings of my machine to inspect or try an find a fix, please let me know and I will be happy to send the info to you.

Regards,
Albert
www.ithou.org

#40

webernet - August 10, 2007 - 23:36
Status:active» closed
 
 

Drupal is a registered trademark of Dries Buytaert.