Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handling sessions #15

Open
tyler-sommer opened this issue Sep 11, 2015 · 6 comments
Open

Handling sessions #15

tyler-sommer opened this issue Sep 11, 2015 · 6 comments

Comments

@tyler-sommer
Copy link
Contributor

I've successfully gotten a fairly large Symfony 2 application working under FastCGIDaemon. Initial testing shows up to 400ms saved per request!

However, I ran into some issues with handling multiple simultaneous sessions. I gutted the Symfony 2 handling (other than HttpFoundation Request/Response) to try to get an understanding of how PHP is handling things. Here's what I've got so far:

<?php

require_once __DIR__ . '/../vendor/autoload.php';

use PHPFastCGI\FastCGIDaemon\ApplicationFactory;
use PHPFastCGI\FastCGIDaemon\Http\RequestInterface;
use Symfony\Component\HttpFoundation\Cookie;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpFoundation\Session\Session;

$stream = fopen('php://stdout', 'w');

ini_set('session.use_cookie', false);

$kernel = function (RequestInterface $originalRequest) use ($stream) {
    $request = $originalRequest->getHttpFoundationRequest();

    fwrite($stream, $request->headers."\n");

    if ($sessId = $request->cookies->get(session_name())) {
        session_id($sessId);
    } else {
        session_id(hash('sha1', uniqid(mt_rand(), true)));
    }

    session_start();

    if (!isset($_SESSION['test'])) {
        fwrite($stream, 'Saving test!'."\n");
        $_SESSION['test'] = 'thing';
    } else {
        fwrite($stream, 'Showing test: '.$_SESSION['test']."\n");
    }

    $response = Response::create();

    $cookieParams = session_get_cookie_params();
    $response->headers->setCookie(new Cookie(session_name(), session_id(), time()+86400, $cookieParams['path'], $cookieParams['domain'], $cookieParams['secure'], $cookieParams['httponly']));

    fwrite($stream, $response->headers."\n");

    session_write_close();

    return $response;
};

$application = (new ApplicationFactory)->createApplication($kernel);
$application->run();

This works, though obviously ugly. Key points:

  • You must generate your own session ID, always. PHP seems to want to keep the last used ID if it is not explicitly overwritten.
  • You must set the cookie on the response manually. I used session_get_cookie_params() so that the ini settings could be used.

Thoughts:

  • Implementing a SessionHandlerInterface will invariably be a better solution. This also gives you a clear place to implement the regenerating of a session_id (by setting a create_sid callback [yes, that means the old session_set_save_handler prototype must be used]).
  • Disabling session.use_cookie might not be necessary

I'm concerned that I'm missing something as far as maintaining session security (preventing hijacking or fixation). Since this code blindly checks for a PHPSESSID cookie, it would be a trivial affair to hijack someone else's session, if you could guess the ID. However, I don't think this logic differs from the built-in default PHP session_id validation logic, so maybe it's a moot point.

@AndrewCarterUK
Copy link
Member

Hi Tyler,

Thanks very much for your work here, I'm going to have a more thorough read of your suggestions when I get back from holiday on Tuesday. This is just a quick note as I didn't want you to think I was ignoring the issue!

Sorry for closing and reopening - I'm on a mobile with fat fingers!

Andrew

@AndrewCarterUK
Copy link
Member

Hi Tyler,

Very glad to hear that PHPFastCGI is speeding up your app! Is that 400ms or 40ms? If it's 400ms, your app must be doing quite a lot!

I've just done some research on sessions! I've never really checked out the internals of PHP session handling or used the Symfony session classes before.

A couple of quick points before I get into it. My understanding always was that if you could obtain the session identifier then that could be used to hijack the session. On that note, the method you use to generate the session identifer in the example given isn't cryptographically secure. The output of both mt_rand() and uniqid() is predictable and shouldn't be used for these purposes. I know that this is example code, but I thought that I should mention it anyway!

Moving on, my current thinking is that whilst it does make sense for the core FastCGIDaemon package to provide the ability for integrations to work with sessions, the actual logic for their implementation is outside the scope of the package. To include this would require the package to be coupled to some form of storage layer - and I don't think this would be a good idea. Also, the $_SESSION suplerglobal isn't something that PHPFastCGI applications should be using. I would recommend that applications using the FastCGIDaemon without a framework adapter should employ some form of session middleware/service that is capable of handling multiple request cycles.

All that said, where a framework does provide a wrapper for handling sessions, the PHPFastCGI adapter should maintain the functionality of this wrapper and not break it (as it currently does with Symfony). A well designed framework should provide some pathway to doing this!

With the Speedfony Bundle for Symfony - I think the best approach would be to override the session service by setting the "session.class" parameter. I'm going to have a play with this tomorrow to test if the session storage interface (injected into the session service) is flexible enough to allow for multiple request cycles. If not, we're going to have a problem (and a few suggestions for Symfony 3!). From what I can tell, the bundle may also need to register an extra response listener to set the session cookie, as currently the Symfony framework relies on that being handled by PHP itself.

I've got a bit of refactoring to do on the core FastCGIDaemon package over the next few days. That gives us some time to keep discussing this - and then I can look towards implementing it.

Would be interested to know your thoughts!

Andrew

@jakzal
Copy link

jakzal commented Sep 18, 2015

With the Speedfony Bundle for Symfony - I think the best approach would be to override the session service by setting the "session.class"

@AndrewCarterUK Don't rely on .class parameters please. We're not introducing them to new services, and were considering removing them for existing ones. Either override the service or decorate it.

I ran into some issues with handling multiple simultaneous sessions.

@tyler-sommer What issues exactly? I'm not sure what are we trying to solve here. Would it be possible to reproduce your problems on a vanilla Symfony standard edition? We need to rule out the possibility the problem lies in your project.

@AndrewCarterUK
Copy link
Member

Thanks @jakzal.

The issue is that the default configuration for Symfony lets PHP append the session cookie when the application sends out the response. Out of the box, Symfony pretty much uses the default PHP session_*() api. This doesn't work with PHPFastCGI, because the response doesn't go through PHP and thus the PHPSESSID cookie is never set.

@tyler-sommer
Copy link
Contributor Author

Is that 400ms or 40ms? If it's 400ms, your app must be doing quite a lot!

I'm not sure where I got that number from. A little more testing shows a pretty consistent ~100ms improvement, however.

On that note, the method you use to generate the session identifer in the example given isn't cryptographically secure. The output of both mt_rand() and uniqid() is predictable and shouldn't be used for these purposes. I know that this is example code, but I thought that I should mention it anyway!

Thanks for pointing this out! I looked into it a bit, and it seems the PHP implementation for session id generation does a combination of things. Here's a nice summary on Stack Overflow.

I think it would be ideal if we could continue to depend on PHP for id generation, though it seems like the very best solution would be to completely decouple from the language-provided mechanisms. However, it also seems like session management is not completely trivial-- as you saw me demonstrate.

Moving on, my current thinking is that whilst it does make sense for the core FastCGIDaemon package to provide the ability for integrations to work with sessions, the actual logic for their implementation is outside the scope of the package. To include this would require the package to be coupled to some form of storage layer - and I don't think this would be a good idea. Also, the $_SESSION suplerglobal isn't something that PHPFastCGI applications should be using. I would recommend that applications using the FastCGIDaemon without a framework adapter should employ some form of session middleware/service that is capable of handling multiple request cycles.

All that said, where a framework does provide a wrapper for handling sessions, the PHPFastCGI adapter should maintain the functionality of this wrapper and not break it (as it currently does with Symfony). A well designed framework should provide some pathway to doing this!

This makes sense. I think that probably the best way to provide that pathway is through documentation, though I also wonder if there might be a place for a separate library specifically suited to multi-request session management. That might help remove the guesswork and stop people from making simple mistakes in their own implementations.

@fezfez
Copy link

fezfez commented Mar 3, 2016

Maybe try to invest into https://github.com/Ocramius/PSR7Session ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants