From f75bc5c24b126a1de7b0f9e92f4453b642ae7227 Mon Sep 17 00:00:00 2001
From: Aleksander Machniak <alec@alec.pl>
Date: Tue, 08 Sep 2015 03:38:45 -0400
Subject: [PATCH] Use random security tokens instead of hashes based on encryption key (#1490404)
---
program/lib/Roundcube/rcube.php | 13 +++---
CHANGELOG | 4 +-
program/lib/Roundcube/rcube_session.php | 78 +++++++++-----------------------------
3 files changed, 27 insertions(+), 68 deletions(-)
diff --git a/CHANGELOG b/CHANGELOG
index f97f1fd..6472e26 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 @@
- 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 37b5a3a..09c8f90 100644
--- a/program/lib/Roundcube/rcube.php
+++ b/program/lib/Roundcube/rcube.php
@@ -900,15 +900,14 @@
*/
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)));
+
+ $_SESSION['request_token'] = $plugin['value'];
}
- $plugin = $this->plugins->exec_hook('request_token', array(
- 'value' => md5('RT' . $this->get_user_id() . $this->config->get('des_key') . $sess_id)));
-
- return $plugin['value'];
+ return $_SESSION['request_token'];
}
/**
diff --git a/program/lib/Roundcube/rcube_session.php b/program/lib/Roundcube/rcube_session.php
index 93a37d6..af6a49b 100644
--- a/program/lib/Roundcube/rcube_session.php
+++ b/program/lib/Roundcube/rcube_session.php
@@ -43,9 +43,9 @@
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.
@@ -86,9 +86,6 @@
public function __construct($config)
{
$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 @@
/**
* 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 @@
/**
* 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 @@
}
/**
- * 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 @@
*/
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;
}
/**
--
Gitblit v1.9.1