Check and pin a shop's address on every request, not just on the form

app/Actions/Shops/TestShopConnection.php

@@ -5,6 +5,7 @@
 use App\Data\ShopConnectionResult;
 use App\Enums\ShopConnectionStatus;
 use App\Models\Shop;
+use App\Services\Network\UnsafeDestinationException;
 use App\Services\WooCommerce\WooCommerceClient;
 use Illuminate\Http\Client\ConnectionException;
 use Illuminate\Http\Client\Response;
@@ -45,6 +46,14 @@ private function probe(Shop $shop): ShopConnectionResult
 
         try {
             $response = $this->client->get($shop, 'orders', $query);
+        } catch (UnsafeDestinationException) {
+            // Deliberately not the resolved address: whoever typed the URL
+            // does not need this server's reading of their DNS handed back.
+            return ShopConnectionResult::for(
+                ShopConnectionStatus::Unreachable,
+                'This address points to a private or reserved network, so it is not contacted.',
+                $this->elapsedMs($startedAt),
+            );
         } catch (ConnectionException $exception) {
             return ShopConnectionResult::for(
                 ShopConnectionStatus::Unreachable,
@@ -80,7 +89,7 @@ private function currencyOf(Shop $shop): ?string
             // Without _fields the store returns its whole currency list,
             // some twenty kilobytes of it, on every hourly check.
             $response = $this->client->get($shop, 'settings/general/woocommerce_currency', ['_fields' => 'id,value']);
-        } catch (ConnectionException) {
+        } catch (ConnectionException|UnsafeDestinationException) {
             return null;
         }
 

app/Rules/PublicShopUrl.php

@@ -2,7 +2,8 @@
 
 namespace App\Rules;
 
-use App\Services\Net\HostResolver;
+use App\Services\Network\PublicHostGuard;
+use App\Services\Network\UnsafeDestinationException;
 use Closure;
 use Illuminate\Contracts\Validation\ValidationRule;
 use Illuminate\Translation\PotentiallyTranslatedString;
@@ -14,10 +15,13 @@
  * attached, and the outcome is reported back in the interface. Without this a
  * member could aim a "shop" at a cloud metadata endpoint or an internal
  * service and read the difference between a refused port and an open one.
+ *
+ * This is the early feedback, not the protection. DNS can answer differently
+ * after the form is saved, so the client checks and pins every request too.
  */
 class PublicShopUrl implements ValidationRule
 {
-    public function __construct(private HostResolver $resolver) {}
+    public function __construct(private PublicHostGuard $guard) {}
 
     /**
      * Run the validation rule.
@@ -34,6 +38,10 @@ public function validate(string $attribute, mixed $value, Closure $fail): void
             return;
         }
 
+        if (config('services.woocommerce.allow_private_hosts')) {
+            return;
+        }
+
         // An IPv6 literal arrives wrapped in brackets.
         $host = trim($host, '[]');
 
@@ -46,26 +54,13 @@ public function validate(string $attribute, mixed $value, Closure $fail): void
             return;
         }
 
-        foreach ($this->resolver->addressesFor($host) as $address) {
-            if ($this->isPublic($address)) {
-                continue;
-            }
-
+        try {
+            // A host that does not resolve passes here. It may simply be down
+            // while someone corrects a typo elsewhere on the form, and the
+            // client refuses to connect to it either way.
+            $this->guard->publicAddresses($host);
+        } catch (UnsafeDestinationException) {
             $fail(__('That address resolves to a private network and cannot be reached as a shop.'));
-
-            return;
         }
     }
-
-    /**
-     * Determine whether an address is out on the public internet.
-     */
-    private function isPublic(string $address): bool
-    {
-        return filter_var(
-            $address,
-            FILTER_VALIDATE_IP,
-            FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE,
-        ) !== false;
-    }
 }

app/Services/Network/HostResolver.php

@@ -0,0 +1,36 @@
+<?php
+
+namespace App\Services\Network;
+
+class HostResolver
+{
+    /**
+     * Get every IPv4 and IPv6 address the host name resolves to in DNS.
+     *
+     * An empty list means the host could not be resolved, which callers must
+     * treat as unsafe rather than letting the HTTP client look the name up
+     * again on its own.
+     *
+     * @return array<int, string>
+     */
+    public function resolve(string $host): array
+    {
+        $addresses = [];
+
+        // Asked separately: some resolvers fail a combined A and AAAA query
+        // outright when the host has no records of one type.
+        foreach ([DNS_A => 'ip', DNS_AAAA => 'ipv6'] as $type => $field) {
+            $records = @dns_get_record($host, $type);
+
+            foreach ($records === false ? [] : $records as $record) {
+                $address = $record[$field] ?? null;
+
+                if (is_string($address) && filter_var($address, FILTER_VALIDATE_IP) !== false) {
+                    $addresses[] = $address;
+                }
+            }
+        }
+
+        return array_values(array_unique($addresses));
+    }
+}

app/Services/Network/PublicHostGuard.php

@@ -0,0 +1,66 @@
+<?php
+
+namespace App\Services\Network;
+
+use Symfony\Component\HttpFoundation\IpUtils;
+
+/**
+ * Keeps requests to user supplied hosts off private and reserved networks.
+ *
+ * Every address a host resolves to is checked, since a request may connect to
+ * any of them; one private answer among public ones is enough to refuse.
+ */
+class PublicHostGuard
+{
+    /**
+     * Ranges to refuse beyond the private subnets Symfony already lists.
+     */
+    private const array EXTRA_BLOCKED_SUBNETS = [
+        '192.0.0.0/24',    // IETF protocol assignments
+        '192.88.99.0/24',  // 6to4 relay anycast
+        '224.0.0.0/4',     // Multicast
+        '64:ff9b::/96',    // NAT64, which embeds an IPv4 address
+        '64:ff9b:1::/48',  // Local-use NAT64
+        '100::/64',        // Discard-only
+        'ff00::/8',        // Multicast
+    ];
+
+    public function __construct(private readonly HostResolver $resolver)
+    {
+        //
+    }
+
+    /**
+     * Resolve the host, refusing it if any address is not public.
+     *
+     * @return array<int, string> an empty list when the host did not resolve
+     *
+     * @throws UnsafeDestinationException
+     */
+    public function publicAddresses(string $host): array
+    {
+        // An IP literal is its own answer and is never looked up.
+        $literal = trim($host, '[]');
+
+        $addresses = filter_var($literal, FILTER_VALIDATE_IP) !== false
+            ? [$literal]
+            : $this->resolver->resolve($host);
+
+        foreach ($addresses as $address) {
+            if (! $this->isPublic($address)) {
+                throw new UnsafeDestinationException("The host {$host} resolves to the non-public address {$address}.");
+            }
+        }
+
+        return $addresses;
+    }
+
+    /**
+     * Determine if the address is on the public internet.
+     */
+    public function isPublic(string $address): bool
+    {
+        return filter_var($address, FILTER_VALIDATE_IP) !== false
+            && ! IpUtils::checkIp($address, [...IpUtils::PRIVATE_SUBNETS, ...self::EXTRA_BLOCKED_SUBNETS]);
+    }
+}

app/Services/Network/UnsafeDestinationException.php

@@ -0,0 +1,10 @@
+<?php
+
+namespace App\Services\Network;
+
+use RuntimeException;
+
+class UnsafeDestinationException extends RuntimeException
+{
+    //
+}

app/Services/WooCommerce/WooCommerceClient.php

@@ -3,6 +3,9 @@
 namespace App\Services\WooCommerce;
 
 use App\Models\Shop;
+use App\Services\Network\PublicHostGuard;
+use App\Services\Network\UnsafeDestinationException;
+use Illuminate\Http\Client\ConnectionException;
 use Illuminate\Http\Client\PendingRequest;
 use Illuminate\Http\Client\Response;
 use Illuminate\Support\Facades\Http;
@@ -50,15 +53,20 @@ public function withTimeout(int $seconds): self
      */
     public function get(Shop $shop, string $path, array $query = []): Response
     {
-        $response = $this->request()
+        // Resolved once for the pair: the fallback must go to the same
+        // addresses the first attempt was checked against, and asking twice
+        // would give a rebinding host a second chance to answer differently.
+        $destination = $this->pinnedDestination($shop);
+
+        $response = $this->request($destination)
             ->withBasicAuth($shop->consumer_key, $shop->consumer_secret)
             ->get($this->endpoint($shop, $path), $query);
 
         if ($response->status() !== 401) {
             return $response;
         }
 
-        return $this->request()->get($this->endpoint($shop, $path), [
+        return $this->request($destination)->get($this->endpoint($shop, $path), [
             ...$query,
             'consumer_key' => $shop->consumer_key,
             'consumer_secret' => $shop->consumer_secret,
@@ -82,12 +90,65 @@ public function endpoint(Shop $shop, string $path): string
      *
      * The default timeout is deliberately short: a manual connection test runs
      * inside a web request, so a hanging shop must never hang the page.
+     *
+     * @param  array<string, mixed>  $destination
      */
-    private function request(): PendingRequest
+    private function request(array $destination = []): PendingRequest
     {
         return Http::acceptJson()
             ->withUserAgent(config('app.name').'/1.0')
+            ->withOptions($destination)
             ->connectTimeout((int) config('services.woocommerce.connect_timeout'))
             ->timeout($this->timeout ?? (int) config('services.woocommerce.timeout'));
     }
+
+    /**
+     * Check where the shop's host points now, and nail the request to it.
+     *
+     * The rule on the form is early feedback, nothing more: the name is
+     * looked up again there and then, and whoever controls it can answer
+     * publicly while the form is saved and privately once the sync runs.
+     * Checking here closes that gap, and pinning the answer closes the one
+     * that is left, where curl looks the name up a third time and gets a
+     * different reply than the one just approved.
+     *
+     * @return array<string, mixed>
+     *
+     * @throws UnsafeDestinationException
+     * @throws ConnectionException
+     */
+    private function pinnedDestination(Shop $shop): array
+    {
+        if (config('services.woocommerce.allow_private_hosts')) {
+            return [];
+        }
+
+        $host = trim((string) parse_url($shop->url, PHP_URL_HOST), '[]');
+
+        $addresses = app(PublicHostGuard::class)->publicAddresses($host);
+
+        if ($addresses === []) {
+            throw new ConnectionException("The shop host {$host} could not be resolved.");
+        }
+
+        // A proxy from HTTP_PROXY or HTTPS_PROXY would make the connection to
+        // the proxy instead and resolve the shop on the far side, where the
+        // pin does not reach. An empty proxy is Guzzle's final "no proxy".
+        $direct = ['proxy' => ''];
+
+        // An IP literal is never looked up, so there is nothing to pin.
+        if (filter_var($host, FILTER_VALIDATE_IP) !== false) {
+            return $direct;
+        }
+
+        $port = parse_url($shop->url, PHP_URL_PORT)
+            ?? (strtolower((string) parse_url($shop->url, PHP_URL_SCHEME)) === 'http' ? 80 : 443);
+
+        $pinned = implode(',', array_map(
+            fn (string $address) => str_contains($address, ':') ? "[{$address}]" : $address,
+            $addresses,
+        ));
+
+        return [...$direct, 'curl' => [CURLOPT_RESOLVE => ["{$host}:{$port}:{$pinned}"]]];
+    }
 }

config/services.php

@@ -36,6 +36,11 @@
     ],
 
     'woocommerce' => [
+        // Lets a shop live on a private or loopback address, for a fake shop
+        // during local development. Never enable it where the app runs next
+        // to anything a user should not be able to reach through it.
+        'allow_private_hosts' => (bool) env('WOOCOMMERCE_ALLOW_PRIVATE_HOSTS', env('APP_ENV') === 'local'),
+
         'connect_timeout' => (int) env('WOOCOMMERCE_CONNECT_TIMEOUT', 5),
         'timeout' => (int) env('WOOCOMMERCE_TIMEOUT', 10),
         'recheck_after_minutes' => (int) env('WOOCOMMERCE_RECHECK_AFTER_MINUTES', 60),