summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAndrey Andreev <narf@devilix.net>2014-05-31 20:00:05 +0200
committerAndrey Andreev <narf@devilix.net>2014-05-31 20:00:05 +0200
commit916b176594bcf175417423f33711ac0cbb4082e7 (patch)
treeba8e1fe4e8457005b1ca8635e7459159fea856e7
parentb4c693c50f54ecce1d0e2c1f203312f4bbb4af22 (diff)
Backport HMAC authentication for CI_Session
-rw-r--r--system/libraries/Session.php56
-rw-r--r--user_guide/changelog.html4
2 files changed, 37 insertions, 23 deletions
diff --git a/system/libraries/Session.php b/system/libraries/Session.php
index 53b914fbd..89c699765 100644
--- a/system/libraries/Session.php
+++ b/system/libraries/Session.php
@@ -144,24 +144,36 @@ class CI_Session {
return FALSE;
}
- // Decrypt the cookie data
- if ($this->sess_encrypt_cookie == TRUE)
+ // HMAC authentication
+ if (($len = strlen($session) - 40) <= 0)
{
- $session = $this->CI->encrypt->decode($session);
+ log_message('error', 'Session: The session cookie was not signed.');
+ return FALSE;
}
- else
+
+ // Check cookie authentication
+ $hmac = substr($session, $len);
+ $session = substr($session, 0, $len);
+
+ // Time-attack-safe comparison
+ $hmac_check = hash_hmac('sha1', $session, $this->encryption_key);
+ $diff = 0;
+ for ($i = 0; $i < 40; $i++)
{
- // encryption was not used, so we need to check the md5 hash
- $hash = substr($session, strlen($session)-32); // get last 32 chars
- $session = substr($session, 0, strlen($session)-32);
+ $diff |= ord($hmac[$i]) ^ ord($hmac_check[$i]);
+ }
- // Does the md5 hash match? This is to prevent manipulation of session data in userspace
- if ($hash !== md5($session.$this->encryption_key))
- {
- log_message('error', 'The session cookie data did not match what was expected. This could be a possible hacking attempt.');
- $this->sess_destroy();
- return FALSE;
- }
+ if ($diff !== 0)
+ {
+ log_message('error', 'Session: HMAC mismatch. The session cookie data did not match what was expected.');
+ $this->sess_destroy();
+ return FALSE;
+ }
+
+ // Decrypt the cookie data
+ if ($this->sess_encrypt_cookie == TRUE)
+ {
+ $session = $this->CI->encrypt->decode($session);
}
// Unserialize the session array
@@ -659,20 +671,20 @@ class CI_Session {
else
{
// if encryption is not used, we provide an md5 hash to prevent userside tampering
- $cookie_data = $cookie_data.md5($cookie_data.$this->encryption_key);
+ $cookie_data .= hash_hmac('sha1', $cookie_data, $this->encryption_key);
}
$expire = ($this->sess_expire_on_close === TRUE) ? 0 : $this->sess_expiration + time();
// Set the cookie
setcookie(
- $this->sess_cookie_name,
- $cookie_data,
- $expire,
- $this->cookie_path,
- $this->cookie_domain,
- $this->cookie_secure
- );
+ $this->sess_cookie_name,
+ $cookie_data,
+ $expire,
+ $this->cookie_path,
+ $this->cookie_domain,
+ $this->cookie_secure
+ );
}
// --------------------------------------------------------------------
diff --git a/user_guide/changelog.html b/user_guide/changelog.html
index 4b3145754..ff6603d8b 100644
--- a/user_guide/changelog.html
+++ b/user_guide/changelog.html
@@ -63,7 +63,8 @@ Change Log
<ul>
<li>General Changes
<ul>
- <li><b>Security:</b> The <samp>xor_encode()</samp> method in the Encyption Class has been removed. The Encryption Class now requires the Mcrypt extension to be installed.</li>
+ <li><b>Security:</b> The <samp>xor_encode()</samp> method in the Encrypt Class has been removed. The Encrypt Class now requires the Mcrypt extension to be installed.</li>
+ <li><b>Security:</b> The <a href="libraries/sessions.html">Session Library</a> now uses HMAC authentication instead of a simple MD5 checksum.</li>
</ul>
</li>
</ul>
@@ -74,6 +75,7 @@ Change Log
<li>Fixed a bug (#696) - make <samp>oci_execute()</samp> calls inside <samp>num_rows()</samp> non-committing, since they are only there to reset which row is next in line for oci_fetch calls and thus don't need to be committed.</li>
<li>Fixed a bug (#2689) - <a href="database/forge.html">Database Forge Class</a> methods <samp>create_table()</samp>, <samp>drop_table()</samp> and <samp>rename_table()</samp> produced broken SQL for tge 'sqlsrv' driver.</li>
<li>Fixed a bug (#2427) - PDO <a href="database/index.html">Database driver</a> didn't properly check for query failures.</li>
+ <li>Fixed a bug in the <a href="libraries/sessions.html">Session Library</a> where authentication was not performed for encrypted cookies.</li>
</ul>
<h2>Version 2.1.4</h2>