Page MenuHomePhabricator

User.php : make $_SESSION parameters wiki-database specific ("per-wiki"). Cookies are already.
Closed, DeclinedPublic

Description

diff of my patch against BRANCH 1.16.2 User.php r64678

I found the following bug in User.php which is _only_ apparent when running a plurality of wikis on the same server and when users come to the different wikis during the same session.

(Fortunately, the current software fails safely and logs out the user, because the token will finally not match when users switch from one to another wiki in the same session. The patch presents a clean solution that also session parameters are saved per-wiki, which is currently not the case.)

When users access two wikis like http://server/wiki1 and http://server/wiki2 in the same session, the user credentials are taken with first priority from the session (see User.php loadFromSession).

Unlike the cookies names which already reflect the wiki database names in their cookie names like 'wiki1userID', the session currently only uses a database-INDEPENDT name 'wsUserID' etc. like $_SESSION['wsUserID'].

I developed a patch to make the session variables conform to the cookie names and wish to have this or a similar change submitted to the current TRUNK.

The attached patch is for BRANCH 1.16.2. Basically, I added $wgCookiePrefix to _all_ Session variables.


Version: 1.16.x
Severity: critical

Attached:

Details

Reference
bz27309

Event Timeline

bzimport raised the priority of this task from to Lowest.Nov 21 2014, 11:23 PM
bzimport set Reference to bz27309.
bzimport added a subscriber: Unknown Object (MLST).

I don't like this idea, wikis should not be sharing the session like that, if they aren't using the same user database, then they should be using separate sessions, hence separate login cookies. Cookie prefix shouldn't be used for server size session data either.

You should be setting $wgCookiePath on your wiki.

Circling around, off the top of my head, I believe that your patch will break wikis that DO use a shared user database.

(In reply to comment #1)

I don't like this idea, wikis should not be sharing the session like

Well, that's true. But if the server has several subdirectories in which severals wikis "live", then you have this problem. And I am only telling this; it's a flaw of server installation. My patch "heals" the problem that different MediaWiki installations uses the _same_ Session variable names.

(In reply to comment #2)

(In reply to comment #1)

I don't like this idea, wikis should not be sharing the session like

Well, that's true. But if the server has several subdirectories in which
severals wikis "live", then you have this problem. And I am only telling this;
it's a flaw of server installation. My patch "heals" the problem that different
MediaWiki installations uses the _same_ Session variable names.

The problem became apparent when testing several advanced authentication extensions. Normally, nobody will take notice of it - users will be simply logged out (as if the cookies have expired).

The problem became apparent when testing several advanced authentication
extensions. Normally, nobody will take notice of it - users will be simply
logged out (as if the cookies have expired).

Because it relates to user logins, I set the importance to "critical". It is up to the group of core developers to decide if this is correct. (Brion, pls. can you comment ?)

The only configuration you describe where $wgCookiePath will not work is one where you have a wiki in /foo/index.php and another wiki in /foo/bar/index.php (installing MediaWiki inside another MediaWiki directory)... The usual /foo/index.php /bar/index.php is precisely what $wgCookiePath should be used for.

If you absolutely MUST have a wiki in /foo/index.php and another in /foo/bar/index.php they should be configured to use different session cookies. They should not be sharing a session. The fix is NOT to go prefixing internal session keys.

(In reply to comment #5)

The only configuration you describe where $wgCookiePath will not work is one
where you have a wiki in /foo/index.php and another wiki in /foo/bar/index.php
(installing MediaWiki inside another MediaWiki directory)...

The described effect is also when using /foo1/index.php and another wiki in /foo2/index.php .

You are right in saying to use different session cookies. From Security Audit reason I only wanted to point out that foo2 sees(regards) the foo1 Session parameter in the _current_ mediawiki version WHY NOT adding the $wgCookiePrefix ?????? which solves this finally ?

(In reply to comment #6)

(In reply to comment #5)

The only configuration you describe where $wgCookiePath will not work is one
where you have a wiki in /foo/index.php and another wiki in /foo/bar/index.php
(installing MediaWiki inside another MediaWiki directory)...

The described effect is also when using /foo1/index.php and another wiki in
/foo2/index.php .

You are right in saying to use different session cookies. From Security Audit
reason I only wanted to point out that foo2 sees(regards) the foo1 Session
parameter in the _current_ mediawiki version WHY NOT adding the $wgCookiePrefix
?????? which solves this finally ?

You ONLY see this because you are not properly setting $wgCookiePath. I said "The only configuration you describe where $wgCookiePath will not work is one
where you have a wiki in /foo/index.php and another wiki in /foo/bar/index.php".

/foo1 Should have $wgCookiePath = "/foo1"; and /foo2 should have $wgCookiePath = "/foo2";

There is nothing more secure about prefixing internal $_SESSION data with a cookie prefix. The fact that you have this conflict means that these wikis already know each other's $_SESSION (which they SHOULD NOT), THAT is insecure, prefixing keys we use inside $_SESSION WILL NOT fix that making it more secure, because both wikis will still be able to access each other's session data. The only proper thing to do is to properly make use of http's cookie domain and cookie path to restrict cookies so that the two wikis do not have access to each other's session data.

Created attachment 8123
diff of my patch against TRUNC User.php r81868

This patch has not yet been tested. It contains the same changes as described: per-wiki sessione parameter, if several wikis are run on the same server.

Attached:

There is nothing more secure about prefixing internal $_SESSION data with a
cookie prefix. The fact that you have this conflict means that these wikis
already know each other's $_SESSION (which they SHOULD NOT), THAT is insecure,
prefixing keys we use inside $_SESSION WILL NOT fix that making it more secure,
because both wikis will still be able to access each other's session data. The
only proper thing to do is to properly make use of http's cookie domain and
cookie path to restrict cookies so that the two wikis do not have access to
each other's session data.

Yes, you are right - as said before - I admit. But there's is also nothing wrong in adding $wgCookiePrefix .

I declare this closed, but potential wiki farm users are warned.

(just a little basic info here)
The session data is stored on the server side identified by a unique id. The reference to this is stored in a cookie, so that when you go to the wiki PHP checks which cookie you have so it known which session belongs to you.

When the path of this cookie is set properly, when you go to the different wikis both will set their own cookie with an id for that wiki's path. Then you visit the wiki and PHP get's the id, but because the coookie is set in a certain path it only gets the right one.

In a way 'path' does exactly what you attempt to accomplish with the 'prefix' you request, but better.

the browser only sends cookies that match the current path. so a request to /server/foowiki/some/dir/ will be sent the cookie that was set with path=/foowiki.

Likewise, visiting /server/foowiki/barwiki/somedir will result in the browser sending cookies that match that current path, including:

  • / (root, sitewide)
  • /foowiki (also matches cookies set for the other wiki...)
  • /foowiki/barwiki
  • /foowiki/barwiki/somedir

I hope that made sense.

Unlike the cookies names which already reflect the wiki database names in their
cookie names like 'wiki1userID', the session currently only uses a
database-INDEPENDT name 'wsUserID' etc. like $_SESSION['wsUserID'].

This makes no sense. $_SESSION['wsUserID'] at http://server/wiki1 will access storage pointed by cookie wiki1userID, while $_SESSION['wsUserID'] at http://server/wiki2 will access storage pointed by cookie wiki2userID

I developed a patch to make the session variables conform to the cookie names
and wish to have this or a similar change submitted to the current TRUNK.

The attached patch is for BRANCH 1.16.2. Basically, I added $wgCookiePrefix to
_all_ Session variables.

You would need to have changed $wgSessionName or $wgCookiePrefix in order to have such behavior.

(In reply to comment #12)

This makes no sense. $_SESSION['wsUserID'] at http://server/wiki1 will access
storage pointed by cookie wiki1userID, while $_SESSION['wsUserID'] at
http://server/wiki2 will access storage pointed by cookie wiki2userID

Platonides, please can you check this on your server.

I have really _found_ that (in my) installation of several wikis http://server/wiki1 and http://server/wiki2 there is only _one_ PHPSESSID .
Of course, there are distinct cookies, wiki1UserID and wiki2UserID etc. which brought me creating this bugzilla.

(In reply to comment #13)

(In reply to comment #12)

This makes no sense. $_SESSION['wsUserID'] at http://server/wiki1 will access
storage pointed by cookie wiki1userID, while $_SESSION['wsUserID'] at
http://server/wiki2 will access storage pointed by cookie wiki2userID

Platonides, please can you check this on your server.

I have really _found_ that (in my) installation of several wikis
http://server/wiki1 and http://server/wiki2 there is only _one_ PHPSESSID .
Of course, there are distinct cookies, wiki1UserID and wiki2UserID etc. which
brought me creating this bugzilla.

Remark: wiki1 and wiki2 share the same core code by using symbolic links

(In reply to comment #14)

(In reply to comment #13)

(In reply to comment #12)

This makes no sense. $_SESSION['wsUserID'] at http://server/wiki1 will access
storage pointed by cookie wiki1userID, while $_SESSION['wsUserID'] at
http://server/wiki2 will access storage pointed by cookie wiki2userID

Platonides, please can you check this on your server.

I have really _found_ that (in my) installation of several wikis
http://server/wiki1 and http://server/wiki2 there is only _one_ PHPSESSID .
Of course, there are distinct cookies, wiki1UserID and wiki2UserID etc. which
brought me creating this bugzilla.

Remark: wiki1 and wiki2 share the same core code by using symbolic links

$wgCookiePath, session.name, $wgSessionName. You have all these at your disposal to properly configure the session settings so that each wiki gets it's own session as it should. MediaWiki does not set cookiepath or override session.name on it's own by default because it does not know enough information about the path and config of other wikis to do so without breaking, and overriding the session cookie on it's own would override any php.ini session.name settings you make.

If you have multiple wiki on the same domain $wgCookiePath should be set, if you have one of the edge cases where you absolutely cannot use proper http cookie domain/path settings to restrict sessions to each wiki (a wiki in /foo/ and another in /foo/bar) then use session.name or $wgSessionName to change the name of the cookie used to get the wiki's session from.

$wgCookiePath, session.name, $wgSessionName. You have all these at your
disposal to properly configure the session settings so that each wiki gets...

Thanks for your valuable tips. I tried to use $wgSessionName="something" and found again only PHPSESSID - in my wiki-farm installation. Then I found, that the extension http://www.mediawiki.org/wiki/Extension:HttpAuth caused all of my problems, because this extension uses - it needs some additional code in LocalSettings.php - only

session_start(); // php uses standard session name

Respecting what you wrote I changed that immediately to

Cookie settings:

$wgCookiePath = $wgScriptPath; allow per-wiki cookie paths
$wgSessionName = $wgDBname."Session";
allow per-wiki session names
...
session_name($wgSessionName); // set up a per-wiki session name
session_start();

Thanks for assistance.

references:
http://www.mediawiki.org/wiki/Extension:HttpAuth
http://www.php.net/manual/en/function.session-id.php#72330
http://www.mediawiki.org/wiki/Session.name

Looking at trunk, you don't need the $wgSessionName, we already set $wgCookiePrefix . '_session'. There is one documented instance where we don', because we can't. If you have session.auto_start set in your php.ini $wgSessionName can't do what it does at all. Setting $wgCookiePath and making sure that you don't have session.auto_start off in your config should be enough.

(In reply to comment #17)

Looking at trunk, you don't need the $wgSessionName, we already set
$wgCookiePrefix . '_session'. There is one documented instance where we don',
because we can't. If you have session.auto_start set in your php.ini
$wgSessionName can't do what it does at all. Setting $wgCookiePath and making
sure that you don't have session.auto_start off in your config should be
enough.

I like that, and find it useful, but:

(MW 1.15.1, 1.16.2) Unfortunately, the session name you are referring to is setup later (in setup.php) and thus not yet available when LocalSettings.php runs with my - yes, like you, I do not like it - extension HttpAuth code as mentioned (I mean the early session_start() command).

Perhaps, other and better Authentication plugins are possible, but not available for my problem. For the moment, I am very happy to the thorough explanations, and many useful suggestions to overcome this "non-"problem. I already fixed my code by adding appropriate settings of

$wgCookiePath = $wgScriptPath; allow per-wiki cookie paths
$wgSessionName = $wgDBname."Session";
allow per-wiki session names
session_name($wgSessionName); // set up a per-wiki session name

in LocalSettings.php .

(In reply to comment #18)

(In reply to comment #17)

Looking at trunk, you don't need the $wgSessionName, we already set
$wgCookiePrefix . '_session'. There is one documented instance where we don',
because we can't. If you have session.auto_start set in your php.ini
$wgSessionName can't do what it does at all. Setting $wgCookiePath and making
sure that you don't have session.auto_start off in your config should be
enough.

I like that, and find it useful, but:

(MW 1.15.1, 1.16.2) Unfortunately, the session name you are referring to is
setup later (in setup.php) and thus not yet available when LocalSettings.php
runs with my - yes, like you, I do not like it - extension HttpAuth code as
mentioned (I mean the early session_start() command).

Perhaps, other and better Authentication plugins are possible, but not
available for my problem. For the moment, I am very happy to the thorough
explanations, and many useful suggestions to overcome this "non-"problem. I
already fixed my code by adding appropriate settings of

$wgCookiePath = $wgScriptPath; allow per-wiki cookie paths
$wgSessionName = $wgDBname."Session";
allow per-wiki session names
session_name($wgSessionName); // set up a per-wiki session name

in LocalSettings.php .

$wgSessionName's fallback has always been set in Setup.php, when you don't set it we default to $wgCookiePrefix . '_session'. FWIW, that HttpAuth extension states on it's own extension page that it has not been maintained in years.

Thinking about this more. We have another good reason to not fix this.
Retaining the same session token across login leaves the potential for session fixation attacks to be made. It's possible that in the future we could conceivably want to reset the session id on login. And that case, unless we do a full clone of the whole session data that means that logging you on one wiki will kill the session data for another wiki. So we DO want to encourage users to set a cookie prefix.

If we stop using $_SESSION and $_COOKIE directly and use wrappers like we should, this is easily fixable.