Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:56682
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.160.170 as permitted sender)
MIME-Version: 1.0
Sender: yohgaki@gmail.com
In-Reply-To: <CAGa2bXaorKWLw2B+0-refBH6B1M_CrW3n8jistNkAHPkYqtKOw@mail.gmail.com>
References: <CAGa2bXY3bjS_WR3sezQ+z=Knq6_v_9dGgi-HZfAqDpQsoUnAGA@mail.gmail.com>
 <4ED5EBA0.7020000@sugarcrm.com> <CAGa2bXaorKWLw2B+0-refBH6B1M_CrW3n8jistNkAHPkYqtKOw@mail.gmail.com>
Date: Wed, 30 Nov 2011 19:49:05 +0900
Message-ID: <CAGa2bXZO0fcrPsEsFEOv1uZBrQP3BBLuTYDajvVMaTw9X9p1Vw@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
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] [RFC] Strict Session
From: yohgaki@ohgaki.net (Yasuo Ohgaki)

Hi Stats,

One additional comment.

2011/11/30 Yasuo Ohgaki <yohgaki@ohgaki.net>:
> 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.

It is possible to make open always initialize with new session ID if
there is not exiting data. In this case, PS_MOD/PS_MOD_FUNCS macro can
be used. If we do this, users cannot know if ps_modules is new one
that prevents adoption or not, especially for third party ps_modules.
If API is the same, it cannot distinguish if ps_module supports
validation or not.

Those who are using user save handlers should rewrite open function so
that it validates session ID and regenerate  session ID using
session_regenerate_id(). It would nicer if we provide API for
validation and there will be less mistakes in user save handlers.

--
Yasuo Ohgaki

>
>> 2. Very restrictive limits on session key values don't look useful for m=
e -
>> I know some custom solutions use characters beyond alphanumerics in sess=
ion
>> IDs. Of course it can be worked around with encoding, etc. - but what do=
es
>> 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 s=
ame
>> in each standard module?
>
> Bundled ps_module is now using PS_MOD_SID/PS_MOD_FUNCS_SID instead of
> PS_MOD/PS_MOD_FUNCS. =A0In 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 chang=
e,
>> logic change, etc.) with unknown effects this late in 5.4. I'd much bett=
er
>> 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