Fails at login when logging in from new IP

[kd35a]

What I do and what happens:
First I login, and everything works great. Then i move my computer to a new place (from work to home for example), and my session ends or something. So Roundcube asks me to login again. I do that, and get kicked back with the response that my "Login was incorrect or has timed out" (freely translated from swedish: "Din inloggning är felaktig eller har gått ut").

What I expect to happen:
That I get logged in and can see my mail.

Info:
I turned up the logging to 8 ($rcmail_config['debug_level'] = 8; in config/main.inc.php). logs/userlogins says:
[25-Aug-2011 18:52:10 +0200]: Successful login for my_account_name@localhost (ID: 1) from xx.xxx.xxx.xx
No errors show up in logs/errors

Background
I successfully ran 0.5.4 earlier, and upgraded some days ago to 0.6-beta using the update-script, but that failed, so I still had to manually upgrade all the files and the database afterwards.

[kd35a]

Can add that I'm running Roundcube on a Ubuntu 10.10 Server, with lighttpd.

A work-a-round (in Firefox 6.0, on Ubuntu 10.10) that fixes my problem is to open Tools -> Clear Private Data, and then tell it to clear all history of cookies, cache and authenticated sessions.

[kd35a]

Can this piece of code maybe have something to do with it? branches/release-0.6/index.php@5125#L108 (down to line 126)

I'm not that well familiar with the Roundcube-code. But it's maybe a start for where to look.

[remort]

there is an option in RC config file that allows not to check an ip adress while logging in.
Check this option first.

check client IP in session athorization
$rcmail_configip_check? = true;

[kd35a]

Replying to remort:

there is an option in RC config file that allows not to check an ip adress while logging in.
Check this option first.

check client IP in session athorization
$rcmail_configip_check? = true;

That option is already set to true. Thanks anyway :)

[Chumba]

This issue is also prevalent in 0.6 final. I am actually surprised it wasn't solved in the beta before the release.

0.5x didn't show this issue. Comparing the code 0.5x versus 0.6, I noticed quite a few changes in the way RC handles logins / cookies.

--

Logging with IP x.x.x.x: success

Logging out: success

Logging with IP y.y.y.y (same credentials): fail

--

Deletion of 'roundcube_sessid' cookie

Logging with IP y.y.y.y (same credentials): success

--

Some info: HTTPS, SQLite database

[kd35a]

Yes, this issue persists for me too. The cookie that is causing this problem (for me at least) is roundcube_sessid. Deleting that specific cookie makes it possible for me to login again.

[Chumba]

What also works is closing/restarting the browser (here: Chrome 15 beta). It seems the session cookie isn't properly deleted/invalidated even after log out, and it is only automatically removed upon restarting the browser.

[Chumba]

Scrap my last post. Closing/restarting the browser actually does not solve the issue. Some more details:

$rcmail_config['ip_check'] = false;

• Upon login, Roundcube sets two session-related cookies:

roundcube_sessid
  Value: Hash, Domain: mailhost/, Expires: Session (secure cookie)
roundcube_sessauth
  Value: Hash, Domain: mailhost/, Expires: Session (secure cookie)

• Upon logout, only one cookie is deleted: roundcube_sessauth

• roundcube_sessid remains.

• Upon browser restart, roundcube_sessid still remains (tested with Chrome 15, Firefox 7.01).

• As long as roundcube_sessid remains, logging in from another IP fails with "Your session is invalid or expired."

• Roundcube 0.54 did not exhibit this behavior.

[Chumba]

Found what causes the problem.

Upon logout, rcmail.php calls session->kill() (from rcube_session.php), which unfortunately only deletes 'roundcube_sessauth', but not roundcube_sessid. Below is a quick fix for that issue.

Note you need to leave $rcmail_config['ip_check'] to false. I noticed that the IP check routine is also borked in 0.6. If your IP changes during a Roundcube session, Roundcube would log you out with $rcmail_config['ip_check'] = true, and that's how it should be. But: afterwards you have no way of logging in again with the new IP, as the session code in rcube_session.php keeps inserting the old IP in the database (responsible code: db_read($key)).

Anyway, to fix the first problem, in /program/include/rcmail.php find:

  /**
   * Destroy session data and remove cookie
   */
  public function kill_session()
  {
    $this->plugins->exec_hook('session_destroy');

    $this->session->kill();
    $_SESSION = array('language' => $this->user->language, 'temp' => true);
    $this->user->reset();
  }

and replace it with

  /**
   * Destroy session data and remove cookie
   */
  public function kill_session()
  {
    $this->plugins->exec_hook('session_destroy');

    $this->session->kill();
    $_SESSION = array('language' => $this->user->language, 'temp' => true);
    $this->user->reset();
    setcookie(session_name(), '', time() - 42000);
  }

[kd35a]

The problem still exists in 0.7-beta. Chumbas solution works quite good in 0.6, but you need one or two refreshes of the page before the cookie is deleted. I will try Chumbas solution in 0.7-beta when i get the possibility to switch networks next time.

[thomasb]

I don't understand why this issue still exists in 0.7-beta. IP check is only done *after* the user logged in and upon login kill_session() is called which destroys the session record and also kills the cookie (see rcube_session::kill()).

[Chumba]

Thomas, it's been a few weeks since I last looked at the issue, but at least in 0.6, the kill_session() method fails to destroy the 'roundcube_sessid' cookie (it only destroys 'roundcube_sessauth'). I only had a quick glance at the SVN (rev 5511), but the behavior doesn't seem to have been changed.

The 'roundcube_sessid' is created in the session_init() method:

--snip--

    $sess_name   = $this->config->get('session_name');
    $sess_domain = $this->config->get('session_domain');
    $lifetime    = $this->config->get('session_lifetime', 0) * 60;

    // set session domain
    if ($sess_domain) {
      ini_set('session.cookie_domain', $sess_domain);
    }
    // set session garbage collecting time according to session_lifetime
    if ($lifetime) {
      ini_set('session.gc_maxlifetime', $lifetime * 2);
    }

--snip--

    ini_set('session.name', $sess_name ? $sess_name : 'roundcube_sessid');

From then on, it fully relies on the PHP garbage collector; there is no code in RC that would invalidate the 'roundcube_sessid' cookie upon logout.

The other issue, related to the IP check in 0.6, has to with the entry in the session table not properly being invalidated upon a forced logout (when an IP change occurs). If I recall correctly, RC would take the old IP address via the db_read() method (yes, the session record is still present in the database here) causing the aforementioned conflict when the user tries to login with his new IP address.

[thomasb]

Upon logout rcmail::kill_session() is called which itself calls rcube_session::kill() which will call rcube_session::destroy() which deletes the session record from the database. This should do the job but you're right, the session cookie isn't deleted.

[Chumba]

I am _fairly certain_ that at the time I was debugging the code (0.6), the session record was still in the database after the forced logout.

I have no time for further debugging right now, but two thoughts:

1. I am using SQLite. Issue perhaps in the SQLite backend code?

2. Does a forced logout (with ip_check enabled) go through the same process as a manual logout? Perhaps the manual logout properly deletes the session record, while a forced logout doesn't?

[kd35a]

Replying to Chumba:

I am _fairly certain_ that at the time I was debugging the code (0.6), the session record was still in the database after the forced logout.

I have no time for further debugging right now, but two thoughts:

1. I am using SQLite. Issue perhaps in the SQLite backend code?

2. Does a forced logout (with ip_check enabled) go through the same process as a manual logout? Perhaps the manual logout properly deletes the session record, while a forced logout doesn't?

I'm using MySQL, if that info helps.

[alec]

Confirmed with 0.7. Enabling ip_check makes problems with log in. This is something with cookies handling, but I wasn't able to fix this. Do we need both roundcube_sessid and roundcube_sessauth cookies?

[Chumba]

These are really two issues we're discussing here.

One is related to session handling (roundcube_sessid not being invalidated upon logout). I find the code very difficult to read (Spaghetti comes to my mind), and I am still not sure why two cookies are used here.

This issue is not affected by whether ip_check is true or false.

The other issue is related to ip_check, and it prevents the session record from not being fully erased from the database. thomasb pointed out that upon logout the proper methods are used to remove that record, but I can confirm that the record is still there if the logout was "forced" (it is properly removed when you do a regular manual logout). My theory is that the logout forced upon an IP conflict doesn't call the proper routines to get rid of the record; though I haven't had time to validate this claim.

[thomasb]

Replying to alec:

Confirmed with 0.7. Enabling ip_check makes problems with log in. This is something with cookies handling, but I wasn't able to fix this. Do we need both roundcube_sessid and roundcube_sessauth cookies?

The roundcube_sessauth is used for protection against session hijacking and changes it's value every couple of minutes. It's a security mechanism and the answer is yes, we need both unless anybody comes up with a better way to protect sessions.

[thomasb]

Set 'log_session' to true in your config and check logs/session for details about what check fails. I cannot reproduce the problem that the session record isn't deleted from the database even after a forced logout. This is done in rcmail::kill_session() which is called in both cases. Maybe log these calls as well by adding write_log('session', "rcmail::kill_session()"); to that function.

[thomasb]

Replying to Chumba:

Thomas, it's been a few weeks since I last looked at the issue, but at least in 0.6, the kill_session() method fails to destroy the 'roundcube_sessid' cookie (it only destroys 'roundcube_sessauth').

That's true, the regular session cookie is not deleted and there's a reason for that:

$_SESSION = array('language' => $this->user->language, 'temp' => true);

We want to remember certain settings from the users session (e.g. language, skin, etc.) even after logout.

[Chumba]

Replying to thomasb:

Set 'log_session' to true in your config and check logs/session for details about what check fails. I cannot reproduce the problem that the session record isn't deleted from the database even after a forced logout. This is done in rcmail::kill_session() which is called in both cases. Maybe log these calls as well by adding write_log('session', "rcmail::kill_session()"); to that function.

Using 0.71, with sqlite as database backend, the issue with $rcmail_configip_check? = true is still prevalent. From log session:

[03-Mar-2012 16:04:29 +0100]: IP check failed for 0bb125f8235c40345ed3db507eca7191; expected [myoldip]; got [mynewip]

0bb125f8235c40345ed3db507eca7191 is the sessionid from the previous session that I logged out before forcing an IP change and trying to relogin.

Thomas, have you tried to reproduce it running Sqlite? Perhaps the following ticket is related:

​http://trac.roundcube.net/ticket/1484299

rcmail::kill_session() is logged, so the method is called and yet the session remains in the database.

[thomasb]

Replying to Chumba:

rcmail::kill_session() is logged, so the method is called and yet the session remains in the database.

Please read my previous comment. The session record is deleted but recreated right afterwards because we want to keep certain settings. In [c73efcc7] we now also update the IP stored in memory to the new one. This should resolve this issue.