Database Session Class

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:

```
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 =

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.

StackExchange Code Review Q#116486, answer score: 2