Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:94380
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain ohgaki.net designates 180.42.98.130 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CA+kxMuQT=XMSGuJ2jen9-8AzpdMdTTD5+uw67tEn6eTT2favvA@mail.gmail.com>
References: <CAGa2bXaSkozJLLtycujwmHyas9TofRw5s6JzG0EQQvPyU0pxNQ@mail.gmail.com>
 <CAGa2bXZ=DiqQ4PozYntvGApZHdkjWyTBQhk_Udy5Y3_m0ry9Lw@mail.gmail.com> <CA+kxMuQT=XMSGuJ2jen9-8AzpdMdTTD5+uw67tEn6eTT2favvA@mail.gmail.com>
Date: Tue, 5 Jul 2016 11:59:27 +0900
Message-ID: <CAGa2bXbrvH7Wbj1DDkZfK1piNEGcfKt0BzZCQH54jp++3VbRww@mail.gmail.com>
To: Dan Ackroyd <danack@basereality.com>
Cc: "internals@lists.php.net" <internals@lists.php.net>, Dan Ackroyd <danack@php.net>, 
	Stanislav Malyshev <smalyshev@gmail.com>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [RFC][VOTE] Session ID without hashing
From: yohgaki@ohgaki.net (Yasuo Ohgaki)

Hi Dan,

On Tue, Jul 5, 2016 at 9:36 AM, Dan Ackroyd <danack@basereality.com> wrote:
>> Could you share the reason why against this change?
>
> The RFC is doing separate things:

No.
It simply follows best practice not to reinvent wheel and keep things simple.

>
> * Using a proper random number generator - which is probably a good
> thing, and I probably would vote/support that change by itself.

Use of not a really random values like pid and time is bad practice.

>
> * Removing old stuff for performance reasons - probably a bad thing,
> as minor performance improvements don't justify changing the ini
> settings imo.

It's not an objective, but side effect improvement.

>
> * Slipping in a change to make 'session.use_strict_mode' be enbled by
> default. I actually agree with this change, but I don't like how it's
> been combined into an unrelated RFC.

There is a request for inclusion, but I made separate RFC.

Besides that making separate RFCs for this kind of simple mandatory
change does not make much sense. I have too many RFCs already. I
created RFC for it, though.

> * Exposing information that could be useful to an attacker, from the RFC:

I don't understand this part.

>
>> If PRNG has vulnerability that generates predictable value, exposing
>> raw PRNG state helps attackers. Although, this is unlikely with modern PRNGs, but
>> risk is there.
>
> I don't agree that faster session generation is worth exposing the
> random number generator in a completely known way to the outside
> world. Reading an extra 60 bytes almost certainly does nothing to
> mitigate that exposure.

Don't miss the basis of this RFC. It's not PRNG, but CSPRNG.
Exposure of raw CSPRNG should not be a security issue at all by CSPRNG
definition. Additional random data that read from CSPRNG should be good
enough mitigation.

>
>> cryptographers recommends
>> not to reinvent crypt related functions/features.
>
> They also recommend reducing your attack surface as much as possible,
> not opening it up to the point where you're relying on the CSPRNG to
> work perfectly to avoid having a security hole.

Use of poor random sources like pid and time is bad practice.

Old SSL used multiple hashes to mitigate hash functions'
vulnerabilities, but this method is not used today for reason.

>
> But more importantly, as I wrote before
> (http://news.php.net/php.internals/90940) I don't think the session
> extension is in a state where incremental improvements are a good way
> to proceed:
>
>> The problem is that you're trying to build on a foundation of sand.
>> The session handling works but is incredibly fragile.
>
>> To me, there are two good ways to proceed:
>
>> i) Develop a new session extension, that doesn't depend on magic
>> behaviour of globals and leave the current session handler as it is.
>> The new session extension could be shipped as a 'work in progress' when
>> it's good enough, before PHP 8. Then when it's stable, we could figure
>> out how to transition users from the old extension.

Basic design of session module is good enough, IMHO.
Even if we rewrite session module, it will end up with similar
implementation. Complexity of state(session) management cannot be
reduced by rewrite.

Object interface has/had lots of problems. This part could be improved
one by one. If nobody objects, I can clean them up with a single RFC.

>>
>> ii) Develop a session handler in userland code only. PHP is powerful
>> enough to support this. Although obviously there are big benefits to
>> shipping a session handler with PHP, I don't see any need for it to be
>> done internally other than we don't currently have a way of shipping
>> userland code with an extension. I'm hoping that before PHP 8, the
>> ability to ship PHP code as part of extensions would be in place.

I think this is not good option. Session management is complex and
difficult to understand what it should be. I've proposed both
minimal/simple and complex/comprehensive timestamp based session
management which is mandatory for session management, but both of RFCs
are declined. I suppose this proved difficulty of proper session
management. It's slow when it is implemented as PHP script also.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net