Last modified: 2010-02-09 16:25:47 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 T24108, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 22108 - Restricting OpenID using $wgOpenIDConsumerAllow can be bypassed
Restricting OpenID using $wgOpenIDConsumerAllow can be bypassed
Status: RESOLVED FIXED
Product: MediaWiki extensions
Classification: Unclassified
OpenID (Other open bugs)
unspecified
All All
: Normal critical (vote)
: ---
Assigned To: Sergey Chernyshev
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-01-15 15:31 UTC by Craig Box
Modified: 2010-02-09 16:25 UTC (History)
0 users

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


Attachments
Security fix for issue 22108 (1.71 KB, patch)
2010-01-27 09:48 UTC, Craig Box
Details

Description Craig Box 2010-01-15 15:31:10 UTC
Picture this situation:

LocalSettings.php:

    # Deny all OpenIDs except the ones we allow:
    $wgOpenIDConsumerDenyByDefault = true;
    $wgOpenIDConsumerAllow = array("@^(http://)?mydomain.com@");

Someone enters in http://mydomain.com/ - validation passes - all is good.

Then, then take the URL that their browser is redirected to (I used Firefox's Live HTTP Headers plugin to see this), copy the OpenID parameters off it, and then suffix them onto a server which they have an account on (say, https://login.launchpad.net/+openid).

Voila, I am logged into this supposedly-restricted wiki with an OpenID that fails the consumer allow test.

Two things come to mind here:

  - Aren't OpenID requests meant to be signed, so they can't be tampered with in this way?
  - You should validate after the OpenID is returned, not (just) before. 

For example, I have solved this by adding to SpecialOpenIDFinish.body.php 

			 case Auth_OpenID_SUCCESS:
				// This means the authentication succeeded.
				$openid = $response->getDisplayIdentifier();
				// Check we have been given an identifier we accept.
				if (!$this->canLogin($openid_url)) {
					$wgOut->showErrorPage('openidpermission', 'openidpermissiontext');
					return;
				}

However, I don't think this will stop a OP that is prepared to issue an ID in any name - only signing will fix that.
Comment 1 Craig Box 2010-01-15 16:53:23 UTC
Andrew Arnott from DotNetOpenAuth has explained the situation to me here.

In summary, the RP library should stop the "ID issued in any name" case, by signature verification, so the only thing we need to do is check that the assertion is acceptable with the code above.  

However, we shouldn't be checking the display identifier, which can be set to whatever you want - we should be checking the identity_url.  See http://openidenabled.com/files/php-openid/docs/2.1.3/OpenID/Auth_OpenID_ConsumerResponse.html.  

Patch forthcoming...
Comment 2 Craig Box 2010-01-27 09:48:08 UTC
Created attachment 7031 [details]
Security fix for issue 22108

Here is the patch.

Note, I am now checking identity_url rather than displayIdentifier; the user can set the display identifier, so you shouldn't ever check it (e.g. I could set up a provider to give out the ID http://badprovider.com/me but the display identifier set to http://impersonatedprovider.com/you.
Comment 3 Sergey Chernyshev 2010-01-27 20:36:47 UTC
I applied it in r61601 to current trunk. Please test it - I didn't have time to set up testing environment for this problem, unfortunately.
Comment 4 Craig Box 2010-02-09 16:25:47 UTC
Tests OK for me.

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


Navigation
Links