fix: harden mx helper and cleanup flow

This commit is contained in:
Marek Miklewicz
2026-06-30 11:37:27 +02:00
parent ed6146c0d8
commit ad95ecdacb
8 changed files with 252 additions and 43 deletions
+6
View File
@@ -7,6 +7,12 @@ return static function (): void {
}
$settings = Settings::load(Settings::EXTERNAL_SETTINGS);
if (!$settings->defaultEnable()) {
Http::errorPage('Plugin nie jest włączony dla tego serwera.', 403);
}
if ($settings->method() !== 'login_url') {
Http::errorPage('Metoda logowania nie jest obsługiwana.', 500);
}
$ctx = DirectAdminContext::fromServer($_SERVER, $settings);
SourceAuthorizer::assertAllowed($ctx->sourceHost(), $settings);
+16
View File
@@ -105,6 +105,16 @@ final class Settings
return rtrim($this->getString('MAIL_LOGIN_TARGET_BASE_URL'), '/');
}
public function defaultEnable(): bool
{
return in_array(strtolower($this->getString('DEFAULT_ENABLE', 'true')), ['1', 'true', 'yes', 'on'], true);
}
public function method(): string
{
return $this->getString('MAIL_LOGIN_METHOD', 'login_url');
}
public function redirectUrl(): string
{
return $this->getString('MAIL_LOGIN_REDIRECT_URL', '/');
@@ -229,6 +239,12 @@ final class Settings
if (!in_array($this->getString('USER_IP_BOUND', ''), ['0', '1'], true)) {
throw new InvalidArgumentException('USER_IP_BOUND must be 0 or 1');
}
if (!in_array(strtolower($this->getString('DEFAULT_ENABLE', 'true')), ['0', '1', 'true', 'false', 'yes', 'no', 'on', 'off'], true)) {
throw new InvalidArgumentException('DEFAULT_ENABLE must be a boolean value');
}
if ($this->method() !== 'login_url') {
throw new InvalidArgumentException('MAIL_LOGIN_METHOD must be login_url');
}
if (!in_array($this->clientIpMode(), ['direct', 'trusted_proxy'], true)) {
throw new InvalidArgumentException('MAIL_LOGIN_CLIENT_IP_MODE must be direct or trusted_proxy');
}
+81 -23
View File
@@ -2,51 +2,109 @@
<?php
declare(strict_types=1);
$config = [
if (realpath((string)($_SERVER['SCRIPT_FILENAME'] ?? '')) === __FILE__) {
mailLoginCleanupMain();
}
function mailLoginCleanupMain(): void
{
$config = [
'base_url' => getenv('DA_BASE_URL') ?: 'https://127.0.0.1:2222',
'user' => getenv('DA_API_USER') ?: '',
'key' => getenv('DA_API_LOGIN_KEY') ?: '',
'audit_log' => getenv('MAIL_LOGIN_AUDIT_LOG') ?: '/usr/local/hitme_plugins/mail-login-helper/logs/audit.log',
];
'state_dir' => getenv('MAIL_LOGIN_STATE_DIR') ?: '/usr/local/hitme_plugins/mail-login-helper/state',
];
if ($config['user'] === '' || $config['key'] === '') {
if ($config['user'] === '' || $config['key'] === '') {
fwrite(STDERR, "DirectAdmin API credentials are required for cleanup\n");
exit(2);
}
}
$base = rtrim($config['base_url'], '/');
$now = time();
$entries = apiRequest('GET', $base . '/api/login-keys/urls', $config);
if (!is_array($entries)) {
$base = rtrim($config['base_url'], '/');
$now = time();
$states = mailLoginCleanupLoadStates($config['state_dir']);
$entries = mailLoginCleanupApiRequest('GET', $base . '/api/login-keys/urls', $config);
if (!is_array($entries)) {
fwrite(STDERR, "DirectAdmin returned invalid login URL list\n");
exit(1);
}
$deleted = 0;
foreach ($entries as $entry) {
if (!is_array($entry) || empty($entry['id']) || empty($entry['expires'])) {
continue;
}
$expires = strtotime((string)$entry['expires']);
if ($expires !== false && $expires < $now) {
apiRequest('DELETE', $base . '/api/login-keys/urls/' . rawurlencode((string)$entry['id']), $config);
$deleted = 0;
foreach ($entries as $entry) {
if (is_array($entry) && mailLoginCleanupShouldDelete($entry, $states, $now)) {
mailLoginCleanupApiRequest('DELETE', $base . '/api/login-keys/urls/' . rawurlencode((string)$entry['id']), $config);
$deleted++;
}
}
}
audit($config['audit_log'], [
mailLoginCleanupAudit($config['audit_log'], [
'event' => 'cleanup_expired_login_urls',
'result' => 'success',
'deleted' => $deleted,
]);
]);
echo "Deleted expired login URLs: {$deleted}\n";
echo "Deleted expired login URLs: {$deleted}\n";
}
/**
* @return list<array<string,mixed>>
*/
function mailLoginCleanupLoadStates(string $stateDir): array
{
$states = [];
foreach (glob(rtrim($stateDir, '/') . '/autologin-*.json') ?: [] as $file) {
$decoded = json_decode((string)file_get_contents($file), true);
if (is_array($decoded)) {
$states[] = $decoded;
}
}
return $states;
}
/**
* @param array<string,mixed> $entry
* @param list<array<string,mixed>> $states
*/
function mailLoginCleanupShouldDelete(array $entry, array $states, int $now): bool
{
if (empty($entry['id']) || empty($entry['expires'])) {
return false;
}
$entryExpires = strtotime((string)$entry['expires']);
if ($entryExpires === false || $entryExpires >= $now) {
return false;
}
foreach ($states as $state) {
if (($state['status'] ?? '') !== 'created') {
continue;
}
$stateExpires = (int)($state['expires'] ?? 0);
if ($stateExpires <= 0 || $stateExpires > $now) {
continue;
}
if ((string)($entry['redirectURL'] ?? '/') !== (string)($state['redirect_path'] ?? '/')) {
continue;
}
$entryNetworks = $entry['allowNetworks'] ?? [];
if (!is_array($entryNetworks)) {
$entryNetworks = [];
}
$stateIp = (string)($state['client_ip'] ?? '');
$ipBound = (int)($state['ip_bound'] ?? 0) === 1;
if ($ipBound && !in_array($stateIp . (str_contains($stateIp, ':') ? '/128' : '/32'), $entryNetworks, true)) {
continue;
}
return true;
}
return false;
}
/**
* @param array<string,string> $config
* @return mixed
*/
function apiRequest(string $method, string $url, array $config): mixed
function mailLoginCleanupApiRequest(string $method, string $url, array $config): mixed
{
$ch = curl_init($url);
if ($ch === false) {
@@ -77,7 +135,7 @@ function apiRequest(string $method, string $url, array $config): mixed
/**
* @param array<string,mixed> $fields
*/
function audit(string $path, array $fields): void
function mailLoginCleanupAudit(string $path, array $fields): void
{
$dir = dirname($path);
if (!is_dir($dir)) {
+15 -2
View File
@@ -4,8 +4,8 @@ set -eu
BASE_DIR="/usr/local/hitme_plugins/mail-login-helper"
SCRIPT_DIR="$(CDPATH= cd -- "$(dirname -- "$0")" && pwd)"
mkdir -p "$BASE_DIR/bin" "$BASE_DIR/logs" "$BASE_DIR/state"
chmod 700 "$BASE_DIR" "$BASE_DIR/bin" "$BASE_DIR/logs" "$BASE_DIR/state"
mkdir -p "$BASE_DIR/bin" "$BASE_DIR/config" "$BASE_DIR/logs" "$BASE_DIR/state"
chmod 700 "$BASE_DIR" "$BASE_DIR/bin" "$BASE_DIR/config" "$BASE_DIR/logs" "$BASE_DIR/state"
cp "$SCRIPT_DIR/mail-login-authorized-command.sh" "$BASE_DIR/bin/create-login-url"
cp "$SCRIPT_DIR/cleanup-expired-login-urls.php" "$BASE_DIR/bin/cleanup-expired-login-urls"
@@ -13,4 +13,17 @@ chmod 755 "$BASE_DIR/bin/create-login-url" "$BASE_DIR/bin/cleanup-expired-login-
touch "$BASE_DIR/logs/audit.log"
chmod 600 "$BASE_DIR/logs/audit.log"
if [ ! -f "$BASE_DIR/config/helper-settings.conf" ]; then
cat > "$BASE_DIR/config/helper-settings.conf" <<'EOF'
MAIL_LOGIN_ALLOWED_SOURCE_HOSTS=h4.domena.pl
MAIL_LOGIN_DA_BIN=/usr/bin/da
EOF
chmod 600 "$BASE_DIR/config/helper-settings.conf"
fi
cat > "$BASE_DIR/authorized_keys.example" <<'EOF'
from="SOURCE_SERVER_PUBLIC_IP",restrict,no-agent-forwarding,no-port-forwarding,no-X11-forwarding,no-pty,command="/usr/local/hitme_plugins/mail-login-helper/bin/create-login-url" ssh-ed25519 SOURCE_SERVER_PUBLIC_KEY mail-login-source
EOF
chmod 600 "$BASE_DIR/authorized_keys.example"
echo "mail-login MX helper installed at $BASE_DIR"
+35 -4
View File
@@ -3,9 +3,15 @@ set -eu
set -f
DA_BIN="${MAIL_LOGIN_DA_BIN:-/usr/bin/da}"
CONFIG_FILE="${MAIL_LOGIN_HELPER_CONFIG:-/usr/local/hitme_plugins/mail-login-helper/config/helper-settings.conf}"
AUDIT_LOG="${MAIL_LOGIN_AUDIT_LOG:-/usr/local/hitme_plugins/mail-login-helper/logs/audit.log}"
STATE_DIR="${MAIL_LOGIN_STATE_DIR:-/usr/local/hitme_plugins/mail-login-helper/state}"
if [ -f "$CONFIG_FILE" ]; then
# shellcheck disable=SC1090
. "$CONFIG_FILE"
fi
audit() {
dir=$(dirname "$AUDIT_LOG")
mkdir -p "$dir"
@@ -25,6 +31,27 @@ clean_label() {
printf '%s' "$1" | tr '[:upper:]' '[:lower:]' | sed 's/[^a-z0-9-]/-/g; s/^-*//; s/-*$//'
}
first_label() {
printf '%s' "$1" | sed 's/:.*$//' | awk -F. '{print $1}'
}
host_allowed() {
allowed="${MAIL_LOGIN_ALLOWED_SOURCE_HOSTS:-}"
[ -n "$allowed" ] || return 1
old_ifs=$IFS
IFS=,
for host in $allowed; do
normalized=$(printf '%s' "$host" | tr '[:upper:]' '[:lower:]' | sed 's#^https\?://##; s#/.*$##; s/:[0-9][0-9]*$//; s/^ *//; s/ *$//')
[ "$normalized" = "$SOURCE_HOST" ] && IFS=$old_ifs && return 0
done
IFS=$old_ifs
return 1
}
valid_ip() {
php -r 'exit(filter_var($argv[1], FILTER_VALIDATE_IP) ? 0 : 1);' "$1"
}
if [ "$#" -eq 0 ] && [ -n "${SSH_ORIGINAL_COMMAND:-}" ]; then
# The authorized_keys forced command receives the requested command here.
# All accepted fields are validated below before use.
@@ -43,11 +70,15 @@ REDIRECT_PATH="${8:-}"
[ "$COMMAND" = "mail-login-create-url" ] || fail
printf '%s' "$USERNAME_ARG" | grep -Eq '^[A-Za-z0-9][A-Za-z0-9._-]{0,31}$' || fail
printf '%s' "$SOURCE_HOST" | grep -Eq '^[A-Za-z0-9.-]+$' || fail
printf '%s' "$SERVER_NAME" | grep -Eq '^[A-Za-z0-9-]{1,32}$' || fail
SOURCE_HOST=$(printf '%s' "$SOURCE_HOST" | tr '[:upper:]' '[:lower:]')
host_allowed || fail
SERVER_NAME=$(first_label "$SOURCE_HOST")
SERVER_NAME=$(clean_label "$SERVER_NAME")
printf '%s' "$SERVER_NAME" | grep -Eq '^[a-z0-9-]{1,32}$' || fail
printf '%s' "$TTL" | grep -Eq '^[0-9]+$' || fail
[ "$TTL" -ge 5 ] && [ "$TTL" -le 300 ] || fail
[ "$USER_IP_BOUND" = "0" ] || [ "$USER_IP_BOUND" = "1" ] || fail
printf '%s' "$CLIENT_IP" | grep -Eq '^[0-9A-Fa-f:.]+$' || fail
valid_ip "$CLIENT_IP" || fail
case "$REDIRECT_PATH" in
/*) ;;
*) fail ;;
@@ -86,8 +117,8 @@ case "$URL" in
*) fail ;;
esac
printf '{"timestamp":%s,"correlation":"%s","username":"%s","source_host":"%s","client_ip":"%s","ttl":%s,"ip_bound":%s}\n' \
"$TIMESTAMP" "$CORRELATION" "$USERNAME_ARG" "$SOURCE_HOST" "$CLIENT_IP" "$TTL" "$USER_IP_BOUND" > "$STATE_DIR/$CORRELATION.json"
printf '{"timestamp":%s,"expires":%s,"correlation":"%s","username":"%s","source_host":"%s","client_ip":"%s","ttl":%s,"ip_bound":%s,"redirect_path":"%s","status":"created"}\n' \
"$TIMESTAMP" "$((TIMESTAMP + TTL))" "$CORRELATION" "$USERNAME_ARG" "$SOURCE_HOST" "$CLIENT_IP" "$TTL" "$USER_IP_BOUND" "$REDIRECT_PATH" > "$STATE_DIR/$CORRELATION.json"
chmod 600 "$STATE_DIR/$CORRELATION.json"
audit "create_login_url" "success" "$CORRELATION" "$USERNAME_ARG" "$SOURCE_HOST" "$CLIENT_IP"
printf 'URL: %s\n' "$URL"
+35
View File
@@ -0,0 +1,35 @@
<?php
declare(strict_types=1);
require_once PLUGIN_ROOT . '/mx-helper/cleanup-expired-login-urls.php';
test('cleanup deletes only expired login urls matching local mail-login state', function (): void {
$state = [
'timestamp' => 1782810000,
'expires' => 1782810030,
'correlation' => 'autologin-h4-mxtest-1782810000',
'redirect_path' => '/',
'client_ip' => '198.51.100.10',
'ip_bound' => 1,
'status' => 'created',
];
$matching = [
'id' => 'HASHURLMATCH',
'created' => '2026-06-30T10:00:00Z',
'expires' => '2026-06-30T10:00:30Z',
'redirectURL' => '/',
'allowNetworks' => ['198.51.100.10/32'],
];
$wrongRedirect = $matching;
$wrongRedirect['id'] = 'HASHURLWRONG';
$wrongRedirect['redirectURL'] = '/admin';
$notExpired = $matching;
$notExpired['id'] = 'HASHURLFRESH';
$notExpired['expires'] = '2099-06-30T10:00:30Z';
assert_true(mailLoginCleanupShouldDelete($matching, [$state], strtotime('2026-06-30T10:01:00Z')));
assert_false(mailLoginCleanupShouldDelete($wrongRedirect, [$state], strtotime('2026-06-30T10:01:00Z')));
assert_false(mailLoginCleanupShouldDelete($notExpired, [$state], strtotime('2026-06-30T10:01:00Z')));
});
+41 -4
View File
@@ -12,10 +12,11 @@ test('mx helper conditionally passes user ip binding to da login-url', function
$script = PLUGIN_ROOT . '/mx-helper/mail-login-authorized-command.sh';
$cmd = sprintf(
'MAIL_LOGIN_DA_BIN=%s MAIL_LOGIN_AUDIT_LOG=%s MAIL_LOGIN_STATE_DIR=%s %s %s',
'MAIL_LOGIN_DA_BIN=%s MAIL_LOGIN_AUDIT_LOG=%s MAIL_LOGIN_STATE_DIR=%s MAIL_LOGIN_ALLOWED_SOURCE_HOSTS=%s %s %s',
escapeshellarg($bin),
escapeshellarg($audit),
escapeshellarg($state),
escapeshellarg('h4.domena.pl'),
escapeshellarg($script),
'mail-login-create-url mxtest h4.domena.pl h4 30 1 198.51.100.10 /'
);
@@ -26,10 +27,11 @@ test('mx helper conditionally passes user ip binding to da login-url', function
file_put_contents($log, '');
$cmdNoIp = sprintf(
'MAIL_LOGIN_DA_BIN=%s MAIL_LOGIN_AUDIT_LOG=%s MAIL_LOGIN_STATE_DIR=%s %s %s',
'MAIL_LOGIN_DA_BIN=%s MAIL_LOGIN_AUDIT_LOG=%s MAIL_LOGIN_STATE_DIR=%s MAIL_LOGIN_ALLOWED_SOURCE_HOSTS=%s %s %s',
escapeshellarg($bin),
escapeshellarg($audit),
escapeshellarg($state),
escapeshellarg('h4.domena.pl'),
escapeshellarg($script),
'mail-login-create-url mxtest h4.domena.pl h4 30 0 198.51.100.10 /'
);
@@ -50,11 +52,12 @@ test('mx helper accepts forced command arguments from ssh original command', fun
$script = PLUGIN_ROOT . '/mx-helper/mail-login-authorized-command.sh';
$cmd = sprintf(
'MAIL_LOGIN_DA_BIN=%s MAIL_LOGIN_AUDIT_LOG=%s MAIL_LOGIN_STATE_DIR=%s SSH_ORIGINAL_COMMAND=%s %s',
'MAIL_LOGIN_DA_BIN=%s MAIL_LOGIN_AUDIT_LOG=%s MAIL_LOGIN_STATE_DIR=%s MAIL_LOGIN_ALLOWED_SOURCE_HOSTS=%s SSH_ORIGINAL_COMMAND=%s %s',
escapeshellarg($bin),
escapeshellarg($audit),
escapeshellarg($state),
escapeshellarg('mail-login-create-url mxtest h4.domena.pl h4 30 1 198.51.100.10 /'),
escapeshellarg('h4.domena.pl'),
escapeshellarg('mail-login-create-url mxtest h4.domena.pl evil 30 1 198.51.100.10 /'),
escapeshellarg($script)
);
exec($cmd, $out, $code);
@@ -62,4 +65,38 @@ test('mx helper accepts forced command arguments from ssh original command', fun
assert_same(0, $code, implode("\n", $out));
assert_contains('--user=mxtest', file_get_contents($log) ?: '');
assert_contains('autologin-h4-mxtest-', file_get_contents($audit) ?: '');
assert_not_contains('autologin-evil-mxtest-', file_get_contents($audit) ?: '');
});
test('mx helper rejects disallowed source hosts and invalid client ips', function (): void {
$bin = TEST_TMP . '/fake-da-reject';
$audit = TEST_TMP . '/audit-reject.log';
$state = TEST_TMP . '/state-reject';
test_write($bin, "#!/bin/sh\necho 'URL: https://mx1.domena.pl:2222/api/login/url?key=SECRET'\n");
chmod($bin, 0755);
$script = PLUGIN_ROOT . '/mx-helper/mail-login-authorized-command.sh';
$badHost = sprintf(
'MAIL_LOGIN_DA_BIN=%s MAIL_LOGIN_AUDIT_LOG=%s MAIL_LOGIN_STATE_DIR=%s MAIL_LOGIN_ALLOWED_SOURCE_HOSTS=%s %s %s 2>/dev/null',
escapeshellarg($bin),
escapeshellarg($audit),
escapeshellarg($state),
escapeshellarg('h4.domena.pl'),
escapeshellarg($script),
'mail-login-create-url mxtest evil.domena.pl evil 30 1 198.51.100.10 /'
);
exec($badHost, $outHost, $codeHost);
assert_same(1, $codeHost);
$badIp = sprintf(
'MAIL_LOGIN_DA_BIN=%s MAIL_LOGIN_AUDIT_LOG=%s MAIL_LOGIN_STATE_DIR=%s MAIL_LOGIN_ALLOWED_SOURCE_HOSTS=%s %s %s 2>/dev/null',
escapeshellarg($bin),
escapeshellarg($audit),
escapeshellarg($state),
escapeshellarg('h4.domena.pl'),
escapeshellarg($script),
'mail-login-create-url mxtest h4.domena.pl h4 30 1 999.999.999.999 /'
);
exec($badIp, $outIp, $codeIp);
assert_same(1, $codeIp);
});
+13
View File
@@ -8,6 +8,8 @@ test('settings expose secure defaults from plugin-settings.conf', function (): v
assert_same(30, $settings->loginKeyTtl());
assert_true($settings->userIpBound());
assert_true($settings->defaultEnable());
assert_same('login_url', $settings->method());
assert_same('root', $settings->sshUser());
assert_same('https://mx1.mojserwer.pl:2222', $settings->targetBaseUrl());
});
@@ -41,3 +43,14 @@ test('settings parse allowed source hosts as normalized hostnames', function ():
assert_same(['h4.domena.pl', 'serwer2.example.net'], $settings->allowedSourceHosts());
});
test('settings reject disabled plugin and unsupported methods through access guard', function (): void {
try {
Settings::fromArray(['MAIL_LOGIN_METHOD' => 'password']);
throw new RuntimeException('Expected unsupported method to fail');
} catch (InvalidArgumentException $e) {
assert_contains('MAIL_LOGIN_METHOD', $e->getMessage());
}
$settings = Settings::fromArray(['DEFAULT_ENABLE' => 'false']);
assert_false($settings->defaultEnable());
});