diff --git a/system/HTTP/IncomingRequest.php b/system/HTTP/IncomingRequest.php index f9a57a4098ba..604da567ea14 100644 --- a/system/HTTP/IncomingRequest.php +++ b/system/HTTP/IncomingRequest.php @@ -272,11 +272,31 @@ public function isSecure(): bool return true; } - if ($this->hasHeader('X-Forwarded-Proto') && $this->header('X-Forwarded-Proto')->getValue() === 'https') { - return true; + if ($this->hasHeader('X-Forwarded-Proto') || $this->hasHeader('Front-End-Https')) { + $isFromTrustedProxy = false; + $remoteAddr = $this->getServer('REMOTE_ADDR'); + + if ($remoteAddr !== null && is_array($this->config->proxyIPs)) { + foreach (array_keys($this->config->proxyIPs) as $proxyIP) { + if ($this->checkIPAgainstProxy($remoteAddr, (string) $proxyIP)) { + $isFromTrustedProxy = true; + break; + } + } + } + + if ($isFromTrustedProxy) { + if ($this->hasHeader('X-Forwarded-Proto') && $this->header('X-Forwarded-Proto')->getValue() === 'https') { + return true; + } + + if ($this->hasHeader('Front-End-Https') && ! empty($this->header('Front-End-Https')->getValue()) && strtolower($this->header('Front-End-Https')->getValue()) !== 'off') { + return true; + } + } } - return $this->hasHeader('Front-End-Https') && ! empty($this->header('Front-End-Https')->getValue()) && strtolower($this->header('Front-End-Https')->getValue()) !== 'off'; + return false; } /** diff --git a/system/HTTP/RequestTrait.php b/system/HTTP/RequestTrait.php index 6adfe3794935..95a5b7de49da 100644 --- a/system/HTTP/RequestTrait.php +++ b/system/HTTP/RequestTrait.php @@ -85,69 +85,8 @@ public function getIPAddress(): string return $this->ipAddress = '0.0.0.0'; } - // @TODO Extract all this IP address logic to another class. foreach ($proxyIPs as $proxyIP => $header) { - // Check if we have an IP address or a subnet - if (! str_contains($proxyIP, '/')) { - // An IP address (and not a subnet) is specified. - // We can compare right away. - if ($proxyIP === $this->ipAddress) { - $spoof = $this->getClientIP($header); - - if ($spoof !== null) { - $this->ipAddress = $spoof; - break; - } - } - - continue; - } - - // We have a subnet ... now the heavy lifting begins - if (! isset($separator)) { - $separator = $ipValidator($this->ipAddress, 'ipv6') ? ':' : '.'; - } - - // If the proxy entry doesn't match the IP protocol - skip it - if (! str_contains($proxyIP, $separator)) { - continue; - } - - // Convert the REMOTE_ADDR IP address to binary, if needed - if (! isset($ip, $sprintf)) { - if ($separator === ':') { - // Make sure we're having the "full" IPv6 format - $ip = explode(':', str_replace('::', str_repeat(':', 9 - substr_count($this->ipAddress, ':')), $this->ipAddress)); - - for ($j = 0; $j < 8; $j++) { - $ip[$j] = intval($ip[$j], 16); - } - - $sprintf = '%016b%016b%016b%016b%016b%016b%016b%016b'; - } else { - $ip = explode('.', $this->ipAddress); - $sprintf = '%08b%08b%08b%08b'; - } - - $ip = vsprintf($sprintf, $ip); - } - - // Split the netmask length off the network address - sscanf($proxyIP, '%[^/]/%d', $netaddr, $masklen); - - // Again, an IPv6 address is most likely in a compressed form - if ($separator === ':') { - $netaddr = explode(':', str_replace('::', str_repeat(':', 9 - substr_count($netaddr, ':')), $netaddr)); - - for ($i = 0; $i < 8; $i++) { - $netaddr[$i] = intval($netaddr[$i], 16); - } - } else { - $netaddr = explode('.', $netaddr); - } - - // Convert to binary and finally compare - if (strncmp($ip, vsprintf($sprintf, $netaddr), $masklen) === 0) { + if ($this->checkIPAgainstProxy($this->ipAddress, (string) $proxyIP)) { $spoof = $this->getClientIP($header); if ($spoof !== null) { @@ -164,6 +103,36 @@ public function getIPAddress(): string return $this->ipAddress; } + /** + * Checks if the given IP address matches the proxy IP/subnet. + */ + protected function checkIPAgainstProxy(string $ipAddress, string $proxyIP): bool + { + if (! str_contains($proxyIP, '/')) { + return $proxyIP === $ipAddress; + } + + [$netAddr, $maskLen] = explode('/', $proxyIP, 2); + + if (str_contains($ipAddress, ':') !== str_contains($netAddr, ':')) { + return false; + } + + $ipPacked = inet_pton($ipAddress); + $netPacked = inet_pton($netAddr); + + if ($ipPacked === false || $netPacked === false) { + return false; + } + + $toBits = static fn (string $packed): string => implode('', array_map( + static fn ($byte): string => str_pad(decbin(ord($byte)), 8, '0', STR_PAD_LEFT), + str_split($packed), + )); + + return strncmp($toBits($ipPacked), $toBits($netPacked), (int) $maskLen) === 0; + } + /** * Gets the client IP address from the HTTP header. */ diff --git a/tests/system/HTTP/IncomingRequestTest.php b/tests/system/HTTP/IncomingRequestTest.php index 509cf69643d2..913f122958f0 100644 --- a/tests/system/HTTP/IncomingRequestTest.php +++ b/tests/system/HTTP/IncomingRequestTest.php @@ -750,24 +750,176 @@ public function testIsAJAX(): void $this->assertTrue($this->request->isAJAX()); } - public function testIsSecure(): void + public function testIsSecureWithHttps(): void { service('superglobals')->setServer('HTTPS', 'on'); + $this->assertTrue($this->request->isSecure()); + $this->request->isSecure(); + $this->request->isSecure(); } - public function testIsSecureFrontEnd(): void + public function testIsSecureWithFrontEndHttps(): void { + service('superglobals')->setServer('REMOTE_ADDR', '10.0.1.200'); + $config = new App(); + $config->proxyIPs = ['10.0.1.200' => 'Front-End-Https']; + $this->request = $this->createRequest($config); + $this->request->appendHeader('Front-End-Https', 'on'); + $this->assertTrue($this->request->isSecure()); } - public function testIsSecureForwarded(): void + public function testIsSecureWithXForwardedProto(): void { + service('superglobals')->setServer('REMOTE_ADDR', '10.0.1.200'); + $config = new App(); + $config->proxyIPs = ['10.0.1.200' => 'X-Forwarded-Proto']; + $this->request = $this->createRequest($config); + $this->request->appendHeader('X-Forwarded-Proto', 'https'); + $this->assertTrue($this->request->isSecure()); } + /** + * @param array $server + * @param array $proxyIPs + * @param array $headers + */ + #[DataProvider('provideIsSecure')] + public function testIsSecure(array $server, array $proxyIPs, array $headers, bool $expected): void + { + $superglobals = new Superglobals(); + + foreach ($server as $key => $value) { + $superglobals->setServer($key, $value); + } + Services::injectMock('superglobals', $superglobals); + + $config = new App(); + $config->proxyIPs = $proxyIPs; + $request = $this->createRequest($config); + + foreach ($headers as $name => $value) { + $request->appendHeader($name, $value); + } + + $this->assertSame($expected, $request->isSecure()); + } + + /** + * @return iterable, proxyIPs: array, headers: array, expected: bool}> + */ + public static function provideIsSecure(): iterable + { + yield from [ + 'HTTPS on' => [ + 'server' => ['HTTPS' => 'on'], + 'proxyIPs' => [], + 'headers' => [], + 'expected' => true, + ], + 'HTTPS ON case insensitive' => [ + 'server' => ['HTTPS' => 'ON'], + 'proxyIPs' => [], + 'headers' => [], + 'expected' => true, + ], + 'HTTPS 1' => [ + 'server' => ['HTTPS' => '1'], + 'proxyIPs' => [], + 'headers' => [], + 'expected' => true, + ], + 'HTTPS off' => [ + 'server' => ['HTTPS' => 'off'], + 'proxyIPs' => [], + 'headers' => [], + 'expected' => false, + ], + 'HTTPS not set' => [ + 'server' => [], + 'proxyIPs' => [], + 'headers' => [], + 'expected' => false, + ], + 'Front-End-Https on with trusted proxy' => [ + 'server' => ['REMOTE_ADDR' => '10.0.1.200'], + 'proxyIPs' => ['10.0.1.200' => 'Front-End-Https'], + 'headers' => ['Front-End-Https' => 'on'], + 'expected' => true, + ], + 'Front-End-Https off with trusted proxy' => [ + 'server' => ['REMOTE_ADDR' => '10.0.1.200'], + 'proxyIPs' => ['10.0.1.200' => 'Front-End-Https'], + 'headers' => ['Front-End-Https' => 'off'], + 'expected' => false, + ], + 'Front-End-Https on with untrusted proxy' => [ + 'server' => ['REMOTE_ADDR' => '10.0.1.201'], + 'proxyIPs' => ['10.0.1.200' => 'Front-End-Https'], + 'headers' => ['Front-End-Https' => 'on'], + 'expected' => false, + ], + 'X-Forwarded-Proto https with trusted proxy' => [ + 'server' => ['REMOTE_ADDR' => '10.0.1.200'], + 'proxyIPs' => ['10.0.1.200' => 'X-Forwarded-Proto'], + 'headers' => ['X-Forwarded-Proto' => 'https'], + 'expected' => true, + ], + 'X-Forwarded-Proto http with trusted proxy' => [ + 'server' => ['REMOTE_ADDR' => '10.0.1.200'], + 'proxyIPs' => ['10.0.1.200' => 'X-Forwarded-Proto'], + 'headers' => ['X-Forwarded-Proto' => 'http'], + 'expected' => false, + ], + 'X-Forwarded-Proto https with untrusted proxy' => [ + 'server' => ['REMOTE_ADDR' => '10.0.1.201'], + 'proxyIPs' => ['10.0.1.200' => 'X-Forwarded-Proto'], + 'headers' => ['X-Forwarded-Proto' => 'https'], + 'expected' => false, + ], + 'Front-End-Https on with trusted proxy subnet IPv4' => [ + 'server' => ['REMOTE_ADDR' => '192.168.5.25'], + 'proxyIPs' => ['192.168.5.0/24' => 'Front-End-Https'], + 'headers' => ['Front-End-Https' => 'on'], + 'expected' => true, + ], + 'Front-End-Https on with untrusted proxy subnet IPv4' => [ + 'server' => ['REMOTE_ADDR' => '192.168.6.25'], + 'proxyIPs' => ['192.168.5.0/24' => 'Front-End-Https'], + 'headers' => ['Front-End-Https' => 'on'], + 'expected' => false, + ], + 'X-Forwarded-Proto https with trusted proxy subnet IPv6' => [ + 'server' => ['REMOTE_ADDR' => '2001:db8:1234::1'], + 'proxyIPs' => ['2001:db8:1234::/48' => 'X-Forwarded-Proto'], + 'headers' => ['X-Forwarded-Proto' => 'https'], + 'expected' => true, + ], + 'X-Forwarded-Proto https with untrusted proxy subnet IPv6' => [ + 'server' => ['REMOTE_ADDR' => '2001:db8:1235::1'], + 'proxyIPs' => ['2001:db8:1234::/48' => 'X-Forwarded-Proto'], + 'headers' => ['X-Forwarded-Proto' => 'https'], + 'expected' => false, + ], + 'IPv4 client IP against IPv6 proxy subnet' => [ + 'server' => ['REMOTE_ADDR' => '192.168.5.25'], + 'proxyIPs' => ['2001:db8:1234::/48' => 'X-Forwarded-Proto'], + 'headers' => ['X-Forwarded-Proto' => 'https'], + 'expected' => false, + ], + 'IPv6 client IP against IPv4 proxy subnet' => [ + 'server' => ['REMOTE_ADDR' => '2001:db8:1234::1'], + 'proxyIPs' => ['192.168.5.0/24' => 'X-Forwarded-Proto'], + 'headers' => ['X-Forwarded-Proto' => 'https'], + 'expected' => false, + ], + ]; + } + public function testUserAgent(): void { service('superglobals')->setServer('HTTP_USER_AGENT', 'Mozilla');