Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:95100
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain thefsb.org designates 108.166.43.83 as permitted sender)
To: Yasuo Ohgaki <yohgaki@ohgaki.net>
References: <CAGa2bXaZhfXbU6N0-JGVPzkkxRdkcjEhkHP+YrQNH=XFX714-Q@mail.gmail.com>
 <CAPwsocFAMofROQE-OBfNiR4yrKKSss_owu0svu+Db66DmRbShw@mail.gmail.com>
 <CAGa2bXaxRA91q+ev-X3okKpNC=s3XEwC9deUYikTaUQPfOE-JQ@mail.gmail.com>
Cc: Leigh <leight@gmail.com>,
 "internals@lists.php.net" <internals@lists.php.net>
Message-ID: <cc719c6c-598d-b103-96bf-eb3714a61365@thefsb.org>
Date: Fri, 12 Aug 2016 14:11:12 -0400
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:45.0)
 Gecko/20100101 Thunderbird/45.2.0
MIME-Version: 1.0
In-Reply-To: <CAGa2bXaxRA91q+ev-X3okKpNC=s3XEwC9deUYikTaUQPfOE-JQ@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] [RFC][VOTE] Add session_create_id() function
From: fsb@thefsb.org (Tom Worster)

On 8/11/16 6:58 PM, Yasuo Ohgaki wrote:
> Hi Leigh,
>
> On Fri, Aug 12, 2016 at 3:25 AM, Leigh <leight@gmail.com> wrote:
>> On Wed, 10 Aug 2016 at 10:15 Yasuo Ohgaki <yohgaki@ohgaki.net> wrote:
>>>
>>> Hi all,
>>>
>>> This is RFC for adding session_create_id() function.
>>>
>>> Session ID string uses special binary to string conversion. Users
>>> should write lengthy and slow code to have the same session ID string
>>> as session module does.
>>
>>
>> I disagree, this pretty much covers it:
>>
>> function session_create_id()
>> {
>> $encoded = base64_encode(random_bytes(random_bytes(32)));
>> // Use same charset as PHP
>> return rtrim(strtr($encoded, '+/', ',-'), '=');
>> }
>
> Thank you for insight!
>
> You've missed to set SID to proper length and SID validation.

Replacing rtrim() with substr() is fixes that.


> function session_create_id(string $prefix)
> {
>     $encoded = base64_encode(ini_get('session.sid_length')*2);

Did you omit random_bytes() in this line?


>     // Use same charset as PHP
>     $sid = substr(rtrim(strtr($encoded, '+/', ',-'), '='), 0,
>                           ini_get('session.sid_length');
>
>     $sid .= $prefix;
>
>     // Now validate SID so that it does not have collisions
>     when session is active, connect to database and validate SID
>       try to fetch sid
>         if sid is not there
>           try again to generate SID few times
>       if SID validation failed
>          fatal error
>       return safe SID
>    when session is inactive
>       return unvalidated SID
> }
>
> This is what proposed session_create_id() does.
> I used pseudo, but it should be easy to imagine it would be lengthy code.

You don't need to waste time checking for collisions if the SID has a 
random component of sufficient length. 32 random base-64 characters is 
sufficient.

There are lots of purposes for random strings with negligible chance of 
collision. Hence some frameworks provide the function, e.g.
http://www.yiiframework.com/doc-2.0/yii-base-security.html#generateRandomString()-detail 


Only the search of the the session database for collisions seems hard to 
me. But I don't understand why it is needed.

Tom