From f514d330a0bf3c969ccd35766d482e97f3058321 Mon Sep 17 00:00:00 2001 From: Mark Shust Date: Tue, 6 Oct 2026 11:14:56 -0400 Subject: [PATCH] feat!: pin webhook connections to the ip the url policy validated WebhookUrlPolicy resolved the host and rejected internal ranges, but the HTTP client then resolved the name again, so a DNS answer that flipped to an internal address between the two lookups (rebinding) slipped through. - marko/http: new RequestOptions::RESOLVE_TO ('resolve_to') option that connects to a pre-resolved IP while keeping the Host header and TLS SNI. It must be an IP literal, cannot be combined with a proxy, and requires 'allow_redirects' => false. Drivers that cannot pin must throw InvalidRequestOptionException::unsupportedByDriver() instead of ignoring it. - marko/http-guzzle: maps resolve_to to cURL CURLOPT_RESOLVE plus CURLOPT_FRESH_CONNECT; rejects setups that would bypass the pin (no curl extension, guzzle stream/proxy/redirect/resolve overrides, relative URLs, mismatched IP-literal hosts). - marko/webhook: WebhookUrlPolicyInterface::validate() now returns the approved IP address and WebhookDispatcher pins the request to it. Fixes #514 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01KyjR4cJTfro54iFZ9CWkv1 --- .../docs/packages/http-guzzle.md | 15 +- packages/docs-markdown/docs/packages/http.md | 29 +++- .../docs-markdown/docs/packages/webhook.md | 12 +- packages/http-guzzle/src/GuzzleHttpClient.php | 113 +++++++++++- .../tests/Unit/GuzzleHttpClientTest.php | 162 ++++++++++++++++++ .../InvalidRequestOptionException.php | 46 +++++ packages/http/src/RequestOptions.php | 36 ++++ packages/http/tests/Unit/ExceptionTest.php | 15 ++ .../http/tests/Unit/RequestOptionsTest.php | 39 +++++ .../Contracts/WebhookUrlPolicyInterface.php | 13 +- .../webhook/src/Sending/WebhookDispatcher.php | 12 +- .../webhook/src/Sending/WebhookUrlPolicy.php | 6 +- .../tests/Fixtures/RebindingHostResolver.php | 36 ++++ .../tests/Fixtures/ResolvingHttpClient.php | 75 ++++++++ .../tests/Sending/WebhookDispatcherTest.php | 40 +++++ .../tests/Sending/WebhookUrlPolicyTest.php | 12 +- 16 files changed, 642 insertions(+), 19 deletions(-) create mode 100644 packages/webhook/tests/Fixtures/RebindingHostResolver.php create mode 100644 packages/webhook/tests/Fixtures/ResolvingHttpClient.php diff --git a/packages/docs-markdown/docs/packages/http-guzzle.md b/packages/docs-markdown/docs/packages/http-guzzle.md index 0547f0d19..9a2b598b4 100644 --- a/packages/docs-markdown/docs/packages/http-guzzle.md +++ b/packages/docs-markdown/docs/packages/http-guzzle.md @@ -107,10 +107,23 @@ Portable options map to Guzzle as follows: | `allow_redirects` `true`/`false` | Guzzle `allow_redirects` | | `allow_redirects` `int` | Guzzle `allow_redirects` with `['max' => $int]` | | `http_errors` | Guzzle `http_errors` (default `true`) | +| `resolve_to` | Guzzle `curl` options `CURLOPT_RESOLVE` (`host:port:address`) and `CURLOPT_FRESH_CONNECT` | + +### Pinning to an IP + +`resolve_to` (see [Pinning the Connection to an IP](/docs/packages/http/#pinning-the-connection-to-an-ip)) is implemented with cURL's `CURLOPT_RESOLVE`, which maps the URL's host and port (`443`/`80` by default) to the given address, so the `Host` header, SNI and certificate checks keep the hostname. `CURLOPT_FRESH_CONNECT` makes cURL open a new connection rather than reuse a pooled one opened to an earlier DNS answer. + +Only Guzzle's cURL handlers can pin a connection, so the request throws `InvalidRequestOptionException` before anything is sent when: + +- the PHP `curl` extension is not loaded (Guzzle would fall back to its stream handler), +- the `guzzle` escape hatch sets `stream`, `proxy`, a non-`false` `allow_redirects`, or `curl` options `CURLOPT_RESOLVE` / `CURLOPT_CONNECT_TO`, +- the URL is not an absolute `http`/`https` URL, or its host is an IP literal other than the pinned address. + +Other `curl` options in the escape hatch are kept alongside the pin. Proxies configured through `HTTP_PROXY`/`HTTPS_PROXY` environment variables are applied by Guzzle and cURL themselves; a proxied request is resolved by the proxy, so don't route pinned requests through one. ### The `guzzle` Escape Hatch -For a Guzzle feature outside the portable set (`cert`, `ssl_key`, `sink`, `on_stats`, `decode_content`, `debug`, ...), pass a `guzzle` array. It is merged verbatim over the options above, so it wins on conflict: +For a Guzzle feature outside the portable set (`cert`, `ssl_key`, `sink`, `on_stats`, `decode_content`, `debug`, ...), pass a `guzzle` array. It is merged verbatim over the options above, so it wins on conflict (except for the pinning options above, which are rejected rather than overridden): ```php $response = $this->httpClient->get('https://internal.example.com/report.csv', [ diff --git a/packages/docs-markdown/docs/packages/http.md b/packages/docs-markdown/docs/packages/http.md index 32daffa3e..cc896723a 100644 --- a/packages/docs-markdown/docs/packages/http.md +++ b/packages/docs-markdown/docs/packages/http.md @@ -92,6 +92,7 @@ All methods accept an `$options` array. The keys are a portable set defined as c | `allow_redirects` | `RequestOptions::ALLOW_REDIRECTS` | `bool`, or the maximum number of redirects as an `int` | | `proxy` | `RequestOptions::PROXY` | Proxy URL | | `http_errors` | `RequestOptions::HTTP_ERRORS` | `bool`, default `true` --- throw `HttpException` on 4xx/5xx | +| `resolve_to` | `RequestOptions::RESOLVE_TO` | IPv4 or IPv6 address (no brackets) to connect to instead of resolving the host --- see [Pinning the Connection to an IP](#pinning-the-connection-to-an-ip) | ```php use Marko\Http\RequestOptions; @@ -107,10 +108,34 @@ Options are validated before the request is sent. Each of these throws `InvalidR - An unknown key, such as the typo `form_param` --- the message names the key and the suggestion lists every supported key. - More than one body option (`body`, `json`, `form_params`, `multipart`) in the same request. -- A malformed `auth`, or a `http_errors`, `verify` or `allow_redirects` value of the wrong type. +- A malformed `auth`, or a `http_errors`, `verify`, `allow_redirects` or `resolve_to` value of the wrong type. +- `resolve_to` combined with `proxy`, or without `'allow_redirects' => false`. Drivers may accept an extra, clearly non-portable key of their own (for example `guzzle` in [`marko/http-guzzle`](/docs/packages/http-guzzle/)). Any other key fails loudly instead of being ignored. +### Pinning the Connection to an IP + +When you have already resolved and vetted a host --- for example to reject internal addresses before calling a user-supplied URL --- pass the address you checked as `resolve_to`. The client connects to that address instead of resolving the host again, while the `Host` header, TLS SNI and certificate verification still use the hostname from the URL: + +```php +use Marko\Http\RequestOptions; + +$response = $this->httpClient->post('https://hooks.example.com/in', [ + RequestOptions::JSON => $event, + RequestOptions::RESOLVE_TO => '93.184.215.14', + RequestOptions::ALLOW_REDIRECTS => false, +]); +``` + +Without the pin, a hostname whose DNS answer changes between your check and the connection (DNS rebinding) can steer the request to an address you never approved. [`marko/webhook`](/docs/packages/webhook/#outgoing-url-policy) uses this option for every outgoing webhook. + +The pin covers only the URL's host and port, so: + +- `resolve_to` requires `'allow_redirects' => false` --- a redirect to another host would be resolved normally. Check and pin a redirect target yourself before following it. +- `resolve_to` cannot be combined with `proxy`, because a proxy resolves the destination itself. + +A driver that cannot pin the connection throws `InvalidRequestOptionException` instead of ignoring the option, so the request is never silently sent unpinned. + ### Error Responses With the default `'http_errors' => true`, a 4xx or 5xx response throws `HttpException`, and the response is available from `getResponse()`: @@ -217,6 +242,6 @@ Drivers call `validate()` before sending, passing any driver-specific keys they | Exception | Description | |-----------|-------------| -| `InvalidRequestOptionException` | Thrown before sending when an option key is unknown, body options conflict, or an option value has the wrong shape (extends `MarkoException`) | +| `InvalidRequestOptionException` | Thrown before sending when an option key is unknown, options conflict, an option value has the wrong shape, or the driver cannot honour an option such as `resolve_to` (extends `MarkoException`) | | `HttpException` | Base exception for HTTP errors --- provides `getResponse()` to access the underlying `HttpResponse` if available | | `ConnectionException` | Thrown when the HTTP connection itself fails (extends `HttpException`) | diff --git a/packages/docs-markdown/docs/packages/webhook.md b/packages/docs-markdown/docs/packages/webhook.md index c08b07af3..4793adb0d 100644 --- a/packages/docs-markdown/docs/packages/webhook.md +++ b/packages/docs-markdown/docs/packages/webhook.md @@ -134,8 +134,10 @@ public function register(string $url): void Hostnames are resolved through `HostResolverInterface` (bound to `DnsHostResolver`, which uses the system resolver). Bind your own resolver to use a different DNS source, or a fixed map in tests. To allow destinations the default policy rejects, for example a receiver on your private network, bind your own `WebhookUrlPolicyInterface` implementation. -:::caution[DNS rebinding] -`marko/http` has no option to pin a request to an already-resolved IP, so the HTTP client resolves the host again when it connects. The policy runs immediately before each request to keep that window small, but a hostname whose DNS answer changes between the check and the connection can still reach an internal address. If your receivers are untrusted and your network exposes sensitive internal services, also block egress to internal ranges at the network level (firewall or egress proxy). +`validate()` returns the address the policy approved (the first one the host resolved to), and `WebhookDispatcher` pins the connection to it with the `marko/http` [`resolve_to`](/docs/packages/http/#pinning-the-connection-to-an-ip) option. The HTTP client never resolves the host a second time, so a hostname whose DNS answer flips to an internal address after the check (DNS rebinding) cannot redirect the delivery. The `Host` header and TLS certificate check still use the hostname. A custom `WebhookUrlPolicyInterface` implementation must return the IP address the request should connect to. + +:::note[HTTP driver support] +Pinning needs an HTTP client driver that supports `resolve_to`, such as [`marko/http-guzzle`](/docs/packages/http-guzzle/#pinning-to-an-ip) with the PHP `curl` extension. A driver that cannot pin throws `InvalidRequestOptionException`, so a webhook is never sent unpinned. Deliveries are pinned to a direct connection; if your servers must reach the internet through an egress proxy, the proxy resolves the host itself, so enforce the internal-range block on the proxy too. ::: ### Sending Asynchronously with Retry @@ -385,7 +387,8 @@ public function __construct( ); // Returns a WebhookResponse for every HTTP response (4xx/5xx and 3xx included; redirects are not followed). -// Validates the URL with the URL policy first and sends webhook.timeout as the request timeout. +// Validates the URL with the URL policy first, pins the connection to the address it approved (resolve_to), +// and sends webhook.timeout as the request timeout. // Sends X-Webhook-Id, X-Webhook-Timestamp and X-Webhook-Signature headers. // @throws UnsafeWebhookUrlException when the URL policy rejects the URL (nothing is sent) // @throws InvalidWebhookPayloadException when the data cannot be encoded as JSON (nothing is sent) @@ -405,8 +408,9 @@ public function __construct( HostResolverInterface $resolver, ); +// Returns the approved IP address to pin the connection to (the first address the host resolved to). // @throws UnsafeWebhookUrlException -public function validate(string $url): void; +public function validate(string $url): string; ``` ### DnsHostResolver diff --git a/packages/http-guzzle/src/GuzzleHttpClient.php b/packages/http-guzzle/src/GuzzleHttpClient.php index e4fe1b45f..f51945638 100644 --- a/packages/http-guzzle/src/GuzzleHttpClient.php +++ b/packages/http-guzzle/src/GuzzleHttpClient.php @@ -48,7 +48,7 @@ public function request( string $url, array $options = [], ): HttpResponse { - $guzzleOptions = $this->buildOptions($options); + $guzzleOptions = $this->buildOptions($url, $options); try { $response = $this->client()->request($method, $url, $guzzleOptions); @@ -150,6 +150,7 @@ private function client(): GuzzleClientInterface * @return array */ private function buildOptions( + string $url, array $options, ): array { RequestOptions::validate($options, [self::GUZZLE_OPTIONS]); @@ -163,6 +164,17 @@ private function buildOptions( unset($options[self::GUZZLE_OPTIONS]); $guzzleOptions = $options; + unset($guzzleOptions[RequestOptions::RESOLVE_TO]); + + if (isset($options[RequestOptions::RESOLVE_TO])) { + $guzzleOptions['curl'] = $this->pinnedCurlOptions( + $url, + (string) $options[RequestOptions::RESOLVE_TO], + $escapeHatch, + ); + unset($escapeHatch['curl']); + } + $guzzleOptions[RequestOptions::HTTP_ERRORS] = RequestOptions::throwsOnHttpError($options); $guzzleOptions[RequestOptions::TIMEOUT] ??= $this->timeoutFromConfig('timeout'); $guzzleOptions[RequestOptions::CONNECT_TIMEOUT] ??= $this->timeoutFromConfig('connect_timeout'); @@ -183,6 +195,105 @@ private function buildOptions( return array_replace($guzzleOptions, $escapeHatch); } + /** + * The cURL options that pin the connection to $address: CURLOPT_RESOLVE maps the URL's + * host and port to the address, so the Host header, TLS SNI and certificate check still + * use the hostname. CURLOPT_FRESH_CONNECT stops cURL from reusing a pooled connection + * that was opened to whatever address the host resolved to earlier. + * + * Only Guzzle's cURL handlers honour these options; its stream handler would silently + * ignore them, so anything that routes the request there is rejected. + * + * @param array $escapeHatch + * + * @return array + * + * @throws InvalidRequestOptionException + */ + private function pinnedCurlOptions( + string $url, + string $address, + array $escapeHatch, + ): array { + $option = RequestOptions::RESOLVE_TO; + + if (!extension_loaded('curl')) { + throw InvalidRequestOptionException::unsupportedByDriver( + $option, + self::class, + 'Pinning the connection needs the PHP curl extension; without it Guzzle falls back to its' + . ' stream handler, which cannot pin a connection to an IP address.', + ); + } + + if (!empty($escapeHatch['stream'])) { + throw InvalidRequestOptionException::unsupportedByDriver( + $option, + self::class . " with the 'stream' Guzzle option", + 'Guzzle sends streamed requests through its stream handler, which cannot pin a connection' + . ' to an IP address.', + ); + } + + if (array_key_exists('proxy', $escapeHatch)) { + throw InvalidRequestOptionException::conflictingOptions( + $option, + self::GUZZLE_OPTIONS . '.proxy', + 'A proxy resolves the destination host itself, so the connection cannot be pinned to an IP.', + ); + } + + if (($escapeHatch[RequestOptions::ALLOW_REDIRECTS] ?? false) !== false) { + throw InvalidRequestOptionException::pinnedRequestFollowsRedirects(); + } + + $curl = $escapeHatch['curl'] ?? []; + + if (!is_array($curl)) { + throw InvalidRequestOptionException::invalidType(self::GUZZLE_OPTIONS . '.curl', 'array', $curl); + } + + foreach ([CURLOPT_RESOLVE => 'CURLOPT_RESOLVE', CURLOPT_CONNECT_TO => 'CURLOPT_CONNECT_TO'] as $curlOption => $name) { + if (array_key_exists($curlOption, $curl)) { + throw InvalidRequestOptionException::conflictingOptions( + $option, + self::GUZZLE_OPTIONS . ".curl[$name]", + "$name decides which address cURL connects to, which is what '$option' sets.", + ); + } + } + + $parts = parse_url($url); + $scheme = is_array($parts) ? strtolower($parts['scheme'] ?? '') : ''; + $host = is_array($parts) ? ($parts['host'] ?? '') : ''; + + if (!in_array($scheme, ['http', 'https'], true) || $host === '') { + throw InvalidRequestOptionException::unpinnableUrl(); + } + + $pinned = [CURLOPT_FRESH_CONNECT => true]; + $literal = trim($host, '[]'); + + if (filter_var($literal, FILTER_VALIDATE_IP) !== false) { + // cURL never resolves an IP literal, so the URL itself must already name the pinned address. + if (inet_pton($literal) !== inet_pton($address)) { + throw InvalidRequestOptionException::conflictingOptions( + $option, + 'the URL host', + "The URL names the IP address $literal, which cURL connects to directly instead of $address.", + ); + } + + return array_replace($curl, $pinned); + } + + $port = $parts['port'] ?? ($scheme === 'https' ? 443 : 80); + $target = str_contains($address, ':') ? "[$address]" : $address; + $pinned[CURLOPT_RESOLVE] = ["$host:$port:$target"]; + + return array_replace($curl, $pinned); + } + /** * @throws ConfigException */ diff --git a/packages/http-guzzle/tests/Unit/GuzzleHttpClientTest.php b/packages/http-guzzle/tests/Unit/GuzzleHttpClientTest.php index f6505924e..debd6b2b2 100644 --- a/packages/http-guzzle/tests/Unit/GuzzleHttpClientTest.php +++ b/packages/http-guzzle/tests/Unit/GuzzleHttpClientTest.php @@ -539,3 +539,165 @@ protected function createClient(): GuzzleClientInterface } }); }); + +describe('GuzzleHttpClient resolve_to', function (): void { + it('pins the connection with CURLOPT_RESOLVE for the URL host and default port', function ( + string $url, + string $address, + string $entry, + ): void { + $mock = new MockHandler([new Response(200)]); + $history = []; + $client = createTestableClient($mock, $history); + + $client->post($url, ['resolve_to' => $address, 'allow_redirects' => false]); + + expect($history[0]['options']['curl'][CURLOPT_RESOLVE])->toBe([$entry]) + ->and($history[0]['options']['curl'][CURLOPT_FRESH_CONNECT])->toBeTrue() + ->and($history[0]['options'])->not->toHaveKey('resolve_to') + ->and($history[0]['request']->getHeaderLine('Host'))->toBe(parse_url($url, PHP_URL_HOST) + . (parse_url($url, PHP_URL_PORT) !== null ? ':' . parse_url($url, PHP_URL_PORT) : '')); + })->with([ + 'https default port' => ['https://hooks.example.com/in', '93.184.215.14', 'hooks.example.com:443:93.184.215.14'], + 'http default port' => ['http://hooks.example.com/in', '93.184.215.14', 'hooks.example.com:80:93.184.215.14'], + 'explicit port' => ['https://hooks.example.com:8443/in', '93.184.215.14', 'hooks.example.com:8443:93.184.215.14'], + 'ipv6 address' => ['https://hooks.example.com/in', '2606:2800:21f::1', 'hooks.example.com:443:[2606:2800:21f::1]'], + ]); + + it('sets no curl options when resolve_to is absent', function (): void { + $mock = new MockHandler([new Response(200)]); + $history = []; + $client = createTestableClient($mock, $history); + + $client->get('https://example.com/a'); + + expect($history[0]['options'])->not->toHaveKey('curl'); + }); + + it('keeps other escape-hatch curl options alongside the pin', function (): void { + $mock = new MockHandler([new Response(200)]); + $history = []; + $client = createTestableClient($mock, $history); + + $client->get('https://example.com/a', [ + 'resolve_to' => '93.184.215.14', + 'allow_redirects' => false, + 'guzzle' => ['curl' => [CURLOPT_TCP_NODELAY => true]], + ]); + + expect($history[0]['options']['curl'][CURLOPT_TCP_NODELAY])->toBeTrue() + ->and($history[0]['options']['curl'][CURLOPT_RESOLVE])->toBe(['example.com:443:93.184.215.14']); + }); + + it('accepts an IP-literal URL that names the pinned address', function (): void { + $mock = new MockHandler([new Response(200)]); + $history = []; + $client = createTestableClient($mock, $history); + + $client->get('https://[2606:2800:21f::1]/a', [ + 'resolve_to' => '2606:2800:21f:0::1', + 'allow_redirects' => false, + ]); + + expect($history[0]['options']['curl'])->not->toHaveKey(CURLOPT_RESOLVE) + ->and($history[0]['options']['curl'][CURLOPT_FRESH_CONNECT])->toBeTrue(); + }); + + it('rejects pinning options that would bypass or override the pin, before sending', function ( + string $url, + array $guzzle, + string $message, + ): void { + $mock = new MockHandler([new Response(200)]); + $history = []; + $client = createTestableClient($mock, $history); + + expect(fn () => $client->get($url, [ + 'resolve_to' => '93.184.215.14', + 'allow_redirects' => false, + 'guzzle' => $guzzle, + ]))->toThrow(InvalidRequestOptionException::class, $message) + ->and($history)->toBeEmpty(); + })->with([ + 'streamed request' => ['https://example.com/a', ['stream' => true], "with the 'stream' Guzzle option"], + 'guzzle proxy' => ['https://example.com/a', ['proxy' => 'http://proxy:8080'], "'guzzle.proxy'"], + 'guzzle redirects' => ['https://example.com/a', ['allow_redirects' => true], "requires 'allow_redirects' => false"], + 'curl resolve' => ['https://example.com/a', ['curl' => [CURLOPT_RESOLVE => ['x:443:1.1.1.1']]], 'CURLOPT_RESOLVE'], + 'curl connect_to' => ['https://example.com/a', ['curl' => [CURLOPT_CONNECT_TO => ['::1.1.1.1:']]], 'CURLOPT_CONNECT_TO'], + 'curl not an array' => ['https://example.com/a', ['curl' => 'x'], "'guzzle.curl'"], + 'relative url' => ['/a', [], 'needs an absolute http or https URL'], + 'ftp url' => ['ftp://example.com/a', [], 'needs an absolute http or https URL'], + 'different ip literal' => ['https://10.0.0.1/a', [], "'resolve_to' and 'the URL host' cannot be used together"], + ]); + + it('connects to the pinned address while sending the original Host header', function (): void { + $server = startPinningTestServer(); + + try { + $client = new GuzzleHttpClient(guzzleConfig()); + $response = $client->get("http://pinned.marko.invalid:{$server['port']}/hook", [ + 'resolve_to' => '127.0.0.1', + 'allow_redirects' => false, + ]); + + expect($response->statusCode())->toBe(200) + ->and($response->body())->toBe("pinned.marko.invalid:{$server['port']}"); + } finally { + proc_terminate($server['process']); + proc_close($server['process']); + unlink($server['router']); + } + }); + + it('cannot reach a name that does not resolve without resolve_to', function (): void { + $client = new GuzzleHttpClient(guzzleConfig(connectTimeout: 5.0)); + + expect(fn () => $client->get('http://pinned.marko.invalid/hook')) + ->toThrow(ConnectionException::class); + }); +}); + +/** + * Starts PHP's built-in server on a free loopback port, answering every request with its Host header. + * + * @return array{process: resource, port: int, router: string} + */ +function startPinningTestServer(): array +{ + $socket = stream_socket_server('tcp://127.0.0.1:0'); + $port = (int) substr((string) strrchr((string) stream_socket_get_name($socket, false), ':'), 1); + fclose($socket); + + $router = sys_get_temp_dir() . '/marko-pinning-router-' . getmypid() . '.php'; + file_put_contents($router, ' ['file', '/dev/null', 'w'], 2 => ['file', '/dev/null', 'w']], + $pipes, + ); + + // Connection attempts are refused until the server is listening; those warnings are expected. + set_error_handler(static fn (): bool => true); + + try { + for ($attempt = 0; $attempt < 100; $attempt++) { + $connection = fsockopen('127.0.0.1', $port, $errno, $errstr, 0.1); + + if ($connection !== false) { + fclose($connection); + + return ['process' => $process, 'port' => $port, 'router' => $router]; + } + + usleep(20_000); + } + } finally { + restore_error_handler(); + } + + proc_terminate($process); + unlink($router); + + throw new RuntimeException("The pinning test server did not start on port $port."); +} diff --git a/packages/http/src/Exceptions/InvalidRequestOptionException.php b/packages/http/src/Exceptions/InvalidRequestOptionException.php index 1e5f9a72a..388d0ece2 100644 --- a/packages/http/src/Exceptions/InvalidRequestOptionException.php +++ b/packages/http/src/Exceptions/InvalidRequestOptionException.php @@ -60,4 +60,50 @@ public static function invalidType( suggestion: "Pass $expected for the '$key' option.", ); } + + public static function conflictingOptions( + string $key, + string $otherKey, + string $reason, + ): self { + return new self( + message: "HTTP request options '$key' and '$otherKey' cannot be used together.", + context: $reason, + suggestion: "Remove either '$key' or '$otherKey' from the request options.", + ); + } + + public static function unsupportedByDriver( + string $key, + string $driver, + string $reason, + ): self { + return new self( + message: "HTTP request option '$key' is not supported by $driver.", + context: $reason, + suggestion: "Use an HTTP client driver that supports '$key', or remove the option. " + . 'The option is never silently ignored, because callers rely on it for security.', + ); + } + + public static function pinnedRequestFollowsRedirects(): self + { + return new self( + message: "HTTP request option 'resolve_to' requires 'allow_redirects' => false.", + context: "'resolve_to' pins the connection for the request URL's host only. A redirect to another host" + . ' would be resolved normally, escaping the pin.', + suggestion: "Pass 'allow_redirects' => false alongside 'resolve_to', and validate and pin any" + . ' redirect target yourself before following it.', + ); + } + + public static function unpinnableUrl(): self + { + return new self( + message: "HTTP request option 'resolve_to' needs an absolute http or https URL with a host.", + context: "The connection is pinned by mapping the URL's host and port to the given IP address," + . ' so the URL must name both a scheme and a host.', + suggestion: "Pass an absolute URL such as 'https://example.com/path', or remove 'resolve_to'.", + ); + } } diff --git a/packages/http/src/RequestOptions.php b/packages/http/src/RequestOptions.php index d93eb7b38..68baf9f63 100644 --- a/packages/http/src/RequestOptions.php +++ b/packages/http/src/RequestOptions.php @@ -53,6 +53,17 @@ class RequestOptions /** Throw HttpException on 4xx/5xx responses: bool (default true) */ public const string HTTP_ERRORS = 'http_errors'; + /** + * Connect to this pre-resolved IP address instead of resolving the URL's host, + * while keeping the host in the Host header and TLS SNI/certificate checks: string + * (an IPv4 or IPv6 literal, IPv6 without brackets). Pins only the request URL's + * host and port, so it cannot be combined with a proxy and requires + * 'allow_redirects' => false (a redirect could lead to a host the pin does not + * cover). A driver that cannot pin + * the connection must throw InvalidRequestOptionException rather than ignore it. + */ + public const string RESOLVE_TO = 'resolve_to'; + /** @var array */ public const array SUPPORTED = [ self::HEADERS, @@ -68,6 +79,7 @@ class RequestOptions self::ALLOW_REDIRECTS, self::PROXY, self::HTTP_ERRORS, + self::RESOLVE_TO, ]; /** @var array */ @@ -134,6 +146,30 @@ public static function validate( throw InvalidRequestOptionException::invalidType(self::VERIFY, 'bool or a CA bundle path', $value); } } + + if (array_key_exists(self::RESOLVE_TO, $options)) { + $value = $options[self::RESOLVE_TO]; + + if (!is_string($value) || filter_var($value, FILTER_VALIDATE_IP) === false) { + throw InvalidRequestOptionException::invalidType( + self::RESOLVE_TO, + 'an IPv4 or IPv6 address string', + $value, + ); + } + + if (array_key_exists(self::PROXY, $options)) { + throw InvalidRequestOptionException::conflictingOptions( + self::RESOLVE_TO, + self::PROXY, + 'A proxy resolves the destination host itself, so the connection cannot be pinned to an IP.', + ); + } + + if (($options[self::ALLOW_REDIRECTS] ?? true) !== false) { + throw InvalidRequestOptionException::pinnedRequestFollowsRedirects(); + } + } } /** diff --git a/packages/http/tests/Unit/ExceptionTest.php b/packages/http/tests/Unit/ExceptionTest.php index 2d51781ed..b7305e756 100644 --- a/packages/http/tests/Unit/ExceptionTest.php +++ b/packages/http/tests/Unit/ExceptionTest.php @@ -4,6 +4,7 @@ use Marko\Http\Exceptions\ConnectionException; use Marko\Http\Exceptions\HttpException; +use Marko\Http\Exceptions\InvalidRequestOptionException; use Marko\Http\HttpResponse; describe('HttpException', function (): void { @@ -40,3 +41,17 @@ ->and($exception->getResponse())->toBeNull(); }); }); + +describe('InvalidRequestOptionException', function (): void { + it('names the option and driver when a driver cannot honour an option', function (): void { + $exception = InvalidRequestOptionException::unsupportedByDriver( + 'resolve_to', + 'ExampleDriver', + 'ExampleDriver resolves hosts itself.', + ); + + expect($exception->getMessage())->toBe("HTTP request option 'resolve_to' is not supported by ExampleDriver.") + ->and($exception->getContext())->toBe('ExampleDriver resolves hosts itself.') + ->and($exception->getSuggestion())->toContain('never silently ignored'); + }); +}); diff --git a/packages/http/tests/Unit/RequestOptionsTest.php b/packages/http/tests/Unit/RequestOptionsTest.php index fadc74579..43a1abf0a 100644 --- a/packages/http/tests/Unit/RequestOptionsTest.php +++ b/packages/http/tests/Unit/RequestOptionsTest.php @@ -22,6 +22,7 @@ RequestOptions::ALLOW_REDIRECTS, RequestOptions::PROXY, RequestOptions::HTTP_ERRORS, + RequestOptions::RESOLVE_TO, ])->and(RequestOptions::SUPPORTED)->toBe([ 'headers', 'body', @@ -36,6 +37,7 @@ 'allow_redirects', 'proxy', 'http_errors', + 'resolve_to', ]); }); @@ -142,6 +144,43 @@ ->toThrow(InvalidRequestOptionException::class, 'verify'); }); + it('accepts an IPv4 or IPv6 address for resolve_to', function (string $ip): void { + RequestOptions::validate(['resolve_to' => $ip, 'allow_redirects' => false]); + + expect(true)->toBeTrue(); + })->with([ + 'ipv4' => ['93.184.215.14'], + 'ipv6' => ['2606:2800:21f:cb07:6820:80da:af6b:8b2c'], + ]); + + it('throws when resolve_to is not an IP address', function (mixed $value): void { + expect(fn () => RequestOptions::validate(['resolve_to' => $value, 'allow_redirects' => false])) + ->toThrow(InvalidRequestOptionException::class, "Invalid value for HTTP request option 'resolve_to'"); + })->with([ + 'hostname' => ['example.com'], + 'bracketed ipv6' => ['[::1]'], + 'empty' => [''], + 'int' => [2130706433], + 'array' => [['1.2.3.4']], + ]); + + it('throws when resolve_to is combined with a proxy', function (): void { + expect(fn () => RequestOptions::validate([ + 'resolve_to' => '93.184.215.14', + 'proxy' => 'http://proxy.local:8080', + 'allow_redirects' => false, + ]))->toThrow(InvalidRequestOptionException::class, "'resolve_to' and 'proxy' cannot be used together"); + }); + + it('throws when resolve_to is used without disabling redirects', function (array $options): void { + expect(fn () => RequestOptions::validate(['resolve_to' => '93.184.215.14', ...$options])) + ->toThrow(InvalidRequestOptionException::class, "requires 'allow_redirects' => false"); + })->with([ + 'redirects left at the default' => [[]], + 'redirects enabled' => [['allow_redirects' => true]], + 'a redirect limit' => [['allow_redirects' => 2]], + ]); + it('returns the bearer token from the auth option', function (): void { expect(RequestOptions::bearerToken(['auth' => ['bearer' => 'abc']]))->toBe('abc') ->and(RequestOptions::bearerToken(['auth' => ['user', 'pass']]))->toBeNull() diff --git a/packages/webhook/src/Contracts/WebhookUrlPolicyInterface.php b/packages/webhook/src/Contracts/WebhookUrlPolicyInterface.php index 5fc91cb95..5d9e603e9 100644 --- a/packages/webhook/src/Contracts/WebhookUrlPolicyInterface.php +++ b/packages/webhook/src/Contracts/WebhookUrlPolicyInterface.php @@ -8,15 +8,22 @@ /** * Decides whether an outgoing webhook may be sent to a URL. WebhookDispatcher calls - * validate() right before every request; call it yourself when a user registers a - * webhook URL so an unsafe destination is rejected before it is ever stored. + * validate() right before every request and pins the connection to the address it + * returns; call it yourself when a user registers a webhook URL so an unsafe + * destination is rejected before it is ever stored. */ interface WebhookUrlPolicyInterface { /** + * Return the IP address the request must connect to. The dispatcher pins the + * connection to it, so the HTTP client never resolves the host a second time + * (which a DNS rebinding attack could answer with an internal address). + * + * @return string An IPv4 or IPv6 address (IPv6 without brackets) that the policy approved + * * @throws UnsafeWebhookUrlException When the URL must not receive webhooks */ public function validate( string $url, - ): void; + ): string; } diff --git a/packages/webhook/src/Sending/WebhookDispatcher.php b/packages/webhook/src/Sending/WebhookDispatcher.php index 46b5ca74a..78c1eeef6 100644 --- a/packages/webhook/src/Sending/WebhookDispatcher.php +++ b/packages/webhook/src/Sending/WebhookDispatcher.php @@ -32,10 +32,11 @@ public function __construct( * Returns a WebhookResponse for every HTTP response, 4xx/5xx included; throws only on transport failures. * The request is abandoned after webhook.timeout seconds, which surfaces as a ConnectionException. * - * The URL is checked against the webhook URL policy immediately before the request, and redirects are - * never followed, so a receiver cannot bounce the request to an address the policy would reject. - * marko/http has no option to pin the connection to the checked IP, so a host whose DNS answer changes - * between the check and the connection (DNS rebinding) is narrowed to that window, not ruled out. + * The URL is checked against the webhook URL policy immediately before the request, and the connection + * is pinned to the address the policy approved (RequestOptions::RESOLVE_TO), so the HTTP client never + * resolves the host again: a host whose DNS answer changes after the check (DNS rebinding) cannot steer + * the request to an internal address. Redirects are never followed, so a receiver cannot bounce the + * request to an address the policy would reject. * * The body is signed together with the payload's delivery ID (X-Webhook-Id) and the timestamp. * A payload whose data cannot be encoded as JSON throws InvalidWebhookPayloadException before anything is sent. @@ -45,7 +46,7 @@ public function __construct( public function dispatch( WebhookPayload $payload, ): WebhookResponse { - $this->urlPolicy->validate($payload->url); + $address = $this->urlPolicy->validate($payload->url); try { $body = json_encode(['event' => $payload->event, 'data' => $payload->data], JSON_THROW_ON_ERROR); @@ -67,6 +68,7 @@ public function dispatch( RequestOptions::HTTP_ERRORS => false, RequestOptions::ALLOW_REDIRECTS => false, RequestOptions::TIMEOUT => $this->config->timeout, + RequestOptions::RESOLVE_TO => $address, ]); return new WebhookResponse( diff --git a/packages/webhook/src/Sending/WebhookUrlPolicy.php b/packages/webhook/src/Sending/WebhookUrlPolicy.php index e6c57ec81..468f9153d 100644 --- a/packages/webhook/src/Sending/WebhookUrlPolicy.php +++ b/packages/webhook/src/Sending/WebhookUrlPolicy.php @@ -64,11 +64,13 @@ public function __construct( } /** + * Returns the first address the host resolved to; every address was checked, so any of them is safe to pin. + * * @throws UnsafeWebhookUrlException */ public function validate( string $url, - ): void { + ): string { // Backslashes and whitespace are where URL parsers disagree, so parse_url() could see a // different host than the HTTP client. Such URLs are rejected rather than interpreted. $parts = preg_match('/[\\\\\s]/', $url) === 1 ? false : parse_url($url); @@ -112,6 +114,8 @@ public function validate( throw UnsafeWebhookUrlException::disallowedAddress($url, $host, $address, $range); } } + + return $addresses[0]; } /** diff --git a/packages/webhook/tests/Fixtures/RebindingHostResolver.php b/packages/webhook/tests/Fixtures/RebindingHostResolver.php new file mode 100644 index 000000000..94e340f82 --- /dev/null +++ b/packages/webhook/tests/Fixtures/RebindingHostResolver.php @@ -0,0 +1,36 @@ + */ + public private(set) array $lookups = []; + + /** + * @param list> $answers + */ + public function __construct( + private readonly array $answers, + ) {} + + /** + * @return list + */ + public function resolve( + string $host, + ): array { + $this->lookups[] = $host; + + return $this->answers[min(count($this->lookups), count($this->answers)) - 1]; + } +} diff --git a/packages/webhook/tests/Fixtures/ResolvingHttpClient.php b/packages/webhook/tests/Fixtures/ResolvingHttpClient.php new file mode 100644 index 000000000..f0fbcbf53 --- /dev/null +++ b/packages/webhook/tests/Fixtures/ResolvingHttpClient.php @@ -0,0 +1,75 @@ + */ + public private(set) array $connectedTo = []; + + public function __construct( + private readonly HostResolverInterface $resolver, + ) {} + + public function request( + string $method, + string $url, + array $options = [], + ): HttpResponse { + RequestOptions::validate($options); + + $address = $options[RequestOptions::RESOLVE_TO] + ?? $this->resolver->resolve((string) parse_url($url, PHP_URL_HOST))[0]; + + $this->connectedTo[] = (string) $address; + + return new HttpResponse(200, 'OK'); + } + + public function get( + string $url, + array $options = [], + ): HttpResponse { + return $this->request('GET', $url, $options); + } + + public function post( + string $url, + array $options = [], + ): HttpResponse { + return $this->request('POST', $url, $options); + } + + public function put( + string $url, + array $options = [], + ): HttpResponse { + return $this->request('PUT', $url, $options); + } + + public function patch( + string $url, + array $options = [], + ): HttpResponse { + return $this->request('PATCH', $url, $options); + } + + public function delete( + string $url, + array $options = [], + ): HttpResponse { + return $this->request('DELETE', $url, $options); + } +} diff --git a/packages/webhook/tests/Sending/WebhookDispatcherTest.php b/packages/webhook/tests/Sending/WebhookDispatcherTest.php index c7766828d..75dd8a15e 100644 --- a/packages/webhook/tests/Sending/WebhookDispatcherTest.php +++ b/packages/webhook/tests/Sending/WebhookDispatcherTest.php @@ -16,7 +16,10 @@ use Marko\Webhook\Exceptions\UnsafeWebhookUrlException; use Marko\Webhook\Sending\WebhookDispatcher; use Marko\Webhook\Sending\WebhookSignature; +use Marko\Webhook\Sending\WebhookUrlPolicy; use Marko\Webhook\Tests\Fixtures\FakeHostResolver; +use Marko\Webhook\Tests\Fixtures\RebindingHostResolver; +use Marko\Webhook\Tests\Fixtures\ResolvingHttpClient; use Marko\Webhook\Value\WebhookPayload; use Marko\Webhook\Value\WebhookResponse; @@ -194,6 +197,43 @@ public function delete( expect($httpClient->requests[0]->options[RequestOptions::ALLOW_REDIRECTS])->toBeFalse(); }); + it('pins the connection to the address the URL policy validated', function (): void { + $httpClient = new FakeHttpClient()->stub('https://example.com/webhook', new HttpResponse(200, 'OK')); + + $dispatcher = new WebhookDispatcher( + $httpClient, + new FakeClock(), + webhookDispatcherConfig(), + FakeHostResolver::policy(['example.com' => ['93.184.215.14', '93.184.215.15']]), + ); + $dispatcher->dispatch(webhookDispatcherPayload()); + + expect($httpClient->requests[0]->options[RequestOptions::RESOLVE_TO])->toBe('93.184.215.14'); + }); + + it('cannot be steered to an internal address by DNS that flips after the policy check', function (): void { + $resolver = new RebindingHostResolver([['93.184.215.14'], ['169.254.169.254']]); + $httpClient = new ResolvingHttpClient($resolver); + $policy = new WebhookUrlPolicy(new FakeConfigRepository(['webhook.allow_http' => false]), $resolver); + + $dispatcher = new WebhookDispatcher($httpClient, new FakeClock(), webhookDispatcherConfig(), $policy); + $dispatcher->dispatch(webhookDispatcherPayload('https://rebind.attacker.test/hook')); + + expect($httpClient->connectedTo)->toBe(['93.184.215.14']) + ->and($resolver->lookups)->toBe(['rebind.attacker.test']); + }); + + it('would reach the internal address if the HTTP client resolved the host again', function (): void { + $resolver = new RebindingHostResolver([['93.184.215.14'], ['169.254.169.254']]); + $httpClient = new ResolvingHttpClient($resolver); + $policy = new WebhookUrlPolicy(new FakeConfigRepository(['webhook.allow_http' => false]), $resolver); + + $policy->validate('https://rebind.attacker.test/hook'); + $httpClient->post('https://rebind.attacker.test/hook'); + + expect($httpClient->connectedTo)->toBe(['169.254.169.254']); + }); + it('refuses to send to a URL the policy rejects, before any request is made', function ( string $url, string $reason, diff --git a/packages/webhook/tests/Sending/WebhookUrlPolicyTest.php b/packages/webhook/tests/Sending/WebhookUrlPolicyTest.php index d8b902c97..8ca4d640b 100644 --- a/packages/webhook/tests/Sending/WebhookUrlPolicyTest.php +++ b/packages/webhook/tests/Sending/WebhookUrlPolicyTest.php @@ -17,11 +17,19 @@ ); $policy = new WebhookUrlPolicy(new FakeConfigRepository(['webhook.allow_http' => false]), $resolver); - $policy->validate('https://hooks.example.com/webhook'); + $address = $policy->validate('https://hooks.example.com/webhook'); - expect($resolver->lookups)->toBe(['hooks.example.com']); + expect($resolver->lookups)->toBe(['hooks.example.com']) + ->and($address)->toBe('93.184.215.14'); }); + it('returns the address of an IP-literal host', function (string $url, string $address): void { + expect(FakeHostResolver::policy()->validate($url))->toBe($address); + })->with([ + 'ipv4' => ['https://93.184.215.14/webhook', '93.184.215.14'], + 'ipv6' => ['https://[2606:2800:21f:cb07:6820:80da:af6b:8b2c]/webhook', '2606:2800:21f:cb07:6820:80da:af6b:8b2c'], + ]); + it('rejects the cloud metadata address', function (): void { expect(fn () => FakeHostResolver::policy()->validate('https://169.254.169.254/latest/meta-data/iam/')) ->toThrow(UnsafeWebhookUrlException::class, 'link-local range (169.254.0.0/16)');