Our Phabricator instance is behind a load balancer making the client IP mostly useless. Phabricator should show the contents of the X-Forwarded-For header for the %r log output (or a new format string should be created)
Description
Description
Event Timeline
Comment Actions
Doing this blindly would let clients spoof remote addresses on installs that are not behind load balancers. The current recommendation is to replace REMOTE_ADDR in preamble.php if you trust the connecting device. Here's what we do in the Phacility cluster (although we're in SiteConfig here, not preamble):
// If this request is coming from within the cluster, we'll trust HTTP // headers added by the load balancer. We do not trust these headers for // requests which do not originate from within the cluster, because they // are client-controlled if the request is being received directly. if (PhacilityServices::isClusterRequest()) { // Since we terminate SSL at the ELB, requests are normally HTTP by the // time they reach the server, even if they were originally HTTPS. The // ELB sets this header to indicate that the original request was HTTPS. if (idx($_SERVER, 'HTTP_X_FORWARDED_PROTO') == 'https') { $_SERVER['HTTPS'] = true; } $forwarded_for = idx($_SERVER, 'HTTP_X_FORWARDED_FOR'); if ($forwarded_for) { // This may be a list of IPs, like "1.2.3.4, 4.5.6.7", if the // request the load balancer received also had this header. In // particular, this happens routinely with requests received through // the CloudFront CDN. // We only care about or trust the last IP in the list: the others are // controlled by the client. $forwarded_for = explode(',', $forwarded_for); $forwarded_for = last($forwarded_for); $forwarded_for = trim($forwarded_for); $_SERVER['REMOTE_ADDR'] = $forwarded_for; }
A minimal preamble might be:
phabricator/support/preamble.php
<?php $_SERVER['REMOTE_ADDR'] = $_SERVER['HTTP_X_FORWARDED_FOR'];
But you may need more logic than that if a connection may possibly be forwarded multiple times, the LB handles terminating SSL, etc.
Comment Actions
In our instance the LB handles SSL but we already account for that in preamble.php
Closed since its better offer handled elsewhere.