Nutzer bestaetigte "funktioniert". Gitea-Issue #25 angelegt und geschlossen, Status in bugs.md/found.md auf live bestaetigt aktualisiert. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
408 lines
28 KiB
Markdown
408 lines
28 KiB
Markdown
# bugs.md
|
|
|
|
Ergebnis eines systematischen Tester-/Senior-Dev-Durchgangs (2026-09-17) gegen den echten
|
|
hAP lite Testrouter (192.168.88.1, RouterOS 7.24.4). Hinweis zur Methode: die App-UI selbst
|
|
konnte nicht geklickt werden (keine macOS-UI-Automatisierung verfügbar) — stattdessen
|
|
Code-Audit jedes Feature-Bereichs, Router-Ground-Truth per SSH gegenprüft (`/export terse`,
|
|
Live-Konfiguration), und wo möglich per Skript verifiziert statt geraten (siehe je Eintrag).
|
|
Router-Zustand zum Testzeitpunkt: nahezu Werkszustand, nur die WLAN-Testkonfiguration aus
|
|
einer früheren Session vorhanden.
|
|
|
|
**Status:** `offen` | `postponed` | `fixed`
|
|
|
|
---
|
|
|
|
## 2026-09-17
|
|
|
|
### 1. Race Condition bei Port-Konflikt-Prüfung im LAN-Schritt
|
|
**Status:** fixed (Build grün, noch nicht live gegenreproduziert)
|
|
**Gitea-Issue:** [#13](http://192.168.178.222:3500/kay/RouterOS/issues/13)
|
|
**Confidence:** hoch (Logikfehler direkt im Code nachvollzogen, nicht live reproduziert)
|
|
|
|
`SetupViewModel.checkPortConflict(for:)` (`SetupViewModel.swift:146`) liest `config.interfaceName`
|
|
synchron beim Aufruf und startet dann einen `Task`, der das Ergebnis nach dem `await` unbedingt in
|
|
`lanPortConflicts[configID]` schreibt — ohne Generation-Counter oder Abbruch des vorherigen `Task`.
|
|
|
|
Der Aufruf passiert an zwei Stellen für dieselbe `configID`: `.onAppear` (bei jedem Erscheinen des
|
|
LAN-Schritts) und `.onChange(of: config.interfaceName)` (bei jeder Port-Auswahl). Wechselt der
|
|
Nutzer den Port zügig zweimal hintereinander (oder wechselt ihn, bevor die `onAppear`-Prüfung des
|
|
vorherigen Ports fertig ist), können zwei `Task`s parallel laufen. Der zuerst gestartete, aber
|
|
zuletzt fertige Task überschreibt das Ergebnis des neueren mit dem Stand des alten Ports — "last
|
|
response wins" statt "last request wins".
|
|
|
|
**Konkretes Fehlerbild:** Nutzer wählt Port A (hat Konflikt, z.B. Bridge-Mitgliedschaft), App prüft
|
|
noch, Nutzer wechselt schnell zu Port B (frei). Kommt Port As Prüfergebnis später zurück als Port
|
|
Bs, zeigt die App fälschlich eine Konflikt-Warnung für den inzwischen ausgewählten, tatsächlich
|
|
freien Port B — oder, im umgekehrten Fall, verschluckt eine echte Warnung für einen Port, der
|
|
tatsächlich bereits belegt ist, sodass "Weiter" freigeschaltet wird, obwohl der gewählte Port beim
|
|
Anwenden unbemerkt vorhandene Konfiguration überschreibt.
|
|
|
|
**Fix:** Generation-Zähler `portConflictRequestGeneration: [LanDhcpConfig.ID: Int]` ergänzt, bei
|
|
jedem `checkPortConflict(for:)`-Aufruf hochgezählt; ein abgeschlossener `Task` schreibt sein
|
|
Ergebnis nur, wenn seine Generation beim Abschluss noch die aktuellste ist — ein überholter Task
|
|
verwirft sein Ergebnis stillschweigend statt es zu übernehmen.
|
|
|
|
### 2. "Firewall (optional)"-Titel widerspricht sich selbst im Einfach-Modus
|
|
**Status:** fixed (Build grün, noch nicht live gegenreproduziert)
|
|
**Gitea-Issue:** [#14](http://192.168.178.222:3500/kay/RouterOS/issues/14)
|
|
**Confidence:** hoch (direkt im Code sichtbar)
|
|
|
|
`FirewallStepView.swift:63`: `.navigationTitle(... "Firewall (optional)" ...)` ist fest, unabhängig
|
|
vom Modus. Im Einfach-Modus zeigt derselbe Screen aber den Text "Firewall-Grundschutz ist im
|
|
einfachen Modus immer aktiv." (Zeile 20) — der Titel behauptet "optional", der Inhalt sagt "immer
|
|
an". Für Experte-Modus stimmt der Titel (dort gibt es den Toggle). Kleiner, aber sofort sichtbarer
|
|
Text-Widerspruch für jeden, der im Einfach-Modus durch den Wizard geht.
|
|
|
|
**Fix:** Titel modusabhängig gemacht — `viewModel.mode == .expert ? "Firewall (optional)" : "Firewall"`, neuer Key `"Firewall"` in `L10n.swift` ergänzt.
|
|
|
|
### 3. Fehlende Englisch-Übersetzungen (sichtbar im EN-UI)
|
|
**Status:** fixed (skriptgeprüft: beide Keys jetzt in `L10n.swift`, verbleibende zwei fehlende
|
|
**Gitea-Issue:** [#15](http://192.168.178.222:3500/kay/RouterOS/issues/15)
|
|
Keys "OK"/"Revision" bewusst nicht ergänzt — identisch in beiden Sprachen, kein sichtbarer Effekt)
|
|
**Confidence:** hoch (skriptgeprüft: alle `L10n.t(...)`-Aufrufstellen gegen `L10n.swift`s
|
|
Übersetzungs-Dictionary abgeglichen — `L10n.t` fällt bei fehlendem Key auf den deutschen
|
|
Originaltext zurück, siehe `L10n.swift:12-15`)
|
|
|
|
Zwei Stellen zeigen im Sprachmodus Englisch weiterhin deutschen Text:
|
|
- `Features/Devices/DevicesView.swift:335` — `"Netzwerk-Test fehlgeschlagen"` (Fehlermeldungstitel)
|
|
- `Features/Overview/OverviewView.swift:350` — `"Fokus-Ansicht schließen"` (Tooltip auf dem
|
|
Schließen-Button der Fokus-Ansicht)
|
|
|
|
(Ein dritter fehlender Key, `"Revision"`, ist praktisch nicht sichtbar, da das Wort in beiden
|
|
Sprachen identisch ist — nicht extra gelistet.)
|
|
|
|
**Fix-Ansatz:** Beide Strings in `L10n.swift`s `translations`-Dictionary ergänzen (`"Netzwerk-Test
|
|
fehlgeschlagen": "Network test failed"`, `"Fokus-Ansicht schließen": "Close focus view"`).
|
|
|
|
### 4. Health-Check-Heartbeat ignoriert laufende Wizard-/Experte-Schreibvorgänge
|
|
**Status:** fixed (Build grün, noch nicht live gegenreproduziert)
|
|
**Gitea-Issue:** [#16](http://192.168.178.222:3500/kay/RouterOS/issues/16)
|
|
|
|
Fix: `ConnectionService.beginWrite()`/`endWrite()` (Zähler `activeWriteCount`) ergänzt,
|
|
`checkConnectionHealthAndReconnectIfNeeded()` prüft jetzt zusätzlich `activeWriteCount == 0`.
|
|
`SetupViewModel.apply()` und `ExpertViewModel.saveEditingItem()`/`confirmRemoval()` klammern ihren
|
|
Task-Body jetzt mit `beginWrite()`/`defer { endWrite() }`.
|
|
**Confidence:** mittel (Logiklücke im Code nachvollzogen; nicht live reproduziert — REST läuft über
|
|
unabhängige HTTP-Requests und dürfte robust sein, SSH-Exec-Kanäle sind laut SSH-Protokoll
|
|
grundsätzlich nebenläufig nutzbar, siehe `SSHTransport.swift`/`RestTransport.swift` — insofern kein
|
|
Daten-Korruptionsrisiko, aber eine echte Lücke in der Ablaufsteuerung)
|
|
|
|
`ConnectionService.checkConnectionHealthAndReconnectIfNeeded()` (`ConnectionService.swift:273`)
|
|
läuft alle 10 Sekunden (`healthCheckInterval`) und prüft nur `!isReconnecting` — nicht, ob gerade
|
|
ein `SetupViewModel.apply()` oder `ExpertViewModel.saveEditingItem()` in Arbeit ist. Der hAP-lite-
|
|
Testrouter ist sehr schwach (MIPS 24Kc, 1 Kern, 650MHz, im Test bereits 54% CPU-Last im Leerlauf
|
|
gemessen). Ein mehrere Sekunden dauernder Apply (z.B. Bonding, mehrere Firewall-Regeln) kann
|
|
zeitlich mit dem Heartbeat kollidieren; schlägt der Heartbeat unter Last mit Timeout fehl, während
|
|
der eigentliche Apply eigentlich noch normal durchläuft, zeigt die App fälschlich den
|
|
"Wiederverbinden…"-Banner an, obwohl gar keine echte Verbindungsunterbrechung vorliegt — und
|
|
`reconnectLoop` könnte im ungünstigsten Fall sogar `finishConnecting` mit einer neuen
|
|
Transport-Instanz auslösen, während der ursprüngliche Apply noch auf der alten weiterläuft.
|
|
|
|
**Fix-Ansatz:** Einfachster Schutz: `SetupViewModel.isApplying` / `ExpertViewModel.isApplying` (oder
|
|
ein neuer gemeinsamer "isWriting"-Zähler auf `ConnectionService`) zusätzlich zur Guard-Bedingung in
|
|
`checkConnectionHealthAndReconnectIfNeeded()` prüfen, damit der Heartbeat während eines aktiven
|
|
Schreibvorgangs aussetzt.
|
|
|
|
---
|
|
|
|
## 2026-09-17 (Nachtrag: "erneuter Gegencheck mit echtem Deep Dive")
|
|
|
|
Zweite, tiefere Testrunde auf expliziten Nutzerwunsch. Diesmal inkl. echter Exploit-Verifikation
|
|
gegen den Router (nicht nur Code-Lesen) — beide folgenden Funde live nachgewiesen und live
|
|
gegenverifiziert, dass der Fix greift, mit anschließendem Aufräumen der Testartefakte.
|
|
|
|
### 5. RouterOS-CLI-Injection über beliebige Textfelder (SSH-Transportpfad)
|
|
**Status:** fixed (live exploitiert UND live als behoben bestätigt, siehe unten)
|
|
**Gitea-Issue:** [#17](http://192.168.178.222:3500/kay/RouterOS/issues/17)
|
|
**Confidence:** sehr hoch — kein Verdacht, sondern reproduzierter Exploit gegen den echten Router
|
|
|
|
`RouterOSCommand.cliLine` (`RouterOSCommand.swift`) baute CLI-Zeilen für den SSH-Transport per
|
|
String-Interpolation. Die alte `quoteIfNeeded(_:)` quotete einen Wert nur, wenn er ein Leerzeichen
|
|
enthielt, und escapte darin enthaltene `"`-Zeichen nie. RouterOS' Konsole behandelt `;` als
|
|
Befehlstrenner (dieselbe Gefahr, die `SSHTransport.runDiagnosticCommand`s Doku-Kommentar für
|
|
eigene, aufruferseitig sanitisierte Eingaben bereits benennt) — ein Wert wie
|
|
`test" ; :log warning "INJECTED` (ein völlig plausibler freier Kommentar/SSID/Hostname) schloss
|
|
das Anführungszeichen vorzeitig und ließ den Rest als zweiten, unabhängigen RouterOS-Befehl laufen.
|
|
|
|
**Exploit live bestätigt** (Testrouter, aufgeräumt danach): Befehl
|
|
`/ip firewall address-list add list=injection-test address=10.10.10.10 comment="test" ; :log warning "INJECTED-VIA-COMMENT-FIELD"`
|
|
über SSH ausgeführt → Log zeigt `script,warning INJECTED-VIA-COMMENT-FIELD` als eigenständig
|
|
ausgeführten zweiten Befehl, ausgelöst rein durch ein Kommentarfeld.
|
|
|
|
Betroffen: jeder Text, der über den SSH-Transport in ein `RouterOSCommand` läuft — praktisch jedes
|
|
Feld im Setup-Wizard (SSID, Kommentare, DNS-Server, …) und jedes Freitextfeld im Experte-Tab
|
|
("Weitere Parameter"). Der REST-Pfad ist NICHT betroffen (JSON-Encoding, kein String-Interpolieren
|
|
— siehe `RestTransport.apply`). Auf dem aktuellen hAP-lite-Testrouter ist `www-ssl` deaktiviert
|
|
(`/ip service print` bestätigt: Zeile 9, `X` = disabled) — REST ist also gar nicht erreichbar, jede
|
|
Schreiboperation läuft aktuell über SSH. Der Bug war damit live aktiv, nicht nur theoretisch.
|
|
|
|
**Fix:** `quoteIfNeeded` quotet jetzt immer und escaped `\` → `\\`, `"` → `\"` innerhalb der
|
|
Anführungszeichen — live verifiziert, dass das RouterOS' eigene Escape-Syntax ist (`\"`/`\\`
|
|
rundeten korrekt durch `print detail`). Unconditionelles Quoting live als unproblematisch bestätigt
|
|
(auch für einwortige Werte und `yes`/`no`-Booleans anstandslos akzeptiert). Gleiche Lücke defensiv
|
|
auch in `SSHTransport.fetchFieldValues` geschlossen (aktuell nur mit hartkodiertem `"no"`
|
|
aufgerufen, aber generische `RouterOSTransport`-Methode). Exploit-PoC danach erneut gegen den
|
|
Router gefahren — kein injizierter Log-Eintrag mehr, Kommentar korrekt als reiner Text gespeichert.
|
|
Neuer Regressionstest `testCliLineEscapesEmbeddedQuotesPreventingCommandInjection`. Build + alle
|
|
101 Unit-Tests grün.
|
|
|
|
### 6. Genereller RouterOS-Antwort-Parser trunkiert mehrwortige Werte
|
|
**Status:** fixed (Ursache live nachgewiesen, Fix per Unit-Test abgesichert)
|
|
**Gitea-Issue:** [#18](http://192.168.178.222:3500/kay/RouterOS/issues/18)
|
|
**Confidence:** sehr hoch — live gegen zwei unabhängige Menüs nachgewiesen
|
|
|
|
`RouterOSCliParser.keyValues(from:)` nahm an, `print terse` quote mehrwortige Werte in
|
|
Anführungszeichen (Regex `("[^"]*"|\S+)`). Live-Test (zwei unabhängige Menüs, `/ip firewall
|
|
address-list` und `/interface ethernet`, RouterOS 7.24.4) zeigt: **RouterOS quotet dort gar
|
|
nichts** — ein Kommentar `"multi word test value"` kommt als literales, unquotiertes
|
|
`comment=multi word test value name=ether2 ...` zurück. Der alte Regex-Parser matchte dafür nur
|
|
`\S+` und schnitt den Wert beim ersten Leerzeichen ab — "word test value" ging komplett verloren,
|
|
ohne Fehler, ohne Warnung.
|
|
|
|
Betrifft praktisch jedes mehrwortige Freitextfeld, das über den generischen Parser gelesen wird —
|
|
vor allem das Experte-Tab-"Weitere Parameter (frei)"-Grid, das laut eigenem Doku-Kommentar "fast
|
|
alle" Menüs generisch parst. Der Bug war bereits indirekt im eigenen Testcode sichtbar: der
|
|
existierende Live-Dump-Test `testParseInterfacesMatchesLiveHexOutput` enthält exakt dieses Muster
|
|
(`last-link-up-time=2026-09-15 20:19:19`, ein echter Gerätedump) — nur fiel es nie auf, weil dieses
|
|
Feld von keinem kuratierten Schema gelesen wird.
|
|
|
|
**Fix:** `keyValues` von Regex-Matching auf Token-Scanning umgestellt — ein Leerzeichen-getrenntes
|
|
Wort startet nur dann ein neues Feld, wenn es selbst wie `key=...` aussieht; alles andere wird an
|
|
den Wert des zuletzt begonnenen Feldes angehängt. Bekannter Restfall (inhärent durch RouterOS'
|
|
mehrdeutiges unquotiertes Format, nicht clientseitig lösbar): ein Wert, der selbst ein
|
|
`wort=`-Muster enthält (z.B. ein Kommentar "config=broken"), wird fälschlich als neues Feld
|
|
gelesen — deutlich seltener als die vorherige 100%-Fehlerquote bei jedem mehrwortigen Wert. Neuer
|
|
Regressionstest `testParseGenericItemsPreservesUnquotedMultiWordValue`, abgeleitet vom echten
|
|
Live-Dump. Build + alle 101 Unit-Tests grün.
|
|
|
|
### 7. Isolation wirkte nicht rückwirkend auf bereits bestehende Verbindungen
|
|
**Status:** fixed (best-effort, Mechanismus teilweise live verifiziert — Details unten)
|
|
**Gitea-Issue:** [#19](http://192.168.178.222:3500/kay/RouterOS/issues/19)
|
|
**Confidence:** mittel — siehe ehrliche Verifikationslage unten
|
|
|
|
`FirewallConfig.buildCommands()` (`FirewallConfig.swift`) setzt die Regel "forward
|
|
established,related → accept" auf Position 4, die Isolations-Drop-Regeln erst ab Position
|
|
`filterCommands.count` (7+). Für eine neue Verbindung zwischen zwei isolierten Netzen ist das
|
|
korrekt (erstes Paket hat `connection-state=new`, trifft also nicht Regel 4, sondern die
|
|
Isolations-Regel weiter hinten). Für eine zum Zeitpunkt des Anwendens bereits **bestehende**
|
|
(im Conntrack getrackte) Verbindung zwischen zwei gerade erst als isoliert markierten Netzen
|
|
griff dagegen weiterhin Regel 4 zuerst — sie blieb offen, bis sie von selbst endete. Das ist
|
|
Standardverhalten jeder stateful/conntrack-basierten Firewall (RouterOS, iptables, pf, …), kein
|
|
App-spezifischer Fehler.
|
|
|
|
**Fix:** neue `SSHTransport.flushConnections(between:and:)` + `ConnectionService.flushConnections`
|
|
(dedizierte SSH-Verbindung, kein REST-Äquivalent für RouterOS' CIDR-Mitgliedschafts-Abfragesprache
|
|
vorhanden) — entfernt per `/ip firewall connection remove [find where (src-address in A) and
|
|
(dst-address in B)]` (beide Richtungen) bereits getrackte Verbindungen zwischen zwei gerade
|
|
isolierten Netzen, aufgerufen von `SetupViewModel.apply()` direkt nach den Firewall-Befehlen
|
|
selbst (damit eine sofort neu aufgebaute Verbindung schon auf die neue Drop-Regel trifft).
|
|
`FirewallConfig.NetworkSegment` um `networkAddress` (CIDR) erweitert, Paar-Logik in eine
|
|
wiederverwendbare `isolatedNetworkPairs`-Property extrahiert. `networkA`/`networkB` werden vor
|
|
der Interpolation ins Skript als reine CIDR-Notation validiert (dieselbe Vorsicht wie bei Fund
|
|
#5 — sonst neue Injection-Fläche).
|
|
|
|
**Ehrliche Verifikationslage:** `print count-only where src-address in <cidr>` ist live bestätigt,
|
|
korrekt nach Subnetz zu filtern (40 Treffer bei besetztem /24 vs. 0 bei leerem). Ein einfaches
|
|
`remove [find where dst-address=<exakte-IP>]` (Gleichheit, kein `in`) ist live bestätigt, einen
|
|
Eintrag wirklich zu löschen (Test-ICMP-Verbindung verschwand). Die kombinierte Form `remove [find
|
|
where (a in X) and (b in Y)]`, die der Fix tatsächlich nutzt, ließ sich NICHT sauber live
|
|
beweisen: ein erster Testlauf sah erfolgreich aus, erwies sich aber als Messfehler (ICMP-Conntrack
|
|
verfällt von selbst in wenigen Sekunden — der Mehrfach-SSH-Testablauf mit Verzögerung dazwischen
|
|
täuschte ein Löschen nur vor). Ein sauberer Nachtest an einer echten, weiterhin aktiven
|
|
TCP-Verbindung zeigte den Eintrag sofort wieder auftauchen — erwartbares Verhalten für Connection-
|
|
Tracking allgemein (kein RST, nächstes Paket einer aktiven Verbindung wird einfach neu getrackt),
|
|
kein Beweis, dass `remove` wirkungslos ist, aber eben auch kein Beweis, dass die neue
|
|
Isolations-Regel das neu getrackte Paket tatsächlich abfängt. Dafür bräuchte es zwei echte, getrennte
|
|
Testnetze mit echten Endgeräten, die hier nicht verfügbar sind. Die `remove [find where ... in
|
|
...]`-Technik selbst ist real und community-dokumentiert (MikroTik-Forum), nur nicht
|
|
Ende-zu-Ende gegen dieses konkrete App-Szenario bewiesen. Best-effort per Design (Fehler bricht
|
|
den Apply-Vorgang nicht ab) — ein echter Mehrnetz-Test ist der natürliche nächste Schritt.
|
|
|
|
Build + alle 103 Unit-Tests grün (1 neuer Regressionstest für die Paar-Logik).
|
|
|
|
Nicht automatisch gefixt: ein Fix würde bedeuten, beim Aktivieren der Isolation gezielt
|
|
`/ip firewall connection remove` für die betroffenen Netzpaare auszulösen — ein zusätzlicher,
|
|
potenziell überraschender Seiteneffekt (kappt aktive Verbindungen, die der Nutzer evtl. bewusst
|
|
offen hat), der über die reine Bugfix-Aufgabe dieses Durchgangs hinausgeht und eine bewusste
|
|
Produktentscheidung ist, keine reine Korrektur. Nur als Beobachtung vermerkt.
|
|
|
|
---
|
|
|
|
## 2026-09-17 (Nachtrag 2: "finale Test für maximale Sicherheit")
|
|
|
|
Dritter Durchgang, gezielt sicherheitsfokussiert: Zugangsdaten-Speicherung (Keychain, SavedRoutersStore),
|
|
TOFU-Implementierung (Zertifikat/SSH-Hostkey), alle verbleibenden Stellen, die Text in Router-Befehle
|
|
oder URLs interpolieren. Positiv: `KeychainService` speichert Passwörter korrekt im macOS-Keychain
|
|
(nie Klartext/UserDefaults), `SavedRoutersStore`/`SavedRouter` enthalten kein Passwort-Feld,
|
|
`BackupService.escapeForRouterOSScript` escapte bereits vor diesem Durchgang korrekt (unabhängig von
|
|
Fund #5 — offenbar aus einem früheren Lockout-Vorfall gelernt), `SSHHostKeyFingerprint` hat keine
|
|
Fallback-Schwäche. Zwei weitere Lücken gefunden und gehärtet (defensiv, nicht live exploitiert):
|
|
|
|
### 8. REST-Pendant zu Fund #5: ungeschützte Query-String-Interpolation
|
|
**Status:** fixed (Build grün, nicht live exploitiert — aktuell nur mit hartkodiertem Wert aufgerufen)
|
|
**Gitea-Issue:** [#20](http://192.168.178.222:3500/kay/RouterOS/issues/20)
|
|
**Confidence:** hoch (dieselbe Bugklasse wie #5, nur im REST- statt SSH-Pfad)
|
|
|
|
`RestTransport.fetchFieldValues` baute den Query-String `"\(restPath)?\(whereField)=\(whereValue)"`
|
|
ohne jede Kodierung — beim SSH-Gegenstück wurde das in Fund #5 bereits gefixt, das REST-Pendant
|
|
dabei übersehen. Ein `whereValue` mit `&` hätte einen zusätzlichen Query-Parameter einschleusen
|
|
können. Aktuell nur mit hartkodiertem `"no"` aufgerufen (kein Live-Exploit möglich), aber generische
|
|
`RouterOSTransport`-Methode — Härtung für zukünftige Aufrufer mit echtem Nutzer-/Gerätetext.
|
|
|
|
**Fix:** `whereValue` wird jetzt RFC-3986-"unreserved characters"-only percent-encoded (strenger
|
|
als `.urlQueryAllowed`, das `&`/`=`/`+`/`#` weiterhin durchlässt).
|
|
|
|
### 9. TOFU-Zertifikatsprüfung hatte einen Fallback-Konstante-Blindfleck
|
|
**Status:** fixed (Build grün, Härtung — kein realistisch auslösbarer Live-Exploit gefunden)
|
|
**Gitea-Issue:** [#21](http://192.168.178.222:3500/kay/RouterOS/issues/21)
|
|
**Confidence:** mittel (theoretische Lücke im Code nachvollzogen, keine funktionierende PoC gebaut,
|
|
da eine echte TLS-Handshake-Situation gebraucht würde, in der `SecTrustCopyCertificateChain` trotz
|
|
abgeschlossenem Handshake leer zurückkommt — unüblich, aber laut Apple-API-Vertrag nicht ausgeschlossen)
|
|
|
|
`CertificateFingerprint.sha256(of:)` gab bei fehlgeschlagener Zertifikatsketten-Extraktion den festen
|
|
String `"unbekannt"` statt eines echten Fingerabdrucks zurück. Da `CertificateTrustStore.isTrusted`
|
|
Fingerabdrücke per exaktem String-Vergleich prüft, hätte das Vertrauen in EIN Zertifikat, dessen
|
|
Extraktion fehlschlägt, automatisch JEDES andere ebenfalls extraktions-fehlschlagende Zertifikat für
|
|
denselben Host als vertrauenswürdig durchgehen lassen — ein theoretisches TOFU-Pinning-Bypass-Fenster.
|
|
|
|
**Fix:** Rückgabetyp auf `String?` geändert; schlägt die Extraktion fehl, lehnt
|
|
`RestTransport.urlSession(_:didReceive:completionHandler:)` die Challenge jetzt hart ab
|
|
(`.cancelAuthenticationChallenge`, kein `lastRejectedFingerprint` gesetzt) — der Nutzer bekommt in
|
|
diesem Fall nie einen "Zertifikat vertrauen?"-Dialog mit einer nicht verifizierbaren Kennung
|
|
angeboten, sondern einen normalen Verbindungsfehler.
|
|
|
|
Build + alle 101 Unit-Tests grün nach beiden Fixes.
|
|
|
|
---
|
|
|
|
## Noch nicht geprüft / außerhalb dieses Durchgangs
|
|
|
|
- UI-Interaktion selbst (Klickpfade, Darstellung) — nicht automatisierbar, siehe Hinweis oben.
|
|
- PPPoE-Client — laut Nutzer am Testrouter nicht testbar (hängt hinter einem weiteren
|
|
konfigurierten Router, kein direkter ISP-Uplink). Kein neuer Fund, nur zur Vollständigkeit.
|
|
- Restliche Tabs/Bereiche (Übersicht-Diagramm-Interaktion, Sicherungen, Mode-Taste, Settings) wurden
|
|
im Code überflogen, ohne konkreten neuen Fund über die bereits in `found.md`/`HANDOFF.md`
|
|
dokumentierten Punkte hinaus.
|
|
|
|
---
|
|
|
|
## 2026-09-17 (Nachtrag 3: README-Milestone-Nachcheck)
|
|
|
|
Nutzer wies auf den noch offenen Milestone "LAN-Port-Konflikt-Prüfung + 'Fertig'-Button" in
|
|
README.md hin. Beim erneuten Code-Review dieses Bereichs (u.a. wegen des in Runde 1 bereits
|
|
gefixten Race-Bugs #1 in genau dieser Feature) drei weitere, eigenständige Funde derselben
|
|
Bugklasse ("Alert-Dismiss tut nichts") entdeckt und gefixt.
|
|
|
|
### 10. Drei nicht funktionierende Alert-"Abbrechen"/"OK"-Buttons
|
|
**Status:** fixed (Build + 102 Unit-Tests grün, 1 neuer Regressionstest)
|
|
**Gitea-Issue:** [#22](http://192.168.178.222:3500/kay/RouterOS/issues/22)
|
|
**Confidence:** hoch (Logikfehler direkt im Code nachvollzogen, nicht live reproduziert)
|
|
|
|
1. `ReviewApplyView`s "Anwenden fehlgeschlagen"-Alert: OK-Button-Action leer, `isPresented`-
|
|
Bindings-Setter ebenfalls No-Op — `applyError` wurde nie zurückgesetzt.
|
|
2. `ConnectView`s "Unbekanntes Zertifikat"-Alert: "Abbrechen"-Button-Action komplett leer —
|
|
`connectionService.state` blieb für immer auf `.needsCertificateConfirmation` hängen, kein Weg
|
|
zurück außer dem Zertifikat zu vertrauen.
|
|
3. `ConnectView`s "Unbekannter SSH-Schlüssel"-Alert: "Abbrechen" rief nur
|
|
`dismissPendingSSHTrust()` (löschte nur `pendingSSHTrustFingerprint`) — funktionierte nur für
|
|
einen von zwei möglichen Auslöse-Pfaden, beim anderen (`state == .needsSSHHostKeyConfirmation`)
|
|
blieb der Dialog hängen.
|
|
|
|
**Fix:** neue `ConnectionService.cancelPendingTrustConfirmation()` (setzt `state` bei beiden
|
|
"needs...Confirmation"-Fällen auf `.idle` zurück, löscht zusätzlich `pendingSSHTrustFingerprint`),
|
|
neue `SetupViewModel.dismissApplyError()`. Alle drei Alerts verdrahtet (Button-Action UND
|
|
Bindings-Setter, deckt auch Tap-Outside/Esc-Dismiss ab). Totes `dismissPendingSSHTrust()`
|
|
entfernt.
|
|
|
|
**Weiterhin offen:** die Port-Konflikt-Prüfung selbst (Warndialoge, "Weiter"-Sperre, "Fertig"-
|
|
Button) ist nach diesem zweiten Code-Review-Durchgang funktional plausibel, aber noch nie
|
|
tatsächlich in der App-UI durchgeklickt worden — dafür bräuchte es einen Live-Test durch den
|
|
Nutzer, da UI-Automatisierung hier nicht verfügbar ist.
|
|
|
|
---
|
|
|
|
## 2026-09-17 (Nachtrag 4: Nutzer-gemeldet, live in der App)
|
|
|
|
### 11. Hänger beim Anlegen eines DHCP-Pools
|
|
**Status:** fixed (Build + 106 Unit-Tests grün, 3 neue Regressionstests — Mechanismus isoliert
|
|
verifiziert, Root Cause vom Nutzer bestätigt, nicht erneut live im UI nachgestellt)
|
|
**Gitea-Issue:** [#24](http://192.168.178.222:3500/kay/RouterOS/issues/24)
|
|
**Confidence:** hoch (Nutzer bestätigte exakt das vorhergesagte Bild — Experte-Tab, dauerhaft
|
|
hängend, kein Fehlertext — bevor der Fix geschrieben wurde)
|
|
|
|
Nutzer-Meldung: "Hänger beim Anlegen eines DHCP-Pools" — App bleibt beim Anlegen eines
|
|
`/ip pool`-Eintrags hängen. Rückfrage bestätigte: Experte-Tab, bleibt dauerhaft hängen (kein
|
|
Selbst-Erholen, Neustart nötig) — exakt das Bild, das der Code-Audit-Kandidat vorhergesagt hatte.
|
|
|
|
**Code-Audit-Befund, plausibler Kandidat:** `SSHTransport.connect()` (`SSHTransport.swift`) setzt
|
|
für den `Citadel.SSHClient.connect(...)`-Aufruf **keinerlei Timeout** — im Gegensatz zu
|
|
`RestTransport`, das für jede Anfrage `request.timeoutInterval = 5` explizit setzt. Jeder erste
|
|
Schreibvorgang einer Session (egal ob Experte-Tab oder Einrichten-Assistent) löst zuerst
|
|
`ensureSessionBackup()` aus, was `BackupService.createBackup(for:)` über eine **dedizierte, neue
|
|
SSH-Verbindung** aufruft (`BackupService.swift`, "always over SSH"-Muster). Hängt dieser
|
|
`connect()`-Aufruf (z.B. durch einen kurzen Netzwerk-Aussetzer oder einen unter Last
|
|
langsam/nicht antwortenden Router — der hAP-lite-Testrouter ist mit MIPS 24Kc/650MHz/1 Kern sehr
|
|
schwach, in dieser Session bereits bis 80% CPU-Last beobachtet), gibt es keinen Timeout, der die
|
|
App wieder freigibt — `isApplying`/der Ladeindikator bliebe dann unbegrenzt aktiv, exakt das vom
|
|
Nutzer beschriebene Bild eines "Hängers" ohne Fehlermeldung.
|
|
|
|
Noch nicht bestätigt, ob das tatsächlich die Ursache ist — nur ein durch Code-Lesen gefundener,
|
|
plausibler Kandidat, kein reproduzierter Fehler. Gleiche Lücke beträfe auch `ConnectionService.
|
|
applyViaSSH`/`flushConnections` (beide nutzen ebenfalls `SSHTransport.connect()` ohne Timeout) und
|
|
`UpdateService`/`FactoryResetService`/`NetworkToolsService`, die dieselbe Transport-Klasse nutzen.
|
|
|
|
**Fix:** neuer generischer `SSHTransport.withTimeout(_:operation:)` (`withThrowingTaskGroup`-Race
|
|
zwischen der echten Operation und einem `Task.sleep`-Deadline-Task, verliert die Operation den
|
|
Wettlauf wird sie abgebrochen). Angewendet auf `connect()` (10s) und `run(_:)` (30s, großzügiger
|
|
bemessen, da Exports/Backups auf schwacher Hardware legitim länger brauchen können). Beide
|
|
Methoden bleiben `async throws`, keine Signaturänderung für Aufrufer.
|
|
|
|
**Verifikation:** die Race-Logik selbst ist per 3 neuer Unit-Tests isoliert bewiesen (unabhängig
|
|
von Citadel/echtem Netzwerk) — ein hängender Vorgang wirft nach der Deadline (getestet mit
|
|
200ms statt Produktions-Werten, damit der Test schnell bleibt), ein schneller Vorgang liefert sein
|
|
Ergebnis unverändert, ein eigener Fehler der Operation selbst propagiert unverändert durch. Nicht
|
|
erneut live im Experte-Tab nachgestellt (bräuchte einen erneuten, absichtlich provozierten
|
|
Netzwerk-Hänger) — der Nutzer hat die Ursache aber bereits vor dem Fix exakt bestätigt.
|
|
|
|
Nebenbefund beim Umsetzen: `xcodegen generate` überschreibt `Info.plist` komplett aus
|
|
`project.yml`s `info.properties` (kein Merge mit der Datei auf der Platte) — ein Regenerieren für
|
|
diese neue Testdatei setzte die App-Version dabei stillschweigend von 1.1.0 auf 1.0 zurück.
|
|
`CFBundleShortVersionString`/`CFBundleVersion` jetzt explizit in `project.yml` verankert, damit
|
|
das nicht wieder passiert.
|
|
|
|
---
|
|
|
|
## 2026-09-17 (Nachtrag 5: Nutzer-gemeldet, Feature-Erweiterung)
|
|
|
|
### 12. Experte-Tab: Port-Konflikt-Prüfung wie im Einrichten-Assistenten
|
|
**Status:** fixed (Build + 111 Unit-Tests grün, 6 neue Regressionstests, live vom Nutzer bestätigt:
|
|
"funktioniert")
|
|
**Gitea-Issue:** [#25](http://192.168.178.222:3500/kay/RouterOS/issues/25)
|
|
|
|
Live-Anlass: Nutzer legte über den Experte-Tab manuell ein eigenes Netz auf `ether4` an
|
|
(IP-Adresse, Pool, DHCP-Server). `ether4` blieb dabei unbemerkt Bridge-Mitglied der Haupt-Bridge —
|
|
zwei DHCP-Server im selben Broadcast-Domain, Isolation dadurch grundsätzlich nicht möglich. Kein
|
|
App-Bug im engeren Sinn, aber genau der Fall, den die Wizard-eigene Port-Konflikt-Prüfung
|
|
(found.md #1, bugs.md #1) normalerweise abfängt — beim manuellen Anlegen über den Experte-Tab gab
|
|
es diese Warnung bisher nicht.
|
|
|
|
Auf Nutzerwunsch ("die Abfrage vom Einrichten-Assistenten auf Expert anwenden ... mit allen
|
|
Warnungen") dieselbe Prüfung samt "Port freimachen?"-Dialog jetzt auch im Experte-Tab:
|
|
|
|
- `PortConflictWarningView` aus `LanStepView.swift` nach `Features/Shared/` extrahiert (jetzt von
|
|
Wizard UND Experte-Tab genutzt), neuer `immediateApply`-Parameter für die kontextabhängige
|
|
Abschluss-Meldung (Wizard: erst bei "Jetzt anwenden"; Experte-Tab: sofort bei
|
|
"Anlegen"/"Speichern", da es dort keinen separaten Review-Schritt gibt).
|
|
- Neue `PortConflict.resolutionCommandsIncludingBridgeDetach()` — anders als der Wizard hat der
|
|
Experte-Tab keinen automatischen, unbedingten Bridge-Detach-Schritt
|
|
(`DhcpServerCommandBuilder`), muss die Bridge-Entfernung bei Bestätigung also selbst mit
|
|
ausführen (sonst würde "Port freimachen" für genau den Fall, der diese Erweiterung ausgelöst
|
|
hat, wirkungslos bleiben).
|
|
- `ExpertViewModel` bekommt dieselbe race-sichere Generation-Zähler-Logik wie `SetupViewModel`
|
|
(bugs.md #1), angewendet auf das `.interfacePick`-Feld des jeweils geöffneten Schemas.
|
|
- Speichern-Button gesperrt, bis der Konflikt bestätigt oder ein anderer Port gewählt wurde.
|
|
|
|
**Verifikation:** Build grün, alle 111 Unit-Tests grün (6 neue Regressionstests — Schema-Erkennung
|
|
ohne/mit Interface-Feld, Acknowledge-State-Reset, `resolutionCommandsIncludingBridgeDetach()`
|
|
inkl. Bridge-Entfernung). Live im Experte-Tab durchgeklickt vom Nutzer bestätigt ("funktioniert").
|