Page MenuHomePhabricator

Disable irrelevant extensions on SUL3 login domain
Closed, DeclinedPublic

Description

T363695: Create a Wikimedia login domain that can be served by any wiki will use a shared domain for login, and restrict functionality on that domain to what's necessary for authentication workflows. Per the discussion with Security that happened as part of T367995: Security Preview for shared login domain, we should see if we can just disable unnecessary (non-login-related) extensions in that situation. This is both somewhat fragile (e.g. because of caches shared with the normal domain of the wiki) and (given the messy, manual way extensions are managed in Wikimedia configuration) definitely doesn't generalize to all extensions, but if we can just disable extensions which are particularly complex and/or ill-maintained, that would already be a win.

Related Objects

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

The status quo is that loginwiki differs from the average wiki in the following ways:

  • uploads disabled
  • restricted user rights config
  • CategoryTree, SecurePoll, CentralNotice, Score, Phonos, Nuke, LabeledSectionTransclusion, Gadgets, GlobalCssJs, PdfHandler, MassMessage, TimedMediaHandler, BetaFeatures, CommonsMetadata, MultimediaViewer, Linter, Parsoid, VisualEditor, Flow, Math, Babel, ApiFeatureUsage, FeaturedFeeds, GuidedTour, EducationProgram, Disambiguator, Scribunto, DiscussionTools, FileExporter, Collection, Score + ScoreShellbox, RealMe, UniversalLanguageSelector, TemplateSandbox, Cite, CiteThisPage, Poem, InputBox, ImageMap, SyntaxHighlight, Timeline, WikiHiero, PagedTiffHandler, ParserFunctions, TemplateData, CharInsert, PageImages, NearbyPages disabled
  • wgAllowUserJs, wgAllowUserCss, wgUseSiteJs, wgUseSiteCss set to false
  • CheckUserLogLogins disabled
  • wmgEnableIPMasking enabled
  • wgPageImagesNamespaces set to false (seems pointless since PageImages is disabled anyway)
  • added to wgSecurePollExcludedWikis (for T303135)
  • wgEnableEventBus is TYPE_JOB|TYPE_PURGE (rather than TYPE_ALL which also includes TYPE_EVENT)
  • on Beta: ReportIncident, IPInfo, CampaignEvents, NetworkSession, Chart disabled

Of the extensions, the most important are CentralNotice (XSS vector), Gadgets and GlobalCssJs (XSS vector, already disabled per T373738 but could use defense in depth), BetaFeature (because it is unpredictable what it is going to be used for), GuidedTour (prone to cause all kinds of errors if it tries to continue a tour while on the login domain). We probably can't disable Parsoid and ParserFunctions (needed for parsing i18n messages), maybe CommonsMetadata/MultimediaViewer/TimedMediaHandler (used for signup page campaigns in the past). The others would be nice to disable but don't seem particularly important - they reduce the amount of code that can run in the context of the shared domain, but they aren't especially risky, and mostly aren't reachable on the shared domain anyway. On loginwiki they are disabled using the loginwiki DB name or the lockeddown DB group name; there is no easy way to transfer that to the shared domain. We can hack it into MWMultiVersion but that will break eventually when configuration loading becomes more static.

Of the other settings, user rights restrictions and [User/Site][Js/Css] settings are good defense in depth. (We should also check how CentralAuth global permissions interact with user right restrictions.) Not sure what wgEnableEventBus is about. The others seem inapplicable or unimportant.

Change #1091922 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[operations/mediawiki-config@master] [WIP] Add 'lockeddown' wiki tag when using the shared login domain

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

Having a wikitag that's not a tag of a wiki, seems likely confusing in the long-term. It hides complexity in a familiar shape, but without fully representing/abstracting it for those not familiar with it.

Note that the way we process wiki tags, assumes that there isn't ambiguity in the array, e.g. if the same wiki is in tags that are set for a given value, with different values, this produces an error. wiki id -> tag -> default.

// valid, deterministic
'a' => [
  'default' => true,

  'open'   => true, # includes enwiki
  'closed' => false,

  'enwiki' => true,
],

'b' => [
  'default' => true,

  'open' => true, # includes enwiki
  'echo' => true, # includes enwiki

  'enwiki' => false,
],

// invalid, ambigious
'c' => [
  'default' => false,

  'open' => false, # includes enwiki
  'echo' => true, # includes enwiki
  'closed' => true,
]

It's not obvious where lockeddown fits in, or how that ambiguity would be enforced/encoded in an array. I assume we'd want it to "win", but that's a very temporal decision based on where we are now. The "next" thing to come along will also want to win. Right now we have no ambiguity with the three levels being self-explanatory, with any confusing values detected and rejected by CI. This seems valuable to preserve, so that there aren't landmines for future editors to step on.

Most of today's loginwiki restrictions aren't requirements. We maximised the difference between normal wikis and loginwikis, but it was easy to do so at the wiki-wide level. With the shared SUL3 domain, I think the opposite is true, where, for caching and interoperability reasons, we'd want as little difference as possible, in order to minimise potential corruption and confusion where things work in one place but not the other.

I don't know exactly which restrictions we want on the shared domain, but wmf-config settings generally fit into these two categories:

  1. MW config values.
  2. intermediate wmg values to toggle extensions.

MW config values can be overridden en-mass from a conditional block as we do already in CommonSettings in the @$_SERVER['SERVER_NAME'] === $wmgHostnames['sso'] block.

Extensions could be toggled the same way if that conditional block is early enough, but this seems counter-intuitive since there's a reasonable expectation that when something says if ( $wmgUseFoo ) and IS.php sets wmgUseFoo => true that this can't then be false.

Perhaps a more discoverable mechanism for extension toggling, less dependant on knowing the arbitrary order of things in CS.php, would be for these conditional blocks to include if ( $wmgUseFoo && !$wmgIsSsoDomain ) at the last mile. That encodes the fact that these are context dependent, and in a way that is discoverable along with how the setting is consumed. What do you think?

Change #1092334 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[operations/mediawiki-config@master] Disable various extensions when using the shared login domain

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

I tried both ways (wiki tag vs. a big block of CommonSettings overrides) to see how they look:
https://gerrit.wikimedia.org/r/c/operations/mediawiki-config/+/1091922
https://gerrit.wikimedia.org/r/c/operations/mediawiki-config/+/1092334
Not sure how I feel about them in terms of DX. The override makes much more sense as a patch, and makes it easier to review how the auth domain differs from normal operation when you are specifically looking for that; the wiki tag makes it much easier to become aware of differences when looking at how a certain extension / feature is configured with specifically looking for the auth domain. I guess looking at auth differences specifically would be the more common use case? I think the only situation where someone might be interested in differences to the auth domain, without knowing they are interested in the auth domain, would be if toggling an extension or config flag causes problems (e.g. due to writing different values to cache) and someone is investigating that without being aware of the toggling happening at all. Hopefully that situation will be very rare.

Good point about ambiguity, that seems potentially very problematic. Though as far as I see this is already a problem today; but maybe doesn't happen much in practice, because there is not much reason to use a dblist + a project family, or a dblist + a language, at the same time. A magic wiki tag whose purpose is specifically to override things would definitely make things worse.

Perhaps a more discoverable mechanism for extension toggling, less dependant on knowing the arbitrary order of things in CS.php, would be for these conditional blocks to include if ( $wmgUseFoo && !$wmgIsSsoDomain ) at the last mile. That encodes the fact that these are context dependent, and in a way that is discoverable along with how the setting is consumed. What do you think?

Like the wiki tags, it's more discoverable but makes it hard to review auth domain changes as a whole. I don't have a strong feeling about it but the latter seems more important. Maybe we could add comments or something.

Change #1091922 abandoned by Gergő Tisza:

[operations/mediawiki-config@master] Add 'auth' wiki tag when using the shared login domain

Reason:

per T373737#10331184

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

Change #1092334 merged by jenkins-bot:

[operations/mediawiki-config@master] Disable various extensions when using the shared login domain

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

Mentioned in SAL (#wikimedia-operations) [2024-11-21T22:06:47Z] <brennen@deploy2002> Started scap sync-world: Backport for [[gerrit:1092334|Disable various extensions when using the shared login domain (T373737)]]

Mentioned in SAL (#wikimedia-operations) [2024-11-21T22:10:23Z] <brennen@deploy2002> tgr, brennen: Backport for [[gerrit:1092334|Disable various extensions when using the shared login domain (T373737)]] synced to the testservers (https://wikitech.wikimedia.org/wiki/Mwdebug)

Mentioned in SAL (#wikimedia-operations) [2024-11-21T22:25:03Z] <brennen@deploy2002> Finished scap sync-world: Backport for [[gerrit:1092334|Disable various extensions when using the shared login domain (T373737)]] (duration: 18m 16s)

This breaks hard:
https://auth.wikimedia.beta.wmflabs.org/enwiki/wiki/Special:Userlogin

Uncaught MediaWiki\Registration\ExtensionDependencyError: ContentTranslation requires Cite to be installed.
ContentTranslation requires UniversalLanguageSelector to be installed.
ContentTranslation requires VisualEditor to be installed.
ExternalGuidance requires UniversalLanguageSelector to be installed.
GrowthExperiments requires VisualEditor to be installed.
AutoModerator requires DiscussionTools to be installed.
 in /srv/mediawiki/php-master/includes/registration/ExtensionRegistry.php:480
#0 /srv/mediawiki/php-master/includes/registration/ExtensionRegistry.php(332): MediaWiki\Registration\ExtensionRegistry->readFromQueue(Array)
#1 /srv/mediawiki/php-master/includes/Setup.php(293): MediaWiki\Registration\ExtensionRegistry->loadFromQueue()
#2 /srv/mediawiki/php-master/includes/WebStart.php(85): require_once('/srv/mediawiki/...')
#3 /srv/mediawiki/php-master/api.php(37): require('/srv/mediawiki/...')
#4 /srv/mediawiki/w/api.php(3): require('/srv/mediawiki/...')
#5 {main}
  thrown

Change #1094071 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[operations/mediawiki-config@master] Disable more extensions when using the shared login domain

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

We can't have authentication break every time someone deploys a new extension, so we'll either need to re-think this approach or automate testing (e.g. add something on the auth domain to the canary checks).

Change #1094071 merged by jenkins-bot:

[operations/mediawiki-config@master] Disable more extensions when using the shared login domain

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

Mentioned in SAL (#wikimedia-operations) [2024-11-25T08:08:39Z] <tgr@deploy2002> Started scap sync-world: Backport for [[gerrit:1094071|Disable more extensions when using the shared login domain (T373737)]]

Mentioned in SAL (#wikimedia-operations) [2024-11-25T08:25:02Z] <tgr@deploy2002> tgr: Backport for [[gerrit:1094071|Disable more extensions when using the shared login domain (T373737)]] synced to the testservers (https://wikitech.wikimedia.org/wiki/Mwdebug)

Mentioned in SAL (#wikimedia-operations) [2024-11-25T08:39:15Z] <tgr@deploy2002> Finished scap sync-world: Backport for [[gerrit:1094071|Disable more extensions when using the shared login domain (T373737)]] (duration: 30m 35s)

$ curl -s 'https://auth.wikimedia.beta.wmflabs.org/enwiki/w/api.php?action=query&meta=siteinfo&siprop=extensions&format=json' | jq --raw-output '.query.extensions[].name' | sort
3D
Abuse Filter
AdvancedSearch
AntiSpoof
ApiFeatureUsage
ArticleCreationWorkflow
ArticlePlaceholder
BounceHandler
CampaignEvents
Campaigns
Capiunto
CentralAuth
CharInsert
Chart
ChessBrowser
CirrusSearch
CLDR
CodeEditor
CodeMirror
Collection
Cologne Blue
CommunityConfiguration
ConfirmEdit
ContactPage
Dashiki
DismissableSiteNotice
Echo
Elastica
ElectronPdfService
EntitySchema
EventBus
EventLogging
EventStreamConfig
FancyCaptcha
FlaggedRevs
GeoData
GlobalBlocking
GlobalCssJs
GlobalPreferences
Global Usage
GlobalUserPage
Graph
Interwiki
InterwikiSorting
IPInfo
JsonConfig
Kartographer
LoginNotify
MediaModeration
MinervaNeue
MobileApp
MobileFrontend
Modern
MonoBook
NavigationTiming
NetworkSession
Newsletter
OATHAuth
OAuth
ORES
PageAssessments
PageTriage
PageViewInfo
ParserFunctions
ParserMigration
Parsoid
Popups
QuickSurveys
ReadingLists
RelatedArticles
ReportIncident
RevisionSlider
SandboxLink
Scribunto
SearchVue
SecureLinkFixer
SecurePoll
SiteMatrix
SpamBlacklist
StopForumSpam
TemplateStyles
TextExtracts
TheWikipediaLibrary
Timeless
TitleBlacklist
TorBlock
TrustedXFF
TwoColConflict
Vector
VueTest
WebAuthn
WikibaseClient
WikibaseLexeme
WikiEditor
WikiLove
WikimediaBadges
WikimediaCampaignEvents
WikimediaEvents
WikimediaMessages
Wikistories
XAnalytics

So this worked, except Parsoid, which is still listed - I suppose the wmg setting is just outdated there, now that it's vendorized. I'll remove the config override.

The status quo is that loginwiki differs from the average wiki in the following ways:

  • uploads disabled

Not bothering with this - the API and special page are unavailable, and it's not particularly security-relevant.

This would probably break things. We need a bunch of userrights for login / signup (IP block exemptions, anti-abuse feature overrides etc). Even more problematically, user rights and the consequences of user rights are likely to get cached.

  • CategoryTree, SecurePoll, CentralNotice, Score, Phonos, Nuke, LabeledSectionTransclusion, Gadgets, GlobalCssJs, PdfHandler, MassMessage, TimedMediaHandler, BetaFeatures, CommonsMetadata, MultimediaViewer, Linter, Parsoid, VisualEditor, Flow, Math, Babel, ApiFeatureUsage, FeaturedFeeds, GuidedTour, EducationProgram, Disambiguator, Scribunto, DiscussionTools, FileExporter, Collection, Score + ScoreShellbox, RealMe, UniversalLanguageSelector, TemplateSandbox, Cite, CiteThisPage, Poem, InputBox, ImageMap, SyntaxHighlight, Timeline, WikiHiero, PagedTiffHandler, ParserFunctions, TemplateData, CharInsert, PageImages, NearbyPages disabled

It seems I forgot a number of things from this list: ApiFeatureUsage, CharInsert, Collection, GlobalCssJs, ParserFunctions, Scribunto, SecurePoll.
ApiFeatureUsage actually seems useful as we still use the API on the auth domain; ParserFunctions and Scribunto might conceivably be involved in parsing messages shown on authentication pages. I'll add the rest.

(Although TBH not sure how much point there is to most of these changes. The list of disabled extensions seems very random. OTOH I don't see anything in the list of remaining extensions that I'd particularly want to disable, either.)

  • wgAllowUserJs, wgAllowUserCss, wgUseSiteJs, wgUseSiteCss set to false

This is done (if mostly pointless, because they mainly affect ResourceLoader and we use load.php from the normal domain).

  • CheckUserLogLogins disabled

No reason to want this now that every authentication is associated with the initiating wiki on the DB level.

  • wmgEnableIPMasking enabled

Again, no reason to want this.

  • wgPageImagesNamespaces set to false (seems pointless since PageImages is disabled anyway)

Still seems pointless so didn't add.

  • added to wgSecurePollExcludedWikis (for T303135)

No equivalent of this for the login domain. If that's causing problems, we can just not disable SecurePoll.

  • wgEnableEventBus is TYPE_JOB|TYPE_PURGE (rather than TYPE_ALL which also includes TYPE_EVENT)

Not really sure why this was done on loginwiki, seems like a bad idea.

  • on Beta: ReportIncident, IPInfo, CampaignEvents, NetworkSession, Chart disabled

Except for ReportIncident, these are already in production.
There is some small amount of value in disabling Chart (could possibly be included in some authentication i18n message by an attacker, and has maybe slightly more XSS risk than a plain message), so adding that. The rest seems too irrelevant to bother with.

We can't have authentication break every time someone deploys a new extension, so we'll either need to re-think this approach or automate testing (e.g. add something on the auth domain to the canary checks).

T380574: Add SUL3 authentication domain to deploy canary checks

Change #1097327 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[operations/mediawiki-config@master] SUL3: Sort overrides

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

Change #1097328 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[operations/mediawiki-config@master] More authentication domain overrides

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

Change #1097327 merged by jenkins-bot:

[operations/mediawiki-config@master] SUL3: Sort overrides

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

Change #1097328 merged by jenkins-bot:

[operations/mediawiki-config@master] More authentication domain overrides

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

Mentioned in SAL (#wikimedia-operations) [2024-11-25T22:21:09Z] <tgr@deploy2002> Started scap sync-world: Backport for [[gerrit:1097327|SUL3: Sort overrides (T373737)]], [[gerrit:1097328|More authentication domain overrides (T373737)]], [[gerrit:1097322|Update private/readme.php to match production]]

Mentioned in SAL (#wikimedia-operations) [2024-11-25T22:25:36Z] <tgr@deploy2002> tgr: Backport for [[gerrit:1097327|SUL3: Sort overrides (T373737)]], [[gerrit:1097328|More authentication domain overrides (T373737)]], [[gerrit:1097322|Update private/readme.php to match production]] synced to the testservers (https://wikitech.wikimedia.org/wiki/Mwdebug)

Mentioned in SAL (#wikimedia-operations) [2024-11-25T22:33:59Z] <tgr@deploy2002> Finished scap sync-world: Backport for [[gerrit:1097327|SUL3: Sort overrides (T373737)]], [[gerrit:1097328|More authentication domain overrides (T373737)]], [[gerrit:1097322|Update private/readme.php to match production]] (duration: 12m 49s)

Change #1111343 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[operations/mediawiki-config@master] Yet more authentication domain overrides

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

Change #1111343 merged by jenkins-bot:

[operations/mediawiki-config@master] Yet more authentication domain overrides

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

Mentioned in SAL (#wikimedia-operations) [2025-01-15T14:27:18Z] <lucaswerkmeister-wmde@deploy2002> Started scap sync-world: Backport for [[gerrit:1111343|Yet more authentication domain overrides (T383729 T373737)]], [[gerrit:1111598|Move Beta Cluster favicons to this repository]], [[gerrit:1075211|Replace favicon.php with static.php (T374997)]]

Mentioned in SAL (#wikimedia-operations) [2025-01-15T14:34:58Z] <lucaswerkmeister-wmde@deploy2002> tgr, matmarex, lucaswerkmeister-wmde: Backport for [[gerrit:1111343|Yet more authentication domain overrides (T383729 T373737)]], [[gerrit:1111598|Move Beta Cluster favicons to this repository]], [[gerrit:1075211|Replace favicon.php with static.php (T374997)]] synced to the testservers (https://wikitech.wikimedia.org/wiki/Mwdebug)

Mentioned in SAL (#wikimedia-operations) [2025-01-15T14:43:10Z] <lucaswerkmeister-wmde@deploy2002> Finished scap sync-world: Backport for [[gerrit:1111343|Yet more authentication domain overrides (T383729 T373737)]], [[gerrit:1111598|Move Beta Cluster favicons to this repository]], [[gerrit:1075211|Replace favicon.php with static.php (T374997)]] (duration: 15m 52s)

I increasingly have doubts about the sustainability of this approach. It means two requests can largely share configuration (notably, use the same DB and the same cache prefix) but have different extensions enabled; it is hard to know whether that will cause the pollution of some cache, and when it happens, it will be hard to understand and debug what's going on. (We have the first such bug now: T384919: SUL3 prevents GrowthExperiments from accessing CommunityConfiguration on testwiki) It also requires constant monitoring of what hook handlers don't run because the extension isn't enabled when the hook gets called (again, there's a recent example: T384236: [regression] Accounts created at test wikis are not receiving Growth features) - this issue is easier to debug but it seems unrealistic to expect everyone adding a new hook handler would be aware of this issue in the future.

Limiting the set of active extensions is not security crictical. Leaving all extensions enabled would not be very different from SUL2 where no extensions were disabled during login - they were restricted during requests served from the central domain, ie. the loginwiki side of central login and autologin; but once you can exploit a vulnerability on a local domain, there are all kinds of easy ways to extend it to other domains, like using the centralauthtoken API, so I think we wouldn't be losing much by giving it up; having to forever worry about cache pollution and hook unreliability doesn't seem worth it.

Maybe we could disable a smaller set of extensions where there is a specific security concern, after carefully vetting their cache and hook usage, and making sure the maintainers are aware and the constraints are documented. Per T373737#10265544 I think those would be CentralNotice, Gadgets, GlobalCssJs and GuidedTour. (Especially CentralNotice; Gadgets and GlobalCssJs are defense in depth but rECAU52d93d7eceb0: SUL3: Disallow user JS on the shared domain should already prevent them from doing anything, and GuidedTour is more of a UX concern than a security one. But displaying notices with Javascript code in them during login seems like a real risk.)

Maybe we could disable a smaller set of extensions where there is a specific security concern, after carefully vetting their cache and hook usage, and making sure the maintainers are aware and the constraints are documented. Per T373737#10265544 I think those would be CentralNotice, Gadgets, GlobalCssJs and GuidedTour. (Especially CentralNotice; Gadgets and GlobalCssJs are defense in depth but rECAU52d93d7eceb0: SUL3: Disallow user JS on the shared domain should already prevent them from doing anything, and GuidedTour is more of a UX concern than a security one. But displaying notices with Javascript code in them during login seems like a real risk.)

This sounds like a reasonable compromise that still buys us some extra protection. I guess it's difficult to understand what, if anything, this might break in the future though. We had rated this as a high risk within the SUL3 threat model, but it was mashed together with other, related concerns. I don't think that this compromise should change the initial and residual risk ratings very much unless someone has some very specific, immediate concerns or demonstrable attack vectors related to these proposed changes.

I think those would be CentralNotice, Gadgets, GlobalCssJs and GuidedTour

A review of those four extensions:

  • CentralNotice:
    • Hooks: CanonicalNamespaces, ChangeTagsListActive, ListDefinedTags, LoadExtensionSchemaUpdates, SkinTemplateNavigation::Universal, ResourceLoaderRegisterModules, UserMergeAccountFields, GetPreferences, PreferencesGetIcon, MakeGlobalVariablesScript, BeforePageDisplay, SiteNoticeAfter, ResourceLoaderGetConfigVars. Also does global changes in a registration callback, equivalent to having a UserGetDefaultOptions handler. Other than the RL and change tags hooks, these seem fine.
    • Further hooks on metawiki: TranslatePostInitGroups, TranslateEventMessageGroupStateChange. Also adds a user right and a restriction level.
    • It defines an additional namespace. It's probably not possible to encounter that namespace during an authentication process, but it still seems scary to disable that.
    • IIUC there is no way to disable showing banners (and specifically, forcing banners via the banner= URL parameter) when the extension is enabled. They do not show on Special: namespace pages though, which are the only kind of page accessible on the shared domain.
  • Gadgets:
    • Hooks: PageDeleteComplete, PageSaveComplete, BeforePageDisplay, CodeEditorGetPageLanguage, ContentHandlerDefaultModelFor, UserGetDefaultOptions, GetPreferences, PreferencesGetLegend, ResourceLoaderRegisterModules, wgQueryPages, DeleteUnknownPreferences, PreferencesGetIcon, getUserPermissionsErrors. Other than the RL and UserGetDefaultOptions hooks, the others seem fine. The ContentHandler hook cannot be invoked because gadget pages are not transcludable. The GetUserPermissionsErrors hook only affects page editing, and permission errors cannot be cached.
    • All of what it does will be suppressed by disallowUserJs, so disabling it is just defense in depth.
  • GlobalCssJs:
    • Hooks: BeforePageDisplay, ResourceLoaderRegisterModules, EditPage::showEditForm:initial, GetPreferences.
    • All of what it does will be suppressed by disallowUserJs, so disabling it is just defense in depth.
  • GuidedTour:
    • Hooks: BeforePageDisplay, ResourceLoaderRegisterModules, RedirectSpecialArticleRedirectParams, MakeGlobalVariablesScript. Other than adding RL modules, everything else is trivial and and only affects the content of the current page.
    • Most of what it does will be suppressed by disallowUserJs and the rest is irrelevant, so disabling it is just defense in depth. Except, there is an ordering issue - CentralAuth disallows user JS in BeforePageDisplay, and GuidedTour checks if it's disallowed in the same hook. So that will have to be fixed somehow.

The result of GetPreferences is not cached but it seems like the kind of thing that could conceivably get cached in the future. But it's only invoked via Special:Preferences and the options API, and neither of those are enabled on the shared domain, so it does not seem risky.

The result of UserGetDefaultOptions can get cached (e.g. during a UserOptionsManager::loadUserOptions() call) so there's probably a risk of cache pollution here (probably resulting in default-on gadgets not loading reliably).

The result of ListDefinedTags and ChangeTagsListActive are cached in ChangeTagsStore::listSoftwareDefinedTags() and ChangeTagsStore::listSoftwareActivatedTags() respectively, so disabling an extension with such hook handlers will poison the cache.

As far as I can see, ResourceLoaderRegisterModules output does not get cached. (We also don't allow load.php requests on the shared domain, but I'm sure the module registry gets accessed by OutputPage at some point.) But it also seems like something that might get cached in the future.

ExtensionRegistry uses a main cache where the cache key does not depend on extension names. I think in practice it won't get poisoned because it gets warmed by a maintenance script; but depending on that is not a good place to be. Even from a strict security POV, I think the risk that due to cache poisoning some part of extension configuration might randomly go missing in a normal request which is not limited in the way shared domain requests are, is far larger than the risk of loading all extensions.


In conclusion, I think we should discard the idea of disabling extensions entirely. For Gadgets and GlobalCssJs, we can just rely on disallowUserJs(). For GuidedTours, the same, except we need to fix the ordering issue. For CentralNotice, we can rely on it not being loaded on special pages, but we should also make it respect disallowUserJs(), as defense in depth.

Change #1115369 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[operations/mediawiki-config@master] Do not disable extensions on SUL3 shared authentication domain

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

Change #1115450 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/extensions/CentralAuth@master] Call disallowUserJs() for the shared domain from ExtensionFunctions

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

Change #1115450 merged by jenkins-bot:

[mediawiki/extensions/CentralAuth@master] Call disallowUserJs() for the shared domain from ExtensionFunctions

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

Pppery changed the task status from Resolved to Declined.Jan 30 2025, 6:49 PM

Change #1115369 merged by jenkins-bot:

[operations/mediawiki-config@master] Do not disable extensions on SUL3 shared authentication domain

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

Mentioned in SAL (#wikimedia-operations) [2025-01-30T21:10:42Z] <tgr@deploy2002> Started scap sync-world: Backport for [[gerrit:1115369|Do not disable extensions on SUL3 shared authentication domain (T373737 T384919 T384236)]]

Mentioned in SAL (#wikimedia-operations) [2025-01-30T21:13:29Z] <tgr@deploy2002> tgr: Backport for [[gerrit:1115369|Do not disable extensions on SUL3 shared authentication domain (T373737 T384919 T384236)]] synced to the testservers (https://wikitech.wikimedia.org/wiki/Mwdebug)

Mentioned in SAL (#wikimedia-operations) [2025-01-30T21:23:24Z] <tgr@deploy2002> Finished scap sync-world: Backport for [[gerrit:1115369|Do not disable extensions on SUL3 shared authentication domain (T373737 T384919 T384236)]] (duration: 12m 42s)

Change #1115508 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/extensions/CentralNotice@master] Do not show banners when custom site scripts are disallowed

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

In conclusion, I think we should discard the idea of disabling extensions entirely. For Gadgets and GlobalCssJs, we can just rely on disallowUserJs(). For GuidedTours, the same, except we need to fix the ordering issue. For CentralNotice, we can rely on it not being loaded on special pages, but we should also make it respect disallowUserJs(), as defense in depth.

Ok. Do you see any other potential, future issues regarding this decision or should the disallowUserJs() defense-in-depth likely guard against any potential concerns?

Do you see any other potential, future issues regarding this decision or should the disallowUserJs() defense-in-depth likely guard against any potential concerns?

Disabling the extensions would have helped any (built-in) XSS or RCE vulnerabilities from reaching the central domain, and there isn't any way to achieve that without disabling. I just don't think it makes a huge difference because an XSS or RCE vulnerability is almost as bad on an arbitrary other domain - once you take over an account on one wiki, there are all kinds of ways to escalate that, that are much easier than the takeover itself. And it's also not much different from the old status quo where most extensions weren't enabled on loginwiki but were enabled on the login form. So I think we lost the ability for a nice extra layer of defense, which is unfortunate, but it's not something we depended on for security.

In the long term, I still hope we can make authentication more restricted, and eventually not use MediaWiki entirely. SUL3 makes MediaWiki use a remote identity provider for authentication (even if, in a weird hack, that remote identity provider is itself; but other than UX equivalence with the old authentication flow, nothing really depends on that being true) so I think it's a step in the right direction, even if the shared domain approach means we don't see any immediate security benefits from that. I hope we can follow up on that in the future - we wanted to avoid making major backend / control flow changes and major UX changes at the same time, but once the dust settles, we could simplify the login/signup UI so that there is no difference between wikis anymore, and then we could either go back to a loginwiki-like approach with a very restricted dedicated wiki or (preferably) use a single-purpose non-MediaWiki app. That's not in the time budgeted for the SUL3 project though, so it would depend on how the WMF prioritizes enhancing login security.

Ok. I know we've looked into further separating concerns with work from Authentication-Experiments-2022 and other projects. For simple auth-related processes, it likely wouldn't be that terrible. The problem is that some FOSS options that we explored (e.g. Keycloak) kind of want to be the anything-and-everything when it comes to both authn and authz, which we obviously do not want given how heavily-integrated the current authz model is within MediaWiki core.

In the long term, I still hope we can make authentication more restricted, and eventually not use MediaWiki entirely.

which is T120484: Store CentralAuth password hashes outside the main database cluster

Change #1115508 merged by jenkins-bot:

[mediawiki/extensions/CentralNotice@master] Do not show banners when custom site scripts are disallowed

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

Can we change the context of all requests to auth.wikimedia.org to be loginwiki only? I know it could break some stuff (e.g. CU entries will be central instead) but that should take care of a lot of complexities and reduces a lot of risk.

Can we change the context of all requests to auth.wikimedia.org to be loginwiki only? I know it could break some stuff (e.g. CU entries will be central instead) but that should take care of a lot of complexities and reduces a lot of risk.

Why we use such an auth domain instead of just redirect every login/register request to loginwiki is auth domain can handle local IP blocks, and auth domain is automatically localized. See T363411 and T363695.

Lots of long tail UX issues are avoided by having the same wiki render the login page. As @Bugreporter says, see T363695: Create a Wikimedia login domain that can be served by any wiki, and the decision record linked there. You are right that this takes a big tradeoff on UX vs. security (in terms of missed opportunities for improvements).

I still think the path to eventually reversing that (the SUL3 authentication logic itself doesn't really care if it's a real wiki or a shared domain, so not too hard to switch) is through making the login/signup page look radically simpler, so at least the UX and i18n differences between wikis become irrelevant.

Makes sense. Let me know if I can help on anything. Maybe we can do some of it in the hackathon.