Reputation: 36205
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
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
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
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