Page MenuHomePhabricator

Monolog added "ip" is REMOTE_ADDR and does not honor XFF headers
Closed, ResolvedPublic


The WMF production logging configuration uses Monolog's Monolog\Processor\WebProcessor class to decorate each log event with several context fields:

  • url: $_SERVER['REQUEST_URI']
  • http_method: $_SERVER['REQUEST_METHOD']
  • server: $_SERVER['SERVER_NAME']
  • referrer: $_SERVER['HTTP_REFERER']

The use of $_SERVER['REMOTE_ADDR'] to capture the requesting IP address is not of much use in the WMF production cluster where all requests are made indirectly from behind a Varnish server. If we continue to log this we should instead use a custom processor that processes XFF headers sensibly using WebRequest::getIP() or something similar.

Event Timeline

bd808 raised the priority of this task from to Needs Triage.
bd808 updated the task description. (Show Details)
bd808 added a project: MediaWiki-Debug-Logger.
bd808 subscribed.

Change 273376 had a related patch set uploaded (by BryanDavis):
Monolog: Add processor for XFF resolved IP

bd808 triaged this task as Low priority.Feb 26 2016, 4:06 AM

Change 273376 merged by jenkins-bot:
Monolog: Add processor for XFF resolved IP

bd808 removed bd808 as the assignee of this task.Jan 24 2017, 6:08 AM
bd808 removed projects: User-bd808, Patch-For-Review.

My attempt was reverted by due to segfaults.

There have been various conversations about this on IRC and Gerrit but just writing back here for the record:

Resolving the trusted XFF IP ranges on every request (eg via WebRequest::getIP like MediaWiki already does in cases for saving edits, determining blocks and throttling), is too expensive.

As a compromise I’m suggesting we start simply log the XFF header as-is to Monolog, in addition to the current ‘ip’ field. This means that the end-user IP and potentially one or more proxies are all in the log message. For the rarer cases where there is three or more and someone needs to know which IP mediawiki would’ve considered for the user, we can always resolve it manually on a case by case basis.

Change 730905 had a related patch set uploaded (by Krinkle; author: Krinkle):

[operations/mediawiki-config@master] logging: Remove host 'ip' field

Change 730905 merged by jenkins-bot:

[operations/mediawiki-config@master] logging: Remove host 'ip' field