Ist dieser Login sicher?

Einklappen
X
 
  • Filter
  • Zeit
  • Anzeigen
Alles löschen
neue Beiträge

  • Ist dieser Login sicher?

    Hallo alle miteinander,

    ich hab vor einigen Jahren angefangen, mit PHP herumzuspielen (Betonung liegt auf "spielen), bin aber über gewisse Grundfunktionen nie hinausgekommen.

    Nun habe ich mein erstes, größeres projekt vor der Brust.
    Und zwar arbeite ich als (Hilfs-)Admin bei einer Firma, die Webauftritte erstellt und diese auch hostet.
    Um nun unsere Server zu überwachen setzen wir das Tool-Nagios ein, welches seine Daten per Plugin (auch) in eine MySQL-Datenbank schreibt.
    Nun hätten wir gerne, einfach als nettes Feature, dass jeder Kunde per Weboberfläche den aktuellen Status "seines" Servers einsehen kann. Das ist an sich keine großartig sicherheitskritische Sache, dennoch mache ich mir nun, im Gegensatz zu meinen früheren Spielereien, Gedanken über die Sicherheit, weswegen ich mich zuerst damit beschäftigt habe, wie man einen halbwegs sicheren Login gestalten könnte.

    Das Folgende ist bis jetzt heraus gekommen, es wäre toll, wenn jemand die Zeit fände, einmal drüber zu schauen und mir dann sagt, was ich verbessern, anders machen soll.

    Ich habe das natürlich jetzt nicht einfach ins Blaue programmiert, ich habe mich vorher schon informiert (Stichwort RTFM), aber da ich eben bisher nicht so eine große Ahnung von PHP habe... aber seht selbst:

    Login.php - Login-Formular
    Logout.php - Session killen
    info.php - zu schützende Seite
    auth.inc.php - Prüfung, ob User eingeloggt ist

    Login.php
    PHP-Code:
    <?php
    if ($_SERVER['REQUEST_METHOD'] == 'POST'
    {
        
    session_start();

        
    $db_link mysql_connect("127.0.0.1","root","");
        
    mysql_select_db("nagios");    
        
        
    $username mysql_real_escape_string($_POST['username']);
        
    $password md5(mysql_real_escape_string($_POST['password']));
        
        
    $sql "SELECT username FROM user WHERE username='$username' AND password='$password';";
        
        
    $result mysql_query($sql);
        
        if(
    mysql_num_rows($result) == 1)
        {    
            
    $_SESSION['login'] = true;
            
    $_SESSION['user'] = $username;
            
            
    header('Location: http://127.0.0.1/info.php');
            exit;
            
        }
        else
        {
            
    header('Location: http://127.0.0.1/login/login.php');
            exit;
        }
    }
    ?>
    <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
    <html xmlns="http://www.w3.org/1999/xhtml" xml:lang="de" lang="de">
    <head>
        <title>Geschützter Bereich</title>
    </head>
    <body>
        <form action="login.php" method="post">
        <p>
            Username:<input type="text" name="username" /><br />
            Passwort:<input type="password" name="password" /><br />
            <input type="submit" value="Anmelden" />
        </p>
        </form>
    </body>
    </html>
    logout.php
    PHP-Code:
    <?php
         session_start
    ();
         
    session_destroy();

         
    header('Location: http://127.0.0.1/login/login.php');
         exit;
    ?>
    info.php
    PHP-Code:
    <?php
    include("login/auth.inc.php");

    phpinfo();

    ?>

    <a href="login/logout.php">Logout</a>
    auth.inc.php
    PHP-Code:
    <?php
    session_start
    ();

    if( (!isset(
    $_SESSION['login']) || !$_SESSION['login']) || (!isset($_SESSION['user'])))
    {
        
    header('Location: http://127.0.0.1/login/login.php');
        exit;
    }
    ?>
    Vielen Dank für eure Mühe im voraus.

    Mfg,
    Daniel

  • #2
    Kleine Korrekturen der login.php:
    - der else-Zweig ist überflüssig
    - du prüfst nicht, ob die _POST-Werte überhaupt vorhanden sind
    - du gibst keine Fehlermeldung aus
    - das md5() kannst du MySQL machen lassen

    Schau mal ins Manual, wie man eine Session korrekt zerstört.

    Der Rest scheint in Ordnung zu sein.

    Kommentar


    • #3
      Hi,

      vielen Dank fürs Drüberschauen.

      Fehlermeldungen gebe ich deswegen nicht aus, weils nur für mich zum testen ist. Später, wenn ich das Projekt dann angehe hatte ich allerdings vor, eine vernünftige Fehlerbehandlung vorzusehen

      Den Rest werde ich dann jetzt mal umbasteln.

      Gruß,
      Daniel

      Kommentar


      • #4
        ich habe auch zwei kleinigkeiten.

        include("login/auth.inc.php"); da würde ich require benutzen, falls der include nicht richtig läuft.

        !$_SESSION['login'] kannst du zusätzlich auf den typ prüfen $_SESSION['login']!==true

        sind aber nur marginale verbesserungen

        Kommentar


        • #5
          - das md5() kannst du MySQL machen lassen
          Ja, aber nicht schön, weil dann die Passwörter im Klartext zum SQL Server flutschen. Sind ja nicht immer auf der selben Maschine.
          Das mysql_real_escape.... ist an der Stelle allerdings überflüssig.

          Mit fehlt da die magic_quotes Abhandlung. Das würde das Script erheblich portabler machen.

          Das Session_destroy() ist gut und schön.. Aber man sollte sich das Beispiel in der Doku dazu anschauen, das geht noch etwas weiter.

          Mit häufigem Einsatz von session_regenerate_id() und Aufzeichen der abgelaufenen SIDs lassen sich versehendliche oder gar böswillige Sessionübernahmen erkennen.

          Auf die LocationHeader könnte man auch verzichten..
          Wir werden alle sterben

          Kommentar


          • #6
            Hallo,

            warum ist das "mysql_real_escape_string" überflüssig? Ich hatte gelesen, dass man alle Eingaben, die von aussen kommen, erst prüfen bzw. so umwandeln soll, dass keiner damit Mist bauen kann, und "mysql_real_escape_string" wurde bei Variablen, die in SQL-Queries gehen empfohlen...?

            Die "magic_quotes"-Behandlung werde ich noch einbauen, da habe ich gestern abend auch noch was zu gelesen.

            Die Session beende ich nun so:
            PHP-Code:
            $_SESSION = array();
                 
                 if (isset(
            $_COOKIE[session_name()]))
                 {
                     
            setcookie(session_name(), ''time()-42000'/');
                 }
                 
                 
            session_destroy(); 
            session_regenerate_id() werde ich mir mal anschauen, genauso wie die Aufzeichnung der Session-IDs.

            Warum kann ich auf den Location-Header verzichten? Ich hatte gedacht, es wäre eine gute Methode, um schnelle Weiterleitungen auf die entsprechenden Seiten zu machen?

            Kommentar


            • #7
              Location Header starten einen neuen Requestzyklus. Je nach Internetanbindung und Serverkonfiguration kostet das man schnell mal eben 0.3 Sec(pi mal Daumen). Das ist pure Resourcenverplemperung! Wenn dein PHP im CGI Modus läuft, ist die Angelegenheit sehr aufwändig.
              Du könntest mit include schon hinkommen.

              Ich hatte gelesen, dass man alle Eingaben, die von aussen kommen, erst prüfen bzw. so umwandeln soll, dass keiner damit Mist bauen kann, und "mysql_real_escape_string" wurde bei Variablen, die in SQL-Queries gehen empfohlen...?
              Im Grunde stimmt das!!!
              Aber du schaufelst das Passwort doch erstmal durch md5() und dessen Ergebniss KANN NIE gefährlich sein!!!
              Und das Passwort VOR md5() zu escapen macht auch keinen Sinn. Md5() ist auch nicht zu gefährden. Also an diesem konkreten Punkt überflüssig und damit falsch.
              Wir werden alle sterben

              Kommentar


              • #8
                Ok, dann ist es aber nur zur Hälfte falsch, denn den Usernamen muss ich ja noch escapen, der geht ja nicht in MD5.
                Ich dachte das wäre jetzt generell falsch gewesen, aber die Erklärung mit MD5 ist schon logisch.

                Wie sieht es denn dann mit der magic_quotes-Behandlung aus, muss ich auf das Passwort dann trotzdem "stripslashes" anwenden, bevor ich es im SQL-Query verwende? Oder kann das genauso wie "mysql_real_escape_string()" entfallen?
                Zuletzt geändert von Gazorbeam; 28.10.2007, 16:22.

                Kommentar


                • #9
                  Oder kann das genauso wie "mysql_real_escape_string()" entfallen?
                  Natürlich nicht!
                  Dann würde ja evtl. ein Teil deiner Passwörter, bei einer Serverkonfigurationsänderung ungültig werden.
                  Wir werden alle sterben

                  Kommentar


                  • #10
                    Gut, vielen Dank

                    Kommentar


                    • #11
                      Hallo,

                      ich habe da noch eine Frage, wie oft bzw. wann sollte ich idealerweise "session_regenerat_id()" verwenden?

                      Ich hab mein Skript jetzt so erweitert, dass alte Session-IDs in einer Tabelle gspeichert werden und verglichen wird, ob die ID bereits existierte, wenn das der Fall ist wird die aktuelle Session automatisch beendet und der User ausgeloggt.

                      Gruß,
                      Daniel

                      Kommentar


                      • #12
                        Die ganz paranoiden machen es bei jedem Request. Beim Ein und Ausloggen sollte man es schon tun.
                        Aber nur bei aktivierten Cookies sinnvoll.

                        Wenn die SID per URL übergeben wird, macht man sich damit den F5 Button kaputt. Auch das Nachladen von PHP generierten Bildern usw. wird Probleme bereiten wenn dabei Sessiondaten genutzt werden sollen.
                        Wir werden alle sterben

                        Kommentar


                        • #13
                          Tipp: Wenn du schon boolean Werte in der Session speicherst, dann prüfe auch typensicher darauf
                          PHP-Code:
                          if($var === true)
                          //oder
                          if($var !== true
                          Wenn du das nicht machst könntest du v.a. im Umfeld von register_globals ON in Probleme laufen.

                          Gruss

                          tobi
                          Gutes Tutorial | PHP Manual | MySql Manual | PHP FAQ | Apache | Suchfunktion für eigene Seiten

                          [color=red]"An error does not become truth by reason of multiplied propagation, nor does truth become error because nobody sees it."[/color]
                          Mohandas Karamchand Gandhi (Mahatma Gandhi) (Source)

                          Kommentar

                          Lädt...
                          X