Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Session keeps breaking on any POST request #4931

Closed
rcubetrac opened this issue Nov 4, 2015 · 8 comments
Assignees
Milestone

Comments

@rcubetrac
Copy link

@rcubetrac rcubetrac commented Nov 4, 2015

Reported by Ashus on 4 Nov 2015 01:04 UTC as Trac ticket #1490582

In my current environment (HTTPS, HTTP/2, Apache 2.4.17, PHP 5.6.14 via fastcgi) Roundcube does not work. Anytime I submit any form, it logs out with the message "invalid auth cookie sent". I debugged it and found the problem in the core file program/lib/Roundcube/rcube.php on line 528:

        $this->session->set_secret($this->config->get('des_key') . dirname($_SERVER[to be changed to either 
    $this->session->set_secret($this->config->get('des_key') . dirname($_SERVER['SCRIPT_FILENAME']('SCRIPT_NAME']));
needed)));

or

        $this->session->set_secret($this->config->get('des_key') . __DIR__);

I presume the variable $_SERVER[SCRIPT_NAME] was used to check for specific instance of RC on the same server where cookies might collide. My debug has resulted in these:

  'SCRIPT_FILENAME' => '/var/www/mail/index.php',
  'SCRIPT_NAME' => 'https://mail.ashus.net/?_task=settings&_action=edit-prefs&_section=mailview&_framed=1',

Post requests were without Get queries so dirname extracted something else.
So you can see that didn't work for me. Please take this patch to the core before more HTTP/2 speed hungry users start reporting this.

Migrated-From: http://trac.roundcube.net/ticket/1490582

@rcubetrac

This comment has been minimized.

Copy link
Author

@rcubetrac rcubetrac commented Nov 4, 2015

Comment by @alecpl on 4 Nov 2015 07:32 UTC

Strange. Looks to me as a PHP or server configuration issue. According to PHP manual 'SCRIPT_NAME' contains the current script's path. It's not filesystem path, but the path from the URL. So, not something I'd expect either.

We can't use __DIR__ as the rcube.php location may be shared between many Roundcube instances. I think $_SERVER['SCRIPT_FILENAME'] is a good choice. Assigning to Thomas for review.

BTW, there's no such code in git-master, so the fix is needed for 1.1 only.

@rcubetrac

This comment has been minimized.

Copy link
Author

@rcubetrac rcubetrac commented Nov 4, 2015

Owner changed by @alecpl on 4 Nov 2015 07:32 UTC

=> thomasb

@rcubetrac

This comment has been minimized.

Copy link
Author

@rcubetrac rcubetrac commented Nov 4, 2015

Milestone changed by @alecpl on 4 Nov 2015 07:32 UTC

later => 1.1.4

@rcubetrac

This comment has been minimized.

Copy link
Author

@rcubetrac rcubetrac commented Nov 4, 2015

Comment by @alecpl on 4 Nov 2015 08:07 UTC

From the other hand, the uniqueness of the secret key is achieved by the code in rcube_session::_mkcookie(). So, as I understand that variable don't have to be unique to the Roundcube instance, but to the whole server. It's just there to make des_key guessing harder, I suppose.

However, I'm afraid that if we change that code, we may break something unintentionally. So, I propose to keep SCRIPT_NAME use, but just remove everything starting from ? character. What do you think?

ps. I'm looking for most simple solution, I don't want to backport changes from master, as they are too intrusive for a stable branch.

@rcubetrac

This comment has been minimized.

Copy link
Author

@rcubetrac rcubetrac commented Nov 4, 2015

Comment by Ashus on 4 Nov 2015 08:43 UTC

Well I tried it with SCRIPT_FILENAME and found nothing broken. SCRIPT_NAME might still be used inside dirname after removing https:// and server part from the variable.

$var = preg_replace('~^https?://['/', $_SERVER['SCRIPT_NAME'](^/]*/~',));

I googled for old phpinfos and found SCRIPT_FILENAME was working fine even in PHP 5.1.6. Your choice.

@rcubetrac

This comment has been minimized.

Copy link
Author

@rcubetrac rcubetrac commented Nov 4, 2015

Comment by @alecpl on 4 Nov 2015 19:14 UTC

FYI, https://bugs.php.net/bug.php?id=70757

@rcubetrac

This comment has been minimized.

Copy link
Author

@rcubetrac rcubetrac commented Nov 6, 2015

Comment by @alecpl on 6 Nov 2015 07:34 UTC

Fixed in 9953d5c.

@rcubetrac

This comment has been minimized.

Copy link
Author

@rcubetrac rcubetrac commented Nov 6, 2015

Status changed by @alecpl on 6 Nov 2015 07:34 UTC

new => closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.