[php] System logowania, Czy jest w miarę bezpieczny? Opcje

[MGreg]

Witam. Stworzyłem system logowania i chciałbym, abyście sprawdzili, czy jest w miarę szczelny. Dane do logowania osadziłem w skrypcie na sztywno tj. login admin, hasło 123 - dane te będą pobierane z bazy danych oczywiście z zabezpieczeniem przed sql injection. Teraz jednak chodzi mi o sam system logowania.

login.php

[PHP] pobierz, plaintext 
<?php
  session_start();
  session_regenerate_id();
  
  #wylgowywanie
  function logout(){
      if($_SESSION['logged']==1){
          $_SESSION['logged']=0;
          return true;
      }    
  }
  
  if($_GET['a']=='logout'){
      if(logout()==true){
          echo "Zostałeś wylogowany";
      }
      else{
          echo "nie byłeś zalogowany";
      }
  }
  
  #logowanie
  function logged(){
      if($_SESSION['logged']==0){    
          if($_GET['a']=='login'){        
              if($_POST['login'] and $_POST['password']){            
                  if($_POST['login']=='admin' and $_POST['password']=='123'){
                      $_SESSION['logged']=1;
                      $_SESSION['ip'] = $_SERVER['REMOTE_ADDR'];
                      if($_SESSION['logged']==1){
                      return true;
                      }                
                  }
                  else{
                      return false;                
                  }            
              }        
          }
      }
      else{
          if($_SESSION['ip']==$_SERVER['REMOTE_ADDR'] and $_SESSION['logged']==1){
              return true;
          }    
      }
  }
  
  if(logged()==false){
      include('template/login_form.php');
  }
  else{
      echo "zalogowany";
      echo "<a href=login.php?a=logout>Wyloguj</a>";
  }
  ?>
[PHP] pobierz, plaintext 

template/login_form.php

[HTML] pobierz, plaintext 
<form action="login.php?a=login" method=POST>
      Login:<input type=text name=login>
      Hasło:<input type=password name=password>
      <input type=submit value="zaloguj">
  </form>
[HTML] pobierz, plaintext 

Jak widać na początku skryptu generowany jest losowy identyfikator sesji, w przypadku gdyby został w jakiś sposób skradziony dodałem jeszcze porównywanie IP, jeśli się nie zgadza logged() zwraca false.

Ten post edytował MGreg 6.01.2009, 20:23:38

[Axexis]

Za każdym odświeżeniem strony zmieniasz ID sesji. Nie kasują Ci się za każdym razem dane? d;

Dwa. Co jeśli ktoś ma proxy zmieniające IP co odświeżenie strony?
Trzy. Przy wylogowaniu wystarczy unset($_SESSION['logged']);.
Cztery. Jak sprawdzasz czy funkcja została wykonana wystarczy if(funkcja()).

[MGreg]

Zmieniłem trochę cały system, jeśli ktoś może to prosiłbym posprawdzać czy jest w miarę bezpieczny. Jeśli występuje jakaś dziura to prosiłbym byście mi napisali. Byście wszystkie sesje dokładnie widzieli dodałem wyświetlanie całej tablicy $_SESSION.

[PHP] pobierz, plaintext 
<?php
 ob_start();
 session_start();
 $NowTime = time(); #obecny czas
 $SessTime = 3600; #czas trwania sesji
 $LoggedTime = 300; #czas trwania sesji podczas bezczynności
 
 $login_form = "<form action=\"login.php?a=login\" method=POST>
     Login:<input type=text name=login>
     Hasło:<input type=password name=password>
     <input type=submit value=\"zaloguj\">
 </form>";
 
 #sprawdza czy jest zarejestrowana sesja
 function check_session(){
     if(isset($_SESSION['logged'])){
         return true;
     }
     else{        
         return false;
     }
 }
 
 #sprawdza dane z formularza
 function check_form(){
     if($_POST['login'] and $_POST['password']){
         if(addslashes($_POST['login'])=='admin' and addslashes($_POST['password'])=='123'){
             return true;
         }
         else{
             return false;
         }
     }
 }
 
 #zakańcza sesję
 function end_session(){
     if(check_session()){
         unset($_SESSION['logged']);
         unset($_SESSION['last_refresh']);
         unset($_SESSION['session_start']);
         unset($_SESSION['user_login']);
     }
 }
 
 #kontynuacja sesji
 function continue_session(){
     global $NowTime;
     global $LoggedTime;
     global $SessTime;
     session_regenerate_id();
     
     #sprawdza czy nie upłynął czas trwania sesji, lub czy sesja jest zbyt długo bezczynna
     if(isset($_SESSION['last_refresh'])){
         if(($_SESSION['last_refresh']+$LoggedTime < $NowTime) or ($_SESSION['session_start']+$SessTime < $NowTime)){
             return false;            
         }
         else{
             $_SESSION['last_refresh']= $NowTime;
         }
         return true;
     }
     else{
         $_SESSION['last_refresh']= $NowTime;        
         return true;
     }
 }
 
 #właściwa funckja zarządzania sesjami
 function session(){
     global $NowTime;
     global $login_form;
     
     if(check_session()){
         if(continue_session()){
             echo "Zalogowany jako:<b> ".$_SESSION['user_login']."</b> <a href=login.php?a=logout>wyloguj</a>";
             return true;
         }
         else{
             end_session();
             header("location:login.php");
         }
     }
     
     else{        
         if(check_form()){
             $_SESSION['logged']=1;
             $_SESSION['session_start'] = $NowTime;
             $_SESSION['user_login'] = $_POST['login'];
             header("location:login.php");
         }
         echo $login_form;
     }
 }
 
 if($_GET['a']=="logout"){
     end_session();
     header("location:login.php");
 }
 
 session();
 
 echo '<pre>';
 print_r( $_SESSION);
  echo '</pre>';  
 ?>
[PHP] pobierz, plaintext 

Ten post edytował MGreg 15.01.2009, 18:32:23

[Albitos]

Masz podatność na ataki typu session fixation, poza tym ustaw sobie na początku skryptu error_reporting(E_ALL)...

[MGreg]

Skorzystałem za artykułu na stronie i na początku w.w skryptu dodałem

[PHP] pobierz, plaintext 
<?php
$regenerateReq = 10;
 
if(isset($_SESSION['req'])){
    $_SESSION['req']++;
}
else{
    $_SESSION['req']=1;
}
 
if($_SESSION['req']>$regenerateReq){
 session_regenerate_id(true);
 $_SESSION['req']=1;
}
?>
[PHP] pobierz, plaintext 

Chyba session fixtation mamy już z głowy (IMG:http://forum.php.pl/style_emoticons/default/smile.gif)

Cały skrypt wygląda teraz tak:

[PHP] pobierz, plaintext 
<?php
ob_start();
session_start();
error_reporting(E_ALL);
 
$NowTime = time(); #obecny czas
$SessTime = 3600; #czas trwania sesji
$LoggedTime = 300; #czas trwania sesji podczas bezczynności
$regenerateReq = 10; #ilość rządań po której zostanie zregenerowany identyfikator sesji
 
if(isset($_SESSION['req'])){
    $_SESSION['req']++;
}
else{
    $_SESSION['req']=1;
}
 
if($_SESSION['req']>$regenerateReq){
 session_regenerate_id(true);
 $_SESSION['req']=1;
}
 
$login_form = "<form action=\"login.php?a=login\" method=POST>
    Login:<input type=text name=login>
    Hasło:<input type=password name=password>
    <input type=submit value=\"zaloguj\">
</form>";
 
#sprawdza czy jest zarejestrowana sesja
function check_session(){
    if(isset($_SESSION['logged'])){
        return true;
    }
    else{        
        return false;
    }
}
 
#sprawdza dane z formularza
function check_form(){
    if(isset($_POST['login']) and isset($_POST['password'])){
        if(addslashes($_POST['login'])=='admin' and addslashes($_POST['password'])=='123'){
            return true;
        }
        else{
            return false;
        }
    }
}
 
#zakańcza sesję
function end_session(){
    if(check_session()){
        unset($_SESSION['logged']);
        unset($_SESSION['last_refresh']);
        unset($_SESSION['session_start']);
        unset($_SESSION['user_login']);
    }
}
 
#kontynuacja sesji
function continue_session(){
    global $NowTime;
    global $LoggedTime;
    global $SessTime;    
    
    #sprawdza czy nie upłynął czas trwania sesji, lub czy sesja jest zbyt długo bezczynna
    if(isset($_SESSION['last_refresh'])){
        if(($_SESSION['last_refresh']+$LoggedTime < $NowTime) or ($_SESSION['session_start']+$SessTime < $NowTime)){
            return false;            
        }
        else{
            $_SESSION['last_refresh']= $NowTime;
        }
        return true;
    }
    else{
        $_SESSION['last_refresh']= $NowTime;        
        return true;
    }
}
 
#właściwa funckja zarządzania sesjami
function session(){
    global $NowTime;
    global $login_form;
    
    if(check_session()){
        if(continue_session()){
            echo "Zalogowany jako:<b> ".$_SESSION['user_login']."</b> <a href=login.php?a=logout>wyloguj</a>";
            return true;
        }
        else{
            end_session();
            header("location:login.php");
        }
    }
    
    else{        
        if(check_form()){
            $_SESSION['logged']=1;
            $_SESSION['session_start'] = $NowTime;
            $_SESSION['user_login'] = $_POST['login'];
            header("location:login.php");
        }
        echo $login_form;
    }
}
if(isset($_GET['a'])=="logout"){
    end_session();
    header("location:login.php");
}
 
session();
 
echo '<pre>';
print_r( $_SESSION);
 echo '</pre>';  
?>
[PHP] pobierz, plaintext 

Jakie mamy zabezpieczenia?
- regeneracja identyfikatora sesji po 10 rządaniach
- zakończenie sesji po 30 minutach
+ jeśli można to uznać za zabezpieczenie
- zakończenie sesji po 5 minutach bezczynności

Ten post edytował MGreg 15.01.2009, 19:07:32

[rzymek01]

nie potrzebnie w 42 dajesz addslashes

[MGreg]

Hehe, dodałem ten addslashes by później nikt nie mówił o braku filtrowania danych. Akurat ten fragment kodu jest na sztywno, w jego miejscu pojawi się pobieranie danych z bazy danych wraz z mysql_real_esape_string();