Fix #193: SMTP-SSRF-Schutz per IP-Pinning, Admin-Pflicht für SMTP-Einstellungen

Changes.md

@@ -2,6 +2,14 @@
 
 ---
 
+## 2026-08-21 – SMTP-SSRF-Schutz: IP-Pinning gegen DNS-Rebinding, Admin-Pflicht für SMTP-Einstellungen (#193)
+
+- **`src/Services/SsrfGuard.php`:** neue `resolvePinnedHost(string $host): string[]` – wie `assertPublicHost()`, gibt aber zusätzlich die geprüfte(n) IP-Adresse(n) zurück, analog zu `resolvePinned()` für https-URLs. `assertPublicHost()` ruft sie jetzt intern auf.
+- **`src/Services/SmtpMailer.php`:** `connect()` löste den SMTP-Host bei `SsrfGuard::assertPublicHost()` und beim späteren `stream_socket_client('tcp://'.$host …')` zweimal unabhängig auf – ein Angreifer mit kurzer DNS-TTL konnte zwischen Prüfung und Verbindungsaufbau auf eine private/interne Adresse umschwenken (DNS-Rebinding/TOCTOU). Neue private Methode `buildConnectTarget()` nutzt die von `resolvePinnedHost()` gepinnte IP für `stream_socket_client()`; der ursprüngliche Hostname bleibt für SNI/Zertifikatsprüfung über `stream_context`-Option `ssl.peer_name` erhalten.
+- **`src/Controllers/SettingsController.php`:** `update()` und `mailTest()` verlangten bislang keine Admin-Rolle – jeder eingeloggte Benutzer konnte `mail_smtp_host`/`mail_smtp_port` setzen und per „SMTP testen" sofort einen Verbindungsversuch des Servers auslösen. Beide Methoden rufen jetzt `Auth::requireAdmin()`. `GET /settings` (Firmendaten anzeigen) bleibt für alle eingeloggten Benutzer erreichbar.
+- **`tests/98_SmtpSsrfPinningTest.php`** (neu): `SsrfGuard::resolvePinnedHost()` sowie `SmtpMailer::buildConnectTarget()` (per Reflection) – Verbindungsziel enthält die geprüfte IP statt des Hostnamens, `peer_name` bleibt gepinnt, private Hosts werden weiterhin abgelehnt.
+- **`tests/e2e/specs/settings-smtp-authorization.spec.js`** (neu): Nicht-Admin erhält 403 auf `POST /settings` und `POST /settings/mail-test`, Admin weiterhin nicht.
+
 ## 2026-08-18 – Inline-Styles und Inline-Handler reduziert (#160)
 
 Erster Umsetzungsschritt der in Issue #160 beschriebenen schrittweisen Reduktion (975 → 858 `style="`-Vorkommen, 51 → 0 Inline-Handler in `src/Views`). Kein Big-Bang über alle Views, sondern die im Ticket priorisierten Schritte 1–4:

src/Controllers/SettingsController.php

@@ -4,6 +4,7 @@ declare(strict_types=1);
 
 namespace Steuerkram\Controllers;
 
+use Steuerkram\Auth;
 use Steuerkram\Config;
 use Steuerkram\Models\BankSource;
 use Steuerkram\Models\Setting;
@@ -33,6 +34,12 @@ class SettingsController extends BaseController
 
     public function update(array $params): void
     {
+        // Firmendaten und SMTP-Zugangsdaten (inkl. mail_smtp_host) landen im
+        // selben Formular – ohne Admin-Pflicht könnte jeder eingeloggte
+        // Benutzer den SMTP-Host setzen und per mailTest() sofort einen
+        // Verbindungsversuch dorthin auslösen (Issue #193).
+        Auth::requireAdmin();
+
         $data   = $_POST;
         $errors = [];
 
@@ -163,6 +170,8 @@ class SettingsController extends BaseController
 
     public function mailTest(array $params): void
     {
+        Auth::requireAdmin();
+
         header('Content-Type: application/json');
         $settings = Setting::get();
         $to       = $settings['email'] ?? '';

src/Services/SmtpMailer.php

@@ -56,19 +56,7 @@ class SmtpMailer
 
     private function connect(): void
     {
-        SsrfGuard::assertPublicHost($this->host);
+        [$address, $ctx] = $this->buildConnectTarget();
-
-        $address = $this->encryption === 'ssl'
-            ? 'ssl://' . $this->host
-            : 'tcp://' . $this->host;
-
-        $ctx = stream_context_create([
-            'ssl' => [
-                'verify_peer'       => true,
-                'verify_peer_name'  => true,
-                'allow_self_signed' => false,
-            ],
-        ]);
 
         $sock = stream_socket_client(
             $address . ':' . $this->port,
@@ -97,6 +85,38 @@ class SmtpMailer
         }
     }
 
+    /**
+     * Löst den SMTP-Host per SsrfGuard::resolvePinnedHost() auf und pinnt die
+     * dabei geprüfte(n) IP-Adresse(n) für den eigentlichen Verbindungsaufbau:
+     * stream_socket_client() verbindet direkt zur IP statt den Hostnamen
+     * erneut über den System-Resolver aufzulösen. Ohne dieses Pinning könnte
+     * ein Angreifer mit kurzer DNS-TTL zwischen Prüfung und Connect auf eine
+     * private/interne Adresse umschwenken (DNS-Rebinding/TOCTOU, Issue #193).
+     * Für SNI und Zertifikatsprüfung bleibt der ursprüngliche Hostname über
+     * den Stream-Context ('ssl' => 'peer_name') erhalten.
+     *
+     * @return array{0:string,1:resource} [Verbindungsziel "scheme://ip", Stream-Context]
+     */
+    private function buildConnectTarget(): array
+    {
+        $ips = SsrfGuard::resolvePinnedHost($this->host);
+        $ip  = str_contains($ips[0], ':') ? '[' . $ips[0] . ']' : $ips[0];
+
+        $scheme  = $this->encryption === 'ssl' ? 'ssl' : 'tcp';
+        $address = $scheme . '://' . $ip;
+
+        $ctx = stream_context_create([
+            'ssl' => [
+                'verify_peer'       => true,
+                'verify_peer_name'  => true,
+                'allow_self_signed' => false,
+                'peer_name'         => $this->host,
+            ],
+        ]);
+
+        return [$address, $ctx];
+    }
+
     private function ehlo(): void
     {
         $this->cmd('EHLO ' . (gethostname() ?: 'localhost'));

src/Services/SsrfGuard.php

@@ -92,6 +92,22 @@ class SsrfGuard
      * @throws \RuntimeException wenn der Host kein zulässiges Ziel ist
      */
     public static function assertPublicHost(string $host): void
+    {
+        self::resolvePinnedHost($host);
+    }
+
+    /**
+     * Wie assertPublicHost(), gibt aber zusätzlich die dabei geprüfte(n)
+     * IP-Adresse(n) zurück, damit der Aufrufer sie beim tatsächlichen
+     * Verbindungsaufbau pinnen kann – analog zu resolvePinned(), aber ohne
+     * URL-Schema (z.B. für den SMTP-Host aus stream_socket_client(), siehe
+     * Issue #193: ohne Pinning löst der spätere Connect den Hostnamen erneut
+     * auf, was DNS-Rebinding/TOCTOU zwischen Prüfung und Verbindung erlaubt).
+     *
+     * @return string[] geprüfte IP-Adresse(n), mindestens ein Eintrag
+     * @throws \RuntimeException wenn der Host kein zulässiges Ziel ist
+     */
+    public static function resolvePinnedHost(string $host): array
     {
         $host = trim($host, " \t\n\r\0\x0B[]");
         if ($host === '') {
@@ -100,7 +116,7 @@ class SsrfGuard
 
         if (filter_var($host, FILTER_VALIDATE_IP)) {
             self::assertPublicIp($host);
-            return;
+            return [$host];
         }
 
         if (strtolower($host) === 'localhost') {
@@ -114,6 +130,8 @@ class SsrfGuard
         foreach ($ips as $ip) {
             self::assertPublicIp($ip);
         }
+
+        return $ips;
     }
 
     private static function assertPublicIp(string $ip): void

tests/98_SmtpSsrfPinningTest.php

@@ -0,0 +1,93 @@
+<?php
+
+declare(strict_types=1);
+
+use Steuerkram\Services\SmtpMailer;
+use Steuerkram\Services\SsrfGuard;
+
+/**
+ * Test 98: SSRF-Schutz gegen DNS-Rebinding/TOCTOU im SMTP-Pfad (Issue #193).
+ * SsrfGuard::resolvePinnedHost() liefert für einen SMTP-Host die bereits
+ * geprüfte(n) IP-Adresse(n) zurück; SmtpMailer::buildConnectTarget() nutzt
+ * diese für stream_socket_client(), statt den Hostnamen beim eigentlichen
+ * Verbindungsaufbau erneut über den System-Resolver aufzulösen.
+ */
+class SmtpSsrfPinningTest extends TestCase
+{
+    protected string $testName = 'Test 98: SSRF-Schutz gegen DNS-Rebinding im SMTP-Pfad (IP-Pinning)';
+
+    public function run(): void
+    {
+        // SsrfGuard::resolvePinnedHost() liefert für einen gültigen Hostnamen
+        // mindestens eine geprüfte, öffentliche IP-Adresse zurück.
+        $ips = SsrfGuard::resolvePinnedHost('example.com');
+        $this->assertNotEmpty($ips, 'resolvePinnedHost() liefert IP-Adressen für einen gültigen Hostnamen');
+        foreach ($ips as $ip) {
+            $this->assert(
+                filter_var($ip, FILTER_VALIDATE_IP) !== false,
+                'Jeder Eintrag ist eine gültige IP-Adresse (Eintrag: ' . $ip . ')'
+            );
+        }
+
+        // IP-Literal als Host: kein DNS-Lookup nötig, aber weiterhin gegen
+        // private/interne Bereiche geprüft.
+        $this->assertEquals(['8.8.8.8'], SsrfGuard::resolvePinnedHost('8.8.8.8'), 'IP-Literal wird unverändert zurückgegeben');
+        $this->assertRejected('127.0.0.1');
+        $this->assertRejected('localhost');
+        $this->assertRejected('169.254.169.254');
+
+        // SmtpMailer::buildConnectTarget(): verbindet zur gepinnten IP, nicht
+        // erneut zum Hostnamen – und hält den Hostnamen für SNI/Zertifikats-
+        // prüfung über den Stream-Context ('peer_name') fest.
+        $mailer = new SmtpMailer('example.com', 465, 'ssl', 'user', 'pass', 'from@example.com', 'Absender');
+        [$address, $ctx] = $this->buildConnectTarget($mailer);
+
+        $this->assert(str_starts_with($address, 'ssl://'), 'Verbindungsziel nutzt das SSL-Schema für encryption=ssl');
+        $host = substr($address, strlen('ssl://'));
+        $this->assert(
+            filter_var($host, FILTER_VALIDATE_IP) !== false,
+            'Verbindungsziel enthält eine IP-Adresse statt des Hostnamens (Ziel: ' . $address . ')'
+        );
+        $this->assert(
+            in_array($host, SsrfGuard::resolvePinnedHost('example.com'), true),
+            'Ziel-IP stammt aus der SsrfGuard-Auflösung des Hostnamens (Ziel: ' . $host . ')'
+        );
+
+        $sslOptions = stream_context_get_options($ctx)['ssl'] ?? [];
+        $this->assertEquals('example.com', $sslOptions['peer_name'] ?? null, 'Stream-Context pinnt peer_name auf den ursprünglichen Hostnamen (für SNI/Zertifikatsprüfung)');
+        $this->assert(($sslOptions['verify_peer'] ?? false) === true, 'Zertifikatsprüfung bleibt trotz IP-Verbindung aktiv (verify_peer)');
+        $this->assert(($sslOptions['verify_peer_name'] ?? false) === true, 'Zertifikatsprüfung bleibt trotz IP-Verbindung aktiv (verify_peer_name)');
+
+        // tcp:// für unverschlüsselt/STARTTLS statt ssl://
+        $mailerTls = new SmtpMailer('example.com', 587, 'tls', 'user', 'pass', 'from@example.com', 'Absender');
+        [$addressTls] = $this->buildConnectTarget($mailerTls);
+        $this->assert(str_starts_with($addressTls, 'tcp://'), 'Verbindungsziel nutzt das TCP-Schema für encryption=tls (STARTTLS folgt später)');
+
+        // Private/interne SMTP-Hosts werden weiterhin abgelehnt, bevor überhaupt
+        // ein Verbindungsversuch unternommen wird.
+        $mailerPrivate = new SmtpMailer('127.0.0.1', 25, 'none', 'user', 'pass', 'from@example.com', 'Absender');
+        try {
+            $this->buildConnectTarget($mailerPrivate);
+            $this->assert(false, 'buildConnectTarget() lehnt private SMTP-Hosts ab');
+        } catch (\RuntimeException $e) {
+            $this->assert(true, 'buildConnectTarget() lehnt private SMTP-Hosts ab');
+        }
+    }
+
+    private function assertRejected(string $host): void
+    {
+        try {
+            SsrfGuard::resolvePinnedHost($host);
+            $this->assert(false, 'Privater/interner Host (' . $host . ') wird abgelehnt');
+        } catch (\RuntimeException $e) {
+            $this->assert(true, 'Privater/interner Host (' . $host . ') wird abgelehnt');
+        }
+    }
+
+    private function buildConnectTarget(SmtpMailer $mailer): array
+    {
+        $method = new \ReflectionMethod($mailer, 'buildConnectTarget');
+        $method->setAccessible(true);
+        return $method->invoke($mailer);
+    }
+}

tests/e2e/specs/settings-smtp-authorization.spec.js

@@ -0,0 +1,80 @@
+// @ts-check
+// Fix #193: SettingsController::update()/mailTest() verlangten kein
+// Auth::requireAdmin() – jeder eingeloggte Benutzer konnte mail_smtp_host
+// setzen und per "SMTP testen" sofort einen Verbindungsversuch des Servers
+// zu einem beliebigen Ziel auslösen (Ausgangspunkt für DNS-Rebinding/SSRF).
+// Dieser Test prüft die Durchsetzung auf HTTP-Ebene, die die PHP-Suite nicht
+// erreicht (Router-Guard, Auth::requireAdmin()).
+
+const { test, expect } = require('@playwright/test')
+const { uniqueTag } = require('../helpers/app')
+
+test.describe('Autorisierung: SMTP-Einstellungen nur für Admins (Fix #193)', () => {
+	test('Nicht-Admin wird von POST /settings und /settings/mail-test ausgesperrt, Admin nicht', async ({ page, browser }) => {
+		const username = uniqueTag('noadmin').toLowerCase().replace(/[^a-z0-9-]/g, '').slice(0, 64)
+		const password = 'TempPass123!'
+
+		await page.goto('/useradmin')
+		await page.fill('#field-username', username)
+		await page.fill('#field-password', password)
+		await page.click('#submit-btn')
+		await page.waitForURL('/useradmin')
+
+		const row = page.locator('tr', { hasText: username })
+		await expect(row).toBeVisible()
+
+		try {
+			const context = await browser.newContext({ storageState: { cookies: [], origins: [] } })
+			const userPage = await context.newPage()
+			await userPage.goto('/login')
+			await userPage.fill('#username', username)
+			await userPage.fill('#password', password)
+			await userPage.click('form[action="/login"] button[type="submit"]')
+			await userPage.waitForURL(url => !url.pathname.startsWith('/login'))
+
+			// GET /settings bleibt für alle eingeloggten Benutzer erreichbar
+			// (Firmendaten sind kein Admin-Feature) – nur die SMTP-mutierenden
+			// Endpunkte werden geschützt.
+			await userPage.goto('/settings')
+			const csrfToken = await userPage.evaluate(() => document.querySelector('meta[name="csrf-token"]')?.content || '')
+			expect(csrfToken).not.toBe('')
+
+			const mailTestResponse = await userPage.request.post('/settings/mail-test', {
+				headers: { 'X-CSRF-Token': csrfToken },
+				form: {},
+			})
+			expect(mailTestResponse.status()).toBe(403)
+			await expect(mailTestResponse.text()).resolves.toContain('Kein Zugriff')
+
+			const updateResponse = await userPage.request.post('/settings', {
+				headers: { 'X-CSRF-Token': csrfToken },
+				form: { firma: 'Sollte nicht gespeichert werden', strasse: 'X', plz: '12345', ort: 'X', mail_smtp_host: '127.0.0.1' },
+			})
+			expect(updateResponse.status()).toBe(403)
+			await expect(updateResponse.text()).resolves.toContain('Kein Zugriff')
+
+			await context.close()
+		} finally {
+			const delForm = row.locator('form[action$="/delete"]')
+			if (await delForm.count() > 0) {
+				await delForm.locator('button[type="submit"]').click()
+				await page.waitForLoadState('networkidle')
+			}
+		}
+	})
+
+	test('Admin darf /settings/mail-test weiterhin aufrufen (kein 403)', async ({ page }) => {
+		await page.goto('/settings')
+		const csrfToken = await page.evaluate(() => document.querySelector('meta[name="csrf-token"]')?.content || '')
+		expect(csrfToken).not.toBe('')
+
+		const response = await page.request.post('/settings/mail-test', {
+			headers: { 'X-CSRF-Token': csrfToken },
+			form: {},
+		})
+		// Egal ob die SMTP-Verbindung selbst gelingt oder scheitert (abhängig von
+		// den hinterlegten Test-Einstellungen) – der Admin-Zugriff selbst darf
+		// nicht mit 403 verweigert werden.
+		expect(response.status()).not.toBe(403)
+	})
+})