Session keeps breaking on any POST request

[rcubetrac]

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]

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]

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

=> thomasb

[rcubetrac]

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

later => 1.1.4

[rcubetrac]

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]

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]

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

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

[rcubetrac]

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

Fixed in 9953d5c.

[rcubetrac]

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

new => closed