Problem/Motivation

Drupal 8 use Symfony \Symfony\Component\HttpFoundation\IpUtils::checkIp for validating IPs of the trusted reverse proxies.
So the IP could be a string or an array, and it could contain single IP (V4 or V6) or subnet notation (CIDR)

Proposed resolution

Add a line in the comment to avoid confusion that only an array if IP is a valid format :

 /**
+ * Reverse Proxy Addresses.
+ *
  * Specify every reverse proxy IP address in your environment.
+ * String or array of IPV4/6 address or subnet in CIDR notation.
  * This setting is required if $settings['reverse_proxy'] is TRUE.
  */
 # $settings['reverse_proxy_addresses'] = ['a.b.c.d', ...];

Remaining tasks

  • Provide a patch for default.settings.php
  • Review the patch.
  • Commit the patch.
  • Create follow-up issue to add more (functional?) tests for these headers: #3025077: Improve testing of Trusted Proxy Headers ?

API changes

None.

Data model changes

None.

Release notes snippet

None.

CommentFileSizeAuthor
#14 3032746.patch1.64 KBmfb

Comments

O'Briat created an issue. See original summary.

o'briat’s picture

o'briat
Primary language French
Location Nantes
commented
Related issues: +#3030501: [Symfony 4] Drupal\Core\StackMiddleware\ReverseProxyMiddleware calls Symfony\Component\HttpFoundation\Request::setTrustedHeaderName() which does not exist

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Just ran into this. I only found this Drupal 7 issue: #2466417: Add network range support for reverse_proxy_address in ip_address(), but no documentation for Drupal 8 / 9 if it's allowed to specify a network here.
Is it possible to add further examples with this patch? Is it for example allowed to add the network CIDR notation in the array to be able to define multiple networks allowed?

Examples say more than 1000 words, but the current comment and code is too short for all typical cases I think?

So finally I'm also unsure what's allowed and what's not.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Also see #3017957: Improve documentation for (unused) trusted proxy headers

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Ok here are my (Drupal 8.9) results:

This works (array notation):

$settings['reverse_proxy_addresses'] = ['192.168.199.252', '192.168.201.253', '192.168.199.254'];

This works (array notation):

$settings['reverse_proxy_addresses'] = ['192.168.199.0/24'];

This does NOT work (string):

$settings['reverse_proxy_addresses'] = '192.168.199.0/24';

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

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.

Version: 9.4.x-dev » 9.5.x-dev

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.

Version: 9.5.x-dev » 10.1.x-dev

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.

mfb’s picture

mfb
they or he
commented
Title: Improve documentation for trusted proxy headers format. » Improve documentation for reverse proxy addresses setting
Status: Active » Needs review
StatusFileSize
new 3032746.patch1.64 KB

Incorporated the above suggestions into a patch; also clarifying the issue title.

joachim’s picture

joachim commented
Status: Needs review » Reviewed & tested by the community

LGTM.

  • longwave committed e72d43c on 10.0.x 
    Issue #3032746 by mfb, O'Briat, Anybody: Improve documentation for...
  • longwave committed e9e7821 on 10.1.x
    Issue #3032746 by mfb, O'Briat, Anybody: Improve documentation for...
  • longwave committed f303955 on 9.5.x
    Issue #3032746 by mfb, O'Briat, Anybody: Improve documentation for...
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Reviewed & tested by the community » Fixed

I double checked and the CIDR range syntax has been available in Symfony since 2.3, so this is good to go into all active branches. Thanks for improving the docs!

Committed and pushed e9e78219c4 to 10.1.x and e72d43c884 to 10.0.x and f3039556ae to 9.5.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.