From f75bc5c24b126a1de7b0f9e92f4453b642ae7227 Mon Sep 17 00:00:00 2001 From: Aleksander Machniak Date: Tue, 8 Sep 2015 09:38:45 +0200 Subject: [PATCH] Use random security tokens instead of hashes based on encryption key (#1490404) --- CHANGELOG | 4 +- program/lib/Roundcube/rcube.php | 13 ++--- program/lib/Roundcube/rcube_session.php | 78 ++++++------------------- 3 files changed, 27 insertions(+), 68 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index f97f1fdf0..6472e26d0 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,7 +1,8 @@ CHANGELOG Roundcube Webmail =========================== -- Improved encrypt/decrypt methods with option to choose the cipher_method (#1489719) +- Security: Use random security tokens instead of hashes based on encryption key (#1490404) +- Security: Improved encrypt/decrypt methods with option to choose the cipher_method (#1489719) - Make optional adding of standard signature separator - sig_separator (#1487768) - Optimize folder_size() on Cyrus IMAP by using special folder annotation (#1490514) - Make optional hidding of folders with name starting with a dot - imap_skip_hidden_folders (#1490468) @@ -13,7 +14,6 @@ CHANGELOG Roundcube Webmail - Remove common subject prefixes Re:, Re[x]:, Re-x: on reply (#1490497) - Added GSSAPI/Kerberos authentication plugin - krb_authentication - Password: Allow temporarily disabling the plugin functionality with a notice -- Support more secure hashing algorithms for auth cookie - configurable by PHP's session.hash_function (#1490403) - Require Mbstring and OpenSSL extensions (#1490415) - Get rid of Mail_mimeDecode package dependency (#1490416) - Add --config and --type options to moduserprefs.sh script (#1490051) diff --git a/program/lib/Roundcube/rcube.php b/program/lib/Roundcube/rcube.php index 37b5a3af0..09c8f903b 100644 --- a/program/lib/Roundcube/rcube.php +++ b/program/lib/Roundcube/rcube.php @@ -900,15 +900,14 @@ class rcube */ public function get_request_token() { - $sess_id = $_COOKIE[ini_get('session.name')]; - if (!$sess_id) { - $sess_id = session_id(); - } + if (empty($_SESSION['request_token'])) { + $plugin = $this->plugins->exec_hook('request_token', array( + 'value' => rcube_utils::random_bytes(32))); - $plugin = $this->plugins->exec_hook('request_token', array( - 'value' => md5('RT' . $this->get_user_id() . $this->config->get('des_key') . $sess_id))); + $_SESSION['request_token'] = $plugin['value']; + } - return $plugin['value']; + return $_SESSION['request_token']; } /** diff --git a/program/lib/Roundcube/rcube_session.php b/program/lib/Roundcube/rcube_session.php index 93a37d626..af6a49b1d 100644 --- a/program/lib/Roundcube/rcube_session.php +++ b/program/lib/Roundcube/rcube_session.php @@ -43,10 +43,10 @@ abstract class rcube_session protected $gc_enabled = 0; protected $gc_handlers = array(); protected $cookiename = 'roundcube_sessauth'; - protected $secret = ''; protected $ip_check = false; protected $logging = false; + /** * Blocks session data from being written to database. * Can be used if write-race conditions are to be expected @@ -87,9 +87,6 @@ abstract class rcube_session { $this->config = $config; - // set secret - $this->set_secret($this->config->get('des_key') . dirname($_SERVER['SCRIPT_NAME'])); - // set ip check $this->set_ip_check($this->config->get('ip_check')); @@ -558,9 +555,19 @@ abstract class rcube_session /** * Setter for cookie encryption secret */ - function set_secret($secret) + function set_secret($secret = null) { - $this->secret = $secret; + // generate random hash and store in session + if (!$secret) { + if (!empty($_SESSION['auth_secret'])) { + $secret = $_SESSION['auth_secret']; + } + else { + $secret = rcube_utils::random_bytes(strlen($this->key)); + } + } + + $_SESSION['auth_secret'] = $secret; } /** @@ -621,7 +628,7 @@ abstract class rcube_session /** * Set session authentication cookie */ - function set_auth_cookie() + public function set_auth_cookie() { $this->cookie = $this->_mkcookie($this->now); rcube_utils::setcookie($this->cookiename, $this->cookie, 0); @@ -629,8 +636,7 @@ abstract class rcube_session } /** - * Create session cookie from session data. - * The cookie will be hashed using a method defined by session.hash_function. + * Create session cookie for specified time slot. * * @param int Time slot to use * @@ -638,57 +644,11 @@ abstract class rcube_session */ protected function _mkcookie($timeslot) { - $auth_string = "$this->key,$this->secret,$timeslot"; - $hash_method = (string) ini_get('session.hash_function'); - $hash_nbits = (int) ini_get('session.hash_bits_per_character'); + // make sure the secret key exists + $this->set_secret(); - if (!$hash_method) { - $hash_method = 'md5'; - } - else if ($hash_method === '1') { - $hash_method = 'sha1'; - } - - if ($hash_nbits < 4 || $hash_nbits > 6) { - $hash_nbits = 4; - } - - // Hash the authentication string - $hash = openssl_digest($auth_string, $hash_method, true); - - // Above method returns "hexits". To be really compatible - // with session hashes generated by PHP core we'll get - // a raw (binary) hash and convert it to a readable form - // as in bin_to_readable() function in ext/session/session.c. - $hextab = "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ,-"; - $length = strlen($hash); - $result = ''; - $char = 0; - $i = 0; - $have = 0; - $mask = (1 << $hash_nbits) - 1; - - while (true) { - if ($have < $hash_nbits) { - if ($i < $length) { - $char |= ord($hash[$i++]) << $have; - $have += 8; - } - else if (!$have) { - break; - } - else { - $have = $hash_nbits; - } - } - - // consume nbits - $result .= $hextab[$char & $mask]; - $char >>= $hash_nbits; - $have -= $hash_nbits; - } - - return $result; + // no need to hash this, it's just a random string + return $_SESSION['auth_secret'] . '-' . $timeslot; } /**