Răsfoiți Sursa

fix: tight redirect policy + private-host guard on GeoIP client (SEC_REVIEW F50)

The GeoIP-downloader Guzzle client in `Container.php` set timeouts
but accepted the Guzzle defaults for `allow_redirects` (5 hops, any
protocol incl. file://, includes Referer). The base URLs for
db-ip / MaxMind / ipinfo are HTTPS constants in the default deploy,
but a malicious upstream or DNS poisoning could 302 a download to
`http://169.254.169.254/...` (instance metadata) or `file://...`
(local file inclusion). Pure defence-in-depth, but the cost is low.

Two layers added:

  1. Tight `allow_redirects` config:
       `['max' => 3, 'protocols' => ['https'], 'strict' => true,
         'referer' => false, 'track_redirects' => false]`
     Caps the chain at 3 hops, refuses non-HTTPS redirect targets
     (kills `file://` / `http://` chains), and stops leaking the
     download URL via Referer.

  2. New `PrivateHostGuardMiddleware` plugged into the handler stack
     before the request goes to the network. Inspects the URL's
     literal host on every outgoing request — including each
     redirect target Guzzle emits internally — and throws
     `GuzzleHttp\Exception\TransferException` before opening a
     socket to:
       - Literal blocked names: `localhost`, `0.0.0.0`,
         `169.254.169.254`, `metadata.google.internal`.
       - IPv4 ranges: 10/8, 127/8, 169.254/16 (link-local incl.
         metadata), 172.16/12, 192.168/16, 100.64/10 (CGNAT),
         224/4 (multicast), 0/8.
       - IPv6 ranges: ::1 (loopback), :: (unspec), fe80::/10
         (link-local), fc00::/7 (unique-local), ff00::/8
         (multicast). Square-bracketed URL forms (`[::1]`) are
         normalised before the lookup.

The guard inspects literal hosts only — pinning DNS to catch a
public hostname that points at a private IP is out of scope for
"defence in depth"; the primary controls are the constant base
URL plus `protocols => ['https']`.

Regression tests in
`api/tests/Unit/Enrichment/PrivateHostGuardMiddlewareTest.php`:
17 blocked-host data-provider cases (IPv4 + IPv6 across all the
ranges above + the metadata hostname + `localhost`), 6 allowed-host
cases (`download.db-ip.com`, `download.maxmind.com`, `ipinfo.io`,
just-outside-172.16/12, `1.1.1.1`, public IPv6 `2001:db8::1`),
`testFactoryProducesMiddlewareThatGuardsBeforeHandler` proving the
inner handler is NOT invoked when the guard rejects, and
`testEmptyHostIsRejected`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chiappa 3 zile în urmă
părinte
comite
6cc66ef4ec

+ 17 - 0
api/src/App/Container.php

@@ -279,10 +279,27 @@ final class Container
             }),
             CleanupExpiredManualBlocksJob::class => autowire(),
             GuzzleClientInterface::class => factory(static function (): GuzzleClientInterface {
+                // SEC_REVIEW F50: lock the GeoIP downloader's HTTP client
+                // down. `allow_redirects` is now a tight whitelist —
+                // HTTPS-only, ≤3 redirects, no Referer header. The
+                // PrivateHostGuardMiddleware refuses to dial loopback /
+                // link-local / RFC1918 / metadata-service hosts even
+                // after a redirect rewrites the URL.
+                $stack = \GuzzleHttp\HandlerStack::create();
+                $stack->push(\App\Infrastructure\Enrichment\Downloaders\PrivateHostGuardMiddleware::factory());
+
                 return new GuzzleClient([
+                    'handler' => $stack,
                     'connect_timeout' => 10,
                     'timeout' => 120,
                     'http_errors' => true,
+                    'allow_redirects' => [
+                        'max' => 3,
+                        'protocols' => ['https'],
+                        'strict' => true,
+                        'referer' => false,
+                        'track_redirects' => false,
+                    ],
                     'headers' => [
                         'User-Agent' => 'irdb-geoip-refresh/1.0',
                     ],

+ 167 - 0
api/src/Infrastructure/Enrichment/Downloaders/PrivateHostGuardMiddleware.php

@@ -0,0 +1,167 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Infrastructure\Enrichment\Downloaders;
+
+use GuzzleHttp\Exception\TransferException;
+use Psr\Http\Message\RequestInterface;
+
+/**
+ * SEC_REVIEW F50: defence-in-depth guard around the GeoIP downloader's
+ * HTTP client. Rejects requests whose URL points at a literal
+ * loopback / link-local / multicast / private host.
+ *
+ * The downloaders' base URLs are constants in the default deploy
+ * (`https://download.maxmind.com`, `https://download.db-ip.com`,
+ * `https://ipinfo.io`), so the only way a request to a private
+ * destination shows up here is via a 30x redirect or a DNS-poisoning
+ * MitM. Combined with `allow_redirects.protocols=['https']` and a
+ * tight redirect cap, this middleware blocks the post-redirect
+ * `http://169.254.169.254/...` / `https://localhost/...` /
+ * `https://10.0.0.1/...` patterns at the request layer before Guzzle
+ * opens a socket to the dangerous host.
+ *
+ * The guard inspects the URL's literal host string only — it does NOT
+ * resolve DNS to catch a public hostname that points at a private IP.
+ * Pinning DNS is out of scope for "defence in depth"; the primary
+ * controls are the constant base URL and the
+ * `allow_redirects.protocols=['https']` config.
+ *
+ * Plug into a Guzzle handler stack with
+ * `$stack->push(PrivateHostGuardMiddleware::factory())`.
+ */
+final class PrivateHostGuardMiddleware
+{
+    /**
+     * Hostnames + IP CIDRs we refuse to dial. Covers the AWS / GCP /
+     * Azure instance-metadata addresses, IPv6 link-local, and the
+     * RFC1918 ranges typical operators don't intend to expose.
+     *
+     * @var list<string>
+     */
+    private const BLOCKED_HOSTS = [
+        'localhost',
+        '0.0.0.0',
+        '169.254.169.254', // AWS / GCP / Azure metadata
+        'metadata.google.internal',
+    ];
+
+    /**
+     * Returns the middleware closure Guzzle expects. Signature is
+     * `function (callable $handler): callable`.
+     */
+    public static function factory(): callable
+    {
+        return static function (callable $handler): callable {
+            return static function (RequestInterface $request, array $options) use ($handler) {
+                self::assertHostAllowed($request);
+
+                return $handler($request, $options);
+            };
+        };
+    }
+
+    /**
+     * Public so tests and the IPinfo downloader can call it directly
+     * (e.g. for redirect targets resolved out-of-band).
+     *
+     * @internal
+     */
+    public static function assertHostAllowed(RequestInterface $request): void
+    {
+        $host = strtolower($request->getUri()->getHost());
+        if ($host === '') {
+            throw new TransferException('refusing to dial empty host');
+        }
+
+        // Strip an IPv6 literal's brackets — `[::1]` → `::1`.
+        if (str_starts_with($host, '[') && str_ends_with($host, ']')) {
+            $host = substr($host, 1, -1);
+        }
+
+        if (in_array($host, self::BLOCKED_HOSTS, true)) {
+            throw new TransferException(sprintf('refusing to dial blocked host: %s', $host));
+        }
+
+        // IPv4: literal dotted quad
+        if (preg_match('/^\d{1,3}(?:\.\d{1,3}){3}$/', $host) === 1) {
+            $ip = ip2long($host);
+            if ($ip === false) {
+                throw new TransferException(sprintf('refusing to dial malformed ipv4 host: %s', $host));
+            }
+            if (self::ipv4InBlockedRanges($ip)) {
+                throw new TransferException(sprintf('refusing to dial private/loopback ipv4 host: %s', $host));
+            }
+        }
+
+        // IPv6: anything containing colons. Treat unique-local
+        // (fc00::/7), link-local (fe80::/10), and loopback (::1) as
+        // blocked; multicast (ff00::/8) too for good measure.
+        if (str_contains($host, ':')) {
+            $packed = @inet_pton($host);
+            if ($packed === false) {
+                throw new TransferException(sprintf('refusing to dial malformed ipv6 host: %s', $host));
+            }
+            if (strlen($packed) === 16 && self::ipv6InBlockedRanges($packed)) {
+                throw new TransferException(sprintf('refusing to dial private/loopback ipv6 host: %s', $host));
+            }
+        }
+    }
+
+    private static function ipv4InBlockedRanges(int $ip): bool
+    {
+        // ip2long can return a negative int on 32-bit PHP for >2^31 hosts;
+        // operate on it as unsigned via bitwise AND with the netmask.
+        $ranges = [
+            ['10.0.0.0', 8],
+            ['127.0.0.0', 8],
+            ['169.254.0.0', 16], // link-local incl. 169.254.169.254 metadata
+            ['172.16.0.0', 12],
+            ['192.168.0.0', 16],
+            ['100.64.0.0', 10], // CGNAT
+            ['224.0.0.0', 4], // multicast
+            ['0.0.0.0', 8],
+        ];
+        foreach ($ranges as [$net, $prefix]) {
+            $netLong = ip2long((string) $net);
+            if ($netLong === false) {
+                continue;
+            }
+            // Each prefix in $ranges is in [4, 32]; no /0 case.
+            $mask = -1 << (32 - $prefix);
+            if (($ip & $mask) === ($netLong & $mask)) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    private static function ipv6InBlockedRanges(string $packed16): bool
+    {
+        $first = ord($packed16[0]);
+        // ::1 (loopback)
+        if ($packed16 === "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01") {
+            return true;
+        }
+        // ::0 (unspec)
+        if ($packed16 === str_repeat("\x00", 16)) {
+            return true;
+        }
+        // fe80::/10 link-local
+        if ($first === 0xfe && (ord($packed16[1]) & 0xc0) === 0x80) {
+            return true;
+        }
+        // fc00::/7 unique-local
+        if (($first & 0xfe) === 0xfc) {
+            return true;
+        }
+        // ff00::/8 multicast
+        if ($first === 0xff) {
+            return true;
+        }
+
+        return false;
+    }
+}

+ 125 - 0
api/tests/Unit/Enrichment/PrivateHostGuardMiddlewareTest.php

@@ -0,0 +1,125 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Unit\Enrichment;
+
+use App\Infrastructure\Enrichment\Downloaders\PrivateHostGuardMiddleware;
+use GuzzleHttp\Exception\TransferException;
+use GuzzleHttp\Psr7\Request;
+use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\TestCase;
+use Psr\Http\Message\RequestInterface;
+
+/**
+ * SEC_REVIEW F50 — defence-in-depth host guard around the GeoIP
+ * downloader's Guzzle client. Refuses to dial loopback, link-local,
+ * RFC1918, CGNAT, multicast, instance-metadata hostnames, and
+ * IPv6 link-local / unique-local / loopback / multicast / unspec
+ * addresses. Public production hosts (`download.db-ip.com`,
+ * `download.maxmind.com`, `ipinfo.io`) pass through.
+ */
+final class PrivateHostGuardMiddlewareTest extends TestCase
+{
+    /**
+     * @return iterable<string, array{string}>
+     */
+    public static function blockedHosts(): iterable
+    {
+        // Literal blocked names.
+        yield 'localhost' => ['https://localhost/foo'];
+        yield 'localhost:8080' => ['https://localhost:8080/foo'];
+        yield 'metadata.google.internal' => ['https://metadata.google.internal/computeMetadata/v1/'];
+        yield 'cloud metadata IP' => ['https://169.254.169.254/latest/meta-data/'];
+
+        // IPv4 ranges.
+        yield 'loopback v4' => ['https://127.0.0.1/'];
+        yield 'rfc1918 10/8' => ['https://10.0.0.1/'];
+        yield 'rfc1918 172.16/12 inside' => ['https://172.16.0.1/'];
+        yield 'rfc1918 172.31/12 (boundary)' => ['https://172.31.255.255/'];
+        yield 'rfc1918 192.168/16' => ['https://192.168.1.1/'];
+        yield 'cgnat 100.64/10' => ['https://100.64.0.1/'];
+        yield 'multicast' => ['https://224.0.0.1/'];
+        yield 'all-zero unspec v4' => ['https://0.0.0.0/'];
+
+        // IPv6 ranges. Square brackets in URL hosts are stripped
+        // before lookup so the guard sees the literal address.
+        yield 'loopback v6' => ['https://[::1]/'];
+        yield 'unspec v6' => ['https://[::]/'];
+        yield 'link-local v6' => ['https://[fe80::1]/'];
+        yield 'unique-local v6' => ['https://[fc00::1]/'];
+        yield 'multicast v6' => ['https://[ff02::1]/'];
+    }
+
+    #[DataProvider('blockedHosts')]
+    public function testBlockedHostThrows(string $url): void
+    {
+        $request = new Request('GET', $url);
+        $this->expectException(TransferException::class);
+        PrivateHostGuardMiddleware::assertHostAllowed($request);
+    }
+
+    /**
+     * @return iterable<string, array{string}>
+     */
+    public static function allowedHosts(): iterable
+    {
+        yield 'db-ip download host' => ['https://download.db-ip.com/free/dbip-country-lite-2026-04.mmdb.gz'];
+        yield 'maxmind download host' => ['https://download.maxmind.com/app/geoip_download'];
+        yield 'ipinfo host' => ['https://ipinfo.io/data/free/country.mmdb'];
+        // 172.32.x.x is just outside RFC1918's 172.16/12 range — must pass.
+        yield 'just outside 172.16/12' => ['https://172.32.0.1/'];
+        // Public DNS resolver IP (should pass — it's a public address).
+        yield 'public 1.1.1.1' => ['https://1.1.1.1/'];
+        // Public IPv6 (RFC 3849 docs prefix; not in any blocked range).
+        yield 'public v6 2001:db8' => ['https://[2001:db8::1]/'];
+    }
+
+    #[DataProvider('allowedHosts')]
+    public function testAllowedHostPasses(string $url): void
+    {
+        $request = new Request('GET', $url);
+        PrivateHostGuardMiddleware::assertHostAllowed($request);
+        // No assertion needed — absence of an exception is success.
+        $this->expectNotToPerformAssertions();
+    }
+
+    public function testFactoryProducesMiddlewareThatGuardsBeforeHandler(): void
+    {
+        // The factory closure should pass through allowed hosts and
+        // block disallowed hosts — without ever invoking the handler
+        // for the blocked branch.
+        $handlerCalled = false;
+        $handler = static function (RequestInterface $request, array $options) use (&$handlerCalled): mixed {
+            $handlerCalled = true;
+
+            return new \GuzzleHttp\Psr7\Response(200);
+        };
+        $middleware = PrivateHostGuardMiddleware::factory()($handler);
+
+        // Allowed: handler runs.
+        $middleware(new Request('GET', 'https://download.db-ip.com/'), []);
+        self::assertTrue($handlerCalled);
+
+        // Blocked: throw before reaching the handler.
+        $handlerCalled = false;
+        $threw = false;
+        try {
+            $middleware(new Request('GET', 'https://169.254.169.254/'), []);
+        } catch (TransferException) {
+            $threw = true;
+        }
+        self::assertTrue($threw, 'expected guard to throw for metadata host');
+        self::assertFalse($handlerCalled, 'handler must not be invoked for blocked host');
+    }
+
+    public function testEmptyHostIsRejected(): void
+    {
+        // Construct a Request with no Host part by going through Uri.
+        $uri = (new \GuzzleHttp\Psr7\Uri('https://download.db-ip.com/'))->withHost('');
+        $request = (new Request('GET', 'https://download.db-ip.com/'))->withUri($uri);
+
+        $this->expectException(TransferException::class);
+        PrivateHostGuardMiddleware::assertHostAllowed($request);
+    }
+}