Bug oder Absicht
-
lokoklo schrieb:
Warum verwenden die einfaches memcpy? Warum wird bei so sicherheitskritischem Code nicht eine eigene memcpy funktion geschrieben die structs nimmt, die den Buffer und die Länge enthalten und diese immer prüfen?
Ich vermute dass die Programmierer von OpenSSL hier performanten Code haben wollte, denn die machen das noch an weiteren Stellen im Code so.
Hier muss man einfach daran denken, dass eine Webseite > 10000 Zugriffe/s haben kann und dann sollte der Code halt auch nicht schnarchlahm sein.
Das erklärt dann die Motivation für so eine memcpy Funktion, wie die OpenSSL Macher sie realisiert haben.
-
TyRoXx schrieb:
So widerlicher Code ist leider allgemein akzeptiert. Das ist das eigentliche Problem an der ganzen Sache.
Hier möchte ich aber auch mal anmerken, dass an Hochschulen nur selten die Qualität des Codes angeschaut wird. Den meisten Profs genügt es leider, wenn das Programm läuft. Da muss man sich dann nicht wundern, wenn so etwas schon im Studium ohne Kritik hingenommen wird.
Ich habe mir jedenfalls schon oft gewünscht, dass die Profs oder die dafür abgestellten Praktikanten meinen Code beruteilen, aber vermutlich haben die auch keine Zeit dafür oder werden dafür nicht bezahlt.
-
Auskenner schrieb:
lokoklo schrieb:
Warum verwenden die einfaches memcpy? Warum wird bei so sicherheitskritischem Code nicht eine eigene memcpy funktion geschrieben die structs nimmt, die den Buffer und die Länge enthalten und diese immer prüfen?
Ich vermute dass die Programmierer von OpenSSL hier performanten Code haben wollte, denn die machen das noch an weiteren Stellen im Code so.
Hier muss man einfach daran denken, dass eine Webseite > 10000 Zugriffe/s haben kann und dann sollte der Code halt auch nicht schnarchlahm sein.
Das erklärt dann die Motivation für so eine memcpy Funktion, wie die OpenSSL Macher sie realisiert haben.So wirds sein. Der ganze Code sieht so wohl überlegt aus.
https://github.com/openssl/openssl/commit/731f431497f463f3a2a97236fe0187b11c44aead
Also, defining constants, especially those with human-readable names, is a massive affront to performance! If another developer wants to know that "payload" actually describes the size of the buffer, they should go read the RFC.
-
lokoklo schrieb:
https://github.com/openssl/openssl/commit/731f431497f463f3a2a97236fe0187b11c44aead
Also, defining constants, especially those with human-readable names, is a massive affront to performance! If another developer wants to know that "payload" actually describes the size of the buffer, they should go read the RFC.
Die ganzen Kommentare für den Patch kannst du getrost ignorieren, die sind von irgendwelchen unbeteiligten Personen, die lustig sein wollen.
Und man darf nicht vergessen, dass das der Codeteil relativ unwichtig ist, und deshalb wenig Beachtung findet.
-
hustbaer schrieb:
Gregor schrieb:
...zumindest nach dem, was Fefe sagt.
Wenn ich das so lese...
Und auch protokolltechnisch ergibt das keinen Sinn, so eine Extension überhaupt zu definieren. TCP hat seit 30 Jahren keep-alive-Support. Es hätte also völlig gereicht, das für TLS über UDP zu definieren (und auch da würde ich die Sinnhaftigkeit bestreiten).
... denke ich mir, der sollte sich auch lieber auf Sachen beschränken von denen er Ahnung hat.
TCP/IP Keepalive meine Fresse.
Es macht definitiv Sinn in einem L7 Protokoll Keepalives zu haben.Warum sollte man ein Keepalive in eine SSL Bibliothek einbauen? Was hat das mit Verschlüsselung zu tun? Ältere OpenSSL Versionen haben auch ohne funktioniert. Warum haben die diese Funktion nicht deaktiviert? Die wurde nur von Versionen verwendet, die diesen Bug haben.
-
Mal eine Frage.
Werden bei dem Bug von belegtem Speicher Daten kopiert oder nur solcher, der zuvor freigegeben* wurde?
* Da OpenSSL seine eigene Speicherverwaltung hat, nehme ich mal an, dass er aus Betriebssystemsicht noch reserviert ist, aber das meine ich nicht, sondern eben, ob er innerhalb des Programms noch belegt war.
Wenn dies nicht der Fall sein sollte, dann hätte ja eine Funktion, die freigegebenen Speicher überschreibt geholfen, den Bug zum PW Diebstahl auszunutzen.
-
Memcopy schrieb:
Wenn dies nicht der Fall sein sollte, dann hätte ja eine Funktion, die freigegebenen Speicher überschreibt geholfen,
Also eine Funktion, die beim freigeben den Speicher vorher überschreibt meine ich.
-
sarkasmusdetektor schrieb:
lokoklo schrieb:
https://github.com/openssl/openssl/commit/731f431497f463f3a2a97236fe0187b11c44aead
Also, defining constants, especially those with human-readable names, is a massive affront to performance! If another developer wants to know that "payload" actually describes the size of the buffer, they should go read the RFC.
Die ganzen Kommentare für den Patch kannst du getrost ignorieren, die sind von irgendwelchen unbeteiligten Personen, die lustig sein wollen.
Und man darf nicht vergessen, dass das der Codeteil relativ unwichtig ist, und deshalb wenig Beachtung findet.
Das war jetzt Ironie (hoffentlich).
-
waruminSSL schrieb:
Warum sollte man ein Keepalive in eine SSL Bibliothek einbauen? Was hat das mit Verschlüsselung zu tun? Ältere OpenSSL Versionen haben auch ohne funktioniert.
Darum geht's nicht. Ich behaupte nicht dass man Keepalives in TLS braucht, oder dass sie super nützlich sind.
Ich behaupte nur dass das Argument "braucht keiner, weil's das ja eh schon in TCP/IP gibt" Blödsinn ist.
-
Memcopy schrieb:
Mal eine Frage.
Werden bei dem Bug von belegtem Speicher Daten kopiert oder nur solcher, der zuvor freigegeben* wurde?
Sowohl als auch.
Es kann sogar auf Speicher zugegriffen werden der nie belegt war, und nichtmal vom OS "gemappt" wurde. Was dann in nem sigsegv/access violation endet, d.h. der Server würde crashen.Es wird einfach das kopiert was hinter dem Empfangspuffer steht - und das kann eben irgendwas sein.
