Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:56681
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.213.42 as permitted sender)
MIME-Version: 1.0
Sender: yohgaki@gmail.com
In-Reply-To: <4ED5EBA0.7020000@sugarcrm.com>
References: <CAGa2bXY3bjS_WR3sezQ+z=Knq6_v_9dGgi-HZfAqDpQsoUnAGA@mail.gmail.com>
 <4ED5EBA0.7020000@sugarcrm.com>
Date: Wed, 30 Nov 2011 19:35:43 +0900
Message-ID: <CAGa2bXaorKWLw2B+0-refBH6B1M_CrW3n8jistNkAHPkYqtKOw@mail.gmail.com>
To: Stas Malyshev <smalyshev@sugarcrm.com>
Cc: "internals@lists.php.net" <internals@lists.php.net>
Content-Type: text/plain; charset=ISO-8859-1
Subject: Re: [PHP-DEV] [RFC] Strict Session
From: yohgaki@ohgaki.net (Yasuo Ohgaki)

Hi Stats,

2011/11/30 Stas Malyshev <smalyshev@sugarcrm.com>:
>> I though I've better to start new thread, since I changed the status
>> to "Under Discussion".
>>
>> This is RFC for making PHP session strict.
>>
>> https://wiki.php.net/rfc/strict_sessions
>>
>> I'll implement DoS protection later, since current patch pretty well
>> tested and suitable for PHP 5.4/5.3, too.
>
>
> I've checked out the RFC and the patch, and I have couple of notes:
>
> 1. Why we need separate validate call in the API? Can't we just do the
> checks in open/read?

open is used for creating session if does not exists. read cannot be
failed if there is session data, so separate API is needed to check if
it's a initialized one or not.

> 2. Very restrictive limits on session key values don't look useful for me -
> I know some custom solutions use characters beyond alphanumerics in session
> IDs. Of course it can be worked around with encoding, etc. - but what does
> it add?

It mainly for precaution for unknown security risks. Allowing special
chars often became cause of security incidents. It may be ok for allow
more chars or just denying lower special chars may be sufficient.

By the way, I'm planning to write security check module that checks
all inputs at once. It may detect session ID cookie as possibly
malicious input, but users may ignore anyway.

> 3. Why replacing php_session_create_id with custom functions doing the same
> in each standard module?

Bundled ps_module is now using PS_MOD_SID/PS_MOD_FUNCS_SID instead of
PS_MOD/PS_MOD_FUNCS.  In order to use validate function, use of
PS_MOD_SID/PS_MOD_FUNCS_SID is required. As you can see, bundled
ps_modules are just calling internal session ID creation function
which has already been there.

PS_MOD_SID/PS_MOD_FUNCS_SID may implement it's own session ID
generation algorithms for whatever reasons. For example, it allows
ps_module writers to give special meaning to session IDs if they want.

We can force to use internal session ID creation function, though.

> 4. I'm not feeling very comfortable getting such a big change (API change,
> logic change, etc.) with unknown effects this late in 5.4. I'd much better
> prefer doing it in 5.4.1 but API change doesn't really allow that either.

For ps_module writers, they still can use old API. User module may use
new API (i.e. new parameters for session ID creation and session ID
validation function), but they still may use old way, too. I would
prefer to have this patch for 5.4.0, but I don't insist. I've waited
for more than 6 years already, so it does not matter much :)

--
Yasuo Ohgaki