Merge pull request 'Fix: Nextcloud-Belegimport – IP-Pinning bei Dual-Stack-Hosts brach jede WebDAV-Verbindung' (#162) from fds2610-bot/Steuerkram:main into main
Reviewed-on: https://codeberg.org/fds2610/Steuerkram/pulls/162
This commit is contained in:
fds2610
commit
8e47c8c3d3
5 changed files with 132 additions and 13 deletions
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
*
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -1 +1 @@
|
|||
1-260731
|
||||
1-260801
|
||||
|
|
|
|||
72
tests/74_SsrfPinningDualStackTest.php
Normal file
72
tests/74_SsrfPinningDualStackTest.php
Normal file
|
|
@ -0,0 +1,72 @@
|
|||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
use Steuerkram\Services\SsrfGuard;
|
||||
|
||||
/**
|
||||
* Test 74: IP-Pinning funktioniert auch bei Dual-Stack-Hosts (A + AAAA).
|
||||
*
|
||||
* Regression zum Nextcloud-Import: resolvePinned() lieferte pro IP einen eigenen
|
||||
* CURLOPT_RESOLVE-Eintrag. libcurl behandelt mehrere Einträge für dieselbe
|
||||
* HOST:PORT-Kombination aber nicht als Alternativen – der zuletzt gesetzte
|
||||
* verdrängt die vorherigen. Fiel dabei die IPv6-Adresse an und hatte der
|
||||
* Container keine IPv6-Konnektivität, scheiterte jeder Request mit
|
||||
* "Could not connect to server". Alle Adressen müssen daher in genau einem
|
||||
* Eintrag als Liste stehen.
|
||||
*/
|
||||
class SsrfPinningDualStackTest extends TestCase
|
||||
{
|
||||
protected string $testName = 'Test 74: IP-Pinning bei Dual-Stack-Hosts (A + AAAA)';
|
||||
|
||||
public function run(): void
|
||||
{
|
||||
$entries = SsrfGuard::resolvePinned('https://example.com/dav');
|
||||
$this->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'
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue