diff --git a/Changes.md b/Changes.md index d5a5ad5..ffbdce8 100644 --- a/Changes.md +++ b/Changes.md @@ -2,6 +2,15 @@ --- +## 2026-07-08 – Fix: SMTP-/Nextcloud-Zugangsdaten im Klartext gespeichert und im HTML sichtbar (Issue #119) + +- **Hoch:** `mail_smtp_pass` (SMTP-Passwort) und `nextcloud_pass` (Nextcloud-App-Passwort) wurden unverschlüsselt in `company_settings` abgelegt, obwohl für vergleichbar sensible Felder (FinTS-PIN, Client-ID/Secret) bereits `Crypto::encrypt()`/`Crypto::decrypt()` (AES-256-GCM) etabliert ist. Zusätzlich schrieb `src/Views/settings/form.php` beide Klartext-Passwörter in das `value`-Attribut zurück, wodurch sie für jeden Betrachter des Seitenquelltexts sichtbar waren. +- **`src/Models/Setting.php`:** `get()` entschlüsselt `nextcloud_pass`/`mail_smtp_pass` nun über `Crypto::decrypt()` (Migrationsmodus: bestehende Klartextwerte in der DB werden unverändert durchgereicht), `update()` verschlüsselt beide Felder vor dem Speichern über `Crypto::encrypt()` – analog zum bestehenden Muster in `BankSource.php`. +- **`src/Controllers/SettingsController.php`:** `update()` behält bei leer gelassenem Passwortfeld das bisherige (entschlüsselte) Passwort bei, statt es zu überschreiben – analog zum bestehenden Muster in `BankController`. +- **`src/Views/settings/form.php`:** `value`-Attribute der beiden Passwortfelder entfernt; stattdessen Platzhalter „(gespeichert – leer lassen für unverändert)", wenn bereits ein Wert existiert. +- **`migrations/085_encrypt_settings_passwords.sql`:** `mail_smtp_pass` von `VARCHAR(255)` auf `TEXT` erweitert, damit der Verschlüsselungs-Overhead (Prefix + Base64) nicht abgeschnitten wird. +- **`tests/52_SettingsPasswordEncryptionTest.php`:** Neuer Test prüft, dass beide Felder verschlüsselt in der DB landen, über `Setting::get()` korrekt entschlüsselt werden und bei leerem Wert nicht verschlüsselt gespeichert werden. + ## 2026-07-08 – Fix: SSRF über ungeprüftes fints_url-Feld bei Bankverbindungen (Issue #118) - **Kritisch:** Beim Anlegen/Bearbeiten einer FinTS-Bankverbindung wurde die vom Benutzer eingegebene `fints_url` ungefiltert übernommen und später von `FinTsFetchService` direkt als Ziel-URL für eine echte HTTP-Verbindung verwendet – ein authentifizierter Benutzer konnte den Server so Requests gegen beliebige Ziele absetzen lassen (interne Dienste, `localhost`, Cloud-Metadata-Endpunkte, CWE-918). diff --git a/migrations/085_encrypt_settings_passwords.sql b/migrations/085_encrypt_settings_passwords.sql new file mode 100644 index 0000000..ef4166c --- /dev/null +++ b/migrations/085_encrypt_settings_passwords.sql @@ -0,0 +1,5 @@ +-- Fix #119: mail_smtp_pass und nextcloud_pass werden nun verschlüsselt (Crypto::encrypt) +-- abgelegt. mail_smtp_pass wird auf TEXT erweitert, damit der Verschlüsselungs-Overhead +-- (enc:v1: Prefix + Base64 aus IV/Tag/Ciphertext) nicht abgeschnitten wird. +ALTER TABLE company_settings + MODIFY COLUMN mail_smtp_pass TEXT NOT NULL; diff --git a/src/Controllers/SettingsController.php b/src/Controllers/SettingsController.php index aec7af0..3c287e3 100644 --- a/src/Controllers/SettingsController.php +++ b/src/Controllers/SettingsController.php @@ -39,6 +39,13 @@ class SettingsController extends BaseController if (empty($data['plz'])) $errors['plz'] = 'Pflichtfeld'; if (empty($data['ort'])) $errors['ort'] = 'Pflichtfeld'; + // Passwortfelder: leer gelassen = bestehendes (verschlüsseltes) Passwort behalten + $current = Setting::get(); + foreach (['nextcloud_pass', 'mail_smtp_pass'] as $pwField) { + $newVal = trim($data[$pwField] ?? ''); + $data[$pwField] = $newVal !== '' ? $newVal : ($current[$pwField] ?? ''); + } + if ($errors) { $vf = dirname(__DIR__) . '/version.txt'; $version = file_exists($vf) ? trim(file_get_contents($vf)) : ''; diff --git a/src/Models/Setting.php b/src/Models/Setting.php index d724f24..0d38f74 100644 --- a/src/Models/Setting.php +++ b/src/Models/Setting.php @@ -5,12 +5,18 @@ declare(strict_types=1); namespace Steuerkram\Models; use Steuerkram\Database; +use Steuerkram\Services\Crypto; class Setting { public static function get(): array { - return Database::fetchOne('SELECT * FROM company_settings WHERE id = 1') ?: []; + $row = Database::fetchOne('SELECT * FROM company_settings WHERE id = 1') ?: []; + if ($row) { + $row['nextcloud_pass'] = Crypto::decrypt((string)($row['nextcloud_pass'] ?? '')); + $row['mail_smtp_pass'] = Crypto::decrypt((string)($row['mail_smtp_pass'] ?? '')); + } + return $row; } public static function isTestMode(): bool @@ -70,14 +76,14 @@ class Setting isset($data['ki_vorschlag_aktiv']) ? 1 : 0, $data['nextcloud_url'] ?: null, $data['nextcloud_user'] ?: null, - $data['nextcloud_pass'] ?: null, + !empty($data['nextcloud_pass']) ? Crypto::encrypt($data['nextcloud_pass']) : null, $data['nextcloud_folder'] ?: '/Belege/Import', $data['nextcloud_done'] ?: null, $data['mail_smtp_host'] ?: '', (int)($data['mail_smtp_port'] ?? 587), in_array($data['mail_smtp_encryption'] ?? '', ['tls', 'ssl', 'none']) ? $data['mail_smtp_encryption'] : 'tls', $data['mail_smtp_user'] ?: '', - $data['mail_smtp_pass'] ?: '', + !empty($data['mail_smtp_pass']) ? Crypto::encrypt($data['mail_smtp_pass']) : '', $data['mail_from_name'] ?: '', $data['mail_from_email'] ?: '', $data['mail_subject_template'] ?: 'Rechnung %RNR%', diff --git a/src/Views/settings/form.php b/src/Views/settings/form.php index e800398..1f93488 100644 --- a/src/Views/settings/form.php +++ b/src/Views/settings/form.php @@ -327,9 +327,8 @@
+ placeholder="">
Empfohlen: App-Passwort aus Nextcloud → Einstellungen → Sicherheit
@@ -381,7 +380,8 @@
- +
diff --git a/tests/52_SettingsPasswordEncryptionTest.php b/tests/52_SettingsPasswordEncryptionTest.php new file mode 100644 index 0000000..e1ec062 --- /dev/null +++ b/tests/52_SettingsPasswordEncryptionTest.php @@ -0,0 +1,64 @@ + $before['firma'] ?: 'Testfirma', + 'strasse' => $before['strasse'] ?: 'Teststr. 1', + 'plz' => $before['plz'] ?: '12345', + 'ort' => $before['ort'] ?: 'Testort', + 'nextcloud_pass' => 'geheimes-nc-passwort', + 'mail_smtp_pass' => 'geheimes-smtp-passwort', + ])); + + // Roh-DB-Spalte muss verschlüsselt sein, nicht im Klartext + $raw = Database::fetchOne('SELECT nextcloud_pass, mail_smtp_pass FROM company_settings WHERE id = 1'); + $this->assert(str_starts_with($raw['nextcloud_pass'], 'enc:v1:'), 'nextcloud_pass ist in der DB verschlüsselt'); + $this->assert(str_starts_with($raw['mail_smtp_pass'], 'enc:v1:'), 'mail_smtp_pass ist in der DB verschlüsselt'); + + // Setting::get() liefert wieder den Klartext + $after = Setting::get(); + $this->assertEquals('geheimes-nc-passwort', $after['nextcloud_pass'], 'nextcloud_pass entschlüsselt korrekt'); + $this->assertEquals('geheimes-smtp-passwort', $after['mail_smtp_pass'], 'mail_smtp_pass entschlüsselt korrekt'); + + // Leerer Wert wird nicht "verschlüsselt" (Crypto::encrypt('') === '') und bleibt leer/null + Setting::update(array_merge($after, [ + 'nextcloud_pass' => '', + 'mail_smtp_pass' => '', + ])); + $rawEmpty = Database::fetchOne('SELECT nextcloud_pass, mail_smtp_pass FROM company_settings WHERE id = 1'); + $this->assert(empty($rawEmpty['nextcloud_pass']), 'nextcloud_pass wird bei leerem Wert nicht verschlüsselt gespeichert'); + $this->assert(empty($rawEmpty['mail_smtp_pass']), 'mail_smtp_pass wird bei leerem Wert nicht verschlüsselt gespeichert'); + } finally { + if ($oldKey === null) { + unset($_ENV['APP_KEY']); + } else { + $_ENV['APP_KEY'] = $oldKey; + } + Config::load(); + } + } +}