Xz: Das eine Zeichen, das Linux Landlock außer Kraft setzte

 (git.tukaani.org)
1 Punkte von GN⁺ 2024-03-31 | 1 Kommentare | Auf WhatsApp teilen
  • Die Änderung an CMakeLists.txt in xz.git ist ein Fall, in dem bei der Umstellung der Landlock-Sandbox-Erkennung von einer Prüfung auf das Vorhandensein des Headers auf einen echten Kompilierungstest ein einzelnes .-Zeichen im Testcode landete
  • Die neue Prüfung wurde hinzugefügt, um Systeme auszufiltern, die zwar linux/landlock.h haben, denen aber die nötigen syscall-Definitionen fehlen könnten
  • Der Kompilierungstest bindet <linux/landlock.h>, <sys/syscall.h> und <sys/prctl.h> ein und referenziert prctl, SYS_landlock_create_ruleset, SYS_landlock_restrict_self sowie LANDLOCK_CREATE_RULESET_VERSION
  • Das bisherige Kriterium HAVE_LINUX_LANDLOCK_H wird in HAVE_LINUX_LANDLOCK geändert; auch die Einstellungen für SANDBOX_COMPILE_DEFINITION und SANDBOX_FOUND folgen diesem Ergebnis
  • Der Commit korrigiert den Linux-Landlock-Feature-Test in den Autotools- und CMake-Builds; der bereitgestellte Diff zeigt die Änderung auf der CMake-Seite

Änderung der Landlock-Erkennung in CMake

  • Die betroffene Datei ist CMakeLists.txt in xz.git; die Änderung befindet sich im Block # Sandboxing: Landlock
  • Die bisherige CMake-Logik prüfte, wenn ENABLE_SANDBOX auf ON oder landlock stand und SANDBOX_FOUND nicht gesetzt war, mit check_include_file(linux/landlock.h HAVE_LINUX_LANDLOCK_H) lediglich das Vorhandensein des Headers
  • Nach der Änderung wird mit check_c_source_compiles ein kleines C-Programm tatsächlich kompiliert, um zu prüfen, ob Landlock nutzbar ist
    • Manche Systeme besitzen zwar linux/landlock.h, haben aber möglicherweise nicht die für Linux Landlock nötigen syscall-Definitionen
    • Der Testcode bindet <linux/landlock.h>, <sys/syscall.h> und <sys/prctl.h> ein
    • In my_sandbox() referenziert er prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0), SYS_landlock_create_ruleset, SYS_landlock_restrict_self und LANDLOCK_CREATE_RULESET_VERSION

Einzelnes Zeichen und Änderung des Variablennamens

  • Nach dem Include-Block des eingefügten Kompilierungstest-Codes steht eine einzelne Zeile mit .
  • Die Ergebnisvariable wird von HAVE_LINUX_LANDLOCK_H in HAVE_LINUX_LANDLOCK geändert
  • Auch die Bedingung wird von if(HAVE_LINUX_LANDLOCK_H) zu if(HAVE_LINUX_LANDLOCK) geändert
  • Der Wert von SANDBOX_COMPILE_DEFINITION verwendet statt "HAVE_LINUX_LANDLOCK_H" nun "HAVE_LINUX_LANDLOCK"
  • Die anschließende Einstellung SANDBOX_FOUND ON bleibt erhalten

Verwandte Beiträge

1 Kommentare

 
GN⁺ 2024-03-31
Meinungen auf Hacker News

  • Antwort: https://git.tukaani.org/?p=xz.git;a=commitdiff;h=f9cf4c05edd...
    Wer das Landlock-Zugriffskontrollsystem von Linux nicht gut kennt, findet hier eine Erklärung: https://docs.kernel.org/userspace-api/landlock.html
    Die offizielle (vermutlich ...) Incident-Response-Seite zu xz: https://tukaani.org/xz-backdoor/

    • Also prüfte diese Funktion, ob der nachfolgende C-Code kompiliert, und aktivierte Landlock nur in diesem Fall?
      Aber wegen eines kleinen Punkts, der nahe am linken Rand des Diffs klebte und deshalb schwer zu erkennen war, wurde der C-Code immer ungültig, sodass Landlock immer deaktiviert blieb?
      Beeindruckend niederträchtig und niederträchtig beeindruckend. Beim ersten Lesen habe ich es auch nicht gesehen
    • Das hat plausible Abstreitbarkeit
      Wenn man etwas verstecken will, gibt es bessere Methoden, etwa Unicode-Doppelgänger auszutauschen. Manche sehen je nach Schriftart bis aufs Pixel gleich aus
      Vielleicht habe ich den Faden verloren, aber ist die Absicht hier schon bewiesen?
    • Das ist so bösartig, dass man selbst dann, wenn es während des PRs auf frischer Tat erwischt würde, einfach mit „Oh, das hat meine IDE beim Auto-Format so gemacht“ durchkommen könnte
    • Lasse Collin ist wieder aufgetaucht und wirkt vermutlich wütend
    • Mein Versionsverwaltungssystem hebt geänderte Zeichen gefühlt besser hervor als solche pauschalen grünen/roten Strings

  • Beim vorsichtigen Überfliegen sah ich an einer Stelle, an der definitiv keiner sein sollte, einen Punkt und dachte: „So schwer ist es ja gar nicht“
    Dann tippte ich gegen den Bildschirm, und der Punkt bewegte sich
    Verdammter Monitorstaub

  • Ich kann nicht glauben, dass Systemsicherheit von einer so lockeren Kette der Korrektheit abhängt. Es gab jede Menge Stellen, an denen man das hätte verhindern können
    + # A compile check is done here because some systems have
    + # linux/landlock.h, but do not have the syscalls defined
    + # in order to actually use Linux Landlock.
    Man sollte die Header solcher Systeme reparieren, damit sie explizit ausgeschlossen werden. Was bringt ein Header, wenn er seine eigene Funktionalität nicht deklariert?
    Und ich verstehe auch nicht, warum es nach der Erstellung des Binary Blob (selbst wenn er aus Open Source kompiliert wurde) keinen einzigen Test gibt, der prüft, ob die Sicherheit intakt ist
    Eine Website-Zahlungsfunktion würde man auch nicht ohne End-to-End-Tests für die Kernfunktionalität ausrollen. Es scheint eine Prioritätenverschiebung zu geben, bei der neue Funktionen vor Stabilität stehen
    Ich hoffe, der Fix besteht nicht einfach darin, den . zu entfernen. Habe gerade in einem anderen HN-Beitrag gesehen, wie das Entfernen des . gezeigt wurde

    • Er führte ein komplett neues Test-Framework ein und schleuste dann über zwei Jahre hinweg langsam eine Backdoor ein
    • Das von dir Zitierte stammt von Jia Tan [1]. Es ist ein Kommentar, den der bösartige Akteur schrieb, während er die Prüfung absichtlich kaputtmachte
      Eine Header-Korrektur oder zusätzliche Tests hätten das nicht verhindert. Es gab zunächst kein Anzeichen dafür, dass der Header kaputt war, und zusätzliche Tests hätten auf andere Weise manipuliert oder im Release-Tarball ignoriert werden können
      [1] https://git.tukaani.org/?p=xz.git;a=commit;h=328c52da8a2bbb8...
    • Etwas am Rande: GCC 9.4.0 hatte einmal einen kaputten arm_acle.h-Header ausgeliefert [https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100985] — Code, der diesen Header einband, schlug beim Kompilieren immer fehl
      Da Nutzer möglicherweise mit GCC 9.4.0 kompilieren wollten und die von diesem Header abhängige Funktion für die Anwendung nicht zentral war, deaktivierte der Build diese Funktion einfach und gab eine Warnung aus, wenn er erkannte, dass der Header kaputt war
      Zurück zu xz: Wenn Sicherheit nicht die oberste Priorität ist, wirkt es durchaus vernünftig, den Ausfall einer optionalen Funktion zu ignorieren und weiterzumachen. Im Nachhinein ist es natürlich leicht, mit dem Finger darauf zu zeigen, aber ohne diesen Kontext ist die Entscheidung nicht völlig unsinnig
    • Als Open-Source-Nutzer weiß ich nicht genug, um so einen Vorschlag zu machen; wenn du das selbst entwickelst und beiträgst, wäre ich dankbar
    • Weißt du, ob der von dir zitierte Kommentarcode korrekt ist?

  • Uff, sein letzter Commit verschlechtert die Sicherheitsmeldungen: https://git.tukaani.org/?p=xz.git;a=commitdiff;h=af071ef7702...

    • Warum wurde das akzeptiert? Ernsthaft gefragt

  • Wofür bzw. wie sollte Landlock eigentlich eingesetzt werden?
    In einer allgemeinen Bibliothek wie für Komprimierung/Dekomprimierung dürfte es in der Praxis schwer nutzbar sein. Eine Bibliothek kann nicht wissen, was das Programm tun soll und was eingeschränkt werden muss.
    Bei einem Programm kann das klarer sein.
    Der Angriff auf sshd nutzte liblzma als Bibliothek. Dann scheint das Abschalten von Landlock irrelevant zu sein, oder? Ist das ein Hinweis darauf, dass es noch weiteren, bisher nicht entdeckten Schadcode gibt, oder dass geplant war, ihn später einzubauen?

    • Vermutlich hätte sshd selbst es genutzt, um ab einem bestimmten Punkt der Ausführung seine eigenen Rechte zu sperren.
      Wenn Landlock entfernt wird, sperrt sich der Daemon nicht selbst, sodass die Ausführung der Payload leichter wird, sobald die Ausnutzungsphase erreicht ist.
      Ich sehe die beiden Dinge nicht als unabhängig. Man hatte wohl bereits eine Payload im Blick und merkte, dass dies ein Hindernis werden würde.
    • Seltsam. Zwei voneinander unabhängige backdoor-artige Arbeiten im selben Projekt zu machen, wirkt auf mich wirklich stümperhaft. Der Nutzen scheint gering, während das Entdeckungsrisiko stark steigt.
      Dieser Vorfall ist sehr verwirrend. Manche Teile sind fast unglaublich raffiniert, andere wirken ziemlich schlampig.
    • Es ist nutzbar und eine gute Idee, aber eher dazu gedacht, Exploits von Schwachstellen zu verhindern, nicht als absichtliche Backdoor.
      Die Idee ist, dass die Bibliothek komplexe Arbeit in einem separaten Thread startet und der User-Code innerhalb der aufgerufenen API auf diesen Thread wartet. Der Thread wendet selbst Landlock an und beginnt dann mit der Arbeit. Wenn etwas schiefläuft, ist der Code isoliert.
      In diesem Fall funktioniert das natürlich nicht, weil die Sandbox vom Angreifer kontrolliert wird. Er kann sie einfach abschalten oder schwächer machen als ursprünglich. Trotzdem kann man es auch auf andere Weise umsetzen.

  • Was genau war hier eigentlich das Ziel? Wollte man später weitere Backdoors einbauen, plausibler abstreitbare Backdoors? Aus meiner Sicht hätten weder oss-fuzz noch diese Änderung die entdeckte Backdoor tatsächlich gefunden.
    Aber warum alle Backdoor-Eier in einen Korb legen, also in eine einzige Bibliothek?

    • Diese Bibliothek sitzt tief genug im Stack, genießt genug Vertrauen, und der Hauptentwickler war wegen psychischer Probleme immer wieder längere Zeit offline.
      Außerdem wird nicht die Bibliothek selbst backdoored, sondern ein Tool angegriffen, das sie verwendet. Ganz im Stil von „Reflections on trusting trust“.
      Bis zum Scheitern klang das nach einem perfekten Plan.
    • Aber wer sagt, dass es nur diese eine Bibliothek war?

  • Ich frage mich, warum es nützlich war, zu verhindern, dass Landlock in xz aktiviert wird. Wollte man in einer späteren Phase schädliche Inhalte in xz-Archive einschleusen? Wenn ja, warum nicht einfach direkt schädliches Verhalten in xz einbauen?

    • Weil eine absichtliche Schwachstelle viel leichter zu verstecken ist als tatsächlich schädliche Inhalte.
      In ein gewartetes C-Projekt heimlich einen Buffer Overflow oder Use-after-free einzubauen, könnte unbemerkt möglich sein. Einen echten Trojaner auszuliefern ist deutlich schwieriger, und bei der xz-zu-sshd-Backdoor hat sich genau das gezeigt.
    • Neben dem Angriff auf ssh könnte es auch eine subtilere Backdoor geben, die auf xz selbst abzielt. Das Ziel war eindeutig Heimlichkeit.

  • Das ist überraschend auffällig. Die Technik zum Abschalten der Funktion ist clever, und der Commit wirkt ziemlich plausibel. Aber warum wurde statt eines einfachen Tippfehlers ein offensichtlicher Syntaxfehler gewählt? Hätte man es zum Beispiel bemerkt, wenn der Fehler PR_SET_NO_NEW_PRIV gewesen wäre?
    Das wäre auch leichter abstreitbar gewesen.

  • Nebenbei hat dieses Malware-Team einen hervorragenden Datensatz geschaffen, um KI darauf zu trainieren, Sicherheitsprobleme zu erkennen. Jeder Commit enthält ein Sicherheitsproblem, und die Open-Source-Community wird sie der Reihe nach durchgehen und finden.
    Danke an die Maintainer, die die Aufräumarbeit leisten. Das ist sicher keine spaßige Aufgabe.

    • Ich glaube nicht, dass sich das gut generalisieren lässt. Bestenfalls ist es ein Wettrüsten.
    • Das ist eine der ziemlich coolen KI-Anwendungen, die ich gesehen habe.
    • Davon höre ich zum ersten Mal; kannst du dazu ein paar Informationen posten?

  • Deshalb mag ich configure-artige Build-Systeme, die automatisch Funktionen ein- und ausschalten, wirklich nicht. Wenn ich etwas will, möchte ich es explizit aktivieren. Wenn es gute Gründe gibt, eine Funktion standardmäßig zu aktivieren, sollte man sie stattdessen explizit deaktivieren können.

    • Beim Packaging ist das ein riesiger Schmerz. Man konfiguriert das Paket, fügt Abhängigkeiten hinzu, bis es baut, und denkt, man sei fertig. Dann fehlt Feature X. Was ist los? Ach, libY fehlt, also wurde es stillschweigend deaktiviert. Also zurück und hinzufügen. Dann meldet ein Nutzer, dass Feature Z fehlt.
      Man sollte einfach ein Standard-Feature-Set haben und nach Bedarf --enable-a --disable-b erlauben.
      Dass Bugs im Prüfprozess stillschweigend geschluckt werden, ist noch ein anderes Problem.