Skip to content

Commit

Permalink
Tighten allowed IPs to avoid brute-force workarounds.
Browse files Browse the repository at this point in the history
  • Loading branch information
BusterNeece committed Apr 28, 2023
1 parent aad0147 commit bdb2359
Show file tree
Hide file tree
Showing 10 changed files with 181 additions and 34 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Expand Up @@ -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

---
Expand Down
5 changes: 4 additions & 1 deletion frontend/vue/components/Admin/Settings.vue
Expand Up @@ -133,6 +133,7 @@ const {form, v$, ifValid} = useVuelidateOnForm(
always_use_ssl: {},
api_access_control: {},
ip_source: {},
check_for_updates: {},
acme_email: {},
Expand All @@ -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',
Expand All @@ -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: '',
Expand Down
46 changes: 45 additions & 1 deletion frontend/vue/components/Admin/Settings/SecurityPrivacyTab.vue
Expand Up @@ -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>
Expand All @@ -98,11 +121,32 @@
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: {
type: Object,
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>
5 changes: 3 additions & 2 deletions src/Controller/Api/Admin/RelaysController.php
Expand Up @@ -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
) {
}

Expand Down Expand Up @@ -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]);
Expand Down
8 changes: 6 additions & 2 deletions src/Controller/Api/Stations/Requests/SubmitAction.php
Expand Up @@ -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;
Expand Down Expand Up @@ -43,7 +44,8 @@
final class SubmitAction
{
public function __construct(
private readonly StationRequestRepository $requestRepo
private readonly StationRequestRepository $requestRepo,
private readonly SettingsRepository $settingsRepo
) {
}

Expand All @@ -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')
);

Expand Down
56 changes: 56 additions & 0 deletions 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);
}
}
26 changes: 26 additions & 0 deletions 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');
}
}
32 changes: 32 additions & 0 deletions src/Entity/Settings.php
Expand Up @@ -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;
Expand Down Expand Up @@ -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';
Expand Down
27 changes: 0 additions & 27 deletions src/Http/ServerRequest.php
Expand Up @@ -14,7 +14,6 @@
use App\Session;
use App\View;
use Mezzio\Session\SessionInterface;
use RuntimeException;

final class ServerRequest extends \Slim\Http\ServerRequest
{
Expand Down Expand Up @@ -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);
}
}
6 changes: 5 additions & 1 deletion src/RateLimit.php
Expand Up @@ -4,6 +4,7 @@

namespace App;

use App\Entity\Repository\SettingsRepository;
use App\Http\ServerRequest;
use App\Lock\LockFactory;
use Psr\Cache\CacheItemPoolInterface;
Expand All @@ -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.');
Expand All @@ -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);
}

Expand Down

0 comments on commit bdb2359

Please sign in to comment.