Hallo allerseits!
Ich mag Tox und respektiere die Teilnehmer dieses Projekts und ihre Arbeit. Um den Entwicklern und Benutzern von Tox zu helfen, habe ich den Code untersucht und potenzielle Probleme festgestellt, die zu einem falschen Sicherheitsgefühl führen können. Seit ich diesen Artikel ursprünglich 2016 ( auf Russisch ) veröffentlicht habe, wurden viele Verbesserungen an Tox vorgenommen, und ich leite ein Team, das sichere Tox-Software mit der Programmiersprache Rust von Grund auf neu geschrieben hat (siehe Tox-rs ). Ich empfehle die Verwendung von Tox im Jahr 2019. Werfen wir einen Blick darauf, warum wir Tox in Rust neu geschrieben haben.
Originalartikel von 2016
Es gibt eine ungesunde Tendenz, die Sicherheit von E2E-Systemen nur auf der Grundlage zu überschätzen, dass es sich um E2E handelt. Ich werde objektive Fakten präsentieren, ergänzt durch meine eigenen Kommentare, damit Sie Ihre eigenen Schlussfolgerungen ziehen können.
Spoiler: Die Tox-Entwickler stimmen meinen Punkten zu und meine Quellcode- Pull-Anfrage wurde akzeptiert.
Alles begann mit Artikeln über Habr über die Installation des Knotens ( auf Russisch ).
In den Kommentaren haben sich die Leute über die Komplexität des Aufbaus und der Installation des Knotens unter CentOS beschwert, daher habe ich beschlossen, ein Build-System auf CMake zu schreiben. Nach ein paar Tagen war ich bereit, meine PR der Tox-Community auf Freenode vorzustellen, aber ich stieß auf Unverständnis:
Jemand hat anfangs cmake beigesteuert, aber andere Entwickler wussten nicht, wie man es verwendet, und konnten es nicht dazu bringen, ihren Code zu erstellen. Deshalb wechselten sie zu Autotools (sic!), die sie jetzt besser kennenlernen.
Ich habe festgestellt, dass Code, der die Tests in Travis CI nicht besteht, immer noch in den Hauptzweig aufgenommen wird, aber sie antworteten: "Wir verstehen, dass wir etwas mit den Tests tun müssen, aber lassen Sie es vorerst sein."
Als nächstes habe ich mir den Code dieses attraktiven Boten genauer angesehen.
Fakt Nr. 2. Memset (ptr, 0, Größe) vor dem kostenlosen Anruf
Mein Blick fiel auf
memset(c, 0, sizeof(Net_Crypto)); free(c);
Wenn Sie mit PVS-Studio und seinem Artikel über die Memset-Funktion PVS-Studio noch nicht vertraut sind: Der Compiler kann den Funktionsaufruf 'memset' löschen, wenn dieser Speicherbereich danach nicht verwendet wird. Die Logik des Compilers ist unkompliziert: "Sie werden diese Variable nach dem Aufruf von 'free' nicht mehr verwenden. Memset hat keinen Einfluss auf das beobachtete Verhalten. Lassen Sie mich diesen nutzlosen Aufruf von 'memset' löschen."
Als fleißiger Schüler habe ich jedes Vorkommen von memset($DST, 0, $SIZE) durch Natrium_Memzero und TESTS CRASHED ersetzt.
Fakt Nr. 3. Der Vergleich öffentlicher Schlüssel ist anfällig für Timing-Angriffe
Es gibt eine wirklich großartige Sonderfunktion, um öffentliche Schlüssel in toxcore zu vergleichen:
int public_key_cmp(const uint8_t *pk1, const uint8_t *pk2) { return crypto_verify_32(pk1, pk2); }
crypto_verify_32 - ist eine Funktion aus der NaCL / Sodium- Kryptobibliothek , mit der Sie Timing-Angriffe vermeiden können, da sie in konstanter Zeit funktioniert, während memcmp beim ersten ungleichen Byte brechen kann. Sie sollten crypto_verify_32 verwenden , um vertrauliche Daten wie Schlüssel zu vergleichen.
Byte-für-Byte- Zeichenfolgenvergleiche können durch Timing-Angriffe ausgenutzt werden , um beispielsweise MACs zu fälschen (siehe diese Sicherheitsanfälligkeit in der Keyczar - Kryptobibliothek von Google).
Die Codebasis des Toxcore-Projekts ist ziemlich umfangreich, weshalb Tox mit einem Fehler in Bezug auf Timing-Schwachstellen geboren wurde:
bool id_equal(const uint8_t *dest, const uint8_t *src) { return memcmp(dest, src, crypto_box_PUBLICKEYBYTES) == 0; }
Aber das ist noch nicht alles. Die Entwickler bevorzugen es immer noch, Schlüssel auf ihre eigene Weise mit drei verschiedenen Funktionen zu vergleichen: id_equal oder public_key_cmp und crypto_verify_32 .
Hier ist eine kurze Grep-Ausgabe von DHT, Zwiebel-Routing und anderen kritischen Subsystemen:
if (memcmp(ping->to_ping[i].public_key, public_key, crypto_box_PUBLICKEYBYTES) == 0) { if (memcmp(public_key, onion_c->friends_list[i].real_public_key, crypto_box_PUBLICKEYBYTES) == 0) if (memcmp(public_key, onion_c->path_nodes_bs[i].public_key, crypto_box_PUBLICKEYBYTES) == 0) if (memcmp(dht_public_key, dht_public_key_temp, crypto_box_PUBLICKEYBYTES) != 0) if (Local_ip(ip_port.ip) && memcmp(friend_con->dht_temp_pk, public_key, crypto_box_PUBLICKEYBYTES) == 0)
Fakt Nr. 4. increment_nonce in einer nicht konstanten Zeit
void increment_nonce(uint8_t *nonce) { uint32_t i; for (i = crypto_box_NONCEBYTES; i != 0; --i) { ++nonce[i - 1]; if (nonce[i - 1] != 0) break;
Wenn solche Operationen geheime Parameter beinhalten, können diese Zeitvariationen einige Informationen verlieren. Bei ausreichendem Wissen über die Implementierung könnte eine sorgfältige statistische Analyse sogar zur vollständigen Wiederherstellung geheimer Parameter führen.
In Natrium gibt es eine spezielle Funktion, um Nonces zu erhöhen:
Ein versehentlich ironisches Osterei ist, dass sich die Funktion increment_nonce in einer Datei befindet, die mit den Worten beginnt:
Dieser Code muss perfekt sein. Wir spielen nicht mit Verschlüsselung herum.
Schauen wir uns diese perfekte Datei genauer an.
Fakt Nr. 5. Sie finden Schlüssel und private Daten im Stapel
Problematischer Code:
void encrypt_precompute(const uint8_t *public_key, const uint8_t *secret_key, uint8_t *enc_key) { crypto_box_beforenm(enc_key, public_key, secret_key);
encrypt_data_symmetric ruft crypto_box_detached_afternm von Nacl / Sodium auf. Ich werde nicht den gesamten Code eingeben. Hier ist ein Link , den Sie selbst überprüfen können.
Es scheint schwierig zu sein, in vier Codezeilen einen Fehler zu machen, nicht wahr?
Lassen Sie uns in Natrium graben:
int crypto_box_detached(unsigned char *c, unsigned char *mac, const unsigned char *m, unsigned long long mlen, const unsigned char *n, const unsigned char *pk, const unsigned char *sk) { unsigned char k[crypto_box_BEFORENMBYTES]; int ret; (void) sizeof(int[crypto_box_BEFORENMBYTES >= crypto_secretbox_KEYBYTES ? 1 : -1]); if (crypto_box_beforenm(k, pk, sk) != 0) { return -1; } ret = crypto_box_detached_afternm(c, mac, m, mlen, n, k); sodium_memzero(k, sizeof k); return ret; }
Löschen aller Schecks, die wir erhalten:
unsigned char k[crypto_box_BEFORENMBYTES]; int ret; crypto_box_beforenm(k, pk, sk); ret = crypto_box_detached_afternm(c, mac, m, mlen, n, k); sodium_memzero(k, sizeof k); return ret;
Kommt es mir bekannt vor? Ja! Es ist ein leicht modifizierter Funktionscode encrypt_data von toxcore. Der einzige Unterschied besteht darin, dass sie vergessen haben, den Schlüssel auf dem Stapel mit natrium_memzero zu bereinigen ... Und es gibt auch Fehler in: handle_TCP_handshake , handle_handshake und vielleicht auch woanders.
Fakt Nr. 6. Compiler-Warnungen sind für Dummies!
Die Entwickler des Toxcore-Projekts bestreiten rundweg die Notwendigkeit, alle Compiler-Warnungen zu aktivieren, oder sie wissen nichts über sie.
Nicht verwendete Funktionen (ich bin besonders zufrieden mit den Warnungen in Tests):
../auto_tests/dht_test.c:351:12: warning: unused function 'test_addto_lists_ipv4' [-Wunused-function] START_TEST(test_addto_lists_ipv4) ^ ../auto_tests/dht_test.c:360:12: warning: unused function 'test_addto_lists_ipv6' [-Wunused-function] START_TEST(test_addto_lists_ipv6) ^ ../toxcore/TCP_server.c:1026:13: warning: unused function 'do_TCP_accept_new' [-Wunused-function] static void do_TCP_accept_new(TCP_Server *TCP_server) ^ ../toxcore/TCP_server.c:1110:13: warning: unused function 'do_TCP_incomming' [-Wunused-function] static void do_TCP_incomming(TCP_Server *TCP_server) ^ ../toxcore/TCP_server.c:1119:13: warning: unused function 'do_TCP_unconfirmed' [-Wunused-function] static void do_TCP_unconfirmed(TCP_Server *TCP_server) ^
../toxcore/Messenger.c:2040:28: warning: comparison of constant 256 with expression of type 'uint8_t' (aka 'unsigned char') is always false [-Wtautological-constant-out-of-range-compare] if (filenumber >= MAX_CONCURRENT_FILE_PIPES) ~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~ ../toxcore/Messenger.c:2095:28: warning: comparison of constant 256 with expression of type 'uint8_t' (aka 'unsigned char') is always false [-Wtautological-constant-out-of-range-compare] if (filenumber >= MAX_CONCURRENT_FILE_PIPES) ~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~ ../toxcore/Messenger.c:2110:28: warning: comparison of constant 256 with expression of type 'uint8_t' (aka 'unsigned char') is always false [-Wtautological-constant-out-of-range-compare] if (filenumber >= MAX_CONCURRENT_FILE_PIPES) ~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~
../auto_tests/TCP_test.c:205:24: warning: unsequenced modification and access to 'len' [-Wunsequenced] ck_assert_msg((len = recv(con->sock, data, length, 0)) == length, "wrong len %i\n", len); ^ ~~~ /usr/include/check.h:273:18: note: expanded from macro 'ck_assert_msg' _ck_assert_msg(expr, __FILE__, __LINE__,\ ^
Und ein paar Dutzend Warnungen vor nicht verwendeten Variablen, dem Vergleich von signierten und nicht signierten und mehr.
Mein Fazit
Zitat aus dem Repository:
Wir möchten, dass Tox so einfach wie möglich und gleichzeitig so sicher wie möglich bleibt.
Wenn ich, ein Nicht-Kryptograf, an einem Tag so schreckliche Fehler finden könnte, stellen Sie sich vor, wie viele Dinge ein Kryptografiespezialist finden könnte, nachdem er einen Monat lang absichtlich nach ihnen gesucht hat.
Die frühen Versionen des Tox stellten eine große Gefahr für Benutzer dar, die sich auf die Sicherheit von Tox verlassen. Proprietäre Lösungen sind nicht vertrauenswürdig und selbst Open Source-Lösungen sind nicht so sicher, wie Sie es möchten. Sehen Sie sich die jüngste Sicherheitsverletzung in Matrix an . Heutzutage werden viele Fehler behoben und Tox ist die beste verfügbare Option für Sicherheit und Datenschutz für Benutzer.
Das nächste Mal werde ich Ihnen mehr über den aktuellen Zustand der Toxine erzählen. Was wir in Rust implementiert haben und warum Sie es versuchen sollten.
Reddit: Kommentare