From cf2da2f9aacd1b13ad9019f44a3f1edd824cd015 Mon Sep 17 00:00:00 2001 From: thomascube Date: Fri, 28 Jan 2011 16:44:22 +0000 Subject: Improve session validity check with changing auth cookies; reduce writes to DB; better phpdoc --- config/main.inc.php.dist | 10 +- index.php | 4 +- program/include/rcmail.php | 51 ++-------- program/include/rcube_session.php | 194 ++++++++++++++++++++++++++++++-------- 4 files changed, 170 insertions(+), 89 deletions(-) diff --git a/config/main.inc.php.dist b/config/main.inc.php.dist index 86b80d4f2..cf111dbcc 100644 --- a/config/main.inc.php.dist +++ b/config/main.inc.php.dist @@ -206,13 +206,12 @@ $rcmail_config['skin_include_php'] = false; // must be greater than 'keep_alive'/60 $rcmail_config['session_lifetime'] = 10; +// session domain: .example.org +$rcmail_config['session_domain'] = ''; + // check client IP in session athorization $rcmail_config['ip_check'] = false; -// Use an additional frequently changing cookie to athenticate user sessions. -// There have been problems reported with this feature. -$rcmail_config['double_auth'] = false; - // this key is used to encrypt the users imap password which is stored // in the session record (and the client cookie if remember password is enabled). // please provide a string of exactly 24 chars. @@ -292,9 +291,6 @@ $rcmail_config['line_length'] = 72; // send plaintext messages as format=flowed $rcmail_config['send_format_flowed'] = true; -// session domain: .example.org -$rcmail_config['session_domain'] = ''; - // don't allow these settings to be overriden by the user $rcmail_config['dont_override'] = array(); diff --git a/index.php b/index.php index 3d57571a4..a80e76d68 100644 --- a/index.php +++ b/index.php @@ -98,7 +98,7 @@ if ($RCMAIL->task == 'login' && $RCMAIL->action == 'login') { $RCMAIL->session->regenerate_id(); // send auth cookie if necessary - $RCMAIL->authenticate_session(); + $RCMAIL->session->set_auth_cookie(); // log successful login rcmail_log_login(); @@ -141,7 +141,7 @@ else if ($RCMAIL->task == 'logout' && isset($_SESSION['user_id'])) { // check session and auth cookie else if ($RCMAIL->task != 'login' && $_SESSION['user_id'] && $RCMAIL->action != 'send') { - if (!$RCMAIL->authenticate_session()) { + if (!$RCMAIL->session->check_auth()) { $OUTPUT->show_message('sessionerror', 'error'); $RCMAIL->kill_session(); } diff --git a/program/include/rcmail.php b/program/include/rcmail.php index 56181a733..7f76ba4c3 100644 --- a/program/include/rcmail.php +++ b/program/include/rcmail.php @@ -599,10 +599,8 @@ class rcmail session_start(); // set initial session vars - if (!isset($_SESSION['auth_time'])) { - $_SESSION['auth_time'] = time(); + if (!$_SESSION['user_id']) $_SESSION['temp'] = true; - } } @@ -624,6 +622,9 @@ class rcmail $keep_alive = max(60, $keep_alive); $this->session->set_keep_alive($keep_alive); } + + $this->session->set_secret($this->config->get('des_key') . $_SERVER['HTTP_USER_AGENT']); + $this->session->set_ip_check($this->config->get('ip_check')); } @@ -776,7 +777,7 @@ class rcmail $_SESSION['imap_ssl'] = $imap_ssl; $_SESSION['password'] = $this->encrypt($pass); $_SESSION['login_time'] = mktime(); - + if (isset($_REQUEST['_timezone']) && $_REQUEST['_timezone'] != '_default_') $_SESSION['timezone'] = floatval($_REQUEST['_timezone']); @@ -998,41 +999,6 @@ class rcmail } - /** - * Check the auth hash sent by the client against the local session credentials - * - * @return boolean True if valid, False if not - */ - function authenticate_session() - { - // advanced session authentication - if ($this->config->get('double_auth')) { - $now = time(); - $valid = ($_COOKIE['sessauth'] == $this->get_auth_hash(session_id(), $_SESSION['auth_time']) || - $_COOKIE['sessauth'] == $this->get_auth_hash(session_id(), $_SESSION['last_auth'])); - - // renew auth cookie every 5 minutes (only for GET requests) - if (!$valid || ($_SERVER['REQUEST_METHOD']!='POST' && $now - $_SESSION['auth_time'] > 300)) { - $_SESSION['last_auth'] = $_SESSION['auth_time']; - $_SESSION['auth_time'] = $now; - rcmail::setcookie('sessauth', $this->get_auth_hash(session_id(), $now), 0); - } - } - else { - $valid = $this->config->get('ip_check') ? $_SERVER['REMOTE_ADDR'] == $this->session->get_ip() : true; - } - - // check session filetime - $lifetime = $this->config->get('session_lifetime'); - $sess_ts = $this->session->get_ts(); - if (!empty($lifetime) && !empty($sess_ts) && $sess_ts + $lifetime*60 < time()) { - $valid = false; - } - - return $valid; - } - - /** * Destroy session data and remove cookie */ @@ -1040,9 +1006,8 @@ class rcmail { $this->plugins->exec_hook('session_destroy'); - $this->session->remove(); - $_SESSION = array('language' => $this->user->language, 'auth_time' => time(), 'temp' => true); - rcmail::setcookie('sessauth', '-del-', time() - 60); + $this->session->kill(); + $_SESSION = array('language' => $this->user->language, 'temp' => true); $this->user->reset(); } @@ -1056,7 +1021,7 @@ class rcmail // on logout action we're not connected to imap server if (($config['logout_purge'] && !empty($config['trash_mbox'])) || $config['logout_expunge']) { - if (!$this->authenticate_session()) + if (!$this->session->check_auth()) return; $this->imap_connect(); diff --git a/program/include/rcube_session.php b/program/include/rcube_session.php index f3c4f1fe7..55c2e1443 100644 --- a/program/include/rcube_session.php +++ b/program/include/rcube_session.php @@ -5,7 +5,7 @@ | program/include/rcube_session.php | | | | This file is part of the Roundcube Webmail client | - | Copyright (C) 2005-2010, The Roundcube Dev Team | + | Copyright (C) 2005-2011, The Roundcube Dev Team | | Licensed under the GNU GPL | | | | PURPOSE: | @@ -31,12 +31,17 @@ class rcube_session { private $db; private $ip; + private $start; private $changed; private $unsets = array(); private $gc_handlers = array(); - private $start; + private $cookiename = 'roundcube_sessauth'; private $vars = false; private $key; + private $now; + private $prev; + private $secret = ''; + private $ip_check = false; private $keep_alive = 0; /** @@ -47,6 +52,12 @@ class rcube_session $this->db = $db; $this->lifetime = $lifetime; $this->start = microtime(true); + $this->ip = $_SERVER['REMOTE_ADDR']; + + // valid time range is now - 1/2 lifetime to now + 1/2 lifetime + $now = time(); + $this->now = $now - ($now % ($this->lifetime / 2)); + $this->prev = $this->now - ($this->lifetime / 2); // set custom functions for PHP session management session_set_save_handler( @@ -93,7 +104,14 @@ class rcube_session } - // save session data + /** + * Save session data. + * handler for session_read() + * + * @param string Session ID + * @param string Serialized session vars + * @return boolean True on success + */ public function write($key, $vars) { $ts = microtime(true); @@ -118,28 +136,22 @@ class rcube_session else $newvars = $vars; - if (!$this->lifetime) { - $timeout = 600; - } - else if ($this->keep_alive>0) { - $timeout = min($this->lifetime * 0.5, $this->lifetime - $this->keep_alive); - } else { - $timeout = 0; - } - - if (!($newvars === $oldvars) || ($ts - $this->changed > $timeout)) { + if ($newvars !== $oldvars) { $this->db->query( - sprintf("UPDATE %s SET vars = ?, changed = %s WHERE sess_id = ?", + sprintf("UPDATE %s SET vars=?, changed=%s WHERE sess_id=?", get_table_name('session'), $now), base64_encode($newvars), $key); } + else if ($ts - $this->changed > $this->lifetime / 2) { + $this->db->query("UPDATE ".get_table_name('session')." SET changed=$now WHERE sess_id=?", $key); + } } else { $this->db->query( sprintf("INSERT INTO %s (sess_id, vars, ip, created, changed) ". "VALUES (?, ?, ?, %s, %s)", get_table_name('session'), $now, $now), - $key, base64_encode($vars), (string)$_SERVER['REMOTE_ADDR']); + $key, base64_encode($vars), (string)$this->ip); } $this->unsets = array(); @@ -147,7 +159,12 @@ class rcube_session } - // handler for session_destroy() + /** + * Handler for session_destroy() + * + * @param string Session ID + * @return boolean True on success + */ public function destroy($key) { $this->db->query( @@ -158,7 +175,12 @@ class rcube_session } - // garbage collecting function + /** + * Garbage collecting function + * + * @param string Session lifetime in seconds + * @return boolean True on success + */ public function gc($maxlifetime) { // just delete all expired sessions @@ -173,7 +195,11 @@ class rcube_session } - // registering additional garbage collector functions + /** + * Register additional garbage collector functions + * + * @param mixed Callback function + */ public function register_gc_handler($func_name) { if ($func_name && !in_array($func_name, $this->gc_handlers)) @@ -181,33 +207,41 @@ class rcube_session } + /** + * Generate and set new session id + */ public function regenerate_id() { + // delete old session record + $this->destroy(session_id()); + $this->vars = false; + $randval = '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ'; for ($random = '', $i=1; $i <= 32; $i++) { $random .= substr($randval, mt_rand(0,(strlen($randval) - 1)), 1); } - // use md5 value for id or remove capitals from string $randval - $random = md5($random); - - // delete old session record - $this->destroy(session_id()); - - session_id($random); + // use md5 value for id + $this->key = md5($random); + session_id($this->key); $cookie = session_get_cookie_params(); $lifetime = $cookie['lifetime'] ? time() + $cookie['lifetime'] : 0; - rcmail::setcookie(session_name(), $random, $lifetime); + rcmail::setcookie(session_name(), $this->key, $lifetime); return true; } - // unset session variable - public function remove($var=NULL) + /** + * Unset a session variable + * + * @param string Varibale name + * @return boolean True on success + */ + public function remove($var=null) { if (empty($var)) return $this->destroy(session_id()); @@ -217,9 +251,20 @@ class rcube_session return true; } + + /** + * Kill this session + */ + public function kill() + { + $this->destroy(session_id()); + rcmail::setcookie($this->cookiename, '-del-', time() - 60); + } - // serialize session data + /** + * Serialize session data + */ private function serialize($vars) { $data = ''; @@ -232,8 +277,10 @@ class rcube_session } - // unserialize session data - // http://www.php.net/manual/en/function.session-decode.php#56106 + /** + * Unserialize session data + * http://www.php.net/manual/en/function.session-decode.php#56106 + */ private function unserialize($str) { $str = (string)$str; @@ -318,26 +365,99 @@ class rcube_session return unserialize( 'a:' . $items . ':{' . $serialized . '}' ); } + /** + * Setter for keep_alive interval + */ public function set_keep_alive($keep_alive) { $this->keep_alive = $keep_alive; } + /** + * Getter for keep_alive interval + */ public function get_keep_alive() { return $this->keep_alive; } - // getter for private variables - public function get_ts() + /** + * Getter for remote IP saved with this session + */ + public function get_ip() + { + return $this->ip; + } + + /** + * Setter for cookie encryption secret + */ + function set_secret($secret) { - return $this->changed; + $this->secret = $secret; } - // getter for private variables - public function get_ip() + + /** + * Enable/disable IP check + */ + function set_ip_check($check) { - return $this->ip; + $this->ip_check = $check; + } + + /** + * Setter for the cookie name used for session cookie + */ + function set_cookiename($cookiename) + { + if ($cookiename) + $this->cookiename = $cookiename; + } + + + /** + * Check session authentication cookie + * + * @return boolean True if valid, False if not + */ + function check_auth() + { + $this->cookie = $_COOKIE[$this->cookiename]; + $result = $this->ip_check ? $_SERVER['REMOTE_ADDR'] == $this->ip : true; + + if ($result && $this->_mkcookie($this->now) != $this->cookie) { + // Check if using id from previous time slot + if ($this->_mkcookie($this->prev) == $this->cookie) + $this->set_auth_cookie(); + else + $result = false; + } + + return $result; + } + + + /** + * Set session authentication cookie + */ + function set_auth_cookie() + { + $this->cookie = $this->_mkcookie($this->now); + rcmail::setcookie($this->cookiename, $this->cookie, 0); + $_COOKIE[$this->cookiename] = $this->cookie; + } + + + /** + * Create session cookie from session data + * + * @param int Time slot to use + */ + function _mkcookie($timeslot) + { + $auth_string = "$this->key,$this->secret,$timeslot"; + return "S" . (function_exists('sha1') ? sha1($auth_string) : md5($auth_string)); } } -- cgit v1.2.3