From bdb23594ad3e0c47c8568ce028a7c244a406cf9d Mon Sep 17 00:00:00 2001
From: Buster Neece <buster@busterneece.com>
Date: Fri, 28 Apr 2023 01:47:44 -0500
Subject: [PATCH] Tighten allowed IPs to avoid brute-force workarounds.

---
 CHANGELOG.md                                  |  4 ++
 frontend/vue/components/Admin/Settings.vue    |  5 +-
 .../Admin/Settings/SecurityPrivacyTab.vue     | 46 ++++++++++++++-
 src/Controller/Api/Admin/RelaysController.php |  5 +-
 .../Api/Stations/Requests/SubmitAction.php    |  8 ++-
 src/Entity/Enums/IpSources.php                | 56 +++++++++++++++++++
 .../Migration/Version20230428062001.php       | 26 +++++++++
 src/Entity/Settings.php                       | 32 +++++++++++
 src/Http/ServerRequest.php                    | 27 ---------
 src/RateLimit.php                             |  6 +-
 10 files changed, 181 insertions(+), 34 deletions(-)
 create mode 100644 src/Entity/Enums/IpSources.php
 create mode 100644 src/Entity/Migration/Version20230428062001.php

diff --git a/CHANGELOG.md b/CHANGELOG.md
index ee16191088..abac36d980 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,10 @@ release channel, you can take advantage of these new features and fixes.
 
 ## Code Quality/Technical Changes
 
+- In sections of our application that depend on IP addresses, we've tightened our allowed IP addresses significantly to
+  improve security and prevent brute-force flooding. If you're using a reverse proxy or CloudFlare, you should update
+  your "IP Address Source" under the "System Settings" page.
+
 ## Bug Fixes
 
 ---
diff --git a/frontend/vue/components/Admin/Settings.vue b/frontend/vue/components/Admin/Settings.vue
index 580f62bef8..d1d39639a3 100644
--- a/frontend/vue/components/Admin/Settings.vue
+++ b/frontend/vue/components/Admin/Settings.vue
@@ -133,6 +133,7 @@ const {form, v$, ifValid} = useVuelidateOnForm(
 
         always_use_ssl: {},
         api_access_control: {},
+        ip_source: {},
 
         check_for_updates: {},
         acme_email: {},
@@ -150,13 +151,14 @@ const {form, v$, ifValid} = useVuelidateOnForm(
         use_external_album_art_in_apis: {},
         use_external_album_art_when_processing_media: {},
         last_fm_api_key: {},
+
         $validationGroups: {
             generalTab: [
                 'base_url', 'instance_name', 'prefer_browser_url', 'use_radio_proxy',
                 'history_keep_days', 'enable_static_nowplaying', 'enable_advanced_features'
             ],
             securityPrivacyTab: [
-                'analytics', 'always_use_ssl', 'api_access_control'
+                'analytics', 'always_use_ssl', 'ip_source', 'api_access_control'
             ],
             servicesTab: [
                 'check_for_updates',
@@ -179,6 +181,7 @@ const {form, v$, ifValid} = useVuelidateOnForm(
         enable_advanced_features: true,
         analytics: null,
         always_use_ssl: false,
+        ip_source: 'local',
         api_access_control: '*',
         check_for_updates: 1,
         acme_email: '',
diff --git a/frontend/vue/components/Admin/Settings/SecurityPrivacyTab.vue b/frontend/vue/components/Admin/Settings/SecurityPrivacyTab.vue
index ad5e6859c6..5ced1c9a1f 100644
--- a/frontend/vue/components/Admin/Settings/SecurityPrivacyTab.vue
+++ b/frontend/vue/components/Admin/Settings/SecurityPrivacyTab.vue
@@ -69,9 +69,32 @@
                 </template>
             </b-wrapped-form-checkbox>
 
+            <b-wrapped-form-group
+                id="edit_form_ip_source"
+                class="col-md-6"
+                :field="form.ip_source"
+            >
+                <template #label>
+                    {{ $gettext('IP Address Source') }}
+                </template>
+                <template #description>
+                    {{
+                        $gettext('Customize this setting to ensure you get the correct IP address for remote users. Only change this setting if you use a reverse proxy, either within Docker or a third-party service like CloudFlare.')
+                    }}
+                </template>
+                <template #default="slotProps">
+                    <b-form-radio-group
+                        :id="slotProps.id"
+                        v-model="slotProps.field.$model"
+                        stacked
+                        :options="ipSourceOptions"
+                    />
+                </template>
+            </b-wrapped-form-group>
+
             <b-wrapped-form-group
                 id="edit_form_api_access_control"
-                class="col-md-12"
+                class="col-md-6"
                 :field="form.api_access_control"
             >
                 <template #label>
@@ -98,6 +121,8 @@
 import BWrappedFormGroup from "~/components/Form/BWrappedFormGroup.vue";
 import BFormFieldset from "~/components/Form/BFormFieldset.vue";
 import BWrappedFormCheckbox from "~/components/Form/BWrappedFormCheckbox.vue";
+import {useTranslate} from "~/vendor/gettext";
+import {computed} from "vue";
 
 const props = defineProps({
     form: {
@@ -105,4 +130,23 @@ const props = defineProps({
         required: true
     }
 });
+
+const {$gettext} = useTranslate();
+
+const ipSourceOptions = computed(() => {
+    return [
+        {
+            value: 'local',
+            text: $gettext('Local IP (Default)')
+        },
+        {
+            value: 'cloudflare',
+            text: $gettext('CloudFlare (CF-Connecting-IP)')
+        },
+        {
+            value: 'xff',
+            text: $gettext('Reverse Proxy (X-Forwarded-For)')
+        }
+    ]
+});
 </script>
diff --git a/src/Controller/Api/Admin/RelaysController.php b/src/Controller/Api/Admin/RelaysController.php
index 3bc5b8ad7f..5f28bb1972 100644
--- a/src/Controller/Api/Admin/RelaysController.php
+++ b/src/Controller/Api/Admin/RelaysController.php
@@ -38,7 +38,8 @@ final class RelaysController
 {
     public function __construct(
         private readonly EntityManagerInterface $em,
-        private readonly Adapters $adapters
+        private readonly Adapters $adapters,
+        private readonly Entity\Repository\SettingsRepository $settingsRepo
     ) {
     }
 
@@ -126,7 +127,7 @@ public function updateAction(
             $base_url = $body['base_url'];
         } else {
             /** @noinspection HttpUrlsUsage */
-            $base_url = 'http://' . $request->getIp();
+            $base_url = 'http://' . $this->settingsRepo->readSettings()->getIp($request);
         }
 
         $relay = $relay_repo->findOneBy(['base_url' => $base_url]);
diff --git a/src/Controller/Api/Stations/Requests/SubmitAction.php b/src/Controller/Api/Stations/Requests/SubmitAction.php
index becef20729..9b82a1f502 100644
--- a/src/Controller/Api/Stations/Requests/SubmitAction.php
+++ b/src/Controller/Api/Stations/Requests/SubmitAction.php
@@ -6,6 +6,7 @@
 
 use App\Entity\Api\Error;
 use App\Entity\Api\Status;
+use App\Entity\Repository\SettingsRepository;
 use App\Entity\Repository\StationRequestRepository;
 use App\Entity\User;
 use App\Exception\InvalidRequestAttribute;
@@ -43,7 +44,8 @@
 final class SubmitAction
 {
     public function __construct(
-        private readonly StationRequestRepository $requestRepo
+        private readonly StationRequestRepository $requestRepo,
+        private readonly SettingsRepository $settingsRepo
     ) {
     }
 
@@ -64,11 +66,13 @@ public function __invoke(
         $isAuthenticated = ($user instanceof User);
 
         try {
+            $ip = $this->settingsRepo->readSettings()->getIp($request);
+
             $this->requestRepo->submit(
                 $station,
                 $media_id,
                 $isAuthenticated,
-                $request->getIp(),
+                $ip,
                 $request->getHeaderLine('User-Agent')
             );
 
diff --git a/src/Entity/Enums/IpSources.php b/src/Entity/Enums/IpSources.php
new file mode 100644
index 0000000000..271390217a
--- /dev/null
+++ b/src/Entity/Enums/IpSources.php
@@ -0,0 +1,56 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Entity\Enums;
+
+use Psr\Http\Message\ServerRequestInterface;
+
+enum IpSources: string
+{
+    case Local = 'local';
+    case XForwardedFor = 'xff';
+    case Cloudflare = 'cloudflare';
+
+    public static function default(): self
+    {
+        return self::Local;
+    }
+
+    public function getIp(ServerRequestInterface $request): string
+    {
+        if (self::Cloudflare === $this) {
+            $ip = $request->getHeaderLine('CF-Connecting-IP');
+            if (!empty($ip)) {
+                return $this->parseIp($ip);
+            }
+        }
+
+        if (self::XForwardedFor === $this) {
+            $ip = $request->getHeaderLine('X-Forwarded-For');
+            if (!empty($ip)) {
+                return $this->parseIp($ip);
+            }
+        }
+
+        $serverParams = $request->getServerParams();
+        $ip = $serverParams['REMOTE_ADDR'] ?? null;
+
+        if (empty($ip)) {
+            throw new \RuntimeException('No IP address attached to this request.');
+        }
+
+        return $this->parseIp($ip);
+    }
+
+    private function parseIp(string $ip): string
+    {
+        // Handle the IP being separated by commas.
+        if (str_contains($ip, ',')) {
+            $ipParts = explode(',', $ip);
+            $ip = array_shift($ipParts);
+        }
+
+        return trim($ip);
+    }
+}
diff --git a/src/Entity/Migration/Version20230428062001.php b/src/Entity/Migration/Version20230428062001.php
new file mode 100644
index 0000000000..7dd874488e
--- /dev/null
+++ b/src/Entity/Migration/Version20230428062001.php
@@ -0,0 +1,26 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Entity\Migration;
+
+use Doctrine\DBAL\Schema\Schema;
+use Doctrine\Migrations\AbstractMigration;
+
+final class Version20230428062001 extends AbstractMigration
+{
+    public function getDescription(): string
+    {
+        return 'Add "IP Source" setting.';
+    }
+
+    public function up(Schema $schema): void
+    {
+        $this->addSql('ALTER TABLE settings ADD ip_source VARCHAR(50) DEFAULT NULL');
+    }
+
+    public function down(Schema $schema): void
+    {
+        $this->addSql('ALTER TABLE settings DROP ip_source');
+    }
+}
diff --git a/src/Entity/Settings.php b/src/Entity/Settings.php
index 0ddb7c2d7d..e44bb49b94 100644
--- a/src/Entity/Settings.php
+++ b/src/Entity/Settings.php
@@ -14,6 +14,7 @@
 use Doctrine\ORM\Mapping as ORM;
 use InvalidArgumentException;
 use OpenApi\Attributes as OA;
+use Psr\Http\Message\ServerRequestInterface;
 use Psr\Http\Message\UriInterface;
 use RuntimeException;
 use Stringable;
@@ -1059,6 +1060,37 @@ static function ($str) {
         $this->acme_domains = $acme_domains;
     }
 
+    #[
+        OA\Property(description: "IP Address Source"),
+        ORM\Column(length: 50, nullable: true),
+        Groups(self::GROUP_GENERAL)
+    ]
+    protected ?string $ip_source = null;
+
+    public function getIpSource(): string
+    {
+        return $this->ip_source ?? Entity\Enums\IpSources::default()->value;
+    }
+
+    public function getIpSourceEnum(): Entity\Enums\IpSources
+    {
+        return Entity\Enums\IpSources::tryFrom($this->ip_source ?? '') ?? Entity\Enums\IpSources::default();
+    }
+
+    public function getIp(ServerRequestInterface $request): string
+    {
+        return $this->getIpSourceEnum()->getIp($request);
+    }
+
+    public function setIpSource(?string $ipSource): void
+    {
+        if (null !== $ipSource && null === Entity\Enums\IpSources::tryFrom($ipSource)) {
+            throw new InvalidArgumentException('Invalid IP source.');
+        }
+
+        $this->ip_source = $ipSource;
+    }
+
     public function __toString(): string
     {
         return 'Settings';
diff --git a/src/Http/ServerRequest.php b/src/Http/ServerRequest.php
index e0265e0a0a..acb33dc06f 100644
--- a/src/Http/ServerRequest.php
+++ b/src/Http/ServerRequest.php
@@ -14,7 +14,6 @@
 use App\Session;
 use App\View;
 use Mezzio\Session\SessionInterface;
-use RuntimeException;
 
 final class ServerRequest extends \Slim\Http\ServerRequest
 {
@@ -122,30 +121,4 @@ private function getAttributeOfClass(string $attr, string $class_name): mixed
 
         return $object;
     }
-
-    /**
-     * Get the remote user's IP address as indicated by HTTP headers.
-     */
-    public function getIp(): string
-    {
-        $params = $this->serverRequest->getServerParams();
-
-        $ip = $params['HTTP_CLIENT_IP']
-            ?? $params['HTTP_X_FORWARDED_FOR']
-            ?? $params['HTTP_X_FORWARDED']
-            ?? $params['HTTP_FORWARDED_FOR']
-            ?? $params['HTTP_FORWARDED']
-            ?? $params['REMOTE_ADDR']
-            ?? null;
-
-        if (null === $ip) {
-            throw new RuntimeException('No IP address attached to this request.');
-        }
-
-        // Handle the IP being separated by commas.
-        $ipParts = explode(',', $ip);
-        $ip = array_shift($ipParts);
-
-        return trim($ip);
-    }
 }
diff --git a/src/RateLimit.php b/src/RateLimit.php
index 5d6ea628d2..e1c3d1d99d 100644
--- a/src/RateLimit.php
+++ b/src/RateLimit.php
@@ -4,6 +4,7 @@
 
 namespace App;
 
+use App\Entity\Repository\SettingsRepository;
 use App\Http\ServerRequest;
 use App\Lock\LockFactory;
 use Psr\Cache\CacheItemPoolInterface;
@@ -18,6 +19,7 @@ final class RateLimit
     public function __construct(
         private readonly LockFactory $lockFactory,
         private readonly Environment $environment,
+        private readonly SettingsRepository $settingsRepo,
         CacheItemPoolInterface $cacheItemPool
     ) {
         $this->psr6Cache = new ProxyAdapter($cacheItemPool, 'ratelimit.');
@@ -41,7 +43,9 @@ public function checkRequestRateLimit(
             return;
         }
 
-        $ipKey = str_replace([':', '.'], '_', $request->getIp());
+        $ip = $this->settingsRepo->readSettings()->getIp($request);
+
+        $ipKey = str_replace([':', '.'], '_', $ip);
         $this->checkRateLimit($groupName, $ipKey, $interval, $limit);
     }