Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Ensuring session strict mode in custom SessionHandlerInterface implementation

Tags:

php

session

Intro

Since PHP 5.5.2 there is a runtime configuration option (session.use_strict_mode) that is meant to prevent session fixation by malicious clients. When this option is enabled and the native session handler is used (files), PHP will not accept any incoming session ID that did not previously exist in the session storage area, like so:

$ curl -I -H "Cookie:PHPSESSID=madeupkey;" localhost
HTTP/1.1 200 OK
Cache-Control: no-store, no-cache, must-revalidate
Connection: close
Content-type: text/html; charset=UTF-8
Expires: Thu, 19 Nov 1981 08:52:00 GMT
Host: localhost
Pragma: no-cache
Set-Cookie: PHPSESSID=4v3lkha0emji0kk6lgl1lefsi1; path=/  <--- looky

(with session.use_strict_mode disabled, the response would have not included a Set-Cookie header and a sess_madeupkey file would have been created in the sessions directory)

Problem

I am in the process of implementing a custom session handler and I'd pretty much like it to adhere to the strict mode, however the interface makes it difficult.

When session_start() is called, MyHandler::read($session_id) is invoked down the line, but $session_id can be either the value fetched from the session cookie or a new session ID. The handler needs to know the difference, because in the former case an error must be raised if the session ID cannot be found. Moreover, according to spec read($session_id) must return either the session contents or an empty string (for new sessions), but there seems to be no way to raise an error up the chain.

So to sum up, the questions I need to answer in order to match the native behavior are:

  1. From the context of read($session_id), how can I tell the difference between a newly minted session ID or a session ID that came from the HTTP request?

  2. Given a session ID that came from the HTTP request and supposing that it was not found in the storage area, how would I signal an error to the PHP engine so that it would call read($session_id) again with a new session ID?

like image 414
Marcel Hernandez Avatar asked Dec 31 '16 20:12

Marcel Hernandez


4 Answers

Seeing that there's an already accepted answer, I'm providing this as a not yet mentioned alternative.


As of PHP 7, if your session handler implements a validateId() method, PHP will use that to determine whether a new ID should be generated or not.

Unfortunately, that doesn't work on PHP 5 where userspace handlers do have to implement use_strict_mode=1 functionality on their own.
There is a shortcut, but let me answer your direct questions first ...

From the context of read($session_id), how can I tell the difference between a newly minted session ID or a session ID that came from the HTTP request?

At first glance, it does seem like this would help, but the issue you'll have here is that read() isn't useful at all for this. Primarily for the following two reasons:

  • At this point the session is already being initialized. You want to reject non-existent session IDs, not initialize and then discard them.
  • There's no difference between reading empty data for a non-existent session ID, and/or just returning empty data for a newly-created ID. Therefore, even if you know that you're processing a call for a non-existent session ID, that doesn't help you much.

You could call session_regenerate_id() from inside read(), but that may have unexpected side-effects, or greatly complicate your logic if you do expect those side-effects ...
For example, file-based storage would be built around file descriptors and those should be open from inside read(), but then session_regenerate_id() will directly call write() and you won't have the (correct) file descriptor to write to at that point.

Given a session ID that came from the HTTP request and supposing that it was not found in the storage area, how would I signal an error to the PHP engine so that it would call read($session_id) again with a new session ID?

For the longest time, I hated that userspace handlers couldn't signal error conditions, until I found out that you could do it.
As it turns out, it was in fact designed to handle boolean true, false as success, failure. It's just that there was a very subtle bug in how PHP handled this ...

Internally, PHP uses the values 0 and -1 to mark success and failure respectively, but the logic that handled conversion to true, false for userspace was erroneous and actually exposed this internal behavior, and leaving it undocumented.
This was fixed in PHP 7, but left as is for PHP 5 as the bug is very, very old and would result in huge BC breaks when fixed. More info in this PHP RFC that proposed the fix for PHP 7.

So for PHP 5, you could in fact return int(-1) from inside session handler methods to signal an error, but that isn't really useful for "strict mode" enforcement, as it results in completely different behavior - it emits an E_WARNING and halts session initialization.


Now for that shortcut I mentioned ...

This is not at all obvious, and in fact very weird, but ext/session doesn't just read cookies and handle them on its own - it actually uses the $_COOKIE superglobal, and that means you can manipulate $_COOKIE to alter the session handler behavior!

So, here's a solution that's even forward compatible with PHP 7:

abstract class StrictSessionHandler
{
    private $savePath;
    private $cookieName;

    public function __construct()
    {
        $this->savePath = rtrim(ini_get('session.save_path'), '\\/').DIRECTORY_SEPARATOR;

        // Same thing that gets passed to open(), it's actually the cookie name
        $this->cookieName = ini_get('session.name');

        if (PHP_VERSION_ID < 70000 && isset($_COOKIE[$this->cookieName]) && ! $this->validateId($_COOKIE[$this->cookieName])) {
            unset($_COOKIE[$this->cookieName]);
        }
    }

    public function validateId($sessionId)
    {
        return is_file($this->savePath.'sess_'.$sessionId);
    }
}

You'll note that I made it an abstract class - that's only because I'm too lazy to write the entire handler here and unless you do actually implement the SessionHandlerInterface methods, PHP will ignore your handler - simply extending SessionHandler without overriding any method is handled the same way as not using a custom handler at all (constructor code will be executed, but strict mode logic will remain from the default PHP implementation).

TL;DR: Check if you have data associated with $_COOKIE[ini_get('session.name')] before calling session_start(), and unset the cookie if you don't - this tells PHP to behave as if you've received no session cookie at all, thus triggering new session ID generation. :)

like image 88
Narf Avatar answered Nov 11 '22 23:11

Narf


Update (2017-03-19)

My original implementation delegated on session_regenerate_id() for generating new session IDs and setting the cookie header when appropriate. As of PHP 7.1.2 this method can no longer be invoked from inside the session handler[1]. Decent Dabbler also reported that this approach won't work in PHP 5.5.9[2].

The following variation of the read() method avoids this pitfall but is somewhat messier, as it has to set the cookie header itself.

/**
 * {@inheritdoc}
 */
public function open($save_path, $name)
{
    // $name is the desired name for the session cookie, as specified
    // in the php.ini file. Default value is 'PHPSESSID'.
    // (calling session_regenerate_id() used to take care of this)
    $this->cookieName = $name;

    // the handling of $save_path is implementation-dependent
}

/**
 * {@inheritdoc}
 */
public function read($session_id)
{
    if ($this->mustRegenerate($session_id)) {
        // Manually set a new ID for the current session
        session_id($session_id = $this->create_sid());

        // Manually set the 'Cookie: PHPSESSID=xxxxx;' header
        setcookie($this->cookieName, $session_id);
    }

    return $this->getSessionData($session_id) ?: '';
}

FWIW the original implementation is known to work under PHP 7.0.x

Original Answer

Combining the insight gained from Dave's answer (i.e. extending the \SessionHandler class instead of implementing the \SessionHandlerInterface to peek into create_sid and solve the first hurdle) and this fine field study of the session lifecycle from Rasmus Schultz I came up with a pretty satisfactory solution: it doesn't encumber itself with SID generation, nor sets any cookie manually, nor kicks the bucket up the chain to client code. Only relevant methods are shown for clarity:

<?php

class MySessionHandler extends \SessionHandler
{
    /**
     * A collection of every SID generated by the PHP internals
     * during the current thread of execution.
     *
     * @var string[]
     */
    private $new_sessions;

    public function __construct()
    {
        $this->new_sessions = [];
    }

    /**
     * {@inheritdoc}
     */
    public function create_sid()
    {
        $id = parent::create_sid();

        // Delegates SID creation to the default
        // implementation but keeps track of new ones
        $this->new_sessions[] = $id;

        return $id;
    }

    /**
     * {@inheritdoc}
     */
    public function read($session_id)
    {
        // If the request had the session cookie set and the store doesn't have a reference
        // to this ID then the session might have expired or it might be a malicious request.
        // In either case a new ID must be generated:
        if ($this->cameFromRequest($session_id) && null === $this->getSessionData($session_id)) {
            // Regenerating the ID will call destroy(), close(), open(), create_sid() and read() in this order.
            // It will also signal the PHP internals to include the 'Set-Cookie' with the new ID in the response.
            session_regenerate_id(true);

            // Overwrite old ID with the one just created and proceed as usual
            $session_id = session_id();
        }

        return $this->getSessionData($session_id) ?: '';
    }

    /**
     * @param string $session_id
     *
     * @return bool Whether $session_id came from the HTTP request or was generated by the PHP internals
     */
    private function cameFromRequest($session_id)
    {
        // If the request had the session cookie set $session_id won't be in the $new_sessions array
        return !in_array($session_id, $this->new_sessions);
    }

    /**
     * @param string $session_id
     *
     * @return string|null The serialized session data, or null if not found
     */
    private function getSessionData($session_id)
    {
        // implementation-dependent
    }
}

Note: the class ignores the session.use_strict_mode option but always follows the strict behavior (this is actually what I want). These are the testing results in my sightly more complete implementation:

marcel@werkbox:~$ curl -i -H "Cookie:PHPSESSID=madeupkey" localhost/tests/visit-counter.php
HTTP/1.1 200 OK
Server: nginx/1.11.6
Date: Mon, 09 Jan 2017 21:53:05 GMT
Content-Type: text/html; charset=UTF-8
Transfer-Encoding: chunked
Connection: keep-alive
Set-Cookie: PHPSESSID=c34ovajv5fpjkmnvr7q5cl9ik5; path=/      <--- Success!
Expires: Thu, 19 Nov 1981 08:52:00 GMT
Cache-Control: no-store, no-cache, must-revalidate
Pragma: no-cache

1

marcel@werkbox:~$ curl -i -H "Cookie:PHPSESSID=c34ovajv5fpjkmnvr7q5cl9ik5" localhost/tests/visit-counter.php
HTTP/1.1 200 OK
Server: nginx/1.11.6
Date: Mon, 09 Jan 2017 21:53:14 GMT
Content-Type: text/html; charset=UTF-8
Transfer-Encoding: chunked
Connection: keep-alive
Expires: Thu, 19 Nov 1981 08:52:00 GMT
Cache-Control: no-store, no-cache, must-revalidate
Pragma: no-cache

2

And the testing script:

<?php

session_set_save_handler(new MySessionHandler(), true);

session_start();

if (!isset($_SESSION['visits'])) {
    $_SESSION['visits'] = 1;
} else {
    $_SESSION['visits']++;
}

echo $_SESSION['visits'];
like image 26
Marcel Hernandez Avatar answered Nov 11 '22 22:11

Marcel Hernandez


I haven't tested this so it may or may not work.

The default SessionHandler class can be extended. This class contains a relevant extra method that the interface does not, namely create_sid(). This is called when PHP generates a new session ID. Thus, it should be possible to use this to distinguish between a new session and an attack; something like:

class MySessionHandler extends \SessionHandler
{
    private $isNewSession = false;

    public function create_sid()
    {
        $this->isNewSession = true;

        return parent::create_sid();
    }

    public function read($id)
    {
        if ($this->dataStore->haveExistingSession($id)) {
            return $this->getSessionData($id);
        }

        if ($this->isNewSession) {
            $this->dataStore->createNewSession($id);
        }

        return '';
    }

    // ...rest of implementation
}

This approach may need padding out with another flag or two to handle legitimate session ID regeneration, if you ever do this.

Regarding the issue of handling the error gracefully, I would experiment with throwing an exception. If this doesn't yield anything useful, I would do it at the application level, by returning a fixed value for the session data itself that you could check for, and then handle it in-app by generating a new ID or destroying the session and presenting the user with an error.

like image 3
DaveRandom Avatar answered Nov 11 '22 21:11

DaveRandom


I guess you could, as simplest approach, extend a bit the sample implementation like this:

private $validSessId = false;

public function read($id)
{
    if (file_exists("$this->savePath/sess_$id")) {
        $this->validSessId = true;
        return (string)@file_get_contents("$this->savePath/sess_$id");
    }
    else {    
        return '';
    }
}

public function write($id, $data)
{
    if (! $this->validSessId) {
        $id = $this->generateNewSessId();
        header("Set-Cookie:PHPSESSID=$id;");
    }

    return file_put_contents("$this->savePath/sess_$id", $data) === false ? false : true;
}

Inside the write method you can generate a new session ID and force it back to the client.

It's not the cleanest thing, really. This concerns setting session save handler, so we "userland" should only provide storage implementation, OR the handler interface should define a validation method to be called automatically, probably before read. Anyway this has been discussed here.

like image 2
alepeino Avatar answered Nov 11 '22 23:11

alepeino