Boardy
Boardy

Reputation: 36205

session_regenerate_id causes two PHPSESSID cookies to be returned

I've developed an API which originally was only used via a browser and never noticed an issue however, I am now trying to connect to it via a third party Android library (OkHttpClient) and I've tested what I am seeing using a REST API test client (Insomnia.rest).

The problem I am having is when I perform the login action of the API I start a session and call session_regenerate_id(true); to avoid sticky session attacks (I'm not sure if that's proper name).

However, when I do this I return two PHPSESSID cookies as shown in the headers below:

< HTTP/1.1 200 OK
< Date: Thu, 18 Apr 2019 22:51:43 GMT
< Server: Apache/2.4.27 (Win64) PHP/7.1.9
< X-Powered-By: PHP/7.1.9
* cookie size: name/val 8 + 6 bytes
* cookie size: name/val 4 + 1 bytes
< Set-Cookie: ClientID=413059; path=/
* cookie size: name/val 9 + 26 bytes
* cookie size: name/val 4 + 1 bytes
< Set-Cookie: PHPSESSID=15u9j1p2oinfl5a8slh518ee9r; path=/
< Expires: Thu, 19 Nov 1981 08:52:00 GMT
< Cache-Control: no-store, no-cache, must-revalidate
< Pragma: no-cache
* cookie size: name/val 9 + 26 bytes
* cookie size: name/val 4 + 1 bytes
* Replaced cookie PHPSESSID="hkkffpj8ta9onsn92pp70r257v" for domain localhost, path /, expire 0
< Set-Cookie: PHPSESSID=hkkffpj8ta9onsn92pp70r257v; path=/
* cookie size: name/val 17 + 1 bytes
* cookie size: name/val 4 + 1 bytes
< Set-Cookie: UsingGoogleSignIn=0; path=/
* cookie size: name/val 6 + 1 bytes
* cookie size: name/val 4 + 1 bytes
< Set-Cookie: UserID=7; path=/
< Access-Control-Allow-Credentials: true
< Content-Length: 47
< Content-Type: application/json

As you can see from the above output there's two Set-Cookies with PHPSESSID. If I remove the session_regenerate_id I then only get the one PHPSESSID cookie and then the Android client successfully works.

I've exhibited this on Apache under Wamp on Windows 10 and Apache in production on a CentOS 7 build.

So question is, how can I generate a new PHP session ID without sending back two different PHPSESSID cookies?

UDPATE

Below is some of the code relating to the login process. I can't include all of the code but it should show the concept of what is going on.

An API request is made to the login function

$email = mysqli_escape_string($this->getDBConn(), $encryption->encrypt($postArray["email"]));
        $password = mysqli_escape_string($this->getDBConn(), $encryption->encrypt($postArray["password"]));
        $externalDevice = isset($postArray["external_device"]) ? abs($postArray["external_device"]) : 0;

        $query = "SELECT * FROM users WHERE Email='$email'";
        $result = $this->getDBConn()->query($query);
        if ($result)
        {
            if (mysqli_num_rows($result) > 0 )
            {
                $myrow = $result->fetch_array();
                if ($myrow["UsingGoogleSignIn"] === '1')
                {
                    //We're trying to login as a normal user, but the account was registered using Google Sign In
                    //so tell the user to login via google instead
                    return new APIResponse(API_RESULT::SUCCESS, "AccountSigninViaGoogle");
                }
                else
                {
                    //Check the password matches
                    if ($myrow["Password"] === $password)
                    {
                        $this->getLogger()->writeToLog("Organisation ID: " . $myrow["Organisation"]);
                        $organisationDetails = $this->getOrganisationDetails(abs($myrow["Organisation"]), false);
                        $this->getLogger()->writeToLog(print_r($organisationDetails, true));

                        $this->createLoginSession($myrow, $organisationDetails, false, $paymentRequired, $passwordChangeRequired);
                        $data = null;
                        if ($externalDevice === 1)
                        {
                            $data = new stdClass();
                            $data->AuthToken = $_SESSION["AuthToken"];
                            $data->ClientID = $_SESSION["ClientID"];
                            $data->UserID = abs($_SESSION["UserID"]);
                        }

                        $this->getLogger()->writeToLog("Login Response Headers");
                        $headers = apache_response_headers();
                        $this->getLogger()->writeToLog(print_r($headers, true));

At this point an API response is returned which contains JSON object

In the code above, if the email and password matches (not using Google sign in here) it calls createLoginSession which is the following:

private function createLoginSession($myrow, $organisationDetails, $usingGoogleSignIn, &$paymentRequired, &$passwordChangeRequired)
    {
        require_once 'CommonTasks.php';
        require_once 'IPLookup.php';
        require_once 'Encryption.php';
        try
        {
            $this->getLogger()->writeToLog("Creating login session");
            $paymentRequired = false;
            if ($organisationDetails === null)
            {
                $organisationDetails = $this->getOrganisationDetails($myrow["Organisation"]);
            }
            $encryption = new Encryption();
            $userID = mysqli_escape_string($this->getDBConn(), $myrow["UserID"]);
            $organisationID = intval(abs($myrow["Organisation"]));

            $commonTasks = new CommonTasks();
            $browserDetails = $commonTasks->getBrowserName();
            $this->getLogger()->writeToLog("Browser Details");
            $this->getLogger()->writeToLog(print_r($browserDetails, true));
            $clientName = $browserDetails["name"];

            $iplookup = new IPLookup(null, $this->getLogger());
            $ipDetails = json_decode($iplookup->getAllIPDetails($commonTasks->getIP()));

            if ($ipDetails !== null)
            {
                $ip = $ipDetails->ip;
                $country = $ipDetails->country_name;
                $city = $ipDetails->city;
            }
            else
            {
                $ip = "";
                $country = "";
                $city = "";
            }

            //Create a random client ID and store this as a cookie
            if (isset($_COOKIE["ClientID"]))
            {
                $clientID = $_COOKIE["ClientID"];
            }
            else
            {
                $clientID = $commonTasks->generateRandomString(6, "0123456789");
                setcookie("ClientID", $clientID, 0, "/");
            }

            //Create an auth token
            $authToken = $commonTasks->generateRandomString(25);
            $encryptedAuthToken = $encryption->encrypt($authToken);

            $query = "REPLACE INTO client (ClientID, UserID, AuthToken, ClientName, Country, City, IPAddress) " .
                "VALUES ('$clientID', '$userID', '$encryptedAuthToken', '$clientName', '$country', '$city', '$ip')";
            $result = $this->getDBConn()->query($query);
            if ($result)
            {

                session_start();
                $this->getLogger()->writeToLog("Logging in and regnerating session id");
                session_regenerate_id(true);

                $_SESSION["AuthToken"] = $authToken;
                $_SESSION["ClientID"] = $clientID;
                $_SESSION["UserID"] = $userID;
                $_SESSION["FirstName"] = $this->getEncryption()->decrypt($myrow["FirstName"]);
                $_SESSION["LastName"] = $this->getEncryption()->decrypt($myrow["LastName"]);

                $passwordChangeRequired = $myrow["PasswordChangeRequired"] === "1" ? true : false;
                //Check if the last payment failure reason is set, if so, set a cookie with the message but only
                //if the organisation is not on the free plan
                //Logger::log("Current Plan: " . $this->getOrganisationDetails(->getPlan()));
                if ($organisationDetails->getPlan() !== "Free")
                {
                    if (!empty($organisationDetails->getLastPaymentFailureReason()))
                    {
                        $this->getLogger()->writeToLog("Detected last payment as a failure. Setting cookies for organisation id: " . $organisationDetails->getId());
                        setcookie("HavePaymentFailure", true, 0, "/");
                        setcookie("PaymentFailureReason", $organisationDetails->getLastPaymentFailureReason(), 0, "/");

                    }
                    //Check if the current SubscriptionPeriodEnd is in the past
                    $subscriptionPeriodEnd = $organisationDetails->getSubscriptionOfPeriod();
                    $currentTime = DateTimeManager::getEpochFromCurrentTime();
                    if ($currentTime > $subscriptionPeriodEnd)
                    {
                        $this->getLogger()->writeToLog("Detected payment overdue for organisation: " . $organisationDetails->getId());
                        //The payment was overdue, determine the number of days grace period (there's a 7 day grace period) that's left
                        $subscriptionPeriodEndGracePeriod = $subscriptionPeriodEnd + (86400 * 7);
                        $numberOfDaysRemaining = floor((($subscriptionPeriodEndGracePeriod - $currentTime) / 86400));

                        setcookie("PaymentOverdue", true, 0, "/");
                        setcookie("DaysGraceRemaining", $numberOfDaysRemaining, 0, "/");
                        if ($numberOfDaysRemaining <= 0)
                        {
                            $paymentRequired = true;
                        }
                    }
                }
                setcookie("UsingGoogleSignIn", $usingGoogleSignIn ? "1" : "0", 0, "/");
                if ($organisationDetails->getId() !== 0)
                {

                    $_SESSION["OrganisationDetails"] = array();
                    $_SESSION["OrganisationDetails"]["id"] = $organisationDetails->getId();
                    $_SESSION["OrganisationDetails"]["Name"] = $organisationDetails->getName();

                }
                setcookie("UserID", $userID, 0, "/");
                $this->getLogger()->writeToLog("Successfully created login session. User ID '$userID' and Organisation ID '$organisationID'");
                return true;
            }
            else
            {
                $error = mysqli_error($this->getDBConn());
                $this->getLogger()->writeToLog("Failed to create login session. DB Error: $error");
                $this->getAlarms()->setAlarm(AlarmLevel::CRITICAL, "AccountManagement", "Failed to create login session. DB Error");
                throw new DBException($error);
            }
        }
        catch (DBException $ex)
        {
            throw $ex;
        }
    }

In the function above I call session_start() and then regenerate_session_id() and I then get the two PHPSESSID cookies in the response although the log line is only outputted once so its definetely not getting called multiple times.

If I remove regenerate_session_id then the problem goes away. To be safe I've tried swapping the session_start() so it comes after regenerate_session_id but that looks like the session id doesn't get re-created as expected.

UPDATE 2

From the comment from @waterloomatt I've created a PHP script that just has the following:

<?php
session_start();
session_regenerate_id(true);
phpinfo();

and the HTTP header information outputted from phpinfo is as follows

**HTTP Request Headers**
GET /api/session_test.php HTTP/1.1
Host    localhost
Connection  keep-alive Upgrade-Insecure-Requests    1
User-Agent  Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/73.0.3683.103 Safari/537.36
Accept  text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3
Accept-Encoding gzip, deflate, br
Accept-Language en-GB,en-US;q=0.9,en;q=0.8
Cookie  _ga=GA1.1.1568991346.1553017442

**HTTP Response Headers**
X-Powered-By    PHP/7.2.10
Set-Cookie  PHPSESSID=i19irid70cqbvpkrh0ufffi0jk; path=/
Expires Thu, 19 Nov 1981 08:52:00 GMT 
Cache-Control   no-store, no-cache,> must-revalidate Pragma no-cache
Set-Cookie  PHPSESSID=48qvia5e6bpmmk251qfrqs8urd; path=/

Upvotes: 3

Views: 3278

Answers (3)

Boardy
Boardy

Reputation: 36205

I think I've figured out what's going on. I thought it a cookies expires it will be all on the same line but it looks like - at least the way I'm passing the cookie strings that the cookies expiry is a different line.

I've therefore changed how I parse the cookie string so if it there's expire on the next line I don't include the cookie which seems to work. Below is how I am passing the cookies and not including the cookie if its expired:

for (int i = 0; i < headers.length; i++)
                {
                    Log.d("BaseAPI", "Header: " + headers[i]);
                    if (headers[i].trim().toLowerCase().startsWith("set-cookie:"))
                    {
                        if (headers[i+1].toLowerCase().startsWith("expires"))
                        {
                            Log.d("BaseAPI", "Found expired header. The cookie is: " + headers[i+1]);
                            //Thu, 19 Nov 1981 08:52:00 GMT
                            long epoch = new SimpleDateFormat("EEE, dd MMM YYYY HH:mm:ss").parse(headers[i+1].replace("Expires: ", "").replace("GMT", "").trim()).getTime();
                            Log.d("BaseAPI", "Cookie Epoch: " + epoch);
                            long currentEpoch = new Date().getTime();
                            Log.d("BaseAPI", "Current Epoch: " + currentEpoch);
                            if (epoch < currentEpoch)
                            {
                                continue;
                            }
                        }
                        cookieBuilder.append(headers[i].trim().replace("Set-Cookie:", "").replace("path=/", ""));
                        cookieBuilder.append(" ");

                    }
                }

Upvotes: 1

Hidran
Hidran

Reputation: 328

This is a known and documented issue. Just call session_regenerate_id() without passing true. The manual clearly says that you should not delete old session data if you want to avoid racing condition and also concurrent access may lead to inconsistent state. See https://www.php.net/manual/en/function.session-regenerate-id.php for more info

Upvotes: 0

Anatoliy  Gusarov
Anatoliy Gusarov

Reputation: 808

This header will actually remove cookie. This is the only way to remove HTTP-only cookie: to make it expire backdate.

Set-Cookie: PHPSESSID=15u9j1p2oinfl5a8slh518ee9r; path=/
< Expires: Thu, 19 Nov 1981 08:52:00 GMT

Upvotes: 2

Related Questions