[HttpClient] Fix checking for private IPs before connecting

nicolas-grekas · nicolas-grekas · commit 63a12783b8b3 · 2024-11-27T12:19:14.000+01:00
diff --git a/NativeHttpClient.php b/NativeHttpClient.php
@@ -141,22 +141,13 @@ public function request(string $method, string $url, array $options = []): Respo
             // Memoize the last progress to ease calling the callback periodically when no network transfer happens
             $lastProgress = [0, 0];
             $maxDuration = 0 < $options['max_duration'] ? $options['max_duration'] : \INF;
-            $multi = $this->multi;
-            $resolve = static function (string $host, ?string $ip = null) use ($multi): ?string {
-                if (null !== $ip) {
-                    $multi->dnsCache[$host] = $ip;
-                }
-
-                return $multi->dnsCache[$host] ?? null;
-            };
-            $onProgress = static function (...$progress) use ($onProgress, &$lastProgress, &$info, $maxDuration, $resolve) {
+            $onProgress = static function (...$progress) use ($onProgress, &$lastProgress, &$info, $maxDuration) {
                 if ($info['total_time'] >= $maxDuration) {
                     throw new TransportException(sprintf('Max duration was reached for "%s".', implode('', $info['url'])));
                 }
 
                 $progressInfo = $info;
                 $progressInfo['url'] = implode('', $info['url']);
-                $progressInfo['resolve'] = $resolve;
                 unset($progressInfo['size_body']);
 
                 if ($progress && -1 === $progress[0]) {
diff --git a/NoPrivateNetworkHttpClient.php b/NoPrivateNetworkHttpClient.php
@@ -13,9 +13,11 @@
 
 use Psr\Log\LoggerAwareInterface;
 use Psr\Log\LoggerInterface;
-use Symfony\Component\HttpClient\Exception\InvalidArgumentException;
 use Symfony\Component\HttpClient\Exception\TransportException;
+use Symfony\Component\HttpClient\Response\AsyncContext;
+use Symfony\Component\HttpClient\Response\AsyncResponse;
 use Symfony\Component\HttpFoundation\IpUtils;
+use Symfony\Contracts\HttpClient\ChunkInterface;
 use Symfony\Contracts\HttpClient\HttpClientInterface;
 use Symfony\Contracts\HttpClient\ResponseInterface;
 use Symfony\Contracts\HttpClient\ResponseStreamInterface;
@@ -25,10 +27,12 @@
  * Decorator that blocks requests to private networks by default.
  *
  * @author Hallison Boaventura <hallisonboaventura@gmail.com>
+ * @author Nicolas Grekas <p@tchwork.com>
  */
 final class NoPrivateNetworkHttpClient implements HttpClientInterface, LoggerAwareInterface, ResetInterface
 {
     use HttpClientTrait;
+    use AsyncDecoratorTrait;
 
     private const PRIVATE_SUBNETS = [
         '127.0.0.0/8',
@@ -45,11 +49,14 @@ final class NoPrivateNetworkHttpClient implements HttpClientInterface, LoggerAwa
         '::/128',
     ];
 
+    private $defaultOptions = self::OPTIONS_DEFAULTS;
     private $client;
     private $subnets;
+    private $ipFlags;
+    private $dnsCache;
 
     /**
-     * @param string|array|null $subnets String or array of subnets using CIDR notation that will be used by IpUtils.
+     * @param string|array|null $subnets String or array of subnets using CIDR notation that should be considered private.
      *                                   If null is passed, the standard private subnets will be used.
      */
     public function __construct(HttpClientInterface $client, $subnets = null)
@@ -62,56 +69,113 @@ public function __construct(HttpClientInterface $client, $subnets = null)
             throw new \LogicException(sprintf('You cannot use "%s" if the HttpFoundation component is not installed. Try running "composer require symfony/http-foundation".', __CLASS__));
         }
 
+        if (null === $subnets) {
+            $ipFlags = \FILTER_FLAG_IPV4 | \FILTER_FLAG_IPV6;
+        } else {
+            $ipFlags = 0;
+            foreach ((array) $subnets as $subnet) {
+                $ipFlags |= str_contains($subnet, ':') ? \FILTER_FLAG_IPV6 : \FILTER_FLAG_IPV4;
+            }
+        }
+
+        if (!\defined('STREAM_PF_INET6')) {
+            $ipFlags &= ~\FILTER_FLAG_IPV6;
+        }
+
         $this->client = $client;
-        $this->subnets = $subnets;
+        $this->subnets = null !== $subnets ? (array) $subnets : null;
+        $this->ipFlags = $ipFlags;
+        $this->dnsCache = new \ArrayObject();
     }
 
     /**
      * {@inheritdoc}
      */
     public function request(string $method, string $url, array $options = []): ResponseInterface
     {
-        $onProgress = $options['on_progress'] ?? null;
-        if (null !== $onProgress && !\is_callable($onProgress)) {
-            throw new InvalidArgumentException(sprintf('Option "on_progress" must be callable, "%s" given.', get_debug_type($onProgress)));
-        }
+        [$url, $options] = self::prepareRequest($method, $url, $options, $this->defaultOptions, true);
 
-        $subnets = $this->subnets;
-        $lastUrl = '';
-        $lastPrimaryIp = '';
+        $redirectHeaders = parse_url($url['authority']);
+        $host = $redirectHeaders['host'];
+        $url = implode('', $url);
+        $dnsCache = $this->dnsCache;
 
-        $options['on_progress'] = function (int $dlNow, int $dlSize, array $info) use ($onProgress, $subnets, &$lastUrl, &$lastPrimaryIp): void {
-            if ($info['url'] !== $lastUrl) {
-                $host = parse_url($info['url'], PHP_URL_HOST) ?: '';
-                $resolve = $info['resolve'] ?? static function () { return null; };
-
-                if (($ip = trim($host, '[]'))
-                    && !filter_var($ip, \FILTER_VALIDATE_IP)
-                    && !($ip = $resolve($host))
-                    && $ip = @(gethostbynamel($host)[0] ?? dns_get_record($host, \DNS_AAAA)[0]['ipv6'] ?? null)
-                ) {
-                    $resolve($host, $ip);
-                }
+        $ip = self::dnsResolve($dnsCache, $host, $this->ipFlags, $options);
+        self::ipCheck($ip, $this->subnets, $this->ipFlags, $host, $url);
 
-                if ($ip && IpUtils::checkIp($ip, $subnets ?? self::PRIVATE_SUBNETS)) {
-                    throw new TransportException(sprintf('Host "%s" is blocked for "%s".', $host, $info['url']));
-                }
+        if (0 < $maxRedirects = $options['max_redirects']) {
+            $options['max_redirects'] = 0;
+            $redirectHeaders['with_auth'] = $redirectHeaders['no_auth'] = $options['headers'];
 
-                $lastUrl = $info['url'];
+            if (isset($options['normalized_headers']['host']) || isset($options['normalized_headers']['authorization']) || isset($options['normalized_headers']['cookie'])) {
+                $redirectHeaders['no_auth'] = array_filter($redirectHeaders['no_auth'], static function ($h) {
+                    return 0 !== stripos($h, 'Host:') && 0 !== stripos($h, 'Authorization:') && 0 !== stripos($h, 'Cookie:');
+                });
             }
+        }
 
-            if ($info['primary_ip'] !== $lastPrimaryIp) {
-                if ($info['primary_ip'] && IpUtils::checkIp($info['primary_ip'], $subnets ?? self::PRIVATE_SUBNETS)) {
-                    throw new TransportException(sprintf('IP "%s" is blocked for "%s".', $info['primary_ip'], $info['url']));
-                }
+        $onProgress = $options['on_progress'] ?? null;
+        $subnets = $this->subnets;
+        $ipFlags = $this->ipFlags;
+        $lastPrimaryIp = '';
 
+        $options['on_progress'] = static function (int $dlNow, int $dlSize, array $info) use ($onProgress, $subnets, $ipFlags, &$lastPrimaryIp): void {
+            if (($info['primary_ip'] ?? '') !== $lastPrimaryIp) {
+                self::ipCheck($info['primary_ip'], $subnets, $ipFlags, null, $info['url']);
                 $lastPrimaryIp = $info['primary_ip'];
             }
 
             null !== $onProgress && $onProgress($dlNow, $dlSize, $info);
         };
 
-        return $this->client->request($method, $url, $options);
+        return new AsyncResponse($this->client, $method, $url, $options, static function (ChunkInterface $chunk, AsyncContext $context) use (&$method, &$options, $maxRedirects, &$redirectHeaders, $subnets, $ipFlags, $dnsCache): \Generator {
+            if (null !== $chunk->getError() || $chunk->isTimeout() || !$chunk->isFirst()) {
+                yield $chunk;
+
+                return;
+            }
+
+            $statusCode = $context->getStatusCode();
+
+            if ($statusCode < 300 || 400 <= $statusCode || null === $url = $context->getInfo('redirect_url')) {
+                $context->passthru();
+
+                yield $chunk;
+
+                return;
+            }
+
+            $host = parse_url($url, \PHP_URL_HOST);
+            $ip = self::dnsResolve($dnsCache, $host, $ipFlags, $options);
+            self::ipCheck($ip, $subnets, $ipFlags, $host, $url);
+
+            // Do like curl and browsers: turn POST to GET on 301, 302 and 303
+            if (303 === $statusCode || 'POST' === $method && \in_array($statusCode, [301, 302], true)) {
+                $method = 'HEAD' === $method ? 'HEAD' : 'GET';
+                unset($options['body'], $options['json']);
+
+                if (isset($options['normalized_headers']['content-length']) || isset($options['normalized_headers']['content-type']) || isset($options['normalized_headers']['transfer-encoding'])) {
+                    $filterContentHeaders = static function ($h) {
+                        return 0 !== stripos($h, 'Content-Length:') && 0 !== stripos($h, 'Content-Type:') && 0 !== stripos($h, 'Transfer-Encoding:');
+                    };
+                    $options['header'] = array_filter($options['header'], $filterContentHeaders);
+                    $redirectHeaders['no_auth'] = array_filter($redirectHeaders['no_auth'], $filterContentHeaders);
+                    $redirectHeaders['with_auth'] = array_filter($redirectHeaders['with_auth'], $filterContentHeaders);
+                }
+            }
+
+            // Authorization and Cookie headers MUST NOT follow except for the initial host name
+            $options['headers'] = $redirectHeaders['host'] === $host ? $redirectHeaders['with_auth'] : $redirectHeaders['no_auth'];
+
+            static $redirectCount = 0;
+            $context->setInfo('redirect_count', ++$redirectCount);
+
+            $context->replaceRequest($method, $url, $options);
+
+            if ($redirectCount >= $maxRedirects) {
+                $context->passthru();
+            }
+        });
     }
 
     /**
@@ -139,14 +203,73 @@ public function withOptions(array $options): self
     {
         $clone = clone $this;
         $clone->client = $this->client->withOptions($options);
+        $clone->defaultOptions = self::mergeDefaultOptions($options, $this->defaultOptions);
 
         return $clone;
     }
 
     public function reset()
     {
+        $this->dnsCache->exchangeArray([]);
+
         if ($this->client instanceof ResetInterface) {
             $this->client->reset();
         }
     }
+
+    private static function dnsResolve(\ArrayObject $dnsCache, string $host, int $ipFlags, array &$options): string
+    {
+        if ($ip = filter_var(trim($host, '[]'), \FILTER_VALIDATE_IP) ?: $options['resolve'][$host] ?? false) {
+            return $ip;
+        }
+
+        if ($dnsCache->offsetExists($host)) {
+            return $dnsCache[$host];
+        }
+
+        if ((\FILTER_FLAG_IPV4 & $ipFlags) && $ip = gethostbynamel($host)) {
+            return $options['resolve'][$host] = $dnsCache[$host] = $ip[0];
+        }
+
+        if (!(\FILTER_FLAG_IPV6 & $ipFlags)) {
+            return $host;
+        }
+
+        if ($ip = dns_get_record($host, \DNS_AAAA)) {
+            $ip = $ip[0]['ipv6'];
+        } elseif (extension_loaded('sockets')) {
+            if (!$info = socket_addrinfo_lookup($host, 0, ['ai_socktype' => \SOCK_STREAM, 'ai_family' => \AF_INET6])) {
+                return $host;
+            }
+
+            $ip = socket_addrinfo_explain($info[0])['ai_addr']['sin6_addr'];
+        } elseif ('localhost' === $host || 'localhost.' === $host) {
+            $ip = '::1';
+        } else {
+            return $host;
+        }
+
+        return $options['resolve'][$host] = $dnsCache[$host] = $ip;
+    }
+
+    private static function ipCheck(string $ip, ?array $subnets, int $ipFlags, ?string $host, string $url): void
+    {
+        if (null === $subnets) {
+            // Quick check, but not reliable enough, see https://github.com/php/php-src/issues/16944
+            $ipFlags |= \FILTER_FLAG_NO_PRIV_RANGE | \FILTER_FLAG_NO_RES_RANGE;
+        }
+
+        if (false !== filter_var($ip, \FILTER_VALIDATE_IP, $ipFlags) && !IpUtils::checkIp($ip, $subnets ?? self::PRIVATE_SUBNETS)) {
+            return;
+        }
+
+        if (null !== $host) {
+            $type = 'Host';
+        } else {
+            $host = $ip;
+            $type = 'IP';
+        }
+
+        throw new TransportException($type.\sprintf(' "%s" is blocked for "%s".', $host, $url));
+    }
 }
diff --git a/Response/AmpResponse.php b/Response/AmpResponse.php
@@ -89,17 +89,9 @@ public function __construct(AmpClientState $multi, Request $request, array $opti
         $info['max_duration'] = $options['max_duration'];
         $info['debug'] = '';
 
-        $resolve = static function (string $host, ?string $ip = null) use ($multi): ?string {
-            if (null !== $ip) {
-                $multi->dnsCache[$host] = $ip;
-            }
-
-            return $multi->dnsCache[$host] ?? null;
-        };
         $onProgress = $options['on_progress'] ?? static function () {};
-        $onProgress = $this->onProgress = static function () use (&$info, $onProgress, $resolve) {
+        $onProgress = $this->onProgress = static function () use (&$info, $onProgress) {
             $info['total_time'] = microtime(true) - $info['start_time'];
-            $info['resolve'] = $resolve;
             $onProgress((int) $info['size_download'], ((int) (1 + $info['download_content_length']) ?: 1) - 1, (array) $info);
         };
 
diff --git a/Response/CurlResponse.php b/Response/CurlResponse.php
@@ -115,20 +115,13 @@ public function __construct(CurlClientState $multi, $ch, ?array $options = null,
         curl_pause($ch, \CURLPAUSE_CONT);
 
         if ($onProgress = $options['on_progress']) {
-            $resolve = static function (string $host, ?string $ip = null) use ($multi): ?string {
-                if (null !== $ip) {
-                    $multi->dnsCache->hostnames[$host] = $ip;
-                }
-
-                return $multi->dnsCache->hostnames[$host] ?? null;
-            };
             $url = isset($info['url']) ? ['url' => $info['url']] : [];
             curl_setopt($ch, \CURLOPT_NOPROGRESS, false);
-            curl_setopt($ch, \CURLOPT_PROGRESSFUNCTION, static function ($ch, $dlSize, $dlNow) use ($onProgress, &$info, $url, $multi, $debugBuffer, $resolve) {
+            curl_setopt($ch, \CURLOPT_PROGRESSFUNCTION, static function ($ch, $dlSize, $dlNow) use ($onProgress, &$info, $url, $multi, $debugBuffer) {
                 try {
                     rewind($debugBuffer);
                     $debug = ['debug' => stream_get_contents($debugBuffer)];
-                    $onProgress($dlNow, $dlSize, $url + curl_getinfo($ch) + $info + $debug + ['resolve' => $resolve]);
+                    $onProgress($dlNow, $dlSize, $url + curl_getinfo($ch) + $info + $debug);
                 } catch (\Throwable $e) {
                     $multi->handlesActivity[(int) $ch][] = null;
                     $multi->handlesActivity[(int) $ch][] = $e;
diff --git a/Tests/AmpHttpClientTest.php b/Tests/AmpHttpClientTest.php
@@ -14,6 +14,9 @@
 use Symfony\Component\HttpClient\AmpHttpClient;
 use Symfony\Contracts\HttpClient\HttpClientInterface;
 
+/**
+ * @group dns-sensitive
+ */
 class AmpHttpClientTest extends HttpClientTestCase
 {
     protected function getHttpClient(string $testCase): HttpClientInterface
diff --git a/Tests/CurlHttpClientTest.php b/Tests/CurlHttpClientTest.php
@@ -17,6 +17,7 @@
 
 /**
  * @requires extension curl
+ * @group dns-sensitive
  */
 class CurlHttpClientTest extends HttpClientTestCase
 {
diff --git a/Tests/HttpClientTestCase.php b/Tests/HttpClientTestCase.php
@@ -12,6 +12,7 @@
 namespace Symfony\Component\HttpClient\Tests;
 
 use PHPUnit\Framework\SkippedTestSuiteError;
+use Symfony\Bridge\PhpUnit\DnsMock;
 use Symfony\Component\HttpClient\Exception\ClientException;
 use Symfony\Component\HttpClient\Exception\InvalidArgumentException;
 use Symfony\Component\HttpClient\Exception\TransportException;
@@ -490,6 +491,38 @@ public function testNoPrivateNetworkWithResolve()
         $client->request('GET', 'http://symfony.com', ['resolve' => ['symfony.com' => '127.0.0.1']]);
     }
 
+    public function testNoPrivateNetworkWithResolveAndRedirect()
+    {
+        DnsMock::withMockedHosts([
+            'localhost' => [
+                [
+                    'host' => 'localhost',
+                    'class' => 'IN',
+                    'ttl' => 15,
+                    'type' => 'A',
+                    'ip' => '127.0.0.1',
+                ],
+            ],
+            'symfony.com' => [
+                [
+                    'host' => 'symfony.com',
+                    'class' => 'IN',
+                    'ttl' => 15,
+                    'type' => 'A',
+                    'ip' => '10.0.0.1',
+                ],
+            ],
+        ]);
+
+        $client = $this->getHttpClient(__FUNCTION__);
+        $client = new NoPrivateNetworkHttpClient($client, '10.0.0.1/32');
+
+        $this->expectException(TransportException::class);
+        $this->expectExceptionMessage('Host "symfony.com" is blocked');
+
+        $client->request('GET', 'http://localhost:8057/302?location=https://symfony.com/');
+    }
+
     public function testNoRedirectWithInvalidLocation()
     {
         $client = $this->getHttpClient(__FUNCTION__);
diff --git a/Tests/NativeHttpClientTest.php b/Tests/NativeHttpClientTest.php
@@ -14,6 +14,9 @@
 use Symfony\Component\HttpClient\NativeHttpClient;
 use Symfony\Contracts\HttpClient\HttpClientInterface;
 
+/**
+ * @group dns-sensitive
+ */
 class NativeHttpClientTest extends HttpClientTestCase
 {
     protected function getHttpClient(string $testCase): HttpClientInterface
diff --git a/Tests/NoPrivateNetworkHttpClientTest.php b/Tests/NoPrivateNetworkHttpClientTest.php

Original file line number	Diff line number	Diff line change
`@@ -14,6 +14,9 @@`
`14`	`14`	`use Symfony\Component\HttpClient\AmpHttpClient;`
`15`	`15`	`use Symfony\Contracts\HttpClient\HttpClientInterface;`
`16`	`16`
	`17`	`+/**`
	`18`	`+ * @group dns-sensitive`
	`19`	`+ */`
`17`	`20`	`class AmpHttpClientTest extends HttpClientTestCase`
`18`	`21`	`{`
`19`	`22`	`protected function getHttpClient(string $testCase): HttpClientInterface`
Original file line number	Diff line number	Diff line change
`@@ -17,6 +17,7 @@`
`17`	`17`
`18`	`18`	`/**`
`19`	`19`	`* @requires extension curl`
	`20`	`+ * @group dns-sensitive`
`20`	`21`	`*/`
`21`	`22`	`class CurlHttpClientTest extends HttpClientTestCase`
`22`	`23`	`{`