Tighten up the default configuration for Trusted Proxies. Added Test Suite.

Author: uberbrady

.env.example

@@ -9,6 +9,13 @@ APP_TIMEZONE='UTC'
 APP_LOCALE='en-US'
 MAX_RESULTS=500
 
+# -------------------------------------------
+# TRUSTED PROXY SETTINGS
+# -------------------------------------------
+# (See config/trustedproxy.php for details)
+APP_TRUSTED_PROXIES=""
+APP_TRUSTED_HEADERS=""
+
 # --------------------------------------------
 # REQUIRED: UPLOADED FILE STORAGE SETTINGS
 # --------------------------------------------
@@ -286,4 +293,4 @@ SCIM_STANDARDS_COMPLIANCE=false
 # --------------------------------------------
 # OPTIONAL: WEBHOOK SETTINGS
 # --------------------------------------------
-WEBHOOK_ALLOW_INTERNAL_TARGETS=false
+WEBHOOK_ALLOW_INTERNAL_TARGETS=false

app/Http/Middleware/TrustProxies.php

@@ -2,27 +2,57 @@
 
 namespace App\Http\Middleware;
 
-use Illuminate\Http\Middleware\TrustProxies as Middleware;
+use Illuminate\Http\Middleware\TrustProxies as TrustedProxyMiddleware;
 use Illuminate\Http\Request;
 
-class TrustProxies extends Middleware
+class TrustProxies extends TrustedProxyMiddleware
 {
     /**
-     * The trusted proxies for this application.
+     * The trusted proxy headers computed from config('trustedproxy.headers').
      *
-     * @var array<int, string>|string|null
+     * Overrides the parent's $headers property/headers() fallback so that an
+     * empty or entirely invalid APP_TRUSTED_HEADERS results in trusting *no*
+     * headers, rather than silently falling back to the framework's default
+     * header set. See headers().
+     *
+     * @var int
      */
-    protected $proxies;
+    protected int $headerBitmask = 0;
+
+    public function __construct()
+    {
+        $header_bitmask = 0;
+
+        foreach (explode(",", config('trustedproxy.headers')) as $header) {
+            $header = trim($header);
+
+            if (!$header) {
+                continue;
+            }
+
+            try {
+                $header_bitmask |= constant(Request::class . "::$header"); // once we get to PHP 8.3, this can become Request::{$header}
+            } catch (\Throwable $e) {
+                \Log::error("Error parsing APP_TRUSTED_HEADERS: " . $header . " is not a valid setting, ignoring");
+            }
+        }
+        \Log::debug("Final header bitmask: $header_bitmask");
+
+        $this->headerBitmask = $header_bitmask;
+        // note, we do *not* need to also set the Proxies themselves since the middleware does that itself.
+    }
 
     /**
-     * The headers that should be used to detect proxies.
+     * Get the trusted headers.
      *
-     * @var int
+     * Deliberately does not fall back to the parent's hardcoded default
+     * header set: an empty/unconfigured APP_TRUSTED_HEADERS must mean *no*
+     * headers are trusted, not "trust everything".
+     *
+     * @return int
      */
-    protected $headers =
-        Request::HEADER_X_FORWARDED_FOR |
-        Request::HEADER_X_FORWARDED_HOST |
-        Request::HEADER_X_FORWARDED_PORT |
-        Request::HEADER_X_FORWARDED_PROTO |
-        Request::HEADER_X_FORWARDED_AWS_ELB;
+    protected function headers()
+    {
+        return $this->headerBitmask;
+    }
 }

config/trustedproxy.php

@@ -31,37 +31,48 @@
      * always gets the originating client IP, no matter
      * how many proxies that client's request has
      * subsequently passed through.
+     *
+     * If you're looking for a sensible default for this value,
+     * in many cases that would be 'PRIVATE_SUBNETS' or 'private_ranges'
+     * (They both do the same thing; they 'trust' any private non-routable
+     * IP, which is usually how most load-balancers and proxies are configured).
+     *
+     * This is a comma-separated list of IP addresses, **or** '*', or '**', or
+     * 'PRIVATE_SUBNETS' or 'private_ranges'
      */
-    'proxies' => env('APP_TRUSTED_PROXIES') !== null ?
-        explode(',', env('APP_TRUSTED_PROXIES')) : '*',
+    'proxies' => env('APP_TRUSTED_PROXIES') ?
+        explode(',', env('APP_TRUSTED_PROXIES')) : '',
 
     /*
-     * To trust one or more specific proxies that connect
-     * directly to your server, use an array of IP addresses:
+     * If APP_TRUSTED_HEADERS is left unset, NO forwarded headers are
+     * trusted, regardless of 'proxies' above. There is no fallback to a
+     * default header set - trusting X-Forwarded-* is strictly opt-in, one
+     * header at a time, via the setting below.
+     *
+     * This is a comma-delimited list.
      */
-    // 'proxies' => ['192.168.1.1'],
 
-    /*
-     * Or, to trust all proxies that connect
-     * directly to your server, use a "*"
-     */
-    // 'proxies' => '*',
+    'headers' => env('APP_TRUSTED_HEADERS', ''),
 
     /*
-     * Trusted forwarded-header list intentionally not configured here.
+     * These are the valid headers you can choose to trust.
      *
-     * The runtime defaults trust X-Forwarded-For, X-Forwarded-Host,
-     * X-Forwarded-Port, X-Forwarded-Proto, and the AWS ELB set. This is
-     * the right answer for essentially every reverse-proxy deployment,
-     * so there is nothing to change under normal circumstances.
+     * The list of headers should be comma-separated; use as many as you need, but only use those that you actually
+     * _need_ - don't just pick everything at random.
      *
-     * Older versions of this file shipped a commented-out example
-     * referencing Illuminate\Http\Request::HEADER_X_FORWARDED_ALL. That
-     * constant was removed from Symfony (see symfony/symfony#38928) and
-     * uncommenting the example produced a fatal "Undefined constant"
-     * error. It has been removed to avoid the foot-gun. See #6852.
+     * Generally you should *not* be using a combination of HEADER_X_FORWARDED_AWS_ELB and anything else,
+     * nor HEADER_X_FORWARDED_TRAEFIK and anything else. If you're using HEADER_FORWARDED, you generally won't
+     * need anything else either. the various X_FORWARDED_FOR, X_FORWARDED_HOST, etc are often used together.
      *
-     * @link https://symfony.com/doc/current/deployment/proxies.html
+       HEADER_FORWARDED // When using RFC 7239
+       HEADER_X_FORWARDED_FOR
+       HEADER_X_FORWARDED_HOST
+       HEADER_X_FORWARDED_PROTO
+       HEADER_X_FORWARDED_PORT
+       HEADER_X_FORWARDED_PREFIX
+
+       HEADER_X_FORWARDED_AWS_ELB // AWS ELB doesn't send X-Forwarded-Host
+       HEADER_X_FORWARDED_TRAEFIK // All "X-Forwarded-*" headers sent by Traefik reverse proxy
      */
 
 ];

tests/Unit/TrustedProxiesTest.php

@@ -0,0 +1,119 @@
+<?php
+
+namespace Tests\Unit;
+
+use App\Http\Middleware\TrustProxies;
+use Illuminate\Http\Request;
+use Tests\TestCase;
+
+class TrustedProxiesTest extends TestCase
+{
+    protected function tearDown(): void
+    {
+        // These are process-wide statics on the Symfony/Illuminate classes, not
+        // part of the Laravel application container, so they survive the
+        // per-test application reset and must be cleared manually.
+        Request::setTrustedProxies([], -1);
+        TrustProxies::flushState();
+
+        parent::tearDown();
+    }
+
+    private function makeSpoofedRequest(string $remoteAddr, string $forwardedFor): Request
+    {
+        $request = Request::create('http://example.com/test', 'GET', [], [], [], [
+            'REMOTE_ADDR' => $remoteAddr,
+        ]);
+        $request->headers->set('X-Forwarded-For', $forwardedFor);
+
+        return $request;
+    }
+
+    public function test_untrusted_direct_client_cannot_spoof_its_ip_by_default()
+    {
+        config([
+            'trustedproxy.proxies' => '',
+            'trustedproxy.headers' => '',
+        ]);
+
+        $request = $this->makeSpoofedRequest('203.0.113.9', '1.1.1.1');
+
+        $resolvedIp = (new TrustProxies())->handle($request, fn ($req) => $req->ip());
+
+        $this->assertSame('203.0.113.9', $resolvedIp);
+    }
+
+    public function test_a_configured_trusted_proxy_can_report_the_real_client_ip()
+    {
+        config([
+            'trustedproxy.proxies' => '10.0.0.5',
+            'trustedproxy.headers' => 'HEADER_X_FORWARDED_FOR',
+        ]);
+
+        $request = $this->makeSpoofedRequest('10.0.0.5', '203.0.113.42');
+
+        $resolvedIp = (new TrustProxies())->handle($request, fn ($req) => $req->ip());
+
+        $this->assertSame('203.0.113.42', $resolvedIp);
+    }
+
+    public function test_an_untrusted_client_cannot_spoof_its_ip_by_impersonating_a_trusted_proxy()
+    {
+        config([
+            'trustedproxy.proxies' => '10.0.0.5',
+            'trustedproxy.headers' => 'HEADER_X_FORWARDED_FOR',
+        ]);
+
+        $request = $this->makeSpoofedRequest('203.0.113.9', '1.1.1.1');
+
+        $resolvedIp = (new TrustProxies())->handle($request, fn ($req) => $req->ip());
+
+        $this->assertSame('203.0.113.9', $resolvedIp);
+    }
+
+    public function test_an_invalid_header_name_in_config_is_ignored_without_breaking_valid_ones()
+    {
+        config([
+            'trustedproxy.proxies' => '10.0.0.5',
+            'trustedproxy.headers' => 'NOT_A_REAL_HEADER,HEADER_X_FORWARDED_FOR',
+        ]);
+
+        $request = $this->makeSpoofedRequest('10.0.0.5', '203.0.113.42');
+
+        $resolvedIp = (new TrustProxies())->handle($request, fn ($req) => $req->ip());
+
+        $this->assertSame('203.0.113.42', $resolvedIp);
+    }
+
+    public function test_a_trusted_proxy_cannot_forward_headers_when_no_headers_are_configured()
+    {
+        config([
+            'trustedproxy.proxies' => '10.0.0.5',
+            'trustedproxy.headers' => '',
+        ]);
+
+        // The request genuinely comes from the trusted proxy, but since
+        // APP_TRUSTED_HEADERS is unset, X-Forwarded-For must still be
+        // ignored - it must NOT fall back to trusting the framework's
+        // default header set.
+        $request = $this->makeSpoofedRequest('10.0.0.5', '203.0.113.42');
+
+        $resolvedIp = (new TrustProxies())->handle($request, fn ($req) => $req->ip());
+
+        $this->assertSame('10.0.0.5', $resolvedIp);
+    }
+
+    public function test_a_trusted_proxy_cannot_forward_headers_when_every_configured_header_is_invalid()
+    {
+        config([
+            'trustedproxy.proxies' => '10.0.0.5',
+            'trustedproxy.headers' => 'NOT_A_REAL_HEADER,ALSO_NOT_REAL',
+        ]);
+
+        $request = $this->makeSpoofedRequest('10.0.0.5', '203.0.113.42');
+
+        $resolvedIp = (new TrustProxies())->handle($request, fn ($req) => $req->ip());
+
+        $this->assertSame('10.0.0.5', $resolvedIp);
+    }
+}