From 496ae92f0c0bedd2b7309cb6a8554a17523a4c4a Mon Sep 17 00:00:00 2001 From: fds2610-bot Date: Sat, 1 Aug 2026 11:03:48 +0200 Subject: [PATCH] Fix #nextcloud-import: IP-Pinning bei Dual-Stack-Hosts brach jede WebDAV-Verbindung --- Changes.md | 7 +++ src/Services/NextcloudImportService.php | 36 +++++++++++-- src/Services/SsrfGuard.php | 28 +++++++--- src/version.txt | 2 +- tests/74_SsrfPinningDualStackTest.php | 72 +++++++++++++++++++++++++ 5 files changed, 132 insertions(+), 13 deletions(-) create mode 100644 tests/74_SsrfPinningDualStackTest.php diff --git a/Changes.md b/Changes.md index d5539ee..57b8f78 100644 --- a/Changes.md +++ b/Changes.md @@ -2,6 +2,13 @@ --- +## 2026-08-01 – Nextcloud-Belegimport wieder funktionsfähig (IP-Pinning bei Dual-Stack-Hosts) + +- Seit dem Deployment des SSRF-Schutzes (#130) schlug der Nextcloud-Import bei jedem Lauf mit „PROPFIND fehlgeschlagen. URL/Zugangsdaten prüfen." fehl. Ursache war nicht die Konfiguration, sondern das IP-Pinning: `SsrfGuard::resolvePinned()` gab pro aufgelöster IP-Adresse einen eigenen `CURLOPT_RESOLVE`-Eintrag zurück. libcurl behandelt mehrere Einträge für dieselbe `HOST:PORT`-Kombination aber nicht als Alternativen – der zuletzt gesetzte verdrängt die vorherigen. Bei einem Host mit A- **und** AAAA-Record blieb dadurch nur die IPv6-Adresse übrig, und da der Container keine IPv6-Konnektivität hat, scheiterte jede Verbindung sofort mit „Could not connect to server". +- **`src/Services/SsrfGuard.php`:** `resolvePinned()` liefert jetzt genau einen Eintrag mit allen geprüften Adressen als Liste (`HOST:PORT:IP1,IP2,…`), IPv6 in eckigen Klammern. cURL probiert die Adressen damit wie bei eigener Auflösung nacheinander durch und fällt auf IPv4 zurück. Der DNS-Rebinding-Schutz bleibt unverändert – gepinnt wird weiterhin ausschließlich auf die zuvor validierten Adressen. +- **`src/Services/NextcloudImportService.php`:** Fehlgeschlagene Requests landeten bisher ohne jedes Detail im Ereignis-Log, was die Eingrenzung ohne Server-Zugriff unmöglich machte. PROPFIND- und Download-Fehler nennen jetzt den cURL-Fehlercode bzw. den HTTP-Status samt tatsächlich kontaktierter Gegenstelle (neue Methode `describeTransfer()`). +- **`tests/74_SsrfPinningDualStackTest.php`** (neu): Stellt sicher, dass `resolvePinned()` genau einen Eintrag liefert, beide Adressfamilien eines Dual-Stack-Hosts darin enthalten sind und IPv6-Adressen geklammert werden. + ## 2026-07-31 – Rechnungsstatus folgt automatisch den erfassten Zahlungen - Bisher blieb eine Ausgangsrechnung auch dann auf „Gesendet", wenn dem zugehörigen Beleg bereits Zahlungsbuchungen zugeordnet waren – der Status wurde ausschließlich manuell über „Rechnung als bezahlt markieren" gesetzt. Neu leitet sich der Status aus der Summe der zugeordneten Buchungen ab: **Teilzahlung** (weniger als der Rechnungsbetrag), **Bezahlt** (exakt der Rechnungsbetrag) und **Überzahlt** (mehr als der Rechnungsbetrag). Werden alle Buchungen storniert, fällt die Rechnung auf „Gesendet" bzw. „Finalisiert" zurück. diff --git a/src/Services/NextcloudImportService.php b/src/Services/NextcloudImportService.php index c9542e0..68166ac 100644 --- a/src/Services/NextcloudImportService.php +++ b/src/Services/NextcloudImportService.php @@ -49,6 +49,8 @@ class NextcloudImportService private string $davBase; private array $pendingEvents = []; private string|null $configError = null; + /** Details des letzten fehlgeschlagenen Requests (HTTP-Code bzw. cURL-Fehler) für das Ereignis-Log */ + private string $lastTransferError = ''; /** @var string[] CURLOPT_RESOLVE-Einträge für die beim Konstruieren geprüfte(n) IP(s) von baseUrl */ private array $pinnedResolve = []; @@ -105,7 +107,8 @@ class NextcloudImportService $files = $this->listFiles(); if ($files === null) { - $msg = 'FEHLER: PROPFIND fehlgeschlagen. URL/Zugangsdaten prüfen.'; + $msg = 'FEHLER: PROPFIND fehlgeschlagen. URL/Zugangsdaten prüfen.' + . ($this->lastTransferError !== '' ? ' – ' . $this->lastTransferError : ''); $result['details'][] = $msg; $result['errors']++; $this->persistEvent($msg); @@ -160,9 +163,11 @@ class NextcloudImportService $response = curl_exec($ch); $httpCode = curl_getinfo($ch, CURLINFO_HTTP_CODE); + $failed = $response === false || $httpCode < 200 || $httpCode >= 300; + $this->lastTransferError = $failed ? $this->describeTransfer($ch) : ''; curl_close($ch); - if ($response === false || $httpCode < 200 || $httpCode >= 300) { + if ($failed) { return null; } @@ -255,7 +260,8 @@ class NextcloudImportService $tmpPath = sys_get_temp_dir() . '/nextcloud_import_' . uniqid() . '.' . $ext; if (!$this->downloadFile($fileUrl, $tmpPath)) { - $msg = "FEHLER beim Download: {$filename}"; + $msg = "FEHLER beim Download: {$filename}" + . ($this->lastTransferError !== '' ? ' – ' . $this->lastTransferError : ''); $result['details'][] = $msg; $this->persistEvent($msg); $result['errors']++; @@ -353,10 +359,12 @@ class NextcloudImportService $ok = curl_exec($ch); $httpCode = curl_getinfo($ch, CURLINFO_HTTP_CODE); + $failed = $ok === false || $httpCode < 200 || $httpCode >= 300; + $this->lastTransferError = $failed ? $this->describeTransfer($ch) : ''; curl_close($ch); fclose($fh); - if ($ok === false || $httpCode < 200 || $httpCode >= 300) { + if ($failed) { @unlink($targetPath); return false; } @@ -392,6 +400,26 @@ class NextcloudImportService } } + /** + * Beschreibt einen fehlgeschlagenen Request so, dass er ohne Server-Zugriff + * eingegrenzt werden kann: Transportfehler mit cURL-Code, sonst HTTP-Status + * samt tatsächlich kontaktierter Gegenstelle. + */ + private function describeTransfer(\CurlHandle $ch): string + { + $errno = curl_errno($ch); + if ($errno !== 0) { + return sprintf('cURL-Fehler %d: %s', $errno, curl_error($ch)); + } + + $ip = (string)curl_getinfo($ch, CURLINFO_PRIMARY_IP); + return sprintf( + 'HTTP %d von %s', + (int)curl_getinfo($ch, CURLINFO_HTTP_CODE), + $ip !== '' ? $ip : 'unbekannt' + ); + } + /** * Erstellt einen vorkonfigurierten cURL-Handle mit Basis-Auth und TLS-Einstellungen. * diff --git a/src/Services/SsrfGuard.php b/src/Services/SsrfGuard.php index 0810a63..18e4bda 100644 --- a/src/Services/SsrfGuard.php +++ b/src/Services/SsrfGuard.php @@ -22,14 +22,22 @@ class SsrfGuard /** * Wie assertPublicHttpsUrl(), gibt aber zusätzlich die bereits geprüften - * IP-Adressen als CURLOPT_RESOLVE-Einträge ("HOST:PORT:IP") zurück. Damit - * kann der aufrufende Code den Host für den tatsächlichen Request auf genau - * diese IP(s) festnageln, statt ihn (z.B. durch cURL selbst) erneut über - * den System-Resolver aufzulösen – Schutz gegen DNS-Rebinding/TOCTOU + * IP-Adressen als CURLOPT_RESOLVE-Eintrag ("HOST:PORT:IP1,IP2,…") zurück. + * Damit kann der aufrufende Code den Host für den tatsächlichen Request auf + * genau diese IP(s) festnageln, statt ihn (z.B. durch cURL selbst) erneut + * über den System-Resolver aufzulösen – Schutz gegen DNS-Rebinding/TOCTOU * zwischen Prüfung und Verbindungsaufbau (siehe Issue #130). * - * @return string[] leer, wenn der Host bereits ein IP-Literal ist (dann - * ist kein Pinning nötig, cURL löst nichts auf) + * Alle Adressen stehen bewusst in einem einzigen Eintrag: Mehrere Einträge + * für dieselbe HOST:PORT-Kombination behandelt libcurl nicht als Alternativen, + * sondern der zuletzt gesetzte verdrängt die vorherigen. Bei einem Host mit + * A- und AAAA-Record blieb so nur eine Adressfamilie übrig – ohne IPv6 im + * Container scheiterte damit jeder Request. Als Liste in einem Eintrag + * probiert cURL die Adressen wie bei eigener Auflösung nacheinander durch. + * + * @return string[] genau ein Eintrag, oder leer, wenn der Host bereits ein + * IP-Literal ist (dann ist kein Pinning nötig, cURL löst + * nichts auf) * @throws \RuntimeException wenn die URL kein zulässiges Ziel ist */ public static function resolvePinned(string $url): array @@ -65,10 +73,14 @@ class SsrfGuard self::assertPublicIp($ip); } - return array_map( - static fn(string $ip): string => sprintf('%s:%d:%s', $host, $port, $ip), + // IPv6-Adressen in eckigen Klammern, damit ihre Doppelpunkte in der + // kommaseparierten Liste eindeutig bleiben. + $addresses = array_map( + static fn(string $ip): string => str_contains($ip, ':') ? '[' . $ip . ']' : $ip, $ips ); + + return [sprintf('%s:%d:%s', $host, $port, implode(',', $addresses))]; } private static function assertPublicIp(string $ip): void diff --git a/src/version.txt b/src/version.txt index f994931..e5c403d 100644 --- a/src/version.txt +++ b/src/version.txt @@ -1 +1 @@ -1-260731 +1-260801 diff --git a/tests/74_SsrfPinningDualStackTest.php b/tests/74_SsrfPinningDualStackTest.php new file mode 100644 index 0000000..1d0c207 --- /dev/null +++ b/tests/74_SsrfPinningDualStackTest.php @@ -0,0 +1,72 @@ +assertEquals(1, count($entries), 'resolvePinned() liefert genau einen CURLOPT_RESOLVE-Eintrag'); + + $entry = $entries[0]; + $this->assert( + str_starts_with($entry, 'example.com:443:'), + 'Eintrag beginnt mit "example.com:443:" (Eintrag: ' . $entry . ')' + ); + + $addresses = explode(',', substr($entry, strlen('example.com:443:'))); + $this->assertNotEmpty($addresses, 'Der Eintrag enthält mindestens eine Adresse'); + + foreach ($addresses as $address) { + // IPv6 muss geklammert sein, sonst sind seine Doppelpunkte in der Liste nicht abgrenzbar + $isBracketed = str_starts_with($address, '[') && str_ends_with($address, ']'); + $ip = $isBracketed ? substr($address, 1, -1) : $address; + + $this->assert( + filter_var($ip, FILTER_VALIDATE_IP) !== false, + 'Adresse "' . $address . '" ist eine gültige IP' + ); + $this->assertEquals( + str_contains($ip, ':'), + $isBracketed, + 'Genau IPv6-Adressen stehen in eckigen Klammern (Adresse: ' . $address . ')' + ); + } + + // Ein Dual-Stack-Host darf nicht dazu führen, dass eine Adressfamilie verlorengeht: + // beide aufgelösten Familien müssen im selben Eintrag landen. + $resolved = []; + foreach ((array)@dns_get_record('example.com', DNS_A + DNS_AAAA) as $record) { + if (!empty($record['ip'])) { + $resolved[] = $record['ip']; + } + if (!empty($record['ipv6'])) { + $resolved[] = $record['ipv6']; + } + } + + foreach ($resolved as $ip) { + $expected = str_contains($ip, ':') ? '[' . $ip . ']' : $ip; + $this->assert( + in_array($expected, $addresses, true), + 'Aufgelöste Adresse ' . $ip . ' ist im Pinning-Eintrag enthalten' + ); + } + } +}