Last modified: 2012-04-12 19:03:43 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T29309, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 27309 - User.php : make $_SESSION parameters wiki-database specific ("per-wiki"). Cookies are already.
User.php : make $_SESSION parameters wiki-database specific ("per-wiki"). Coo...
Status: RESOLVED WONTFIX
Product: MediaWiki
Classification: Unclassified
User login and signup (Other open bugs)
1.16.x
All All
: Lowest critical (vote)
: ---
Assigned To: Nobody - You can work on this!
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-02-10 23:10 UTC by T. Gries
Modified: 2012-04-12 19:03 UTC (History)
2 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments
diff of my patch against BRANCH 1.16.2 User.php r64678 (3.75 KB, patch)
2011-02-10 23:10 UTC, T. Gries
Details
diff of my patch against TRUNC User.php r81868 (3.48 KB, patch)
2011-02-10 23:56 UTC, T. Gries
Details

Description T. Gries 2011-02-10 23:10:01 UTC
Created attachment 8122 [details]
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.
Comment 1 Daniel Friesen 2011-02-10 23:27:27 UTC
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.
Comment 2 T. Gries 2011-02-10 23:31:09 UTC
(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.
Comment 3 T. Gries 2011-02-10 23:33:07 UTC
(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).
Comment 4 T. Gries 2011-02-10 23:35:13 UTC
> 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 ?)
Comment 5 Daniel Friesen 2011-02-10 23:39:40 UTC
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.
Comment 6 T. Gries 2011-02-10 23:43:36 UTC
(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 ?
Comment 7 Daniel Friesen 2011-02-10 23:54:21 UTC
(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.
Comment 8 T. Gries 2011-02-10 23:56:43 UTC
Created attachment 8123 [details]
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.
Comment 9 T. Gries 2011-02-10 23:58:31 UTC
> 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 .
Comment 10 T. Gries 2011-02-11 00:00:57 UTC
I declare this closed, but potential wiki farm users are warned.
Comment 11 Krinkle 2011-02-11 00:02:51 UTC
(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.
Comment 12 Platonides 2011-02-11 00:03:25 UTC
> 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.
Comment 13 T. Gries 2011-02-11 00:09:04 UTC
(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.
Comment 14 T. Gries 2011-02-11 00:10:08 UTC
(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
Comment 15 Daniel Friesen 2011-02-11 00:18:15 UTC
(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.
Comment 16 T. Gries 2011-02-11 01:48:56 UTC
> $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
Comment 17 Daniel Friesen 2011-02-11 01:58:18 UTC
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.
Comment 18 T. Gries 2011-02-11 02:14:14 UTC
(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 .
Comment 19 Daniel Friesen 2011-02-11 02:55:49 UTC
(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.
Comment 20 Daniel Friesen 2012-04-12 18:48:56 UTC
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.
Comment 21 Chad H. 2012-04-12 19:03:43 UTC
If we stop using $_SESSION and $_COOKIE directly and use wrappers like we should, this is easily fixable.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links