From 2aa7c3a6fa1cd31057878fe08f4357fd1de0ef90 Mon Sep 17 00:00:00 2001
From: Lee Rowlands <lee.rowlands@previousnext.com.au>
Date: Thu, 14 Feb 2019 19:02:31 +1000
Subject: [PATCH] =?UTF-8?q?Issue=20#3030501=20by=20alexpott,=20G=C3=A1bor?=
=?UTF-8?q?=20Hojtsy,=20Berdir:=20[Symfony=204]=20Drupal\Core\StackMiddlew?=
=?UTF-8?q?are\ReverseProxyMiddleware=20calls=20Symfony\Component\HttpFoun?=
=?UTF-8?q?dation\Request::setTrustedHeaderName()=20which=20does=20not=20e?=
=?UTF-8?q?xist?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
---
.../ReverseProxyMiddleware.php | 43 +++++----
.../ReverseProxyMiddlewareTest.php | 88 +++++++++++++++----
sites/default/default.settings.php | 62 ++++++-------
3 files changed, 129 insertions(+), 64 deletions(-)
diff --git a/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php b/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php
index cf4cd89cb52c..ded2b92c31d4 100644
--- a/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php
+++ b/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php
@@ -58,24 +58,35 @@ public function handle(Request $request, $type = self::MASTER_REQUEST, $catch =
public static function setSettingsOnRequest(Request $request, Settings $settings) {
// Initialize proxy settings.
if ($settings->get('reverse_proxy', FALSE)) {
- $ip_header = $settings->get('reverse_proxy_header', 'X_FORWARDED_FOR');
- $request::setTrustedHeaderName($request::HEADER_X_FORWARDED_FOR, $ip_header);
-
- $proto_header = $settings->get('reverse_proxy_proto_header', 'X_FORWARDED_PROTO');
- $request::setTrustedHeaderName($request::HEADER_X_FORWARDED_PROTO, $proto_header);
-
- $host_header = $settings->get('reverse_proxy_host_header', 'X_FORWARDED_HOST');
- $request::setTrustedHeaderName($request::HEADER_X_FORWARDED_HOST, $host_header);
-
- $port_header = $settings->get('reverse_proxy_port_header', 'X_FORWARDED_PORT');
- $request::setTrustedHeaderName($request::HEADER_X_FORWARDED_PORT, $port_header);
-
- $forwarded_header = $settings->get('reverse_proxy_forwarded_header', 'FORWARDED');
- $request::setTrustedHeaderName($request::HEADER_FORWARDED, $forwarded_header);
-
$proxies = $settings->get('reverse_proxy_addresses', []);
if (count($proxies) > 0) {
- $request::setTrustedProxies($proxies, Request::HEADER_X_FORWARDED_ALL | Request::HEADER_FORWARDED);
+ $deprecated_settings = [
+ 'reverse_proxy_header' => Request::HEADER_X_FORWARDED_FOR,
+ 'reverse_proxy_proto_header' => Request::HEADER_X_FORWARDED_PROTO,
+ 'reverse_proxy_host_header' => Request::HEADER_X_FORWARDED_HOST,
+ 'reverse_proxy_port_header' => Request::HEADER_X_FORWARDED_PORT,
+ 'reverse_proxy_forwarded_header' => Request::HEADER_FORWARDED,
+ ];
+
+ $all = $settings->getAll();
+ // Set the default value. This is the most relaxed setting possible and
+ // not recommended for production.
+ $trusted_header_set = Request::HEADER_X_FORWARDED_ALL | Request::HEADER_FORWARDED;
+ foreach ($deprecated_settings as $deprecated_setting => $bit_value) {
+ if (array_key_exists($deprecated_setting, $all)) {
+ @trigger_error(sprintf("The '%s' setting in settings.php is deprecated in Drupal 8.7.0 and will be removed before Drupal 9.0.0. Use the 'reverse_proxy_trusted_headers' setting instead. See https://www.drupal.org/node/3030558", $deprecated_setting), E_USER_DEPRECATED);
+ $request::setTrustedHeaderName($bit_value, $all[$deprecated_setting]);
+ if ($all[$deprecated_setting] === NULL) {
+ // If the value is NULL do not trust the header.
+ $trusted_header_set &= ~$bit_value;
+ }
+ }
+ }
+
+ $request::setTrustedProxies(
+ $proxies,
+ $settings->get('reverse_proxy_trusted_headers', $trusted_header_set)
+ );
}
}
}
diff --git a/core/tests/Drupal/Tests/Core/StackMiddleware/ReverseProxyMiddlewareTest.php b/core/tests/Drupal/Tests/Core/StackMiddleware/ReverseProxyMiddlewareTest.php
index 9350327c1003..ba9877ce3133 100644
--- a/core/tests/Drupal/Tests/Core/StackMiddleware/ReverseProxyMiddlewareTest.php
+++ b/core/tests/Drupal/Tests/Core/StackMiddleware/ReverseProxyMiddlewareTest.php
@@ -4,6 +4,7 @@
use Drupal\Core\Site\Settings;
use Drupal\Core\StackMiddleware\ReverseProxyMiddleware;
+use Drupal\Tests\Traits\ExpectDeprecationTrait;
use Drupal\Tests\UnitTestCase;
use Symfony\Component\HttpFoundation\Request;
@@ -13,6 +14,7 @@
* @group StackMiddleware
*/
class ReverseProxyMiddlewareTest extends UnitTestCase {
+ use ExpectDeprecationTrait;
/**
* @var \Symfony\Component\HttpKernel\HttpKernelInterface|\PHPUnit_Framework_MockObject_MockObject
@@ -47,30 +49,83 @@ public function testNoProxy() {
* Tests that subscriber sets trusted headers when reverse proxy is set.
*
* @dataProvider reverseProxyEnabledProvider
+ */
+ public function testReverseProxyEnabled($provided_settings, $expected_trusted_header_set) {
+ // Enable reverse proxy and add test values.
+ $settings = new Settings(['reverse_proxy' => 1] + $provided_settings);
+ $this->trustedHeadersAreSet($settings, $expected_trusted_header_set);
+ }
+
+ /**
+ * Data provider for testReverseProxyEnabled.
+ */
+ public function reverseProxyEnabledProvider() {
+ return [
+ 'Proxy with default trusted headers' => [
+ ['reverse_proxy_addresses' => ['127.0.0.2', '127.0.0.3']],
+ Request::HEADER_FORWARDED | Request::HEADER_X_FORWARDED_ALL,
+ ],
+ 'Proxy with AWS trusted headers' => [
+ [
+ 'reverse_proxy_addresses' => ['127.0.0.2', '127.0.0.3'],
+ 'reverse_proxy_trusted_headers' => Request::HEADER_X_FORWARDED_AWS_ELB,
+ ],
+ Request::HEADER_X_FORWARDED_AWS_ELB,
+ ],
+ 'Proxy with custom trusted headers' => [
+ [
+ 'reverse_proxy_addresses' => ['127.0.0.2', '127.0.0.3'],
+ 'reverse_proxy_trusted_headers' => Request::HEADER_X_FORWARDED_FOR | Request::HEADER_X_FORWARDED_HOST,
+ ],
+ Request::HEADER_X_FORWARDED_FOR | Request::HEADER_X_FORWARDED_HOST,
+ ],
+ ];
+ }
+
+ /**
+ * Tests that subscriber sets trusted headers when reverse proxy is set.
*
+ * @dataProvider reverseProxyEnabledProviderLegacy
* @group legacy
- *
- * @expectedDeprecation The "Symfony\Component\HttpFoundation\Request::setTrustedHeaderName()" method is deprecated since Symfony 3.3 and will be removed in 4.0. Use the $trustedHeaderSet argument of the Request::setTrustedProxies() method instead.
*/
- public function testReverseProxyEnabled($provided_settings) {
+ public function testReverseProxyEnabledLegacy($provided_settings, $expected_trusted_header_set, array $expected_deprecations) {
+ $this->expectedDeprecations($expected_deprecations);
// Enable reverse proxy and add test values.
$settings = new Settings(['reverse_proxy' => 1] + $provided_settings);
- $this->trustedHeadersAreSet($settings);
+ $this->trustedHeadersAreSet($settings, $expected_trusted_header_set);
}
/**
* Data provider for testReverseProxyEnabled.
*/
- public function reverseProxyEnabledProvider() {
+ public function reverseProxyEnabledProviderLegacy() {
return [
- [
+ 'Proxy with deprecated custom headers' => [
[
- 'reverse_proxy_header' => 'X_FORWARDED_FOR_CUSTOMIZED',
- 'reverse_proxy_proto_header' => 'X_FORWARDED_PROTO_CUSTOMIZED',
- 'reverse_proxy_host_header' => 'X_FORWARDED_HOST_CUSTOMIZED',
- 'reverse_proxy_port_header' => 'X_FORWARDED_PORT_CUSTOMIZED',
- 'reverse_proxy_forwarded_header' => 'FORWARDED_CUSTOMIZED',
'reverse_proxy_addresses' => ['127.0.0.2', '127.0.0.3'],
+ 'reverse_proxy_host_header' => NULL,
+ 'reverse_proxy_forwarded_header' => NULL,
+ ],
+ // For AWS configuration forwarded and x_forwarded_host headers are not
+ // trusted.
+ Request::HEADER_X_FORWARDED_AWS_ELB,
+ [
+ 'The \'reverse_proxy_host_header\' setting in settings.php is deprecated in Drupal 8.7.0 and will be removed before Drupal 9.0.0. Use the \'reverse_proxy_trusted_headers\' setting instead. See https://www.drupal.org/node/3030558',
+ 'The \'reverse_proxy_forwarded_header\' setting in settings.php is deprecated in Drupal 8.7.0 and will be removed before Drupal 9.0.0. Use the \'reverse_proxy_trusted_headers\' setting instead. See https://www.drupal.org/node/3030558',
+ 'The "Symfony\Component\HttpFoundation\Request::setTrustedHeaderName()" method is deprecated since Symfony 3.3 and will be removed in 4.0. Use the $trustedHeaderSet argument of the Request::setTrustedProxies() method instead.',
+ ],
+ ],
+ 'Proxy with deprecated custom header' => [
+ [
+ 'reverse_proxy_addresses' => ['127.0.0.2', '127.0.0.3'],
+ 'reverse_proxy_forwarded_header' => NULL,
+ ],
+ // The forwarded header is not trusted which is the same as trusting all
+ // the x_forwarded headers.
+ Request::HEADER_X_FORWARDED_ALL,
+ [
+ 'The \'reverse_proxy_forwarded_header\' setting in settings.php is deprecated in Drupal 8.7.0 and will be removed before Drupal 9.0.0. Use the \'reverse_proxy_trusted_headers\' setting instead. See https://www.drupal.org/node/3030558',
+ 'The "Symfony\Component\HttpFoundation\Request::setTrustedHeaderName()" method is deprecated since Symfony 3.3 and will be removed in 4.0. Use the $trustedHeaderSet argument of the Request::setTrustedProxies() method instead.',
],
],
];
@@ -85,18 +140,17 @@ public function reverseProxyEnabledProvider() {
*
* @param \Drupal\Core\Site\Settings $settings
* The settings object that holds reverse proxy configuration.
+ * @param int $expected_trusted_header_set
+ * The expected bit value returned by
+ * \Symfony\Component\HttpFoundation\Request::getTrustedHeaderSet()
*/
- protected function trustedHeadersAreSet(Settings $settings) {
+ protected function trustedHeadersAreSet(Settings $settings, $expected_trusted_header_set) {
$middleware = new ReverseProxyMiddleware($this->mockHttpKernel, $settings);
$request = new Request();
$middleware->handle($request);
- $this->assertSame($settings->get('reverse_proxy_header'), $request->getTrustedHeaderName($request::HEADER_X_FORWARDED_FOR));
- $this->assertSame($settings->get('reverse_proxy_proto_header'), $request->getTrustedHeaderName($request::HEADER_X_FORWARDED_PROTO));
- $this->assertSame($settings->get('reverse_proxy_host_header'), $request->getTrustedHeaderName($request::HEADER_X_FORWARDED_HOST));
- $this->assertSame($settings->get('reverse_proxy_port_header'), $request->getTrustedHeaderName($request::HEADER_X_FORWARDED_PORT));
- $this->assertSame($settings->get('reverse_proxy_forwarded_header'), $request->getTrustedHeaderName($request::HEADER_FORWARDED));
$this->assertSame($settings->get('reverse_proxy_addresses'), $request->getTrustedProxies());
+ $this->assertSame($expected_trusted_header_set, $request->getTrustedHeaderSet());
}
}
diff --git a/sites/default/default.settings.php b/sites/default/default.settings.php
index 5bcecc267fab..ae64d2ffe89a 100644
--- a/sites/default/default.settings.php
+++ b/sites/default/default.settings.php
@@ -342,11 +342,10 @@
* configuration requires the IP addresses of all remote proxies to be
* specified in $settings['reverse_proxy_addresses'] to work correctly.
*
- * Enable this setting to get Drupal to determine the client IP from
- * the X-Forwarded-For header (or $settings['reverse_proxy_header'] if set).
- * If you are unsure about this setting, do not have a reverse proxy,
- * or Drupal operates in a shared hosting environment, this setting
- * should remain commented out.
+ * Enable this setting to get Drupal to determine the client IP from the
+ * X-Forwarded-For header. If you are unsure about this setting, do not have a
+ * reverse proxy, or Drupal operates in a shared hosting environment, this
+ * setting should remain commented out.
*
* In order for this setting to be used you must specify every possible
* reverse proxy IP address in $settings['reverse_proxy_addresses'].
@@ -365,34 +364,35 @@
# $settings['reverse_proxy_addresses'] = ['a.b.c.d', ...];
/**
- * Set this value if your proxy server sends the client IP in a header
- * other than X-Forwarded-For.
- */
-# $settings['reverse_proxy_header'] = 'X_CLUSTER_CLIENT_IP';
-
-/**
- * Set this value if your proxy server sends the client protocol in a header
- * other than X-Forwarded-Proto.
- */
-# $settings['reverse_proxy_proto_header'] = 'X_FORWARDED_PROTO';
-
-/**
- * Set this value if your proxy server sends the client protocol in a header
- * other than X-Forwarded-Host.
- */
-# $settings['reverse_proxy_host_header'] = 'X_FORWARDED_HOST';
-
-/**
- * Set this value if your proxy server sends the client protocol in a header
- * other than X-Forwarded-Port.
+ * Reverse proxy trusted headers.
+ *
+ * Sets which headers to trust from your reverse proxy.
+ *
+ * Common values are:
+ * - \Symfony\Component\HttpFoundation\Request::HEADER_X_FORWARDED_ALL
+ * - \Symfony\Component\HttpFoundation\Request::HEADER_FORWARDED
+ *
+ * Note the default value of
+ * @code
+ * \Symfony\Component\HttpFoundation\Request::HEADER_X_FORWARDED_ALL | \Symfony\Component\HttpFoundation\Request::HEADER_FORWARDED
+ * @endcode
+ * is not secure by default. The value should be set to only the specific
+ * headers the reverse proxy uses. For example:
+ * @code
+ * \Symfony\Component\HttpFoundation\Request::HEADER_X_FORWARDED_ALL
+ * @endcode
+ * This would trust the following headers:
+ * - X_FORWARDED_FOR
+ * - X_FORWARDED_HOST
+ * - X_FORWARDED_PROTO
+ * - X_FORWARDED_PORT
+ *
+ * @see \Symfony\Component\HttpFoundation\Request::HEADER_X_FORWARDED_ALL
+ * @see \Symfony\Component\HttpFoundation\Request::HEADER_FORWARDED
+ * @see \Symfony\Component\HttpFoundation\Request::setTrustedProxies
*/
-# $settings['reverse_proxy_port_header'] = 'X_FORWARDED_PORT';
+# $settings['reverse_proxy_trusted_headers'] = \Symfony\Component\HttpFoundation\Request::HEADER_X_FORWARDED_ALL | \Symfony\Component\HttpFoundation\Request::HEADER_FORWARDED;
-/**
- * Set this value if your proxy server sends the client protocol in a header
- * other than Forwarded.
- */
-# $settings['reverse_proxy_forwarded_header'] = 'FORWARDED';
/**
* Page caching:
--
GitLab