[RFC][Discussion] Precise session data management View as Markdown

Hi Joe,

First, thanks for all the effort ... You already know that, a refusal to

merge a patch doesn't mean you wasted your time, it should serve to refocus
your research.

So, I thought I'd say some stuff, to aid with that ...

The first thing I would do is break down the problem into smaller, much

smaller, RFC's: We know there are many inconsistencies in the language, we
could write a huge long list, and do a bumper RFC to solve them all at once.
But would that be a good idea !?

Splitting patch is not a problem. Some changes are interrelated, but
many could be applied individually.

As already mentioned it's much easier to consider each problem on it's

own, and the solution for that problem on it's own, than it is to consider
what we are going to do about this huge mess. Try to remember that while you
have had your head buried in this stuff for a long time, the rest of us
likely haven't.

I don't really like to talk about security, it's not a thing I feel

qualified to talk about ... but, there is inconsistency in saying "we should
have a secure by default session module", and then introducing ini settings
that can vary the effectiveness of the solution, based on guesses about the
length, and speed at which elevators travel ...

Having another setting which seems to be 18 times longer than the

recommended value for "low security applications" is also extremely odd to
me ...

That you can't find sensible defaults for these values, says to me that

these problems don't have solutions in our domain; Application security is a
problem that can only be solved by the programmer using PHP.

We should provide the tools, we should not lie about the validity of a

session because of faults in our design, every layer we provide should be
consistent, but the problem still belongs to the programmer using PHP.

I agree. It's like using TCP or UDP. I don't mind much either way, but
I prefer TCP like behavior for session manager because session manager
jobs is keeping reliable session.

Current PHP session is UDP, users are responsible use it correctly. If
we keep UDP way, I think we should remove session_regenerate_id(TURE)
at least. i.e. Remove deletion option from session_regenerate_id()
because it doesn't work and bad way to manage session ID.

I'll write PHP code for those who are care about session reliability
and security. Following code is minimum code. I wrote this code
directly, so take this as pseudo code. I might be missing something as
I do not verify this code, but it should be good enough to know what
users must do.

Using UDP like TCP is not too difficult, but not too simple also.
Session management is the same.

<?php
ini_set('session.use_strict_mode', 1);
session_start();
if (isset($_SESSION['deleted']) && $_SESSION['deleted'] < time() - 600) {
trigger_error('Possible attack detected');
session_unset();
session_destroy();
session_start();
session_regenerate_id();
}
// if you cannot loose session on occasion, execute following if block also.
if (isset($_SESSION['deleted']) && $_SESSION['deleted'] < time() - 60
&& isset($_SESSION['new_id'])) {
$new_id = $_SESSION['new_id'];
unset($_SESSION['new_id']);
session_commit();
session_id($new_id);
session_start();
}
if (!isset($_SESSION['update'] ||
(!isset($_SESSION['deleted']) && $_SESSION['updated'] < time() - 180)) {
// Should not update updated timestamp always
$_SESSION['updated'] = time();
}
if (empty($_SESSION['regenerated']) {
// New session
$_SESSION['regenerated'] = time();
}
if ($_SESSION['regenerated'] < time() - 1800) {
// This code does not keep older session IDs. If you need them, save
them here also.
$_SESSION['deleted'] = time();
session_commit(); // Older PHP must close session to write session
data. PHP 7.0 and up does not need this.
session_start(); // Since session is closed, session has to be
started. PHP 7.0 and up does not need this.
$old_id = session_id();
session_regenerate_id();
$new_id = session_id();
$_SESSION['regenerated'] = time();
unset($_SESSION['deleted']);
// if you cannot loose session on occasion, execute following code also.
session_commit();
session_id($old_session);
session_start();
$_SESSION['new_id'] = $new_id;
session_commit();
session_id($new_id);
session_start();
}
?>

This is not equal to what the proposed patch does. This is the least
requirement for reliable session management. It's much less efficient
in user script. It's cost of user space implementation.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

Hi all,

Since the vote for
https://wiki.php.net/rfc/precise_session_management
is declined 15 vs 11.
https://wiki.php.net/rfc/precise_session_management#vote

We have to come up with other solutions for

• Session loss by race conditions
• Method to make session abuse harder

I'm open to implement better solution than proposed RFC.

These issues are serious issues that cannot be ignored.
Looking forward alternative implementation ideas!

Thank you.

--
Yasuo Ohgaki
yohgaki@ohgaki.net

--

I'm disappointed this RFC failed as improving session security should be a
high priority. I'd propose that we focus on items which will improve
session security overall. This jumped out at me to start with: "This RFC
also
includes minor security improvements like httponly cookie, better hash
function."

Perhaps a first RFC for improving session security can start with those
items: Set the session cookie to be httponly (or provide an ini setting to
do so, defaulting to on) instead of using a better hash function for the
session id, why not generate it from random_bytes()? Then the id is crypto
secure.

I'd then consider a second RFC setting out the extra internal data that
would be required to kill off old sessions correctly. That would require
to
keys under ['PHP__SESSION'] (could the key name be an ini setting to
preserve BC?) 'destroyed' and 'expires', the latter would be a bool flag
stating if the session had been explicitly destroyed. (A destroyed session
could no longer be used, could throw an exception or just have no data in
it) the expires key would be used in a similar manner, being set to
Time+cookie expiry time on each read/write to the session. (again an
expired session would not be accessible and would function like a
destroyed
one)

The combination of the two would resolve most of the security issues and
establish the PHP__SESSION key which could later be used to handle the
race condition issue.

Could we also add HTTPS detection and enable the secure flag by default
when a session is established on an HTTPS endpoint?

Scott Arciszewski
Chief Development Officer
Paragon Initiative Enterprises https://paragonie.com/
​

I had considered this suggestion myself, but I wasn't sure if it would be
easy to implement. It could also cause some very strange (to a novice
developer) behaviour which would be hard to debug (Think sites which have
part http part https, session behaviour would be dependant on the entry
page the user hits) I'd support this, but it should probably be opt-in to
start with (alternatively we could come up with a different way to allow
the developer to specify sessions being ssl only)