Database Session Class

Question

I have made the decision to move the storing of session data from the filesystem to the database. Our application is growing at pace and we are having issues with the load balancer breaking the sessions (even with sticky sessions on).

Whilst obviously the idea here is to have my code reviewed, I have a few questions/areas I'd really appreciate some feedback on, which I have detailed after the code.

My sessions table:

CREATE TABLE sessions
(
  id VARCHAR(40) NOT NULL,
  data TEXT,
  access TEXT,
  PRIMARY KEY (id),
  INDEX (id)
);

Session class:

<?php
namespace Company\Project;

use \PDO;


class Session
{
    private $dblayer;
    private $user_agent;

    public function __construct(PDO $dblayer)
    {
        $this->dblayer = $dblayer;
        $this->user_agent = $_SERVER['HTTP_USER_AGENT'];


        session_set_save_handler(
            array($this, 'open'),
            array($this, 'close'),
            array($this, 'read'),
            array($this, 'write'),
            array($this, 'destroy'),
            array($this, 'gc')
        );

        if ('LIVE' == DEVELOPMENT_MODE) {
            session_set_cookie_params(0, '/', '', true, true);
        } else {
            session_set_cookie_params(0, '/', '', false, true);
        }

        session_register_shutdown();
        session_start();

    }

    public function checkUserAgent()
    {
        if ($_SERVER['HTTP_USER_AGENT'] === $this->user_agent) {
            session_regenerate_id(true);
            return true;
        }

        return false;
    }

    public function open()
    {
        if ($this->dblayer) {
            return true;
        }

        return false;
    }

    public function close()
    {
        $this->dblayer = null;
        if (!$this->dblayer) {
            return true;
        }

        return false;
    }

    public function read($id)
    {
        try {
            $this->dblayer->beginTransaction();
            $stmt = $this->dblayer->prepare("SELECT data FROM sessions WHERE id = :id LIMIT 1");
            $stmt->bindParam(':id', $id);
            $stmt->execute();

            $this->dblayer->commit();

            if ($data = $stmt->fetch()) {
                return $data['data'];
            }

            return '';

        } catch (\Exception $e) {
            $this->dblayer->rollBack();
            // will use file_put_contents to save error message, file etc to error log
            return false;
        }
    }

    public function write($id, $data)
    {
            $access = time();
            $this->dblayer->beginTransaction();
            $stmt = $this->dblayer->prepare("REPLACE INTO sessions VALUES(:id, :data, :access)");
            $stmt->bindParam(':data', $data);
            $stmt->bindParam(':id', $id);
            $stmt->bindParam(':access', $access);
            $stmt->execute();

            $this->dblayer->commit();

            if ($stmt) {
                return true;
            }
            echo 'error';
            $this->dblayer->rollBack();
            // can i save to error log here?
            return false;

    }

    public function destroy($id)
    {
        try {
            $this->dblayer->beginTransaction();
            $stmt = $this->dblayer->prepare("DELETE FROM sessions WHERE id = :id");
            $stmt->bindParam(':id', $id);
            $stmt->execute();

            $this->dblayer->commit();

        } catch (\PDOException $e) {
            $this->dblayer->rollBack();
            // again, will save error data to log
            echo $e->getMessage();
            return false;
        }
    }

    public function gc($max)
    {
        $to_delete = time() - $max;

        try {
            $this->dblayer->beginTransaction();
            $stmt = $this->dblayer->prepare("DELETE FROM sessions WHERE access < :to_delete");
            $stmt->bindParam(':to_delete', $to_delete);

            $this->dblayer->commit();

            return true;

        } catch (\PDOException $e) {
            $this->dblayer->rollBack();
            // save error data to log;
            return false;
        }
    }

}

Sample usage:

include '../config.php';
$dblayer = dbh_connect();
new \Company\Project\Session($dblayer);

Questions / areas of interest:

1. This manual page seem to strongly suggest using register_shutdown_function or session_register_shutdown, yet most examples I see online of similar classes, do not. Am I using it correctly (or does it need to go in each script where I call this session class)?

2. I always use transactions when querying my database - I assume I'm OK to do so here, too?

3. My close function - the PHP Manual states the function should return true or false, yet I'm not doing anything that could possibly fail, so this seems a bit pointless to me.

4. Currently, I'm not using my check user agent method. Where could I incorporate it?

5. Where would I put session_regenerate_id() when using this class?

6. Are there other steps I can take to increase security? I use a LOT of ajax in this application.

7. I'm concerned about the sessions table becoming very large with the normal session garbage probability - would a cron job to clear out old sessions be better?

Answer 1

I haven't personally implemented a custom session handler, so take everything below with a grain of salt.

3) My close function - the PHP Manual states the function should return true or false, yet I'm not doing anything that could possibly fail, so this seems a bit pointless to me.

If there is nothing that can fail - which I would agree is the case here - just always return true instead of making the code unnecessarily complex.

4) Currently, I'm not using my check user agent method, where could I incorporate it?

First of all, I would remove the call to session_regenerate_id. There is really no reason to regenerate the session id each time you check the useragent.

If you want to bind a session to a user agent*, the best place would probably be read (and then you could destroy the session and return the empty string if it doesn't match).

But then it's important to not use the stored user agent from the Session class - because this will always match - but to store the useragent in the database when first creating the session, and to then compare the current useragent against that stored value.

_{* this would somewhat increase security as it makes it more difficult for an attacker to use leaked sessions. Of course, if the attacker has eg XSS, they can simply read out the user agent (or use CSRF to perform any action the victim can perform); still, it's a good idea as defense in depth.}

5) Where would I put session_regenerate_id() when using this class?

From a security standpoint, you want to regenerate the session id when the session state changes, to prevent session fixation. So you definitely want to do that when the session is created, and on subsequent writes.

Additionally, you could regenerate the id in intervals of X minutes, but a regeneration on each request is not needed and causes unnecessary overhead (and may actually cause problems if requests overlap).

Misc

• is there a reason that you don't use the SessionHandlerInterface? It seems that it would simplify your code a bit.
• the documentation for read says that it must return the empty string when nothing was read, so I would also return that if an exception occurred.