Eigenes Login-Script

megustalalluvia · 26.06.2010, 14:53

Hallo zusammen,

ich bin neu hier im Forum und beschäftige mich noch nicht sehr lange mit php. Das erste wirklich nützliche Skript, dass ich programmiert habe war jetzt ein Login.

Meine Frage an die erfahrenen PHPler: Ist dieses Skript so in Ordnung oder gibt es irgendwelche groben Fehler die ich gemacht habe oder Sicherheitsaspekte die ich nicht beachtet habe?

Danke für jeden Kommentar.
mfg, daniel.

login.php: Loginseite mit Überprüfung der Logindaten

PHP-Code:

  <?php

 
//Geschützter Bereich

$safe_page = "intern/index.php";

 
//Session starten

ini_set('session.use_only_cookies', 1);

session_set_cookie_params(1440);

session_name("EBSESSID");    

session_start();

 
//Falls bereits eingeloggt

if ($_SESSION['login'] == true) {

  header('Location: ' . $safe_page);

  exit();

}

 
//Logindaten prüfen

if (isset ($_POST['user_name'],$_POST['user_pass']) )

    {

     //Cookie prüfen

       if ($_COOKIE[session_name()] != session_id()) {

          $_SESSION = array();

          session_destroy();

          if (!isset($_COOKIE[session_name()])) {

            header('Location: login.php?cookie=false');

          } else {

          header('Location: login.php');

          } 

          exit();

        }

        

        //Eingaben aus POST als Hash

        $user_name = $_POST['user_name'];

        $user_hname = hash('sha256', $user_name);

        $user_hpass = hash('sha256', $_POST['user_pass']);

        

        //Username und Passwort aus Datenbank

        $dbarray = file("userdb.dat");

        $user_found = null;

        foreach($dbarray as $user){

          $elemente = explode("|",$user);

          if ($elemente[0] == $user_hname){

            $dbuser_hname = $elemente[0];

            $dbuser_hpass = $elemente[1];

            $user_found = $elemente[2];

            }

          }  

        

        if ($user_hname == $dbuser_hname && $user_hpass == $dbuser_hpass && isset($user_found)){

            $_SESSION['login'] = true;

            $_SESSION['user_clearname'] = $user_found;

            $_SESSION['user_name'] = $user_name;

            header('Location: ' . $safe_page);

        } else {

            //Login falsch

            $_SESSION['login'] = false;

            header('Location: login.php?valid=false');

        }

    }  else {

        if ($_GET['valid'] == 'false') { echo 'Benutzername oder Passwort falsch.<br />'; }

        if ($_GET['cookie'] == 'false') { echo 'Bitte Cookie akzeptieren.<br />'; }

        //Formular

        echo '<form action="login.php" method="post">';

        echo 'User Name: <input type="text" name="user_name"><br />';

        echo 'User Passwort: <input type="password" name="user_pass"><br />';

        echo '<input type="submit" name="Submit" value="einloggen">';

        echo '</form>';  

    }

?>

logout.php: Skript auf das zum Logout verlinkt wird

PHP-Code:

  <?php

  ini_set('session.use_only_cookies', 1);

  session_name("EBSESSID");    

  session_start();

  $_SESSION = array();

  setcookie(session_name(), '', time()-3600,'/');

  session_destroy();

  header('Location: login.php');

?>

secure.php: Zur einbindung in jede Seite des gesperrten Bereiches (mit require() )

PHP-Code:

  <?php

  ini_set('session.use_only_cookies', 1);

  session_name("EBSESSID");    

  session_start();

  if ($_SESSION['login'] != true) {

    header('Location: login.php');

    exit;

  }

?>

ChrisB · 26.06.2010, 15:28

Ständig mittels header wild durch die Gegend zu leiten, finde ich persönlich ziemlich unsauber.

megustalalluvia · 26.06.2010, 15:42

Zitat:

Zitat von ChrisB

Ständig mittels header wild durch die Gegend zu leiten, finde ich persönlich ziemlich unsauber.

Wie würde man das sinnvoller lösen?

Papst · 26.06.2010, 16:35

Direkte Fehlerausgabe oder Fehler sammeln und dann ausgeben zusammen mit dem Formular.
Ggf. kannst du dies auch in eine funktion oder in eine "Authentifiezierungsklasse" kapseln!

T!G3R · 26.06.2010, 16:44

Wenn dein Script funktioniert und du dich nur mit bestimmten Usernamen anmelden kannst, ist dein Script in Ordnung.

Zitat:

Zitat von ChrisB

Ständig mittels header wild durch die Gegend zu leiten, finde ich persönlich ziemlich unsauber.

eigentlich finde ich das nicht so schlimm, aber hier ist es ein bisschen zu übetrieben. Wie schon Papst gemeint hat, eine Fehlervariable einführen, in welcher alle aufgetretenen Fehler gespeichert werden. Du kannst am Ende prüfen, ob es einen Fehler gab. Wenn nicht leitest du auf deine Secureseite um, welche ich übrigens nicht unbedingt in dem Login Script speichern würde, sondern in einer externen variables.inc.php datei o.ä..
Wenn es einen Fehler gibt, gibst du einfach den fehler aus.

PHP-Code:

  $safe_page = "intern/index.php";

//Session starten 
ini_set('session.use_only_cookies', 1); 
session_set_cookie_params(1440); 
session_name("EBSESSID");     
session_start(); 

//Falls bereits eingeloggt 
if ($_SESSION['login'] == true) { 
  header('Location: ' . $safe_page); 
  exit(); 
}

Eine Frage: Warum führst du eine Variable ein, die du gerademal einmal benutzt?

PS: Ich steh ja mehr auf md5() und nicht hash("md5", ...), aber das is Geschmacksache

PhillKill12 · 26.06.2010, 16:50

Zitat:

//Username und Passwort aus Datenbank

lol, also unter Datenbank versteh ich mysql oder oracle, wie auch immer, die solltest du auch benutzen !!

Papst · 26.06.2010, 17:22

Noch etwas gesehen...
[php]
&& isset($user_found)
[php]
ist so falsch, denn $user_found initialisierst du etwas weiter oben mit $user_found = null;

kevbog · 26.06.2010, 23:12

den login script gibt es wesentlich kürzer.
Hast du dort schon sicherheit beachtet?

den logout hätt ich einfach

session_start();
session_destroy();

aber ich kenn mich mit der sicherheit im login selber noch nciht aus kanns nich weiter einschätzen.
will auch nix falsches sagen

Sirke · 26.06.2010, 23:52

Ich will auch mal meinen Senf dazu geben ...

Die schon angesprochene Header Geschichte würde ich auch abändern, indem du bei Erfolg umleitest und jeder Misserfolg/Fehler gesammelt wird und am Ende beim Formular angezeigt wird.
Ich finde deinen Programmierstil unsauber, weil du keine klare Linie beim Einrücken und Klammern einhälst, aber die Lesbarkeit ist ja eher deine Sache...

Dann aber zu den sicherheitsbedenklichen bis sicherheitskritischen Teilen:

PHP-Code:

  $user_hpass = hash('sha256', $_POST['user_pass']);

Diese Zeile um ein Salt (Kryptologie) – Wikipedia erweitern, um von vornherein den Standard zu halten. Positiv anzumerken möchte ich zudem an dieser Stelle SHA-256!

PHP-Code:

  $dbarray = file("userdb.dat");

Diese Zeile wird mit großer Wahrscheinlichkeit bei dir eine Sicherheitslücke sein, weil diese Datei von jedem Client/Browser aus gelesen werden kann, sofern du keine Vorkehrungen dagegen getroffen hast! Sollte das der Fall sein nehme ich meine Kritik natürlich zurück... :P

Zudem nochmal Punkte wie Session Fixation – Wikipedia und Session Hijacking – Wikipedia ansehen und die Funktionen zum Zerstören und Regenerieren der Session ansehen!

...und zu guter Letzt noch eine Performance Anmerkung:

PHP-Code:

  foreach($dbarray as $user) {

    $elemente = explode("|",$user);

    if ($elemente[0] == $user_hname) {

        $dbuser_hname = $elemente[0];

        $dbuser_hpass = $elemente[1];

        $user_found = $elemente[2];

    }

}

Du durchläufst IMMER die gesamte Datei, auch wenn ein Benutzer bereits gefunden wurde und das dann auch noch mit einer langsamen foreach-Schleife, wo du doch gar nicht auf die Keys zugreifen möchtest/brauchst: Also das ganze in eine Funktion kapseln, dann kannst du bei Erfolg rausspringen!

christophk · 27.06.2010, 10:01

Zitat:

Zitat von Papst

Noch etwas gesehen...

PHP-Code:

  && isset($user_found)

ist so falsch, denn $user_found initialisierst du etwas weiter oben mit $user_found = null;

isset prüft nicht nur, ob eine Variable existiert, sondern auch ob sie nicht null ist. Insofern schon okay, wobei die Intialisierung natürlich irgendwie keinen Sinn macht.

Das Perfomance-Problem sehe ich auch, wüsste auf die Schnelle allerdings auch keine Alternative, du könntest per break aber zumindest aus der Schleife rausgehen, wenn du den Benutzer gefunden hast.

Themen-Optionen
Druckbare Version zeigen Jemanden per E-Mail auf dieses Thema hinweisen
Thema bewerten
Thema bewerten:

Ähnliche Themen
Thema	Autor	Forum	Antworten	Letzter Beitrag
Script Login system mit Registrierung+aktivierungsmail	SkyX	PHP Tipps 2010	1	03.02.2010 18:57
Php Login & Register Script	lamopius	PHP Tipps 2009	5	12.01.2010 18:43
Scriptsuche Login script	Meenu	Scriptbörse	4	31.12.2009 17:21
Sicheres Login Script	Emskopp	Scriptbörse	11	26.12.2009 14:28
[Erledigt] Umgehung von Login Script verhindern.	YEP	PHP Tipps 2009	3	29.11.2009 18:27
FREE Login Script für alle	Marti95.	Scriptbörse	24	12.05.2009 15:34
Login Script mit Gruppenrichtlinien?	MatzeMuc86	Scriptbörse	6	04.01.2009 11:29
Login script in loginscript einbinden	Change	PHP Tipps 2008	8	26.08.2008 15:25
[Erledigt] PHP/Mysql login script	killerkretsche	PHP Tipps 2008	24	21.08.2008 15:03
Neues Login Script funzt nicht!	michi174	PHP Tipps 2006	9	31.05.2006 16:42
phpBB Loginscript in eigenes Login Script einbinden	2wuck	PHP Tipps 2007	4	19.12.2005 23:10
login script macht pobs	fadass	PHP Tipps 2007	9	04.12.2005 10:53
Problem mit Login Script		PHP Tipps 2007	4	15.11.2005 17:29
Login Script		PHP Tipps 2005-2	7	02.07.2005 04:53
Eigenes Script archiv script?		Beitragsarchiv	2	20.01.2005 21:39

26.06.2010, 14:53
megustalalluvia Neuer Benutzer Registriert seit: 26.06.2010 Beiträge: 3 PHP-Kenntnisse: Anfänger	Eigenes Login-Script Hallo zusammen, ich bin neu hier im Forum und beschäftige mich noch nicht sehr lange mit php. Das erste wirklich nützliche Skript, dass ich programmiert habe war jetzt ein Login. Meine Frage an die erfahrenen PHPler: Ist dieses Skript so in Ordnung oder gibt es irgendwelche groben Fehler die ich gemacht habe oder Sicherheitsaspekte die ich nicht beachtet habe? Danke für jeden Kommentar. mfg, daniel. login.php: Loginseite mit Überprüfung der Logindaten PHP-Code: <?php //Geschützter Bereich $safe_page = "intern/index.php"; //Session starten ini_set('session.use_only_cookies', 1); session_set_cookie_params(1440); session_name("EBSESSID"); session_start(); //Falls bereits eingeloggt if ($_SESSION['login'] == true) { header('Location: ' . $safe_page); exit(); } //Logindaten prüfen if (isset ($_POST['user_name'],$_POST['user_pass']) ) { //Cookie prüfen if ($_COOKIE[session_name()] != session_id()) { $_SESSION = array(); session_destroy(); if (!isset($_COOKIE[session_name()])) { header('Location: login.php?cookie=false'); } else { header('Location: login.php'); } exit(); } //Eingaben aus POST als Hash $user_name = $_POST['user_name']; $user_hname = hash('sha256', $user_name); $user_hpass = hash('sha256', $_POST['user_pass']); //Username und Passwort aus Datenbank $dbarray = file("userdb.dat"); $user_found = null; foreach($dbarray as $user){ $elemente = explode("\|",$user); if ($elemente[0] == $user_hname){ $dbuser_hname = $elemente[0]; $dbuser_hpass = $elemente[1]; $user_found = $elemente[2]; } } if ($user_hname == $dbuser_hname && $user_hpass == $dbuser_hpass && isset($user_found)){ $_SESSION['login'] = true; $_SESSION['user_clearname'] = $user_found; $_SESSION['user_name'] = $user_name; header('Location: ' . $safe_page); } else { //Login falsch $_SESSION['login'] = false; header('Location: login.php?valid=false'); } } else { if ($_GET['valid'] == 'false') { echo 'Benutzername oder Passwort falsch.<br />'; } if ($_GET['cookie'] == 'false') { echo 'Bitte Cookie akzeptieren.<br />'; } //Formular echo '<form action="login.php" method="post">'; echo 'User Name: <input type="text" name="user_name"><br />'; echo 'User Passwort: <input type="password" name="user_pass"><br />'; echo '<input type="submit" name="Submit" value="einloggen">'; echo '</form>'; } ?> logout.php: Skript auf das zum Logout verlinkt wird PHP-Code: `<?php ini_set('session.use_only_cookies', 1); session_name("EBSESSID"); session_start(); $_SESSION = array(); setcookie(session_name(), '', time()-3600,'/'); session_destroy(); header('Location: login.php'); ?>` secure.php: Zur einbindung in jede Seite des gesperrten Bereiches (mit require() ) PHP-Code: `<?php ini_set('session.use_only_cookies', 1); session_name("EBSESSID"); session_start(); if ($_SESSION['login'] != true) { header('Location: login.php'); exit; } ?>`


Mister Ad PHP Code Flüsterer Registriert seit: 21.08.2005 Beiträge: 4682 PHP-Kenntnisse: Fortgeschritten

26.06.2010, 15:28
ChrisB Moderator¹ Registriert seit: 28.03.2010 Beiträge: 7.470 PHP-Kenntnisse: Fortgeschritten	Ständig mittels header wild durch die Gegend zu leiten, finde ich persönlich ziemlich unsauber. __________________ RGB is totally confusing - I mean, at least #C0FFEE should be brown, right?

26.06.2010, 16:35
Papst Erfahrener Benutzer Registriert seit: 14.10.2007 Beiträge: 306 PHP-Kenntnisse: Fortgeschritten	Direkte Fehlerausgabe oder Fehler sammeln und dann ausgeben zusammen mit dem Formular. Ggf. kannst du dies auch in eine funktion oder in eine "Authentifiezierungsklasse" kapseln!

26.06.2010, 17:22
Papst Erfahrener Benutzer Registriert seit: 14.10.2007 Beiträge: 306 PHP-Kenntnisse: Fortgeschritten	Noch etwas gesehen... [php] && isset($user_found) [php] ist so falsch, denn $user_found initialisierst du etwas weiter oben mit $user_found = null;

26.06.2010, 23:12
kevbog Erfahrener Benutzer Registriert seit: 21.02.2010 Beiträge: 222 PHP-Kenntnisse: Fortgeschritten	den login script gibt es wesentlich kürzer. Hast du dort schon sicherheit beachtet? den logout hätt ich einfach session_start(); session_destroy(); aber ich kenn mich mit der sicherheit im login selber noch nciht aus kanns nich weiter einschätzen. will auch nix falsches sagen

26.06.2010, 23:52
Sirke Erfahrener Benutzer Registriert seit: 04.07.2003 Beiträge: 359 PHP-Kenntnisse: Fortgeschritten	Ich will auch mal meinen Senf dazu geben ... Die schon angesprochene Header Geschichte würde ich auch abändern, indem du bei Erfolg umleitest und jeder Misserfolg/Fehler gesammelt wird und am Ende beim Formular angezeigt wird. Ich finde deinen Programmierstil unsauber, weil du keine klare Linie beim Einrücken und Klammern einhälst, aber die Lesbarkeit ist ja eher deine Sache... Dann aber zu den sicherheitsbedenklichen bis sicherheitskritischen Teilen: PHP-Code: `$user_hpass = hash('sha256', $_POST['user_pass']);` Diese Zeile um ein Salt (Kryptologie) – Wikipedia erweitern, um von vornherein den Standard zu halten. Positiv anzumerken möchte ich zudem an dieser Stelle SHA-256! PHP-Code: `$dbarray = file("userdb.dat");` Diese Zeile wird mit großer Wahrscheinlichkeit bei dir eine Sicherheitslücke sein, weil diese Datei von jedem Client/Browser aus gelesen werden kann, sofern du keine Vorkehrungen dagegen getroffen hast! Sollte das der Fall sein nehme ich meine Kritik natürlich zurück... :P Zudem nochmal Punkte wie Session Fixation – Wikipedia und Session Hijacking – Wikipedia ansehen und die Funktionen zum Zerstören und Regenerieren der Session ansehen! ...und zu guter Letzt noch eine Performance Anmerkung: PHP-Code: `foreach($dbarray as $user) { $elemente = explode("\|",$user); if ($elemente[0] == $user_hname) { $dbuser_hname = $elemente[0]; $dbuser_hpass = $elemente[1]; $user_found = $elemente[2]; } }` Du durchläufst IMMER die gesamte Datei, auch wenn ein Benutzer bereits gefunden wurde und das dann auch noch mit einer langsamen foreach-Schleife, wo du doch gar nicht auf die Keys zugreifen möchtest/brauchst: Also das ganze in eine Funktion kapseln, dann kannst du bei Erfolg rausspringen!