1
\$\begingroup\$

I have this class that I've been working on for the past 2 or 3 days, now it's working I just want to know what you think about it, what other methods should I add, are the basic security checks impplemented correctly? etc etc

    /*
 * 
 * Author: Carlos Arturo Alaniz
 * Contact info: caag1993@gmail.com; @alaniz93;  caag1993.wordpress.com
 * Last Modified date: 06 /03 /2013
 * 
 * Description:
 *  This class replaces the standard php session managment system
 *  it uses a Mysql Database to stor the session info
 * 
 *  The data encryopptuion can be turned off by passing a NULL encription
 *  key, the cookies and session can be set to be permanent by passing a 
 *  true value to the start session method.
 *  
 *  It requieres a PDO object to stablish the connection to the DB
 *  it also implements 2 very basic security checks to prevent session
 *  related attacks 
 *  it store the SERVER_USER_AGENT with every newly opened session to be
 *  later compared, it also stores a cookie counter on the client side and 
 *  session counter on the server side both of them must be syncronized
 *  the data its encrypted with a private key and it sets the php.ini
 *  file to a secure configuration.
 * 
 */



class _session {

private $key = NULL;
private $permanent = FALSE;
private $lifetime = 0;

public function __construct(&$connecion_settings, $key = NULL) {
    $this->key = $key;
    $this->pdo = $connecion_settings;
    //set our new session handler
    session_set_save_handler(
            array($this, 'open'), array($this, 'close'), array($this, 'read'), array($this, 'write'), array($this, 'destroy'), array($this, 'gc')
    );
    //Set php.in to secure setting.
    //This will tell PHP not to include the identifier in the URL, and not to read the URL for identifiers.
    if (ini_get('session.use_trans_sid'))
        ini_set('session.use_trans_sid', 0);

    //This will tell PHP to never use URLs with session identifiers.
    if (!ini_get('session.use_only_cookies'))
        ini_set('session.use_only_cookies', 1);
    //Using a strong session hash identifier
    //PHP <= 5.3
    // ini_set('session.hash_function', 1);
    // PHP > 5.3
    ini_set('session.hash_function', 'sha256');

    // Send a strong hash
    ini_set('session.hash_bits_per_character', 5);
}

public function start_session($sessionName = NULL, $permanent = FALSE) {
    if ($permanent) {
        $this->permanent = $permanent;
        $this->lifetime = time() + 86400 * 365 * 2; //2 years
        session_set_cookie_params($this->lifetime);
    }
    if ($sessionName != NULL)
        session_name($sessionName);
    session_start();
    if (!isset($_SESSION['_userAgent']))
        $_SESSION['_userAgent'] = $_SERVER['HTTP_USER_AGENT'];
    if (!isset($_SESSION['_counter'])) {
        $_SESSION['_counter'] = 0;
        setcookie("sessionCounter", "", time() - 7200); //unset cookie
        setcookie('sessionCounter', 0, $this->lifetime, '/', NULL, NULL, 1);
    }
}

private function updateCounter() {
    $cookieCount = $_COOKIE['sessionCounter'] + 1;
    $_SESSION['_counter'] += 1;
    setcookie("sessionCounter", "", time() - 7200); //unset cookie
    setcookie('sessionCounter', $cookieCount, $this->lifetime, '/', NULL, NULL, 1); //set new value
}

/*
  //This function implements some basic sessions security checks
  //it checks for anychange in the user agent variable and the one stored in the session
  //it implements a counter on the server side and one on the client side
  //both of them must be the same.
 */

public function securityCheck() {
    $this->updateCounter();
    if (($_SESSION['_userAgent'] != $_SERVER['HTTP_USER_AGENT']) || ($_SESSION['_counter'] != ($_COOKIE['sessionCounter'] + 1))
    ) {
        session_destroy();
        session_write_close();
        //Prompt for password or do something
        //echo "DUH!";
    }
    // else
    // echo "wereGood";
}

public function open() {
    //This should be a contructor but the connection 'open'
    //settings are in the $connetion variable
    return 0;
}

public function close() {
    return session_write_close();
}

public function read($sessionId) {
    $qry = "SELECT data
           FROM sessions
           WHERE id = :id";

    //We want to only prepare the statement once
    if (!isset($this->rStatement)) {
        $this->rStatement = $this->pdo->prepare($qry);
    }

    $this->rStatement->bindParam(':id', $sessionId, PDO::PARAM_INT);

    if ($this->rStatement->execute()) {
        $row = $this->rStatement->fetch(PDO::FETCH_ASSOC);
        if ($this->key != NULL)
            return $this->decrypt($row['data']);
        else
            return $row['data'];
    }
    else {
        return false;
    }
}

public function write($sessionId, $data) {
    if ($this->key != NULL)
        $data = $this->encrypt($data);

    $qry = "REPLACE INTO sessions (id, set_time, data, permanent) VALUES (:id, :time, :data, :permanent)";
    $time = time();

    //We want to only prepare the statement once
    if (!isset($this->wStatement)) {
        $this->wStatement = $this->pdo->prepare($qry);
    }
    $this->wStatement->bindParam(':id', $sessionId, PDO::PARAM_STR);
    $this->wStatement->bindParam(':time', $time, PDO::PARAM_INT);
    $this->wStatement->bindParam(':data', $data, PDO::PARAM_STR);
    $this->wStatement->bindParam(':permanent', $this->permanent, PDO::PARAM_BOOL);
    if ($this->wStatement->execute()) {
        return true;
    } else {
        echo "error";
        return false;
    }
}

public function destroy($sessionId) {
    $qry = "DELETE FROM sessions WHERE id = :id";
    //We want to only prepare the statement once
    if (!isset($this->dStatement)) {
        $this->dStatement = $this->pdo->prepare($qry);
    }
    $this->dStatement->bindParam(':id', $sessionId, PDO::PARAM_INT);
    if ($this->dStatement->execute()) {
        unset($_SESSION);   //unset session variables
        setcookie("sessionCounter", "sdf", time() - 7200); //unset cookie
        return true;
    } else {
        return false;
    }
}

public function gc($max) {
    $qry = "DELETE FROM sessions WHERE set_time < :max AND permanent = 0";

    //We want to only prepare the statement once
    if (!isset($this->gcStatement)) {
        $this->gcStatement = $this->pdo->prepare($qry);
    }
    $this->gcStatement->bindParam(':max', $max, PDO::PARAM_INT);
    if ($this->gcStatement->execute()) {
        return 'true';
    } else {
        return false;
    }
}

private function encrypt($data) {
    $iv_size = mcrypt_get_iv_size(MCRYPT_RIJNDAEL_256, MCRYPT_MODE_ECB);
    $iv = mcrypt_create_iv($iv_size, MCRYPT_RAND);
    $en = base64_encode(mcrypt_encrypt(MCRYPT_RIJNDAEL_256, $this->key, $data, MCRYPT_MODE_ECB, $iv));
    return $en;
}

private function decrypt($data) {
    $iv_size = mcrypt_get_iv_size(MCRYPT_RIJNDAEL_256, MCRYPT_MODE_ECB);
    $iv = mcrypt_create_iv($iv_size, MCRYPT_RAND);
    $des = mcrypt_decrypt(MCRYPT_RIJNDAEL_256, $this->key, base64_decode($data), MCRYPT_MODE_ECB, $iv);
    return $des;
}


}

http://pastebin.com/JZ7tuLb0

\$\endgroup\$
1
\$\begingroup\$

If you are using PHP >= 5.4, you can look at the SessionHandlerInterface, which will allow you to interface and extend PHP's default session handling mechanism.

Your code looks good, you are using PDO which is advisable. You may have to perform type hinting in the constructor on the PDO connection, so the class knows what to expect. Further, there is no benefit from passing the connection in by reference. You may want to declare private $pdo = null; as well.

The script has some documentation in, however could benefit from DocBlocks. You may want to check that there unnecessary echo where a method is returning a boolean, and be careful in using return 'true'; vs return true; - note that they are not the same.

\$\endgroup\$
1
  • \$\begingroup\$ thanks for the feedback, I'll clean the script a little bit more before implementing it, i've just adden a comment blokc in the top with a description. :) \$\endgroup\$ Jul 3 '13 at 22:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.