Page MenuHomePhabricator

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

Description

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']
  • ip: $_SERVER['REMOTE_ADDR']
  • 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 subscriber: bd808.

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

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

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

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

My attempt was reverted by https://gerrit.wikimedia.org/r/#/c/324333/ 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

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

Change 730905 merged by jenkins-bot:

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

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