Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:90959
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.160.172 as permitted sender)
MIME-Version: 1.0
Sender: yohgaki@gmail.com
In-Reply-To: <56A81CA5.2050106@gmail.com>
References: <CAGa2bXbg_kebBVg3tSh4uFHZ6nQXRbjUXNQa0JZx9sezQqb5kg@mail.gmail.com>
 <CAGa2bXYZXkCJwFKVwaNfV+OEw0FzeFFbtxLm0zRmFVE_b4xsMw@mail.gmail.com>
 <CAGa2bXbRJMZFcXfZc9o7YnAhkDRfYKPjO+10k-eKXdoGNTREbg@mail.gmail.com>
 <CAGa2bXYGpV=P-680ig1LOkYWsSRS-Ya2rYVa3wo3vrxdT=zwiw@mail.gmail.com>
 <56A727C3.9070505@gmail.com> <CAGa2bXYj6iCvKqcpf_p6Dm_TNOxtVozry=AONKWu9-VKzb_FHw@mail.gmail.com>
 <CAGa2bXZ3gFegniSBCtfnnrrotbdCh+MjGufC11nYhd7kH31tuQ@mail.gmail.com>
 <56A80626.6080008@gmail.com> <CAGa2bXYWO+0U0urbbUi-nJjKnACYXdRDsg0Jn6AcYus1VsrExQ@mail.gmail.com>
 <56A81CA5.2050106@gmail.com>
Date: Wed, 27 Jan 2016 11:02:10 +0900
Message-ID: <CAGa2bXZwcuvw+HyV3q6DD0kghWyBO-YOiajwB4ykL4pnXPCQGw@mail.gmail.com>
To: Stanislav Malyshev <smalyshev@gmail.com>
Cc: "internals@lists.php.net" <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [PHP-DEV] Re: [RFC Discussion] Precise Session Management
From: yohgaki@ohgaki.net (Yasuo Ohgaki)

Hi Stas,

On Wed, Jan 27, 2016 at 10:25 AM, Stanislav Malyshev
<smalyshev@gmail.com> wrote:
>
>> Other than HTTPS, setting unremovable cookies is easy with JavaScript
>> vulnerability. Currently, we only has session.use_strict_mode=1. This
>> is not good enough because attacker can generate valid session ID by
>
> That sound unlikely, given how much space needs to be searched. By the
> same vein, you could say one could defeat encryption by just guessing
> the key. Which, if you are unbelievably lucky, can be true, but nobody
> is that lucky. I don't see how guessing session ID is easier than
> guessing encryption key.

I should have been precise. JavaScript vulnerability is "JavaScript
injection" vulnerability. It's most common problem of web apps as we
know.

>
>> themselves, and set it to victims cookie. Without session ID
>> regeneration with precise expiration, attacker may keep logged in
>> session forever.
>
> That's not true, as many application provide session expiration in
> userspace.

User may have timestamp for expiration. Problem is it's not a part of
session module management and many codes just let GC to expire session
data as you've mentioned before.

>
>> Described how easy to steal session permanently above.
>
> It was claimed, but not described how that could actually happen.

There are 2 ways to keep/generate stolen session

 - Set undeletable cookie to browser
 - Get active session via exploit and access it before GC

As I have already explained, getting active session ID is trivial with
access to psychical device. e.g. Steal colleges' session ID while they
are leaving desk. It's just a matter of displaying session ID cookie
and take picture of it.

>
>> Without this proposal, user/web site owner will never know possible
>> security breach. With this proposal, user/web site owner can detect it
>> by raised errors for access to expired sessions.
>
> I'm not sure what would be the use of it. Suppose you detected somebody
> is trying to use old sessions to breach your security. Then what?
> I am also not sure which proposal you refer to that helps with access to
> expired sessions - aren't they just deleted? If so, you can't
> distinguish access to expired session from access to one that never
> existed.

Expired sessions are not deleted silently when there is access to it.
Expired sessions will stay in session storage until GCed. Thus, if
there is access to expired sessions, session module detects it and
raise error.

Access to expired session should not happen under normal circumstance.
It's likely a some kind of attacks.

As you've mentioned, my patch resends new session ID only once in case
session ID is lost. It is possible that raised error is due to bad
connections, but it should be very rare. If we get a lot of false
positive, we may resend new session ID more than once.

>
>> Other than restricting chars for session_id(), this proposal will not
>> break application. Other possible minor BC happens when apps rely on
>> constant session ID. Relying on constant session ID is _bad_ practice,
>> but the patch allows to disable automatic session ID regeneration for
>> better compatibility. Tests for session behavior may be affected, but
>> it will not affect production code.
>
> Still do not see how restricting chars is beneficial (handlers already
> restrict what is unacceptable for them).
> Also, as I said, as a particular implementation detail, I don't think
> specific session regeneration pattern belongs in core - not all
> applications would want it, and not all in the same way. Some
> applications would rather prefer to drop the session and force the user
> to re-login, for example.

I don't think it's a good way to allow invalid session key for
initialization, but silently changes key and continues.

Silent removal is done because session module has transid feature.
Users can send any session ID easily. Bad person may set bad chars in
session ID and set it to their web page to give impression that web
apps are broken, etc.

>
>> Other than these, user should not have problems in their production
>> codes. If any, please let me know.
>
> Changing defaults and functionality always brings possibility for
> breakage, and the fact that so many tests needed to be changed also
> suggests that the applications that would use the same code unmodified
> would be broken.

I agree that. That's the reason why I stopped working a year ago to
concentrate PHP7 development. We have almost a year to PHP7.1 release.
It's good timing for this kind of changes, IMHO.

I carefully considered BC impacts and implemented "should/must be"
feature/behavior. I wouldn't say there is no BC, but almost all
production code should not be affected. I'm sure there will be some
broken application unit tests, though.

Many lines of changes in phpt does not mean production code BC because
session's phpts test internal behavior/data, require immediate session
destroy. There is new data, tests should see it and verify it.
session_destroy() requires -1 parameter for immediate destroy for
cleanup. You can verify what I'm saying in phpt diffs.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net