Page MenuHomePhabricator

Restrict access to most actions on $wgWhitelistRead pages on private wikis
Closed, ResolvedPublicSecurity

Description

Original task description:

We should unset $wgWhitelistRead on WMF wikis and instead encourage private wikis to override MediaWiki:Loginreqpagetext to serve the same purpose. This makes it impossible to perform any actions on any pages, fixing some issues on our setup (T34716) and lessening the impact of others (T297322).


The new plan is described in T297416#7568198

Event Timeline

Legoktm triaged this task as Unbreak Now! priority.Dec 9 2021, 7:16 PM
Legoktm subscribed.

Originally I suggested this could wait but given that we found another vulnerable spot, it should be done ASAP. Private wikis can be told to update MediaWiki:Loginreqpagetext afterward, not the end of the world if they don't have a public homepage for a day.

Majavah is doing it already, avoiding stepping on toes.

taavi lowered the priority of this task from Unbreak Now! to High.Dec 9 2021, 7:42 PM
sbassett lowered the priority of this task from High to Low.Dec 13 2021, 5:35 PM
sbassett moved this task from Incoming to Watching on the Security-Team board.
sbassett added a project: SecTeam-Processed.
sbassett subscribed.

So the immediate issue should be resolved for Wikimedia production wikis. I think two questions remain:

  1. Do we ever want to re-enable $wgWhitelistRead options for Wikimedia production wikis? I think the Security-Team would likely rate that as high risk as we're doubtful the issues from last week are the only method of exploiting this config.
  2. Per @Ladsgroup's suggestion (T297322#7564744) do we just want to hard-deprecate $wgWhitelistRead? Seems like a dangerous, hacky thing to keep around unless there isn't another work-around (there is) or there's an extremely important use-case for it (?)

So the immediate issue should be resolved for Wikimedia production wikis. I think two questions remain:

  1. Do we ever want to re-enable $wgWhitelistRead options for Wikimedia production wikis? I think the Security-Team would likely rate that as high risk as we're doubtful the issues from last week are the only method of exploiting this config.

I don't think so.

  1. Per @Ladsgroup's suggestion (T297322#7564744) do we just want to hard-deprecate $wgWhitelistRead? Seems like a dangerous, hacky thing to keep around unless there isn't another work-around (there is) or there's an extremely important use-case for it (?)

This is the way I'm leaning too. Originally $wgWhitelistRead was needed to allow access to Special:Userlogin itself (see https://www.mediawiki.org/w/index.php?title=Manual:$wgWhitelistRead&oldid=234257) but today that's hardcoded in to title permission checks. I do think it needs a bit of public discussion before going ahead with it though, in case other use cases besides "a public main page, everything else private" emerge.

If people have the use case for a few public pages on a otherwise private wiki, I think they're better served by using Lockdown, which also appropriately warns them of the potential risks that people (reasonably IMO) don't expect from a core setting.

There's also $wgWhitelistReadRegexp which should get identical treatment.

  1. Per @Ladsgroup's suggestion (T297322#7564744) do we just want to hard-deprecate $wgWhitelistRead? Seems like a dangerous, hacky thing to keep around unless there isn't another work-around (there is) or there's an extremely important use-case for it (?)

This is the way I'm leaning too...

Filed for (eventual) public awareness/discussion: T297651: Deprecate $wgWhitelistRead and $wgWhitelistReadRegexp (and related code)

I don't think it's necessary to deprecate $wgWhitelistRead in order to prevent unauthorized users from executing arbitrary actions. We can just have actions opt in to being allowed in whitelist read mode, the same as we do for API modules with isReadMode():

		if ( $module->isReadMode() && !$this->getPermissionManager()->isEveryoneAllowed( 'read' ) &&
			!$this->getAuthority()->isAllowed( 'read' )
		) {
			$this->dieWithError( 'apierror-readapidenied' );
		}

Just do the same thing for actions, and we'll be sorted. The REST API already has the same logic in BasicRequestAuthorizer. Maybe ResourceLoader is a remaining attack surface but that is not addressed by deprecating $wgWhitelistRead.

That works for me too. For ResourceLoader we decided that's just how it works since it's intended to work cookieless, see T60291#7306202 (that ticket should probably be made public now).

Here's what that looks like. It prevents access to all other actions besides view. You can still see diffs on $wgWhitelistRead pages, except you have to guess the revision IDs since you can't see history. And if one of the revisions belongs to a separate page, you get a "login required" error.

Here's what that looks like. It prevents access to all other actions besides view. You can still see diffs on $wgWhitelistRead pages, except you have to guess the revision IDs since you can't see history. And if one of the revisions belongs to a separate page, you get a "login required" error.

LGTM; just need to make sure we alter/update the @since in the backports too (so versions >= 1.35.5 doesn't say @since 1.38 etc)

Deployed to wmf.9, wmf.12 and wmf.13, I temporarily reinstated $wgWhitelistRead and and verified that none of the other actions (history, edit, undo, mcrundo, info, rollback) were accessible from the main page, just view.

So I think we can restore $wgWhitelistRead to its original value (removing the override from PrivateSettings.php) now that other actions can't be used for exploits.

The patches are identical, just with different @since values. This also applies directly to REL1_31/REL1_27.

Legoktm renamed this task from Unset $wgWhitelistRead on private WMF wikis to Restrict access to most actions on $wgWhitelistRead pages on private wikis.Dec 15 2021, 3:24 AM
Legoktm added a project: MediaWiki-General.
Legoktm updated the task description. (Show Details)
Reedy changed the visibility from "Custom Policy" to "Public (No Login Required)".
Reedy changed the edit policy from "Custom Policy" to "All Users".

Change 747587 merged by jenkins-bot:

[mediawiki/core@REL1_37] SECURITY: Require 'read' right for most actions

https://gerrit.wikimedia.org/r/747587

Deployed to wmf.9, wmf.12 and wmf.13, I temporarily reinstated $wgWhitelistRead and and verified that none of the other actions (history, edit, undo, mcrundo, info, rollback) were accessible from the main page, just view.

Thanks. Sorry I didn't get to fully review these, but it looks like @Reedy was cool with everything and clearly nothing has broken. I'll decline T297651 and track the patch at T276237 since it'll still need to be on .12 and .13 for the remainder of this week, at least.

So I think we can restore $wgWhitelistRead to its original value (removing the override from PrivateSettings.php) now that other actions can't be used for exploits.

Should be a trivial deploy that @Reedy or I can handle after the afternoon train today.

So I think we can restore $wgWhitelistRead to its original value (removing the override from PrivateSettings.php) now that other actions can't be used for exploits.

Should be a trivial deploy that @Reedy or I can handle after the afternoon train today.

I was planning on doing that tomorrow but that works too. Thanks!

Change 747580 merged by jenkins-bot:

[mediawiki/core@REL1_36] SECURITY: Require 'read' right for most actions

https://gerrit.wikimedia.org/r/747580

Change 747598 merged by jenkins-bot:

[mediawiki/core@master] SECURITY: Require 'read' right for most actions

https://gerrit.wikimedia.org/r/747598

Mentioned in SAL (#wikimedia-operations) [2021-12-16T21:29:12Z] <sbassett> Reverted previous mitigation for T297416

In T297416#7573639, @Majavah wrote:

So I think we can restore $wgWhitelistRead to its original value (removing the override from PrivateSettings.php) now that other actions can't be used for exploits.

Should be a trivial deploy that @Reedy or I can handle after the afternoon train today.

I was planning on doing that tomorrow but that works too. Thanks!

Reverted and synced. There was a comment to add the mitigation to operations/mediawiki-config, but I don't believe that's necessary, since there should no longer be an active vulnerability regarding $wgWhitelistRead at this time.