Re: [RFC] Secure Session Module Options by Default

From: Date: Mon, 03 Feb 2014 23:05:06 +0000
Subject: Re: [RFC] Secure Session Module Options by Default
References: 1 2 3 4 5  Groups: php.internals 
Request: Send a blank email to [email protected] to get a copy of this message
Hi Andrey,

On Tue, Feb 4, 2014 at 7:29 AM, Andrey Andreev <[email protected]> wrote:

> >> I was just pointing out that 'use_strict_mode' shouldn't change the
> >> behavior of session_id().
> >> In other words, you don't need a "force" option, because passing a
> >> custom made ID by itself already tells it to force something.
> >
> >
> > Change has been done already. It does not accept uninitialized ID when
> > use_strict_mode=On since 5.5.
>
> I know that. I'm saying it should work like you suggested without the
> need of another option.
>
> session_id(<user input>); // changes session ID to the user-supplied
> string, regardless of use_strict_mode
>
>
You may be referring old RFC. Current RFC is

Add optional $force_id parameter for session_id() to allow any id
regardless of this mode.

  bool session_id(string $new_id [, bool $force_id=FALSE]);

This was changed as Stas(?) suggested since I agree to him. It would be
more secure
than old one.


>> Don't know why you're talking about prefixes here ... nothing to do
> >> with security.
> >
> >
> > I see many codes that uses insecure session ID. The main reason why
> > they write such bad code is 'lack of secure ID generation feature'.
> >
> > It's security matter, IMHO.
>
> Why is that necessary at all? session_regenerate_id() not sufficient?
> Either way, this shouldn't be a part of this RFC.
>
> It's your RFC, but still - it's about changing default settings for
> improved security, not new features.
>

You are ignoring the fact that there is no  'default' function that is
securely generates random bytes/ID.

There are code that requires prefix for much efficient session data
handling.
For example, user may want to know session creation time/owner/etc
*without reading* session data. With prefixed session ID, creation
time/owner/etc
is known just selecting session ID names. For databases like PostgreSQL
which
supports function index, searching specific session is done in msec order
even
when there are millions of session IDs.


>
> >> Not that I have any authority around here, but I'd recommend updating
> >> the RFC to properly explain this. As it stands, it makes it almost
> >> makes it look like incrementing the 'hash_bits_per_character' value by
> >> itself increases security, and it doesn't. :)
> >
> >
> > It does not improve security, but it reduces session ID length changed by
> > this RFC. Are you suggesting another RFC for changing
> > hash_bits_per_character?
> > Or are you suggesting I should change the default without a RFC?
>
> We've obviously got a language barrier here.
> All I'm suggesting is that you improve the *wording* in your RFC, so
> that it is clear why hash_bits_per_character is changed.
>

Thank you for the suggestion.
I'll try to improve.


>
> >> >> That "allow an underscore when hash_bits_per_character=6" is also
> >> >> not
> >> >> in the scope of security and the hash function itself wouldn't
> >> >> generate an underscore, so ... what has it got to do with anything?
> >> >
> >> >
> >> > If hash_bits_per_character=6, files save handler uses all chars for
> >> > session
> >> > ID and users cannot have prefix delimiter char. User may use string
> >> > offset,
> >> > though.  That's the reason why there is '_' proposal.
> >>
> >> This is hard to follow, I really don't understand why we're talking
> >> about prefixing here at all and how does it relate to
> >> 'hash_bits_per_character' or security.
> >
> >
> > To improve security, I proposed larger hash and hash bits to decrease
> number
> > of chars. I know there are users that use prefix and delimiter. Why
> > shouldn't I
> > address these affected users in the same RFC?
> >
> > This security related changes may affect users and if it does, we should
> > provide
> > work round for it at the same time if it is feasible. IMHO.
>
> Because, as I said above - I thought this RFC was about changing
> default settings and nothing else.
> And again, an underscore *is not* a part of the hash, regardless of
> hash_bits_per_character.
>
> I personally don't care about the prefix. It's just that it's a
> completely new feature that has nothing to do with the default
> settings and it's just confusing that you mention it in the same
> context. :)
>

It's new for PHP, but there are codes do this and many of them are
insecure...
I would like to eliminate insecure code by introducing easy to use API also.

I might have chosen wrong name for the RFC.

Regards,

--
Yasuo Ohgaki
[email protected]


Thread (30 messages)

« previous php.internals (#72155) next »