Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:96147
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.192.177 as permitted sender)
To: Yasuo Ohgaki <yohgaki@ohgaki.net>,
 "internals@lists.php.net" <internals@lists.php.net>
References: <CAGa2bXaX1eZS=_Xg3JjCVZRBSruMy6yUk-PTrKoGOwW5Tg=0_Q@mail.gmail.com>
Message-ID: <c90779f0-0034-b3fc-e2fe-e19503a196ce@gmail.com>
Date: Sun, 25 Sep 2016 23:02:58 -0700
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:45.0)
 Gecko/20100101 Thunderbird/45.3.0
MIME-Version: 1.0
In-Reply-To: <CAGa2bXaX1eZS=_Xg3JjCVZRBSruMy6yUk-PTrKoGOwW5Tg=0_Q@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] Fixing halfway implemented session management -
 timestamp based session management OR remove session_regenerate_id()
From: smalyshev@gmail.com (Stanislav Malyshev)

Hi!

> Timestamp based session management is required to manage session as it
> should. I've updated the session manual pages a while a ago to explain
> why.

Could you explain what you mean here? "As it should" is kind of broad :)

> http://php.net/manual/en/session.security.php
> http://php.net/manual/en/function.session-regenerate-id.php

It looks like those need some polishing. If somebody with native English
volunteers it'd be great, otherwise I'll do it a bit later. It'd be also
nice to avoid claims of "mandatory" and instead just put explanations of
what each option is doing and what is the recommended setting. Nothing
that is controlled by an option is "mandatory" by definition.

I also see http://php.net/manual/en/function.session-regenerate-id.php
"warning" which I assume was added by you claims "Immediate session data
deletion disables session hijack attack detection and prevention also."
- I do not think this is accurate. Or at least it's not clear enough if
I can't properly understand what it means :)

> Although session module has over 10 years of history, session module
> lacks basic feature and is not implemented fully yet. As I mentioned
> in above manual pages, it does not have _mandatory_ timestamp based
> session management.
> 
> I proposed implementation [1], but it was declined even if it is
> mandatory for session module to manage session data correctly and
> precisely.

Obviously, other people disagree about how mandatory it is. :)

> Some may think "timestamp management should be part of user task", but
> even simple basic feature like session_regenerate_id() can NOT work as
> it supposed without timestamp based management. (Other mandatory tasks
> have problems also, but I ignore them for now)

I was under impression it already works as it supposed to, but obviously
you are of different opinion, so I'd like to hear details about why. If
you're talking about the case where network connection drops in the
middle of reading the new session ID, I still think it is a very exotic
and rare use case and claiming this function needs to be deleted because
it doesn't cover this case makes little sense to me.

I've been using PHP servers with session_regenerate_id() for many years
and never heard complaints about this. Of course, your user base may be
different, but I still question that we need to implement a complex
system and overhaul most of the session management to serve this rare case.

> There is userland workaround as described. User can implement their
> own session_regenerate_id() as described in the manual page.
> 
> Since session management is very important feature for web apps, we
> shouldn't keep providing halfway implemented API forever.
> Implementation or removal is required.
>   timestamp based (precise) session management again.

What substantial changes do you propose to make since the previous RFC?
We can't just keep voting on the same thing until we get result that you
like, that's not how it works.

>   session_regenerate_id() deprecation now and removal in future version.

This makes no sense to me, at least without a lot of additional
explanation.

-- 
Stas Malyshev
smalyshev@gmail.com