From bd9aff47b69d62b42dd3e450ea76bb1616dbec58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 27 Jun 2022 16:48:58 +0200 Subject: [PATCH 1/4] Improve local IP detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/private/Http/Client/LocalAddressChecker.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/private/Http/Client/LocalAddressChecker.php b/lib/private/Http/Client/LocalAddressChecker.php index c69d1007a160e..b233f34b19c63 100644 --- a/lib/private/Http/Client/LocalAddressChecker.php +++ b/lib/private/Http/Client/LocalAddressChecker.php @@ -41,6 +41,12 @@ public function ThrowIfLocalIp(string $ip) : void { throw new LocalServerException('Host violates local access rules'); } + $localIps = ['100.100.100.200']; + if ((bool)filter_var($ip, FILTER_VALIDATE_IP) && in_array($ip, $localIps)) { + $this->logger->warning("Host $ip was not connected to because it violates local access rules"); + throw new LocalServerException('Host violates local access rules'); + } + // Also check for IPv6 IPv4 nesting, because that's not covered by filter_var if ((bool)filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6) && substr_count($ip, '.') > 0) { $delimiter = strrpos($ip, ':'); // Get last colon From d0830432a7ce4dece2af7a5ca3c0f3831dbd1291 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 28 Jun 2022 11:35:45 +0200 Subject: [PATCH 2/4] Refactor local IP if and set strict to true for in_array MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/private/Http/Client/LocalAddressChecker.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/private/Http/Client/LocalAddressChecker.php b/lib/private/Http/Client/LocalAddressChecker.php index b233f34b19c63..749a1a97c8acf 100644 --- a/lib/private/Http/Client/LocalAddressChecker.php +++ b/lib/private/Http/Client/LocalAddressChecker.php @@ -36,13 +36,13 @@ public function __construct(LoggerInterface $logger) { } public function ThrowIfLocalIp(string $ip) : void { - if ((bool)filter_var($ip, FILTER_VALIDATE_IP) && !filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE)) { - $this->logger->warning("Host $ip was not connected to because it violates local access rules"); - throw new LocalServerException('Host violates local access rules'); - } - $localIps = ['100.100.100.200']; - if ((bool)filter_var($ip, FILTER_VALIDATE_IP) && in_array($ip, $localIps)) { + if ( + (bool)filter_var($ip, FILTER_VALIDATE_IP) && + ( + !filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE) || + in_array($ip, $localIps, true) + )) { $this->logger->warning("Host $ip was not connected to because it violates local access rules"); throw new LocalServerException('Host violates local access rules'); } From 707b46bb01e67b764274fc00275e2076aeea5327 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 28 Jun 2022 16:48:38 +0200 Subject: [PATCH 3/4] Check for local IPs nested in IPv6 as well MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/private/Http/Client/LocalAddressChecker.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/private/Http/Client/LocalAddressChecker.php b/lib/private/Http/Client/LocalAddressChecker.php index 749a1a97c8acf..b0c420a4fe8ca 100644 --- a/lib/private/Http/Client/LocalAddressChecker.php +++ b/lib/private/Http/Client/LocalAddressChecker.php @@ -52,7 +52,9 @@ public function ThrowIfLocalIp(string $ip) : void { $delimiter = strrpos($ip, ':'); // Get last colon $ipv4Address = substr($ip, $delimiter + 1); - if (!filter_var($ipv4Address, FILTER_VALIDATE_IP, FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE)) { + if ( + !filter_var($ipv4Address, FILTER_VALIDATE_IP, FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE) || + in_array($ipv4Address, $localIps, true)) { $this->logger->warning("Host $ip was not connected to because it violates local access rules"); throw new LocalServerException('Host violates local access rules'); } From c5ffd7ce32a74c06dddd55652edea5c896ee9b3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 12 Jul 2022 12:09:05 +0200 Subject: [PATCH 4/4] Use Symfony IpUtils to check for local IP ranges MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/private/Http/Client/LocalAddressChecker.php | 10 +++++++--- tests/lib/Http/Client/LocalAddressCheckerTest.php | 5 +++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/lib/private/Http/Client/LocalAddressChecker.php b/lib/private/Http/Client/LocalAddressChecker.php index b0c420a4fe8ca..f4fea503ab9d5 100644 --- a/lib/private/Http/Client/LocalAddressChecker.php +++ b/lib/private/Http/Client/LocalAddressChecker.php @@ -27,6 +27,7 @@ use OCP\Http\Client\LocalServerException; use Psr\Log\LoggerInterface; +use Symfony\Component\HttpFoundation\IpUtils; class LocalAddressChecker { private LoggerInterface $logger; @@ -36,12 +37,15 @@ public function __construct(LoggerInterface $logger) { } public function ThrowIfLocalIp(string $ip) : void { - $localIps = ['100.100.100.200']; + $localRanges = [ + '100.64.0.0/10', // See RFC 6598 + '192.0.0.0/24', // See RFC 6890 + ]; if ( (bool)filter_var($ip, FILTER_VALIDATE_IP) && ( !filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE) || - in_array($ip, $localIps, true) + IpUtils::checkIp($ip, $localRanges) )) { $this->logger->warning("Host $ip was not connected to because it violates local access rules"); throw new LocalServerException('Host violates local access rules'); @@ -54,7 +58,7 @@ public function ThrowIfLocalIp(string $ip) : void { if ( !filter_var($ipv4Address, FILTER_VALIDATE_IP, FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE) || - in_array($ipv4Address, $localIps, true)) { + IpUtils::checkIp($ip, $localRanges)) { $this->logger->warning("Host $ip was not connected to because it violates local access rules"); throw new LocalServerException('Host violates local access rules'); } diff --git a/tests/lib/Http/Client/LocalAddressCheckerTest.php b/tests/lib/Http/Client/LocalAddressCheckerTest.php index 0bba1cee5f4a2..9f2f6c72993e3 100644 --- a/tests/lib/Http/Client/LocalAddressCheckerTest.php +++ b/tests/lib/Http/Client/LocalAddressCheckerTest.php @@ -96,6 +96,8 @@ public function dataInternalIPs() : array { ['10.0.0.1'], ['::'], ['::1'], + ['100.100.100.200'], + ['192.0.0.1'], ]; } @@ -116,6 +118,9 @@ public function dataPreventLocalAddress():array { ['another-host.local'], ['service.localhost'], ['!@#$'], // test invalid url + ['100.100.100.200'], + ['192.0.0.1'], + ['randomdomain.internal'], ]; }