Checking User Agent and regenerating session id in Session Class

Question

I have written a class to store sessions in a database. I am stuck as to whether my checking of the user agent will work, as I cannot think of a way to test it.

I am also concerned that session_regenerate_id() is being called on every session_start() and am confused by the manual's warnings regarding object destruction and the need for session_register_shutdown().

Will my user agent check always match?

Where is a better place for regenerating the session id?

Is the constructor a good place for session_register_shutdown()?

Thanks in advance.

The code:

Session.class.php

<?php
namespace Company\Project;

use \PDO;


class Session
{
    private $dblayer;
    private $user_agent;

    /**
     * Session constructor.
     * @param PDO $dblayer
     */
    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();
        session_regenerate_id(true);

    }

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

        return false;
    }

    /**
     * @return bool
     */
    public function close()
    {
        $this->dblayer = null;
        return true;
    }

    /**
     * @param $id
     * @return bool|string
     */
    public function read($id)
    {
        try {
            $this->dblayer->beginTransaction();
            $stmt = $this->dblayer->prepare("SELECT data, user_agent FROM sessions WHERE id = :id LIMIT 1");
            $stmt->bindParam(':id', $id);
            $stmt->execute();

            $this->dblayer->commit();

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

            if ($original_user_agent != $this->user_agent) {
                session_destroy();
                header('Location:' . SITE_PATH . '/login.php');
                exit;
            }

            return $data;

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

    /**
     * @param $id
     * @param $data
     * @return bool
     */
    public function write($id, $data)
    {
            $access = time();
            $user_agent = $_SERVER['HTTP_USER_AGENT'];
            $this->dblayer->beginTransaction();
            $stmt = $this->dblayer->prepare("REPLACE INTO sessions VALUES(:id, :data, :user_agent, :access)");
            $stmt->bindParam(':id', $id);
            $stmt->bindParam(':data', $data);
            $stmt->bindParam(':user_agent', $user_agent);
            $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;

    }

    /**
     * @param $id
     * @return bool
     */
    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;
        }
    }

    /**
     * @param $max
     * @return bool
     */
    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;
        }
    }


}

Answer 1

You have three questions here (at least):

Testing your User Agent check

To test, you can change your browsers User Agent; in Safari, there are a few options available in the Debug menu, Chrome and Firefox may have similar, or config options, but definitely have plugins to do this.

However, I would recommend against this type of checking (basic fingerprinting) — there are no guarantees that the user agent will not change — what happens if their browser installs an update mid-session? Frankly, if you can take over a users session, especially with the regeneration of session of IDs, you can spoof the users user agent.

Regenerating Session IDs in constructor

While this is fine, at high load it may have a performance impact — a better option if you can is to only regenerate session IDs when you have an escalation of privileges, for example:

• On login
• When accessing user profile page
• When changing password
• When creating new content
• When checking out
• When logging out

Using session_register_shutdown(); in constructor

While the manual says that it's better to use session_register_shutdown(); (or register_shutdown_function('session_write_close') in older versions) I actually disagree with this. It is possible to register multiple shutdown functions and they will be called in the order added with register_shutdown_function() — however, if one of them calls exit then no more are called.

I think it's better to call session_write_close() in your destructor instead,

class Session
{
    … 

    public function __destruct()
    {
         session_write_close();
    }
}

This will get called even on after exit is called, when the stack is unraveled.