There are some smaller ongoing tasks that are relevant from a security perspective (T375788, T376488, T375796, T373737), I'll update the list of patches with those in the next few days as they get done; but apart from that, I think the request is ready. I'll make some diagrams about the old and new authentication flow soon. Please let me know what other information will be useful.

@Tgr - For the more config-related patches, are there updates to now use auth. instead of sso. as the shared auth sub-domain? I understand this is a minor issue, but was curious if any of these config changes exist yet. Thanks.

In T378722#10349024, @sbassett wrote:

@Tgr - For the more config-related patches, are there updates to now use auth. instead of sso. as the shared auth sub-domain?

Yeah, the changes are in T379811: Update URL structure for SUL3 shared domain.

In an effort to get this finished within the next week or two, I'm going to focus on the 6-ish affected CentralAuth files mentioned above along with the provided list of "important patches". There are 122+ #sul3-migration-tagged change sets, spanning about 6 months of engineering work, many of which seem to be SRE/ops config changes that we don't have much insight into from the application layers.

Security Review Summary - T378722 - 2024-12-20

Last Commits Reviewed

• core: 92680dc752
• CentralAuth: c132101ee9
• CheckUser: dbaa1678a0
• OATHAuth: a5fb1ee448
• OAuth: 594e7a67aa
• WebAuthn: 7c438011a7

Summary

Overall, the SUL3 project gets us to a far better place in supporting current browser privacy safeguards and user-tracking restrictions (as noted within T345249 and related tasks). Many steps were taken during the initial development phases of the SUL3 project and a threat-modeling session to ensure that certain security concerns were mitigated. Additionally, several static analysis tools were run against code within relevant change sets and a manual review of important files under the CentralAuth extension was performed. Finally, manual pentesting in addition to dynamic-scanning was performed against relevant portions of the beta cluster where SUL3 is currently enabled. The SUL3 work (as scoped within the task description and below) currently warrants an overall risk rating of: low.

Scope

As noted within the review request, there are 6 or so files within ext:CentralAuth where most of the new SUL3-related code now exists. And several important change sets that will be the focus of this review.

Important changed files

1. CentralAuthRedirectingPrimaryAuthenticationProvider.php
2. RedirectingLoginHookHandler.php
3. SharedDomainHookHandler.php (formerly SsoHookHandler.php)
4. FilteredRequestTracker.php
5. CentralAuthSharedDomainPreAuthenticationProvider.php (formerly CentralAuthSsoPreAuthenticationProvider.php)
6. SharedDomainUtils.php

Important change sets

Initial Findings

Several steps were taken to address theoretical or actual security issues as the SUL3 project progressed:

1. As noted within the above change set table, phan and the security check plugin ran against relevant PHP codebases (primarily CentralAuth) and passed without the detection of any static analysis findings.
2. As noted in the request, a STRIDE-based threat model (internal) was performed by the Security-Team and relevant MediaWiki engineering team members. A brief summary of these threats and their mitigations are provided in the table below:

Additional Static Analysis Performed

(Note: static analysis was performed on relevant files from the SUL3 work when possible, as opposed to entire repositories.)

Semgrep (run with various CI, PHP, secret-finding and general rulesets/policies)

Bearer SAST (default policies)

Horusec SAST (default policies)

Dynamic Scanning And Manual Pentesting Against Beta Cluster

ZAP (default attack policy)

Wapiti (policies: backup, buster, cookieflags, crlf, csp, csrf, http_headers, methods, redirect, shellshock, ssl, ssrf, takeover)

Manual pentesting

I also performed a small amount of manual pentesting against the beta cluster, just to ensure that my basic understanding of the security concepts addressed within SharedDomainHookHandler.php, FilteredRequestTracker.php and SharedDomainUtils.php were performing as expected. I confirmed basic concepts such as only the small, curated list of special pages are available on the shared domain (T373738#10265552), that userJS does not run under the shared domain, that various extensions and other features appear disabled under the shared domain (T373737#10352026), etc. I did notice some different behavior between various wiki directories under auth.wikimedia.beta (e.g. /officewiki/ [404] vs /foundationwiki/ [503] vs /loginwiki/ [200, perm error] vs /commonswiki/ [503] vs /enwiktionary/ [200, perm error], etc.) But I didn't find a clever way to really exploit that as I imagine these are expected behaviors. I also noticed that I'm able to move easily between Special:Userlogin on enwiki.beta, auth.w.beta/enwiki, et al where I can enable/disable my OATH TOTP (if I set it under enwiki.beta, it checks for it during reauth under auth.w.beta/enwiki during a new auth). There are many additional rabbit holes one could likely go down during a pentest like this, but in my opinion, the critical security concerns surfaced and addressed during the threat-model seem well-implemented and warrant a rating of low risk.