Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:73161
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain digitalsun.pl from 176.31.254.40 cause and error)
Date: Fri, 14 Mar 2014 11:03:26 +0000
To: Yasuo Ohgaki <yohgaki@ohgaki.net>
Cc: "internals@lists.php.net" <internals@lists.php.net>
Message-ID: <20140314110326.GA80300@mail>
References: <CAGa2bXZT8w9=usaPcJzs8NaiGfCpqvw3NVOBvTuGmSAs3vwxEQ@mail.gmail.com>
 <20140314074112.GB26909@mail>
 <CAGa2bXYQXgJi2jxLxR4iCKb79LwQREUHLZ0z-+D5c-8fd6P1Kw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAGa2bXYQXgJi2jxLxR4iCKb79LwQREUHLZ0z-+D5c-8fd6P1Kw@mail.gmail.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Subject: Re: [PHP-DEV] Solution for session_regenerate_id() issues
From: shm@digitalsun.pl (Mateusz Kocielski)

Hello,

On Fri, Mar 14, 2014 at 06:37:35PM +0900, Yasuo Ohgaki wrote:
> On Fri, Mar 14, 2014 at 4:41 PM, Mateusz Kocielski <shm@digitalsun.pl>wrote:
> 
> > I'm not sure if we should handle that in PHP, application usually
> > regenerates
> > session on important events (i.e. on user login/logout etc.), so any
> > requests
> > with old session should be denied, and this can be achieved using
> > session_regenerate_id(TRUE). Wouldn't it be better to write a security
> > note in the documentation rather than making whole thing more complex?
> >
> 
> The issue is that session_regenerate_id(TRUE) is unreliable. It could cause
> race condition since requests from client are not synchronized. There is no
> way to force synchronized access resources to clients. Out of sync issue
> would be more noticeable under mobile environment, networks with poor
> congestion control and/or the same web app in multiple tabs.

I don't see why we should be so worried about it. Answer from server could not
be delivered, thousand things can simply go wrong, all those "exceptions"
should be handled by the application.

> Users may set timeout flag in $_SESSION array and check the value if
> session could be active or not. In order to do that in user land, one may do
> 
> session_start()
> // ** check timeout flag **
> if (!empty($_SESSION['VALID_UNTIL']) && $_SESSION['VALID_UNTIL'] < time()) {
>    session_destroy();
>    trigger_error('Possible session hijack attack detected');
>    die('Possible session hijack attack detected');
>  }
> 
> // ** set timeout flag **
> if ($_SESSION['LAST_REGENERATE'] < time() + 600) {
>   $_SESSION['VALID_UNTIL'] = time() + 60; // Shorter is better, but rather
> large value is set for lost radio/hand over/etc. Old session is allowed to
> use as valid session for 60 seconds.
>   session_commit(); // Need to save above data in old session.
>   session_start();
>   $_SESSION['LAST_REGENERATE'] = time(); // Update regenerate time here.
>   session_regenerate_id(); // New session ID and old session data with old
> session ID is left
>   unset($_SESSION['VALID_UNTIL']; // This session should not be deleted
> later.
> }
> 
> 
> Something like this should be done for reliable session_regenerate_id(). I
> think not many apps do this.
> 
> Above example is allowing 60 seconds window for legitimate user and
> attacker. If session is hijacked,
>  - User could know attack if session ID is regenerated by attacker.
>  - Attacker could know there is hijack protection if session ID is
> regenerated by user.
> 
> Without code like above, both attacker and user may use the session as long
> as web app allows. User has no chance to know if he/she is under session
> hijack attack or not. Attacker feels safe to enjoy stolen session.

Every pot has two handles, user has no chance to know is he/she is under
session attack, but also server doesn't know who's the attacker and who's the
victim. What if we'll regenerate session upon attacker's request?

> This should be session manager task and calling session_regenerate_id()
> should be enough for PHP users, IMO.

I see your arguments valid, but I feel that it's sweeping under the rug the
real problem, which is handling async requests properly.

> I didn't include automatic session ID regeneration in the RFC, but it could
> be handled by session manager also. It's just a matter of storing/checking
> last regenerate time.  If users are following security best practices, they
> should renew session ID periodically even when HTTPS is used.

Is periodically renewing session ID really a big benefit? If an attacker can
obtain session id from victim once, then with probability ~1 session can be
stolen again.

BTW, don't get me wrong, I'm not against making PHP more secure, I just want
to keep things as simple as possible (which is usually good for security too).

 Regards,
 Mateusz Kocielski