How to regenerate my session ID often when using this class?

Question

I have created a class that will manage my session. I am hoping to acomplish a class that will secure my site from all known attacks (ie. fixation, precedent, and capture.)

The idea is to

1. Change the session storage engine from files on the servers to a database.
2. Create a random session id and not relay on PHP to generate the session id for me.
3. Change the length of the session id to the max allowed (ie. 40 Characters)
4. Store the new session id into a table along with a user_id (application user identifier,) along with the user's IP address and user agent info.
5. Set an idle time out so that the system will automatically destroy the user session after 1800 seconds of idle time.
6. In addition, I will only use SSL connection to ensure the session_ids are not sniffed

After implementing the above, for every requested page I check for the following to be true

1. A valid session ID in the user's Browser.
2. Make sure I can match the current IP address and the current user agent info to those info that were captured at the time of login.
3. If the session is valid and not expired then use it otherwise create a new one if the user_is is available (aka a log in attempt.)

Can you please review my code below and tell me

1. How can I regenerate the session id often? do I really need to do this?
2. Is this class secured? if not how can I improve it?

<?php

ini_set('session.hash_function', 1);
ini_set('session.hash_bits_per_character', 4);

class sessionManager {
    private $db;
    private $user_id;
    private $user_ip;
    private $user_agent;
    private $autherizedUser = false;
    private $cookie_name;
    private $current_session_id;
    private $max_session_idle_time = 1800;
    private $current_time;


    public function __construct($name, $limit = 0, $path = '/', $domain = null, $secure = null){
        // Set the cookie name
        session_name($name);

        //assign the cookie name that will be used for the session
        $this->cookie_name = $name;

        //get the current time
        $this->current_time = time();

        if(isset($_SERVER['REMOTE_ADDR']))
            $this->user_ip = $_SERVER['REMOTE_ADDR'];

        if(isset($_SERVER['HTTP_USER_AGENT']))
            $this->user_agent = $_SERVER['HTTP_USER_AGENT'];

        // Set SSL level
        $https = isset($secure) ? $secure : isset($_SERVER['HTTPS']);

        //set the session storage to point custom method
        session_set_save_handler(
            array($this, "open"),
            array($this, "close"),
            array($this, "read"),
            array($this, "write"),
            array($this, "delete"),
            array($this, "garbageCollector")
        );

        //Set session cookie options
        session_set_cookie_params($limit, $path, $domain, $https, true);

        //if there is no IP detected - make it invalid
        if( empty($this->user_ip) || empty($this->user_agent)  ){
            echo 'Invalid Request!!!';
            exit();
        }
    }

    //This function resume existing session
    public function resumeSession(){

        //if this it not an attempt to log in then resume session
        if(session_id() == '' )
            session_start();

        //grab the current session_id            
        $this->current_session_id = session_id();

        // Make sure the session hasn't expired, and destroy it if it has    
        if( $this->isValidSession()  ){

            if($this->isHijacking()){
                //destroy the session
                $this->destroy();
            } else {
                //reset the idle time out
                $_SESSION['MA_IDLE_TIMEOUT'] = $this->current_time + $this->max_session_idle_time;
                $this->autherizedUser = true;
            }

        } 
    }

    //This function starts a new session - on the login
    //@userid is the logged in user id    
    public function startNewSession($userid){
        //Set the user id
        $this->user_id = $userid;
        $new_session_id = $this->generateSessionID();
        session_id($new_session_id);
        //grab the current session_id    
        $this->current_session_id = $new_session_id;
        //echo 'New';
        //exit();
        session_start();
        $this->setSessionValues();
        $this->autherizedUser = true;
    }

    //This function destroy existing session
    public function destroy(){
        if(session_id() == '' )
            session_start();

        $this->autherizedUser = false;
        session_unset();
        session_destroy();
        unset($_COOKIE[$this->cookie_name]);
    }


    //This function set a new values to the session
    private function setSessionValues(){

        $_SESSION = array();

        //set the IP address info
        $_SESSION['MA_IP_ADDRESS'] = $this->user_ip;

        // save the agent information
        $_SESSION['MA_USER_AGENT'] = $this->user_agent;

        //set the idle timeout
        $_SESSION['MA_IDLE_TIMEOUT'] = $this->current_time + $this->max_session_idle_time;
    }

    //This function check if the current session is valid or not
    private function isValidSession(){

        if( empty($this->current_session_id) )
            return false;

        if( !isset($_SESSION['MA_IP_ADDRESS']) || !isset($_SESSION['MA_USER_AGENT']) || !isset($_SESSION['MA_IDLE_TIMEOUT']) )
            return false;

        if( empty($_SESSION['MA_IP_ADDRESS']) || empty($_SESSION['MA_USER_AGENT']) || empty($_SESSION['MA_IDLE_TIMEOUT']) )
            return false;

        //if the session expired - make it invalid
        if( $_SESSION['MA_IDLE_TIMEOUT'] < $this->current_time   )
            return false;

        //the session is valid
        return true;
    }


    //This function check if this is a session Hijacking attempt or nor
    private function isHijacking(){

        //if the set IP address no not match the current user's IP address value - make it invalid
        if( $_SESSION['MA_IP_ADDRESS'] != $this->user_ip )
            return true;

        //if the set user agent value do not match the current user agent value - make it invalid
        if(   $_SESSION['MA_USER_AGENT'] != $this->user_agent  )
            return true;

        //the session is valid
        return false;
    }

    public function isAutherized(){
        return $this->autherizedUser;
    }

    public function currentSessionID(){
        return $this->current_session_id;
    }

    //This function generate new random string
    private function generateSessionID($len = 40) {
        $characters = '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ,-';
        $newStr = '';
        $maxLen = strlen($characters)-1;
        for ($i = 0; $i < $len; ++$i) 
            $newStr .= $characters[rand(0, $maxLen)];

        return $newStr;
    }

    //open the database connection for the session storage engine
    public function open(){
        $this->db = new connection();
        if($this->db)
            return true;

        // Return False
        return false;
    }

    //close the database connection for the session storage engine
    public function close(){

        if($this->db->endConnection())
            return true;

        // Return False
        return false;
    }

    //read current session variables from the session database
    public function read($id){
        // Set query
        $data = $this->db->getDataSet('SELECT data FROM sessions WHERE session_id = ?', array($id));

        if(count($data) == 1)
                return $data[0]['data'];

        return '';
    }

    //replace the existing data using the current session id
    public function write($id, $data){

        // Set query  
        $replace = $this->db->processQuery('INSERT INTO sessions(session_id, access, data, user_id) VALUES (?, ?, ?, ?)
                                            ON DUPLICATE KEY UPDATE
                                            session_id = ?,
                                            access = ?,
                                            data = ?', array($id, $this->current_time, $data, $this->user_id, $id, $this->current_time, $data));

        if($replace)
            return true;

        // Return False
        return false;
    }

    //delete a session record from the storage engine
    public function delete($id){
        // Set query
        $delete = $this->db->processQuery('DELETE FROM sessions WHERE session_id = ? OR user_id IS NULL', array($id));

        if($delete)
            return true;

        // Return False
        return false;
    } 

    //deletes all expired session - if the access time is less that current time
    public function garbageCollector($max){
        // Calculate what is to be deemed old
        $old = $this->current_time - $max;
        // Set query
        $delete = $this->db->processQuery('DELETE FROM sessions WHERE access < ?', array($old));

        if($delete)
            return true;

        // Return False
        return false;
    }    

}

?>

The following are the steps that I follow to use the class.

On the login page and after the user name and password match the database. I start a new session:

$session = new sessionManager('test_session', 0, '/uat/', 'uat.domain.com', true);  
$session->startNewSession($info['user_id']);

if( $session->isAutherized() === true ){
    header('Location: index.php');
    exit();
}

Then on every page I do the following to check if the info are valid:

$session = new sessionManager(SESSION_NAME, SESSION_MAX_AVAILABLE, SESSION_SAVE_PATH, SESSION_SAVE_URL, SESSION_HTTPS_ONLY);
$session->resumeSession();
$session_id = $session->currentSessionID();

if( $session->isAutherized() === false ){
    $session->destroy();
    header('Location: '.ABSOLUTE_PATH.'login.php');
    exit();
}

Following to this code I do a database lookup using the $session_id value:

SELECT u.user_id, u.name, u.access_level
FROM sessions AS s
INNER JOIN users AS u ON u.user_id = s.user_id
WHERE s.session_id = $session_id

Answer 1

I like this question because I think it has a lot of answering potential. I'm going to touch on a few things, but my answer shouldn't be considered complete!

Security of custom sessions

There's one thing that's most important here: don't roll out a security system that's been built with only one pair of hands. It's asking for a disaster! I won't do the research for you, but I suggest you look at other more established session customization libraries.

Head over to Github and start searching. You're looking for a library that meets your requirements, plus has some sort of accreditation. The more users using the code, the safer it will be.

Posting code here on Code Review is a start, but it's not enough. you'll want to see a security expert before implementing your own security guards, especially if you have data worth protecting. (That includes users' passwords!)

---

So are there any flaws that the naked eye can see in your code? Most definitely! Let's point out a few:

• You're relying a lot on the user here: $_SERVER['REMOTE_ADDR'], $_SERVER['HTTP_USER_AGENT'], and $_SESSION.

  These come from the user, therefore I'd advise not trusting these solely 100%, as it's unlikely they'll be consistent/reliable/safe.

• I don't like the way generateSessionID() looks. rand() is hardly random, and therefore predictable.

• Perhaps you've read this already, but here's a good session hijacking article. It's a couple years old, but I think it has some useful links.Don't pour all of this in your code, as I'm not sure how much is outdated.

Coding style

There's a lot to say here, but I'll try and make a quick summary:

Follow a coding style convention (Zend, PHP-FIG, etc.), be consistent, and DRY out your code.

Examples? Sure:

1. Comments are very important. Stuff like //This function resume existing session doesn't help the reader. Consider using PHPDocs, and keep the comments to only necessary remarks that tell why, not how.

   Classes are normally CapitalCase, so sessionManager wouldn't fit convention.

2. Is it $user_agent or $autherizedUser? What style will you choose? :)

3. This one is very apparent in your code! It really sticks out in places like:

   if($this->db)
       return true;
   
   // Return False
   return false;
   

   Why not:

   return isset($this->db);
   

   Notice I left out the comment too. It was useless to have a comment like that. The last thing comments should do is reiterate what the code does!

   There was no reason to have two return statements.

---

And I'll wrap it up there! Hopefully we get the gist of what needs to be done!