Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

getJSON and session_regenerate_id()

I am performing a standard getJSON query from a page which is session protected:

$.getJSON('queries.php',{q: 'updateEvent', param1: p1},
    function(data){
        ...
    }
);

On my session constructor I have set the following :

function startSession() 
{
    ini_set('session.use_only_cookies', SESSION_USE_ONLY_COOKIES);

    $cookieParams = session_get_cookie_params();
    session_set_cookie_params(
        $cookieParams["lifetime"], 
        $cookieParams["path"], 
        $cookieParams["domain"], 
        SESSION_SECURE, 
        SESSION_HTTP_ONLY
     );

    session_start();

    if ( SESSION_REGENERATE_ID )
        session_regenerate_id(SESSION_REGENERATE_ID);   
}

If I set SESSION_REGENERATE_ID to true, then my getJSON sends a token, but receives a different one, making the request fail. So for the moment I'm dealing with SESSION_REGENERATE_ID set to false.

Is there a way to make getJSON work in such conditions ?

EDIT : all files are under the same domain.

We have index.php where the js is included, we have queries.php which is the php file called by the ajax requests, we have s_session.php which includes the constructor written above.

Files index.html and queries.php are both protected at the begining this way :

include "s_session.php"; 
if(!$login->isLoggedIn()) {
  header('Content-Type: application/json'); 
  echo json_encode(array('content' => 'Login failed')); 
  exit;
}

The PHPSESSID is in the header of the ajax request under set-cookie. The PHPSESSID returned in the answer is different, as expected from session_regenerate_id.

If SESSION_REGENERATE_ID is set to FALSE the requests are going through without problem. If it is set to TRUE, then I get the error message "Login failed".

Here is the isLoggedIn() :

public function isLoggedIn() {
    //if $_SESSION['user_id'] is not set return false
    if(ASSession::get("user_id") == null)
         return false;

    //if enabled, check fingerprint
    if(LOGIN_FINGERPRINT == true) {
        $loginString  = $this->_generateLoginString();
        $currentString = ASSession::get("login_fingerprint");
        if($currentString != null && $currentString == $loginString)
            return true;
        else  {
            //destroy session, it is probably stolen by someone
            $this->logout();
            return false;
        }
    }

    $user = new ASUser(ASSession::get("user_id"));
    return $user->getInfo() !== null;
}

EDIT 2 : Here is the full ASSession code :

class ASSession {

/**
 * Start session.
 */
public static function startSession() 
{
    ini_set('session.use_only_cookies', SESSION_USE_ONLY_COOKIES);

    session_start();
    $s = $_SESSION;

    $cookieParams = session_get_cookie_params();

    session_set_cookie_params(
        $cookieParams["lifetime"], 
        $cookieParams["path"], 
        $cookieParams["domain"], 
        SESSION_SECURE, 
        SESSION_HTTP_ONLY
     );

    if ( SESSION_REGENERATE_ID )
        session_regenerate_id(SESSION_REGENERATE_ID);

    //$_SESSION = $s;

}

/**
 * Destroy session.
 */
public static function destroySession() {

    $_SESSION = array();

    $params = session_get_cookie_params();

    setcookie(  session_name(), 
                '', 
                time() - 42000, 
                $params["path"], 
                $params["domain"], 
                $params["secure"], 
                $params["httponly"]
            );

    session_destroy();
}

/**
 * Set session data.
 * @param mixed $key Key that will be used to store value.
 * @param mixed $value Value that will be stored.
 */
public static function set($key, $value) {
    $_SESSION[$key] = $value;
}

/**
 * Unset session data with provided key.
 * @param $key
 */
public static function destroy($key) {
    if ( isset($_SESSION[$key]) )
        unset($_SESSION[$key]);
}

/**
 * Get data from $_SESSION variable.
 * @param mixed $key Key used to get data from session.
 * @param mixed $default This will be returned if there is no record inside
 * session for given key.
 * @return mixed Session value for given key.
 */
public static function get($key, $default = null) {
    if(isset($_SESSION[$key]))
        return $_SESSION[$key];
    else
        return $default;
}

}

EDIT 3: here are the request headers and response cookie :

enter image description here enter image description here

I noticed that the very first getJSON which is performed during the onload is successfull. All the others done after and triggered by user are unsuccessfull

like image 503
Vincent Teyssier Avatar asked Jan 25 '16 12:01

Vincent Teyssier


1 Answers

This is mostly caused by a race condition, but a browser bug is also possible.

I would rule out the browser bug scenario, but there's a conflict in the provided info, more specifically in this comment:

It is several calls, made one by one on user action, never simultaneously.

If the requests are never executed simultaneously, then that could only mean that your browser is not functioning properly and one of the following happens:

  • Discarding the Set-Cookie header it receives in the response (if that logic depends on the HttpOnly flag, this would explain why the web still works :D)
  • The onLoad event in fact executed during the page load (I know that doesn't make sense, but everything's possible if it's a browser bug)

Of course, those are highly unlikely to happen, so I'm inclined to say that you are in fact processing multiple AJAX requests at a time, in which case the race condition is a plausible scenario:

  1. First request starts (with your initial PHPSESSID)
  2. Second request starts (again, with the same PHPSESSID)
  3. First request is processed and receives response with a new PHPSESSID
  4. Second request was blocked until now (the session handler uses locking to prevent multiple processes modifying the same data simultaneously) and is just now starting to be processed with the initial PHPSESSID, which is invalid at this point, hence why it triggers a log-out.

I would personally look at what gets triggered by that onLoad event - it's easy to just put all initialization logic in there and forget that this may include multiple asynchronous requests.


Either way, the real logical error on your part is this piece of code:

if ( SESSION_REGENERATE_ID )
    session_regenerate_id(SESSION_REGENERATE_ID);

You're using the same value for what are two different conditions:

  1. Determining whether to regenerate the session ID at all
  2. Telling session_regenerate_id() if it should immediately destroy data associated with the old session ID

The option not do destroy that data immediately is there exactly to provide a solution to these race conditions with asynchronous requests, because they are practically unavoidable. A race condition will happen at some point, regardless of how hard you try to avoid it - even if there's no logical flaw, network lags (for example) may still trigger it.
Preserving the old session's data (temporarily of course) works around that problem by simply allowing a "late" or "out of sync" request to work with whatever data was available at the time it was started.

Expired sessions will be cleaned up later by the session garbage collector. That's possibly not ideal, but pretty much the only solution for storages that require you to delete data (as opposed to cache stores like Redis, which allow you to set a TTL value instead of having to manually delete).

Personally, I prefer to avoid session ID regeneration specifically during AJAX requests ... as you can see, it's a can of worms. :)

like image 55
Narf Avatar answered Sep 22 '22 03:09

Narf