From 95f9f344ec6719c2e5705ed48ce8d76213560233 Mon Sep 17 00:00:00 2001 From: fds2610-bot Date: Tue, 21 Jul 2026 19:21:16 +0200 Subject: [PATCH] =?UTF-8?q?Fix=20#testsuite:=20Test=2054=20(Session-Cookie?= =?UTF-8?q?)=20und=20Test=2056=20(Nextcloud-SSRF)=20dauerhaft=20gr=C3=BCn?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Changes.md | 8 ++++++++ src/Auth.php | 18 ++++++++++++++---- tests/54_SessionCookieHardeningTest.php | 24 ++++++++++-------------- tests/56_NextcloudSsrfTest.php | 7 +++++-- 4 files changed, 37 insertions(+), 20 deletions(-) diff --git a/Changes.md b/Changes.md index 671686a..f36bb60 100644 --- a/Changes.md +++ b/Changes.md @@ -2,6 +2,14 @@ --- +## 2026-07-21 – Fix: Zwei dauerhaft fehlschlagende Tests (Session-Cookie-Härtung, Nextcloud-SSRF) + +- **Test 54 (Session-Cookie-Härtung):** Der Test rief echtes `Auth::startSession()` auf. Da `tests/run.php` Ergebnisse laufend per `echo` ausgibt und Test 54 der einzige Test ist, der Sessions anfasst, war zum Testzeitpunkt bereits Output gesendet – PHP lehnt `session_set_cookie_params()`/`session_start()` danach mit „headers already sent" ab (Warnung + wirkungslos). Ein nachträglicher Output-Buffer im Test half nicht, da bereits vor dem Test unbuffered Output geflossen war. + - **`src/Auth.php`:** Neue reine Methode `Auth::cookieParams()` berechnet die gehärteten Cookie-Attribute (HttpOnly, SameSite, Secure inkl. Override/Proxy-Erkennung) ohne Seiteneffekt; `startSession()` nutzt sie jetzt intern. Verhalten zur Laufzeit unverändert. + - **`tests/54_SessionCookieHardeningTest.php`:** Prüft jetzt `Auth::cookieParams()` direkt statt eine echte Session zu starten – unabhängig vom bereits erfolgten Output der Test-Suite. +- **Test 56 (Nextcloud-SSRF):** `SsrfGuard::assertPublicHttpsUrl()` löst den Hostnamen bewusst per echtem DNS auf (Schutz vor DNS-Rebinding). Der Test nutzte `https://cloud.example.com` als „gültiges" Beispiel – diese Subdomain ist nicht registriert (NXDOMAIN), wurde also fälschlich als ungültig abgelehnt. + - **`tests/56_NextcloudSsrfTest.php`:** Nutzt stattdessen die laut IANA dauerhaft real auflösbare Apex-Domain `https://example.com`. + ## 2026-07-21 – Feature: Kontostand nach Direktabruf auf der Bank-Verbindungskachel - **`migrations/087_bank_sources_last_balance.sql`:** Neue Spalte `last_balance` in `bank_sources`. diff --git a/src/Auth.php b/src/Auth.php index 6330d71..047f7a8 100644 --- a/src/Auth.php +++ b/src/Auth.php @@ -21,6 +21,18 @@ class Auth return; } + session_set_cookie_params(self::cookieParams()); + session_start(); + } + + /** + * Berechnet die gehärteten Cookie-Attribute (HttpOnly, SameSite, Secure) anhand + * des aktuellen Requests bzw. des SESSION_COOKIE_SECURE-Overrides. Bewusst als + * reine Funktion ohne Seiteneffekt von startSession() getrennt, damit sie sich + * isoliert testen lässt, ohne eine echte PHP-Session zu starten. + */ + public static function cookieParams(): array + { $override = getenv('SESSION_COOKIE_SECURE'); if ($override === '1' || $override === '0') { $secure = $override === '1'; @@ -29,16 +41,14 @@ class Auth || ($_SERVER['HTTP_X_FORWARDED_PROTO'] ?? '') === 'https'; } - session_set_cookie_params([ + return [ 'lifetime' => 0, 'path' => '/', 'domain' => '', 'secure' => $secure, 'httponly' => true, 'samesite' => 'Lax', - ]); - - session_start(); + ]; } public static function require(): void diff --git a/tests/54_SessionCookieHardeningTest.php b/tests/54_SessionCookieHardeningTest.php index a627857..16dd4b3 100644 --- a/tests/54_SessionCookieHardeningTest.php +++ b/tests/54_SessionCookieHardeningTest.php @@ -10,43 +10,39 @@ class SessionCookieHardeningTest extends TestCase public function run(): void { - $httpsBackup = $_SERVER['HTTPS'] ?? null; - $forwardBackup = $_SERVER['HTTP_X_FORWARDED_PROTO'] ?? null; + $httpsBackup = $_SERVER['HTTPS'] ?? null; + $forwardBackup = $_SERVER['HTTP_X_FORWARDED_PROTO'] ?? null; unset($_SERVER['HTTPS'], $_SERVER['HTTP_X_FORWARDED_PROTO']); putenv('SESSION_COOKIE_SECURE'); + // Auth::cookieParams() berechnet die Cookie-Attribute ohne Seiteneffekt (kein echter + // session_start()). Ein echter Sessionstart würde hier scheitern, da der Test-Runner + // bereits Ergebnisse per echo ausgibt – PHP behandelt Header/Session-Cookies danach + // als "bereits gesendet". cookieParams() prüft dieselbe Logik ohne dieses Problem. try { // Plain HTTP ohne Override: httponly/samesite immer gesetzt, secure aus - Auth::startSession(); - $params = session_get_cookie_params(); + $params = Auth::cookieParams(); $this->assert($params['httponly'] === true, 'HttpOnly-Flag ist gesetzt'); $this->assertEquals('Lax', $params['samesite'], 'SameSite steht auf Lax'); $this->assert($params['secure'] === false, 'Secure ist bei Plain-HTTP ohne Override deaktiviert'); - session_write_close(); // Erzwungenes Secure per Env, unabhängig vom Request putenv('SESSION_COOKIE_SECURE=1'); - Auth::startSession(); - $params = session_get_cookie_params(); + $params = Auth::cookieParams(); $this->assert($params['secure'] === true, 'SESSION_COOKIE_SECURE=1 erzwingt Secure-Flag'); - session_write_close(); // Override hat Vorrang vor erkanntem HTTPS-Request putenv('SESSION_COOKIE_SECURE=0'); $_SERVER['HTTPS'] = 'on'; - Auth::startSession(); - $params = session_get_cookie_params(); + $params = Auth::cookieParams(); $this->assert($params['secure'] === false, 'SESSION_COOKIE_SECURE=0 deaktiviert Secure trotz HTTPS-Request'); - session_write_close(); // Ohne Override: Reverse-Proxy-Header aktiviert Secure automatisch putenv('SESSION_COOKIE_SECURE'); unset($_SERVER['HTTPS']); $_SERVER['HTTP_X_FORWARDED_PROTO'] = 'https'; - Auth::startSession(); - $params = session_get_cookie_params(); + $params = Auth::cookieParams(); $this->assert($params['secure'] === true, 'X-Forwarded-Proto: https aktiviert Secure automatisch'); - session_write_close(); } finally { putenv('SESSION_COOKIE_SECURE'); if ($httpsBackup === null) { diff --git a/tests/56_NextcloudSsrfTest.php b/tests/56_NextcloudSsrfTest.php index a9c98f3..1a3b19d 100644 --- a/tests/56_NextcloudSsrfTest.php +++ b/tests/56_NextcloudSsrfTest.php @@ -25,13 +25,16 @@ class NextcloudSsrfTest extends TestCase $this->assertRejected('https://localhost', 'localhost wird abgelehnt'); // Gültiges Ziel: baseUrl bleibt gesetzt, kein configError, davBase korrekt gebildet. - $this->configure('https://cloud.example.com', 'importuser', 'geheim'); + // Hinweis: SsrfGuard löst den Hostnamen per echtem DNS auf (Schutz vor DNS-Rebinding) – + // "cloud.example.com" ist keine registrierte Subdomain und liefert NXDOMAIN. Die + // Apex-Domain "example.com" ist dagegen laut IANA dauerhaft real auflösbar. + $this->configure('https://example.com', 'importuser', 'geheim'); $svc = new NextcloudImportService(); $baseUrl = $this->readProperty($svc, 'baseUrl'); $davBase = $this->readProperty($svc, 'davBase'); $cfgError = $this->readProperty($svc, 'configError'); - $this->assertEquals('https://cloud.example.com', $baseUrl, 'Gültige https-URL bleibt als baseUrl erhalten'); + $this->assertEquals('https://example.com', $baseUrl, 'Gültige https-URL bleibt als baseUrl erhalten'); $this->assert($cfgError === null, 'Kein configError bei gültiger URL'); $this->assert( str_contains((string)$davBase, '/remote.php/dav/files/importuser'),