Security-, Bugfix- und UX-Überarbeitung auf Basis des Code-Reviews
Sicherheit: - Bulk-Erstellung serverseitig auf eingeloggte Nutzer beschränkt; expire_minutes wird validiert (anonym: nur Default/Template-Werte, eingeloggt: max. 1 Jahr) - IP-basiertes Rate-Limit über neue Tabelle request_throttle (Voucher-Erstellung + Passwort-Reset-Anfragen), Session-Fallback für Alt-Installationen; Migration 0002 - session_regenerate_id() nach Login, Secure-Cookie-Flag bei HTTPS - Admin-/Aktiv-Status wird pro Request live aus der DB geprüft (Rechteentzug & Deaktivierung wirken sofort); Schutz vor Selbst-Degradierung im Benutzer-Edit - Alle state-ändernden Admin-Aktionen von GET auf POST umgestellt (kein CSRF-Token mehr in URLs) - login_simple.php (Legacy, Debug-Leak) entfernt; cron_test.php nur noch für Admins; .htaccess auf Apache-2.4-Syntax inkl. cron_test.php - M365 Client Secret wird nicht mehr ins Formular zurückgegeben - Updater: Zip-Slip-/Pfad-Traversal-Schutz, Backup vor dem Anwenden mit automatischem Rollback bei Fehlern, AuditLogger-Bug behoben - cron_sync: Token-Vergleich mit hash_equals; login_attempts-Pruning - CSV-Export gegen Excel-Formula-Injection abgesichert Bugfixes: - M365-Login: Fallback auf userPrincipalName, wenn Graph kein 'mail' liefert (Nutzer ohne Exchange-Postfach konnten sich nie anmelden) - PRG-Pattern überall: F5 erzeugt keine Duplikat-Voucher und wiederholt keine Admin-Aktionen (Session-Flash-Messages) - QR-Code nicht mehr invertiert (schwarz auf weiß, scanbar) - Bulk-Erstellung nutzt den UniFi 'n'-Parameter: 1 API-Call statt n× Login + Voucherlisten-Abruf; exaktes Code-Matching per create_time statt "global neuester Voucher" - Mailer: doppelte Zeilenumbrüche behoben, AUTH nur mit Credentials, SMTP-Dot-Stuffing, CLI-sicherer EHLO-Host - forgot_password: System-URL-Auto-Detect (Reset-Link war sonst relativ/kaputt) + Rate-Limit - Audit-Log-Labels an tatsächliche Action-Keys angepasst; Voucher-Erstellung (einzeln & bulk) wird jetzt auditiert - Site-Edit testet die Verbindung auch ohne Passwortänderung UX/UI: - Alert-/Badge-Styles zentral in global.css mit Dark-Mode-Variablen (vorher 7× dupliziert mit hart codierten Hellfarben) - Sticky-Formulare + Tab-Erhalt nach Validierungsfehlern (Bulk), Settings kehren nach dem Speichern zum aktiven Tab zurück - Gültigkeit menschenlesbar (z.B. "8 Stunden" statt "480 Minuten") - Voucher-Name-Default "Gast/Guest" im öffentlichen Modus - Favicon auch auf Login-/öffentlichen Seiten - Verbindungstest-Button pro Site-Karte (Health-Check) - i18n-Pass: Confirm-Dialoge, Toasts, Fehl-/Erfolgsmeldungen in de/en - Sprachumschalter ohne fetch+reload (kein Re-Submit-Dialog) - A11y: Esc schließt Modals, aria-live für Toasts, aria-labels auf Icon-Buttons; APP_KEY-Warnbanner im Dashboard - Dashboard-Sync: set_time_limit passend zur Site-Anzahl; Voucher-Sync mit Map statt SELECT pro Voucher Tooling: - GitHub-Actions-Workflow: PHP-Lint aller Dateien + de/en-Key-Parität https://claude.ai/code/session_01KKVpVPJjrTKGoRgpJcySD4
This commit is contained in:
Claude
parent
f747a3d429
commit
6e19958a37
31 changed files with 1040 additions and 628 deletions
|
|
@ -86,3 +86,17 @@ rm updater/storage/.maintenance # wieder normal
|
|||
|
||||
> Hinweis: Ein vollständiger Installations-Durchlauf (`action=install`) setzt
|
||||
> einen erreichbaren Update-Proxy unter den oben genannten URLs voraus.
|
||||
|
||||
## Sicherheits-Hinweise & Limitierungen
|
||||
|
||||
- **Keine Paket-Signatur:** Die Integrität der Updates hängt derzeit allein an
|
||||
TLS zur Update-Proxy-URL. Der Updater validiert Zip-Einträge und Dateipfade
|
||||
gegen Pfad-Traversal und legt vor dem Anwenden ein Backup an
|
||||
(`updater/storage/.backup-last`), das bei Fehlern automatisch
|
||||
zurückgespielt wird. Eine kryptografische Signaturprüfung der Pakete
|
||||
(z.B. signierte SHA-256-Manifeste) erfordert serverseitige Unterstützung
|
||||
des Update-Proxys und steht noch aus.
|
||||
- **Rollback:** Schlägt das Anwenden des Updates oder eine Migration fehl,
|
||||
werden die überschriebenen Dateien aus dem Backup wiederhergestellt.
|
||||
Datenbank-Migrationen werden dabei nicht automatisch rückgängig gemacht
|
||||
(jede Migration läuft aber in einer eigenen Transaktion).
|
||||
|
|
|
|||
|
|
@ -17,12 +17,15 @@ class UpdateController
|
|||
private $auth;
|
||||
/** @var UpdateManager */
|
||||
private $manager;
|
||||
/** @var AuditLogger */
|
||||
private $audit;
|
||||
|
||||
public function __construct(\Database $db, \Auth $auth)
|
||||
{
|
||||
$this->db = $db;
|
||||
$this->auth = $auth;
|
||||
$this->manager = UpdaterFactory::create($db, new AuditLogger($db));
|
||||
$this->audit = new AuditLogger($db);
|
||||
$this->manager = UpdaterFactory::create($db, $this->audit);
|
||||
}
|
||||
|
||||
public function handle(): void
|
||||
|
|
|
|||
|
|
@ -197,18 +197,33 @@ class UpdateManager
|
|||
}
|
||||
$stagingRoot = $this->resolveStagingRoot($stagingDir);
|
||||
|
||||
// 4) Staging -> Production (geschuetzte Pfade ueberspringen)
|
||||
$this->setProgress(65, 'Wende Update an …');
|
||||
$this->applyStaging($stagingRoot);
|
||||
// 4) Backup aller Dateien anlegen, die gleich ueberschrieben werden.
|
||||
// Schlaegt das Anwenden oder eine Migration fehl, wird der alte
|
||||
// Stand wiederhergestellt statt eine halb-aktualisierte
|
||||
// Installation online zu nehmen.
|
||||
$this->setProgress(60, 'Sichere bestehende Dateien …');
|
||||
$backupDir = $this->storageDir . '/.backup-last';
|
||||
$this->cleanDir($backupDir);
|
||||
$this->backupExisting($stagingRoot, $backupDir);
|
||||
|
||||
// 5) Migrationen ausfuehren
|
||||
$this->setProgress(80, 'Fuehre Datenbank-Migrationen aus …');
|
||||
$runner = new MigrationRunner(
|
||||
$this->db->getConnection(),
|
||||
__DIR__ . '/migrations',
|
||||
$this->storageDir
|
||||
);
|
||||
$runner->runPending(true);
|
||||
try {
|
||||
// 5) Staging -> Production (geschuetzte Pfade ueberspringen)
|
||||
$this->setProgress(65, 'Wende Update an …');
|
||||
$this->applyStaging($stagingRoot);
|
||||
|
||||
// 6) Migrationen ausfuehren
|
||||
$this->setProgress(80, 'Fuehre Datenbank-Migrationen aus …');
|
||||
$runner = new MigrationRunner(
|
||||
$this->db->getConnection(),
|
||||
__DIR__ . '/migrations',
|
||||
$this->storageDir
|
||||
);
|
||||
$runner->runPending(true);
|
||||
} catch (\Throwable $e) {
|
||||
$this->setProgress(70, 'Fehler – stelle vorherigen Stand wieder her …');
|
||||
$this->restoreBackup($backupDir);
|
||||
throw $e;
|
||||
}
|
||||
|
||||
// 6) Caches leeren
|
||||
$this->setProgress(90, 'Leere Caches …');
|
||||
|
|
@ -280,6 +295,15 @@ class UpdateManager
|
|||
if ($zip->open($zipPath) !== true) {
|
||||
throw new \RuntimeException('ZIP konnte nicht geoeffnet werden.');
|
||||
}
|
||||
// Zip-Slip-Schutz: Eintraege mit Pfad-Traversal oder absoluten Pfaden
|
||||
// ablehnen, bevor irgendetwas entpackt wird.
|
||||
for ($i = 0; $i < $zip->numFiles; $i++) {
|
||||
$name = (string)$zip->getNameIndex($i);
|
||||
if ($name === '' || $name[0] === '/' || strpos($name, '..') !== false || strpos($name, ':') !== false) {
|
||||
$zip->close();
|
||||
throw new \RuntimeException("ZIP enthaelt unsicheren Pfad: $name");
|
||||
}
|
||||
}
|
||||
if (!is_dir($dest)) {
|
||||
@mkdir($dest, 0775, true);
|
||||
}
|
||||
|
|
@ -308,6 +332,10 @@ class UpdateManager
|
|||
continue;
|
||||
}
|
||||
$relPath = $entry['path'];
|
||||
// Pfad-Traversal-Schutz: Proxy-Antworten nicht blind vertrauen
|
||||
if ($relPath[0] === '/' || strpos($relPath, '..') !== false || strpos($relPath, ':') !== false) {
|
||||
throw new \RuntimeException("Dateiliste enthaelt unsicheren Pfad: $relPath");
|
||||
}
|
||||
[$st, $content] = $this->httpGet($this->proxyUrl . '/download/' . str_replace('%2F', '/', rawurlencode($relPath)));
|
||||
if ($st !== 200) {
|
||||
throw new \RuntimeException("Download fehlgeschlagen: $relPath (HTTP $st)");
|
||||
|
|
@ -369,6 +397,64 @@ class UpdateManager
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Sichert alle Produktionsdateien, die durch das Staging ueberschrieben
|
||||
* wuerden, in ein Backup-Verzeichnis (Spiegelstruktur).
|
||||
*/
|
||||
private function backupExisting(string $stagingRoot, string $backupDir): void
|
||||
{
|
||||
$iterator = new \RecursiveIteratorIterator(
|
||||
new \RecursiveDirectoryIterator($stagingRoot, \FilesystemIterator::SKIP_DOTS),
|
||||
\RecursiveIteratorIterator::SELF_FIRST
|
||||
);
|
||||
foreach ($iterator as $item) {
|
||||
if ($item->isDir()) {
|
||||
continue;
|
||||
}
|
||||
$rel = ltrim(str_replace('\\', '/', substr($item->getPathname(), strlen($stagingRoot))), '/');
|
||||
if ($rel === '' || $this->isProtected($rel)) {
|
||||
continue;
|
||||
}
|
||||
$existing = $this->rootDir . '/' . $rel;
|
||||
if (!is_file($existing)) {
|
||||
continue;
|
||||
}
|
||||
$target = $backupDir . '/' . $rel;
|
||||
$dir = dirname($target);
|
||||
if (!is_dir($dir)) {
|
||||
@mkdir($dir, 0775, true);
|
||||
}
|
||||
@copy($existing, $target);
|
||||
}
|
||||
}
|
||||
|
||||
/** Stellt ein zuvor angelegtes Backup wieder in die Produktion zurueck. */
|
||||
private function restoreBackup(string $backupDir): void
|
||||
{
|
||||
if (!is_dir($backupDir)) {
|
||||
return;
|
||||
}
|
||||
$iterator = new \RecursiveIteratorIterator(
|
||||
new \RecursiveDirectoryIterator($backupDir, \FilesystemIterator::SKIP_DOTS),
|
||||
\RecursiveIteratorIterator::SELF_FIRST
|
||||
);
|
||||
foreach ($iterator as $item) {
|
||||
if ($item->isDir()) {
|
||||
continue;
|
||||
}
|
||||
$rel = ltrim(str_replace('\\', '/', substr($item->getPathname(), strlen($backupDir))), '/');
|
||||
if ($rel === '') {
|
||||
continue;
|
||||
}
|
||||
$target = $this->rootDir . '/' . $rel;
|
||||
$dir = dirname($target);
|
||||
if (!is_dir($dir)) {
|
||||
@mkdir($dir, 0775, true);
|
||||
}
|
||||
@copy($item->getPathname(), $target);
|
||||
}
|
||||
}
|
||||
|
||||
private function isProtected(string $rel): bool
|
||||
{
|
||||
foreach (self::PROTECTED_PATHS as $p) {
|
||||
|
|
|
|||
11
updater/migrations/0002_voucher_request_throttle.sql
Normal file
11
updater/migrations/0002_voucher_request_throttle.sql
Normal file
|
|
@ -0,0 +1,11 @@
|
|||
-- IP-basiertes Request-Throttling (z.B. anonyme Voucher-Erstellung,
|
||||
-- Passwort-Reset-Anfragen). Ersetzt das rein session-basierte Throttling,
|
||||
-- das sich per Cookie-Loeschen umgehen liess.
|
||||
CREATE TABLE IF NOT EXISTS `request_throttle` (
|
||||
`id` INT PRIMARY KEY AUTO_INCREMENT,
|
||||
`ip_address` VARCHAR(45) NOT NULL,
|
||||
`action` VARCHAR(50) NOT NULL,
|
||||
`weight` INT NOT NULL DEFAULT 1,
|
||||
`requested_at` TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
|
||||
INDEX `idx_throttle` (`action`, `ip_address`, `requested_at`)
|
||||
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;
|
||||
Loading…
Add table
Add a link
Reference in a new issue