mahara-contributors team mailing list archive

[Aaron Wells <1570179@xxxxxxxxxxxxxxxxxx>]

My rough outline of how this should go:

0. Well, option 1 would be to avoid using PHP's built-in session handler
system at all. (i.e. http://php.net/manual/en/book.session.php ). Since
the PHP session system was *not* built to support non-blocking sessions,
we may get the best results by building our own from scratch. There is
nothing magic about sessions; it's just a cookie with an ID, and a data
store that links serialized data to that ID. However, to avoid the
pitfalls of reinventing the wheel, it would be best if we could re-use
an existing session library.  For instance the "Illuminate" session
package, which is based on Laravel and does not use $_SESSION:
https://packagist.org/packages/illuminate/session . We can't use that
one specifically, though, because it needs PHP 5.5.9

Now, assuming that we *aren't* going to replace $_SESSION entirely,
here's what I would recommend:

1. Replace $SESSION's "is_live()" method with two new statuses:
"is_initiated()", and "is_open()". The "is_initiated()" method would
indicate that we've called session_start() at least once, and hence
$_SESSION is populated with data. The "is_open()" would mean that we've
called session_start() and have not yet called session_write_close() or
session_destroy(), which means that changes to $_SESSION will be
recorded in the session store. We'd need to track these statuses
ourselves through calls to the $SESSION singleton, because PHP provides
no reliable way of detecting them. (PHP 5.4+ does have a
session_status() method, but it is only equivalent to my "is_open()"
method.)

2. User our knowledge of $SESSION->is_initiated() to make it so
$SESSION->get() doesn't call session_start() or session_write_close(),
thus reducing the number of duplicate session cookies generated.

3. Rewrite clear_duplicate_cookies() to use "header_remove()" instead of
"header_remove('Set-Cookie')". The no-params version removes all headers
(which we'd need to record first and add back), while the one-param
version only removes the named type of header. It appears that in PHP
5.3 the duplicate session headers are not removed by the one-param
version, but they are removed by the no-params version. Both forms
remove the duplicate session headers, in later PHP versions.

4. Have $SESSION record the session's ID when the session is first
initiated, or regenerated, and use that knowledge to remove any
incorrect session cookies in clear_duplicate_cookies(). Currently we
remove any non-unique cookies; but it seems this can cause problems when
you regenerate the session ID. Both the old and new session ID may then
be sent out in the response, and it's uncertain which the browser will
use on its next request.

5. Stop using the session to send messages to the user on the next page
request. This causes a lot of unpredictability and unreproduceability in
session bugs, by changing when the session is initiated. Instead, write
error messages to a database table, and fetch them using an "UPDATE...
WHERE..." statement to ensure atomicity, and use a second ID cookie
alongside the session cookie, to tie IDs to users. This would also
reduce our need to create sessions for logged-out users.

6. Move the session's initiation as early as we can in the page load.
This too will help reduce unpredictability. However, one difficulty here
is that we would ideally not create sessions for logged out users (which
may just be automated scripts that don't need a session). To workaround
that, I'd suggest that if we see a session cookie included in the
request, we launch the session early on in the page lifetime. If not, we
avoid initiating until we get to auth_setup() and need to ascertain that
the user is logged in, or until there's a call to $SESSION->set().

7. Bug 1567799: Get rid of the code in auth/session.php
remove_user_sessions($userid) that highjacks sessions as a hacky way of
deleting those sessions. It *is* the only way the PHP session library
will let you clear a given session ID across all session back-ends, but
as mentioned in that bug report, if it fails it causes the user's
session ID to change. We either need to maintain a list of valid
sessions in the database and compare the session ID to that on each page
load, or we need to move the session highjacking code into a server-side
script... but it seems the CLI SAPI can't access sessions, so that's
probably not possible.

-- 
You received this bug notification because you are a member of Mahara
Contributors, which is subscribed to Mahara.
Matching subscriptions: Subscription for all Mahara Contributors -- please ask on #mahara-dev or mahara.org forum before editing or unsubscribing it!
https://bugs.launchpad.net/bugs/1570179

Title:
  More robust Mahara session handler

Status in Mahara:
  Triaged

Bug description:
  I've recently encountered two issues from clients who are on PHP 5.3
  and have upgraded to Mahara 15.04+ and had serious bugs relating to
  session management. These seem to be caused by the changes we made in
  Bug 1532028 (commit 55a8deb8cbd2 ) to provide non-blocking sessions.
  Our fix relied on some undocumented

  By default, PHP uses blocking sessions. That is, when you call
  session_start(), PHP puts a lock on the session store for your session
  ID. This lock is released when you call session_write_close(),
  session_destroy(), or your script ends. While the lock is in place,
  any other script attempting to access the same session ID will wait
  until the lock is released. This gets in the way if you want to run
  multiple AJAX scripts simultaneously which use the session (like the
  ones we use for loading blocks on a page), or to use AJAX scripts
  while a long-running script is running in the background (like for the
  CSV upload status bar).

  We worked around this by opening and then closing the session multiple
  times during a page load. When the session is first launched, we call
  session_start() to load $_SESSION, then immediately call
  session_write_close(). We do this again when we want to write to the
  session, destroy the session, or regenerate the session ID.

  However, there's an underlying bug in PHP that causes the session
  cookie to be sent out multiple times if you call session_start()
  and/or session_write_close() multiple times during a page load. (See
  https://bugs.php.net/bug.php?id=31455 and
  https://bugs.php.net/bug.php?id=38104 ). So, we tried to work around
  this by adding a function that clears out these duplicate page
  headers.

  Anyway, since the duplicate page headers are a bug, our fix relied on
  undocumented behavior of PHP. It appears this behavior may not be
  correct in some distributions of the PHP binary, particularly in PHP
  5.3.3. So, some changes to how we do things may be necessary to avoid
  major session problems for sites still on PHP 5.3. Additionally, an
  investigation of Mahara's session handler during the course of
  addressing these bugs, has shown me some fairly big design flaws in
  how we are currently handling sessions. So it will probably save us
  some pain down the line if we rewrite the auth/session.php code to be
  more robust.

To manage notifications about this bug go to:
https://bugs.launchpad.net/mahara/+bug/1570179/+subscriptions