Wenn andere auch nicht Programmieren können
Ich begann, den Coldcard-Hack zu untersuchen, und war sofort schockiert. Ich muss erklären, warum.
Wenn Entwickler am Code arbeiten, organisieren sie den Code-Änderungen in sogenannten „Commits“. Der Zweck davon ist es, einen klaren Verlauf der Code-Änderungen aufzuzeigen, einschließlich des Warums und des Wies.
Dies geschieht genau für Fälle wie diesen, in denen es so aussieht, als würden die Gelder von Bitcoinern massenhaft gestohlen, damit man untersuchen und genau nachvollziehen kann, wie das passieren konnte.
Gute Entwickler schreiben klare Commit-Nachrichten – schriftliche Notizen, die mit den Code-Änderungen einhergehen und erklären, was die spezifische Änderung bewirkt.
Um eine klare Commit-Nachricht zu verfassen, sollte der Commit in der Regel eine kleinere Code-Änderung darstellen, damit es weniger zu kommentieren gibt.
Ein gutes Ziel für einen Entwickler ist ein hohes Verhältnis von Commit-Nachricht zu Code-Änderung. Je mehr Codezeilen man ändert, desto mehr Kommentare sollten erklären, warum man den Code ändert. Mehr Text und weniger Code-Änderungen pro Commit sind im Allgemeinen eine gute Idee.
Hier ist ein zufällig ausgewähltes Beispiel aus seiner eigenen Arbeit.
Die Commit-Nachricht hat 235 Zeichen und der Commit ändert 15 Zeilen Code. Das ist ein Verhältnis von 235/15 = ~16.
Bei der Coldcard bestand die Commit-Nachricht mit dem Fehle aus 5 Zeichen, einfach nur das Wort „runs“. Der Commit ändert 1534 Zeilen Code, was das Verhältnis auf 5/1534 = ~0,003 bringt.
Das ist ein grauenhaft schlechtes Verhältnis von Kommentar zu Code-Änderung.
Es gibt einige seltene Fälle, in denen ein niedriges Kommentarverhältnis vertretbar ist – aber die Änderung des wichtigsten Teils des Codes gehört definitiv nicht dazu!
Code, der Funktionen berührt, die für die Sicherheit des Projekts entscheidend sind, muss ein höheres Verhältnis von Kommentaren zu Änderungen und eine strengere Überprüfung aufweisen.
Der zweite Commit, der zu dem Problem der schwachen Entropie auf Coldcards beigetragen hat, war nicht viel besser
Die Commit-Nachricht besteht aus 1 Zeichen: einfach das Zeichen „x“. Der Commit ändert 1000 Zeilen Code, was das Verhältnis auf 1/1000 = 0,001 bringt.
Das Problem
In dem Commit mit dem Titel „runs“ (Verhältnis: ~0,003) sieht es so aus, als würden sie C-Code importieren und konfigurieren, damit maßgeschneiderter MicroPython-Code auf dem STM32 funktioniert – dem Board, auf dem alle Coldcards laufen.
STM32 ist die gängigste CPU für kleine Geräte wie dieses, und Konfigurationen, wie sie dieser Commit einführt, sind üblich.
In dem „runs“-Commit wurde der Hardware-RNG (Zufallszahlengenerator) mit der folgenden Codezeile deaktiviert.
#define MICROPY_HW_ENABLE_RNG (0)
Das ist es, was den Bug verursacht hat. Das Setzen dieses Wertes auf null teilt dem Standard-MicroPython-RNG-Code mit, das Hardware-RNG-Gerät nicht zu verwenden, sondern stattdessen den Yasmarang-RNG zu nutzen.
Der Entwickler fügte einen Inline-Kommentar hinzu, der diese Änderung „erklärt“:
// We have our own version of this code.
Die COLDCARD-Version dieses Codes scheint sich auf die Funktionen zu beziehen, die in rng.h und rng.c hinzugefügt wurden.
rng.h
MP_DECLARE_CONST_FUN_OBJ_0(pyb_rng_get_obj);
MP_DECLARE_CONST_FUN_OBJ_1(pyb_rng_get_bytes_obj);
Diese scheinen ein Versuch zu sein, die Funktion pyb_rng_get_obj der stm32 rng-Bibliothek zu überschreiben. Dieser Ansatz stieß auf Probleme. Die Datei rng.c des stm32 definiert bereits die Variable pyb_rng_get_obj und setzt den Wert auf pyb_rng_get. Man kann nicht zwei Definitionen derselben Variablen haben und den Code kompilieren lassen.
rng.c
MP_DEFINE_CONST_FUN_OBJ_0(pyb_rng_get_obj, pyb_rng_get);
Wenn man das Makro erweitert und logisch darüber nachdenkt, ist es einfach dieser Pseudocode:
var pyb_rng_get_obj = pyb_rng_get
Der Commit 37e4af5 fügt eine benutzerdefinierte Datei rng.c hinzu, in die er dieselbe Makrodefinition per Copy-Paste eingefügt hat:
MP_DEFINE_CONST_FUN_OBJ_0(pyb_rng_get_obj, pyb_rng_get);
Es gibt nun keine Möglichkeit mehr, diesen Code zu kompilieren. Es scheint, als ob der Entwickler naiv versucht, die Variable pyb_rng_get_obj zu überschreiben, indem er eine doppelte Version der Variablen erstellt. So funktioniert C nicht. Dieser Fehler hätte ihm einen Compiler-Fehler wegen „doppeltem Symbol pyb_rng_get_obj“ eingebracht, da es nun zwei Stellen gibt, an denen es definiert ist: die stm32-Bibliothek und die neu hinzugefügte Datei rng.c.
Von hier an, so vermute ich, hat er in einem Anflug von Frustration MICROPY_HW_ENABLE_RNG auf 0 gesetzt, was den Compiler-Fehler behoben hätte.
Manchmal, wenn Entwickler nicht weiterwissen, probieren sie wahllose Dinge aus, um zu sehen, ob es hilft. MICROPY_HW_ENABLE_RNG auf 0 zu setzen, hätte den Compiler-Fehler verschwinden lassen – aus dem falschen Grund.
Dadurch wurde der Compiler-Fehler aufgrund der zwei widersprüchlichen Definitionen einfach unter den Teppich gekehrt.
#define MICROPY_HW_ENABLE_RNG (0)
Das Setzen von MICROPY_HW_ENABLE_RNG auf null hat den Code, der den Hardware-Zufallszahlengenerator verwendete, vollständig entfernt (die Zeilen 31 bis 80 in der stm32-Datei rng.c). Dies hatte den Nebeneffekt, dass die zweite Definition von pyb_rng_get_obj entfernt wurde, was den Compiler-Fehler „behob“.
Der Compiler-Fehler flehte den Programmierer förmlich an, seine Logik zu überdenken. Anstatt dass dies geschah, wurde der Compiler-Fehler einfach stummgeschaltet. Der Compiler gab dem Entwickler eine letzte Chance, sein Vorhaben zu überdenken, aber der Alarm wurde ignoriert und abgestellt.
Jetzt ergibt alles einen Sinn. Es sieht so aus, als hätte der Entwickler versucht, die Variable zu überschreiben, stieß aber auf widersprüchliche Definitionen. Konfrontiert mit einem Fehler wegen eines „doppelten Symbols“, versuchte er wahllose Änderungen vorzunehmen, um den Code zum Laufen zu bringen.
Er entdeckte, dass das Ändern von MICROPY_HW_ENABLE_RNG auf 0 dazu führte, dass der Code kompilierte. Er hatte wahrscheinlich keine Ahnung warum, stellte aber eine Theorie auf: Wir brauchen das nicht mehr, weil wir unsere eigene Version des Codes haben.
Seine Definitionen von pyb_rng_get waren nun vorhanden, und der Code, den er nicht ausführen lassen wollte, war abgeschaltet worden.
Verwirrende Aufrufketten
Die Firmware der Coldcard ist in C geschrieben. Die Anwendungsebene, die der Hardware sagt, was sie tun soll, ist in Python geschrieben. Der Python-Code ruft den C-Code auf. Der Entwickler überschrieb die pyb_rng_get-Funktionen.
Zum Leidwesen vieler waren die pyb_rng_get-Funktionen, die sein Code überschrieb, gar nicht das, was tatsächlich aus dem Python-Code aufgerufen wird. Der Python-Code in der Version 4.0.0 der Coldcard ruft random.bytes() in seiner make_new_wallet()-Funktion auf.
async def make_new_wallet():
await ux_dramatic_pause('Generating...', 4)
seed = random.bytes(32) # OOPS
assert len(set(seed)) > 4
seed = ngu.hash.sha256s(seed)
await approve_word_list(seed)
Das Setzen von MICROPY_HW_ENABLE_RNG auf 0 schaltete den von der stm32-Bibliothek bereitgestellten Hardware-Code ab, ermöglichte es dem Entwickler aber, pyb_rng_get_obj zu setzen. Das Problem ist, dass pyb_rng_get_obj das in Python sichtbare Callable pyb.rng() ist.
Das ist aber nicht das, was der Entwickler hier in der Wallet-Funktion aufruft. Stattdessen verwenden sie random.bytes(32), was den Aufrufpfad von pyb_rng_get komplett überspringt und stattdessen rng_get aufruft. Da MICROPY_HW_ENABLE_RNG auf null gesetzt war, nutzte dies die MicroPython-stm32-Definition in rng.c bei Zeile 112, welche die unsichere Yasmarang-Entropie (keine Hardware-Wallet-Entropie) aufrief.
uint32_t rng_get(void) {
return pyb_rng_yasmarang();
}
Einfach gesagt: Die Firmware-Änderung überschreibt Funktionen, die beim Erstellen eines neuen Wallets gar nicht verwendet werden, während sie als Nebeneffekt der Einbindung der Python-Methoden-Überschreibungen die Nutzung des Hardware-RNG in jedem Fall abschaltet und stattdessen einen sehr schwachen Zufallszahlengenerator verwendet.
Das letzte Beweisstück ist die Commit-Nachricht selbst. Es war einfach die Nachricht „runs“ (läuft).
Entwickler, wenn ihr jemals in dieses Szenario geratet, hört bitte auf. Was auch immer ihr dafür bezahlt bekommt, es ist die Verwüstung nicht wert, die ihr potenziell für andere anrichten könntet.
Liefert keinen Code aus, den ihr nicht versteht.
Ein Fluch auf MicroPython
Im Kern scheint dies eine Konsequenz aus zu vielen Komplexitätsschichten zu sein. Da ist die MicroPython-Bibliothek, die Bindings zwischen dem C-Code und der Python-Anwendung, und die neuen Funktionen, die COLDCARD hinzufügt. War der Entwickler, der diesen Patch erstellt hat, eigentlich ein Python-Entwickler, der gezwungen war, C zu schreiben und zu handhaben?
MicroPython erweckt die Illusion, dass Embedded-Entwickler C, ihre CPU oder andere fortgeschrittene Konzepte nicht verstehen müssen, um Embedded-Programmierung zu betreiben.
Das ist eine Lüge.
Und diese Katastrophe ist das Resultat des Glaubens an diese Lüge.
Man muss den Code verstehen, den man ausliefert. Punkt. Es gibt keine Ausreden. Ebenen der Irreführung machen es nur schwerer zu verstehen.
Wenn ihr Änderungen vornehmt, stellt sicher, dass ihr verifiziert, dass sie auch das tun, was sie tun sollen.
Die Commit-Nachrichten erzählen hier die wahre Geschichte. Den kritischsten Teil des Codes mit der hier gezeigten mangelnden Sorgfalt und dem mangelnden Verständnis zu behandeln, ist unentschuldbar.
Ihr, als Entwickler, die an Bitcoin arbeiten, müsst euch die Zeit nehmen, eure Änderungen zu verstehen, sie klar zu dokumentieren und zu verifizieren, dass sie das tun, was ihr denkt.
Wenn das Leben von Menschen ruiniert wird, werden und sollten sie euch kein Mitgefühl entgegenbringen. Ihr müsst die Gelder wertschätzen, die ihr aufs Spiel setzt. Eure Aufgabe als Bitcoin-Entwickler ist von höchster Bedeutung. Ich habe in der Vergangenheit selbst Fehler gemacht, aber es gibt keine Ausreden dafür, nicht zu verstehen, was man ausliefert, wenn der Code derart kritisch ist.
Ich hoffe, dass wir als Branche daraus lernen und uns aufeinander stützen können, um als Entwickler-Community sicheren Code auszuliefern.
Write a comment