refactor: remove source host allowlist from direct login

This commit is contained in:
Marek Miklewicz
2026-06-30 15:25:54 +02:00
parent b27c5c774a
commit c115b2861d
10 changed files with 11 additions and 83 deletions
-1
View File
@@ -19,7 +19,6 @@ foreach ([
'KeyName.php', 'KeyName.php',
'RateLimiter.php', 'RateLimiter.php',
'RemoteLoginUrlClient.php', 'RemoteLoginUrlClient.php',
'SourceAuthorizer.php',
'UrlValidator.php', 'UrlValidator.php',
] as $file) { ] as $file) {
require_once PLUGIN_EXEC_DIR . '/lib/' . $file; require_once PLUGIN_EXEC_DIR . '/lib/' . $file;
-1
View File
@@ -14,7 +14,6 @@ return static function (): void {
Http::errorPage('Metoda logowania nie jest obsługiwana.', 500); Http::errorPage('Metoda logowania nie jest obsługiwana.', 500);
} }
$ctx = DirectAdminContext::fromServer($_SERVER, $settings); $ctx = DirectAdminContext::fromServer($_SERVER, $settings);
SourceAuthorizer::assertAllowed($ctx->sourceHost(), $settings);
$auditBase = [ $auditBase = [
'request_id' => bin2hex(random_bytes(8)), 'request_id' => bin2hex(random_bytes(8)),
+2 -11
View File
@@ -20,7 +20,7 @@ final class DirectAdminContext
if (!preg_match('/^[A-Za-z0-9][A-Za-z0-9._-]{0,31}$/', $username)) { if (!preg_match('/^[A-Za-z0-9][A-Za-z0-9._-]{0,31}$/', $username)) {
throw new InvalidArgumentException('Invalid DirectAdmin username'); throw new InvalidArgumentException('Invalid DirectAdmin username');
} }
$host = self::trustedSourceHost($server, $settings); $host = self::trustedSourceHost($server);
if ($host === '') { if ($host === '') {
throw new InvalidArgumentException('Missing source host'); throw new InvalidArgumentException('Missing source host');
} }
@@ -37,17 +37,8 @@ final class DirectAdminContext
/** /**
* @param array<string,mixed> $server * @param array<string,mixed> $server
*/ */
private static function trustedSourceHost(array $server, Settings $settings): string private static function trustedSourceHost(array $server): string
{ {
$configuredName = $settings->sourceServerName();
if ($configuredName !== '') {
foreach ($settings->allowedSourceHosts() as $allowedHost) {
$label = $settings->sanitizeServerName(explode('.', $allowedHost)[0] ?? '');
if ($label === $configuredName) {
return $allowedHost;
}
}
}
return Settings::normalizeHost((string)($server['SERVER_NAME'] ?? '')); return Settings::normalizeHost((string)($server['SERVER_NAME'] ?? ''));
} }
-28
View File
@@ -64,7 +64,6 @@ final class Settings
'MAIL_LOGIN_REDIRECT_URL' => '/', 'MAIL_LOGIN_REDIRECT_URL' => '/',
'LOGIN_KEY_TTL' => '30', 'LOGIN_KEY_TTL' => '30',
'USER_IP_BOUND' => '1', 'USER_IP_BOUND' => '1',
'MAIL_LOGIN_ALLOWED_SOURCE_HOSTS' => 's1.abc.pl,serwer.mojserwer.pl,serwer2.xxb.ovh',
'MAIL_LOGIN_SOURCE_SERVER_NAME' => '', 'MAIL_LOGIN_SOURCE_SERVER_NAME' => '',
'MAIL_LOGIN_CLIENT_IP_MODE' => 'direct', 'MAIL_LOGIN_CLIENT_IP_MODE' => 'direct',
'MAIL_LOGIN_TRUSTED_PROXIES' => '', 'MAIL_LOGIN_TRUSTED_PROXIES' => '',
@@ -136,14 +135,6 @@ final class Settings
return $this->getString('USER_IP_BOUND', '1') === '1'; return $this->getString('USER_IP_BOUND', '1') === '1';
} }
/**
* @return list<string>
*/
public function allowedSourceHosts(): array
{
return $this->csvHosts('MAIL_LOGIN_ALLOWED_SOURCE_HOSTS');
}
public function sourceServerName(): string public function sourceServerName(): string
{ {
return $this->sanitizeServerName($this->getString('MAIL_LOGIN_SOURCE_SERVER_NAME', '')); return $this->sanitizeServerName($this->getString('MAIL_LOGIN_SOURCE_SERVER_NAME', ''));
@@ -213,25 +204,6 @@ final class Settings
return substr($name, 0, 32); return substr($name, 0, 32);
} }
/**
* @return list<string>
*/
private function csvHosts(string $key): array
{
$raw = $this->getString($key, '');
if ($raw === '') {
return [];
}
$out = [];
foreach (explode(',', $raw) as $item) {
$host = self::normalizeHost($item);
if ($host !== '') {
$out[] = $host;
}
}
return array_values(array_unique($out));
}
/** /**
* @return list<string> * @return list<string>
*/ */
-16
View File
@@ -1,16 +0,0 @@
<?php
declare(strict_types=1);
final class SourceAuthorizer
{
public static function assertAllowed(string $sourceHost, Settings $settings): void
{
$allowed = $settings->allowedSourceHosts();
if ($allowed === []) {
throw new RuntimeException('No source host allowlist configured');
}
if (!in_array(Settings::normalizeHost($sourceHost), $allowed, true)) {
throw new RuntimeException('Source host is not allowed');
}
}
}
+1 -4
View File
@@ -20,10 +20,7 @@ LOGIN_KEY_TTL=30
# 1 = przypnij URL do IP przeglądarki użytkownika, 0 = nie przypinaj do IP. # 1 = przypnij URL do IP przeglądarki użytkownika, 0 = nie przypinaj do IP.
USER_IP_BOUND=1 USER_IP_BOUND=1
# Hosty DirectAdmin WWW, z których wolno inicjować autologowanie do MX. # Opcjonalna jawna nazwa źródłowego serwera. Puste = pierwsza etykieta SERVER_NAME.
MAIL_LOGIN_ALLOWED_SOURCE_HOSTS=s1.abc.pl,serwer.mojserwer.pl,serwer2.xxb.ovh
# Opcjonalna jawna nazwa źródłowego serwera. Puste = pierwsza etykieta HTTP_HOST.
MAIL_LOGIN_SOURCE_SERVER_NAME= MAIL_LOGIN_SOURCE_SERVER_NAME=
# direct = REMOTE_ADDR, trusted_proxy = pierwszy X-Forwarded-For tylko od zaufanego proxy. # direct = REMOTE_ADDR, trusted_proxy = pierwszy X-Forwarded-For tylko od zaufanego proxy.
+1 -1
View File
@@ -2,7 +2,7 @@ name=mail-login
id=mail-login id=mail-login
type=user type=user
author=HITME.PL author=HITME.PL
version=1.0.4 version=1.0.5
active=no active=no
installed=no installed=no
user_run_as=root user_run_as=root
+1 -15
View File
@@ -5,7 +5,6 @@ require_once PLUGIN_ROOT . '/exec/lib/Settings.php';
require_once PLUGIN_ROOT . '/exec/lib/DirectAdminContext.php'; require_once PLUGIN_ROOT . '/exec/lib/DirectAdminContext.php';
require_once PLUGIN_ROOT . '/exec/lib/ClientIp.php'; require_once PLUGIN_ROOT . '/exec/lib/ClientIp.php';
require_once PLUGIN_ROOT . '/exec/lib/KeyName.php'; require_once PLUGIN_ROOT . '/exec/lib/KeyName.php';
require_once PLUGIN_ROOT . '/exec/lib/SourceAuthorizer.php';
test('directadmin context derives server name from first host label', function (): void { test('directadmin context derives server name from first host label', function (): void {
$settings = Settings::fromArray(['MAIL_LOGIN_SOURCE_SERVER_NAME' => '']); $settings = Settings::fromArray(['MAIL_LOGIN_SOURCE_SERVER_NAME' => '']);
@@ -22,7 +21,7 @@ test('directadmin context derives server name from first host label', function (
assert_same('h4', $ctx->serverName()); assert_same('h4', $ctx->serverName());
}); });
test('directadmin context prefers trusted configured source server name over host header', function (): void { test('directadmin context uses configured source server name and trusts local server name host', function (): void {
$settings = Settings::fromArray(['MAIL_LOGIN_SOURCE_SERVER_NAME' => 'h4']); $settings = Settings::fromArray(['MAIL_LOGIN_SOURCE_SERVER_NAME' => 'h4']);
$ctx = DirectAdminContext::fromServer([ $ctx = DirectAdminContext::fromServer([
'USERNAME' => 'mxtest', 'USERNAME' => 'mxtest',
@@ -100,19 +99,6 @@ test('client ip supports trusted proxy cidr ranges', function (): void {
assert_same('203.0.113.20', $ip); assert_same('203.0.113.20', $ip);
}); });
test('source authorizer rejects hosts outside allowlist', function (): void {
$settings = Settings::fromArray(['MAIL_LOGIN_ALLOWED_SOURCE_HOSTS' => 'h4.domena.pl']);
SourceAuthorizer::assertAllowed('h4.domena.pl', $settings);
try {
SourceAuthorizer::assertAllowed('evil.example.net', $settings);
throw new RuntimeException('Expected disallowed source host to fail');
} catch (RuntimeException $e) {
assert_contains('source host', strtolower($e->getMessage()));
}
});
test('key name uses requested format and truncates long values deterministically', function (): void { test('key name uses requested format and truncates long values deterministically', function (): void {
assert_same('autologin-h4-mxtest-1782810000', KeyName::build('h4', 'mxtest', 1782810000)); assert_same('autologin-h4-mxtest-1782810000', KeyName::build('h4', 'mxtest', 1782810000));
+1 -1
View File
@@ -8,7 +8,7 @@ test('plugin metadata and packaging docs match project rules', function (): void
assert_contains('name=mail-login', $conf); assert_contains('name=mail-login', $conf);
assert_contains('id=mail-login', $conf); assert_contains('id=mail-login', $conf);
assert_contains('type=user', $conf); assert_contains('type=user', $conf);
assert_contains('version=1.0.4', $conf); assert_contains('version=1.0.5', $conf);
assert_contains('user_run_as=root', $conf); assert_contains('user_run_as=root', $conf);
assert_contains('/Users/marek/GPT/ChatGPT/da_plugins/mail-login/src', $packing); assert_contains('/Users/marek/GPT/ChatGPT/da_plugins/mail-login/src', $packing);
assert_contains('/Users/marek/GPT/ChatGPT/da_plugins/mail-login/archives/X.Y.Z/mail-login.tar.gz', $packing); assert_contains('/Users/marek/GPT/ChatGPT/da_plugins/mail-login/archives/X.Y.Z/mail-login.tar.gz', $packing);
+5 -5
View File
@@ -49,12 +49,12 @@ test('settings reject unsupported mx login mode values', function (): void {
} }
}); });
test('settings parse allowed source hosts as normalized hostnames', function (): void { test('source plugin settings do not expose local source host allowlist', function (): void {
$settings = Settings::fromArray([ $defaults = Settings::defaults();
'MAIL_LOGIN_ALLOWED_SOURCE_HOSTS' => 'https://h4.domena.pl:2222, serwer2.example.net:2222', $config = file_get_contents(PLUGIN_ROOT . '/plugin-settings.conf') ?: '';
]);
assert_same(['h4.domena.pl', 'serwer2.example.net'], $settings->allowedSourceHosts()); assert_false(array_key_exists('MAIL_LOGIN_ALLOWED_SOURCE_HOSTS', $defaults));
assert_not_contains('MAIL_LOGIN_ALLOWED_SOURCE_HOSTS', $config);
}); });
test('settings reject disabled plugin and unsupported methods through access guard', function (): void { test('settings reject disabled plugin and unsupported methods through access guard', function (): void {