[skrypt] bezpieczne logowanie Opcje

[kiamil]

Link do dema: DEMO
Link do kodu: CODE

Zapraszam do oceniania, autoryzacja oparta na jednej zmiennej sesyjnej, IMHO unikalny skrypt. Potrzebwałbym też porady dotyczące bezpieczeństwa.

[Babcia@Stefa]

Rejestracja i logowanie zwykłe jak widać, może jakiś graficzny captha?

http://www.di3d.boo.pl/logowanie/code.php?file=config.php (IMG:http://forum.php.pl/style_emoticons/default/axesmiley.png)

$cfg['salt'] = '_babia_stefa_lubi_ogorki_xD';

Co to ma znaczyć?! (IMG:http://forum.php.pl/style_emoticons/default/blinksmiley.gif)

Kod jest brzydki (brak odstępów), mógłbyś próbować zrobić go obiektowo skoro "rozdajesz" go publicznie.


Ale "$cfg['salt'] = '_babia_stefa_lubi_ogorki_xD';" mnie zaskoczyło, siedziałem zamurowany przed komputerem na dobre pare minut ... (IMG:http://forum.php.pl/style_emoticons/default/axesmiley.png)

Pozdrawiam, WebNuLL...

Ten post edytował Babcia@Stefa 23.12.2008, 22:41:58

[kiamil]

Odstępów nigdy nie robiłem, nie jestem tak zaawansowany by obiektówką walić ;]
Z tym saltem to jakoś tak spontanicznie mi przyszło, nie ma tam żadnego innego znaczenia..

[SzamanGN]

Dla mnie ciekawe rozwiązanie

[kiamil]

Coś więcej? Porady dot. bezpieczeństwa? Jeszcze zrobie ochrone przed BF w wersji 0.2 (IMG:http://forum.php.pl/style_emoticons/default/winksmiley.jpg)

[pyro]

BF?

[.radex]

BF?

bruteforce.

[pyro]

a sorry (IMG:http://forum.php.pl/style_emoticons/default/tongue.gif)

[erix]

Odstępów nigdy nie robiłem

Pora zacząć. (IMG:http://forum.php.pl/style_emoticons/default/tongue.gif)

Kod

global $_SESSION; global $cfg;

Blah, na chorobę globalizować $_SESSION? Skoro piszesz jakieś skrypty zdobądź chociaż podstawy... (IMG:http://forum.php.pl/style_emoticons/default/dry.gif)

Kod

return sha1(time().$nick);

IMHO trochę za słaby ten token... Masz funkcje generowania liczb pseudolosowych, /dev/urandom, uniqid" title="Zobacz w manualu PHP" target="_manual...
Poza tym, nie wiem, po co podajesz użytkownikowi token...

Kod

<? session_start(); session_regenerate_id(); ?>

A na szto tak?

Kod

md5('adm'.$cfg['salt'])

MD5 za bezpieczne nie jest.

Ogólnie: kod nieczytelny, nic rewelacyjnego.

[Babcia@Stefa]

Odstępów nigdy nie robiłem, nie jestem tak zaawansowany by obiektówką walić ;]
Z tym saltem to jakoś tak spontanicznie mi przyszło, nie ma tam żadnego innego znaczenia..

Wiesz, popatrz na mój nick na forum i porównaj z saltem ;]

Pozdrawiam, WebNuLL

[.radex]

Pora zacząć. (IMG:http://forum.php.pl/style_emoticons/default/tongue.gif)

No właśnie (IMG:http://forum.php.pl/style_emoticons/default/smile.gif)

Porównaj sobie to:

[PHP] pobierz, plaintext 
<?
include('mysql.php');
 
$cfg['mysql']['alias']='log_';
$cfg['salt'] = '_babia_stefa_lubi_ogorki_xD';
 
function new_otoken($nick) {
global $_SESSION; global $cfg;
$ot = otoken($nick);
mysql_query('UPDATE '.$cfg['mysql']['alias'].'users SET u_otoken="'.$ot.'" WHERE u_nick="'.$nick.'"');
$_SESSION['otoken']=$ot;
}
function otoken($nick) {
return sha1(time().$nick);
}
?>
[PHP] pobierz, plaintext 

z tym:

[PHP] pobierz, plaintext 
<?php
   include 'mysql.php';
   
   $cfg['mysql']['alias'] = 'log_';
   $cfg['salt'] = '_babia_stefa_lubi_ogorki_xD';
   
   function new_otoken($nick)
   {
      global $_SESSION;
      global $cfg;
      
      $ot = otoken($nick);
      
      mysql_query('UPDATE ' . $cfg['mysql']['alias'] . 'users SET u_otoken="' . $ot . '" WHERE u_nick="' . $nick . '"');
      
      $_SESSION['otoken'] = $ot;
   }
   
   function otoken($nick)
   {
      return sha1(time() . $nick);
   }
   ?>
[PHP] pobierz, plaintext 

Nie wiem jak Ty, ale mi się wydaje, że moja wersja jest dużo bardziej czytelna. Ot, bardzo dobry nawyk.

Natomiast mam dwie bardzo ważne uwagi:
1. Zaprzestań stosować globali (po konkrety do wyszukiwarki (IMG:http://forum.php.pl/style_emoticons/default/tongue.gif) )
2. Rozpocznij stosować mysql_real_escape_string

globale są złe, a "escapować" ciągi znaków należy przed wysłaniem do bazy danych (dla zabezpieczenia przed sql injection), czyli jeszcze raz:

[PHP] pobierz, plaintext 
<?php
   include 'mysql.php';
   
   $cfg['mysql']['alias'] = 'log_';
   $cfg['salt'] = '_babia_stefa_lubi_ogorki_xD';
   
   function new_otoken($nick)
   {
      global $cfg; // to należy wywalić...
      
      $nick = mysql_real_escape_string($nick);
      
      $ot = otoken($nick);
      
      mysql_query('UPDATE ' . $cfg['mysql']['alias'] . 'users SET u_otoken="' . $ot . '" WHERE u_nick="' . $nick . '"');
      
      $_SESSION['otoken'] = $ot;
   }
   
   function otoken($nick)
   {
      return sha1(time() . $nick);
   }
   ?>
[PHP] pobierz, plaintext 

PS. Stosuj komentarze w swoim kodzie, bo jak będziesz musiał edytować kod, którego nie ruszałeś od kilku miesięcy to nie będziesz wiedział "o co tu chodzi?" (IMG:http://forum.php.pl/style_emoticons/default/winksmiley.jpg)

[Kildyt]

Skupię się na kodzie.

Jak już napisał @.radex: tabulacja!

[PHP] pobierz, plaintext 
<?php
global $_SESSION; global $cfg;
?>
[PHP] pobierz, plaintext 

To jest bez sensu.

[PHP] pobierz, plaintext 
<?php
mysql_query('UPDATE '.$cfg['mysql']['alias'].'users SET u_otoken="'.$ot.'" WHERE u_nick="'.$nick.'"');
?>
[PHP] pobierz, plaintext 

Nie wiem dlaczego (pewnie ze względu na przejrzystość), ale przyjęło się, że zapytania do bazy danych podajemy w cudzysłowach.

[PHP] pobierz, plaintext 
<?php
mysql_query("UPDATE `$cfg['mysql']['alias']users` SET `u_otoken`='$ot' WHERE `u_nick`='$nick'");
?>
[PHP] pobierz, plaintext 

Czy nie ładniej?

W login.php osobiście najpierw zadeklarowałbym zmienną $login i w niej przefiltrował zmienną globalną $_POST ($_POST['nick']) i przez całość skryptu operowałbym na niej. Strzeżonego Pan Bóg strzeże. (IMG:http://forum.php.pl/style_emoticons/default/winksmiley.jpg)

Ogólnie kod jest nieprzejrzysty i to jest jego wielkim minusem.

[l0ud]

przyjęło się, że zapytania do bazy danych podajemy w cudzysłowach.

...chyba w skryptach pokroju jportal. Co jak co, ale to jest bzdurą.

Czy nie ładniej?

Chociażby ze względu na kolorowanie - nie (IMG:http://forum.php.pl/style_emoticons/default/tongue.gif)

[Kildyt]

...chyba w skryptach pokroju jportal. Co jak co, ale to jest bzdurą.

Chociażby ze względu na kolorowanie - nie (IMG:http://forum.php.pl/style_emoticons/default/tongue.gif)

Wiesz, każdy ma swoje przyzwyczajenia. (IMG:http://forum.php.pl/style_emoticons/default/tongue.gif)
Przepraszam bardzo, ale jportal to kiedyś był ho, ho. (IMG:http://forum.php.pl/style_emoticons/default/smile.gif) Może teraz się z niego śmiejecie, ale kilka lat temu to był niezły CMS, a o tym świadczy chociażby ilość stron na nim postawiona. Pamiętam, że kiedyś nawet robiłem themy do niego. ^^ Ahh, te czasu. (IMG:http://forum.php.pl/style_emoticons/default/smile.gif)

[Crozin]

[PHP] pobierz, plaintext 
<?php
abdef('UPDATE %s SET u_token="%s" WHERE u_nick ="%s";', USERS_TABLE, $token, $nick);
?>
[PHP] pobierz, plaintext 

A tak to chyba jeszcze ładniej?

• Brak jakiejs obsługi błędów (dla MySQL przykładowo)
• @ - wywal to coś
• Dlaczego to co jest liczbą (np. w kolumnie u_access z tabeli użytkowników) przekazujesz jako string?
• Po co w nazwach kolumn przedrostki (u_nick, u_access)
• htmlspecialchars() używa się przed wyświetleniem danych, a nie ich wprowadzeniem do bazy danych
• Wprowadź dwa pola dla hasła (wpisz, powtórz) - nie łatwo się pomylić przy rejestracji z hasłem

Jak napisali wcześniej - nic specialnego. Niechludnie napisane, mało elastyczne.

[bim2]

Jak już to

[PHP] pobierz, plaintext 
<?php
echo "UPDATE {$cfg['mysql']['alias']}users";
?>
[PHP] pobierz, plaintext 

(IMG:http://forum.php.pl/style_emoticons/default/smile.gif)
jPortal dla mnie zawsze będzie czymś ważnym, na nim nauczyłem się podstaw i myślenia (no i z pomocą kilku osób). Narzekacie tak na niego, ale to było coś. (IMG:http://forum.php.pl/style_emoticons/default/tongue.gif)

Co do skryptu, zawsze wyobrażałem sobie, że będzie to coś na zasadzie:

[PHP] pobierz, plaintext 
<?php
require_once 'register.module.php';
if(rgWantLogIn())
{
rgLogIn($_POST[RG_LOGIN], $_POST[RG_PASS]);
}
if(rgIsLogin())
{
echo 'Witaj '.rgGetName(); //wiem wiem, klasa byłaby lepsza, ale mówię już o samych podstawach
} else {
rgShowLoginForm();
}
?>
[PHP] pobierz, plaintext 

Wtedy łatwo zaimplementowac to w róznych innych systemach ;]

[Crozin]

@bim2: jak już to obiekt, nie klasa

Można by się pokusić o zrobienie jakiejś podstawowej rzeczy w stylu oddzielenia warstwy logiki od widoku.

[lukaszkkk]

Dodam tylko, ze zgodnie z podrecznikiem php, jesli uzywa sie funkcji

mysql_real_escape_string() to wczesniej trzeba za pomoca funkcji stripslashes() usunac ze zmiennych ukosniki, dodane

automatycznie przez derektywe magic_quotes_gpc.

Ladnie jest to pakazane w linku ponizej. 

php.net