From 68f1100200c48c2c31ff6f68bc5ce5184c23a6fe Mon Sep 17 00:00:00 2001 From: Alexander Schieweck Date: Mon, 3 Jun 2024 22:06:19 +0100 Subject: [PATCH 1/5] feat(lib): Improved remote addr parsing to also handle the Forwarded header Signed-off-by: Alexander Schieweck --- lib/private/AppFramework/Http/Request.php | 79 +++++--- tests/lib/AppFramework/Http/RequestTest.php | 188 ++++++++------------ 2 files changed, 129 insertions(+), 138 deletions(-) diff --git a/lib/private/AppFramework/Http/Request.php b/lib/private/AppFramework/Http/Request.php index 0bd430545d42e..aba8c2f49aee6 100644 --- a/lib/private/AppFramework/Http/Request.php +++ b/lib/private/AppFramework/Http/Request.php @@ -555,39 +555,63 @@ protected function isTrustedProxy($trustedProxies, $remoteAddress) { * @return string IP address */ public function getRemoteAddress(): string { - $remoteAddress = isset($this->server['REMOTE_ADDR']) ? $this->server['REMOTE_ADDR'] : ''; - $trustedProxies = $this->config->getSystemValue('trusted_proxies', []); + $remoteAddress = $this->server['REMOTE_ADDR'] ?? ''; - if (\is_array($trustedProxies) && $this->isTrustedProxy($trustedProxies, $remoteAddress)) { - $forwardedForHeaders = $this->config->getSystemValue('forwarded_for_headers', [ - 'HTTP_X_FORWARDED_FOR' - // only have one default, so we cannot ship an insecure product out of the box - ]); - - // Read the x-forwarded-for headers and values in reverse order as per - // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For#selecting_an_ip_address - foreach (array_reverse($forwardedForHeaders) as $header) { - if (isset($this->server[$header])) { - foreach (array_reverse(explode(',', $this->server[$header])) as $IP) { - $IP = trim($IP); - $colons = substr_count($IP, ':'); - if ($colons > 1) { - // Extract IP from string with brackets and optional port - if (preg_match('/^\[(.+?)\](?::\d+)?$/', $IP, $matches) && isset($matches[1])) { - $IP = $matches[1]; + $trustedProxies = $this->config->getSystemValue('trusted_proxies', []); + if (count($trustedProxies) == 0 || $this->isTrustedProxy($trustedProxies, $remoteAddress)) { + return $remoteAddress; + } + $forwardedForHeaders = $this->config->getSystemValue('forwarded_for_headers', [ + 'HTTP_X_FORWARDED_FOR', // de-facto standard to keep track of the proxy chain + 'HTTP_FORWARDED', // new standard to keep track of the proxy chain + ]); + + foreach ($forwardedForHeaders as $header) { + if (isset($this->server[$header])) { + $headerContent = $this->server[$header]; + + $IPs = []; + if (str_contains($headerContent, 'for')) { + // newer HTTP_FORWARDED format + $proxyEntries = explode(',', $headerContent); + foreach ($proxyEntries as $proxy) { + $parts = explode(';', $proxy); + foreach ($parts as $part) { + if (str_starts_with(strtolower(ltrim($part)), 'for') && substr_count($part, '=') !== false) { + $part = substr($part, strpos($part, '=') + 1, strlen($part)); + $IPs = array_merge($IPs, explode(',', $part)); } - } elseif ($colons === 1) { - // IPv4 with port - $IP = substr($IP, 0, strpos($IP, ':')); } + } + } else { + // old school HTTP_X_FORWARDED_FOR format + $IPs = explode(',', $headerContent); + } - if ($this->isTrustedProxy($trustedProxies, $IP)) { - continue; + // Read the x-forwarded-for headers and values in reverse order as per + // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For#selecting_an_ip_address + foreach (array_reverse($IPs) as $IP) { + $IP = trim($IP); + $colons = substr_count($IP, ':'); + // $colons == 0 -> IPv4 without port, nothing to do + if ($colons === 1) { + // IPv4 with port + $IP = substr($IP, 0, strpos($IP, ':')); + } elseif ($colons > 1) { + // Extract IPv6 from string with brackets and optional port + if (preg_match('/^\[(.+?)\](?::\d+)?$/', $IP, $matches) && isset($matches[1])) { + $IP = $matches[1]; } + } - if (filter_var($IP, FILTER_VALIDATE_IP) !== false) { - return $IP; - } + if (filter_var($IP, FILTER_VALIDATE_IP) === false) { + // TODO: What to do with spoofed values? + break; + } + + if (!$this->isTrustedProxy($trustedProxies, $IP)) { + $remoteAddress = $IP; + break; } } } @@ -596,6 +620,7 @@ public function getRemoteAddress(): string { return $remoteAddress; } + /** * Check overwrite condition * @return bool diff --git a/tests/lib/AppFramework/Http/RequestTest.php b/tests/lib/AppFramework/Http/RequestTest.php index f0e1f45902858..34056ff7da251 100644 --- a/tests/lib/AppFramework/Http/RequestTest.php +++ b/tests/lib/AppFramework/Http/RequestTest.php @@ -546,162 +546,128 @@ public function testSetUrlParameters() { public function dataGetRemoteAddress(): array { return [ - 'IPv4 without trusted remote' => [ + 'IPv4 with untrusted remote & no headers' => [ [ - 'REMOTE_ADDR' => '10.0.0.2', - 'HTTP_X_FORWARDED' => '10.4.0.5, 10.4.0.4', - 'HTTP_X_FORWARDED_FOR' => '192.168.0.233', + 'REMOTE_ADDR' => '10.0.0.10', ], [], [], - '10.0.0.2', + '10.0.0.10', ], - 'IPv4 without trusted headers' => [ + 'IPv4 with trusted remote & no headers' => [ [ - 'REMOTE_ADDR' => '10.0.0.2', - 'HTTP_X_FORWARDED' => '10.4.0.5, 10.4.0.4', - 'HTTP_X_FORWARDED_FOR' => '192.168.0.233', + 'REMOTE_ADDR' => '10.0.0.10', ], - ['10.0.0.2'], + ['10.10.10.10'], [], - '10.0.0.2', + '10.0.0.10', ], - 'IPv4 with single trusted remote' => [ + 'IPv4 with untrusted remote & untrusted headers' => [ [ - 'REMOTE_ADDR' => '10.0.0.2', - 'HTTP_X_FORWARDED' => '10.4.0.5, 10.4.0.4', - 'HTTP_X_FORWARDED_FOR' => '192.168.0.233', + 'REMOTE_ADDR' => '10.0.0.10', + 'HTTP_X_FORWARDED_FOR' => '10.0.0.10, 172.16.0.2, 172.16.0.3', + 'HTTP_FORWARDED' => 'for=10.0.0.10, for=172.16.0.2, for=172.16.0.3', ], - ['10.0.0.2'], - ['HTTP_X_FORWARDED'], - '10.4.0.4', + [], + ['HTTP_X_FORWARDED_FOR', 'HTTP_FORWARDED'], + '10.0.0.10', ], - 'IPv6 with single trusted remote' => [ + 'IPv4 with trusted remote & untrusted headers' => [ [ - 'REMOTE_ADDR' => '2001:db8:85a3:8d3:1319:8a2e:370:7348', - 'HTTP_X_FORWARDED' => '10.4.0.5, 10.4.0.4', - 'HTTP_X_FORWARDED_FOR' => '192.168.0.233', + 'REMOTE_ADDR' => '10.0.0.10', + 'HTTP_X_FORWARDED_FOR' => '10.0.0.10, 172.16.0.2, 172.16.0.3', + 'HTTP_FORWARDED' => 'for=10.0.0.10, for=172.16.0.2, for=172.16.0.3', ], - ['2001:db8:85a3:8d3:1319:8a2e:370:7348'], - ['HTTP_X_FORWARDED'], - '10.4.0.4', + ['10.0.0.10'], + ['HTTP_X_FORWARDED_FOR', 'HTTP_FORWARDED'], + '10.0.0.10', ], - 'IPv4 with multiple trusted remotes' => [ + 'IPv4 with trusted remote & trusted X-FORWARDED-FOR' => [ [ - 'REMOTE_ADDR' => '10.0.0.2', - 'HTTP_X_FORWARDED' => '10.4.0.5, 10.4.0.4, ::1', - 'HTTP_X_FORWARDED_FOR' => '192.168.0.233', + 'REMOTE_ADDR' => '10.0.0.10', + 'HTTP_X_FORWARDED_FOR' => '10.0.0.10, 172.16.0.2, 172.16.0.3', + 'HTTP_FORWARDED' => 'for=10.0.0.10, for=172.16.0.2, for=172.16.0.3', ], - ['10.0.0.2', '::1'], - ['HTTP_X_FORWARDED'], - '10.4.0.4', + ['10.0.0.10', '172.16.0.2', '172.16.0.3'], + ['X-FORWARD-FOR'], + '10.0.0.10', ], - 'IPv4 order of forwarded-for headers' => [ - [ - 'REMOTE_ADDR' => '10.0.0.2', - 'HTTP_X_FORWARDED' => '10.4.0.5, 10.4.0.4', - 'HTTP_X_FORWARDED_FOR' => '192.168.0.233', - ], - ['10.0.0.2'], + 'IPv4 with trusted remote & trusted FORWARDED' => [ [ - 'HTTP_X_FORWARDED', - 'HTTP_X_FORWARDED_FOR', - 'HTTP_CLIENT_IP', + 'REMOTE_ADDR' => '10.0.0.10', + 'HTTP_X_FORWARDED_FOR' => '10.0.0.10, 172.16.0.2, 172.16.0.3', + 'HTTP_FORWARDED' => 'for=10.0.0.10, by=10.0.0.10;for=172.16.0.2, for=172.16.0.3;by=172.16.0.2', ], - '192.168.0.233', + ['10.0.0.10', '172.16.0.2', '172.16.0.3'], + ['HTTP_FORWARDED'], + '10.0.0.10', ], - 'IPv4 order of forwarded-for headers (reversed)' => [ + 'IPv4 with untrusted remote & partally trusted X-FORWARDED-FOR' => [ [ - 'REMOTE_ADDR' => '10.0.0.2', - 'HTTP_X_FORWARDED' => '10.4.0.5, 10.4.0.4', - 'HTTP_X_FORWARDED_FOR' => '192.168.0.233', + 'REMOTE_ADDR' => '10.0.0.10', + 'HTTP_X_FORWARDED_FOR' => '10.0.0.10, 172.16.0.2, 172.16.0.3', + 'HTTP_FORWARDED' => 'for=10.0.0.10, for=172.16.0.2, for=172.16.0.3', ], - ['10.0.0.2'], - [ - 'HTTP_CLIENT_IP', - 'HTTP_X_FORWARDED_FOR', - 'HTTP_X_FORWARDED', - ], - '10.4.0.4', + ['172.16.0.3'], + ['HTTP_X_FORWARDED_FOR'], + '172.16.0.2', ], - 'IPv6 order of forwarded-for headers' => [ + 'IPv4 with untrusted remote & partially trusted FORWARDED' => [ [ - 'REMOTE_ADDR' => '2001:db8:85a3:8d3:1319:8a2e:370:7348', - 'HTTP_X_FORWARDED' => '10.4.0.5, 10.4.0.4', - 'HTTP_X_FORWARDED_FOR' => '192.168.0.233', + 'REMOTE_ADDR' => '10.0.0.10', + 'HTTP_X_FORWARDED_FOR' => '10.0.0.10, 172.16.0.2, 172.16.0.3', + 'HTTP_FORWARDED' => 'for=10.0.0.10, for=172.16.0.2, for=172.16.0.3', ], - ['2001:db8:85a3:8d3:1319:8a2e:370:7348'], - [ - 'HTTP_X_FORWARDED', - 'HTTP_X_FORWARDED_FOR', - 'HTTP_CLIENT_IP', - ], - '192.168.0.233', + ['172.16.0.3'], + ['HTTP_FORWARDED'], + '172.16.0.2', ], - 'IPv4 matching CIDR of trusted proxy' => [ + 'IPv4 with trusted remote & partially trusted headers' => [ [ - 'REMOTE_ADDR' => '192.168.3.99', - 'HTTP_X_FORWARDED' => '10.4.0.5, 10.4.0.4', - 'HTTP_X_FORWARDED_FOR' => '192.168.0.233', + 'REMOTE_ADDR' => '10.0.0.10', + 'HTTP_X_FORWARDED_FOR' => '10.0.0.10, 172.16.0.2, 172.16.0.3', + 'HTTP_FORWARDED' => 'for=10.0.0.10, for=172.16.0.2, for=172.16.0.3', ], - ['192.168.2.0/24'], - ['HTTP_X_FORWARDED_FOR'], - '192.168.3.99', + ['10.0.0.10', '172.16.0.3'], + ['HTTP_X_FORWARDED_FOR', 'HTTP_FORWARDED'], + '10.0.0.10', ], - 'IPv6 matching CIDR of trusted proxy' => [ + 'IPv4 with untrusted remote & and invalid X-FORWARDED-FOR' => [ [ - 'REMOTE_ADDR' => '2001:db8:85a3:8d3:1319:8a21:370:7348', - 'HTTP_X_FORWARDED' => '10.4.0.5, 10.4.0.4', - 'HTTP_X_FORWARDED_FOR' => '192.168.0.233', + 'REMOTE_ADDR' => '10.0.0.10', + 'HTTP_X_FORWARDED_FOR' => '10.0.0.10, nope, 172.16.0.3', + 'HTTP_FORWARDED' => 'for=10.0.0.10, for=1172.16.0.2, for=1172.16.0.3', ], - ['2001:db8:85a3:8d3:1319:8a20::/95'], + ['172.16.0.3'], ['HTTP_X_FORWARDED_FOR'], - '192.168.0.233', + '10.0.0.10', ], - 'IPv6 not matching CIDR of trusted proxy' => [ + 'Trusted chain of different formats within X-FORWARDED-FOR' => [ [ - 'REMOTE_ADDR' => '2001:db8:85a3:8d3:1319:8a2e:370:7348', - 'HTTP_X_FORWARDED' => '10.4.0.5, 10.4.0.4', - 'HTTP_X_FORWARDED_FOR' => '192.168.0.233', + 'REMOTE_ADDR' => '10.0.0.10', + 'HTTP_X_FORWARDED_FOR' => '10.0.0.20, [fdd8:44e6:ce84:22c3:2222:2222:2222:2222]:8080, fdd8:44e6:ce84:22c3:1111:1111:1111:1111, 2001:db8:85a3:8d3:1319:8a2e:370:7348, 172.16.0.2:80, 172.16.0.3', + 'HTTP_FORWARDED' => 'for=10.0.0.10, for=172.16.0.2, for=172.16.0.3', ], - ['fd::/8'], - [], - '2001:db8:85a3:8d3:1319:8a2e:370:7348', + ['172.16.0.0/24', '2001:db8:85a3:8d3:1319:8a2e:370:7348', 'fdd8:44e6:ce84:22c3::/64'], + ['HTTP_X_FORWARDED_FOR'], + '10.0.0.20', ], - 'IPv6 with invalid trusted proxy' => [ + 'Trusted chain of different formats within FORWARDED' => [ [ - 'REMOTE_ADDR' => '2001:db8:85a3:8d3:1319:8a2e:370:7348', - 'HTTP_X_FORWARDED' => '10.4.0.5, 10.4.0.4', - 'HTTP_X_FORWARDED_FOR' => '192.168.0.233', + 'REMOTE_ADDR' => '10.0.0.10', + 'HTTP_X_FORWARDED_FOR' => '10.0.0.10, 172.16.0.2, 172.16.0.3', + 'HTTP_FORWARDED' => 'for=10.0.0.20, for=[fdd8:44e6:ce84:22c3:2222:2222:2222:2222]:8080, for=fdd8:44e6:ce84:22c3:1111:1111:1111:1111, for=2001:db8:85a3:8d3:1319:8a2e:370:7348, for=172.16.0.2:80, for=172.16.0.3', ], - ['fx::/8'], - [], - '2001:db8:85a3:8d3:1319:8a2e:370:7348', + ['172.16.0.0/24', '2001:db8:85a3:8d3:1319:8a2e:370:7348', 'fdd8:44e6:ce84:22c3::/64'], + ['HTTP_FORWARDED'], + '10.0.0.20', ], 'IPv4 forwarded for IPv6' => [ [ 'REMOTE_ADDR' => '192.168.2.99', 'HTTP_X_FORWARDED_FOR' => '[2001:db8:85a3:8d3:1319:8a2e:370:7348]', ], - ['192.168.2.0/24'], - ['HTTP_X_FORWARDED_FOR'], - '2001:db8:85a3:8d3:1319:8a2e:370:7348', - ], - 'IPv4 with port' => [ - [ - 'REMOTE_ADDR' => '2001:db8:85a3:8d3:1319:8a2e:370:7348', - 'HTTP_X_FORWARDED_FOR' => '192.168.2.99:8080', - ], - ['2001:db8::/8'], - ['HTTP_X_FORWARDED_FOR'], - '192.168.2.99', - ], - 'IPv6 with port' => [ - [ - 'REMOTE_ADDR' => '192.168.2.99', - 'HTTP_X_FORWARDED_FOR' => '[2001:db8:85a3:8d3:1319:8a2e:370:7348]:8080', - ], - ['192.168.2.0/24'], + ['192.168.20.0/24'], ['HTTP_X_FORWARDED_FOR'], '2001:db8:85a3:8d3:1319:8a2e:370:7348', ], From 0b943ca56948c5b052eb178930b4ef0ac779c11c Mon Sep 17 00:00:00 2001 From: Alexander Schieweck Date: Mon, 3 Jun 2024 22:38:54 +0100 Subject: [PATCH 2/5] feat(lib): also consider trusted proxies when checking overwritecondaddr Signed-off-by: Alexander Schieweck --- lib/private/AppFramework/Http/Request.php | 41 +++++++++++++++++++---- 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/lib/private/AppFramework/Http/Request.php b/lib/private/AppFramework/Http/Request.php index aba8c2f49aee6..95908716866b5 100644 --- a/lib/private/AppFramework/Http/Request.php +++ b/lib/private/AppFramework/Http/Request.php @@ -555,19 +555,30 @@ protected function isTrustedProxy($trustedProxies, $remoteAddress) { * @return string IP address */ public function getRemoteAddress(): string { + return $this->getRemoateAdressAndProxyChain()['remote_address']; + } + + /** + * Returns the remote address and the trusted proxy chain from the `forwarded_for_headers` + * @return [string remoate_address, [string proxies]] + */ + public function getRemoateAdressAndProxyChain(): array { $remoteAddress = $this->server['REMOTE_ADDR'] ?? ''; $trustedProxies = $this->config->getSystemValue('trusted_proxies', []); - if (count($trustedProxies) == 0 || $this->isTrustedProxy($trustedProxies, $remoteAddress)) { - return $remoteAddress; + if (!is_array($trustedProxies) || count($trustedProxies) == 0 || $this->isTrustedProxy($trustedProxies, $remoteAddress)) { + return ['remote_address' => $remoteAddress, 'proxies' => []]; } $forwardedForHeaders = $this->config->getSystemValue('forwarded_for_headers', [ 'HTTP_X_FORWARDED_FOR', // de-facto standard to keep track of the proxy chain 'HTTP_FORWARDED', // new standard to keep track of the proxy chain ]); + $proxies = []; foreach ($forwardedForHeaders as $header) { if (isset($this->server[$header])) { + $proxies = []; // reset for each possible header + $headerContent = $this->server[$header]; $IPs = []; @@ -605,7 +616,6 @@ public function getRemoteAddress(): string { } if (filter_var($IP, FILTER_VALIDATE_IP) === false) { - // TODO: What to do with spoofed values? break; } @@ -613,11 +623,13 @@ public function getRemoteAddress(): string { $remoteAddress = $IP; break; } + + $proxies[] = $IP; } } } - return $remoteAddress; + return ['remote_address' => $remoteAddress, 'proxies' => $proxies]; } @@ -627,8 +639,25 @@ public function getRemoteAddress(): string { */ private function isOverwriteCondition(): bool { $regex = '/' . $this->config->getSystemValueString('overwritecondaddr', '') . '/'; - $remoteAddr = isset($this->server['REMOTE_ADDR']) ? $this->server['REMOTE_ADDR'] : ''; - return $regex === '//' || preg_match($regex, $remoteAddr) === 1; + if ($regex === '//') { + return true; + } + + $remoteAddressAndProxyChain = $this->getRemoateAdressAndProxyChain(); + $remoteAddress = $remoteAddressAndProxyChain['remote_address']; + $proxies = $remoteAddressAndProxyChain['proxies']; + + if(preg_match($regex, $remoteAddress) === 1) { + return true; + } + + foreach ($proxies as $proxy) { + if(preg_match($regex, $proxy) === 1) { + return true; + } + } + + return false; } /** From 143dc84b8ca9839a07d317e9912385b62d314656 Mon Sep 17 00:00:00 2001 From: Alexander Schieweck Date: Mon, 3 Jun 2024 22:58:55 +0100 Subject: [PATCH 3/5] fix(lib): typos in Request.php Signed-off-by: Alexander Schieweck --- lib/private/AppFramework/Http/Request.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/private/AppFramework/Http/Request.php b/lib/private/AppFramework/Http/Request.php index 95908716866b5..3f0d7227edd5c 100644 --- a/lib/private/AppFramework/Http/Request.php +++ b/lib/private/AppFramework/Http/Request.php @@ -555,14 +555,14 @@ protected function isTrustedProxy($trustedProxies, $remoteAddress) { * @return string IP address */ public function getRemoteAddress(): string { - return $this->getRemoateAdressAndProxyChain()['remote_address']; + return $this->getRemoteAddressAndProxyChain()['remote_address']; } /** * Returns the remote address and the trusted proxy chain from the `forwarded_for_headers` * @return [string remoate_address, [string proxies]] */ - public function getRemoateAdressAndProxyChain(): array { + public function getRemoteAddressAndProxyChain(): array { $remoteAddress = $this->server['REMOTE_ADDR'] ?? ''; $trustedProxies = $this->config->getSystemValue('trusted_proxies', []); @@ -643,7 +643,7 @@ private function isOverwriteCondition(): bool { return true; } - $remoteAddressAndProxyChain = $this->getRemoateAdressAndProxyChain(); + $remoteAddressAndProxyChain = $this->getRemoteAddressAndProxyChain(); $remoteAddress = $remoteAddressAndProxyChain['remote_address']; $proxies = $remoteAddressAndProxyChain['proxies']; From 661c54139c2665bded772ed9fe9918d20c4668c8 Mon Sep 17 00:00:00 2001 From: Alexander Schieweck Date: Tue, 4 Jun 2024 17:34:54 +0100 Subject: [PATCH 4/5] fix(lib): Request.php now passes the static checks Signed-off-by: Alexander Schieweck --- lib/private/AppFramework/Http/Request.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/private/AppFramework/Http/Request.php b/lib/private/AppFramework/Http/Request.php index 3f0d7227edd5c..3f1b6719e5483 100644 --- a/lib/private/AppFramework/Http/Request.php +++ b/lib/private/AppFramework/Http/Request.php @@ -560,7 +560,7 @@ public function getRemoteAddress(): string { /** * Returns the remote address and the trusted proxy chain from the `forwarded_for_headers` - * @return [string remoate_address, [string proxies]] + * @return array{remote_address: string, proxies: list} */ public function getRemoteAddressAndProxyChain(): array { $remoteAddress = $this->server['REMOTE_ADDR'] ?? ''; @@ -588,7 +588,7 @@ public function getRemoteAddressAndProxyChain(): array { foreach ($proxyEntries as $proxy) { $parts = explode(';', $proxy); foreach ($parts as $part) { - if (str_starts_with(strtolower(ltrim($part)), 'for') && substr_count($part, '=') !== false) { + if (str_starts_with(strtolower(ltrim($part)), 'for') && str_contains($part, '=')) { $part = substr($part, strpos($part, '=') + 1, strlen($part)); $IPs = array_merge($IPs, explode(',', $part)); } From a00b6f992c666cac53e185af532d8a1358ef966c Mon Sep 17 00:00:00 2001 From: Alexander Schieweck Date: Sun, 9 Jun 2024 22:17:12 +0100 Subject: [PATCH 5/5] refactor(lib): added tests for the new trusted proxy override Signed-off-by: Alexander Schieweck --- lib/private/AppFramework/Http/Request.php | 1 + tests/lib/AppFramework/Http/RequestTest.php | 52 +++++++++++---------- 2 files changed, 29 insertions(+), 24 deletions(-) diff --git a/lib/private/AppFramework/Http/Request.php b/lib/private/AppFramework/Http/Request.php index 3f1b6719e5483..cb65d59be1045 100644 --- a/lib/private/AppFramework/Http/Request.php +++ b/lib/private/AppFramework/Http/Request.php @@ -569,6 +569,7 @@ public function getRemoteAddressAndProxyChain(): array { if (!is_array($trustedProxies) || count($trustedProxies) == 0 || $this->isTrustedProxy($trustedProxies, $remoteAddress)) { return ['remote_address' => $remoteAddress, 'proxies' => []]; } + $forwardedForHeaders = $this->config->getSystemValue('forwarded_for_headers', [ 'HTTP_X_FORWARDED_FOR', // de-facto standard to keep track of the proxy chain 'HTTP_FORWARDED', // new standard to keep track of the proxy chain diff --git a/tests/lib/AppFramework/Http/RequestTest.php b/tests/lib/AppFramework/Http/RequestTest.php index 34056ff7da251..2ccd6bf4feea9 100644 --- a/tests/lib/AppFramework/Http/RequestTest.php +++ b/tests/lib/AppFramework/Http/RequestTest.php @@ -1613,43 +1613,47 @@ public function testGetRequestUriWithoutOverwrite() { public function providesGetRequestUriWithOverwriteData() { return [ - ['/scriptname.php/some/PathInfo', '/owncloud/', ''], - ['/scriptname.php/some/PathInfo', '/owncloud/', '123', '123.123.123.123'], + ['/nextcloud/tests/lib/AppFramework/Http/RequestTest.php', '/nextcloud', '' , [] , '' , ''], + ['/nextcloud/tests/lib/AppFramework/Http/RequestTest.php', '/nextcloud', '123' , [] , '123.123.123.123', ''], + ['/nextcloud/tests/lib/AppFramework/Http/RequestTest.php', '/nextcloud', '123' , [] , '123.123.123.123', '123.123.123.123, 10.0.0.10, 172.16.0.2'], + ['/RequestTest.php', '/nextcloud', '123' , ['172.16.0.2'], '123.123.123.123', '123.123.123.123, 10.0.0.10, 172.16.0.2'], + ['/nextcloud/tests/lib/AppFramework/Http/RequestTest.php', '/nextcloud', '10\.0\.0\.10', ['172.16.0.2'], '123.123.123.123', '123.123.123.123, 10.0.0.10, 172.16.0.2'], + ['/RequestTest.php', '/nextcloud', '999' , ['172.16.0.2'], '123.123.123.123', '10.0.0.10, 172.16.0.2'], ]; } /** * @dataProvider providesGetRequestUriWithOverwriteData */ - public function testGetRequestUriWithOverwrite($expectedUri, $overwriteWebRoot, $overwriteCondAddr, $remoteAddr = '') { + public function testGetRequestUriWithOverwrite($expectedUri, $overwriteWebRoot, $overwriteCondAddr, $trustedProxies, $remoteAddr, $forwardedForHeader) { $this->config - ->expects($this->exactly(2)) ->method('getSystemValueString') ->willReturnMap([ ['overwritewebroot', '', $overwriteWebRoot], ['overwritecondaddr', '', $overwriteCondAddr], ]); + $this->config + ->method('getSystemValue') + ->willReturnMap([ + ['trusted_proxies', [], $trustedProxies], + ['forwarded_for_headers', ['HTTP_X_FORWARDED_FOR', 'HTTP_FORWARDED'], ['HTTP_X_FORWARDED_FOR']], + ]); - $request = $this->getMockBuilder(Request::class) - ->setMethods(['getScriptName']) - ->setConstructorArgs([ - [ - 'server' => [ - 'REQUEST_URI' => '/test.php/some/PathInfo', - 'SCRIPT_NAME' => '/test.php', - 'REMOTE_ADDR' => $remoteAddr - ] - ], - $this->requestId, - $this->config, - $this->csrfTokenManager, - $this->stream - ]) - ->getMock(); - $request - ->expects($this->once()) - ->method('getScriptName') - ->willReturn('/scriptname.php'); + $request = new Request( + [ + 'server' => [ + 'SCRIPT_FILENAME' => __FILE__, + 'SCRIPT_NAME' => __FILE__, + 'REQUEST_URI' => '/RequestTest.php', + 'REMOTE_ADDR' => $remoteAddr, + 'HTTP_X_FORWARDED_FOR' => $forwardedForHeader, + ] + ], + $this->requestId, + $this->config, + $this->csrfTokenManager, + $this->stream + ); $this->assertSame($expectedUri, $request->getRequestUri()); }