diff --git a/adguard-shield.conf b/adguard-shield.conf index 591b29d..460a054 100644 --- a/adguard-shield.conf +++ b/adguard-shield.conf @@ -77,6 +77,10 @@ EXTERNAL_BLOCKLIST_AUTO_UNBAN=true EXTERNAL_BLOCKLIST_NOTIFY=false # Bei großen Listen auf false lassen EXTERNAL_BLOCKLIST_CACHE_DIR="/var/lib/adguard-shield/external-blocklist" +# --- Externe Listen: Transport --- +# Externe White-/Blocklisten werden standardmäßig nur per https:// geladen. +ALLOW_INSECURE_LIST_URLS=false # true = auch http:// erlauben (unsicher, Listen können manipuliert werden) + # --- Progressive Sperren (Recidive) --- # Wiederholungstäter werden stufenweise länger gesperrt PROGRESSIVE_BAN_ENABLED=true diff --git a/adguard-shield.service b/adguard-shield.service index 8d1e015..805c449 100644 --- a/adguard-shield.service +++ b/adguard-shield.service @@ -15,16 +15,33 @@ Restart=on-failure RestartSec=30 # Sicherheits-Hardening +# Der Daemon läuft als root, aber nur mit den Capabilities, die er wirklich +# nutzt. Log, State-DB, Listen-Caches, GeoIP-Downloads und PID-Datei gehören +# root, daher sind CAP_DAC_OVERRIDE, CAP_DAC_READ_SEARCH, CAP_FOWNER und +# CAP_CHOWN nicht nötig. Der Daemon wechselt keine Benutzer (kein CAP_SETUID/ +# CAP_SETGID) und signalisiert keine fremden Prozesse (kein CAP_KILL). +# E-Mail-Reports laufen per Cron (report-send) außerhalb dieses Service; +# NoNewPrivileges betrifft sendmail/postdrop daher nicht. ProtectSystem=full ReadWritePaths=/var/log /var/lib/adguard-shield /var/run /opt/adguard-shield/geoip ProtectHome=true -NoNewPrivileges=false PrivateTmp=true +NoNewPrivileges=true +RestrictSUIDSGID=true +LockPersonality=true +RestrictRealtime=true +ProtectClock=true +ProtectControlGroups=true +ProtectKernelLogs=true +SystemCallArchitectures=native +# Bewusst nicht gesetzt: ProtectKernelTunables (iptables/ipset können +# /proc/sys/net beschreiben) und ProtectKernelModules (Netfilter-Module). -# iptables benötigt CAP_NET_ADMIN + CAP_NET_RAW -# Weitere Capabilities für Dateizugriff, Signale und Prozessverwaltung -AmbientCapabilities=CAP_NET_ADMIN CAP_NET_RAW -CapabilityBoundingSet=CAP_NET_ADMIN CAP_NET_RAW CAP_DAC_OVERRIDE CAP_DAC_READ_SEARCH CAP_FOWNER CAP_KILL CAP_SETUID CAP_SETGID CAP_CHOWN +# CAP_NET_ADMIN: iptables, ip6tables, ipset +# CAP_NET_RAW: iptables-legacy nutzt einen Raw-Socket +# CAP_NET_BIND_SERVICE: nur für WEB_LISTEN mit einem Port unter 1024 +AmbientCapabilities=CAP_NET_ADMIN CAP_NET_RAW CAP_NET_BIND_SERVICE +CapabilityBoundingSet=CAP_NET_ADMIN CAP_NET_RAW CAP_NET_BIND_SERVICE # Logging StandardOutput=journal diff --git a/cmd/adguard-shieldd/main.go b/cmd/adguard-shieldd/main.go index f620bb6..954bc7a 100644 --- a/cmd/adguard-shieldd/main.go +++ b/cmd/adguard-shieldd/main.go @@ -529,13 +529,6 @@ func flushSource(ctx context.Context, d *daemon.Daemon, source string) error { return nil } -func writePID(path string) error { - if path == "" { - return nil - } - return os.WriteFile(path, []byte(strconv.Itoa(os.Getpid())+"\n"), 0644) -} - func commandContext(d *daemon.Daemon, daemonMode bool) (context.Context, context.CancelFunc) { ctx, cancel := context.WithCancel(context.Background()) signals := make(chan os.Signal, 1) diff --git a/cmd/adguard-shieldd/pidfile.go b/cmd/adguard-shieldd/pidfile.go new file mode 100644 index 0000000..3081cc6 --- /dev/null +++ b/cmd/adguard-shieldd/pidfile.go @@ -0,0 +1,38 @@ +package main + +import ( + "fmt" + "os" + "strconv" +) + +// writePID schreibt die PID des laufenden Prozesses nach path. Da der Daemon +// als root läuft und PID_FILE konfigurierbar ist, folgt das Öffnen keinem +// Symlink (O_NOFOLLOW, siehe pidfile_unix.go) und das Ziel muss eine reguläre +// Datei sein. Die Datei ist bewusst für alle lesbar (0644). +func writePID(path string) error { + if path == "" { + return nil + } + if fi, err := os.Lstat(path); err == nil && !fi.Mode().IsRegular() { + return fmt.Errorf("PID-Datei %s ist keine reguläre Datei (%s)", path, fi.Mode().Type()) + } + f, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC|pidOpenFlags, 0644) + if err != nil { + return fmt.Errorf("PID-Datei %s kann nicht geschrieben werden: %w", path, err) + } + // Erneut am geöffneten Handle prüfen: Zwischen Lstat und OpenFile könnte + // die Datei ersetzt worden sein. + if fi, err := f.Stat(); err != nil || !fi.Mode().IsRegular() { + _ = f.Close() + if err == nil { + err = fmt.Errorf("keine reguläre Datei (%s)", fi.Mode().Type()) + } + return fmt.Errorf("PID-Datei %s: %w", path, err) + } + if _, err := f.WriteString(strconv.Itoa(os.Getpid()) + "\n"); err != nil { + _ = f.Close() + return fmt.Errorf("PID-Datei %s: %w", path, err) + } + return f.Close() +} diff --git a/cmd/adguard-shieldd/pidfile_other.go b/cmd/adguard-shieldd/pidfile_other.go new file mode 100644 index 0000000..f63c883 --- /dev/null +++ b/cmd/adguard-shieldd/pidfile_other.go @@ -0,0 +1,7 @@ +//go:build !unix + +package main + +// Auf Nicht-Unix-Systemen (z. B. Windows-Entwicklungsbuilds) gibt es kein +// O_NOFOLLOW; dort bleibt nur die Prüfung auf eine reguläre Datei. +const pidOpenFlags = 0 diff --git a/cmd/adguard-shieldd/pidfile_test.go b/cmd/adguard-shieldd/pidfile_test.go new file mode 100644 index 0000000..8495c6e --- /dev/null +++ b/cmd/adguard-shieldd/pidfile_test.go @@ -0,0 +1,76 @@ +package main + +import ( + "os" + "path/filepath" + "runtime" + "strconv" + "strings" + "testing" +) + +func TestWritePIDCreatesFile(t *testing.T) { + path := filepath.Join(t.TempDir(), "adguard-shield.pid") + if err := os.WriteFile(path, []byte("999999999\nalter Inhalt\n"), 0600); err != nil { + t.Fatal(err) + } + if err := writePID(path); err != nil { + t.Fatalf("writePID: %v", err) + } + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if got, want := strings.TrimSpace(string(data)), strconv.Itoa(os.Getpid()); got != want { + t.Fatalf("PID-Datei = %q, want %q", got, want) + } +} + +func TestWritePIDEmptyPath(t *testing.T) { + if err := writePID(""); err != nil { + t.Fatalf("writePID(\"\") = %v", err) + } +} + +func TestWritePIDRejectsDirectory(t *testing.T) { + if err := writePID(t.TempDir()); err == nil { + t.Fatal("writePID akzeptiert ein Verzeichnis") + } +} + +func TestWritePIDRejectsSymlink(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Symlinks/O_NOFOLLOW werden unter Windows nicht geprüft") + } + dir := t.TempDir() + target := filepath.Join(dir, "ziel") + if err := os.WriteFile(target, []byte("unverändert\n"), 0600); err != nil { + t.Fatal(err) + } + link := filepath.Join(dir, "adguard-shield.pid") + if err := os.Symlink(target, link); err != nil { + t.Fatal(err) + } + if err := writePID(link); err == nil { + t.Fatal("writePID folgt einem Symlink") + } + data, err := os.ReadFile(target) + if err != nil { + t.Fatal(err) + } + if string(data) != "unverändert\n" { + t.Fatalf("Symlink-Ziel wurde verändert: %q", data) + } + // Auch ein hängender Symlink darf keine Datei am Ziel anlegen. + dangling := filepath.Join(dir, "haengend.pid") + missing := filepath.Join(dir, "fehlt") + if err := os.Symlink(missing, dangling); err != nil { + t.Fatal(err) + } + if err := writePID(dangling); err == nil { + t.Fatal("writePID folgt einem hängenden Symlink") + } + if _, err := os.Stat(missing); !os.IsNotExist(err) { + t.Fatalf("Ziel des hängenden Symlinks wurde angelegt: %v", err) + } +} diff --git a/cmd/adguard-shieldd/pidfile_unix.go b/cmd/adguard-shieldd/pidfile_unix.go new file mode 100644 index 0000000..1da24df --- /dev/null +++ b/cmd/adguard-shieldd/pidfile_unix.go @@ -0,0 +1,9 @@ +//go:build unix + +package main + +import "syscall" + +// O_NOFOLLOW: ein Symlink als letztes Pfadelement führt zu ELOOP. +// O_NONBLOCK: ein untergeschobenes FIFO blockiert das Öffnen nicht. +const pidOpenFlags = syscall.O_NOFOLLOW | syscall.O_NONBLOCK diff --git a/docs/architektur.md b/docs/architektur.md index d8e77de..bcc819e 100644 --- a/docs/architektur.md +++ b/docs/architektur.md @@ -381,6 +381,25 @@ Nach einer Standardinstallation sieht die Struktur so aus: └── adguard-shield-report # Cron-Job für Reports (optional) ``` +### Rechte des systemd-Service + +Der Daemon läuft als root, die Unit beschränkt die Capabilities aber auf das, was er tatsächlich braucht: + +| Capability | Wofür | +|---|---| +| `CAP_NET_ADMIN` | `iptables`, `ip6tables`, `ipset` (Chain, Regeln, Sets) | +| `CAP_NET_RAW` | `iptables-legacy` öffnet einen Raw-Socket | +| `CAP_NET_BIND_SERVICE` | nur nötig, wenn `WEB_LISTEN` einen Port unter 1024 nutzt | + +Alles andere (u. a. `CAP_DAC_OVERRIDE`, `CAP_SETUID`, `CAP_KILL`) ist entfernt. Zusätzlich gelten `NoNewPrivileges=true`, `RestrictSUIDSGID`, `LockPersonality`, `RestrictRealtime`, `ProtectClock`, `ProtectControlGroups`, `ProtectKernelLogs` und `SystemCallArchitectures=native`. `ProtectKernelTunables` und `ProtectKernelModules` sind bewusst nicht gesetzt, damit iptables/ipset Netfilter-Einstellungen und -Module wie bisher nutzen können. + +Ein Betrieb als eigener Benutzer (`User=`) ist nicht vorgesehen: Firewall-Befehle, `/var/run`, `/var/log` und die Installation unter `/opt` setzen root voraus, und eine Umstellung würde bestehende Dateirechte ändern. + +Folgen für die Praxis: + +- **Dateirechte:** Ohne `CAP_DAC_OVERRIDE` gelten auch für root die normalen Dateirechte. Alle Pfade, die der Daemon schreibt (`LOG_FILE`, `STATE_DIR`, Listen-Caches, `geoip/`, `PID_FILE`), müssen root gehören oder für root beschreibbar sein; gelesene Dateien (Konfiguration, `GEOIP_MMDB_PATH`) müssen für root lesbar sein. Bei der Standardinstallation ist das der Fall. +- **E-Mail-Reports:** Reports werden nicht vom Service, sondern per Cron (`report-send`) bzw. per CLI versendet und unterliegen diesen Einschränkungen nicht. `NoNewPrivileges` würde z. B. den setgid-Helfer `postdrop` von Postfix ausbremsen – das betrifft den Report-Versand daher nicht. + ## Hintergrundjobs im Daemon Es gibt in der Go-Version keine separaten Worker-Skripte mehr. Diese Aufgaben laufen als Goroutines im Daemon: diff --git a/docs/konfiguration.md b/docs/konfiguration.md index f38759d..7a86e14 100644 --- a/docs/konfiguration.md +++ b/docs/konfiguration.md @@ -38,6 +38,7 @@ Ungültige Werte führen nicht zum Abbruch, damit der Schutz weiterläuft. Statt | Ungültige Ports | `BLOCKED_PORTS="53 abc 70000"` (ungültige Einträge werden ignoriert; ist kein Port gültig, gilt `53 443 853`) | | Ungültige Auswahlwerte | `GEOIP_MODE`, `FIREWALL_MODE`, `LOG_LEVEL`, `NOTIFY_TYPE`, `REPORT_FORMAT`, `REPORT_INTERVAL` | | Ungültige Web-Adresse/-Pfade | `WEB_LISTEN="8443"`, `WEB_BASE_PATH="/../x"`, nur `WEB_TLS_CERT` ohne `WEB_TLS_KEY` | +| Listen-URLs ohne `https://` | `EXTERNAL_BLOCKLIST_URLS="http://..."` oder `file://...` (die Quelle wird nicht geladen, siehe [Externe Listen: Transport und Größe](#externe-listen-transport-und-größe)) | Veraltete Schlüssel der Shell-Version wie `BAN_HISTORY_FILE` werden ignoriert und mit einem eigenen Hinweis gemeldet. @@ -99,7 +100,7 @@ AdGuard Shield ruft intern diesen Endpunkt ab: /control/querylog?limit=&response_status=all ``` -**Hinweis:** Standardmäßig (`ADGUARD_TLS_INSECURE=false`) prüft AdGuard Shield das TLS-Zertifikat der AdGuard-Home-API. Nutzt AdGuard Home ein selbstsigniertes Zertifikat, schlägt die Verbindung fehl. Abhilfe: gültiges Zertifikat, die API lokal per `http://127.0.0.1:` ansprechen oder `ADGUARD_TLS_INSECURE=true` setzen. Letzteres erlaubt Man-in-the-Middle-Angriffe auf die API-Verbindung (Zugangsdaten und Querylog) und sollte nur in vertrauenswürdigen Netzen genutzt werden. Benachrichtigungen prüfen Zertifikate standardmäßig; nur mit `NOTIFY_TLS_INSECURE=true` werden für das Benachrichtigungsziel auch selbstsignierte Zertifikate akzeptiert. AbuseIPDB, externe Listen und der MaxMind-Download prüfen Zertifikate immer. +**Hinweis:** Standardmäßig (`ADGUARD_TLS_INSECURE=false`) prüft AdGuard Shield das TLS-Zertifikat der AdGuard-Home-API. Nutzt AdGuard Home ein selbstsigniertes Zertifikat, schlägt die Verbindung fehl. Abhilfe: gültiges Zertifikat, die API lokal per `http://127.0.0.1:` ansprechen oder `ADGUARD_TLS_INSECURE=true` setzen. Letzteres erlaubt Man-in-the-Middle-Angriffe auf die API-Verbindung (Zugangsdaten und Querylog) und sollte nur in vertrauenswürdigen Netzen genutzt werden. Benachrichtigungen prüfen Zertifikate standardmäßig; nur mit `NOTIFY_TLS_INSECURE=true` werden für das Benachrichtigungsziel auch selbstsignierte Zertifikate akzeptiert. AbuseIPDB, externe Listen und der MaxMind-Download prüfen Zertifikate immer. Externe Listen werden außerdem nur per `https://` geladen (siehe [Externe Listen: Transport und Größe](#externe-listen-transport-und-größe)). --- @@ -559,6 +560,24 @@ EXTERNAL_BLOCKLIST_NOTIFY=false --- +## Externe Listen: Transport und Größe + +Die externe Whitelist und die externe Blocklist fließen direkt in Firewall-Entscheidungen ein. Deshalb gelten für beide dieselben Regeln beim Download: + +| Parameter | Standard | Beschreibung | +|---|---|---| +| `ALLOW_INSECURE_LIST_URLS` | `false` | `true` = Listen auch unverschlüsselt per `http://` laden (unsicher) | + +- Erlaubt sind nur `https://`-URLs mit Host. `http://` wird nur mit `ALLOW_INSECURE_LIST_URLS=true` akzeptiert, andere Schemata wie `file://` oder `ftp://` nie. +- Eine abgelehnte URL meldet schon beim Start eine Konfigurationswarnung und bei jedem Sync eine Download-Warnung. Sie zählt als fehlgeschlagene Quelle: Die externe Whitelist bleibt dann unverändert, der Auto-Unban der Blocklist wird übersprungen. Ein vorhandener Cache dieser Quelle wird bewusst nicht genutzt, da er aus einem unverschlüsselten Download stammen kann. +- Mit `ALLOW_INSECURE_LIST_URLS=true` wird für jede `http://`-Quelle beim Start und beim ersten Download eine Warnung geschrieben. +- Weiterleitungen müssen dieselben Regeln erfüllen. Eine Weiterleitung von `https://` auf `http://` wird ohne `ALLOW_INSECURE_LIST_URLS=true` abgelehnt; dann wird wie bei anderen Download-Fehlern der Cache der Quelle verwendet. +- Eine Liste darf höchstens 50 MB groß sein. Größere Downloads werden verworfen statt abgeschnitten: Der Cache bleibt unverändert und wird verwendet (Warnung im Log); ohne Cache gilt die Quelle als fehlgeschlagen. + +**Hinweis für Updates:** Bis v1.1.x wurden `http://`-Listen ohne Rückfrage geladen. Nach dem Update werden sie abgelehnt, im Log steht z.B. `blocklist Download Warnung Quelle #1 (http://example.com): unverschlüsseltes http:// ist nicht erlaubt ...`. Abhilfe: die URL in `EXTERNAL_WHITELIST_URLS` bzw. `EXTERNAL_BLOCKLIST_URLS` auf `https://` umstellen (die meisten Anbieter unterstützen das) und den Dienst neu starten. Nur wenn die Quelle kein HTTPS anbietet und das Netz vertrauenswürdig ist, bewusst `ALLOW_INSECURE_LIST_URLS=true` setzen. Ein Angreifer im Netzwerkpfad kann sonst beliebige IPs auf die Blocklist setzen oder die Whitelist verändern. + +--- + ## Web-Statusseite Optionale, schreibgeschützte Statusseite mit Live-Ansicht. Einrichtung, Absicherung und Reverse-Proxy-Beispiele: [Web-Statusseite](webinterface.md). diff --git a/docs/report.md b/docs/report.md index bf90314..dc1de0c 100644 --- a/docs/report.md +++ b/docs/report.md @@ -53,6 +53,8 @@ Sie werden ins Binary eingebettet und bei `install` sowie `update` nach `/opt/ad | `REPORT_MAIL_CMD` | `msmtp` | Mailprogramm für den Versand | | `REPORT_BUSIEST_DAY_RANGE` | `30` | Zeitraum für "Aktivster Tag"; `0` nutzt den Berichtszeitraum | +`REPORT_EMAIL_FROM` erwartet genau eine Adresse, `REPORT_EMAIL_TO` eine oder mehrere kommagetrennte Adressen; beide auch in der Form `Name `. Werte mit Zeilenumbrüchen oder anderen Steuerzeichen werden abgelehnt. + ### Versandintervalle | Intervall | Versandzeitpunkt | diff --git a/docs/tipps-und-troubleshooting.md b/docs/tipps-und-troubleshooting.md index 3575533..00db2c2 100644 --- a/docs/tipps-und-troubleshooting.md +++ b/docs/tipps-und-troubleshooting.md @@ -53,6 +53,8 @@ sudo journalctl -u adguard-shield --no-pager -n 100 | Systempakete fehlen | `which iptables ip6tables ipset systemctl` prüfen | | API nicht erreichbar | Erst AdGuard Home starten | | Alte Shell-Artefakte | Go-Installer meldet Konflikte, alte Version deinstallieren | +| `permission denied` auf Log-, State- oder Cache-Dateien | Der Service hat kein `CAP_DAC_OVERRIDE` mehr (siehe [Rechte des systemd-Service](architektur.md#rechte-des-systemd-service)). Betroffene Dateien müssen root gehören bzw. für root beschreibbar sein, z. B. `sudo chown root: /var/log/adguard-shield.log` | +| `PID-Datei ... ist keine reguläre Datei` | `PID_FILE` zeigt auf einen Symlink, ein Verzeichnis o. Ä.; aus Sicherheitsgründen wird das abgelehnt. Eintrag entfernen oder `PID_FILE` auf eine normale Datei setzen | | Unit manuell geändert | `sudo systemctl daemon-reload` ausführen | | Konfiguration geändert, aber ohne Wirkung | `sudo systemctl restart adguard-shield` ausführen; `systemctl reload` lädt die Konfiguration nicht neu | | Warnungen zur Konfiguration im Log | Tippfehler oder ungültige Werte; betroffene Parameter nutzen den Standardwert (siehe [Konfiguration](konfiguration.md#prüfung-beim-laden)) | diff --git a/docs/update.md b/docs/update.md index eeee1de..14d98ad 100644 --- a/docs/update.md +++ b/docs/update.md @@ -42,8 +42,8 @@ Was automatisch passiert: | Bereich | Änderung | |---|---| | Datenbank | Schema-Migration auf Version 2 (nur zusätzliche Indizes). v1.1.3 kann die Datenbank weiterhin lesen. | -| Konfiguration | Neue Parameter `ADGUARD_TLS_INSECURE`, `NOTIFY_TLS_INSECURE`, `LOG_MAX_SIZE_MB`, `BAN_HISTORY_RETENTION_DAYS` und die `WEB_*`-Parameter der optionalen [Web-Statusseite](webinterface.md) (standardmäßig aus) werden ergänzt (sofern noch nicht vorhanden), vorhandene Werte bleiben unverändert. Die bisherige Datei liegt danach als `adguard-shield.conf.old` vor (eine ältere `.old` wird dabei ersetzt). | -| systemd-Unit | Wird neu geschrieben, `ExecReload` entfällt. | +| Konfiguration | Neue Parameter `ADGUARD_TLS_INSECURE`, `NOTIFY_TLS_INSECURE`, `LOG_MAX_SIZE_MB`, `BAN_HISTORY_RETENTION_DAYS`, `ALLOW_INSECURE_LIST_URLS` und die `WEB_*`-Parameter der optionalen [Web-Statusseite](webinterface.md) (standardmäßig aus) werden ergänzt (sofern noch nicht vorhanden), vorhandene Werte bleiben unverändert. Die bisherige Datei liegt danach als `adguard-shield.conf.old` vor (eine ältere `.old` wird dabei ersetzt). | +| systemd-Unit | Wird neu geschrieben, `ExecReload` entfällt. Die Unit ist deutlich restriktiver: `NoNewPrivileges=true`, Capabilities nur noch `CAP_NET_ADMIN`, `CAP_NET_RAW` und `CAP_NET_BIND_SERVICE` sowie weitere Sandbox-Optionen (siehe [Rechte des systemd-Service](architektur.md#rechte-des-systemd-service)). Bestehende Installationen erhalten die neue Unit nur über `update` bzw. `install`; ein reines Austauschen des Binarys reicht nicht. | | Firewall | Beim Start werden alle Sperren aus der Datenbank per `ipset restore` übernommen. Permanente Sperren, die durch einen Fehler in älteren Versionen nach `BAN_DURATION` aus dem ipset gefallen waren, sind danach wieder aktiv. | Was du prüfen solltest: @@ -52,7 +52,9 @@ Was du prüfen solltest: |---|---| | TLS-Zertifikate | Benachrichtigungen (ntfy, Gotify, Webhooks) und externe Listen prüfen jetzt das TLS-Zertifikat. Nutzt du einen eigenen Server mit selbstsigniertem Zertifikat, schlagen Benachrichtigungen fehl (`Benachrichtigung fehlgeschlagen` im Log). Abhilfe: gültiges Zertifikat oder `NOTIFY_TLS_INSECURE=true`. Für externe Listen gibt es keine Ausnahme – bei Fehlern bleibt die bisherige Whitelist erhalten bzw. der Cache wird genutzt. | | AdGuard-API-Zertifikat | Auch das TLS-Zertifikat der AdGuard-Home-API wird jetzt geprüft (`ADGUARD_TLS_INSECURE=false`, wird beim Update ergänzt). Bis v1.1.x wurde es nie geprüft. Bei selbstsigniertem Zertifikat schlägt die Abfrage fehl (`sudo adguard-shield test` meldet einen Zertifikatsfehler). Abhilfe: gültiges Zertifikat, `ADGUARD_URL` auf `http://127.0.0.1:` umstellen oder bewusst `ADGUARD_TLS_INSECURE=true` setzen. | +| Externe Listen über http:// | Externe White- und Blocklisten werden nur noch per `https://` geladen. `http://`-URLs werden beim Start als `WARNUNG` gemeldet und beim Abruf abgelehnt (die Whitelist bleibt unverändert, Auto-Unban der Blocklist wird ausgesetzt). Weiterleitungen von https auf http werden ebenfalls abgelehnt. Abhilfe: URL auf `https://` umstellen oder bewusst `ALLOW_INSECURE_LIST_URLS=true` setzen. Listen über 50 MB werden verworfen, stattdessen wird der Cache genutzt (siehe [Externe Listen: Transport und Größe](konfiguration.md#externe-listen-transport-und-größe)). | | Konfigurations-Warnungen | Ungültige, unbekannte oder veraltete Parameter werden beim Start und bei jedem CLI-Befehl als `WARNUNG` gemeldet (z. B. `BAN_HISTORY_FILE` aus der alten Shell-Version). Ungültige Werte werden durch den Standardwert ersetzt. Veraltete Zeilen kannst du aus der Konfiguration löschen. | +| Eigene Pfade / Dateirechte | Der Service darf Dateirechte nicht mehr übergehen (kein `CAP_DAC_OVERRIDE`). Liegen `LOG_FILE`, `STATE_DIR`, Cache-Verzeichnisse oder `PID_FILE` an eigenen Orten, müssen sie root gehören bzw. für root beschreibbar sein. `PID_FILE` darf kein Symlink sein. Prüfen nach dem Neustart: `sudo journalctl -u adguard-shield -n 50`. | | Logrotation | `LOG_MAX_SIZE_MB` (Standard 50) wird jetzt ausgewertet: Das Log wird nach `adguard-shield.log.1` rotiert. | | `BLOCKED_PORTS` | Kommas sind jetzt erlaubt, ungültige Ports werden mit Warnung ignoriert. Port-Namen wie `domain` werden nicht mehr akzeptiert – bitte Portnummern verwenden. | | `systemctl reload` | Wird nicht mehr unterstützt; nach Änderungen an der Konfiguration `systemctl restart adguard-shield` verwenden. | diff --git a/internal/config/config.go b/internal/config/config.go index b72c743..af3f130 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -2,8 +2,10 @@ package config import ( "bufio" + "errors" "fmt" "net" + "net/url" "os" "path" "path/filepath" @@ -83,6 +85,8 @@ type Config struct { ExternalBlocklistDuration int64 ExternalBlocklistAutoUnban bool ExternalBlocklistNotify bool + // AllowInsecureListURLs erlaubt http:// für externe Block-/Whitelists. + AllowInsecureListURLs bool ProgressiveBanEnabled bool ProgressiveBanMultiplier int @@ -193,6 +197,9 @@ func load(values map[string]string) (*Config, *reader) { c.ExternalBlocklistDuration = int64(r.intMin("EXTERNAL_BLOCKLIST_BAN_DURATION", 0, 0)) c.ExternalBlocklistAutoUnban = r.bool("EXTERNAL_BLOCKLIST_AUTO_UNBAN", true) c.ExternalBlocklistNotify = r.bool("EXTERNAL_BLOCKLIST_NOTIFY", false) + c.AllowInsecureListURLs = r.bool("ALLOW_INSECURE_LIST_URLS", false) + r.checkListURLs("EXTERNAL_WHITELIST_URLS", c.ExternalWhitelistURLs, c.AllowInsecureListURLs) + r.checkListURLs("EXTERNAL_BLOCKLIST_URLS", c.ExternalBlocklistURLs, c.AllowInsecureListURLs) c.ProgressiveBanEnabled = r.bool("PROGRESSIVE_BAN_ENABLED", true) c.ProgressiveBanMultiplier = r.intMin("PROGRESSIVE_BAN_MULTIPLIER", 2, 1) c.ProgressiveBanMaxLevel = r.intMin("PROGRESSIVE_BAN_MAX_LEVEL", 5, 0) @@ -485,6 +492,54 @@ func (r *reader) listen(k, def string) string { return v } +// ErrInsecureListURL meldet eine http://-Liste ohne ALLOW_INSECURE_LIST_URLS=true. +var ErrInsecureListURL = errors.New("unverschlüsseltes http:// ist nicht erlaubt, da die Liste per Man-in-the-Middle manipuliert werden kann; URL auf https:// umstellen oder bewusst ALLOW_INSECURE_LIST_URLS=true setzen") + +// CheckListURL prüft eine URL für externe Block-/Whitelists: https:// ist +// immer erlaubt, http:// nur mit allowInsecure (dann ist insecure true), andere +// Schemata und URLs ohne Host nie. Fehler enthalten die URL bewusst nicht, da +// Pfad und Query Tokens enthalten können. +func CheckListURL(raw string, allowInsecure bool) (insecure bool, err error) { + u, err := url.Parse(strings.TrimSpace(raw)) + if err != nil { + return false, errors.New("ungültige URL") + } + switch strings.ToLower(u.Scheme) { + case "https": + case "http": + if !allowInsecure { + return false, ErrInsecureListURL + } + insecure = true + case "": + return false, errors.New("URL ohne Schema, erlaubt ist nur https://") + default: + return false, fmt.Errorf("Schema %s:// wird nicht unterstützt, erlaubt ist nur https://", strings.ToLower(u.Scheme)) + } + if u.Hostname() == "" { + return false, errors.New("URL ohne Host") + } + return insecure, nil +} + +// checkListURLs warnt beim Laden vor Listen-URLs, die der Download ablehnen +// wird oder die unverschlüsselt geladen werden. +func (r *reader) checkListURLs(k string, urls []string, allowInsecure bool) { + for i, raw := range urls { + src := fmt.Sprintf("Quelle #%d", i+1) + if u, err := url.Parse(raw); err == nil && u.Scheme != "" && u.Host != "" { + src += fmt.Sprintf(" (%s://%s)", u.Scheme, u.Host) + } + insecure, err := CheckListURL(raw, allowInsecure) + switch { + case err != nil: + r.warnf("%s: %s wird nicht geladen: %v", k, src, err) + case insecure: + r.warnf("%s: %s wird unverschlüsselt per http:// geladen (ALLOW_INSECURE_LIST_URLS=true); Inhalte können unterwegs manipuliert werden, https:// wird dringend empfohlen", k, src) + } + } +} + // basePath normalisiert einen URL-Pfad auf führenden und abschließenden Slash. func (r *reader) basePath(k, def string) string { v, ok := r.lookup(k) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 72ee617..33a67bf 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -184,7 +184,7 @@ func TestKnownKeysContainsNewKeys(t *testing.T) { for _, k := range KnownKeys() { keys[k] = true } - for _, k := range []string{"ADGUARD_TLS_INSECURE", "NOTIFY_TLS_INSECURE", "BAN_HISTORY_RETENTION_DAYS", "LOG_MAX_SIZE_MB", "CHECK_INTERVAL", "GEOIP_CACHE_TTL"} { + for _, k := range []string{"ADGUARD_TLS_INSECURE", "NOTIFY_TLS_INSECURE", "ALLOW_INSECURE_LIST_URLS", "BAN_HISTORY_RETENTION_DAYS", "LOG_MAX_SIZE_MB", "CHECK_INTERVAL", "GEOIP_CACHE_TTL"} { if !keys[k] { t.Fatalf("KnownKeys misses %s", k) } @@ -323,3 +323,52 @@ func TestLoadWebTLSPairWarning(t *testing.T) { t.Errorf("Doppelpunkt im Benutzernamen nicht gewarnt: %v", c.Warnings) } } + +func TestCheckListURL(t *testing.T) { + cases := []struct { + url string + allow bool + wantInsecure bool + wantErr bool + }{ + {"https://example.com/list.txt", false, false, false}, + {"HTTPS://example.com/list.txt", false, false, false}, + {"http://example.com/list.txt", false, false, true}, + {"http://example.com/list.txt", true, true, false}, + {"file:///etc/passwd", true, false, true}, + {"ftp://example.com/list.txt", true, false, true}, + {"https:///list.txt", true, false, true}, + {"http://", true, false, true}, + {"example.com/list.txt", true, false, true}, + {"", true, false, true}, + } + for _, tc := range cases { + insecure, err := CheckListURL(tc.url, tc.allow) + if (err != nil) != tc.wantErr || insecure != tc.wantInsecure { + t.Errorf("CheckListURL(%q, %v) = %v, %v", tc.url, tc.allow, insecure, err) + } + } + if _, err := CheckListURL("http://example.com/x?token=geheim", false); err == nil || !strings.Contains(err.Error(), "ALLOW_INSECURE_LIST_URLS=true") || strings.Contains(err.Error(), "geheim") { + t.Fatalf("Fehler muss die Abhilfe nennen und darf die URL nicht enthalten: %v", err) + } +} + +func TestLoadWarnsAboutListURLs(t *testing.T) { + c := loadString(t, `EXTERNAL_BLOCKLIST_URLS="https://ok.example/a.txt,http://plain.example/b.txt?token=geheim" +EXTERNAL_WHITELIST_URLS="file:///etc/hosts" +`) + if c.AllowInsecureListURLs { + t.Fatal("ALLOW_INSECURE_LIST_URLS must default to false") + } + if !hasWarning(c, "EXTERNAL_BLOCKLIST_URLS: Quelle #2 (http://plain.example) wird nicht geladen") || !hasWarning(c, "EXTERNAL_WHITELIST_URLS: Quelle #1 wird nicht geladen") { + t.Fatalf("missing warnings: %v", c.Warnings) + } + if hasWarning(c, "Quelle #1 (https://ok.example)") || hasWarning(c, "geheim") { + t.Fatalf("unexpected warning: %v", c.Warnings) + } + + c = loadString(t, "ALLOW_INSECURE_LIST_URLS=true\nEXTERNAL_BLOCKLIST_URLS=\"http://plain.example/b.txt\"\n") + if !c.AllowInsecureListURLs || !hasWarning(c, "unverschlüsselt per http://") { + t.Fatalf("insecure URL must be allowed with a warning: %v", c.Warnings) + } +} diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index 6298770..a9ca32e 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -70,6 +70,9 @@ type Daemon struct { queryLogLimitWarned time.Time + // insecureListWarned merkt sich bereits gemeldete http://-Listen. + insecureListWarned sync.Map + banMu sync.Mutex geoMu sync.Mutex diff --git a/internal/daemon/lists.go b/internal/daemon/lists.go index 0309722..579a431 100644 --- a/internal/daemon/lists.go +++ b/internal/daemon/lists.go @@ -14,6 +14,7 @@ import ( "strings" "time" + "adguard-shield/internal/config" "adguard-shield/internal/db" "adguard-shield/internal/firewall" ) @@ -249,6 +250,48 @@ func previewList(items []string, n int) string { return strings.Join(items[:n], ", ") + ", ..." } +// maxListBytes begrenzt die Größe einer heruntergeladenen Liste. +const maxListBytes = 50 << 20 + +// maxListRedirects entspricht der Standardgrenze von net/http. +const maxListRedirects = 10 + +// listClient liefert eine Kopie von d.Client, deren Weiterleitungen dieselben +// Regeln wie die konfigurierte URL erfüllen müssen (kein https→http ohne +// ALLOW_INSECURE_LIST_URLS). d.Client selbst bleibt unverändert, da ihn auch +// andere Module (z.B. AbuseIPDB) nutzen. +func (d *Daemon) listClient() *http.Client { + base := d.Client + if base == nil { + base = http.DefaultClient + } + c := *base + allow := d.Config.AllowInsecureListURLs + prev := base.CheckRedirect + c.CheckRedirect = func(req *http.Request, via []*http.Request) error { + if _, err := config.CheckListURL(req.URL.String(), allow); err != nil { + return fmt.Errorf("Weiterleitung abgelehnt: %w", err) + } + if prev != nil { + return prev(req, via) + } + if len(via) >= maxListRedirects { + return fmt.Errorf("mehr als %d Weiterleitungen", maxListRedirects) + } + return nil + } + return &c +} + +// warnInsecureList warnt einmal pro Prozess und Quelle vor einem +// unverschlüsselten Listen-Download. +func (d *Daemon) warnInsecureList(prefix, src string) { + if _, loaded := d.insecureListWarned.LoadOrStore(prefix+"|"+src, true); loaded { + return + } + d.warn("%s %s wird unverschlüsselt per http:// geladen (ALLOW_INSECURE_LIST_URLS=true); Inhalte können unterwegs manipuliert werden, bitte auf https:// umstellen", prefix, src) +} + // fetchCachedLines lädt eine Liste; fromCache meldet, dass der Download // fehlschlug und stattdessen der Cache genutzt wurde. func (d *Daemon) fetchCachedLines(ctx context.Context, sourceURL, cacheDir, prefix string, index int) (lines []string, fromCache bool, err error) { @@ -257,6 +300,16 @@ func (d *Daemon) fetchCachedLines(ctx context.Context, sourceURL, cacheDir, pref tmpFile := filepath.Join(cacheDir, fmt.Sprintf("%s_%d.tmp", prefix, index)) src := listSource(index, sourceURL) + // Abgelehnte URLs nutzen bewusst keinen Cache: Dessen Inhalt stammt ggf. + // aus einem unverschlüsselten Download. + insecure, err := config.CheckListURL(sourceURL, d.Config.AllowInsecureListURLs) + if err != nil { + return nil, false, err + } + if insecure { + d.warnInsecureList(prefix, src) + } + req, err := http.NewRequestWithContext(ctx, http.MethodGet, sourceURL, nil) if err != nil { return nil, false, listErr(err) @@ -266,7 +319,7 @@ func (d *Daemon) fetchCachedLines(ctx context.Context, sourceURL, cacheDir, pref req.Header.Set("If-None-Match", value) } } - resp, err := d.Client.Do(req) + resp, err := d.listClient().Do(req) if err != nil { err = listErr(err) if b, readErr := os.ReadFile(cacheFile); readErr == nil { @@ -291,10 +344,19 @@ func (d *Daemon) fetchCachedLines(ctx context.Context, sourceURL, cacheDir, pref } return nil, false, fmt.Errorf("HTTP %d", resp.StatusCode) } - b, err := io.ReadAll(io.LimitReader(resp.Body, 50<<20)) + b, err := io.ReadAll(io.LimitReader(resp.Body, maxListBytes+1)) if err != nil { return nil, false, listErr(err) } + if len(b) > maxListBytes { + // Nie abgeschnitten übernehmen: Eine gekürzte Liste würde Einträge + // verlieren (Auto-Unban bzw. Whitelist-Abgleich). + if cached, readErr := os.ReadFile(cacheFile); readErr == nil { + d.warn("%s Liste größer als %d MB, nutze Cache: %s", prefix, maxListBytes>>20, src) + return strings.Split(string(cached), "\n"), true, nil + } + return nil, false, fmt.Errorf("Liste größer als %d MB", maxListBytes>>20) + } if err := os.WriteFile(tmpFile, b, 0644); err != nil { return nil, false, err } diff --git a/internal/daemon/lists_security_test.go b/internal/daemon/lists_security_test.go new file mode 100644 index 0000000..72bbbb4 --- /dev/null +++ b/internal/daemon/lists_security_test.go @@ -0,0 +1,194 @@ +package daemon + +import ( + "bytes" + "context" + "io" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "sync/atomic" + "testing" + + "adguard-shield/internal/config" + "adguard-shield/internal/syslog" +) + +// plainListServer liefert eine Liste unverschlüsselt per http:// und zählt +// die Abrufe. +func plainListServer(t *testing.T, body string, hits *atomic.Int32) *httptest.Server { + t.Helper() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + hits.Add(1) + io.WriteString(w, body) + })) + t.Cleanup(srv.Close) + return srv +} + +func TestListHTTPRejectedByDefault(t *testing.T) { + var hits atomic.Int32 + srv := plainListServer(t, "203.0.113.9\n", &hits) + cacheDir := t.TempDir() + // Ein vorhandener Cache darf für abgelehnte URLs nicht genutzt werden. + if err := os.WriteFile(filepath.Join(cacheDir, "blocklist_0.txt"), []byte("203.0.113.10\n"), 0644); err != nil { + t.Fatal(err) + } + cfg := &config.Config{ExternalBlocklistURLs: []string{srv.URL + "/list.txt"}, ExternalBlocklistCacheDir: cacheDir} + d, _ := newTestDaemon(t, cfg) + var logBuf bytes.Buffer + d.Logger = syslog.New(&logBuf, "DEBUG") + + if err := d.SyncBlocklist(context.Background()); err == nil { + t.Fatal("http:// ohne ALLOW_INSECURE_LIST_URLS muss fehlschlagen") + } + if hits.Load() != 0 { + t.Fatalf("http-Quelle wurde abgerufen: %d", hits.Load()) + } + for _, ip := range []string{"203.0.113.9", "203.0.113.10"} { + if ok, _ := d.Store.BanExists(ip); ok { + t.Fatalf("%s darf nicht gesperrt werden", ip) + } + } + if !strings.Contains(logBuf.String(), "ALLOW_INSECURE_LIST_URLS=true") || !strings.Contains(logBuf.String(), "https://") { + t.Fatalf("Warnung muss die Abhilfe nennen:\n%s", logBuf.String()) + } +} + +func TestListHTTPAllowedWithFlag(t *testing.T) { + var hits atomic.Int32 + srv := plainListServer(t, "203.0.113.9\n", &hits) + cfg := &config.Config{ExternalBlocklistURLs: []string{srv.URL + "/list.txt"}, ExternalBlocklistCacheDir: t.TempDir(), AllowInsecureListURLs: true} + d, _ := newTestDaemon(t, cfg) + var logBuf bytes.Buffer + d.Logger = syslog.New(&logBuf, "DEBUG") + + for i := 0; i < 2; i++ { + if err := d.SyncBlocklist(context.Background()); err != nil { + t.Fatal(err) + } + } + if ok, _ := d.Store.BanExists("203.0.113.9"); !ok { + t.Fatal("Eintrag der http-Liste muss mit ALLOW_INSECURE_LIST_URLS=true gesperrt werden") + } + if n := strings.Count(logBuf.String(), "unverschlüsselt per http://"); n != 1 { + t.Fatalf("erwartet genau eine Warnung zur unverschlüsselten Quelle, gefunden %d:\n%s", n, logBuf.String()) + } +} + +func TestListRejectsUnsupportedSchemes(t *testing.T) { + cfg := &config.Config{ExternalBlocklistCacheDir: t.TempDir(), AllowInsecureListURLs: true} + d, _ := newTestDaemon(t, cfg) + for _, u := range []string{"file:///etc/passwd", "ftp://example.com/list.txt", "https:///list.txt", "http://", "example.com/list.txt"} { + if _, _, err := d.fetchCachedLines(context.Background(), u, cfg.ExternalBlocklistCacheDir, "blocklist", 0); err == nil { + t.Errorf("%q muss abgelehnt werden", u) + } + } +} + +func TestListRedirectDowngradeRejected(t *testing.T) { + var hits atomic.Int32 + plain := plainListServer(t, "203.0.113.9\n", &hits) + redir := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, plain.URL+"/list.txt", http.StatusFound) + })) + t.Cleanup(redir.Close) + + cfg := &config.Config{ExternalBlocklistURLs: []string{redir.URL + "/list.txt"}, ExternalBlocklistCacheDir: t.TempDir()} + d, _ := newTestDaemon(t, cfg) + useListServer(d, redir) + if err := d.SyncBlocklist(context.Background()); err == nil { + t.Fatal("Weiterleitung https→http muss fehlschlagen") + } + if hits.Load() != 0 { + t.Fatalf("http-Ziel der Weiterleitung wurde abgerufen: %d", hits.Load()) + } + if ok, _ := d.Store.BanExists("203.0.113.9"); ok { + t.Fatal("Eintrag hinter einer https→http-Weiterleitung darf nicht gesperrt werden") + } + if d.Client.CheckRedirect != nil { + t.Fatal("der gemeinsame d.Client darf nicht verändert werden") + } + + cfg.AllowInsecureListURLs = true + if err := d.SyncBlocklist(context.Background()); err != nil { + t.Fatalf("mit ALLOW_INSECURE_LIST_URLS=true muss die Weiterleitung erlaubt sein: %v", err) + } + if ok, _ := d.Store.BanExists("203.0.113.9"); !ok { + t.Fatal("Eintrag nach erlaubter Weiterleitung fehlt") + } +} + +// oversizeServer liefert maxListBytes+1 Bytes gültiger Listeneinträge. +func oversizeServer(t *testing.T) *httptest.Server { + t.Helper() + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("ETag", `"oversize"`) + line := []byte("203.0.113.77\n") + chunk := bytes.Repeat(line, 64<<10/len(line)) + remaining := maxListBytes + 1 + for remaining > 0 { + n := min(remaining, len(chunk)) + if _, err := w.Write(chunk[:n]); err != nil { + return + } + remaining -= n + } + })) + t.Cleanup(srv.Close) + return srv +} + +func TestListOversizeFallsBackToCache(t *testing.T) { + srv := oversizeServer(t) + cacheDir := t.TempDir() + cacheFile := filepath.Join(cacheDir, "blocklist_0.txt") + if err := os.WriteFile(cacheFile, []byte("198.51.100.1\n"), 0644); err != nil { + t.Fatal(err) + } + cfg := &config.Config{ExternalBlocklistURLs: []string{srv.URL + "/list.txt"}, ExternalBlocklistCacheDir: cacheDir} + d, _ := newTestDaemon(t, cfg) + useListServer(d, srv) + var logBuf bytes.Buffer + d.Logger = syslog.New(&logBuf, "DEBUG") + + err := d.SyncBlocklist(context.Background()) + if err == nil || !strings.Contains(err.Error(), "1 von 1 Quellen fehlgeschlagen") { + t.Fatalf("zu große Liste muss als fehlgeschlagen gemeldet werden: %v", err) + } + if b, _ := os.ReadFile(cacheFile); string(b) != "198.51.100.1\n" { + t.Fatalf("Cache wurde überschrieben (%d Bytes)", len(b)) + } + if _, err := os.Stat(filepath.Join(cacheDir, "blocklist_0.etag")); !os.IsNotExist(err) { + t.Fatal("ETag einer verworfenen Liste darf nicht gespeichert werden") + } + if ok, _ := d.Store.BanExists("198.51.100.1"); !ok { + t.Fatal("Einträge aus dem Cache müssen weiter genutzt werden") + } + if ok, _ := d.Store.BanExists("203.0.113.77"); ok { + t.Fatal("Einträge der zu großen Liste dürfen nicht übernommen werden") + } + if !strings.Contains(logBuf.String(), "größer als 50 MB, nutze Cache") { + t.Fatalf("Warnung fehlt:\n%s", logBuf.String()) + } +} + +func TestListOversizeWithoutCacheFails(t *testing.T) { + srv := oversizeServer(t) + cacheDir := t.TempDir() + cfg := &config.Config{ExternalBlocklistCacheDir: cacheDir} + d, _ := newTestDaemon(t, cfg) + useListServer(d, srv) + + lines, fromCache, err := d.fetchCachedLines(context.Background(), srv.URL+"/list.txt", cacheDir, "blocklist", 0) + if err == nil || !strings.Contains(err.Error(), "größer als 50 MB") || lines != nil || fromCache { + t.Fatalf("erwartet Fehler ohne Cache: lines=%d fromCache=%v err=%v", len(lines), fromCache, err) + } + for _, name := range []string{"blocklist_0.txt", "blocklist_0.tmp", "blocklist_0.etag"} { + if _, err := os.Stat(filepath.Join(cacheDir, name)); !os.IsNotExist(err) { + t.Fatalf("%s darf nicht geschrieben werden", name) + } + } +} diff --git a/internal/daemon/robustness_test.go b/internal/daemon/robustness_test.go index 10b5106..5f79729 100644 --- a/internal/daemon/robustness_test.go +++ b/internal/daemon/robustness_test.go @@ -326,8 +326,11 @@ func TestBanSkipsWhenRowInsertedByOtherProcess(t *testing.T) { } } +// listServer liefert einen HTTPS-Testserver; externe Listen werden ohne +// ALLOW_INSECURE_LIST_URLS nur per https:// geladen. Der Daemon braucht dafür +// srv.Client() (siehe useListServer). func listServer(t *testing.T, body *atomic.Value) *httptest.Server { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { v, _ := body.Load().(string) if v == "FAIL" { http.Error(w, "kaputt", http.StatusInternalServerError) @@ -339,8 +342,13 @@ func listServer(t *testing.T, body *atomic.Value) *httptest.Server { return srv } +// useListServer lässt d dem Zertifikat des Testservers vertrauen. +func useListServer(d *Daemon, srv *httptest.Server) { + d.Client = srv.Client() +} + func deadURL(t *testing.T) string { - srv := httptest.NewServer(http.NotFoundHandler()) + srv := httptest.NewTLSServer(http.NotFoundHandler()) u := srv.URL + "/list.txt" srv.Close() return u @@ -352,6 +360,7 @@ func TestSyncWhitelistFailureKeepsOldEntries(t *testing.T) { srv := listServer(t, &body) cfg := &config.Config{ExternalWhitelistURLs: []string{srv.URL, deadURL(t)}, ExternalWhitelistCacheDir: t.TempDir()} d, _ := newTestDaemon(t, cfg) + useListServer(d, srv) if err := d.Store.ReplaceWhitelist([]string{"192.0.2.1"}, "external"); err != nil { t.Fatal(err) } @@ -393,6 +402,7 @@ func TestSyncBlocklistFailureSkipsAutoUnban(t *testing.T) { ExternalBlocklistAutoUnban: true, } d, _ := newTestDaemon(t, cfg) + useListServer(d, srv) if err := d.Store.InsertBan(db.Ban{IP: "198.51.100.7", Source: "external-blocklist", Reason: "external-blocklist", Permanent: true}); err != nil { t.Fatal(err) } @@ -428,6 +438,7 @@ func TestSyncBlocklistAddsOnlyNewEntriesInOneBatch(t *testing.T) { ExternalBlocklistDuration: 7200, } d, ex := newTestDaemon(t, cfg) + useListServer(d, srv) if err := d.Store.InsertBan(db.Ban{IP: "203.0.113.2", Source: "monitor", Reason: "rate-limit", BanUntil: time.Now().Unix() + 100, Duration: 100}); err != nil { t.Fatal(err) } @@ -492,6 +503,7 @@ func TestSyncBlocklistNotificationSummary(t *testing.T) { NotifyEnabled: true, NotifyType: "generic", NotifyWebhook: sink.srv.URL, ExternalBlocklistNotify: true, ExternalBlocklistURLs: []string{srv.URL}, ExternalBlocklistCacheDir: t.TempDir(), }) + useListServer(d, srv) if err := d.SyncBlocklist(context.Background()); err != nil { t.Fatal(err) } diff --git a/internal/daemon/safety_test.go b/internal/daemon/safety_test.go index 61cfb93..c9f5af1 100644 --- a/internal/daemon/safety_test.go +++ b/internal/daemon/safety_test.go @@ -60,6 +60,7 @@ func TestSyncWhitelistIgnoresBroadEntries(t *testing.T) { srv := listServer(t, &body) cfg := &config.Config{ExternalWhitelistURLs: []string{srv.URL}, ExternalWhitelistCacheDir: t.TempDir()} d, _ := newTestDaemon(t, cfg) + useListServer(d, srv) for _, ip := range []string{"203.0.113.1", "198.51.100.7"} { if err := d.Store.InsertBan(db.Ban{IP: ip, Source: "monitor", Reason: "rate-limit", Permanent: true}); err != nil { t.Fatal(err) @@ -124,6 +125,7 @@ func TestSyncWhitelistSafetyBrake(t *testing.T) { srv := listServer(t, &body) cfg := &config.Config{ExternalWhitelistURLs: []string{srv.URL}, ExternalWhitelistCacheDir: t.TempDir()} d, _ := newTestDaemon(t, cfg) + useListServer(d, srv) insertBans(t, d.Store, "10.0.0", 150) insertBans(t, d.Store, "203.0.113", 10) if err := d.SyncWhitelist(context.Background()); err != nil { diff --git a/internal/daemon/webhardening_test.go b/internal/daemon/webhardening_test.go index 05d43d7..7da11b7 100644 --- a/internal/daemon/webhardening_test.go +++ b/internal/daemon/webhardening_test.go @@ -22,9 +22,9 @@ func TestListURLsNotExposed(t *testing.T) { var body atomic.Value body.Store("203.0.113.5\n") srv := listServer(t, &body) - host := strings.TrimPrefix(srv.URL, "http://") - blockURL := "http://listuser:pw-geheim-77@" + host + "/block.txt?token=list-token-999" - dead := httptest.NewServer(http.NotFoundHandler()) + host := strings.TrimPrefix(srv.URL, "https://") + blockURL := "https://listuser:pw-geheim-77@" + host + "/block.txt?token=list-token-999" + dead := httptest.NewTLSServer(http.NotFoundHandler()) deadURL := dead.URL + "/x.txt?token=dead-token-555" dead.Close() wlURL := srv.URL + "/allow.txt?key=wl-token-4242" @@ -37,6 +37,7 @@ func TestListURLsNotExposed(t *testing.T) { cfg.ExternalWhitelistURLs = []string{wlURL} cfg.NTFYServerURL = "https://ntfy-user:ntfy-pw-31337@ntfy.example" d, _ := newTestDaemon(t, cfg) + useListServer(d, srv) logFile, err := os.OpenFile(cfg.LogFile, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0600) if err != nil { t.Fatal(err) @@ -61,7 +62,7 @@ func TestListURLsNotExposed(t *testing.T) { t.Fatal(err) } logged := string(raw) - for _, want := range []string{"nutze Cache: Quelle #1 (http://" + host + ")", "Quelle #2 (http://"} { + for _, want := range []string{"nutze Cache: Quelle #1 (https://" + host + ")", "Quelle #2 (https://"} { if !strings.Contains(logged, want) { t.Fatalf("Log enthält %q nicht:\n%s", want, logged) } @@ -169,6 +170,7 @@ func TestSyncBlocklistReportsStaleCache(t *testing.T) { cfg.ExternalBlocklistURLs = []string{srv.URL} cfg.ExternalBlocklistCacheDir = t.TempDir() d, _ := newTestDaemon(t, cfg) + useListServer(d, srv) if err := d.SyncBlocklist(context.Background()); err != nil { t.Fatal(err) } diff --git a/internal/installer/installer.go b/internal/installer/installer.go index e66431a..baba624 100644 --- a/internal/installer/installer.go +++ b/internal/installer/installer.go @@ -589,16 +589,33 @@ Restart=on-failure RestartSec=30 # Sicherheits-Hardening +# Der Daemon läuft als root, aber nur mit den Capabilities, die er wirklich +# nutzt. Log, State-DB, Listen-Caches, GeoIP-Downloads und PID-Datei gehören +# root, daher sind CAP_DAC_OVERRIDE, CAP_DAC_READ_SEARCH, CAP_FOWNER und +# CAP_CHOWN nicht nötig. Der Daemon wechselt keine Benutzer (kein CAP_SETUID/ +# CAP_SETGID) und signalisiert keine fremden Prozesse (kein CAP_KILL). +# E-Mail-Reports laufen per Cron (report-send) außerhalb dieses Service; +# NoNewPrivileges betrifft sendmail/postdrop daher nicht. ProtectSystem=full ReadWritePaths=/var/log /var/lib/adguard-shield /var/run %[1]s/geoip ProtectHome=true -NoNewPrivileges=false PrivateTmp=true +NoNewPrivileges=true +RestrictSUIDSGID=true +LockPersonality=true +RestrictRealtime=true +ProtectClock=true +ProtectControlGroups=true +ProtectKernelLogs=true +SystemCallArchitectures=native +# Bewusst nicht gesetzt: ProtectKernelTunables (iptables/ipset können +# /proc/sys/net beschreiben) und ProtectKernelModules (Netfilter-Module). -# iptables benötigt CAP_NET_ADMIN + CAP_NET_RAW -# Weitere Capabilities für Dateizugriff, Signale und Prozessverwaltung -AmbientCapabilities=CAP_NET_ADMIN CAP_NET_RAW -CapabilityBoundingSet=CAP_NET_ADMIN CAP_NET_RAW CAP_DAC_OVERRIDE CAP_DAC_READ_SEARCH CAP_FOWNER CAP_KILL CAP_SETUID CAP_SETGID CAP_CHOWN +# CAP_NET_ADMIN: iptables, ip6tables, ipset +# CAP_NET_RAW: iptables-legacy nutzt einen Raw-Socket +# CAP_NET_BIND_SERVICE: nur für WEB_LISTEN mit einem Port unter 1024 +AmbientCapabilities=CAP_NET_ADMIN CAP_NET_RAW CAP_NET_BIND_SERVICE +CapabilityBoundingSet=CAP_NET_ADMIN CAP_NET_RAW CAP_NET_BIND_SERVICE # Logging StandardOutput=journal @@ -733,6 +750,10 @@ EXTERNAL_BLOCKLIST_AUTO_UNBAN=true EXTERNAL_BLOCKLIST_NOTIFY=false # Bei großen Listen auf false lassen EXTERNAL_BLOCKLIST_CACHE_DIR="/var/lib/adguard-shield/external-blocklist" +# --- Externe Listen: Transport --- +# Externe White-/Blocklisten werden standardmäßig nur per https:// geladen. +ALLOW_INSECURE_LIST_URLS=false # true = auch http:// erlauben (unsicher, Listen können manipuliert werden) + # --- Progressive Sperren (Recidive) --- # Wiederholungstäter werden stufenweise länger gesperrt PROGRESSIVE_BAN_ENABLED=true diff --git a/internal/installer/installer_test.go b/internal/installer/installer_test.go index bc47183..abff79e 100644 --- a/internal/installer/installer_test.go +++ b/internal/installer/installer_test.go @@ -82,6 +82,29 @@ func TestServiceUnitMatchesRepoService(t *testing.T) { } } +func TestServiceUnitHardening(t *testing.T) { + d := directives(serviceUnit(DefaultInstallDir)) + has := map[string]bool{} + for _, line := range d { + has[line] = true + } + for _, want := range []string{ + "NoNewPrivileges=true", + "CapabilityBoundingSet=CAP_NET_ADMIN CAP_NET_RAW CAP_NET_BIND_SERVICE", + } { + if !has[want] { + t.Errorf("serviceUnit enthaelt %q nicht", want) + } + } + for _, line := range d { + for _, bad := range []string{"CAP_DAC_OVERRIDE", "CAP_DAC_READ_SEARCH", "CAP_SETUID", "CAP_SETGID", "CAP_CHOWN", "CAP_FOWNER", "CAP_KILL", "CAP_SYS_ADMIN"} { + if strings.Contains(line, bad) { + t.Errorf("serviceUnit gewaehrt %s: %q", bad, line) + } + } + } +} + func TestServiceUnitUsesInstallDir(t *testing.T) { unit := serviceUnit("/srv/shield") for _, want := range []string{ diff --git a/internal/report/report.go b/internal/report/report.go index d10f49d..cf48152 100644 --- a/internal/report/report.go +++ b/internal/report/report.go @@ -11,12 +11,14 @@ import ( "mime" "mime/quotedprintable" "net/http" + "net/mail" "os" "os/exec" "path/filepath" "strconv" "strings" "time" + "unicode" "adguard-shield/internal/appinfo" "adguard-shield/internal/config" @@ -131,6 +133,9 @@ func InstallCron(binary, configPath string, c *config.Config) error { if !c.ReportEnabled { return fmt.Errorf("Report ist deaktiviert (REPORT_ENABLED=false)") } + if err := ValidateEmailConfig(c); err != nil { + return err + } minute, hour, err := parseReportTime(c.ReportTime) if err != nil { return err @@ -169,6 +174,10 @@ func sendMail(ctx context.Context, c *config.Config, subject, body, format, xMai if strings.TrimSpace(c.ReportEmailTo) == "" { return fmt.Errorf("REPORT_EMAIL_TO ist leer") } + msg, err := buildMessage(c.ReportEmailFrom, c.ReportEmailTo, subject, body, format, xMailer) + if err != nil { + return err + } parts := strings.Fields(c.ReportMailCmd) if len(parts) == 0 { return fmt.Errorf("REPORT_MAIL_CMD ist leer") @@ -176,10 +185,6 @@ func sendMail(ctx context.Context, c *config.Config, subject, body, format, xMai if _, err := exec.LookPath(parts[0]); err != nil { return fmt.Errorf("Mail-Befehl nicht gefunden: %s", parts[0]) } - msg, err := buildMessage(c.ReportEmailFrom, c.ReportEmailTo, subject, body, format, xMailer) - if err != nil { - return err - } args := append(parts[1:], "-t") cmd := exec.CommandContext(ctx, parts[0], args...) cmd.Stdin = strings.NewReader(msg) @@ -188,10 +193,74 @@ func sendMail(ctx context.Context, c *config.Config, subject, body, format, xMai return cmd.Run() } +// ValidateEmailConfig prüft REPORT_EMAIL_FROM und REPORT_EMAIL_TO, damit eine +// Fehlkonfiguration früh auffällt (z. B. beim Start oder bei report-install). +// Erlaubt sind eine Absenderadresse und eine oder mehrere kommagetrennte +// Empfängeradressen, jeweils auch in der Form "Name ". +func ValidateEmailConfig(c *config.Config) error { + _, _, err := parseAddresses(c.ReportEmailFrom, c.ReportEmailTo) + return err +} + +// parseAddresses validiert Absender und Empfänger und liefert die Header-Werte +// aus den geparsten Adressen. Da sendmail mit -t läuft, würden per CR/LF +// eingeschleuste Header (z. B. Bcc:) sonst zu echten Empfängern. +func parseAddresses(from, to string) (string, string, error) { + if err := checkHeaderValue("REPORT_EMAIL_FROM", from); err != nil { + return "", "", err + } + if err := checkHeaderValue("REPORT_EMAIL_TO", to); err != nil { + return "", "", err + } + if strings.TrimSpace(from) == "" { + return "", "", fmt.Errorf("REPORT_EMAIL_FROM ist leer") + } + if strings.TrimSpace(to) == "" { + return "", "", fmt.Errorf("REPORT_EMAIL_TO ist leer") + } + fromAddr, err := mail.ParseAddress(from) + if err != nil { + return "", "", fmt.Errorf("REPORT_EMAIL_FROM ist keine gueltige E-Mail-Adresse (%q): %v", from, err) + } + toAddrs, err := mail.ParseAddressList(to) + if err != nil { + return "", "", fmt.Errorf("REPORT_EMAIL_TO enthaelt keine gueltige E-Mail-Adressliste (%q): %v", to, err) + } + if len(toAddrs) == 0 { + return "", "", fmt.Errorf("REPORT_EMAIL_TO ist leer") + } + list := make([]string, 0, len(toAddrs)) + for _, a := range toAddrs { + list = append(list, a.String()) + } + return fromAddr.String(), strings.Join(list, ", "), nil +} + +// checkHeaderValue lehnt Steuerzeichen (insbesondere CR/LF) in Header-Werten ab. +func checkHeaderValue(name, value string) error { + for _, r := range value { + if unicode.IsControl(r) { + return fmt.Errorf("%s enthaelt unzulaessige Steuerzeichen (z. B. Zeilenumbruch): %q", name, value) + } + } + return nil +} + // buildMessage baut die Mail für "sendmail -t". Der Body wird quoted-printable // kodiert, da generierte Tabellen sonst Zeilen über 998 Zeichen erzeugen (RFC 5322). // Zeilenenden bleiben LF wie bei den Headern; der MTA wandelt sie in CRLF um. +// Alle Header-Werte werden vorher validiert, damit keine Header eingeschleust werden können. func buildMessage(from, to, subject, body, format, xMailer string) (string, error) { + fromHeader, toHeader, err := parseAddresses(from, to) + if err != nil { + return "", err + } + if err := checkHeaderValue("Betreff", subject); err != nil { + return "", err + } + if err := checkHeaderValue("X-Mailer", xMailer); err != nil { + return "", err + } contentType := "text/plain; charset=UTF-8" if strings.EqualFold(format, "html") { contentType = "text/html; charset=UTF-8" @@ -205,8 +274,8 @@ func buildMessage(from, to, subject, body, format, xMailer string) (string, erro return "", err } return strings.Join([]string{ - "From: " + from, - "To: " + to, + "From: " + fromHeader, + "To: " + toHeader, "Subject: " + mime.QEncoding.Encode("utf-8", subject), "MIME-Version: 1.0", "Content-Type: " + contentType, diff --git a/internal/report/report_test.go b/internal/report/report_test.go index 496bfa6..c00491c 100644 --- a/internal/report/report_test.go +++ b/internal/report/report_test.go @@ -346,3 +346,88 @@ func TestGenerateDoesNotReplacePlaceholdersInValues(t *testing.T) { t.Fatalf("placeholder inside value was replaced\n%s", txt) } } + +func TestBuildMessageRejectsHeaderInjection(t *testing.T) { + const ( + from = "shield@example.test" + to = "admin@example.test" + subject = "AdGuard Shield Test" + mailer = "test" + ) + tests := []struct { + name string + from, to, subject, xMailer string + wantErr string + }{ + {"From mit LF und Bcc", "shield@example.test\nBcc: evil@example.org", to, subject, mailer, "REPORT_EMAIL_FROM"}, + {"From mit CRLF", "shield@example.test\r\nBcc: evil@example.org", to, subject, mailer, "REPORT_EMAIL_FROM"}, + {"From mit CR", "shield@example.test\rTo: evil@example.org", to, subject, mailer, "REPORT_EMAIL_FROM"}, + {"From mit NUL", "shield@example.test\x00", to, subject, mailer, "REPORT_EMAIL_FROM"}, + {"From ungueltig", "kein-at-zeichen", to, subject, mailer, "REPORT_EMAIL_FROM"}, + {"From mehrere Adressen", "a@example.test, b@example.test", to, subject, mailer, "REPORT_EMAIL_FROM"}, + {"From leer", " ", to, subject, mailer, "REPORT_EMAIL_FROM ist leer"}, + {"To mit LF und Bcc", from, "admin@example.test\nBcc: evil@example.org", subject, mailer, "REPORT_EMAIL_TO"}, + {"To mit CRLF und To", from, "admin@example.test\r\nTo: evil@example.org", subject, mailer, "REPORT_EMAIL_TO"}, + {"To mit Tab", from, "admin@example.test\tx", subject, mailer, "REPORT_EMAIL_TO"}, + {"To ungueltig", from, "admin@", subject, mailer, "REPORT_EMAIL_TO"}, + {"To leer", from, "", subject, mailer, "REPORT_EMAIL_TO ist leer"}, + {"Betreff mit LF", from, to, "Report\nBcc: evil@example.org", mailer, "Betreff"}, + {"Betreff mit CRLF", from, to, "Report\r\nBcc: evil@example.org", mailer, "Betreff"}, + {"X-Mailer mit LF", from, to, subject, "test\nBcc: evil@example.org", "X-Mailer"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + msg, err := buildMessage(tt.from, tt.to, tt.subject, "body", "txt", tt.xMailer) + if err == nil { + t.Fatalf("expected error, got message\n%s", msg) + } + if !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("error %q does not mention %q", err, tt.wantErr) + } + }) + } +} + +func TestBuildMessageValidAddresses(t *testing.T) { + tests := []struct { + name, from, to string + wantFrom, wantTo string + }{ + {"einzelner Empfaenger", "shield@example.test", "admin@example.test", "", ""}, + {"mehrere Empfaenger", "shield@example.test", "admin@example.test, ops@example.test,noc@example.test", "", ", , "}, + {"Anzeigename", "AdGuard Shield ", `"Admin, Team" , Ops `, `"AdGuard Shield" `, `"Admin, Team" , "Ops" `}, + {"Anzeigename mit Umlaut", "Schutzschild Größe ", "admin@example.test", "=?utf-8?q?Schutzschild_Gr=C3=B6=C3=9Fe?= ", ""}, + {"Leerzeichen am Rand", " shield@example.test ", " admin@example.test ", "", ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + msg, err := buildMessage(tt.from, tt.to, "AdGuard Shield Wochenbericht – Test", "body", "txt", "test") + if err != nil { + t.Fatal(err) + } + header, _, _ := strings.Cut(msg, "\n\n") + lines := strings.Split(header, "\n") + if lines[0] != "From: "+tt.wantFrom { + t.Fatalf("From header = %q, want %q", lines[0], "From: "+tt.wantFrom) + } + if lines[1] != "To: "+tt.wantTo { + t.Fatalf("To header = %q, want %q", lines[1], "To: "+tt.wantTo) + } + if !strings.HasPrefix(lines[2], "Subject: =?utf-8?q?") { + t.Fatalf("subject not Q-encoded: %q", lines[2]) + } + if len(lines) != 7 { + t.Fatalf("expected 7 header lines, got %d\n%s", len(lines), header) + } + }) + } +} + +func TestValidateEmailConfig(t *testing.T) { + if err := ValidateEmailConfig(&config.Config{ReportEmailFrom: "shield@example.test", ReportEmailTo: "a@example.test, b@example.test"}); err != nil { + t.Fatal(err) + } + if err := ValidateEmailConfig(&config.Config{ReportEmailFrom: "shield@example.test", ReportEmailTo: "a@example.test\nBcc: evil@example.org"}); err == nil { + t.Fatal("expected error for injected Bcc header") + } +}