This does not affect all autocreations at least:

mysql:research@dbstore1007.eqiad.wmnet [testwiki]> select * from cu_changes where cuc_user=(select user_id from user where user_name="MU test 202106180125")\G
*************************** 1. row ***************************
        cuc_id: 577721
 cuc_namespace: 2
     cuc_title:
      cuc_user: 50519
 cuc_user_text: MU test 202106180125
cuc_actiontext: was automatically created
   cuc_comment:
     cuc_minor: 0
   cuc_page_id: 0
cuc_this_oldid: 0
cuc_last_oldid: 0
      cuc_type: 3
 cuc_timestamp: 20210617232647
        cuc_ip: [redacted, my own IPv4]
    cuc_ip_hex: [redacted]
       cuc_xff:
   cuc_xff_hex: NULL
     cuc_agent: [redacted]
1 row in set (0.001 sec)

mysql:research@dbstore1007.eqiad.wmnet [testwiki]>

Probably relevant to T251509 (or maybe a dupe of that task).

account autocreation happens (at least in some cases)

I'd guess those cases are when a global account is created and CA automatically triggers autocreation on all wikis listed on wgCentralAuthAutoCreateWikis, as opposed to someone visiting a wiki for the first time. The account creation is done as a job, although CA tries to fix the session data:

if ( isset( $this->params['session'] ) ) {
	// restore IP and other request data
	$this->params['session']['userId'] = 0;
	$this->params['session']['sessionId'] = '';
	$callback = RequestContext::importScopedSession( $this->params['session'] );
	$this->addTeardownCallback( static function () use ( &$callback ) {
		ScopedCallback::consume( $callback );
	} );
}

@Joe, @Urbanecm and myself looked more into this today on IRC. Turns out that my wgCentralAuthAutoCreateWikis theory was wrong as these strange CU entries have been seen at least on enwiki too. Martin found a sample API request that caused one of these auto creations, which was an internal cross-wiki Echo notification check. We didn't manage to reproduce this (it seems like there's another Echo bug causing notifications for users without a local account to exist) but we found Echo's EchoForeignWikiRequest class, which is likely the cause here; it sends internal x-wiki API queries and for some cases the user does not seem to exist yet so AuthManager auto creates it.

The following patch attaches the original IP address and UA to requests coming from Echo, like how other headers would using MWHttpRequest::setOriginalRequest(). I verified from my local logs that this causes the headers to be present with Echo requests, but again I haven't been able to reproduce this very issue on CU so can't confirm it's fixed.

I finally managed to reproduce this on my own. Sending an API request to https://en.wikipedia.org/w/api.php?action=query&format=json&meta=notifications&notwikis=enwiki%7Ctestwiki causes an account to be created on testwiki too.

This discovery comes mostly from looking at the Commons Android app's source code, which hardcodes wikidatawiki|commonswiki|enwiki as the wikis to check for. Both T251509 and my queries on Logstash today indicated that the app was somehow involved in this bug.

Updated patch set so it can be synced out in parts:

The above patch sets X-Original-User-Agent header which is what MWHttpRequest::setOriginalRequest uses, however apparently that is not used anywhere. We can either make checkuser read that header or change the echo patch to set the actual User-Agent header instead. At this point I think Echo should set the real User-Agent header just to make sure all parts of the stack think the user-agent is the same. I imagine it's ok for CU to not be aware of this very rare edge case.

X-Original-User-Agent was added in T161029: Forward request data in proxied Action API modules without much thought or consultation as a nice-to-have; the primary goal was enabling IP blocks of aggressive clients. Maybe the more correct thing would have been to just override User-Agent, not sure.

In T285116#7452344, @Tgr wrote:

X-Original-User-Agent was added in T161029: Forward request data in proxied Action API modules without much thought or consultation as a nice-to-have; the primary goal was enabling IP blocks of aggressive clients. Maybe the more correct thing would have been to just override User-Agent, not sure.

Yeah, I guess this should be fine. A nicer solution would be to make use of a standard "UA forwarding" mechanism, but it appears there's none ATM.

@Majavah, mind uploading a follow up patch? I'm happy to deploy next week.

Updated patch that uses the regular User-Agent header:

Please do not mask the user agent making the actual request; We need to know, when looking at requests, that they come from MediaWiki and Ideally which component in making them.

In T285116#7454005, @Joe wrote:

Please do not mask the user agent making the actual request; We need to know, when looking at requests, that they come from MediaWiki and Ideally which component in making them.

Note the user-agent doesn't provide any significant information (it was MediaWiki/XXX, where XXX is MW version). Without a great amount of digging (as you know from the IRC discussion the other day), it's impossible to figure out which part of MW makes the request and why, even though we had both UA and the server IPs. IMHO, this shows limited value of the current UA (at least in this very specific -- and rare -- case).

If you think we should actively collect information about which MW part made a request, perhaps let's include X-Requested-With with __METHOD__ PHP keyword, similar to what we do with DB queries?

Can we combine the two user-agents? E.g. MediaWiki/1.38 Echo (via: $originalUA). That should take care of the CU case, as they can see the original UA, and the sysadmin case, since the request is prefixed with "MediaWiki".

It would be nice to centralize the logic for original UA handling, maybe by adding a static equivalent of setOriginalRequest() to MWHttpRequest which acts on an array instead of a MWHttpRequest instance.

@Urbanecm - did the patch from T285116#7453449 ever get deployed?
I just see the sal for the first deploy mentioned at T285116#7449265 :https://sal.toolforge.org/log/BB4zpHwB1jz_IcWus5VT.

In T285116#7474829, @sbassett wrote:

@Urbanecm - did the patch from T285116#7453449 ever get deployed?
I just see the sal for the first deploy mentioned at T285116#7449265 :https://sal.toolforge.org/log/BB4zpHwB1jz_IcWus5VT.

I don't think so -- I always !log to SAL when deploying security patches. T285116#7454005 likely stopped me from hitting the buttons.

@Urbanecm do we need a new patch so that the user agent is not masked?

In T285116#7583640, @Mstyles wrote:

@Urbanecm do we need a new patch so that the user agent is not masked?

Good question. T285116#7449165 got out (per the SAL), which (as far as i know) fixes the incorrect IP address issue, which is more important for the checkusers (ie. consumers of the data affected). While that patch includes a mitigation for user agent (replacing it with the old one), that mitigation does not work, because the X-Original-User-Agent header is not recognized. A patch that fixes that broken mitigation exists (T285116#7453449), but it wasn't deployed because of @Joe's concerns at T285116#7454005.

It would be nice to resolve the user agent in one way or the other. For that, we'd first need to reach a decision about how should the agent be handled (ie. should T285116#7457263 by @Legoktm be followed? or should it be $originalUA (via MediaWiki/1.37.0-wmf.9)? something else?). I'm not sure who should be the decision maker for that question. We can also pick one route as a temporary route and decide later, however, per first Tim Starling's law, that route can well become permanent without anyone doing anything.

Hope this answers your question @Mstyles. Ask if not :).

In T285116#7583787, @Urbanecm wrote:

It would be nice to resolve the user agent in one way or the other. For that, we'd first need to reach a decision about how should the agent be handled (ie. should T285116#7457263 by @Legoktm be followed? or should it be $originalUA (via MediaWiki/1.37.0-wmf.9)? something else?). I'm not sure who should be the decision maker for that question. We can also pick one route as a temporary route and decide later, however, per first Tim Starling's law, that route can well become permanent without anyone doing anything.

@Legoktm's suggestion is probably the cleanest approach, as long as it doesn't break any of SRE's means of processing this data, as mentioned by @Joe. A likely uglier approach (but one that would work and wouldn't break anything) would be adding Yet Another UA Header...

(btw - we're now tracking this under the next supplemental security announcement [T297839], with the possibility of postponing it further, since I don't think it makes sense to fully disclose this issue when it is only partially-resolved.)

I would agree that @Legoktm's proposal is probably the best option if we need to have all the information in the UA field.

Updated patch to use $ORIGINAL_UA (via EchoForeignWikiRequest MediaWiki/$MW_VERSION) as the UA plus php-codesniffer fixes:

Can we make this task public? I don't think it's exploitable in any way, there is no private information outside the paste, and public tasks and patches tend to get resolved faster.

In T285116#7647718, @Tgr wrote:

Can we make this task public? I don't think it's exploitable in any way, there is no private information outside the paste, and public tasks and patches tend to get resolved faster.

I'd rather deploy the above patch from @majavah first, even if this is fairly low-risk. I believe that's all that we're waiting on here at the moment.

In T285116#7649387, @sbassett wrote:

In T285116#7647718, @Tgr wrote:

Can we make this task public? I don't think it's exploitable in any way, there is no private information outside the paste, and public tasks and patches tend to get resolved faster.

I'd rather deploy the above patch from @majavah first, even if this is fairly low-risk. I believe that's all that we're waiting on here at the moment.

Indeed, I can deploy it after I get a virtual +2 from someone. Note that we have an almost-exact patch already in production, only the user-agent part has been changed.

In T285116#7649425, @Majavah wrote:

Indeed, I can deploy it after I get a virtual +2 from someone. Note that we have an almost-exact patch already in production, only the user-agent part has been changed.

I believe there's enough consensus on the task that I'm happy to give a +2 to deployment. Feel free to deploy either before or after the train deploy today. I can be around to keep an eye on logstash and we can roll back to the previous, already-applied patch if anything unexpected happens with this one.

Done, verified via a manual SQL query:

wikiadmin@10.64.32.82(testwiki)> select cuc_agent from cu_changes where cuc_user_text = 'Majavah create test 20220125 02' limit 1\G
*************************** 1. row ***************************
cuc_agent: Mozilla/5.0 _redacted_ (via EchoForeignWikiRequest MediaWiki/1.38.0-wmf.18)
1 row in set (0.071 sec)

@sbassett this should be fine to include in the next security release.

@Majavah - Thanks for the deploy. Since Echo isn't bundled, this will go out with the next supplemental release (T297839). We were already tracking the first patch there, so I'll add a note to make sure we use the updated patch in the release. This task can be made public now if folks want to start on the backports or just have it available for further discussion.

In T285116#7649573, @sbassett wrote:

@Majavah - Thanks for the deploy. Since Echo isn't bundled, this will go out with the next supplemental release (T297839). We were already tracking the first patch there, so I'll add a note to make sure we use the updated patch in the release. This task can be made public now if folks want to start on the backports or just have it available for further discussion.

hi @majavah and @sbassett

• I don't have access to T297839; is that done?
• could you clarify which backports are needed?
• is there anything needed from Growth-Team for this task?

thanks!

@kostajh -

• I don't have access to T297839; is that done?

No, this is just the tracking task for the next supplemental security release, where this issue will be announced/disclosed.

• could you clarify which backports are needed?

Likely to master, REL1_35, REL1_36 and REL1_37 in accordance with the current version lifecycle. But obviously only on branches where the patch is relevant and applies. Since the patch has been in production for a while, the backports could certainly be started. I'm not 100% sure we'd want to make this bug public - there's internal ipv6 wikimedia app server info in the task description, but I'm not sure if that's a big deal from a Privacy perspective or not.

• is there anything needed from Growth-Team for this task?

I don't think so, unless your team wanted to start on the relevant backports.

Checked for the presence of X-Forwarded-For header on wmf.23/wmf.24 - the header was not present (x-client-ip is present). Just a note: cu_changes and cu_log tables are empty on betalabs cluster.
Removing Growth-Team tag.

In T285116#7709371, @sbassett wrote:

I'm not 100% sure we'd want to make this bug public - there's internal ipv6 wikimedia app server info in the task description, but I'm not sure if that's a big deal from a Privacy perspective or not.

WMF address spaces are public information (see at least https://wikitech.wikimedia.org/wiki/IP_and_AS_allocations, rONED netbox-exported-dns and https://gerrit.wikimedia.org/r/plugins/gitiles/operations/puppet/+/refs/heads/production/modules/network/data/data.yaml, $ host mw1456.eqiad.wmnet ns0.wikimedia.org).

In T285116#7751504, @Majavah wrote:

In T285116#7709371, @sbassett wrote:

I'm not 100% sure we'd want to make this bug public - there's internal ipv6 wikimedia app server info in the task description, but I'm not sure if that's a big deal from a Privacy perspective or not.

WMF address spaces are public information (see at least https://wikitech.wikimedia.org/wiki/IP_and_AS_allocations, rONED netbox-exported-dns and https://gerrit.wikimedia.org/r/plugins/gitiles/operations/puppet/+/refs/heads/production/modules/network/data/data.yaml, $ host mw1456.eqiad.wmnet ns0.wikimedia.org).

Ok, I'm not seeing anything else that'd prevent this task from being made public then, at this point.

Change 776994 had a related patch set uploaded (by SBassett; author: Majavah):

[mediawiki/extensions/Echo@master] SECURITY: Send original client info on x-wiki requests

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

Change 777007 had a related patch set uploaded (by SBassett; author: Majavah):

[mediawiki/extensions/Echo@REL1_38] SECURITY: Send original client info on x-wiki requests

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

Change 777008 had a related patch set uploaded (by SBassett; author: Majavah):

[mediawiki/extensions/Echo@REL1_37] SECURITY: Send original client info on x-wiki requests

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

Change 776994 merged by jenkins-bot:

[mediawiki/extensions/Echo@master] SECURITY: Send original client info on x-wiki requests

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

Change 777007 merged by jenkins-bot:

[mediawiki/extensions/Echo@REL1_38] SECURITY: Send original client info on x-wiki requests

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

Change 777008 merged by jenkins-bot:

[mediawiki/extensions/Echo@REL1_37] SECURITY: Send original client info on x-wiki requests

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