[PHP] Ist dieses Loginscript gut und sicher?

Exterminans · 15 Juli 2013

Das Loginscript ist so NICHT sicher.

Bzw. das Script an sich ist soweit sicher, als dass in der Fassung von accC zu mindestens SQL-Injections ausgeschlossen sind, aber das Script ist so nicht funktional.

Was hindert mich daran, als Außenstehenden direkt die URL "https://meineseite.de/intern/index.php" auf zu rufen?
Effektiv nichts.

Wenn der User eingeloggt ist, dann hinterlege in der Session einen Schlüssel der einen Wert ungleich false/0/null/"" hat (sprich nicht nach false evaluiert) und überprüfe in jedem gesicherten Script ob dieses Flag bereits gesetzt ist.
Ist das Flag nicht gesetzt, so musst du den Benutzer auf die Login-Maske weiterleiten und das Script per exit() terminieren. Diese Logik packst du am besten in ein separates Script, und includierst dieses am Anfang jeder zu sichernden Datei mit dem Befehl require_once(). Dieser Befehl sorgt dafür, dass das Script nur genau ein einziges mal includiert wird und sollte das Script nicht auffindbar sein, so terminiert PHP mit einem Fehler. ( include_once() hingegen wirft nur eine Warnung und PHP läuft weiter.)

Für einen Logout kannst du entweder die Session zerstören oder einfach das Flag entfernen.

In einer modernen Webanwendung hättest du typischerweise nur noch ein einzige PHP-Datei die überhaupt direkt zugänglich ist und könntest diesen Test damit einmalig schreiben und er wäre universell gültig, aber in deinem Fall musst du jede PHP-Datei die direkt über eine URL angesprochen werden kann entsprechend absichern.

PS: Die Erklärung von accC ist nicht ganz zutreffend. Ausgerechnet sein Beispiel funktioniert so nicht. Die Anweisung mysql_query() führt nur einen einzigen Query aus, auch wenn mehrere Querys per ; verknüpft wurden. Die Schwierigkeit ist jedoch eine andere, und zwar könnte der User als Namen einfach folgendes angeben:

Code:

' OR TRUE;

Das ist so kurz wie effektiv und quasi der Generalschlüssel für deine Anwendung.

Schlimmeres ist in diesem speziellen Fall nicht möglich, du führst nur einen SELECT aus und verarbeitest die Daten anschließend nicht weiter. Allerdings schaut dies anders aus, wenn du die aus der Datenbank extrahierten Daten in irgendeiner Form ausgibst. Über die UNION-Klausel kann ein Angreifer in so einem Fall beliebige Daten aus anderen Tabellen holen und diese zusammen mit den Original-Daten ausgeben lassen, das könnten z.B. die Emailadressen und Passworthashes aller User sein. Innerhalb eines UPDATE-Befehles kann der User sogar noch deutlich mehr Schaden anrichten...

accC · 15 Juli 2013

Dass er in den anderen Dateien natürlich noch nachschauen muss, ob die Session gültig ist (wobei ich hier nicht unbedingt nur einen beliebigen zu true evaluierenden Ausdruck, sondern eher einen vergleichbaren Wert setzen würde) habe ich mal voraus gesetzt. Ich denke, den Rest wird er sich dann noch zusammen reimen können. Sollte ja nur eine Minimalversion sein, damit er nachvollziehen kann, was wo passiert.

Mit dem mysql_query hast du natürlich recht, es wird nur ein Query ausgeführt.
Allerdings habe ich bei dem Beispiel auch nicht wirklich darauf geachtet, was genau ich da überhaupt query'e, ich wollte nur schnell etwas nachvollziehbares zusammen basteln.

Kugelfisch · 15 Juli 2013

Exterminans schrieb:
Schlimmeres ist in diesem speziellen Fall nicht möglich, du führst nur einen SELECT aus und verarbeitest die Daten anschließend nicht weiter. Allerdings schaut dies anders aus, wenn du die aus der Datenbank extrahierten Daten in irgendeiner Form ausgibst.

Auch wenn das bloss noch entfernt mit dem Thema zu tun hat - das ist so nicht korrekt. Zwar kann man nicht trivial Daten ausgeben lassen, allerdings liefert jede Abfrage ein Bit an Information zurück: die Information, ob die Abfrage genau eine Zeile liefert oder nicht. Man könnte das ausnutzen, um beliebige Inhalte bitweise aus der Datenbank auszulesen. Selbst komplett ohne Ausgabe wäre das noch möglich, über zeitbasierte Angriffe. Das dauert zwar länger und der Angriffsvektor eignet sich kaum, um einen kompletten Datenbank-Dump herunterzuladen, kleinere Mengen sensibler Informationen auszulesen, etwa den Passwort-Hash eines bestimmten Benutzers, ist aber sehr realistisch.

Cyperfriend · 15 Juli 2013

Ach Leute. Ich fühle mich direkt zu Hause. Alles wie früher. Ich mache einen Thread auf und stelle eine einfache Frage und poste etwas Code. Daraus wird dann direkt eine Wissenschaft und jeder hilft wo er kann, aber ich selbst verstehe am Ende gar nichts mehr mehr. Das soll keinesfalls negativ sein. Ich finde es toll, dass ihr euch so engagiert, aber ich verstehe es jetzt echt nicht mehr. Ich verstehe oft nicht wie ich das was ihr sagt umsetzen muss.

Wie gesagt: Ich denke das kommt daher, dass ich zu wenig Ahnung davon habe wie PHP / MySQL die Daten intern verarbeitet. Schlimmer noch habe ich keine Ahnung wie Angriffsszenaien in der Praxis aussehen und kann es daher nicht selbst testen. Ich bin halt ein artiger User der nichts manipuliert und rumhackt, sondern die Scripte so nutzt wie sie gedacht sind.

Eigentlich wollte ich den Code von accC heute testen und euch mit Fragen dazu löchern, da mir einige Zeilen unklar sind, aber jetzt habe ich ja gelesen, dass der Code so auch nicht der Weisheit letzter Schuss ist.

Ich tu mich auch etwas schwer damit euch zu bitten mein Script so umzuschreiben, dass es sicher und funktional ist und dabei die Änderungen zu kommentieren und auch dazu zu schreiben, WARUM es geändert wurde. Ich denke aber es wäre das Beste um zu verstehen, warum mein Code so nicht verwendet werden darf.

Um dennoch kurz auf das Script von accC einzugehen: Warum muss das so stehen

PHP:

if($query && mysql_num_rows($query)==1)

und es reicht nicht

PHP:

if(mysql_num_rows($db_result) > 0){

Gut, es stimmt. Im Regelfall darf es nur ein Ergebnis geben, also könnte man auch

PHP:

if(mysql_num_rows($db_result) ==1){

machen, aber wozu hat er das $query am Anfang stehen?

Eine andere Zeile die mich irritiert ist, warum aus der $_POST['benutzername']-Variable erst eine normale Variable gemacht wird?

PHP:

$benutzer = mysql_real_escape_string($_POST['benutzername'], $dbhandle);

Könnte man das weiter unten nicht direkt in die Session einbauen? Ihr habt oben geschrieben, dass alles was [] hat von PHP als Array interpretiert wird und somit manipulierbar wäre. Das trifft aber auf die genannte Zeile genauso gzu, als wenn man es weiter unten macht, wo der Benutzername in die Session wandert?

Das sind so die Punkte, bzw. Gedanken die ich gerade habe.

Exterminans · 15 Juli 2013

Cyperfriend schrieb:
Um dennoch kurz auf das Script von accC einzugehen: Warum muss das so stehen

PHP:

if($query && mysql_num_rows($query)==1)

und es reicht nicht

PHP:

if(mysql_num_rows($db_result) > 0){

Gut, es stimmt. Im Regelfall darf es nur ein Ergebnis geben, also könnte man auch

PHP:

if(mysql_num_rows($db_result) ==1){

machen, aber wozu hat er das $query am Anfang stehen?

Sollte der Query fehl geschlagen sein (aufgrund irgend eines Fehlers), so liefert mysql_query kein Ergebnis, sondern den Wert "false" zurück. Rufst du mysql_num_rows() dann nicht mit einem gültigen Ergebnis sondern mit "false" als Parameter auf, gibt es gleich den nächsten Fehler.

Cyperfriend schrieb:
Eine andere Zeile die mich irritiert ist, warum aus der $_POST['benutzername']-Variable erst eine normale Variable gemacht wird?

PHP:

$benutzer = mysql_real_escape_string($_POST['benutzername'], $dbhandle);

Könnte man das weiter unten nicht direkt in die Session einbauen? Ihr habt oben geschrieben, dass alles was [] hat von PHP als Array interpretiert wird und somit manipulierbar wäre. Das trifft aber auf die genannte Zeile genauso gzu, als wenn man es weiter unten macht, wo der Benutzername in die Session wandert?

Jein, so ganz stimmt das nicht...
Ja, $_POST['benutzername'] kann anstelle eines Strings tatsächlich ein Array sein, wenn jemand den Namen des Felds im Formular von "benutzername" auf "benutzername[]" geändert hat. Das wird allerdings behoben, sobald das Feld in irgend einer Weise in einen anderen Datentyp, in diesem Fall konkret in einen String konvertiert wird. Das passiert automatisch wenn das Feld an eine Funktion übergeben wird, welche einen String als Parameter erwartet.

Das was accC da gemacht hat ist allerdings trotzdem sinnlos, in diesem Fall sogar doppelt. Es gibt KEINEN Grund den Benutzernamen überhaupt vor dem Speichern in der Session zu escapen, sogar noch weniger doppelt escaped.

PHP:

$_SESSION['benutzername'] = htmlspecialchars(mysql_real_escape_string($_POST['benutzername']));// Sinnlos / Falsch
$_SESSION['benutzername'] = (string)$_POST['benutzername']; // Richtig

Das einzige was notwendig ist, ist sicher zu stellen dass es sich tatsächlich um einen String handelt, das kann am einfachsten mit einem sogenannten "Cast" erreicht werden, in PHP funktioniert das indem du den gewünschten Datentyp in runde Klammern setzt und vor die Variable schreibst. Siehe auch: http://de2.php.net/manual/en/language.types.type-juggling.php#language.types.typecasting

Damit liegt dann allerdings potentiell gefährlicher Inhalt in der Session, daher noch mal die ursprüngliche Aufforderung: Daten immer erst unmittelbar vor der Ausgabe escapen. Nicht in escapter Form speichern, sondern immer davon ausgehen, dass die Datengefährlich sind, die Speicherung erfolgt grundsätzlich in Reinform. Du escapest jeweils nur genau so viel wie es im jeweiligen Kontext notwendig ist, sprich htmlspecialchars() unmittelbar vor der Ausgabe im HTML-Kontext, mysql_real_escape_string() unmittelbar vor einer SQL-Abfrage etc.

Was sonst noch zu erwähnen wäre, du machst noch einen anderen Anfängerfehler. Du solltest an dieser Stelle nicht den Namen aus den POST-Daten in die Session übernehmen. Der Grund, du hast in der Datenbank bereits einen Benutzernamen hinterlegt welches als Referenz dienen sollte. Daher solltest du nur einen Abgleich durchführen ob die Benutzerdaten gültig sind, anschließend allerdings Benutzer-ID und Name aus der Datenbank in die Session laden. Damit reduzierst du das Risiko, dass ein Benutzer über seinen Benutzernamen beim Login potentiell schädliche Inhalt einschleppt, je nach gewählter Kollation in der Datenbank kann sich der übergebene Text nämlich schon ziemlich deutlich vom hinterlegten Benutzernamen unterscheiden und wird trotzdem als "identisch" akzeptiert.

Wenn du es z.B. in Browsergames irgendwo siehst, dass der Name von Benutzern in zwei Schreibweisen auftaucht (einmal in "sauberer" Form in Ranglisten und Co. und einmal mit Akzenten, Caps und dergleichen z.B. in Chats), dann hat dort der Entwickler genau den gleichen Fehler gemacht und die "exotische" Formatierung ist das was der User beim Login angegeben hat, die andere Schreibweise stammt von der Registrierung. Das was bei der Registrierung verwendet wurde, sollte immer bindend sein.

accC · 15 Juli 2013

@ Exterminans:

Das was accC da gemacht hat ist allerdings trotzdem sinnlos, in diesem Fall sogar doppelt. Es gibt KEINEN Grund den Benutzernamen überhaupt vor dem Speichern in der Session zu escapen, sogar noch weniger doppelt escaped.

Tatsache, an der Stelle $benutzer zu verwenden war unnötig, man kann natürlich auch $_POST['benutzername'] nehmen. Das würde ausreichen.

Was sonst noch zu erwähnen wäre, du machst noch einen anderen Anfängerfehler. Du solltest an dieser Stelle nicht den Namen aus den POST-Daten in die Session übernehmen. Der Grund, du hast in der Datenbank bereits einen Benutzernamen hinterlegt welches als Referenz dienen sollte. Daher solltest du nur einen Abgleich durchführen ob die Benutzerdaten gültig sind, anschließend allerdings Benutzer-ID und Name aus der Datenbank in die Session laden. Damit reduzierst du das Risiko, dass ein Benutzer über seinen Benutzernamen beim Login potentiell schädliche Inhalt einschleppt, je nach gewählter Kollation in der Datenbank kann sich der übergebene Text nämlich schon ziemlich deutlich vom hinterlegten Benutzernamen unterscheiden und wird trotzdem als "identisch" akzeptiert.

Das kommt dann aber auch stark darauf an, was ich überhaupt in die Datenbank lege, beim Registrieren. Ob ich jetzt beim Eintragen in die Datenbank darauf achte, dass dort nur ein "sauberer Name" steht und den später verwende oder beim Vergleich in der Datenbank und einen "bösen Namen" beim Login ablehne, macht mMn keinen besonderen Unterschied. Eventuell was Laufzeit betrifft, wenn man wirklich wirklich wirklich viele Abfragen generiert. Zumal ein Benutzername mit Accents jetzt nicht unbedingt als "böse" oder "gefährlich" einzustufen ist, oder übersehe ich da jetzt was?

Lex · 15 Juli 2013

Ich würde generell die Handhabung von $_SESSION nicht dem User alleine überlassen.
Schreibe die Session-ID in eine Datenbank, zusammen mit IP, Browser, Gültigkeit usw, und gleiche bei jedem Seitenaufbau diese Informationen ab, bevor dem User den Content zur Verfügung stellst. Wie schnell man Cookies oder Sessions Hijacken kann, hat man ja bei TOG damals gesehen.

Lg Lex

Rakorium-M · 15 Juli 2013

Um nochmal auf das bereits angesprochene mysqli zurückzukommen:
Das lässt sich in php genauso bedienen wie mysql, einfach aus allen mysql_... ein mysqli_... machen. Mysqli hat (unter anderem) den Vorteil, dass es deutlich schneller ist.

drfuture · 15 Juli 2013

Im Grunde hätte ich nun den zusammengefassten Infos hier auch nichts mehr hinzu zu fügen - @TS Das ändern deines Codes macht ja nun denke ich nicht mehr so viel sinn - könntest du noch mal "deinen" Code nun so posten wie du ihn schreiben möchtest nach den ganzen Hinweisen - und evtl. direkt im Code per Kommentar markieren wenn noch offene Fragen sind

Shodan · 15 Juli 2013

@Lex: Der User hat lediglich die sessionID, alles andere liegt doch sowieso auf dem Server.

Es macht in der Regel wenig Sinn diese Daten zusätzlich noch in einer DB zu speichern. (Stichwort: zusätzlich! wenn überhaupt, dann schon stattdessen)
Sinnvoll wäre es zum Beispiel:
- Wenn du dem Dateisystem des Servers, auf dem dein PHP läuft, nicht vertraust. Das könnte zum Beispiel der Fall sein, wenn einen günstigen Webspace nutzt und dir einen schlecht konfigurierten Server mit anderen Seiten teilst. Ich würde dann aber eher empfehlen sich einen besseren Hoster zu suchen.
- Wenn deine Anwendung über mehrere Server verteilt ist.

Zum Speichern der IP:
Kann man machen, hat aber auch Probleme: ist der User zum Beispiel hinter einem load balancing proxy wechselt seine IP möglicherweise mit jeder Anfrage.

Was man sonst noch so tun kann:
Dem Login einen Logout gegenüberstellen, der die Session vernichtet. (Benutz ich persönlich HIER aber nie, NGB kickt mich immer per timeout :rolleyes:

)
Ist der geschützte Bereich kritisch, User mit dicken roten Warnhinweisen darauf aufmerksam machen, dass sie sich ausloggen sollen, wenn sie das beim letzten mal nicht getan haben.
Hilft gegen Leute die sich von öffentlich zugänglichen Rechnern aus einloggen. Vielleicht... mit viel Glück...

In der Ini:
session.use_only_cookies = 1
session.use_trans_sid = 0
Keine IDs in URLs -> DAUs geben keine URLs mit IDs weiter. (Die meist verbreitete Form des "Forenaccounthackings" in IRC Kanälen)
Das sind inzwischen defaults, je nachdem wo dein Server herkommt (*hust* günstiger Webspace *hust*) sollte man das aber vielleicht prüfen.

session.cookie_httponly = 1
Das fordert den Client auf seinen Keks nicht an Scripte weiterzugeben und stellt XSS Angriffen auf die Session eine nette Hürde in den Weg (was natürlich nicht heißt, dass man diese tolerieren kann). Hilft allerdings nicht gegen User mit extrem veralteten Browsern und es kann auch immer noch Möglichkeiten geben dem User die Kekse zu klauen.
Ist ab 5.3 auch default, siehe oben.

session_regenerate_id();
Beim Login. Hilft gegen Fixation Attacken. Wer sich einloggt bekommt eine neue ID. Gut Fixation ist schwierig bei cookie only, aber wer weiß, vielleicht hat der User ja noch einen alten Keks (igitt) oder man hat ihm einen unter geschoben o.O
(öffentlicher / nicht öffentlicher Bereich. XSS Schwachstelle Übersehen, Hacker im Öffentlichen, User benutzt IE6 (ohne httponly) oder die Schwachstelle kommt daran vorbei -> Hacker hat die alte ID, User die neue)

Der Login sollte, wenn möglich, über SSL laufen, Passwörter gehören nicht im Klartext übertragen. Ist SSL schlicht nicht drin, vorher mindestens mit Javascript hashen ;-)
Leute neigen dazu Passwörter wiederzuverwenden, lauscht jemand an der Leitung ist der User nicht nur in deiner Anwendung, sondern vielleicht sogar überall kompromittiert.

Wenn notwendig auch alles hinter dem Login nur per SSL erreichbar machen. In dem Fall kann man den Cookie dann auch noch auf secure setzen, dann wird er nur bei https-Aufrufen übertragen und kann auf dem Weg auch nicht geklaut werden (fiese Keksediebe sind mancheinmal in öffentlichen Netzwerken zu finden, zB im WLan des Straßencafes).

Wir kommen damit dann langsam in den Bereich "Angreifer hat SSL gebrochen oder den Rechner des Users oder schlimmer den Server erobert".
In wie weit dann ein IP (oder ganz eklig SSL Session) Binding noch helfen ist fragwürdig, ein ordentliches Logging und ein Anruf bei der Polizei wären zu dem Zeitpunkt vielleicht angebrachter.
(Sofern "Logging von vor Gericht verwertbaren Informationen" in deinem Projekt angebracht ist ;-)

Lex · 16 Juli 2013

Shodan schrieb:
@Lex: Der User hat lediglich die sessionID, alles andere liegt doch sowieso auf dem Server.

Reicht doch für einen Hijack.

Shodan schrieb:
Zum Speichern der IP:
Kann man machen, hat aber auch Probleme: ist der User zum Beispiel hinter einem load balancing proxy wechselt seine IP möglicherweise mit jeder Anfrage.

Wer will denn solche User? Die haben dann mal plum gesagt "Pech gehabt"

Shodan schrieb:
session_regenerate_id();
Beim Login. Hilft gegen Fixation Attacken. Wer sich einloggt bekommt eine neue ID. Gut Fixation ist schwierig bei cookie only, aber wer weiß, vielleicht hat der User ja noch einen alten Keks (igitt) oder man hat ihm einen unter geschoben o.O
(öffentlicher / nicht öffentlicher Bereich. XSS Schwachstelle Übersehen, Hacker im Öffentlichen, User benutzt IE6 (ohne httponly) oder die Schwachstelle kommt daran vorbei -> Hacker hat die alte ID, User die neue)

Sollte sowieso benutzt werden, um das Hijacken der Session zu erschweren. Wird bei mir z.B. bei jedem Seitenaufruf aufgerufen.

Shodan schrieb:
Der Login sollte, wenn möglich, über SSL laufen, Passwörter gehören nicht im Klartext übertragen. Ist SSL schlicht nicht drin, vorher mindestens mit Javascript hashen ;-)
Leute neigen dazu Passwörter wiederzuverwenden, lauscht jemand an der Leitung ist der User nicht nur in deiner Anwendung, sondern vielleicht sogar überall kompromittiert.

Wenn notwendig auch alles hinter dem Login nur per SSL erreichbar machen. In dem Fall kann man den Cookie dann auch noch auf secure setzen, dann wird er nur bei https-Aufrufen übertragen und kann auf dem Weg auch nicht geklaut werden (fiese Keksediebe sind mancheinmal in öffentlichen Netzwerken zu finden, zB im WLan des Straßencafes).

Wir kommen damit dann langsam in den Bereich "Angreifer hat SSL gebrochen oder den Rechner des Users oder schlimmer den Server erobert".
In wie weit dann ein IP (oder ganz eklig SSL Session) Binding noch helfen ist fragwürdig, ein ordentliches Logging und ein Anruf bei der Polizei wären zu dem Zeitpunkt vielleicht angebrachter.
(Sofern "Logging von vor Gericht verwertbaren Informationen" in deinem Projekt angebracht ist ;-)

Sehr gut formuliert. Hoffe damit können alle etwas anfangen

Lg Lex

Cyperfriend · 16 Juli 2013

Ich versuche das jetzt erstmal alles zu verstehen und in der Praxis (=in einem Script) zu verarbeiten und was dann dabei rauskommt hier zu posten.

https ist bei mir leider nicht möglich.

Cyperfriend · 16 Juli 2013

OK, hier nun das Script.
Zunächst einige Anmerkungen:
1) HTTPS ist bei mir nicht möglich
2) Für das Passwort habe ich der Einfachheit halber noch md5 genommen.
3) Klammeraffe (@) habe ich nicht verwendet da ich, zumindest in der Entwicklung ja noch Fehlermeldungen sehen will, sofern welche auftreten.
3) Ich hoffe das Script stellt jetzt eine vernünftige Basis dar.

PHP:

<?php
# Diese Datei enthält Funktionen, welche für das Script zwingend benötigt werden.
require_once("functions.inc.php");
# Diese Datei enthält die Fehlercodes.
include("codes.php");

# Wenn sowohl Benutzername als auch Passwort im Login-Formular eingetragen worden sind.
if(!empty($_POST['benutzername'])  && !empty($_POST['passwort'])) {
	#Session starten.
	session_start();
	# Datenbankverbindung herstellen
	$db_connect = db_connect();
	# Nach Benutzernamen und Passwort in der Datenbank suchen.
	# Die ID brauchen wir später für die Session, daher wird sie ebenfalls ausgelesen.
	$db_read = "select id, benutzername, passwort from db_benutzer 
				where (benutzername='".mysql_real_escape_string($_POST['benutzername'])."') && (passwort='".md5($_POST['passwort'])."')";
	$db_result = mysql_query($db_read, $db_connect) or die (mysql_error());

	# Wenn eine Übereinstimmung gefunden wurde SESSION setzen und in den internen Bereich weiterleiten
	if(mysql_num_rows($db_result) == 1){
		# ID zum Benutzer aus der Datenbank auslesen.
		$res = mysql_fetch_array($db_result);
		# Warum kann ich das nicht direkt in den update-String schreiben? Gibt ein unexpected ;
		$id = htmlspecialchars($res['id']);
		# IP
		$ip = htmlspecialchars($_SERVER['REMOTE_ADDR']);
		# Session-ID erzeugen.
		$sid = htmlspecialchars(session_id());
		# Zeit festlegen, wann die Session ablaufen soll, wenn Inaktivität (900 Sekunden == 15 Minuten)
		$term = htmlspecialchars(time() + 900);
		# Verifikationsdaten in die Datenbank schreiben
		$db_write = "update db_benutzer set sid='".mysql_real_escape_string($sid)."', 
											ip='".mysql_real_escape_string($ip)."', 
											term='".mysql_real_escape_string($term)."' 
										where id='".mysql_real_escape_string($id)."'";
		$db_result = mysql_query($db_write, $db_connect) or die (mysql_error());
		$_SESSION['sid'] = $sid;
		$_SESSION['ip'] = $ip;
		$_SESSION['term'] = $term;
		$_SESSION['code'] = 0;
		header ("Location:  http://".$_SERVER['HTTP_HOST']."/neu/index.html");
	}
	else{
		# Wenn dier Login falsch war
		# -> Umleitung zum Login
		$_SESSION['code'] = 1;
		header ("Location: http://".$_SERVER['HTTP_HOST']."/neu/index.html");
	}
}
else{
	# Wenn das Loginscript direkt aufgerufen wird oder wenn die Felder Benutzername und Passwort leer abgeschickt werden
	# -> Umleitung zum Login
	header ("Location: http://".$_SERVER['HTTP_HOST']."/neu/index.php");
}
?>

Krutius · 16 Juli 2013

Hoi

Naja, im Falle von aktivem JS könntest du Clientseitig noch hashen, wenn du kein HTTPs zur Verfügung hast.

Zum hashing ganz allgemein, hier ein kleiner Text:
http://crackstation.net/hashing-security.htm

Was du auch machen kannst, zusätzlich noch, ist vom server ne challenge generieren (zufalls-bytes) und dann PW+ diese zusammenhashen, und das Ergebnis wieder an den Server schicken. Da kannst du das dann nochmal machen, und vergleichen.
Der Vorteil ist, das man sich selbst wenn man die Kommunikation abgefangen hat, sich nicht erneut einloggen kann. Einzige Möglichkeit wäre eine Man-in-the-middle attack, was aber nochmals etwas aufwendiger ist. Das könnte man nur mit HTTPS verhindern!

Das Problem ist das du das PW in der DB speichern müsstest, was ja auch nicht geht. Von daher musst du davor noch einen vorgelagerten, ersten hash zyklus haben.
Was dann noch bleibt, ist die Möglichkeit des Benutzers sich mit den Daten in der DB am System anzumelden, was bei einer geklauten DB Problematisch sein kann, da der Angreifer auch später zugriff erhält ohne die Notwendigkeit das Passwort zu ändern. Um dies jetzt auch noch zu verhindern, muss man beides kombinieren.

Also in etwa so (Pseudocode):

PHP:

// zuerst mal die Daten(salt1, salt2, salt3, challange) für den User vom Server holen, dann in JS:

virtualPW = HASH(PW + salt1) // erlaubt auch beim neusetzen eines PWs nur den hash übers netz senden zu müssen, da auch für die serverseitige erstgenerierung der Daten nur das virtualPW nötig ist.

publicHash1 = HASH(virtualPW + salt2) //idee: public hash 1 geht niemals über netzwerk
publicHash2 = HASH(virtualPW + salt3) //idee: public hash 2 geht niemals in die DB

challengeVerification = HASH(publicHash1 + challenge);

// Jetzt an server senden: publicHash2, challengeVerification

//Da jetzt

IF HASH(publicHash2 + salt4) == secureDbHash AND challengeVerification == HASH(publicHash1 + challange) THEN alles OK

Dabei ist zu sagen, dass die challange für jeden loginversuch eine andere sein muss, und dann auch nur kurz valide sein sollte.

in der DB steht dann etwa:
salt 1, salt2, salt3, salt4, secureDbHash, publicHash1

Naja, glaub das ist so das sicherste was man ohne HTTPS erreichen kann!
Man-in-the-middle attack bleibt aber nach wie vor eine Möglichkeit!

Und natürlich, wenn kein JS da ist, brauchts einen unsicheren Fallback, bei dem das PW im Klartext übertragen wird!

Hier gibts übrigens passende JS cryptofunktionen.

Und naja, niemals MD5 einsetzen, SHA256 wär vermutlich nicht falsch.

Das alles wird aber auf jeden Fall ein bisschen komplexer!

Mfg

Krutius

Cyperfriend · 16 Juli 2013

Ich hatte vergessen zu erwähnen, dass ich JavaScript komplett ablehne. Ich hasse diese Scriptsprache regelrecht.
Ich baue auch kein Fort Nox oder eine Bankseite. Natürlich kann man so ein Loginsystem bin ins kleinste Detail verfeinern bis hin zum SMS Versand und weiter.

Können wir uns vielleicht darauf einigen, dass das Script Standardangriffen standhalten soll. Was man da ja immer wieder hört ist SQL-Injektion und XSS. Wenn es wirklich einer drauf abgesehen hat ist denke ich eh alles verloren und ich sag mal der Board Login hier kommt ja auch ohne HTTPS und JavaScript aus. Ich möchte mich am ende nicht in Details verlieren.

Krutius · 16 Juli 2013

Wie hier zu sehen ist, auch hier gibt es zumindest rudimentäres JS-Hasing!

JS bietet dir einfach im Web Vorteile, da du Sachen ohne den Server-roundtrip machen kannst.

Und, was ist ein Standardangriff?
Ein HTTP-Sniffer ist schnell installiert!

Cyperfriend · 17 Juli 2013

Mag sich noch jemand ansehen, was man am Script ohne JavaScript noch ändern kann? Wie gesagt: Der Login dieses Forums kommt auch ohne https und JavaScript aus.

Denke sicherer als so ein Forum muss meine Anwendung am Ende auch nicht sein.

Wo stehe ich den momentan mit dem Script? SQL Injektion ausgeschlossen? XSS ausgeschlossen? Leicht zu hacken, mit mäßigem Aufwand zu hacken, mit größerem Aufwand zu hacken wenn man es darauf abzielt?

drfuture · 17 Juli 2013

Ich würde noch die Zeilen hinzufügen:

PHP:

include("codes.php");
$_POST['benutzername'] = (string)$_POST['benutzername'];
$_POST['passwort'] = (string)$_POST['passwort'];
# Wenn sowohl Benutzername als auch Passwort im Login-Formular eingetragen worden sind.
if(!empty($_POST['benutzername'])  && !empty($_POST['passwort'])) {

Und du solltest dir eben überlegen mysqli_ statt mysql_ zu verwenden - dürfte aber in diesem Fall keine auswirkungen haben.

Cyperfriend · 17 Juli 2013

Hab ich dazugefügt.

Shodan · 18 Juli 2013

@Cyperfriend

1: Ok ich verstehe ja Paradigmen wie "besser einmal zu viel wie zu wenig" und "trust no one" aber:
$term = htmlspecialchars(time() + 900) und mysql_real_escape_string($term)? Ernsthaft?

Wenn überhaupt, dann:

PHP:

$timeout = time() + 900;
if ($timeout != mysql_real_escape_string(htmlspecialchars($timeout)) {
	catastrophLog("time() returned a malicious string!!!11elf");
	callAdminOnPrivatePhone();
	setServerOnFire();
}

1b:

Exterminans schrieb:
Daten immer erst unmittelbar vor der Ausgabe escapen. Nicht in escapter Form speichern, sondern immer davon ausgehen, dass die Daten gefährlich sind, die Speicherung erfolgt grundsätzlich in Reinform. Du escapest jeweils nur genau so viel wie es im jeweiligen Kontext notwendig ist, sprich htmlspecialchars() unmittelbar vor der Ausgabe im HTML-Kontext, mysql_real_escape_string() unmittelbar vor einer SQL-Abfrage etc.

2: Speichern der SID, des Timeouts und der IP in einer DB.
Zum zweiten mal: Das ist sinnfreier Overhead, es sei denn deine Anwendung verteilt sich über mehrere Server.
Entweder dein PHP Server speichert deine Sessions an einem sicheren Ort, oder dein PHP Server ist Schrott.
Und wenn du letzteres wirklich befürchtest, dann brauchst du ein paar ganz andere Dinge wie zB. ini_set... oder besser noch einen anderen Server.
Falls du das Session Handling wirklich selber übernehmen willst, dann sei auch konsequent und schreib einen ganzen SessionHandler ;-)
Oder hast du etwa wirklich vor callAdminOnPrivatePhone(); zu implementieren und schaffst Redundanzen um das Session Handling deines PHP Servers auf Angriffe zu prüfen?

3:

PHP:

...
	if(mysql_num_rows($db_result) == 1){
		# SID Fixation unterbinden. Sicherheitslevel erhöhen -> neue SID vergeben.
		session_regenerate_id();
		# ID zum Benutzer aus der Datenbank auslesen.
...

4: ... or die (mysql_error());
Das ist für ein kleines Script ok. Generell empfiehlt es sich aber mal einen Blick in Exceptions und Logging zu werfen.

5: Klartext Passwörter. "Andere machen das auch" akzeptiere ich nicht als Argument für bad practice. :P

[PHP] Ist dieses Loginscript gut und sicher?

Neu angemeldet

gesperrt

Nerd

Der ohne Avatar

Neu angemeldet

gesperrt

NvT Terrorist

NGBler

Zeitreisender

runs on biochips

NvT Terrorist

Der ohne Avatar

Der ohne Avatar

Verrückter

Der ohne Avatar

Verrückter

Der ohne Avatar

Zeitreisender

Der ohne Avatar

runs on biochips