Meik/LIAM
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Files
dd0245985119602e3d8a8c5cad78660b7bd77909
LIAM/Sonstiges/LIAM_Finding.md
Meik dd02459851 Document AD group name length finding
2026-03-18 16:11:43 +01:00

23 KiB
Raw Blame History

LIAM NTFS Provider Findings

Umfang der Prüfung

Die Prüfung umfasst den kompletten NTFS-Provider-Code im Projekt LiamNtfs, also nicht nur den aktuellen Provider-Einstieg, sondern auch die darunterliegenden Engine- und Hilfsklassen.

Geprüft wurden insbesondere:

  • LiamNtfs/C4IT.LIAM.Ntfs.cs
  • LiamNtfs/cNtfsBase.cs
  • LiamNtfs/cActiveDirectoryBase.cs
  • LiamNtfs/C4IT_IAM_SET/DataArea_FileSystem.cs
  • LiamNtfs/C4IT_IAM_SET/SecurityGroup.cs
  • LiamNtfs/C4IT_IAM_SET/DataArea.cs
  • LiamNtfs/C4IT_IAM_SET/DataArea_FileSystem_GET.cs
  • LiamNtfs/C4IT_IAM_SET/cNetworkConnection.cs
  • LiamNtfs/C4IT_IAM_SET/Helper.cs
  • LiamNtfs/C4IT_IAM_SET/ResultToken.cs

Nicht im Detail geprüft wurden andere Provider wie Exchange, AD oder Teams in diesem Dokument. Verweise dorthin wurden nur dann berücksichtigt, wenn sie für das Verhalten des NTFS-Providers direkt relevant waren.

Priorisierte Findings

1. Kritisch: Traverse-Fehler können als Erfolg zurückgegeben werden

Der gefährlichste Punkt liegt im Schreibpfad für die Ordneranlage und das Ensure von Berechtigungen.

Im Create-Pfad wird zunächst die AD-Gruppenanlage ausgeführt, danach der Ordner erstellt und danach SetTraversePermissions() aufgerufen. Das Problem ist, dass das Ergebnis der Ordnererstellung anschließend vollständig durch das Ergebnis des Traverse-Schritts überschrieben wird. Wenn SetTraversePermissions() intern in einen Fehlerzustand läuft, aber trotzdem ein ResultToken mit resultErrorId = 0 zurückgibt, sieht der Aufrufer am Ende formal einen Erfolg.

Genau das passiert in SetTraversePermissions() an mehreren Stellen. Dort werden Fehler zwar geloggt, aber häufig nur mit return resultToken; beendet, ohne einen fachlichen Fehlercode zu setzen. Damit ist die Methode in vielen Fällen „logisch fehlgeschlagen“, aber technisch weiterhin erfolgreich.

Die Folge ist gefährlich:

  • Ordner kann erstellt worden sein, aber Traverse-Gruppen fehlen teilweise oder vollständig.
  • AD-Gruppen können erzeugt worden sein, aber Traverse-Nesting ist unvollständig.
  • Aufrufende Schichten wie Workflow, Diagnostics oder API können einen Erfolg anzeigen, obwohl das Ergebnis fachlich inkonsistent ist.

Betroffene Stellen:

2. Hoch: Die Security-Group-Templates werden mutiert und danach wiederverwendet

SecurityGroups.GenerateNewSecurityGroups() verändert die übergebenen Templates direkt. Dabei werden NamingTemplate, DescriptionTemplate und WildcardTemplate in-place mit konkreten Pfaden, Tags und Platzhaltern überschrieben.

Das ist problematisch, weil dieselbe Template-Liste innerhalb derselben Engine-Instanz mehrfach verwendet wird:

  • im Retry-Loop bei Namenskollisionen während der Gruppenerzeugung
  • später erneut beim Traverse-Handling
  • potenziell auch zwischen Ensure- und Create-Pfaden innerhalb derselben Session

Sobald das Template einmal materialisiert wurde, ist es kein Template mehr. Ein zweiter Lauf arbeitet nicht mehr mit der ursprünglichen Konfiguration, sondern mit bereits ersetzten Strings. Dadurch entstehen mehrere Risiken:

  • der Loop-Counter arbeitet nicht mehr auf dem Originalmuster
  • spätere Durchläufe können bereits ersetzte Namen nochmals verarbeiten
  • Wildcards passen nicht mehr sauber zum ursprünglichen Naming-Schema
  • Traverse-Gruppen können auf anderen Namensmustern basieren als Owner/Write/Read

Das ist kein kosmetisches Problem, sondern ein echter Zustandsfehler. Konfiguration wird in Laufzeitstatus verwandelt und danach weiterbenutzt.

Betroffene Stellen:

Status:

  • Am 2026-03-10 umgesetzt.
  • GenerateNewSecurityGroups() arbeitet jetzt auf einer pro Aufruf geklonten Template-Liste statt auf der übergebenen Originalsammlung.
  • Die Konfigurationstemplates im Engine-Kontext bleiben dadurch unverändert und können in Retry-Loops und später im Traverse-Pfad erneut korrekt materialisiert werden.
  • Der Fix adressiert bewusst nur die Zustandsmutation. Die fachliche Gruppenerzeugung und das Naming-Verhalten selbst wurden dabei nicht geändert.

3. Hoch: Drei Minuten harter Blocker im Traverse-Pfad

Im Traverse-Pfad steckt ein explizites Thread.Sleep(180000). Das blockiert den ausführenden Thread für drei Minuten.

Das ist in einer Bibliothek dieser Art ein massiver Risikofaktor:

  • Workflows können in Timeouts laufen
  • GUI-Aktionen wirken eingefroren
  • Web-/Service-Threads werden unnötig belegt
  • parallele Verarbeitung skaliert sehr schlecht

Der Code dokumentiert das zusätzlich falsch mit „60 Sekunden warten“, obwohl tatsächlich 180 Sekunden geschlafen wird. Das erschwert Diagnose und Betrieb zusätzlich.

Wenn dieses Sleep als AD-Replikations-Workaround gedacht war, ist es trotzdem der falsche technische Schnitt. Dann müsste stattdessen gezielt auf die Verfügbarkeit der erzeugten Gruppen geprüft werden, idealerweise mit Polling und Timeout statt blindem Warten.

Betroffene Stelle:

Status:

  • Am 2026-03-18 umgesetzt.
  • Das harte Thread.Sleep(180000) wurde entfernt.
  • Zunächst wurde der Wait auf die bloße Auflösbarkeit neu erzeugter Gruppen umgestellt. Nach fachlicher Rückmeldung wurde der Fix auf den tatsächlich kritischen Folgeschritt verschoben: die Membership-Änderung an der Traverse-Gruppe.
  • Die Traverse-Logik retryt jetzt direkt Members.Contains(...), Members.Add(...) und Save() mit sofortigem Erstversuch und kurzem Backoff.
  • Die maximale Obergrenze bleibt bewusst bei 3 Minuten, damit das bisherige Sicherheitsfenster für langsame AD-Konsistenz erhalten bleibt.
  • Im Normalfall endet die Wartezeit jetzt deutlich früher, sobald die Membership-Änderung erfolgreich durchläuft.

4. Hoch: Unterschiedliches SMB-Verhalten zwischen Lesen und Schreiben

Der Provider verwendet für das Lesen des NTFS-Baums cNtfsBase.LogonAsync(). Dort wird der bekannte SMB-Fehler 1219 abgefangen, die bestehende Verbindung getrennt und anschließend ein neuer Versuch gestartet.

Die Create- und Ensure-Pfade verwenden dagegen cNetworkConnection, und diese Klasse wirft bei jedem Fehler direkt eine Exception. Ein Retry für 1219 findet dort nicht statt.

Das führt zu einem inkonsistenten Betriebsverhalten:

  • Ein und dieselbe Konfiguration kann beim Lesen funktionieren.
  • Dieselbe Konfiguration kann beim Schreiben scheitern, obwohl die Ursache nur eine bestehende Session auf den Share ist.

Das ist besonders tückisch, weil es im Betrieb wie ein „sporadischer“ Fehler aussieht, tatsächlich aber ein systematischer Unterschied zwischen Read- und Write-Pfad ist.

Betroffene Stellen:

5. Hoch: Geerbte ACLs werden im Ensure-Pfad als „vorhanden“ behandelt

Fachlich war bereits geklärt, dass geerbte Rechte hier keine Rolle spielen sollen. Genau das macht der neue Ensure-Pfad aktuell aber nicht konsequent.

Bei der Prüfung, ob eine ACL bereits vorhanden ist, werden Regeln mit GetAccessRules(true, true, ...) geladen. Das zweite true bedeutet, dass auch geerbte Regeln berücksichtigt werden. Damit kann eine Berechtigung, die nur von oben geerbt wurde, dazu führen, dass der Provider glaubt, die explizite ACL sei bereits gesetzt.

Im Ergebnis kann das Ensure-Feature damit stillschweigend zu wenig tun:

  • fehlende explizite ACLs werden nicht gesetzt
  • das Ergebnis sieht erfolgreich aus
  • späteres Mapping oder Downstream-Logik kann trotzdem unerwartetes Verhalten zeigen

Das betrifft nicht nur Traverse, sondern auch die allgemeine ACL-Sicherstellung im neuen Ensure-Pfad.

Betroffene Stellen:

6. Hoch: Per Call übergebene CustomTags erfüllen die Pflichtanforderungen nicht wirklich

Im Provider wird in CreateFilesystemEngine() zwar eine gemergte Tag-Sammlung aufgebaut, in der Provider-Tags und per Aufruf übergebene Tags zusammengeführt werden. Das klingt zunächst korrekt.

Die Pflichtwerte für groupPrefix, groupOwnerTag, groupWriteTag, groupReadTag, groupTraverseTag, groupDLTag und groupGTag werden aber nicht aus dieser gemergten Sammlung gelesen, sondern weiterhin direkt aus Provider.CustomTags.

Die Folge ist eine verdeckte Inkonsistenz:

  • im Debug-/GUI-Output sehen die Tags vorhanden aus
  • im Engine-Aufbau schlagen Pflichtprüfungen trotzdem fehl
  • Call-spezifische Overrides wirken nur teilweise

Das ist besonders ungünstig für Workflow- oder Diagnostics-Szenarien, in denen Tags bewusst pro Aufruf ergänzt oder überschrieben werden sollen.

Betroffene Stellen:

7. Hoch: ReplaceNtfsCustomTags() ist zu früh, zu strikt und verändert Konfiguration dauerhaft

Bereits im Provider-Konstruktor wird ReplaceNtfsCustomTags() ausgeführt. Die Methode greift direkt per CustomTags[...] auf Dictionary-Einträge zu und ersetzt Platzhalter in den Naming-Conventions.

Das erzeugt drei Probleme gleichzeitig:

  1. Fehlende Tags schlagen schon beim Provider-Aufbau fehl, bevor eine gezielte Validierung mit verständlicher Fehlermeldung greifen kann.
  2. Die Naming-Conventions werden direkt verändert. Danach ist nicht mehr klar, was Originalkonfiguration und was bereits materialisierter Laufzeitzustand ist.
  3. Alias-Verhalten wie ADGroupPrefix wird hier noch nicht sauber berücksichtigt, obwohl es später teilweise unterstützt wird.

Damit wird Konfiguration zu früh und zu aggressiv „kompiliert“.

Betroffene Stellen:

8. Hoch: AD-Gruppennamen werden nicht gegen Längenlimits abgesichert

Die Namensbildung für AD-Gruppen materialisiert Templates, Platzhalter und CustomTags direkt zu einem finalen Gruppennamen. Dieser Wert wird anschließend unverändert sowohl als CN als auch als sAMAccountName verwendet.

Aktuell gibt es davor keine echte Längenprüfung und keine Kürzungslogik. Der Code prüft nur, ob der Name bereits existiert, und erhöht bei Bedarf über den LOOP-Mechanismus die Eindeutigkeit. Gegen zu lange Namen schützt das aber nicht.

Dadurch entsteht ein reales Betriebsrisiko:

  • tiefe Ordnerpfade oder lange Ordnernamen können AD-seitig unzulässige Gruppennamen erzeugen
  • der Fehler tritt erst spät beim eigentlichen AD-Create auf
  • das Verhalten ist nicht deterministisch vorbereitet, sondern von der AD-Rückmeldung abhängig
  • auch eine spätere manuelle Korrektur ist unsauber, weil Naming und ACL-Zuordnung bereits auf dem ursprünglichen Namen basieren können

Betroffene Stellen:

Status:

  • Offen, noch nicht umgesetzt.

Vorschlag zum Fixen:

  • Vor dem AD-Write jeden final generierten Gruppennamen gegen eine zentrale, konservative Maximalgrenze prüfen, die für die tatsächlich beschriebenen AD-Attribute sicher eingehalten wird.
  • Wenn ein Name innerhalb der Grenze liegt, unverändert weiterarbeiten.
  • Wenn ein Name zu lang ist, keinen fachlichen Fehler werfen, sondern den Umstand nur im Log dokumentieren und anschließend automatisch auf einen kontrolliert verkürzten Namen umschalten.
  • Die Verkürzung sollte deterministisch sein und die stabilen fachlichen Tags erhalten. Kürzen sollte möglichst nur der dynamische Pfadanteil (NAME bzw. RELATIVEPATH). Wenn das nicht reicht, sollte als Fallback ein stabiler Hash-/Kurzschlüssel im dynamischen Mittelteil verwendet werden, damit der Name reproduzierbar und kollisionsarm bleibt.
  • Der bestehende LOOP-Mechanismus zur Kollisionsbehandlung sollte danach unverändert weiterlaufen, damit Längenbegrenzung und Eindeutigkeitslogik sauber getrennt bleiben.

9. Mittel-Hoch: LoadDataArea() behandelt UNC-Pfade fachlich falsch

Die Einzel-Ladefunktion LoadDataArea() ist sichtbar unfertig. Das ist nicht nur ein Stilproblem, sondern erzeugt reales Fehlverhalten.

Der Code splittet den Pfad mit UID.Split(Path.DirectorySeparatorChar) ohne RemoveEmptyEntries. Bei UNC-Pfaden wie \\server\share entstehen dadurch führende leere Segmente. Das führt dazu, dass ein Share-Pfad nicht sauber als Share erkannt wird, sondern in den Default-Branch läuft.

Zusätzlich wird DisplayName aus Path.GetDirectoryName(UID) ermittelt. Das ist für den Anwendungsfall falsch, weil damit der Elternpfad statt des Blattnamens verwendet wird.

Konkrete Folgen:

  • Share-Roots können beim Einzel-Laden als Folder erscheinen
  • Anzeigenamen stimmen nicht
  • Root-Level und Child-Level verhalten sich inkonsistent

Betroffene Stellen:

10. Mittel-Hoch: Depth = 0 verliert die Root-DataArea

Der Provider baut in getDataAreasAsync() zunächst korrekt das Root-Objekt auf. Danach wird die Unterordnerliste über ntfsBase.RequestFoldersListAsync() geladen. Wenn Depth == 0 ist, liefert RequestFoldersListAsync() aber null statt einer leeren Liste zurück.

Im Provider wird null dann als harter Fehler interpretiert und es wird insgesamt null zurückgegeben. Das bereits erzeugte Root-Objekt geht damit verloren.

Für einen Aufrufer, der bewusst nur Ebene 0 sehen möchte, ist das klar fehlerhaft.

Betroffene Stellen:

11. Mittel: AD-Gruppenabfragen können still unvollständig sein

cActiveDirectoryBase.privRequestSecurityGroupsListAsync() baut die Gruppenliste aus LDAP-Ergebnissen auf. Dort wird jedoch GroupPrincipal.FindByIdentity(...).GroupScope ohne Null-Check verwendet.

Wenn eine Gruppe aus dem LDAP-Result zwar gefunden wird, aber nicht sauber als GroupPrincipal auflösbar ist, entsteht ein Fehler. Dieser wird anschließend durch einen allgemeinen catch verschluckt, und es wird einfach die bis dahin gesammelte Teilliste zurückgegeben.

Das ist problematisch, weil der Aufrufer nicht klar zwischen „Liste vollständig“ und „Liste vorzeitig abgebrochen“ unterscheiden kann.

Betroffene Stellen:

12. Mittel: Zentrale Fehlerschnittstelle des Providers deckt Schreibfehler nicht ab

GetLastErrorMessage() sammelt nur Fehler aus ntfsBase und activeDirectoryBase. Die eigentlichen Fehler aus DataArea_FileSystem laufen aber über ResultToken und landen nicht in dieser zentralen Fehlerschnittstelle.

Das bedeutet:

  • Logon- und Read-Fehler sind über GetLastErrorMessage() erkennbar
  • Create-/Ensure-Fehler dagegen oft nicht

Für aufrufende Schichten, die nur diese zentrale Schnittstelle kennen, entsteht damit ein unvollständiges Fehlerbild.

Betroffene Stellen:

13. Mittel: ACL-Mapping ist stark von vollständigen Naming-Conventions abhängig und fällt auf Sentinel-Werte zurück

Die Permission-Auflösung in ResolvePermissionGroupsAsync() verwendet .First(...) für die Conventions von Owner, Write und Read. Fehlt eine passende Konvention, wirft der Code sofort eine Exception.

Parallel dazu sind die Zielwerte mit S-1-0-0 vorbelegt. Das führt zu zwei unterschiedlichen Fehlermustern:

  • bei fehlender Konfiguration: harter Laufzeitfehler
  • bei nicht gefundenem Match: scheinbar gültige SID, fachlich aber nur ein Platzhalter

Das macht Ergebnisse schwer interpretierbar und erhöht das Risiko, dass Fehler spät erkannt werden.

Betroffene Stellen:

14. Mittel: Mitgliederauflösung bricht intern mit NullReference ab und wird dann nur noch als null sichtbar

GetMembersAsync() ruft privGetMembersAsync(sid).ToList() auf, bevor geprüft wird, ob überhaupt ein Result vorliegt. Wenn die Gruppe nicht gefunden wird, kommt null zurück und .ToList() wirft intern.

Die Exception wird zwar gefangen, aber nach außen sieht der Aufrufer nur noch null. Damit verschwimmt der Unterschied zwischen:

  • Gruppe nicht vorhanden
  • AD-Auflösung fehlgeschlagen
  • echte Kommunikationsstörung

Betroffene Stellen:

15. Mittel/Niedrig: Share-Objekte können keine Kinder nachladen

cLiamNtfsShare.getFolders() und getChildrenAsync() liefern aktuell immer nur leere Listen zurück. Das bedeutet, dass Share-Objekte zwar initial erzeugt werden können, ein späteres Nachladen über die Objektmethoden aber praktisch nicht funktioniert.

Wenn diese Methoden nicht mehr verwendet werden, ist das nur Altlast. Wenn doch, ist das ein funktionaler Bruch.

Betroffene Stellen:

16. Niedrig, aber inkonsistent: Der alte GET-Pfad verhält sich anders als der aktuelle Providerpfad

Im älteren DataArea_FileSystem_GET-Pfad wird die Root-Struktur anders aufgebaut als im aktuellen Providerpfad. Ebene 0 bekommt dort kein Root-ACL-Mapping, und das Verhalten unterscheidet sich insgesamt von der neueren Logik im eigentlichen Provider.

Wenn dieser Altpfad noch produktiv verwendet wird, gibt es damit zwei unterschiedliche Wahrheiten für denselben fachlichen Bereich.

Betroffene Stellen:

Fazit

Der NTFS-Provider ist an mehreren Stellen funktionsfähig, aber der Schreibpfad ist deutlich fragiler als der Lesepfad.

Die wichtigsten Risiken sind:

  • stille Erfolgsrückgaben trotz unvollständiger Traverse-Verarbeitung
  • mutierende Templates mit Folgeschäden für Naming und Wiederholungslogik
  • blockierende Wartezeiten und inkonsistentes SMB-Verbindungsverhalten
  • Berücksichtigung geerbter ACLs im neuen Ensure-Pfad trotz fachlicher Gegenanforderung
  • unvollständige oder irreführende Fehlerkommunikation nach außen

Für eine technische Bereinigung würde ich die Themen in dieser Reihenfolge angehen:

  1. harte Erfolgs-/Fehlersemantik im Create-/Ensure-/Traverse-Pfad
  2. Entfernen des Thread.Sleep und Ersatz durch kontrolliertes Polling
  3. Templates unveränderlich behandeln und pro Lauf kopieren
  4. ACL-Prüfungen im Ensure-Pfad auf explizite Regeln einschränken
  5. Tag- und Konfigurationsauflösung sauber zentralisieren
Reference in New Issue View Git Blame Copy Permalink