Skip to content

Commit 60a1136

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '5.4' into 6.4
* 5.4: [HttpClient] Fix checking for private IPs before connecting
2 parents 7aed35f + 63a1278 commit 60a1136

9 files changed

+247
-76
lines changed

‎NativeHttpClient.php

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -140,22 +140,13 @@ public function request(string $method, string $url, array $options = []): Respo
140140

141141
if ($onProgress = $options['on_progress']) {
142142
$maxDuration = 0 < $options['max_duration'] ? $options['max_duration'] : \INF;
143-
$multi = $this->multi;
144-
$resolve = static function (string $host, ?string $ip = null) use ($multi): ?string {
145-
if (null !== $ip) {
146-
$multi->dnsCache[$host] = $ip;
147-
}
148-
149-
return $multi->dnsCache[$host] ?? null;
150-
};
151-
$onProgress = static function (...$progress) use ($onProgress, &$info, $maxDuration, $resolve) {
143+
$onProgress = static function (...$progress) use ($onProgress, &$info, $maxDuration) {
152144
if ($info['total_time'] >= $maxDuration) {
153145
throw new TransportException(sprintf('Max duration was reached for "%s".', implode('', $info['url'])));
154146
}
155147

156148
$progressInfo = $info;
157149
$progressInfo['url'] = implode('', $info['url']);
158-
$progressInfo['resolve'] = $resolve;
159150
unset($progressInfo['size_body']);
160151

161152
// Memoize the last progress to ease calling the callback periodically when no network transfer happens

‎NoPrivateNetworkHttpClient.php

Lines changed: 155 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,11 @@
1313

1414
use Psr\Log\LoggerAwareInterface;
1515
use Psr\Log\LoggerInterface;
16-
use Symfony\Component\HttpClient\Exception\InvalidArgumentException;
1716
use Symfony\Component\HttpClient\Exception\TransportException;
17+
use Symfony\Component\HttpClient\Response\AsyncContext;
18+
use Symfony\Component\HttpClient\Response\AsyncResponse;
1819
use Symfony\Component\HttpFoundation\IpUtils;
20+
use Symfony\Contracts\HttpClient\ChunkInterface;
1921
use Symfony\Contracts\HttpClient\HttpClientInterface;
2022
use Symfony\Contracts\HttpClient\ResponseInterface;
2123
use Symfony\Contracts\HttpClient\ResponseStreamInterface;
@@ -25,16 +27,21 @@
2527
* Decorator that blocks requests to private networks by default.
2628
*
2729
* @author Hallison Boaventura <[email protected]>
30+
* @author Nicolas Grekas <[email protected]>
2831
*/
2932
final class NoPrivateNetworkHttpClient implements HttpClientInterface, LoggerAwareInterface, ResetInterface
3033
{
3134
use HttpClientTrait;
35+
use AsyncDecoratorTrait;
3236

37+
private array $defaultOptions = self::OPTIONS_DEFAULTS;
3338
private HttpClientInterface $client;
34-
private string|array|null $subnets;
39+
private array|null $subnets;
40+
private int $ipFlags;
41+
private \ArrayObject $dnsCache;
3542

3643
/**
37-
* @param string|array|null $subnets String or array of subnets using CIDR notation that will be used by IpUtils.
44+
* @param string|array|null $subnets String or array of subnets using CIDR notation that should be considered private.
3845
* If null is passed, the standard private subnets will be used.
3946
*/
4047
public function __construct(HttpClientInterface $client, string|array|null $subnets = null)
@@ -43,54 +50,112 @@ public function __construct(HttpClientInterface $client, string|array|null $subn
4350
throw new \LogicException(sprintf('You cannot use "%s" if the HttpFoundation component is not installed. Try running "composer require symfony/http-foundation".', __CLASS__));
4451
}
4552

53+
if (null === $subnets) {
54+
$ipFlags = \FILTER_FLAG_IPV4 | \FILTER_FLAG_IPV6;
55+
} else {
56+
$ipFlags = 0;
57+
foreach ((array) $subnets as $subnet) {
58+
$ipFlags |= str_contains($subnet, ':') ? \FILTER_FLAG_IPV6 : \FILTER_FLAG_IPV4;
59+
}
60+
}
61+
62+
if (!\defined('STREAM_PF_INET6')) {
63+
$ipFlags &= ~\FILTER_FLAG_IPV6;
64+
}
65+
4666
$this->client = $client;
47-
$this->subnets = $subnets;
67+
$this->subnets = null !== $subnets ? (array) $subnets : null;
68+
$this->ipFlags = $ipFlags;
69+
$this->dnsCache = new \ArrayObject();
4870
}
4971

5072
public function request(string $method, string $url, array $options = []): ResponseInterface
5173
{
52-
$onProgress = $options['on_progress'] ?? null;
53-
if (null !== $onProgress && !\is_callable($onProgress)) {
54-
throw new InvalidArgumentException(sprintf('Option "on_progress" must be callable, "%s" given.', get_debug_type($onProgress)));
74+
[$url, $options] = self::prepareRequest($method, $url, $options, $this->defaultOptions, true);
75+
76+
$redirectHeaders = parse_url($url['authority']);
77+
$host = $redirectHeaders['host'];
78+
$url = implode('', $url);
79+
$dnsCache = $this->dnsCache;
80+
81+
$ip = self::dnsResolve($dnsCache, $host, $this->ipFlags, $options);
82+
self::ipCheck($ip, $this->subnets, $this->ipFlags, $host, $url);
83+
84+
if (0 < $maxRedirects = $options['max_redirects']) {
85+
$options['max_redirects'] = 0;
86+
$redirectHeaders['with_auth'] = $redirectHeaders['no_auth'] = $options['headers'];
87+
88+
if (isset($options['normalized_headers']['host']) || isset($options['normalized_headers']['authorization']) || isset($options['normalized_headers']['cookie'])) {
89+
$redirectHeaders['no_auth'] = array_filter($redirectHeaders['no_auth'], static function ($h) {
90+
return 0 !== stripos($h, 'Host:') && 0 !== stripos($h, 'Authorization:') && 0 !== stripos($h, 'Cookie:');
91+
});
92+
}
5593
}
5694

95+
$onProgress = $options['on_progress'] ?? null;
5796
$subnets = $this->subnets;
97+
$ipFlags = $this->ipFlags;
5898

59-
$options['on_progress'] = static function (int $dlNow, int $dlSize, array $info) use ($onProgress, $subnets): void {
60-
static $lastUrl = '';
99+
$options['on_progress'] = static function (int $dlNow, int $dlSize, array $info) use ($onProgress, $subnets, $ipFlags): void {
61100
static $lastPrimaryIp = '';
62101

63-
if ($info['url'] !== $lastUrl) {
64-
$host = parse_url($info['url'], PHP_URL_HOST) ?: '';
65-
$resolve = $info['resolve'] ?? static function () { return null; };
102+
if (($info['primary_ip'] ?? '') !== $lastPrimaryIp) {
103+
self::ipCheck($info['primary_ip'], $subnets, $ipFlags, null, $info['url']);
104+
$lastPrimaryIp = $info['primary_ip'];
105+
}
106+
107+
null !== $onProgress && $onProgress($dlNow, $dlSize, $info);
108+
};
66109

67-
if (($ip = trim($host, '[]'))
68-
&& !filter_var($ip, \FILTER_VALIDATE_IP)
69-
&& !($ip = $resolve($host))
70-
&& $ip = @(gethostbynamel($host)[0] ?? dns_get_record($host, \DNS_AAAA)[0]['ipv6'] ?? null)
71-
) {
72-
$resolve($host, $ip);
73-
}
110+
return new AsyncResponse($this->client, $method, $url, $options, static function (ChunkInterface $chunk, AsyncContext $context) use (&$method, &$options, $maxRedirects, &$redirectHeaders, $subnets, $ipFlags, $dnsCache): \Generator {
111+
if (null !== $chunk->getError() || $chunk->isTimeout() || !$chunk->isFirst()) {
112+
yield $chunk;
74113

75-
if ($ip && IpUtils::checkIp($ip, $subnets ?? IpUtils::PRIVATE_SUBNETS)) {
76-
throw new TransportException(sprintf('Host "%s" is blocked for "%s".', $host, $info['url']));
77-
}
114+
return;
115+
}
116+
117+
$statusCode = $context->getStatusCode();
78118

79-
$lastUrl = $info['url'];
119+
if ($statusCode < 300 || 400 <= $statusCode || null === $url = $context->getInfo('redirect_url')) {
120+
$context->passthru();
121+
122+
yield $chunk;
123+
124+
return;
80125
}
81126

82-
if ($info['primary_ip'] !== $lastPrimaryIp) {
83-
if ($info['primary_ip'] && IpUtils::checkIp($info['primary_ip'], $subnets ?? IpUtils::PRIVATE_SUBNETS)) {
84-
throw new TransportException(sprintf('IP "%s" is blocked for "%s".', $info['primary_ip'], $info['url']));
127+
$host = parse_url($url, \PHP_URL_HOST);
128+
$ip = self::dnsResolve($dnsCache, $host, $ipFlags, $options);
129+
self::ipCheck($ip, $subnets, $ipFlags, $host, $url);
130+
131+
// Do like curl and browsers: turn POST to GET on 301, 302 and 303
132+
if (303 === $statusCode || 'POST' === $method && \in_array($statusCode, [301, 302], true)) {
133+
$method = 'HEAD' === $method ? 'HEAD' : 'GET';
134+
unset($options['body'], $options['json']);
135+
136+
if (isset($options['normalized_headers']['content-length']) || isset($options['normalized_headers']['content-type']) || isset($options['normalized_headers']['transfer-encoding'])) {
137+
$filterContentHeaders = static function ($h) {
138+
return 0 !== stripos($h, 'Content-Length:') && 0 !== stripos($h, 'Content-Type:') && 0 !== stripos($h, 'Transfer-Encoding:');
139+
};
140+
$options['header'] = array_filter($options['header'], $filterContentHeaders);
141+
$redirectHeaders['no_auth'] = array_filter($redirectHeaders['no_auth'], $filterContentHeaders);
142+
$redirectHeaders['with_auth'] = array_filter($redirectHeaders['with_auth'], $filterContentHeaders);
85143
}
86-
87-
$lastPrimaryIp = $info['primary_ip'];
88144
}
89145

90-
null !== $onProgress && $onProgress($dlNow, $dlSize, $info);
91-
};
146+
// Authorization and Cookie headers MUST NOT follow except for the initial host name
147+
$port = parse_url($url, \PHP_URL_PORT);
148+
$options['headers'] = $redirectHeaders['host'] === $host && ($redirectHeaders['port'] ?? null) === $port ? $redirectHeaders['with_auth'] : $redirectHeaders['no_auth'];
149+
150+
static $redirectCount = 0;
151+
$context->setInfo('redirect_count', ++$redirectCount);
92152

93-
return $this->client->request($method, $url, $options);
153+
$context->replaceRequest($method, $url, $options);
154+
155+
if ($redirectCount >= $maxRedirects) {
156+
$context->passthru();
157+
}
158+
});
94159
}
95160

96161
public function stream(ResponseInterface|iterable $responses, ?float $timeout = null): ResponseStreamInterface
@@ -109,14 +174,73 @@ public function withOptions(array $options): static
109174
{
110175
$clone = clone $this;
111176
$clone->client = $this->client->withOptions($options);
177+
$clone->defaultOptions = self::mergeDefaultOptions($options, $this->defaultOptions);
112178

113179
return $clone;
114180
}
115181

116182
public function reset(): void
117183
{
184+
$this->dnsCache->exchangeArray([]);
185+
118186
if ($this->client instanceof ResetInterface) {
119187
$this->client->reset();
120188
}
121189
}
190+
191+
private static function dnsResolve(\ArrayObject $dnsCache, string $host, int $ipFlags, array &$options): string
192+
{
193+
if ($ip = filter_var(trim($host, '[]'), \FILTER_VALIDATE_IP) ?: $options['resolve'][$host] ?? false) {
194+
return $ip;
195+
}
196+
197+
if ($dnsCache->offsetExists($host)) {
198+
return $dnsCache[$host];
199+
}
200+
201+
if ((\FILTER_FLAG_IPV4 & $ipFlags) && $ip = gethostbynamel($host)) {
202+
return $options['resolve'][$host] = $dnsCache[$host] = $ip[0];
203+
}
204+
205+
if (!(\FILTER_FLAG_IPV6 & $ipFlags)) {
206+
return $host;
207+
}
208+
209+
if ($ip = dns_get_record($host, \DNS_AAAA)) {
210+
$ip = $ip[0]['ipv6'];
211+
} elseif (extension_loaded('sockets')) {
212+
if (!$info = socket_addrinfo_lookup($host, 0, ['ai_socktype' => \SOCK_STREAM, 'ai_family' => \AF_INET6])) {
213+
return $host;
214+
}
215+
216+
$ip = socket_addrinfo_explain($info[0])['ai_addr']['sin6_addr'];
217+
} elseif ('localhost' === $host || 'localhost.' === $host) {
218+
$ip = '::1';
219+
} else {
220+
return $host;
221+
}
222+
223+
return $options['resolve'][$host] = $dnsCache[$host] = $ip;
224+
}
225+
226+
private static function ipCheck(string $ip, ?array $subnets, int $ipFlags, ?string $host, string $url): void
227+
{
228+
if (null === $subnets) {
229+
// Quick check, but not reliable enough, see https://github.com/php/php-src/issues/16944
230+
$ipFlags |= \FILTER_FLAG_NO_PRIV_RANGE | \FILTER_FLAG_NO_RES_RANGE;
231+
}
232+
233+
if (false !== filter_var($ip, \FILTER_VALIDATE_IP, $ipFlags) && !IpUtils::checkIp($ip, $subnets ?? IpUtils::PRIVATE_SUBNETS)) {
234+
return;
235+
}
236+
237+
if (null !== $host) {
238+
$type = 'Host';
239+
} else {
240+
$host = $ip;
241+
$type = 'IP';
242+
}
243+
244+
throw new TransportException($type.\sprintf(' "%s" is blocked for "%s".', $host, $url));
245+
}
122246
}

‎Response/AmpResponse.php

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -88,17 +88,9 @@ public function __construct(AmpClientState $multi, Request $request, array $opti
8888
$info['max_duration'] = $options['max_duration'];
8989
$info['debug'] = '';
9090

91-
$resolve = static function (string $host, ?string $ip = null) use ($multi): ?string {
92-
if (null !== $ip) {
93-
$multi->dnsCache[$host] = $ip;
94-
}
95-
96-
return $multi->dnsCache[$host] ?? null;
97-
};
9891
$onProgress = $options['on_progress'] ?? static function () {};
99-
$onProgress = $this->onProgress = static function () use (&$info, $onProgress, $resolve) {
92+
$onProgress = $this->onProgress = static function () use (&$info, $onProgress) {
10093
$info['total_time'] = microtime(true) - $info['start_time'];
101-
$info['resolve'] = $resolve;
10294
$onProgress((int) $info['size_download'], ((int) (1 + $info['download_content_length']) ?: 1) - 1, (array) $info);
10395
};
10496

‎Response/CurlResponse.php

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -118,20 +118,13 @@ public function __construct(CurlClientState $multi, \CurlHandle|string $ch, ?arr
118118
curl_pause($ch, \CURLPAUSE_CONT);
119119

120120
if ($onProgress = $options['on_progress']) {
121-
$resolve = static function (string $host, ?string $ip = null) use ($multi): ?string {
122-
if (null !== $ip) {
123-
$multi->dnsCache->hostnames[$host] = $ip;
124-
}
125-
126-
return $multi->dnsCache->hostnames[$host] ?? null;
127-
};
128121
$url = isset($info['url']) ? ['url' => $info['url']] : [];
129122
curl_setopt($ch, \CURLOPT_NOPROGRESS, false);
130-
curl_setopt($ch, \CURLOPT_PROGRESSFUNCTION, static function ($ch, $dlSize, $dlNow) use ($onProgress, &$info, $url, $multi, $debugBuffer, $resolve) {
123+
curl_setopt($ch, \CURLOPT_PROGRESSFUNCTION, static function ($ch, $dlSize, $dlNow) use ($onProgress, &$info, $url, $multi, $debugBuffer) {
131124
try {
132125
rewind($debugBuffer);
133126
$debug = ['debug' => stream_get_contents($debugBuffer)];
134-
$onProgress($dlNow, $dlSize, $url + curl_getinfo($ch) + $info + $debug + ['resolve' => $resolve]);
127+
$onProgress($dlNow, $dlSize, $url + curl_getinfo($ch) + $info + $debug);
135128
} catch (\Throwable $e) {
136129
$multi->handlesActivity[(int) $ch][] = null;
137130
$multi->handlesActivity[(int) $ch][] = $e;

‎Tests/AmpHttpClientTest.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@
1414
use Symfony\Component\HttpClient\AmpHttpClient;
1515
use Symfony\Contracts\HttpClient\HttpClientInterface;
1616

17+
/**
18+
* @group dns-sensitive
19+
*/
1720
class AmpHttpClientTest extends HttpClientTestCase
1821
{
1922
protected function getHttpClient(string $testCase): HttpClientInterface

‎Tests/CurlHttpClientTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
/**
1919
* @requires extension curl
20+
* @group dns-sensitive
2021
*/
2122
class CurlHttpClientTest extends HttpClientTestCase
2223
{

‎Tests/HttpClientTestCase.php

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Component\HttpClient\Tests;
1313

14+
use Symfony\Bridge\PhpUnit\DnsMock;
1415
use Symfony\Component\HttpClient\Exception\ClientException;
1516
use Symfony\Component\HttpClient\Exception\InvalidArgumentException;
1617
use Symfony\Component\HttpClient\Exception\TransportException;
@@ -485,6 +486,38 @@ public function testNoPrivateNetworkWithResolve()
485486
$client->request('GET', 'http://symfony.com', ['resolve' => ['symfony.com' => '127.0.0.1']]);
486487
}
487488

489+
public function testNoPrivateNetworkWithResolveAndRedirect()
490+
{
491+
DnsMock::withMockedHosts([
492+
'localhost' => [
493+
[
494+
'host' => 'localhost',
495+
'class' => 'IN',
496+
'ttl' => 15,
497+
'type' => 'A',
498+
'ip' => '127.0.0.1',
499+
],
500+
],
501+
'symfony.com' => [
502+
[
503+
'host' => 'symfony.com',
504+
'class' => 'IN',
505+
'ttl' => 15,
506+
'type' => 'A',
507+
'ip' => '10.0.0.1',
508+
],
509+
],
510+
]);
511+
512+
$client = $this->getHttpClient(__FUNCTION__);
513+
$client = new NoPrivateNetworkHttpClient($client, '10.0.0.1/32');
514+
515+
$this->expectException(TransportException::class);
516+
$this->expectExceptionMessage('Host "symfony.com" is blocked');
517+
518+
$client->request('GET', 'http://localhost:8057/302?location=https://symfony.com/');
519+
}
520+
488521
public function testNoRedirectWithInvalidLocation()
489522
{
490523
$client = $this->getHttpClient(__FUNCTION__);

‎Tests/NativeHttpClientTest.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@
1414
use Symfony\Component\HttpClient\NativeHttpClient;
1515
use Symfony\Contracts\HttpClient\HttpClientInterface;
1616

17+
/**
18+
* @group dns-sensitive
19+
*/
1720
class NativeHttpClientTest extends HttpClientTestCase
1821
{
1922
protected function getHttpClient(string $testCase): HttpClientInterface

0 commit comments

Comments
 (0)