settings.php is too late to set $_SERVER['REMOTE_ADDR'] using a CDN's client IP header

Symfony's docs suggest you should override HTTP_X_FORWARDED_* in index.php:

https://symfony.com/doc/4.4/deployment/proxies.html#custom-headers-when-...

Instead of doing this in PHP, you could also do this in the web server configuration, so the header gets translated correctly before PHP even sees it. This is still a docs bug though if it doesn't work as described.

https://www.drupal.org/project/reverse_proxy_header → addresses this issue.

Looks like the developer / maintainer reached the same conclusion about settings.php being too late to manipulate header values in $_SERVER.

Should we do something about this in core other than just fixing the docs? What would we change the recommendation in the docs to?

It seems like a regression that D7 can do this cleanly (acknowledging the reason for the change is down to Symfony doing the heavy lifting here as opposed to core's own code).

I think you should change the value the other way around.
Do not change $_SERVER['REMOTE_ADDR'] but use the remote REMOTE_ADDR to set the reverse_proxy_addresses.

Like so: $settings['reverse_proxy_addresses'] = [$_SERVER['REMOTE_ADDR']];

Doing something like this:

$settings['reverse_proxy_addresses'] = [$_SERVER['REMOTE_ADDR']];

...would often work okay I think, but it may not always work and could in fact be risky.

If just one proxy is involved in the CDN's handling of the request, the approach of adding the REMOTE_ADDR as a trusted proxy will probably work fine (this will be the CDN's IP), and Drupal will correctly derive the client IP from the X-Forwarded-For (XFF) header.

However, if more than one proxy is involved in the CDN's handling of the request (which is perhaps not typical, but I'd say not uncommon), doing this won't work.

Even worse, if it's possible to bypass the CDN and send requests directly to the origin (again not typical / ideal, but not unheard of in the real world), adding REMOTE_ADDR as a trusted proxy may open Drupal up to extracting a spoofed IP from the XFF header.

You could work around that by only adding REMOTE_ADDR as a trusted proxy if other conditions are satisfied (e.g. checking for other headers from the CDN). This is the sort of thing the existing comments probably mean when they say "this would allow IP address spoofing unless more advanced precautions are taken."

Generally this seems like a workaround, and a regression in terms of no longer being able to simply set up a configuration which says "this header from the CDN will contain the client IP".

Again this is a workaround rather than an elegant solution, but if the CDN is supplying a client IP in an alternative HTTP header, and we can only alter the list of trusted proxies... so long as that client IP appears in the X-Forwarded-For header, we can add any IPs to the right of client IP as trusted proxies (in addition to the IP in REMOTE_ADDR).

It's actually very similar to the idea of marking the REMOTE_ADDR IP as a trusted proxy, but covers more scenarios.

I've done some basic testing of this approach within settings.php and it seems to work.

Whether this is something we should consider adding to core, and if so how... I'm not certain.

Drupal 9.5.0-beta2 → and Drupal 10.0.0-beta2 → were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule → and the Allowed changes during the Drupal core release cycle → .

Drupal 9.4.0-alpha1 → was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule → and the Allowed changes during the Drupal core release cycle → .

Drupal 9.3.0-rc1 → was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule → and the Allowed changes during the Drupal core release cycle → .

To elaborate Drew's comment with code, here's an example implementation.

if (isset($_SERVER['HTTP_CF_CONNECTING_IP'])) {
  // If the CloudFlare header is contained in the X-Forwarded-For header, then
  // all IP addresses to the right of that entry are reverse-proxies, which are
  // additional to the value in $_SERVER['REMOTE_ADDR].
  // E.g. <client> --- <CDN> --- <Varnish> --- <drupal>.
  $client = $_SERVER['HTTP_CF_CONNECTING_IP'];
  $ips = explode(', ', $_SERVER['HTTP_X_FORWARDED_FOR']);
  if ($keys = array_keys($ips, $client)) {
    $position = end($keys);
    $reverseProxies = array_slice($ips, $position + 1);
    $reverseProxies[] = $_SERVER['REMOTE_ADDR'];

    $settings['reverse_proxy'] = TRUE;
    $settings['reverse_proxy_addresses'] = $reverseProxies;
  }
}

I just got bit by the incorrect documentation on setting $_SERVER['REMOTE_ADDR'].

Given there are various workarounds, and https://www.drupal.org/project/reverse_proxy_header → exists, I agree we should just fix the docs on this.

I've opened an MR with a first pass at correcting the docs.

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened → , as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule → and the Allowed changes during the Drupal core release cycle → .

The only existing docs I found are at https://www.drupal.org/node/425990 → but it's also in a "To be migrated" section: https://www.drupal.org/to-be-migrated → .

I imagine when docs are moved, redirects are created, so it's probably safe to link there and update it?

Please lets not hold up useful doc changes for trivial improvements.

We can add short note to https://www.drupal.org/node/425990 → and link to that. It would have taken very little time for someone to do that or just confirm #17 was OK, instead this has been held up further by status pong.

Please merge the merge request of 'deviantintegral' after all this time the wrong comment still exists.
https://git.drupalcode.org/project/drupal/-/merge_requests/3218

Based on https://symfony.com/doc/current/deployment/proxies.html we can add a proposed configuration:
$settings['reverse_proxy_addresses'] = ['127.0.0.1','REMOTE_ADDR'];