From 514426e27d9e6c9c7e3882697ea66a57f20a8bc0 Mon Sep 17 00:00:00 2001
From: Roeland Jago Douma <roeland@famdouma.nl>
Date: Mon, 17 Dec 2018 12:44:23 +0100
Subject: [PATCH] Only trust the X-FORWARDED-HOST header for trusted proxies
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
---
lib/private/AppFramework/Http/Request.php | 10 +-
tests/lib/AppFramework/Http/RequestTest.php | 143 ++++++++++++--------
2 files changed, 98 insertions(+), 55 deletions(-)
diff --git a/lib/private/AppFramework/Http/Request.php b/lib/private/AppFramework/Http/Request.php
index 2c745973ed2..00668e87e34 100644
--- a/lib/private/AppFramework/Http/Request.php
+++ b/lib/private/AppFramework/Http/Request.php
@@ -691,7 +691,7 @@ class Request implements \ArrayAccess, \Countable, IRequest {
return $this->config->getSystemValue('overwriteprotocol');
}
- if (isset($this->server['HTTP_X_FORWARDED_PROTO'])) {
+ if ($this->fromTrustedProxy() && isset($this->server['HTTP_X_FORWARDED_PROTO'])) {
if (strpos($this->server['HTTP_X_FORWARDED_PROTO'], ',') !== false) {
$parts = explode(',', $this->server['HTTP_X_FORWARDED_PROTO']);
$proto = strtolower(trim($parts[0]));
@@ -862,7 +862,7 @@ class Request implements \ArrayAccess, \Countable, IRequest {
*/
public function getInsecureServerHost(): string {
$host = 'localhost';
- if (isset($this->server['HTTP_X_FORWARDED_HOST'])) {
+ if ($this->fromTrustedProxy() && isset($this->server['HTTP_X_FORWARDED_HOST'])) {
if (strpos($this->server['HTTP_X_FORWARDED_HOST'], ',') !== false) {
$parts = explode(',', $this->server['HTTP_X_FORWARDED_HOST']);
$host = trim(current($parts));
@@ -924,4 +924,10 @@ class Request implements \ArrayAccess, \Countable, IRequest {
return null;
}
+ private function fromTrustedProxy(): bool {
+ $remoteAddress = isset($this->server['REMOTE_ADDR']) ? $this->server['REMOTE_ADDR'] : '';
+ $trustedProxies = $this->config->getSystemValue('trusted_proxies', []);
+
+ return \is_array($trustedProxies) && $this->isTrustedProxy($trustedProxies, $remoteAddress);
+ }
}
diff --git a/tests/lib/AppFramework/Http/RequestTest.php b/tests/lib/AppFramework/Http/RequestTest.php
index c0e8dc97ef2..b01176f0cf1 100644
--- a/tests/lib/AppFramework/Http/RequestTest.php
+++ b/tests/lib/AppFramework/Http/RequestTest.php
@@ -715,15 +715,20 @@ class RequestTest extends \Test\TestCase {
public function testGetServerProtocolWithProtoValid() {
$this->config
- ->expects($this->exactly(2))
->method('getSystemValue')
- ->with('overwriteprotocol')
- ->will($this->returnValue(''));
+ ->will($this->returnCallback(function($key, $default) {
+ if ($key === 'trusted_proxies') {
+ return ['1.2.3.4'];
+ }
+
+ return $default;
+ }));
$requestHttps = new Request(
[
'server' => [
- 'HTTP_X_FORWARDED_PROTO' => 'HtTpS'
+ 'HTTP_X_FORWARDED_PROTO' => 'HtTpS',
+ 'REMOTE_ADDR' => '1.2.3.4',
],
],
$this->secureRandom,
@@ -734,7 +739,8 @@ class RequestTest extends \Test\TestCase {
$requestHttp = new Request(
[
'server' => [
- 'HTTP_X_FORWARDED_PROTO' => 'HTTp'
+ 'HTTP_X_FORWARDED_PROTO' => 'HTTp',
+ 'REMOTE_ADDR' => '1.2.3.4',
],
],
$this->secureRandom,
@@ -750,10 +756,10 @@ class RequestTest extends \Test\TestCase {
public function testGetServerProtocolWithHttpsServerValueOn() {
$this->config
- ->expects($this->once())
->method('getSystemValue')
- ->with('overwriteprotocol')
- ->will($this->returnValue(''));
+ ->will($this->returnCallback(function($key, $default) {
+ return $default;
+ }));
$request = new Request(
[
@@ -771,10 +777,10 @@ class RequestTest extends \Test\TestCase {
public function testGetServerProtocolWithHttpsServerValueOff() {
$this->config
- ->expects($this->once())
->method('getSystemValue')
- ->with('overwriteprotocol')
- ->will($this->returnValue(''));
+ ->will($this->returnCallback(function($key, $default) {
+ return $default;
+ }));
$request = new Request(
[
@@ -792,10 +798,10 @@ class RequestTest extends \Test\TestCase {
public function testGetServerProtocolWithHttpsServerValueEmpty() {
$this->config
- ->expects($this->once())
->method('getSystemValue')
- ->with('overwriteprotocol')
- ->will($this->returnValue(''));
+ ->will($this->returnCallback(function($key, $default) {
+ return $default;
+ }));
$request = new Request(
[
@@ -813,10 +819,10 @@ class RequestTest extends \Test\TestCase {
public function testGetServerProtocolDefault() {
$this->config
- ->expects($this->once())
->method('getSystemValue')
- ->with('overwriteprotocol')
- ->will($this->returnValue(''));
+ ->will($this->returnCallback(function($key, $default) {
+ return $default;
+ }));
$request = new Request(
[],
@@ -830,15 +836,20 @@ class RequestTest extends \Test\TestCase {
public function testGetServerProtocolBehindLoadBalancers() {
$this->config
- ->expects($this->once())
->method('getSystemValue')
- ->with('overwriteprotocol')
- ->will($this->returnValue(''));
+ ->will($this->returnCallback(function($key, $default) {
+ if ($key === 'trusted_proxies') {
+ return ['1.2.3.4'];
+ }
+
+ return $default;
+ }));
$request = new Request(
[
'server' => [
- 'HTTP_X_FORWARDED_PROTO' => 'https,http,http'
+ 'HTTP_X_FORWARDED_PROTO' => 'https,http,http',
+ 'REMOTE_ADDR' => '1.2.3.4',
],
],
$this->secureRandom,
@@ -1025,12 +1036,23 @@ class RequestTest extends \Test\TestCase {
}
public function testInsecureServerHostHttpFromForwardedHeaderSingle() {
+ $this->config
+ ->method('getSystemValue')
+ ->will($this->returnCallback(function($key, $default) {
+ if ($key === 'trusted_proxies') {
+ return ['1.2.3.4'];
+ }
+
+ return $default;
+ }));
+
$request = new Request(
[
'server' => [
'SERVER_NAME' => 'from.server.name:8080',
'HTTP_HOST' => 'from.host.header:8080',
'HTTP_X_FORWARDED_HOST' => 'from.forwarded.host:8080',
+ 'REMOTE_ADDR' => '1.2.3.4',
]
],
$this->secureRandom,
@@ -1043,12 +1065,23 @@ class RequestTest extends \Test\TestCase {
}
public function testInsecureServerHostHttpFromForwardedHeaderStacked() {
+ $this->config
+ ->method('getSystemValue')
+ ->will($this->returnCallback(function($key, $default) {
+ if ($key === 'trusted_proxies') {
+ return ['1.2.3.4'];
+ }
+
+ return $default;
+ }));
+
$request = new Request(
[
'server' => [
'SERVER_NAME' => 'from.server.name:8080',
'HTTP_HOST' => 'from.host.header:8080',
'HTTP_X_FORWARDED_HOST' => 'from.forwarded.host2:8080,another.one:9000',
+ 'REMOTE_ADDR' => '1.2.3.4',
]
],
$this->secureRandom,
@@ -1062,20 +1095,16 @@ class RequestTest extends \Test\TestCase {
public function testGetServerHostWithOverwriteHost() {
$this->config
- ->expects($this->at(0))
- ->method('getSystemValue')
- ->with('overwritehost')
- ->will($this->returnValue('my.overwritten.host'));
- $this->config
- ->expects($this->at(1))
->method('getSystemValue')
- ->with('overwritecondaddr')
- ->will($this->returnValue(''));
- $this->config
- ->expects($this->at(2))
- ->method('getSystemValue')
- ->with('overwritehost')
- ->will($this->returnValue('my.overwritten.host'));
+ ->will($this->returnCallback(function($key, $default) {
+ if ($key === 'overwritecondaddr') {
+ return '';
+ } else if ($key === 'overwritehost') {
+ return 'my.overwritten.host';
+ }
+
+ return $default;
+ }));
$request = new Request(
[],
@@ -1090,15 +1119,22 @@ class RequestTest extends \Test\TestCase {
public function testGetServerHostWithTrustedDomain() {
$this->config
- ->expects($this->at(3))
->method('getSystemValue')
- ->with('trusted_domains')
- ->will($this->returnValue(['my.trusted.host']));
+ ->will($this->returnCallback(function($key, $default) {
+ if ($key === 'trusted_proxies') {
+ return ['1.2.3.4'];
+ } else if ($key === 'trusted_domains') {
+ return ['my.trusted.host'];
+ }
+
+ return $default;
+ }));
$request = new Request(
[
'server' => [
'HTTP_X_FORWARDED_HOST' => 'my.trusted.host',
+ 'REMOTE_ADDR' => '1.2.3.4',
],
],
$this->secureRandom,
@@ -1112,20 +1148,22 @@ class RequestTest extends \Test\TestCase {
public function testGetServerHostWithUntrustedDomain() {
$this->config
- ->expects($this->at(3))
- ->method('getSystemValue')
- ->with('trusted_domains')
- ->will($this->returnValue(['my.trusted.host']));
- $this->config
- ->expects($this->at(4))
->method('getSystemValue')
- ->with('trusted_domains')
- ->will($this->returnValue(['my.trusted.host']));
+ ->will($this->returnCallback(function($key, $default) {
+ if ($key === 'trusted_proxies') {
+ return ['1.2.3.4'];
+ } else if ($key === 'trusted_domains') {
+ return ['my.trusted.host'];
+ }
+
+ return $default;
+ }));
$request = new Request(
[
'server' => [
'HTTP_X_FORWARDED_HOST' => 'my.untrusted.host',
+ 'REMOTE_ADDR' => '1.2.3.4',
],
],
$this->secureRandom,
@@ -1139,20 +1177,19 @@ class RequestTest extends \Test\TestCase {
public function testGetServerHostWithNoTrustedDomain() {
$this->config
- ->expects($this->at(3))
->method('getSystemValue')
- ->with('trusted_domains')
- ->will($this->returnValue([]));
- $this->config
- ->expects($this->at(4))
- ->method('getSystemValue')
- ->with('trusted_domains')
- ->will($this->returnValue([]));
+ ->will($this->returnCallback(function($key, $default) {
+ if ($key === 'trusted_proxies') {
+ return ['1.2.3.4'];
+ }
+ return $default;
+ }));
$request = new Request(
[
'server' => [
'HTTP_X_FORWARDED_HOST' => 'my.untrusted.host',
+ 'REMOTE_ADDR' => '1.2.3.4',
],
],
$this->secureRandom,
--
GitLab