Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:72299
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.215.48 as permitted sender)
MIME-Version: 1.0
Sender: yohgaki@gmail.com
In-Reply-To: <CAPhkiZwosTiKRudaNDhsoXHxkXjP=Qd_T4DCwhFmdJkVRmV=KA@mail.gmail.com>
References: <CAPhkiZxPZPatCKsCCih3YnOUmoUydk6MCvFjFB=AVvS-6bDYUQ@mail.gmail.com>
 <CAGa2bXb1ijUNA=RNFaTDnT7JjuVjWHp1aAiFxFXbCMwQqP-GjQ@mail.gmail.com>
 <CAPhkiZztkiKJQhU8ajajzUWOkbj_Hzon+8iCAVErRw=cMQGA8w@mail.gmail.com>
 <CAGa2bXbRjDZ6j-HuOTbv7OYO8=8nNxwhqKP7kDD=8e6Hnuqs+w@mail.gmail.com>
 <CAPhkiZwkdzzppBjsiFvFh7_mmWtbCKfo=OtYt-1RoB=4Kkcrzg@mail.gmail.com>
 <CAGa2bXYKtKX24p=XfTZdT_2E_543JDmZit6cKGMTLr-GuU7++A@mail.gmail.com>
 <CAPhkiZyP3TOb8Y48GB58zYD6oSbpQ7k5Ngk6Yn2sd4YcJzRwPg@mail.gmail.com>
 <CAGa2bXafy794OxJDk0CNTaY5Te98O84HFAyuMJU7s819bX+tHw@mail.gmail.com> <CAPhkiZwosTiKRudaNDhsoXHxkXjP=Qd_T4DCwhFmdJkVRmV=KA@mail.gmail.com>
Date: Thu, 6 Feb 2014 06:58:44 +0900
Message-ID: <CAGa2bXbwqn5NsTVLDsF01qTEQqX5bzzS=wTDQXtetkKwDY5a0g@mail.gmail.com>
To: Andrey Andreev <narf@devilix.net>
Cc: "internals@lists.php.net" <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=001a1133a818ad8a7704f1afde4a
Subject: Re: [PHP-DEV] [RFC] Secure Session Module Options by Default
From: yohgaki@ohgaki.net (Yasuo Ohgaki)

--001a1133a818ad8a7704f1afde4a
Content-Type: text/plain; charset=UTF-8

Hi Andrey,

On Thu, Feb 6, 2014 at 5:00 AM, Andrey Andreev <narf@devilix.net> wrote:

> >> See my previous comment above.
> >> You didn't chose a wrong name, you seem to have done a 2-in-1 RFC and
> >> that's not the right approach IMO. At the very least - it's obviously
> >> confusing and most of what you've said in this conversation isn't
> >> described in the RFC at all.
> >
> >
> > To me, it's the same objective that make session usage as secure as
> > possible.
> > I don't see reason to make 2 patches/RFC for making session module
> secure.
> > Perhaps, better title would be
> >
> > "Make session options/API as secure as possible"
> >
> > Regarding "_" addition to files save handler, it may not be RFC issue as
> it
> > does not break anything at all. Just an simple addition of safe char that
> > is needed for new safe prefixed session ID with hash bits=6. It may apply
> > even prefixed session. I think there are many changes like this w/o RFC.
> >
> > I tried to write RFC to be minimum and sufficient. I should add more
> > description
> > if it is not. Or add link of this thread. I think it's preferred way.
>
>     Changing default settings in the proposed way makes ext/session
> more secure by default.
>
>     Adding a new parameter to session_id() only gives users an easier
> way to do complete a task that they otherwise *could* do the wrong
> way.
>
> The first has real, straight-forward impact on security and doesn't
> change existing functionality.
> The second only *might* lead to some userland code being more secure
> and it is questionable if that's the proper tool for the job. I for
> one would like more tools that allow me to change a session's
> behavior, but a prefix is not one of them.
>

If you handle millions of sessions and would like to find specific
active sessions with marginal overhead, prefixing is the way to
go. Many users may not need it, but there are users who need.

Currently, there is no easy way to get secure random bytes. Even if
there is, session module is better to have it's own secure ID generator.


> It's not the same thing, not even the same category.
>
> On the '_' character:
>
> Any character passed to session_id() should be allowed, depending on
> hash_bits_per_character and just for the underscore doesn't sound good
> to me.


Since we have transid, some chars are not allow by session module

    /* Finally check session id for dangarous characters
     * Security note: session id may be embedded in HTML pages.*/
    if (PS(id) && strpbrk(PS(id), "\r\n\t <>'\"\\")) {

Save handlers may have stricter limitations. Files is the one, since it
saves
data as file, only safe chars(white list) are allowed.

User may set any ID via session_id(), but session module does not accept
any dangerous session ID and creates new secure ID with use_strict_mode=On.

Anyway, I don't mind removing things that aren't related to INI.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

--001a1133a818ad8a7704f1afde4a--