From ead753dc3d3d141900782c2a42d946077859b073 Mon Sep 17 00:00:00 2001
From: Graham Cole <chckens@sucs.org>
Date: Wed, 16 Jan 2008 20:37:27 +0000
Subject: [PATCH] Attempt to fix some issues with the session library,
including:
- Begin to stop it being so logout happy for ordinary users who aren't doing anything particularly sensitive on the site by keeping track of when a user was last asked for credentials
- Don't continue to use the same session identifier once a user is logged in; it's likely been sent insecurely
- Mark session cookies as "SSL only" once logged in
- Automatically bump users from HTTP to HTTPS for all requests whilst they're logged in
---
lib/session.php | 100 ++++++++++++++++++++++++++++++++++--------------
1 file changed, 72 insertions(+), 28 deletions(-)
diff --git a/lib/session.php b/lib/session.php
index 5e98f6a..f8bd2e9 100644
--- a/lib/session.php
+++ b/lib/session.php
@@ -8,7 +8,7 @@
// other attributes are :
// username - the username they logged in with
// fullname - whatever full name we know for them
-// last - unix timestamp for their previous page access
+// lastseen - unix timestamp for their previous page access
// data - var/array for persistant data, commit by calling the 'save' method
@@ -22,35 +22,54 @@ public $email_forward; // Email forwarded?
public $groups =array(); // users groups
public $data=''; // Var/array for session persistant data
public $token=''; // session identifier
-public $last=''; // Time of last page request
-private $timeout = 300; // Idle timeout limit in minutes
+public $logintime=''; // Time which user last gave us credentials
+public $lastseen=''; // Time of last page request
+private $timeout = 2880; // Idle timeout limit in minutes (session deleted)
+private $secure_timeout = 30; // Idle timeout limit in minutes (consider session less secure, require reauth for sensitive ops)
private $table = "session"; // session storage table (const)
private $datahash=''; // hash of data field
- // Create a new session id
+
+
+ // Create a new (insecure) session
private function newsession()
{
global $DB;
- $try = 0;
- do {
- $tt=date("D M d H:i:s Y");
- $ip = $_SERVER['REMOTE_ADDR'];
- $token = md5("$ip$tt$try");
- $old = $DB->GetAll("select hash from session where hash=?", array($token));
- }while ($old);
- $DB->Execute("insert into session (hash, time, ip) values (?,NOW(),?)", array($token, $ip));
- setcookie("session", $token, NULL, "/");
+ $token = $this->genSessionID();
+ $DB->Execute("insert into {$this->table} (hash, lastseen, ip) values (?,NOW(),?)", array($token, $_SERVER['REMOTE_ADDR']));
+ setcookie("sucssite_session", $token, NULL, "/");
$this->token = $token;
return;
}
+ public function isSecure()
+ {
+ global $DB;
+ // is user coming from the IP address they were when they logged in?
+ if ($detail['ip'] != $_SERVER['REMOTE_ADDR']) {
+ return false;
+ } elseif (time() > ($self->logintime + $secure_timeout)) {
+ // has it been too long since we last asked for credentials?
+ return false;
+ }
+
+ }
+
// Public Object constructor
function __construct()
{
- global $DB;
+ global $DB, $preferred_hostname, $baseurl;
unset($token);
+ // if user requests a page via HTTP and claims to be logged in, bump them to HTTPS
+ if (!isset($_SERVER['HTTPS']) && (@$_COOKIE['sucssite_loggedin'] == "true")) {
+ header("HTTP/1.0 307 Temporary redirect");
+ header("Location: https://{$preferred_hostname}{$baseurl}{$_SERVER['PATH_INFO']}");
+ return;
+ }
+
+
// The possible form elements
$submit = @$_POST['Login'];
$logout = @$_POST['Logout'];
@@ -61,7 +80,7 @@ private $datahash=''; // hash of data field
$this->loggedin = FALSE;
// Time out any old sessions
- $DB->Execute("delete from {$this->table} where time < NOW() - '{$this->timeout} minutes'::reltime");
+ $DB->Execute("delete from {$this->table} where lastseen < NOW() - '{$this->timeout} minutes'::reltime");
// Log them out if they ask
if ($logout=="Logout") {
@@ -75,8 +94,8 @@ private $datahash=''; // hash of data field
// Check if we were handed a specific token identifier
// Otherwise use the value from the cookie we gave out
- if (!isset($token) && isset($_COOKIE['session']))
- $token=@$_COOKIE['session'];
+ if (!isset($token) && isset($_COOKIE['sucssite_session']))
+ $token=@$_COOKIE['sucssite_session'];
if (isset($token)) $this->token = $token;
@@ -106,7 +125,8 @@ private $datahash=''; // hash of data field
// Extract detail of session for pass-back
$detail = $oldsess[0];
$this->data = unserialize((string)$detail['data']);
- $this->last = strtotime($detail['time']);
+ $this->lastseen = strtotime($detail['lastseen']);
+ $this->logintime = strtotime($detail['logintime']);
$this->datahash = md5(serialize($this->data));
// are we actually logged in, fill in more
@@ -117,19 +137,31 @@ private $datahash=''; // hash of data field
$this->loggedin = FALSE;
return;
}
- // User is valid but they're coming from the wrong IP
- if ($detail['ip'] != $_SERVER['REMOTE_ADDR']) {
- $this->errormsg = "Your IP address has changed - you have been logged out";
- $this->logout();
- return;
- }
$this->username=$detail['username'];
$this->fetch_detail($detail['username']);
$this->loggedin = TRUE;
}
// update time stamp
- $DB->Execute( "update {$this->table} set time=NOW() where hash=?", array($this->token));
+ $DB->Execute( "update {$this->table} set lastseen=NOW() where hash=?", array($this->token));
+ }
+
+ // generate a string suitable to be used as a session ID
+ private function genSessionID()
+ {
+ global $DB;
+ $try = 0;
+
+ $tt=date("D M d H:i:s Y");
+ $ip = $_SERVER['REMOTE_ADDR'];
+ $nonce = rand(); // this should stop session IDs being (easily) guessable by someone with the algorithm
+
+ do {
+ $token = md5("$ip$tt$nonce".$try++);
+ $old = $DB->GetAll("select hash from {$this->table} where hash=?", array($token));
+ } while ($old);
+
+ return $token;
}
// Public function: Store the session data away in the database
@@ -150,9 +182,10 @@ private $datahash=''; // hash of data field
public function logout( )
{
global $DB;
- $DB->Execute("delete from session where hash=?", array($this->token));
+ $DB->Execute("delete from {$this->table} where hash=?", array($this->token));
$this->newsession();
$this->loggedin = FALSE;
+ setcookie("sucssite_loggedin", "false");
}
// Fill out any extra details we know about the user
@@ -259,7 +292,7 @@ private $datahash=''; // hash of data field
// Private function: process login form
private function session_init($user, $pass)
{
- global $DB;
+ global $DB, $preferred_hostname;
// Check that this is a valid session start
// This prevents replay attacks
$sess = $DB->GetAll("select * from {$this->table} where hash=? and username is NULL", array($this->token));
@@ -271,8 +304,19 @@ private $datahash=''; // hash of data field
if (!$this->check_pass($user, $pass)) return;
$this->username = $user;
+ // the token has likely been used on an insecure connection
+ // so generate a new one with the secure flag set
+ $oldtoken = $this->token;
+ $this->token = $this->genSessionID();
+ setcookie("sucssite_session", $this->token, NULL, "/", $preferred_hostname, TRUE);
+
+ // set a cookie as a hint that we're logged in
+ // this can be checked for to allow redirecting to SSL to get the secure cookie
+ setcookie("sucssite_loggedin", "true");
+
// Update the session, filling in the blanks
- $DB->Execute("update {$this->table} set username=?, time='NOW()', ip=? where hash=?", array($this->username, $_SERVER['REMOTE_ADDR'], $this->token));
+ $DB->Execute("update {$this->table} set hash=?, username=?, logintime='NOW()', lastseen='NOW()', ip=? where hash=?",
+ array($this->token, $this->username, $_SERVER['REMOTE_ADDR'], $oldtoken));
// Return back to normal session retrieval
}
--
GitLab