Keskustelu: Nettisivujen teko: Kirjautuminen/rekisteröityminen

Terve. Olen aika aloittelija PHP:n kanssa ja olen kirjoitellut sivuilleni vähän PHP:ta. Minulla on nyt käytössä kirjautumis/rekisteröitymis-skriptat. Haluaisin vähän teidän ammattilaisten kommenttia niistä. Lähinnä kaipaan kriittisiä parannusvinkkejä, siis vinkkejä jotka olisi välttämätöntä muuttaa, esim. turvallisuuden takia. En välttämättä kaipaa "tavalla x olisit säästänyt 3 riviä koodia!". Toki niitäkin kommentteja voi laittaa jos se omaa egoa pönkittää.
login.php:

<?php
     // Otetaan yhteys kantaan
     require($_SERVER['DOCUMENT_ROOT']."/config/db_config.php");
     $connection = @mysql_connect($db_host, $db_user, $db_password) or die("error connecting");
     mysql_select_db($db_name, $connection);

    //Tarkastaa jos login- cookie on saatavilla
    if(isset($_COOKIE['ID_my_site']))

    //jos on, sinut logataan sisään ja ohjataan members-sivulle
    {
    $username = $_COOKIE['ID_my_site'];
    $pass = $_COOKIE['Key_my_site'];
    $check = mysql_query("SELECT * FROM users WHERE username = '$username'")or die(mysql_error());
    while($info = mysql_fetch_array( $check ))
    {
    if ($pass != $info['password'])
    {
    }
    else
    {
    header("Location: members.php");

    }
    }
    }

    // jos kirjautumis- lomake on lähetetty
    if (isset($_POST['submit'])) {

    // varmistaa että kentät on täytetty
    if(!$_POST['username'] | !$_POST['pass']) {
    die('You did not fill in a required field.');
    }
    // tarkastaa tietokannasta
    if (!get_magic_quotes_gpc()) {
    $_POST['email'] = addslashes($_POST['email']);
    }
    $check = mysql_query("SELECT * FROM users WHERE username = '".$_POST['username']."'")or die(mysql_error());

    // Antaa virheen jos ei löydy
    $check2 = mysql_num_rows($check);
    if ($check2 == 0) {
    die('That user does not exist in our database.

    <a href=add.php>Click Here to Register</a>');
    }
    while($info = mysql_fetch_array( $check ))
    {
    $_POST['pass'] = stripslashes($_POST['pass']);
    $info['password'] = stripslashes($info['password']);
    $_POST['pass'] = md5($_POST['pass']);

    // Antaa virheen jos syötetään väärä salasana
    if ($_POST['pass'] != $info['password']) {
    die('Incorrect password, please try again.');
    }

    else
    {

    // jos login onnistuu, tehdään cookie
    $_POST['username'] = stripslashes($_POST['username']);
    $hour = time() + 3600;
    setcookie(ID_my_site, $_POST['username'], $hour);
    setcookie(Key_my_site, $_POST['pass'], $hour);

    // Ja sitten vaan ohjataan members-sivulle
    header("Location: members.php");
    }
    }
    }
    else
    {

    // tulostetaan jos ei olla kirjauduttu sisään
?>
    <form action="<?php echo $_SERVER['PHP_SELF']?>" method="post">
    <table border="0">
    <tr><td colspan=2><h1>Login</h1></td></tr>
    <tr><td>Username:</td><td>
    <input type="text" name="username" maxlength="40">
    </td></tr>
    <tr><td>Password:</td><td>
    <input type="password" name="pass" maxlength="50">
    </td></tr>
    <tr><td colspan="2" align="right">
    <input type="submit" name="submit" value="Login">
    </td></tr>
    </table>
    </form>
<?php
    }

?>

logout.php

<?php
    $past = time() - 100;
    setcookie(ID_my_site, gone, $past);
    setcookie(Key_my_site, gone, $past);
    header("Location: login.php");
?

members.php

<?php
     // Otetaan yhteys kantaan
     require($_SERVER['DOCUMENT_ROOT']."/config/db_config.php");
     $connection = @mysql_connect($db_host, $db_user, $db_password) or die("error connecting");
     mysql_select_db($db_name, $connection);

    // tarkastaa cookiet loggautumisesta
    if(isset($_COOKIE['ID_my_site']))
    {
    $username = $_COOKIE['ID_my_site'];
    $pass = $_COOKIE['Key_my_site'];
    $check = mysql_query("SELECT * FROM users WHERE username = '$username'")or die(mysql_error());
    while($info = mysql_fetch_array( $check ))
    {

    // jos cookiessa on väärä salasana, ohjataa login-sivulle
    if ($pass != $info['password'])
    { header("Location: login.php");
    }

    // muuten näytetään admin-alue
    else
    {
    echo "Admin Area<p>";
    echo "Your Content<p>";
    echo "<a href=logout.php>Logout</a>";
    }
    }
    }
    else

    // Jos ei cookieta, näytetään login
    {
    header("Location: login.php");
    }
?>

register.php

<?php
     // Otetaan yhteys kantaan
     require($_SERVER['DOCUMENT_ROOT']."/config/db_config.php");
     $connection = @mysql_connect($db_host, $db_user, $db_password) or die("error connecting");
     mysql_select_db($db_name, $connection);

    // Koodi suoritetaan vain jos Submit:tia painettu
    if (isset($_POST['submit'])) {

    // Tällä varmistetaan ettei kentät ole tyhjiä
    if (!$_POST['username'] | !$_POST['pass'] | !$_POST['pass2'] ) {
    die('You did not complete all of the required fields');
    }

    // Tarkastaa onko käyttäjänimi jo käytössä
    if (!get_magic_quotes_gpc()) {
    $_POST['username'] = addslashes($_POST['username']);
    }
    $usercheck = $_POST['username'];
    $check = mysql_query("SELECT username FROM users WHERE username = '$usercheck'")
    or die(mysql_error());
    $check2 = mysql_num_rows($check);

    //...jos on, antaa virheen
    if ($check2 != 0) {
    die('Sorry, the username '.$_POST['username'].' is already in use.');
    }

    // Tämä varmistaa että syötetyt salasananat ovat samat
    if ($_POST['pass'] != $_POST['pass2']) {
    die('Your passwords did not match. ');
    }

    // Sitten suojaamme salasanan
    $_POST['pass'] = md5($_POST['pass']);
    if (!get_magic_quotes_gpc()) {
    $_POST['pass'] = addslashes($_POST['pass']);
    $_POST['username'] = addslashes($_POST['username']);
    }

    // Sitten vaan tiedot kantaan
    $insert = "INSERT INTO users (username, password)
    VALUES ('".$_POST['username']."', '".$_POST['pass']."')";
    $add_member = mysql_query($insert);
?>

    <!-- Ja sitten kerrotaan jos rekisteröityminen onnistui -->
    <h1>Registered</h1>
    <p>Thank you, you have registered - you may now login</a>.</p>
<?php
    }
    else
    {
?>

    <!-- Nämä käyttäjä näkee ennen rekisteröitymistä-->
    <form action="<?php echo $_SERVER['PHP_SELF']; ?>" method="post">
    <table border="0">
    <tr><td>Username:</td><td>
    <input type="text" name="username" maxlength="60">
    </td></tr>
    <tr><td>Password:</td><td>
    <input type="password" name="pass" maxlength="10">
    </td></tr>
    <tr><td>Confirm Password:</td><td>
    <input type="password" name="pass2" maxlength="10">
    </td></tr>
    <tr><th colspan=2><input type="submit" name="submit" value="Register"></th></tr> </table>
    </form>

<?php
}
?>

Siinä ne, kommentteja. Olen aloittelija, mutta ehkä se tästä.. :)

Miksi MYSQL? Miks ei pelkkä PHP?
"/config/db_config.php Ja mitä jos jollai ei oo tota?

Niin, no siis en nyt halunnut välttämättä hirveästi laittaa tätä miksikään koodivinkiksi vaan lähinnä toivoisin että tosta korjattaisiin pahimmat turvallisuusriskit. Tuo db_config.php sisältää MySQL serverin yhteystiedot, niin ettei niitä tarvitse joka skriptiin kirjoittaa erikseen.
Ja voisin kysyä että miksi ei MySQL jos se kerran on käytettävissä?

Koodin periaate on hyvä: tunnus ja salasana ovat evästeessä, ja niiden oikeellisuus tarkistetaan joka sivun alussa.

Kenoviivojen lisäys ja poisto kaipaavat vielä tarkistusta. Selkeyden vuoksi kenoviivat kannattaa minusta lisätä tarpeen tullen heti alkajaisiksi kaikkiin lomakkeen ja evästeen kautta tulleisiin muuttujiin. Tämän jälkeen muuttujat voi sellaisinaan yhdistää SQL-kyselyyn, ja jos muuttujan arvo täytyy tulostaa, kenoviivat täytyy aina poistaa.

Paras tapa tarkistaa kenoviivojen toiminta on kokeilla lisätä kaikkiin syötteisiin kenoviivoja ja lainausmerkkejä ja heittomerkkejä. Jos tietokantaan ilmestyy oikeita tietoja ja sivulle tulostuu oikeita tietoja, niin silloin kenoviivojen käsittely on oikein.

Koodissasi on monia toimintaan vaikuttamattomia asioita, jotka minä tekisin toisin, mutta niihin en pyynnöstäsi puutu. Suosittelen kuitenkin sisentämään koodin - silloin sekä sinun että meidän muiden on paljon helpompi ymmärtää sitä.

Itse ehkä käyttäisin ennemmin PHP:n istunnonhallintaa kuin evästeisiin tallennettuja käyttäjätietoja. Silloin ei tarvitsisi siirtää salasanaa (tosin md5-koodattua, mutta kuitenkin) verkon yli joka sivunlatauksella.

Tämä ei liity varsinaisesti tietoturvaan, mutta on minusta myöskin tärkeä asia: on hyvä, että olet laittanut koodisi ilmoittamaan tarkasti, menikö kirjautuessa pieleen salasana vai käyttäjätunnus. Aivan liian usein törmää sivuihin, jotka vain ilmoittavat tylysti: "kirjautuminen ei onnistunut!" Tuollainen tarkka "virheellinen salasana"-ilmoitus ei edellytä kovin montaa näppäimen lisäpainallusta, eikä käsittääkseni vaaranna tietoturvaakaan. Tuohon logout-skriptiisi voisit kanssa jollain keinolla väsätä vastaavan ilmoituksen, joka kertoisi uloskirjauksen onnistuneen.

Istunnot ftw. Md5 ei koodaa salasanaa vaan laskee siitä tiivisteen/summan/hashin merkkijonolle. Takertuakseni tuohon "käyttäjätunnus väärin" - kun käyttäjätunnus menee väärin niin silloin on "annettu tunnus, jota ei ole olemassa." Kun salasana menee väärin niin on "annettu virheelliset käyttäjätiedot." Kun jokin menee vikaan, niin "kirjautuminen ei onnistunut." Ei se nyt tietoturvaa huononna vaikka sanoisikin, että salasana on väärin, kohan ite tykkään noin.

Teuvo Töhvelö kirjoitti:

Itse ehkä käyttäisin ennemmin PHP:n istunnonhallintaa kuin evästeisiin tallennettuja käyttäjätietoja. Silloin ei tarvitsisi siirtää salasanaa (tosin md5-koodattua, mutta kuitenkin) verkon yli joka sivunlatauksella.

Jos yhteyttä ei ole kryptattu, niin sniffereiden kannalta on yhdentekevää, lähetetäänkö salasana md5-summana vai selvätekstinä. Tunnuksen kaappaaminen on molemmissa tapauksissa yhtä helppoa. PHP:n oman istunnonhallinnan käyttämä istuntotunnusmenetelmä on hieman varmempi, joskaan ei täysin varma.

Teuvo Töhvelö kirjoitti:

Tämä ei liity varsinaisesti tietoturvaan, mutta on minusta myöskin tärkeä asia: on hyvä, että olet laittanut koodisi ilmoittamaan tarkasti, menikö kirjautuessa pieleen salasana vai käyttäjätunnus. Aivan liian usein törmää sivuihin, jotka vain ilmoittavat tylysti: "kirjautuminen ei onnistunut!" Tuollainen tarkka "virheellinen salasana"-ilmoitus ei edellytä kovin montaa näppäimen lisäpainallusta, eikä käsittääkseni vaaranna tietoturvaakaan.

Sanakirjatyyppistä hyökkäystä käytettäessä urakka nopeutuu huomattavasti, jos tietää onko kokeiltava käyttäjätunnus olemassa vai ei. Eri asia sitten, kuinka todennäköisenä tällaista hyökkäystä pitää. Weppifoorumilla se on melko epätodennäköinen, koska tavan tallaajan tunnuksella ei tee tuon taivaallista ja ylläpitäjien tunnukset saa selville foorumia selaamalla.

EDIT: Pikkulisäyksiä

Jepjep. Urakka nopeutunee kaiken tyyppisillä hyökkäyksillä. On sitten tapauskohtaista onko tuo tietoturvan huonominen huomattavan suurta.

Salasana on kryptattu, hyvä. Tosin pelkästään MD5:llä. Itse laittaisin vielä reilulla suolalla sen. 5 merkkinen MD5 (ja lyhyempi) normaaliteksti murtuu kotikoneilla hyvin nopeasti, ja se hash on näppärä napata kenen tahansa joka pääsee katsomaan koneesi selaimen asetuksia.

Jossain määrin tietoturvaa heikentää tyylisi käsitellä virheitä. Serverin erikoistilanteissa mysql_error voi antaa yllättävänkin paljon tietoa järjestelmästä. Parempi olisi virittää virhe (trigger_error), ja laittaa virheiden tulostus vaikka tiedostoon.

Et tarkista kaikista syötteistä onko get_magic_quotes päällä, vaan käytät monissa paikoissa suoraan POST ym muuttujia, jotka perustuvat siihen että magic quotes on päällä. Jos murtautuja saa jotenkin poistettua palvelimelta ko. ominaisuuden käytöstä (tai voihan se vaihtua ihan omia aikojaankin, mikäli palvelin ei ole täysin muutoksilta hallitussa ylläpidossa), jolloin sql-injektiolle ei ole mitään esteitä. Paras olisi käyttää kaikille syötteille vaikka tälläistä funktiota:

function smart_addslashes($array) {
  if (get_magic_quotes_gpc() == true)
    return $array;
  else {
    if (is_string($array)) {
      return addslashes($array);
    }
    elseif (is_array($array)) {
      foreach ($array as $key => $value) {
        $newarray[$key] = smart_addslashes($value);
      }
      return $newarray;
    }
  }
}

Joka siis lisää syötteeseen (kelpaa myös arrayt) slashit, jos magic quotes ei ole päällä, muuten jättää syötteen ennalleen. Koodi toimii tällöin samalla tavalla oli mq päällä tai ei. Toivottavasti tuo koodi toimii, kaivoin sen naftaliinista, vähän muokkasin sitä (oli yksi virhe vissiinkin) enkä jaksanut nyt testata sen toimivuutta.