Re: Solution for session_regenerate_id() issues
On 14 March 2014 09:37, Yasuo Ohgaki <[email protected]> wrote:
>
> Hi Mateusz,
>
> On Fri, Mar 14, 2014 at 4:41 PM, Mateusz Kocielski <[email protected]>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.
>
> 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.
>
> This should be session manager task and calling session_regenerate_id()
> should be enough for PHP users, IMO.
>
> 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.
>
> Regards,
>
> --
> Yasuo Ohgaki
> [email protected]
For me, an arbitrary delay time is not a solution to this. The point
of this (unless I misunderstand) is to prevent session hijacking with
the old session ID, and simply reducing the the time window where this
can happen doesn't really solve anything, as I see it.
The only flow that makes sense to me (aside from simply nuking the old
session data immediately when the ID is regenerated) goes as follows:
- A request regenerates the session ID
- The old session data is somehow marked as read-only.
- All outstanding requests that the server has *already received* that
are blocked waiting for a lock on the session are able to access the
old data on a read-only basis.
- When the last one of these requests has been processed, destroy the
old session data.
I realise this is likely impractical to implement in such a way that
it works reliably (or even at all) with every SAPI.
Hence I believe the only *practical* solution here is simply to
educate users and encourage the use of session_regenerate_id(TRUE) in
the documentation.
Thread (24 messages)