OpenSSL - Heartbleed-Bug

Also echt, dieser Code

+	if (1 + 2 + payload + 16 > s->s3->rrec.length)

...

+	n2s(p, payload);

+	if (1 + 2 + payload + 16 > s->s3->rrec.length)

...

+		unsigned int write_length = 1 /* heartbeat type */ +

+					    2 /* heartbeat length */ +

+					    payload + padding;

Typisches Beispiel, wie man Kommentare schlecht einsetzt. Das gehört zentral in Konstanten verpackt oder sogar in eine Funktion ausgelagert (in C++ hätte man ein Lambda genommen).

Erhard Henkes

Ich finde Online Banking sehr praktisch.

Ja, das ist für Gauner ideal. Ich habe es nie begonnen, da hier - in diesem Forum von Fachleuten! - davon abgeraten wurde. Ich denke, wir müssen die analoge und unbequeme Welt wieder zurück gewinnen, um die Zukunft zu sichern.

Erhard Henkes schrieb:

Ich finde Online Banking sehr praktisch.

Ja, das ist für Gauner ideal. Ich habe es nie begonnen, da hier - in diesem Forum von Fachleuten! - davon abgeraten wurde. Ich denke, wir müssen die analoge und unbequeme Welt wieder zurück gewinnen, um die Zukunft zu sichern.

Wie ich bereits sagte, Online Banking mit Tan Generator oder via Secoder ist sicher.

Lediglich das alte TAN Verfahren, iTAN, mTAN oder HBCI ohne Class 2 Gerät ist als unsicher einzustufen.

Du kannst heutzutage also ruhigen Gewissens die Vorzüge des Online Banking nutzen.

Und was den Thread betrifft, den würde ich mir mal gerne durchlesen.
Link?

Wie immer gibt's ein gutes Comic und so klar verständlich, selbst für nicht Programmierer:
http://xkcd.com/1354/

TyRoXx

Der OpenSSL-Code liest sich als wäre er aus optimiertem Assembly dekompiliert worden. Das beweist wohl, dass dort Experten am Werk sind und deutet auf gute Performance hin.

Sinnvolle Benennung ist total wichtig in einer 289 Zeilen langen Funktion.

static int ssl3_get_record(SSL *s)
	{
	int ssl_major,ssl_minor,al;
	int enc_err,n,i,ret= -1;
	SSL3_RECORD *rr;
	SSL_SESSION *sess;
	unsigned char *p;
	unsigned char md[EVP_MAX_MD_SIZE];
	short version;
	unsigned mac_size;
	size_t extra;
	unsigned empty_record_count = 0;

	rr= &(s->s3->rrec);
	sess=s->session;

Das Benutzen des Präprozessors und printf zum Debuggen zeugt von "besonderer" Qualität.

#if 0
fprintf(stderr, "Record type=%d, Length=%d\n", rr->type, rr->length);
#endif

#ifdef TLS_DEBUG
printf("dec %d\n",rr->length);
{ unsigned int z; for (z=0; z<rr->length; z++) printf("%02X%c",rr->data[z],((z+1)%16)?' ':'\n'); }
printf("\n");
#endif

#if 1
		clear=s->enc_write_ctx?0:1;	/* must be AEAD cipher */
#else
		clear=1;
#endif

Der schnellste Code ist der, der nicht verwendet wird:

#if 0 && !defined(OPENSSL_NO_MULTIBLOCK) && EVP_CIPH_FLAG_TLS1_1_MULTIBLOCK

Kommentare erläutern wichtige Design-Entscheidungen.

for (i=7; i>=0; i--)	/* increment */
					{
					++seq[i];
					if (seq[i] != 0) break; 
					}
				}

Also ich muss schon sagen: Mit Heartbleed hätte niemand rechnen können! So etwas wird sicher nie wieder vorkommen.

Erhard Henkes

http://www.faz.net/aktuell/wirtschaft/netzwirtschaft/nsa-soll-heartbleed-luecke-seit-zwei-jahren-systematisch-ausgenutzt-haben-12892263.html

TyRoXx schrieb:

Der OpenSSL-Code liest sich als wäre er aus optimiertem Assembly dekompiliert worden. Das beweist wohl, dass dort Experten am Werk sind und deutet auf gute Performance hin.

...

Also ich muss schon sagen: Mit Heartbleed hätte niemand rechnen können! So etwas wird sicher nie wieder vorkommen.

Ich wette 10 € dass der Code von Windows XP besser aussieht.

es wird endlich zeit für eine implementierung in c++. alle c libs sind vuln

cya schrieb:

es wird endlich zeit für eine implementierung in c++. alle c libs sind vuln

dabei könnte man auch in C sauber programmieren.
Man muss sich halt mal die Mühe machen und für Datentypen (Strukturen) gewisse Funktionen zurechtzulegen.
Irgendwo im Code ein memcpy ist halt potentiell gefährlich.

Aber, um Euch zu beruhigen: wir entwickeln hier mit C++ und es gibt Kollegen, die verwenden lieber memcpy und diese Dinge, als mal in einen Copy CTOR zu investieren.
Und natürlich gibts an solchen Stellen dauernd Fehler. Selbst Profis machen bei Zeigern, dyn. Speicher und byteweisen Herumkopieren von Daten von Zeit zu Zeit Fehler.
Das einzige was dagegen hilft, ist es, solche Dinge möglichst zu vermeiden.
Und wenn sie sich nicht vermeiden lassen, dann zumindest kapseln.

den Fix in Form von if (1 + 2 + payload + 16 > s->s3->rrec.length) find ich übrigens einen schlechten Scherz.

gfdgfdgdfgdfgd schrieb:

den Fix in Form von if (1 + 2 + payload + 16 > s->s3->rrec.length) find ich übrigens einen schlechten Scherz.

Begründung?

schlechter scherz schrieb:

gfdgfdgdfgdfgd schrieb:

den Fix in Form von if (1 + 2 + payload + 16 > s->s3->rrec.length) find ich übrigens einen schlechten Scherz.

Begründung?

1, 2 und 16 steht für was?
Warum keine aussagekräftigen Namen?
Und warum dann nicht als Konstanten?

Warum keine Klammerung zwecks mehr Übersichtlichkeit, z.B. so:

if ((1 + 2 + payload + 16) > (s->s3->rrec.length))

Fazit:
Dahingeschluderter Müllcode.